lkml.org 
[lkml]   [2022]   [Jul]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH] HID: amd_sfh: Ignore uninitialized device
Date
[Public]



> -----Original Message-----
> From: Akihiko Odaki <akihiko.odaki@gmail.com>
> Sent: Tuesday, June 28, 2022 10:21
> To: Limonciello, Mario <Mario.Limonciello@amd.com>
> Cc: Natikar, Basavaraj <Basavaraj.Natikar@amd.com>; Jiri Kosina
> <jikos@kernel.org>; Benjamin Tissoires <benjamin.tissoires@redhat.com>;
> linux-input@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] HID: amd_sfh: Ignore uninitialized device
>
> On 2022/06/29 0:14, Limonciello, Mario wrote:
> > On 6/28/2022 10:11, Akihiko Odaki wrote:
> >> On 2022/06/28 23:42, Limonciello, Mario wrote:
> >>> On 6/26/2022 03:13, Akihiko Odaki wrote:
> >>>> Lenovo ThinkPad C13 Yoga has AMD Sensor Fusion Hub, but it is not used
> >>>> because Chrome OS EC Sensor Hub is used instead. The system therefore
> >>>> never loads the firmware for MP2 and MP2 does not work. It results in
> >>>> AMD_P2C_MSG3 register to have -1 as its value.
> >>>>
> >>>> Without this change, the driver interprets the value as it supports all
> >>>> sensor types and exposes them, which confuses a userspace program,
> >>>> iio-sensor-proxy, and makes it to use the non-functioning sensors
> >>>> instead of functioning sensors exposed via Chrome OS EC Sensor Hub.
> >>>>
> >>>> Check the version bits included in AMD_P2C_MSG3 register and ignore the
> >>>> device if all of the bits are set.
> >>>>
> >>>
> >>> Have you already confirmed this failure happens in 5.19-rc1 or later
> >>> as well?  I would think that b5d7f43e97dabfa04a4be5ff027ce7da119332be
> >>> should have fixed it.
> >>
> >> Yes. I confirmed it with 78ca55889a549a9a194c6ec666836329b774ab6d.
> >>
> >
> > Thanks for confirming.
> >
> >> b5d7f43e97dabfa04a4be5ff027ce7da119332be deals with the case where it
> >> advertises v2 but it doesn't in my case.
> >
> > In your case it actually goes down the v1 ops path then right?
>
> Yes, but I doubt even that is correct in this case. I guess the v1
> protocol would have a value 1 for acs in mp2_select_ops(), but it is 15
> in this case. It would be nice if you confirm that hypothesis.

I had some offline conversation about this, and it's probably best to include a way to
block amd_sfh from loading on the DMI of Chromebooks.

What you're finding in these registers is garbage without firmware loaded and isn't
deterministic.

Could you come up with a patch that blocks from the DMI data? I don't think it should
be just your specific chromebook, but maybe one of the strings that indicates "any
chromebook" and has a "Google Inc" or something in it.

>
> Regards,
> Akihiko Odaki
>
> >
> > Basavaraj - is discovery unique to v2?  Or does it also exist for v1?
> > If it also exists for v1 I think that's a cleaner solution.
> >
> >>
> >> Regards,
> >> Akihiko Odaki
> >>
> >>>
> >>>> Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
> >>>> ---
> >>>>   drivers/hid/amd-sfh-hid/amd_sfh_pcie.c | 4 ++++
> >>>>   1 file changed, 4 insertions(+)
> >>>>
> >>>> diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
> >>>> b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
> >>>> index dadc491bbf6b..4137e5da77ad 100644
> >>>> --- a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
> >>>> +++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
> >>>> @@ -271,6 +271,8 @@ static void mp2_select_ops(struct amd_mp2_dev
> >>>> *privdata)
> >>>>       case V2_STATUS:
> >>>>           privdata->mp2_ops = &amd_sfh_ops_v2;
> >>>>           break;
> >>>> +    case 15:
> >>>> +        break;
> >>>>       default:
> >>>>           privdata->mp2_ops = &amd_sfh_ops;
> >>>>           break;
> >>>> @@ -317,6 +319,8 @@ static int amd_mp2_pci_probe(struct pci_dev
> >>>> *pdev, const struct pci_device_id *i
> >>>>           return -ENOMEM;
> >>>>       mp2_select_ops(privdata);
> >>>> +    if (!privdata->mp2_ops)
> >>>> +        return -ENODEV;
> >>>>       rc = amd_sfh_irq_init(privdata);
> >>>>       if (rc) {
> >>>
> >>
> >
\
 
 \ /
  Last update: 2022-07-07 20:19    [W:0.065 / U:0.404 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site