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

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

[margeyang]

From: Marge Yang <marge.yang@synaptics.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.

Signed-off-by: Marge Yang<marge.yang@synaptics.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;
 
-- 
2.22.0.windows.1

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

[Hans de Goede]

Hi,

On 8/8/22 11:07, margeyang wrote:
> From: Marge Yang <marge.yang@synaptics.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@synaptics.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;
>  

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

[Marge Yang]

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 <hdegoede@redhat.com> 
Sent: Monday, August 8, 2022 5:16 PM
To: margeyang <marge.yang@synaptics.corp-partner.google.com>; dmitry.torokhov@gmail.com; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; benjamin.tissoires@redhat.com
Cc: Marge Yang <Marge.Yang@tw.synaptics.com>; derek.cheng@tw.synaptcs.com; Vincent Huang <Vincent.huang@tw.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@synaptics.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@synaptics.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;
>

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@redhat.com> 
> Sent: Monday, August 8, 2022 5:16 PM
> To: margeyang <marge.yang@synaptics.corp-partner.google.com>; dmitry.torokhov@gmail.com; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; benjamin.tissoires@redhat.com
> Cc: Marge Yang <Marge.Yang@tw.synaptics.com>; derek.cheng@tw.synaptcs.com; Vincent Huang <Vincent.huang@tw.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@synaptics.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@synaptics.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;
>>
> 

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

[Marge Yang]

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@redhat.com> 
Sent: Monday, August 8, 2022 5:42 PM
To: Marge Yang <Marge.Yang@tw.synaptics.com>; margeyang <marge.yang@synaptics.corp-partner.google.com>; dmitry.torokhov@gmail.com; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; benjamin.tissoires@redhat.com
Cc: derek.cheng@tw.synaptcs.com; Vincent Huang <Vincent.huang@tw.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@redhat.com>
> Sent: Monday, August 8, 2022 5:16 PM
> To: margeyang <marge.yang@synaptics.corp-partner.google.com>; 
> dmitry.torokhov@gmail.com; linux-input@vger.kernel.org; 
> linux-kernel@vger.kernel.org; benjamin.tissoires@redhat.com
> Cc: Marge Yang <Marge.Yang@tw.synaptics.com>; 
> derek.cheng@tw.synaptcs.com; Vincent Huang 
> <Vincent.huang@tw.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@synaptics.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@synaptics.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;
>>
>