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;
Feedback-ID: i06a0429a:Fastmail
Content-Type: text/plain;
        charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3731.500.231\))
Subject: Re: [PATCH v2] Input: synaptics - disable intertouch for Lenovo L440
From:   Andrew Duggan <andrew@duggan.us>
In-Reply-To: <CALNJtpV4WsknSSfBBer-MM0y_V=O5Fv2Lc3ei3heEyZwvR2rzQ@mail.gmail.com>
Date:   Mon, 17 Apr 2023 13:14:42 -0700
Cc:     Lyude Paul <lyude@redhat.com>,
        Andrew Duggan <aduggan@synaptics.com>,
        "amandhoot12@gmail.com" <amandhoot12@gmail.com>,
        Dmitry Torokhov <dmitry.torokhov@gmail.com>,
        "jdenose@google.com" <jdenose@google.com>,
        "linux-input@vger.kernel.org" <linux-input@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "markpearson@lenovo.com" <markpearson@lenovo.com>,
        "wsa+renesas@sang-engineering.com" <wsa+renesas@sang-engineering.com>,
        benjamin.tissoires@redhat.com
Content-Transfer-Encoding: quoted-printable
Message-Id: <65C23A49-5A55-4CF4-9AFD-2DA504DAABF5@duggan.us>
References: <063c8f77c216ffac463532023009124542d54c19.camel@redhat.com>
 <20230414092353.v2.1.Ieb687047a5b75c7b7ee5dd258207ef5ca9a3b728@changeid>
 <CALNJtpXLHHSV8YshUnk0opLNMUJpT7DgBNRYXoP2Yn-fnA8vPA@mail.gmail.com>
 <CALNJtpV4WsknSSfBBer-MM0y_V=O5Fv2Lc3ei3heEyZwvR2rzQ@mail.gmail.com>
To:     Jonathan Denose <jdenose@chromium.org>
Precedence: bulk

Hi Lyude and Jonathan,

I was just about to reply and suggest that we look into this issue a =
little more since the touchpad in the L440 would benefit from the =
additional data from the intertouch interface. Especially, since it has =
a large area and several buttons. PS/2 only reports position data for =
two fingers so three finger gestures is another example.

Generally, these types of suspend / resume issues are the result of the =
touchpad resetting and the firmware expecting commands from the PS/2 =
interface. On resume, the PS/2 driver should send a command over the =
PS/2 interface to switch the touchpad firmware back into intertouch =
(SMBus) mode. The logs you provided look like that's what is happening =
here. The SMBus driver is sending commands, but the touchpad firmware =
won't respond until it is switch back into intertouch mode. It has been =
a while since I have worked on these touchpads, but from what I remember =
I think there is code in the psmouse-smbus driver to handle these =
situations. I added Benjamin Tissoires to CC since I think he worked on =
that handling. I thought suspend / resume was tested on with these "top =
button" touchpads when support for them was added. I don't know if the =
L440 specifically included in the testing. I'm curious if this is a =
regression or not.

Regarding the patch, I do have one comment below:

