Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <598d8186-b07e-5ece-210e-83689791b85a@redhat.com>
Date:   Mon, 8 Aug 2022 11:41:55 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.10.0
Subject: Re: [PATCH] HID: HID-rmi - ignore to rmi_hid_read_block after system
 resumes.
Content-Language: en-US
To:     Marge Yang <Marge.Yang@tw.synaptics.com>,
        margeyang <marge.yang@synaptics.corp-partner.google.com>,
        "dmitry.torokhov@gmail.com" <dmitry.torokhov@gmail.com>,
        "linux-input@vger.kernel.org" <linux-input@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "benjamin.tissoires@redhat.com" <benjamin.tissoires@redhat.com>
Cc:     "derek.cheng@tw.synaptcs.com" <derek.cheng@tw.synaptcs.com>,
        Vincent Huang <Vincent.huang@tw.synaptics.com>
References: <1659949639-3127-1-git-send-email-marge.yang@synaptics.corp-partner.google.com>
 <7507e07a-5094-f129-b75a-69cc32147543@redhat.com>
 <PH0PR03MB5848811A9EC856635CD1E519A3639@PH0PR03MB5848.namprd03.prod.outlook.com>
From:   Hans de Goede <hdegoede@redhat.com>
In-Reply-To: <PH0PR03MB5848811A9EC856635CD1E519A3639@PH0PR03MB5848.namprd03.prod.outlook.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

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