linux-kernel - RE: [PATCH] HID: HID-rmi - ignore to rmi_hid_read

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]
Message-ID: <PH0PR03MB58481BA39CBC1D1D4E0BB487A3629@PH0PR03MB5848.namprd03.prod.outlook.com>
Date:   Tue, 9 Aug 2022 03:11:31 +0000
From:   Marge Yang <Marge.Yang@...synaptics.com>
To:     Hans de Goede <hdegoede@...hat.com>,
        margeyang <marge.yang@...aptics.corp-partner.google.com>,
        "dmitry.torokhov@...il.com" <dmitry.torokhov@...il.com>,
        "linux-input@...r.kernel.org" <linux-input@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "benjamin.tissoires@...hat.com" <benjamin.tissoires@...hat.com>
CC:     Derek Cheng <derek.cheng@...synaptics.com>,
        Vincent Huang <Vincent.huang@...synaptics.com>
Subject: RE: [PATCH] HID: HID-rmi - ignore to rmi_hid_read_block after system
 resumes.

Hi Hans,
	Thanks for your suggestion.
I have used the rmi_transport_dev struct  (inside the xport) instead of making it a global variable.

Thanks
Marge Yang

-----Original Message-----
From: Hans de Goede <hdegoede@...hat.com> 
Sent: Monday, August 8, 2022 5:42 PM
To: Marge Yang <Marge.Yang@...synaptics.com>; margeyang <marge.yang@...aptics.corp-partner.google.com>; dmitry.torokhov@...il.com; linux-input@...r.kernel.org; linux-kernel@...r.kernel.org; benjamin.tissoires@...hat.com
Cc: derek.cheng@...synaptcs.com; Vincent Huang <Vincent.huang@...synaptics.com>
Subject: Re: [PATCH] HID: HID-rmi - ignore to rmi_hid_read_block after system resumes.

CAUTION: Email originated externally, do not click links or open attachments unless you recognize the sender and know the content is safe.


Hi,

On 8/8/22 11:36, Marge Yang wrote:
> Hi Hans,
>       For Synaptics FW design, the interrupt GPIO will be pulled down after RMI driver reads this command(Report ID:0x0A).
> "Dark resume" test case on Chromebook device will detect another interrupt (not finger data) during the process of resume function.
> "Dark resume" test case will fail.
> Hence, this issue should happen on RMI mode of all Synaptics devices

Ok I see.

In that case this change is ok, but please store the ignoreonce flag inside the rmi_transport_dev struct  (inside the xport) instead of making it a global variable.

Regards,

Hans



>
> Thanks
> Marge Yang
>
> -----Original Message-----
> From: Hans de Goede <hdegoede@...hat.com>
> Sent: Monday, August 8, 2022 5:16 PM
> To: margeyang <marge.yang@...aptics.corp-partner.google.com>; 
> dmitry.torokhov@...il.com; linux-input@...r.kernel.org; 
> linux-kernel@...r.kernel.org; benjamin.tissoires@...hat.com
> Cc: Marge Yang <Marge.Yang@...synaptics.com>; 
> derek.cheng@...synaptcs.com; Vincent Huang 
> <Vincent.huang@...synaptics.com>
> Subject: Re: [PATCH] HID: HID-rmi - ignore to rmi_hid_read_block after system resumes.
>
> CAUTION: Email originated externally, do not click links or open attachments unless you recognize the sender and know the content is safe.
>
>
> Hi,
>
> On 8/8/22 11:07, margeyang wrote:
>> From: Marge Yang <marge.yang@...aptics.corp-partner.google.com>
>>
>> The interrupt GPIO will be pulled down once after RMI driver reads 
>> this command(Report ID:0x0A).
>> It will cause "Dark resume test fail" for chromebook device.
>> Hence, TP driver will ignore rmi_hid_read_block function once after 
>> system resumes.
>
> This sounds like it is an issue in one specific touchpad model, yet you are changing the code to ignore the first readblock call on resume on *all* models ?
>
> Regards,
>
> Hans
>
>
>>
>> Signed-off-by: Marge
>> Yang<marge.yang@...aptics.corp-partner.google.com>
>> ---
>>  drivers/hid/hid-rmi.c | 16 +++++++++++++---
>>  1 file changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c index 
>> 311eee599ce9..236a38bfcf9a 100644
>> --- a/drivers/hid/hid-rmi.c
>> +++ b/drivers/hid/hid-rmi.c
>> @@ -101,7 +101,7 @@ struct rmi_data {  };
>>
>>  #define RMI_PAGE(addr) (((addr) >> 8) & 0xff)
>> -
>> +int ignoreonce;
>>  static int rmi_write_report(struct hid_device *hdev, u8 *report, int 
>> len);
>>
>>  /**
>> @@ -203,7 +203,13 @@ static int rmi_hid_read_block(struct rmi_transport_dev *xport, u16 addr,
>>               if (ret < 0)
>>                       goto exit;
>>       }
>> -
>> +     if (ignoreonce == 1) {
>> +             dev_err(&hdev->dev,
>> +                     "ignoreonce (%d)\n",
>> +                     ignoreonce);
>> +             ignoreonce = 0;
>> +             goto exit;
>> +     }
>>       for (retries = 5; retries > 0; retries--) {
>>               data->writeReport[0] = RMI_READ_ADDR_REPORT_ID;
>>               data->writeReport[1] = 0; /* old 1 byte read count */ 
>> @@
>> -468,8 +474,12 @@ static int rmi_post_resume(struct hid_device *hdev)
>>       ret = hid_hw_open(hdev);
>>       if (ret)
>>               return ret;
>> -
>> +     // Avoid to read rmi_hid_read_block once after system resumes.
>> +     // The interrupt will be pulled down
>> +     // after RMI Read command(Report ID:0x0A).
>> +     ignoreonce = 1;
>>       ret = rmi_reset_attn_mode(hdev);
>> +     ignoreonce = 0;
>>       if (ret)
>>               goto out;
>>
>