Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <d0559a60-a4c1-19a9-39c5-18a5e2e460ca@gmail.com>
Date:   Wed, 29 Jun 2022 00:21:21 +0900
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux aarch64; rv:91.0) Gecko/20100101
 Thunderbird/91.10.0
Subject: Re: [PATCH] HID: amd_sfh: Ignore uninitialized device
Content-Language: en-US
To:     "Limonciello, Mario" <mario.limonciello@amd.com>
Cc:     Basavaraj Natikar <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
References: <20220626081339.7243-1-akihiko.odaki@gmail.com>
 <b17f4e85-8a35-65ab-34de-0978bdc4cfea@amd.com>
 <8c95e3d7-cff6-c56e-e197-fdd0911d81aa@gmail.com>
 <3ff059d8-2d89-6ea1-209b-3c8855561acb@amd.com>
From:   Akihiko Odaki <akihiko.odaki@gmail.com>
In-Reply-To: <3ff059d8-2d89-6ea1-209b-3c8855561acb@amd.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

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.

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