Re: [PATCH 2/2] ALSA: usb-audio: Manage auto-pm of all bundled interfaces

[Macpaul Lin <macpaul.lin@mediatek.com>]

On Wed, 2020-06-03 at 17:37 +0200, Takashi Iwai wrote:
> Currently USB-audio driver manages the auto-pm of the primary
> interface although a card may consist of multiple interfaces.
> This may leave the secondary and other interfaces left running
> unnecessarily after the auto-suspend.
> 
> This patch allows the driver managing the auto-pm of all bundled
> interfaces per card.  The chip->pm_intf field is extended as
> chip->intf[] to contain the array of assigned interfaces, and the
> runtime-PM is performed to all those interfaces.
> 
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  sound/usb/card.c     | 35 ++++++++++++++++++++++++++++++-----
>  sound/usb/usbaudio.h |  4 +++-
>  2 files changed, 33 insertions(+), 6 deletions(-)
> 
> diff --git a/sound/usb/card.c b/sound/usb/card.c
> index 359f7a04be1c..f648587d2342 100644
> --- a/sound/usb/card.c
> +++ b/sound/usb/card.c
> @@ -634,7 +634,6 @@ static int usb_audio_probe(struct usb_interface *intf,
>  								   id, &chip);
>  					if (err < 0)
>  						goto __error;
> -					chip->pm_intf = intf;
>  					break;
>  				} else if (vid[i] != -1 || pid[i] != -1) {
>  					dev_info(&dev->dev,
> @@ -651,6 +650,13 @@ static int usb_audio_probe(struct usb_interface *intf,
>  			goto __error;
>  		}
>  	}
> +
> +	if (chip->num_interfaces >= MAX_CARD_INTERFACES) {
> +		dev_info(&dev->dev, "Too many interfaces assigned to the single USB-audio card\n");
> +		err = -EINVAL;
> +		goto __error;
> +	}
> +
>  	dev_set_drvdata(&dev->dev, chip);
>  
>  	/*
> @@ -703,6 +709,7 @@ static int usb_audio_probe(struct usb_interface *intf,
>  	}
>  
>  	usb_chip[chip->index] = chip;
> +	chip->intf[chip->num_interfaces] = intf;
>  	chip->num_interfaces++;
>  	usb_set_intfdata(intf, chip);
>  	atomic_dec(&chip->active);
> @@ -818,19 +825,37 @@ void snd_usb_unlock_shutdown(struct snd_usb_audio *chip)
>  
>  int snd_usb_autoresume(struct snd_usb_audio *chip)
>  {
> +	int i, err;
> +
>  	if (atomic_read(&chip->shutdown))
>  		return -EIO;
> -	if (atomic_inc_return(&chip->active) == 1)
> -		return usb_autopm_get_interface(chip->pm_intf);
> +	if (atomic_inc_return(&chip->active) != 1)
> +		return 0;
> +
> +	for (i = 0; i < chip->num_interfaces; i++) {
> +		err = usb_autopm_get_interface(chip->intf[i]);
> +		if (err < 0) {
> +			/* rollback */
> +			while (--i >= 0)
> +				usb_autopm_put_interface(chip->intf[i]);
> +			atomic_dec(&chip->active))
> +			return err;
> +		}
> +	}
>  	return 0;
>  }
>  
>  void snd_usb_autosuspend(struct snd_usb_audio *chip)
>  {
> +	int i;
> +
>  	if (atomic_read(&chip->shutdown))
>  		return;
> -	if (atomic_dec_and_test(&chip->active))
> -		usb_autopm_put_interface(chip->pm_intf);
> +	if (!atomic_dec_and_test(&chip->active))
> +		return;
> +
> +	for (i = 0; i < chip->num_interfaces; i++)
> +		usb_autopm_put_interface(chip->intf[i]);
>  }
>  
>  static int usb_audio_suspend(struct usb_interface *intf, pm_message_t message)
> diff --git a/sound/usb/usbaudio.h b/sound/usb/usbaudio.h
> index e0ebfb25fbd5..b91c4c0807ec 100644
> --- a/sound/usb/usbaudio.h
> +++ b/sound/usb/usbaudio.h
> @@ -19,11 +19,13 @@
>  struct media_device;
>  struct media_intf_devnode;
>  
> +#define MAX_CARD_INTERFACES	16
> +
>  struct snd_usb_audio {
>  	int index;
>  	struct usb_device *dev;
>  	struct snd_card *card;
> -	struct usb_interface *pm_intf;
> +	struct usb_interface *intf[MAX_CARD_INTERFACES];
>  	u32 usb_id;
>  	struct mutex mutex;
>  	unsigned int system_suspend;

Tested-by: Macpaul Lin <macpaul.lin@mediatek.com>

--
Thanks!
Macpaul Lin