Re: [PATCH 2/2] usb: Add mode select quirk for Denon/Marantz DACs

[Jurgen Kramer <gtmkramer@xs4all.nl>]

On Fri, 2014-11-28 at 14:51 +0100, Takashi Iwai wrote:
> At Fri, 28 Nov 2014 14:04:31 +0100,
> Jurgen Kramer wrote:
> > 
> > Denon/Marantz USB DACs need a specific vendor command to switch between PCM and
> > DSD mode. This patch adds a new quirk function to switch between the two modes
> > using the specific USB vendor command.
> > 
> > This patch applies to the following devices:
> > - Marantz SA-14S1
> > - Marantz HD-DAC1
> > 
> > Signed-off-by: Jurgen Kramer <gtmkramer@xs4all.nl>
> > ---
> >  sound/usb/pcm.c    |  5 +++++
> >  sound/usb/quirks.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> >  sound/usb/quirks.h |  3 +++
> >  3 files changed, 50 insertions(+)
> > 
> > diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
> > index c62a165..0d8aba5 100644
> > --- a/sound/usb/pcm.c
> > +++ b/sound/usb/pcm.c
> > @@ -482,6 +482,11 @@ static int set_format(struct snd_usb_substream *subs, struct audioformat *fmt)
> >  	/* set interface */
> >  	if (subs->interface != fmt->iface ||
> >  	    subs->altset_idx != fmt->altset_idx) {
> > +
> > +		err = snd_usb_select_mode_quirk(subs, fmt);
> > +		if (err < 0)
> > +			return -EIO;
> > +
> >  		err = usb_set_interface(dev, fmt->iface, fmt->altsetting);
> >  		if (err < 0) {
> >  			dev_err(&dev->dev,
> > diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c
> > index cc07388..40fb87f 100644
> > --- a/sound/usb/quirks.c
> > +++ b/sound/usb/quirks.c
> > @@ -1111,6 +1111,48 @@ void snd_usb_set_format_quirk(struct snd_usb_substream *subs,
> >  	}
> >  }
> >  
> > +
> > +/* Marantz/Denon USB DACs need a vendor cmd to switch
> > + * between PCM and native DSD mode */
> > +int snd_usb_select_mode_quirk(struct snd_usb_substream *subs,
> > +			      struct audioformat *fmt)
> > +{
> > +	struct usb_device *dev = subs->dev;
> > +	int err;
> > +
> > +	switch (subs->stream->chip->usb_id) {
> > +	case USB_ID(0x154e, 0x3005): /* Marantz HD-DAC1 */
> > +	case USB_ID(0x154e, 0x3006): /* Marantz SA-14S1 */
> > +
> > +		/* First switch to alt set 0, otherwise the mode switch cmd
> > +		 * will not be accepted by the DAC */
> > +		err = usb_set_interface(dev, fmt->iface, 0);
> > +		if (err < 0)
> > +			return err;
> > +
> > +		mdelay(20); /* Delay needed after setting the interface */
> > +
> > +		switch(fmt->altsetting) {
> 
> Put a space between switch and '('.
> Found in another patch, too.
> 
> > +		case 2: /* DSD mode requested */
> > +			err = snd_usb_ctl_msg(dev, usb_sndctrlpipe(dev, 0), 0,
> > +					      USB_DIR_OUT|USB_TYPE_VENDOR|USB_RECIP_INTERFACE,
> > +					      1, 1, NULL, 0);
> > +			if (err < 0)
> > +				return err;
> > +			break;
> > +		case 1: /* PCM mode requested */
> > +			err = snd_usb_ctl_msg(dev, usb_sndctrlpipe(dev, 0), 0,
> > +					      USB_DIR_OUT|USB_TYPE_VENDOR|USB_RECIP_INTERFACE,
> > +					      0, 1, NULL, 0);
> > +			if (err < 0)
> > +				return err;
> > +			break;
> 
> These two cases can combined one by passing fmt->altsetting-1 as the
> argument, right?
> 
Thanks, will fix the missing spaces and use the fmt->altsetting-1 suggestion in v3.

Jurgen