Re: [PATCH] HID: amd_sfh: Ignore uninitialized device

[Limonciello, Mario]

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?

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@xxxxxxxxx>
> > > ---
> > >   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) {