Hi,
On 8/8/22 11:07, margeyang wrote:
> From: Marge Yang <[email protected]>
>
> 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<[email protected]>
> ---
> 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;
>
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
Thanks
Marge Yang
-----Original Message-----
From: Hans de Goede <[email protected]>
Sent: Monday, August 8, 2022 5:16 PM
To: margeyang <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]
Cc: Marge Yang <[email protected]>; [email protected]; Vincent Huang <[email protected]>
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 <[email protected]>
>
> 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<[email protected]>
> ---
> 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;
>
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 <[email protected]>
> Sent: Monday, August 8, 2022 5:16 PM
> To: margeyang <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]
> Cc: Marge Yang <[email protected]>; [email protected]; Vincent Huang <[email protected]>
> 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 <[email protected]>
>>
>> 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<[email protected]>
>> ---
>> 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;
>>
>
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 <[email protected]>
Sent: Monday, August 8, 2022 5:42 PM
To: Marge Yang <[email protected]>; margeyang <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]
Cc: [email protected]; Vincent Huang <[email protected]>
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 <[email protected]>
> Sent: Monday, August 8, 2022 5:16 PM
> To: margeyang <[email protected]>;
> [email protected]; [email protected];
> [email protected]; [email protected]
> Cc: Marge Yang <[email protected]>;
> [email protected]; Vincent Huang
> <[email protected]>
> 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 <[email protected]>
>>
>> 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<[email protected]>
>> ---
>> 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;
>>
>