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

[Hans de Goede]

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;
>>
>