> On Apr 17, 2023, at 11:52, Jonathan Denose <jdenose@chromium.org> =
wrote:
>=20
> CAUTION: Email originated externally, do not click links or open =
attachments unless you recognize the sender and know the content is =
safe.
>=20
>=20
> Sorry, I thought I sent this as plain text but I think maybe not.
> Trying once more, the message was:
>=20
> I think that disabling synaptics_intertouch would resolve the issue
> mentioned in the commit, but cause a regression in the functionality
> that intertouch is supposed to bring, like three-finger gestures. I'm
> attaching some of the logs that I captured when the touchpad was
> failing on resume. I think the main culprit is
> i2c_smbus_read_byte_data where the driver is unable to get the SMBus
> version number. I get the following lines in dmesg:
>=20
> [ 2869.745860] rmi4_smbus 0-002c: failed to get SMBus version number!
> [ 2869.746060] rmi4_physical rmi4-00: rmi_driver_reset_handler: Failed
> to read current IRQ mask.
> [ 2869.746260] rmi4_f01 rmi4-00.fn01: Failed to restore normal =
operation: -6.
> [ 2869.746262] rmi4_f01 rmi4-00.fn01: Resume failed with code -6.
> [ 2869.746264] rmi4_physical rmi4-00: Failed to suspend functions: -6
> [ 2869.746265] rmi4_smbus 0-002c: Failed to resume device: -6
> [ 2869.746268] rmi4_smbus 0-002c: rmi_smb_resume+0x0/0x6b [rmi_smbus]
> returned 0 after 549 usecs
> [ 2869.746446] rmi4_physical rmi4-00: Failed to read irqs, code=3D-6
>=20
> Any ideas on what might be causing this, only on resume from deep =
sleep?
>=20
>=20
> On Mon, Apr 17, 2023 at 1:47=E2=80=AFPM Jonathan Denose =
<jdenose@chromium.org> wrote:
>>=20
>> I think that disabling synaptics_intertouch would resolve the issue =
mentioned in the commit, but cause a regression in the functionality =
that intertouch is supposed to bring, like three-finger gestures. I'm =
attaching some of the logs that I captured when the touchpad was failing =
on resume. I think the main culprit is i2c_smbus_read_byte_data where =
the driver is unable to get the SMBus version number. I get the =
following lines in dmesg:
>>=20
>> [ 2869.745860] rmi4_smbus 0-002c: failed to get SMBus version number!
>> [ 2869.746060] rmi4_physical rmi4-00: rmi_driver_reset_handler: =
Failed to read current IRQ mask.
>> [ 2869.746260] rmi4_f01 rmi4-00.fn01: Failed to restore normal =
operation: -6.
>> [ 2869.746262] rmi4_f01 rmi4-00.fn01: Resume failed with code -6.
>> [ 2869.746264] rmi4_physical rmi4-00: Failed to suspend functions: -6
>> [ 2869.746265] rmi4_smbus 0-002c: Failed to resume device: -6
>> [ 2869.746268] rmi4_smbus 0-002c: rmi_smb_resume+0x0/0x6b [rmi_smbus] =
returned 0 after 549 usecs
>> [ 2869.746446] rmi4_physical rmi4-00: Failed to read irqs, code=3D-6
>>=20
>> Any ideas on what might be causing this, only on resume from deep =
sleep?
>>=20
>>=20
>> On Fri, Apr 14, 2023 at 11:41=E2=80=AFAM Jonathan Denose =
<jdenose@chromium.org> wrote:
>>>=20
>>> When intertouch is enabled for the L440 a (deep)sleep/resume
>>> cycle causes the touchpad driver to hang which causes the
>>> touchpad to become unresponsive. Disable intertouch resolves
>>> this issue and the touchpad is fine after resume from sleep.
>>>=20
>>> Additionally, when the PNP id for the L440 is only removed
>>> from the topbuttonpad_pnp_ids list, a message is logged to
>>> enable psmouse.synaptics_intertouch, which would cause the
>>> sleep/resume issue again. By removing the PNP id from
>>> topbutton_pnp_ids and then adding it to the
>>> forcepad_pnp_ids array, intertouch is disabled and the
>>> message is not logged.
>>>=20
>>> Signed-off-by: Jonathan Denose <jdenose@google.com>
>>> ---
>>>=20
>>> Changes in v2:
>>> - remove debug statement
>>>=20
>>> drivers/input/mouse/synaptics.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>=20
>>> diff --git a/drivers/input/mouse/synaptics.c =
b/drivers/input/mouse/synaptics.c
>>> index fa021af8506e4..b7218b7652c20 100644
>>> --- a/drivers/input/mouse/synaptics.c
>>> +++ b/drivers/input/mouse/synaptics.c
>>> @@ -150,7 +150,6 @@ static const char * const topbuttonpad_pnp_ids[] =
=3D {
>>>        "LEN2001", /* Edge E431 */
>>>        "LEN2002", /* Edge E531 */
>>>        "LEN2003",
>>> -       "LEN2004", /* L440 */
>>>        "LEN2005",
>>>        "LEN2006", /* Edge E440/E540 */
>>>        "LEN2007",
>>> @@ -198,6 +197,7 @@ static const char * const smbus_pnp_ids[] =3D {
>>> static const char * const forcepad_pnp_ids[] =3D {
>>>        "SYN300D",
>>>        "SYN3014",
>>> +       "LEN2004", /* L440 */

While this does seem to elliminate the message, the touchpad in the L440 =
is not a forcepad. Adding the L440 PnP ID here implies that it is one of =
these special forcepads which reports "force" data for contacts and that =
is not the case here.

>>>        NULL
>>> };
>>>=20
>>> =E2=80=94
>>> 2.39.2
>>>=20


Andrew