[alsa-devel] [PATCH v2] add native DSD support for XMOS based DACs

[Takashi Iwai]

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 at 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
>