Re: [PATCH] ALSA: usb-audio: Fix quirks code is not called

[Takashi Sakamoto <o-takashi@sakamocchi.jp>]

Hi,

On Jul 18 2016 01:16, Kazuki Oikawa wrote:
> snd_usb_{set_interface,ctl_msg}_quirk checks chip->usb_id to need
> calling a quirks code. But existed code path that not calling
> dev_set_drvdata in usb_audio_probe.
> 
> Signed-off-by: Kazuki Oikawa <k@oikw.org>
> Fixes: 79289e24194a ("ALSA: usb-audio: Refer to chip->usb_id for quirks and MIDI creation")
> ---
>  sound/usb/card.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sound/usb/card.c b/sound/usb/card.c
> index 3fc63583a537..2d493501b7f6 100644
> --- a/sound/usb/card.c
> +++ b/sound/usb/card.c
> @@ -552,7 +552,6 @@ static int usb_audio_probe(struct usb_interface *intf,
>  				goto __error;
>  			}
>  			chip = usb_chip[i];
> -			dev_set_drvdata(&dev->dev, chip);
>  			atomic_inc(&chip->active); /* avoid autopm */
>  			break;
>  		}
> @@ -578,6 +577,7 @@ static int usb_audio_probe(struct usb_interface *intf,
>  			goto __error;
>  		}
>  	}
> +	dev_set_drvdata(&dev->dev, chip);
>  
>  	/*
>  	 * For devices with more than one control interface, we assume the

Reviewed-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Tested-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>

Let me describe supplemental comments.

This bug affects all of USB devices which take USB subsystem to call an
implementation of struct usb_driver.probe() in snd-usb-audio
(=usb_audio_probe()) just one time. In this case, dev_set_drvdata() is
not called at first call of usb_audio_probe(), then dev_get_drvdata()
returns NULL.

As a result, some quirks are not handled. snd_usb_set_interface_quirk()
and snd_usb_set_interface_quirk() returns immediately without enough
operations. This brings regressions to some models.

As long as I asked to Mr.Oikawa in this morning, below models are affected.
 * Playback Design
 * TEAC
 * Marantz
 * Denon
 * Zoom R16/24

I tested with EMU 0404 USB. This model has no quirks related to above
functions, but it brings just one call of usb_audio_probe(). Then,
without this patch, I can see dev_get_drvdata() always returns NULL
whole lifetime of usb device instance.
(But even with this patch, in a call of snd_usb_audio_free(),
dev_get_drvdata() returns NULL before a call of dev_set_devdata(NULL),
against my expectation. I don't know why...)

This patch should go for 4.8 merge window.


Regards

Takashi Sakamoto