RE: [PATCH] HID: HID-rmi - ignore to rmi_hid_read_block after system resumes.

From: Marge Yang
Date: Mon Aug 08 2022 - 23:11:57 EST


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@xxxxxxxxxx>
Sent: Monday, August 8, 2022 5:42 PM
To: Marge Yang <Marge.Yang@xxxxxxxxxxxxxxxx>; margeyang <marge.yang@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx>; dmitry.torokhov@xxxxxxxxx; linux-input@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; benjamin.tissoires@xxxxxxxxxx
Cc: derek.cheng@xxxxxxxxxxxxxxx; Vincent Huang <Vincent.huang@xxxxxxxxxxxxxxxx>
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@xxxxxxxxxx>
> Sent: Monday, August 8, 2022 5:16 PM
> To: margeyang <marge.yang@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx>;
> dmitry.torokhov@xxxxxxxxx; linux-input@xxxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx; benjamin.tissoires@xxxxxxxxxx
> Cc: Marge Yang <Marge.Yang@xxxxxxxxxxxxxxxx>;
> derek.cheng@xxxxxxxxxxxxxxx; Vincent Huang
> <Vincent.huang@xxxxxxxxxxxxxxxx>
> 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@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx>
>>
>> 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@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx>
>> ---
>> 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;
>>
>