| Message ID | 1409919897-7040-1-git-send-email-gtmkramer@xs4all.nl (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Takashi Iwai |
| Headers | show |
At Fri, 5 Sep 2014 14:24:57 +0200, Jurgen Kramer wrote: > > Add quirks for XMOS based DACs for native DSD playback support using the new > DSD_U32_LE sample format. > > This version adds native DSD support for: > - iFi Audio micro iDSD/nano iDSD (they use the same prod. id) > - DIYINHK USB to I2S/DSD converter > > Changes from v1: > - use specific product id and alt setting per XMOS based device > > Signed-off-by: Jurgen Kramer <gtmkramer@xs4all.nl> > --- > sound/usb/quirks.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c > index 19a921e..5ae0536 100644 > --- a/sound/usb/quirks.c > +++ b/sound/usb/quirks.c > @@ -1174,5 +1174,20 @@ u64 snd_usb_interface_dsd_format_quirks(struct snd_usb_audio *chip, > } > } > > + /* XMOS based USB DACs */ > + if (le16_to_cpu(chip->dev->descriptor.idVendor) == 0x20b1) { > + switch (le16_to_cpu(chip->dev->descriptor.idProduct)) { > + /* iFi Audio micro/nano iDSD */ > + case 0x3008: > + if (fp->altsetting == 2) > + return SNDRV_PCM_FMTBIT_DSD_U32_LE; Missing break? > + /* DIYINHK DSD DXD 384kHz USB to I2S/DSD */ > + case 0x2009: > + if (fp->altsetting == 3) > + return SNDRV_PCM_FMTBIT_DSD_U32_LE; Ditto. > + default: > + return 0; > + } > + } > return 0; > } > -- > 1.9.3 Takashi
On Fri, 2014-09-05 at 15:36 +0200, Takashi Iwai wrote: > At Fri, 5 Sep 2014 14:24:57 +0200, > Jurgen Kramer wrote: > > > > Add quirks for XMOS based DACs for native DSD playback support using the new > > DSD_U32_LE sample format. > > > > This version adds native DSD support for: > > - iFi Audio micro iDSD/nano iDSD (they use the same prod. id) > > - DIYINHK USB to I2S/DSD converter > > > > Changes from v1: > > - use specific product id and alt setting per XMOS based device > > > > Signed-off-by: Jurgen Kramer <gtmkramer@xs4all.nl> > > --- > > sound/usb/quirks.c | 15 +++++++++++++++ > > 1 file changed, 15 insertions(+) > > > > diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c > > index 19a921e..5ae0536 100644 > > --- a/sound/usb/quirks.c > > +++ b/sound/usb/quirks.c > > @@ -1174,5 +1174,20 @@ u64 snd_usb_interface_dsd_format_quirks(struct snd_usb_audio *chip, > > } > > } > > > > + /* XMOS based USB DACs */ > > + if (le16_to_cpu(chip->dev->descriptor.idVendor) == 0x20b1) { > > + switch (le16_to_cpu(chip->dev->descriptor.idProduct)) { > > + /* iFi Audio micro/nano iDSD */ > > + case 0x3008: > > + if (fp->altsetting == 2) > > + return SNDRV_PCM_FMTBIT_DSD_U32_LE; > > Missing break? It is already returning. Where would the break go? > > > + /* DIYINHK DSD DXD 384kHz USB to I2S/DSD */ > > + case 0x2009: > > + if (fp->altsetting == 3) > > + return SNDRV_PCM_FMTBIT_DSD_U32_LE; > > Ditto. See above. > > > + default: > > + return 0; > > + } > > + } > > return 0; > > } > > -- Jurgen
At Fri, 05 Sep 2014 15:58:39 +0200, Jurgen Kramer wrote: > > On Fri, 2014-09-05 at 15:36 +0200, Takashi Iwai wrote: > > At Fri, 5 Sep 2014 14:24:57 +0200, > > Jurgen Kramer wrote: > > > > > > Add quirks for XMOS based DACs for native DSD playback support using the new > > > DSD_U32_LE sample format. > > > > > > This version adds native DSD support for: > > > - iFi Audio micro iDSD/nano iDSD (they use the same prod. id) > > > - DIYINHK USB to I2S/DSD converter > > > > > > Changes from v1: > > > - use specific product id and alt setting per XMOS based device > > > > > > Signed-off-by: Jurgen Kramer <gtmkramer@xs4all.nl> > > > --- > > > sound/usb/quirks.c | 15 +++++++++++++++ > > > 1 file changed, 15 insertions(+) > > > > > > diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c > > > index 19a921e..5ae0536 100644 > > > --- a/sound/usb/quirks.c > > > +++ b/sound/usb/quirks.c > > > @@ -1174,5 +1174,20 @@ u64 snd_usb_interface_dsd_format_quirks(struct snd_usb_audio *chip, > > > } > > > } > > > > > > + /* XMOS based USB DACs */ > > > + if (le16_to_cpu(chip->dev->descriptor.idVendor) == 0x20b1) { > > > + switch (le16_to_cpu(chip->dev->descriptor.idProduct)) { > > > + /* iFi Audio micro/nano iDSD */ > > > + case 0x3008: > > > + if (fp->altsetting == 2) > > > + return SNDRV_PCM_FMTBIT_DSD_U32_LE; > > > > Missing break? > It is already returning. Where would the break go? What if fp->altsetting == 3 with 20b1:3008 chip? Then it falls down to below, and return SNDRV_PCM_FMTBIT_DSD_U32_LE. Is this expected? > > > > > + /* DIYINHK DSD DXD 384kHz USB to I2S/DSD */ > > > + case 0x2009: > > > + if (fp->altsetting == 3) > > > + return SNDRV_PCM_FMTBIT_DSD_U32_LE; > > > > Ditto. > See above. If the fallthrough is intentional, you *must* put a comment it's really a fallthrough. Takashi > > > > > + default: > > > + return 0; > > > + } > > > + } > > > return 0; > > > } > > > -- > Jurgen >
On Fri, 2014-09-05 at 16:27 +0200, Takashi Iwai wrote: > At Fri, 05 Sep 2014 15:58:39 +0200, > Jurgen Kramer wrote: > > > > On Fri, 2014-09-05 at 15:36 +0200, Takashi Iwai wrote: > > > At Fri, 5 Sep 2014 14:24:57 +0200, > > > Jurgen Kramer wrote: > > > > > > > > Add quirks for XMOS based DACs for native DSD playback support using the new > > > > DSD_U32_LE sample format. > > > > > > > > This version adds native DSD support for: > > > > - iFi Audio micro iDSD/nano iDSD (they use the same prod. id) > > > > - DIYINHK USB to I2S/DSD converter > > > > > > > > Changes from v1: > > > > - use specific product id and alt setting per XMOS based device > > > > > > > > Signed-off-by: Jurgen Kramer <gtmkramer@xs4all.nl> > > > > --- > > > > sound/usb/quirks.c | 15 +++++++++++++++ > > > > 1 file changed, 15 insertions(+) > > > > > > > > diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c > > > > index 19a921e..5ae0536 100644 > > > > --- a/sound/usb/quirks.c > > > > +++ b/sound/usb/quirks.c > > > > @@ -1174,5 +1174,20 @@ u64 snd_usb_interface_dsd_format_quirks(struct snd_usb_audio *chip, > > > > } > > > > } > > > > > > > > + /* XMOS based USB DACs */ > > > > + if (le16_to_cpu(chip->dev->descriptor.idVendor) == 0x20b1) { > > > > + switch (le16_to_cpu(chip->dev->descriptor.idProduct)) { > > > > + /* iFi Audio micro/nano iDSD */ > > > > + case 0x3008: > > > > + if (fp->altsetting == 2) > > > > + return SNDRV_PCM_FMTBIT_DSD_U32_LE; > > > > > > Missing break? > > It is already returning. Where would the break go? > > What if fp->altsetting == 3 with 20b1:3008 chip? > Then it falls down to below, and return SNDRV_PCM_FMTBIT_DSD_U32_LE. > Is this expected? With fp->altsetting == 3 for 20b1:3008 I do not see it returning SNDRV_PCM_FMTBIT_DSD_U32_LE, it falls through to 'return 0'. However, would this be more appropriate/readable?: case 0x3008: if (fp->altsetting == 2) return SNDRV_PCM_FMTBIT_DSD_U32_LE; else break; or even: case 0x3008: if (fp->altsetting == 2) return SNDRV_PCM_FMTBIT_DSD_U32_LE; else return 0; > > > > > > > + /* DIYINHK DSD DXD 384kHz USB to I2S/DSD */ > > > > + case 0x2009: > > > > + if (fp->altsetting == 3) > > > > + return SNDRV_PCM_FMTBIT_DSD_U32_LE; > > > > > > Ditto. > > See above. > > If the fallthrough is intentional, you *must* put a comment it's > really a fallthrough. > > Jurgen
At Fri, 05 Sep 2014 16:37:13 +0200, Jurgen Kramer wrote: > > On Fri, 2014-09-05 at 16:27 +0200, Takashi Iwai wrote: > > At Fri, 05 Sep 2014 15:58:39 +0200, > > Jurgen Kramer wrote: > > > > > > On Fri, 2014-09-05 at 15:36 +0200, Takashi Iwai wrote: > > > > At Fri, 5 Sep 2014 14:24:57 +0200, > > > > Jurgen Kramer wrote: > > > > > > > > > > Add quirks for XMOS based DACs for native DSD playback support using the new > > > > > DSD_U32_LE sample format. > > > > > > > > > > This version adds native DSD support for: > > > > > - iFi Audio micro iDSD/nano iDSD (they use the same prod. id) > > > > > - DIYINHK USB to I2S/DSD converter > > > > > > > > > > Changes from v1: > > > > > - use specific product id and alt setting per XMOS based device > > > > > > > > > > Signed-off-by: Jurgen Kramer <gtmkramer@xs4all.nl> > > > > > --- > > > > > sound/usb/quirks.c | 15 +++++++++++++++ > > > > > 1 file changed, 15 insertions(+) > > > > > > > > > > diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c > > > > > index 19a921e..5ae0536 100644 > > > > > --- a/sound/usb/quirks.c > > > > > +++ b/sound/usb/quirks.c > > > > > @@ -1174,5 +1174,20 @@ u64 snd_usb_interface_dsd_format_quirks(struct snd_usb_audio *chip, > > > > > } > > > > > } > > > > > > > > > > + /* XMOS based USB DACs */ > > > > > + if (le16_to_cpu(chip->dev->descriptor.idVendor) == 0x20b1) { > > > > > + switch (le16_to_cpu(chip->dev->descriptor.idProduct)) { > > > > > + /* iFi Audio micro/nano iDSD */ > > > > > + case 0x3008: > > > > > + if (fp->altsetting == 2) > > > > > + return SNDRV_PCM_FMTBIT_DSD_U32_LE; > > > > > > > > Missing break? > > > It is already returning. Where would the break go? > > > > What if fp->altsetting == 3 with 20b1:3008 chip? > > Then it falls down to below, and return SNDRV_PCM_FMTBIT_DSD_U32_LE. > > Is this expected? > With fp->altsetting == 3 for 20b1:3008 I do not see it returning > SNDRV_PCM_FMTBIT_DSD_U32_LE, it falls through to 'return 0'. Well, read your C text book carefully again :) The tricky part of switch/case behavior is that it falls through to the next case, not going out of switch block. > However, would this be more appropriate/readable?: > > case 0x3008: > if (fp->altsetting == 2) > return SNDRV_PCM_FMTBIT_DSD_U32_LE; > else > break; Just put break. The standard form of switch/case is like: switch (cond) { case xxx: if (something) return abc; break; case yyy: if (another) return def; break; default: break; } return 0; Takashi > > or even: > > case 0x3008: > if (fp->altsetting == 2) > return SNDRV_PCM_FMTBIT_DSD_U32_LE; > else > return 0; > > > > > > > > > > + /* DIYINHK DSD DXD 384kHz USB to I2S/DSD */ > > > > > + case 0x2009: > > > > > + if (fp->altsetting == 3) > > > > > + return SNDRV_PCM_FMTBIT_DSD_U32_LE; > > > > > > > > Ditto. > > > See above. > > > > If the fallthrough is intentional, you *must* put a comment it's > > really a fallthrough. > > > > > Jurgen >
On Fri, 2014-09-05 at 16:51 +0200, Takashi Iwai wrote: > At Fri, 05 Sep 2014 16:37:13 +0200, > Jurgen Kramer wrote: > > > > On Fri, 2014-09-05 at 16:27 +0200, Takashi Iwai wrote: > > > At Fri, 05 Sep 2014 15:58:39 +0200, > > > Jurgen Kramer wrote: > > > > > > > > On Fri, 2014-09-05 at 15:36 +0200, Takashi Iwai wrote: > > > > > At Fri, 5 Sep 2014 14:24:57 +0200, > > > > > Jurgen Kramer wrote: > > > > > > > > > > > > Add quirks for XMOS based DACs for native DSD playback support using the new > > > > > > DSD_U32_LE sample format. > > > > > > > > > > > > This version adds native DSD support for: > > > > > > - iFi Audio micro iDSD/nano iDSD (they use the same prod. id) > > > > > > - DIYINHK USB to I2S/DSD converter > > > > > > > > > > > > Changes from v1: > > > > > > - use specific product id and alt setting per XMOS based device > > > > > > > > > > > > Signed-off-by: Jurgen Kramer <gtmkramer@xs4all.nl> > > > > > > --- > > > > > > sound/usb/quirks.c | 15 +++++++++++++++ > > > > > > 1 file changed, 15 insertions(+) > > > > > > > > > > > > diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c > > > > > > index 19a921e..5ae0536 100644 > > > > > > --- a/sound/usb/quirks.c > > > > > > +++ b/sound/usb/quirks.c > > > > > > @@ -1174,5 +1174,20 @@ u64 snd_usb_interface_dsd_format_quirks(struct snd_usb_audio *chip, > > > > > > } > > > > > > } > > > > > > > > > > > > + /* XMOS based USB DACs */ > > > > > > + if (le16_to_cpu(chip->dev->descriptor.idVendor) == 0x20b1) { > > > > > > + switch (le16_to_cpu(chip->dev->descriptor.idProduct)) { > > > > > > + /* iFi Audio micro/nano iDSD */ > > > > > > + case 0x3008: > > > > > > + if (fp->altsetting == 2) > > > > > > + return SNDRV_PCM_FMTBIT_DSD_U32_LE; > > > > > > > > > > Missing break? > > > > It is already returning. Where would the break go? > > > > > > What if fp->altsetting == 3 with 20b1:3008 chip? > > > Then it falls down to below, and return SNDRV_PCM_FMTBIT_DSD_U32_LE. > > > Is this expected? > > With fp->altsetting == 3 for 20b1:3008 I do not see it returning > > SNDRV_PCM_FMTBIT_DSD_U32_LE, it falls through to 'return 0'. > > Well, read your C text book carefully again :) Haha! > The tricky part of switch/case behavior is that it falls through to > the next case, not going out of switch block. > > > However, would this be more appropriate/readable?: > > > > case 0x3008: > > if (fp->altsetting == 2) > > return SNDRV_PCM_FMTBIT_DSD_U32_LE; > > else > > break; > > Just put break. The standard form of switch/case is like: > > switch (cond) { > case xxx: > if (something) > return abc; > break; > case yyy: > if (another) > return def; > break; > default: > break; > } > > return 0; Thanks, new patch coming up. Jurgen
Jurgen Kramer wrote: > + /* XMOS based USB DACs */ > + if (le16_to_cpu(chip->dev->descriptor.idVendor) == 0x20b1) { > + switch (le16_to_cpu(chip->dev->descriptor.idProduct)) { > + /* iFi Audio micro/nano iDSD */ > + case 0x3008: And you don't need to go through the descriptors; you can test chip->usb_id directly (see snd_usb_set_format_quirk()). Regards, Clemens
On Fri, 2014-09-05 at 17:26 +0200, Clemens Ladisch wrote: > Jurgen Kramer wrote: > > + /* XMOS based USB DACs */ > > + if (le16_to_cpu(chip->dev->descriptor.idVendor) == 0x20b1) { > > + switch (le16_to_cpu(chip->dev->descriptor.idProduct)) { > > + /* iFi Audio micro/nano iDSD */ > > + case 0x3008: > > And you don't need to go through the descriptors; you can test > chip->usb_id directly (see snd_usb_set_format_quirk()). Thanks, will do! Jurgen
diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c index 19a921e..5ae0536 100644 --- a/sound/usb/quirks.c +++ b/sound/usb/quirks.c @@ -1174,5 +1174,20 @@ u64 snd_usb_interface_dsd_format_quirks(struct snd_usb_audio *chip, } } + /* XMOS based USB DACs */ + if (le16_to_cpu(chip->dev->descriptor.idVendor) == 0x20b1) { + switch (le16_to_cpu(chip->dev->descriptor.idProduct)) { + /* iFi Audio micro/nano iDSD */ + case 0x3008: + if (fp->altsetting == 2) + return SNDRV_PCM_FMTBIT_DSD_U32_LE; + /* DIYINHK DSD DXD 384kHz USB to I2S/DSD */ + case 0x2009: + if (fp->altsetting == 3) + return SNDRV_PCM_FMTBIT_DSD_U32_LE; + default: + return 0; + } + } return 0; }
Add quirks for XMOS based DACs for native DSD playback support using the new DSD_U32_LE sample format. This version adds native DSD support for: - iFi Audio micro iDSD/nano iDSD (they use the same prod. id) - DIYINHK USB to I2S/DSD converter Changes from v1: - use specific product id and alt setting per XMOS based device Signed-off-by: Jurgen Kramer <gtmkramer@xs4all.nl> --- sound/usb/quirks.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)