On Tue, 2020-06-02 at 19:53 +0800, Macpaul Lin wrote: > This patch fix incorrect power state changed by usb_audio_suspend() > when CONFIG_PM is enabled. > > After receiving suspend PM message with auto flag, usb_audio_suspend() > change card's power state to SNDRV_CTL_POWER_D3hot. Only when the other > resume PM message with auto flag can change power state to > SNDRV_CTL_POWER_D0 in __usb_audio_resume(). > > However, when system is not under auto suspend, resume PM message with > auto flag might not be able to receive on time which cause the power > state was incorrect. At this time, if a player starts to play sound, > will cause snd_usb_pcm_open() to access the card and setup_hw_info() will > resume the card. > > But even the card is back to work and all function normal, the power > state is still in SNDRV_CTL_POWER_D3hot. Which cause the infinite loop > happened in snd_power_wait() to check the power state. Thus the > successive setting ioctl cannot be passed to card. > > Hence we suggest to change power state to SNDRV_CTL_POWER_D0 when card > has been resumed successfully. > > Signed-off-by: Macpaul Lin <macpaul.lin@xxxxxxxxxxxx> > --- > sound/usb/pcm.c | 11 +++++++++++linux-usb@xxxxxxxxxxxxxxx, > 1 file changed, 11 insertions(+) > > diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c > index a4e4064..d667ecb 100644 > --- a/sound/usb/pcm.c > +++ b/sound/usb/pcm.c > @@ -1322,6 +1322,17 @@ static int setup_hw_info(struct snd_pcm_runtime *runtime, struct snd_usb_substre > if (err < 0) > return err; > > + /* fix incorrect power state when resuming by open and later ioctls */ > + if (IS_ENABLED(CONFIG_PM) && > + snd_power_get_state(subs->stream->chip->card) > + == SNDRV_CTL_POWER_D3hot) { > + /* set these variables for power state correction */ > + subs->stream->chip->autosuspended = 0; > + subs->stream->chip->num_suspended_intf = 1; > + dev_info(&subs->dev->dev, > + "change power state from D3hot to D0\n"); > + } > + > return snd_usb_autoresume(subs->stream->chip); > } > The issue was found on kernel 4.14 (android tree). The test is to add debug log in sound/core/init.c to check if the power state is SNDRV_CTL_POWER_D3hot. diff --git a/sound/core/init.c b/sound/core/init.c index b02a997..a0bee76 100644 --- a/sound/core/init.c +++ b/sound/core/init.c @@ -1011,6 +1011,8 @@ int snd_power_wait(struct snd_card *card, unsigned int power_state) if (snd_power_get_state(card) == power_state) break; set_current_state(TASK_UNINTERRUPTIBLE); + pr_info("%s snd_power_get_state[%x]\n", __func__, + snd_power_get_state(card)); schedule_timeout(30 * HZ); } remove_wait_queue(&card->power_sleep, &wait); After applied a work around by forcing the power state, pcm related ioctl and parameter settings can be set to usb sound card correctly. Otherwise a infinite loop will happened in snd_power_wait(). Here is the origin work around for verifying this power state issue on kernel 4.14. diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index 933adcd7af81..9acd50dd7155 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -1274,6 +1274,16 @@ static int setup_hw_info(struct snd_pcm_runtime *runtime, struct snd_usb_substre if (err < 0) return err; + /* avoid incorrect power state when executing IOCTL */ + if (IS_ENABLED(CONFIG_PM) && + snd_power_get_state(subs->stream->chip->card) + == SNDRV_CTL_POWER_D3hot) { + dev_info(&subs->dev->dev, + "change power state from D3hot to D0\n"); + snd_power_change_state(subs->stream->chip->card, + SNDRV_CTL_POWER_D0); + } + param_period_time_if_needed = SNDRV_PCM_HW_PARAM_PERIOD_TIME; if (subs->speed == USB_SPEED_FULL) /* full speed devices have fixed data packet interval */ However, the patch I've send is meant to make sure the power state will be corrected before snd_usb_autoresume(), It should be adapt to kernel 4.14 and later. Thanks. Macpaul Lin