linux-kernel - Re: Sarien/Dorset device: After system resumed from suspend, 3.5m jack is still shown as detected when unplugged during suspend

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Sat, 17 Sep 2022 06:29:17 +0000
From:   "Arava, Jairaj" <jairaj.arava@...el.com>
To:     Takashi Iwai <tiwai@...e.de>
CC:     Pshou <pshou@...ltek.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Nujella, Sathyanarayana" <sathyanarayana.nujella@...el.com>,
        "Prabhu, Swarna" <swarna.prabhu@...el.com>,
        "Afzal, Naeem M" <naeem.m.afzal@...el.com>,
        "Hsu, Shui-Wen" <swhsu4021@...ltek.com>,
        "Perati, RK" <rk.perati@...el.com>,
        "Mandri, Padmashree" <padmashree.mandri@...el.com>,
        Kailang <kailang@...ltek.com>
Subject: Re: Sarien/Dorset device: After system resumed from suspend, 3.5m
 jack is still shown as detected when unplugged during suspend 

Hi Takashi,

After reading the comment#17, we started looking from codec end and we see the unsol event handler is not invoked to handle it & thought it is mandate for the codec.
 However as said, by will look further deep into it.

@Pshou,
Want to get undersrand from the codec end further. How can we expect the codec to behave for such events during suspend & resume?

Thanks,
Jai

> On Sep 16, 2022, at 10:51 PM, Takashi Iwai <tiwai@...e.de> wrote:
> 
> On Fri, 16 Sep 2022 21:50:51 +0200,
> Arava, Jairaj wrote:
>> 
>> Hi Takashi,
>> 
>> This discussion is regarding the bug https://bugzilla.kernel.org/show_bug.cgi?id=215297.
> 
> Did you read the comment 17?  You haven't analyzed yet properly what
> was going wrong.
> 
>> As we know https://lore.kernel.org/alsa-devel/20210310112809.9215-3-tiwai@suse.de/ is causing the 3.5mm headset jack detection issue during system suspend resume. So after reverting this patch the issue is not seen since the unsol event is handled without this patch.
>> 
>> Since we  see in https://elixir.bootlin.com/linux/latest/source/sound/pci/hda/patch_realtek.c#L992 the codec driver has  snd_hda_jack_unsol_event as the unsol event handler. Hence after reverting your patch, from https://elixir.bootlin.com/linux/latest/source/sound/pci/hda/hda_bind.c#L56  the driver is calling https://elixir.bootlin.com/linux/latest/source/sound/pci/hda/hda_jack.c#L713 to handle the unsolv event.
>> 
>> Based on above observance, seems snd_hda_jack_unsol_event is mandatory for the intel platform Realtek codec driver to handle such unsolv enets.  Hence, @Pshou added the PM flag check as the patch was tested by Nvidia.
> 
> If the hardware can't detect the jack at the resume by some reason,
> that must be the cause, and not because it doesn't handle the unsol
> event during the suspend.
> 
> Please try to debug more deeply at first.
> 
> 
> thanks,
> 
> Takashi
> 
>> 
>> Thanks,
>> Jai 
>> -----Original Message-----
>> From: Takashi Iwai <tiwai@...e.de> 
>> Sent: Friday, September 16, 2022 3:08 AM
>> To: Pshou <pshou@...ltek.com>
>> Cc: linux-kernel@...r.kernel.org; Takashi Iwai <tiwai@...e.de>; Arava, Jairaj <jairaj.arava@...el.com>; Nujella, Sathyanarayana <sathyanarayana.nujella@...el.com>; Prabhu, Swarna <swarna.prabhu@...el.com>; Afzal, Naeem M <naeem.m.afzal@...el.com>; Hsu, Shui-Wen <swhsu4021@...ltek.com>; Perati, RK <rk.perati@...el.com>; Mandri, Padmashree <padmashree.mandri@...el.com>; Kailang <kailang@...ltek.com>
>> Subject: Re: Sarien/Dorset device: After system resumed from suspend, 3.5m jack is still shown as detected when unplugged during suspend 
>> 
>>> On Fri, 16 Sep 2022 07:34:38 +0200,
>>> Pshou wrote:
>>> 
>>> 
>>> Hi Takashi Iwai:
>>> 
>>> Can you help me update this PATCH file?
>>> 
>>> Check if ignore unsol events duing system suspend/resume and NVIDIA 
>>> chip in hda_codec_unsol_event().
>>> 
>>> Signed-off-by:PeiSen Hou<pshou@...ltek.com>
>>> 
>>> Signed-off-by: Jairaj Arava <jairaj.arava@...el.com>
>>> 
>>> diff --git a/sound/pci/hda/hda_bind.c b/sound/pci/hda/hda_bind.c
>>> 
>>> index 1a868dd9dc4b..75560ff6eb83 100644
>>> 
>>> --- a/sound/pci/hda/hda_bind.c
>>> 
>>> +++ b/sound/pci/hda/hda_bind.c
>>> 
>>> @@ -50,7 +50,8 @@ static void hda_codec_unsol_event(struct hdac_device 
>>> *dev, unsigned int ev)
>>> 
>>>       /* ignore unsol events during system suspend/resume */
>>> 
>>>      if (codec->core.dev.power.power_state.event != PM_EVENT_ON)
>>> 
>>> -               return;
>>> 
>>> +              if (codec->core.vendor_id == PCI_VENDOR_ID_NVIDIA)
>>> 
>>> +                      return;
>>> 
>>>       if (codec->patch_ops.unsol_event)
>>> 
>>>              codec->patch_ops.unsol_event(codec, ev);
>> 
>> Hmm, this doesn't look safe.  We also want to avoid the unsol event handling during the PM state transition, too.  So, if any, this should be allowed only at PM_EVENT_SUSPEND or PM_EVENT_HIBERNATE.
>> 
>> Also, checking the codec vendor ID here is no good way.  We may add a new flag for the special behavior (either allowing the unsol handling or prohibiting).
>> 
>> But, from your patch, I don't see any reason *why* this has to be changed in that way.  Could you give more backgrounds?
>> 
>> 
>> thanks,
>> 
>> Takashi
>>