lkml.org 
[lkml]   [2020]   [Jun]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.14 025/190] ALSA: usb-audio: Fix inconsistent card PM state after resume
    Date
    From: Takashi Iwai <tiwai@suse.de>

    commit 862b2509d157c629dd26d7ac6c6cdbf043d332eb upstream.

    When a USB-audio interface gets runtime-suspended via auto-pm feature,
    the driver suspends all functionality and increment
    chip->num_suspended_intf. Later on, when the system gets suspended to
    S3, the driver increments chip->num_suspended_intf again, skips the
    device changes, and sets the card power state to
    SNDRV_CTL_POWER_D3hot. In return, when the system gets resumed from
    S3, the resume callback decrements chip->num_suspended_intf. Since
    this refcount is still not zero (it's been runtime-suspended), the
    whole resume is skipped. But there is a small pitfall here.

    The problem is that the driver doesn't restore the card power state
    after this resume call, leaving it as SNDRV_CTL_POWER_D3hot. So,
    even after the system resume finishes, the card instance still appears
    as if it were system-suspended, and this confuses many ioctl accesses
    that are blocked unexpectedly.

    In details, we have two issues behind the scene: one is that the card
    power state is changed only when the refcount becomes zero, and
    another is that the prior auto-suspend check is kept in a boolean
    flag. Although the latter problem is almost negligible since the
    auto-pm feature is imposed only on the primary interface, but this can
    be a potential problem on the devices with multiple interfaces.

    This patch addresses those issues by the following:

    - Replace chip->autosuspended boolean flag with chip->system_suspend
    counter

    - At the first system-suspend, chip->num_suspended_intf is recorded to
    chip->system_suspend

    - At system-resume, the card power state is restored when the
    chip->num_suspended_intf refcount reaches to chip->system_suspend,
    i.e. the state returns to the auto-suspended

    Also, the patch fixes yet another hidden problem by the code
    refactoring along with the fixes above: namely, when some resume
    procedure failed, the driver left chip->num_suspended_intf that was
    already decreased, and it might lead to the refcount unbalance.
    In the new code, the refcount decrement is done after the whole resume
    procedure, and the problem is avoided as well.

    Fixes: 0662292aec05 ("ALSA: usb-audio: Handle normal and auto-suspend equally")
    Reported-and-tested-by: Macpaul Lin <macpaul.lin@mediatek.com>
    Cc: <stable@vger.kernel.org>
    Link: https://lore.kernel.org/r/20200603153709.6293-1-tiwai@suse.de
    Signed-off-by: Takashi Iwai <tiwai@suse.de>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

    ---
    sound/usb/card.c | 20 +++++++++++++-------
    sound/usb/usbaudio.h | 2 +-
    2 files changed, 14 insertions(+), 8 deletions(-)

    --- a/sound/usb/card.c
    +++ b/sound/usb/card.c
    @@ -768,9 +768,6 @@ static int usb_audio_suspend(struct usb_
    if (chip == (void *)-1L)
    return 0;

    - chip->autosuspended = !!PMSG_IS_AUTO(message);
    - if (!chip->autosuspended)
    - snd_power_change_state(chip->card, SNDRV_CTL_POWER_D3hot);
    if (!chip->num_suspended_intf++) {
    list_for_each_entry(as, &chip->pcm_list, list) {
    snd_pcm_suspend_all(as->pcm);
    @@ -783,6 +780,11 @@ static int usb_audio_suspend(struct usb_
    snd_usb_mixer_suspend(mixer);
    }

    + if (!PMSG_IS_AUTO(message) && !chip->system_suspend) {
    + snd_power_change_state(chip->card, SNDRV_CTL_POWER_D3hot);
    + chip->system_suspend = chip->num_suspended_intf;
    + }
    +
    return 0;
    }

    @@ -795,10 +797,11 @@ static int __usb_audio_resume(struct usb

    if (chip == (void *)-1L)
    return 0;
    - if (--chip->num_suspended_intf)
    - return 0;

    atomic_inc(&chip->active); /* avoid autopm */
    + if (chip->num_suspended_intf > 1)
    + goto out;
    +
    /*
    * ALSA leaves material resumption to user space
    * we just notify and restart the mixers
    @@ -813,9 +816,12 @@ static int __usb_audio_resume(struct usb
    snd_usbmidi_resume(p);
    }

    - if (!chip->autosuspended)
    + out:
    + if (chip->num_suspended_intf == chip->system_suspend) {
    snd_power_change_state(chip->card, SNDRV_CTL_POWER_D0);
    - chip->autosuspended = 0;
    + chip->system_suspend = 0;
    + }
    + chip->num_suspended_intf--;

    err_out:
    atomic_dec(&chip->active); /* allow autopm after this point */
    --- a/sound/usb/usbaudio.h
    +++ b/sound/usb/usbaudio.h
    @@ -37,7 +37,7 @@ struct snd_usb_audio {
    struct usb_interface *pm_intf;
    u32 usb_id;
    struct mutex mutex;
    - unsigned int autosuspended:1;
    + unsigned int system_suspend;
    atomic_t active;
    atomic_t shutdown;
    atomic_t usage_count;

    \
     
     \ /
      Last update: 2020-06-19 16:46    [W:4.267 / U:0.088 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site