5.17-rc regression: X1 Carbon touchpad not resumed

5.17-rc regression: X1 Carbon touchpad not resumed

[Hugh Dickins]

5.17-rc[1-3] on Lenovo ThinkPad X1 Carbon 5th gen: when lid closed
and opened and system resumed, the touchpad cursor cannot be moved.

Some dmesg from bootup:
[    2.211061] rmi4_smbus 6-002c: registering SMbus-connected sensor
[    2.263809] ucsi_acpi USBC000:00: UCSI_GET_PDOS failed (-95)
[    2.291782] rmi4_f01 rmi4-00.fn01: found RMI device, manufacturer: Synaptics, product: TM3289-002, fw id: 2492434
[    2.371377] input: Synaptics TM3289-002 as /devices/pci0000:00/0000:00:1f.4/i2c-6/6-002c/rmi4-00/input/input8
[    2.380820] serio: RMI4 PS/2 pass-through port at rmi4-00.fn03
...
[    2.725471] input: PS/2 Generic Mouse as /devices/pci0000:00/0000:00:1f.4/i2c-6/6-002c/rmi4-00/rmi4-00.fn03/serio2/input/input9

Some dmesg from resume:
[   79.221064] rmi4_smbus 6-002c: failed to get SMBus version number!
[   79.265074] rmi4_physical rmi4-00: rmi_driver_reset_handler: Failed to read current IRQ mask.
[   79.308330] rmi4_f01 rmi4-00.fn01: Failed to restore normal operation: -6.
[   79.308335] rmi4_f01 rmi4-00.fn01: Resume failed with code -6.
[   79.308339] rmi4_physical rmi4-00: Failed to suspend functions: -6
[   79.308342] rmi4_smbus 6-002c: Failed to resume device: -6
[   79.351967] rmi4_physical rmi4-00: Failed to read irqs, code=-6

Bisection led to 172d931910e1db800f4e71e8ed92281b6f8c6ee2
("i2c: enable async suspend/resume on i2c client devices")
and reverting that fixes it for me.

Hugh

Re: 5.17-rc regression: X1 Carbon touchpad not resumed

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 429 bytes --]

Hello Hugh,

> Bisection led to 172d931910e1db800f4e71e8ed92281b6f8c6ee2
> ("i2c: enable async suspend/resume on i2c client devices")
> and reverting that fixes it for me.

Thank you for the report plus bisection and sorry for the regression!

I will wait a few days if people come up with a fix. If not, I will
revert the offending commit. Are you willing to test patches in case
there will be some?

All the best,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: 5.17-rc regression: X1 Carbon touchpad not resumed

[Hugh Dickins]

On Mon, 7 Feb 2022, Wolfram Sang wrote:
> Hello Hugh,
> 
> > Bisection led to 172d931910e1db800f4e71e8ed92281b6f8c6ee2
> > ("i2c: enable async suspend/resume on i2c client devices")
> > and reverting that fixes it for me.
> 
> Thank you for the report plus bisection and sorry for the regression!
> 
> I will wait a few days if people come up with a fix. If not, I will
> revert the offending commit. Are you willing to test patches in case
> there will be some?

Many thanks for such quick response (I found it at rc1, but couldn't
spend time to bisect until yesterday).  Willing to test?  Absolutely.

Hugh

Re: 5.17-rc regression: rmi4 clients cannot deal with asynchronous suspend? (was: X1 Carbon touchpad not resumed)

[Rajat Jain]

+Rafael (for any inputs on asynchronous suspend / resume)
+Dmitry Torokhov (since no other maintainer of rmi4 in MAINTAINERS file)
+loic.poulain@linaro.org (who fixed RMI device hierarchy recently)
+ Some Synaptics folks (from recent commits - Vincent Huang, Andrew
Duggan, Cheiny)

On Mon, Feb 7, 2022 at 12:23 PM Wolfram Sang <wsa@kernel.org> wrote:
>
> Hello Hugh,
>
> > Bisection led to 172d931910e1db800f4e71e8ed92281b6f8c6ee2
> > ("i2c: enable async suspend/resume on i2c client devices")
> > and reverting that fixes it for me.
>
> Thank you for the report plus bisection and sorry for the regression!

+1, Thanks for the bisection, and apologies for the inconveniences.

The problem here seems to be that for some reason, some devices (all
connected to rmi4 adapter) failed to resume, but only when
asynchronous suspend is enabled (by 172d931910e1):

[   79.221064] rmi4_smbus 6-002c: failed to get SMBus version number!
[   79.265074] rmi4_physical rmi4-00: rmi_driver_reset_handler: Failed
to read current IRQ mask.
[   79.308330] rmi4_f01 rmi4-00.fn01: Failed to restore normal operation: -6.
[   79.308335] rmi4_f01 rmi4-00.fn01: Resume failed with code -6.
[   79.308339] rmi4_physical rmi4-00: Failed to suspend functions: -6
[   79.308342] rmi4_smbus 6-002c: Failed to resume device: -6
[   79.351967] rmi4_physical rmi4-00: Failed to read irqs, code=-6

A resume failure that only shows up during asynchronous resume,
typically means that the device is dependent on some other device to
resume first, but this dependency is NOT established in a parent child
relationship (which is wrong and needs to be fixed, perhaps using
device_add_link()). Thus the kernel may be resuming these devices
without first resuming some other device that these devices need to
depend on.

TBH, I'm not sure how to fix this. The only hint I see is that all of
these devices seem to be attached to rmi4 device so perhaps something
there? I see 6e4860410b828f recently fixed device hierarchy for rmi4,
and so seemingly should have fixed this very issue (as also seen in
commit message)?

>
> I will wait a few days if people come up with a fix. If not, I will
> revert the offending commit.

While I'll be sad because this means no i2c-client can now resume in
parallel and increases resume latency by a *LOT* (hundreds of ms on
all Linux systems), I understand that this needs to be done unless
someone comes up with a fix.

I wanted to confirm that the following patches shall continue to stay?

d320ec7acc83 i2c: enable async suspend/resume for i2c adapters
7c5b3c158b38 i2c: designware: Enable async suspend / resume of
designware devices

Thanks & Best Regards,

Rajat


>
> All the best,
>
>    Wolfram
>

Re: 5.17-rc regression: rmi4 clients cannot deal with asynchronous suspend? (was: X1 Carbon touchpad not resumed)

[Rajat Jain]

+linux-input@vger.kernel.org

On Mon, Feb 7, 2022 at 1:09 PM Rajat Jain <rajatja@google.com> wrote:
>
> +Rafael (for any inputs on asynchronous suspend / resume)
> +Dmitry Torokhov (since no other maintainer of rmi4 in MAINTAINERS file)
> +loic.poulain@linaro.org (who fixed RMI device hierarchy recently)
> + Some Synaptics folks (from recent commits - Vincent Huang, Andrew
> Duggan, Cheiny)
>
> On Mon, Feb 7, 2022 at 12:23 PM Wolfram Sang <wsa@kernel.org> wrote:
> >
> > Hello Hugh,
> >
> > > Bisection led to 172d931910e1db800f4e71e8ed92281b6f8c6ee2
> > > ("i2c: enable async suspend/resume on i2c client devices")
> > > and reverting that fixes it for me.
> >
> > Thank you for the report plus bisection and sorry for the regression!
>
> +1, Thanks for the bisection, and apologies for the inconveniences.
>
> The problem here seems to be that for some reason, some devices (all
> connected to rmi4 adapter) failed to resume, but only when
> asynchronous suspend is enabled (by 172d931910e1):
>
> [   79.221064] rmi4_smbus 6-002c: failed to get SMBus version number!
> [   79.265074] rmi4_physical rmi4-00: rmi_driver_reset_handler: Failed
> to read current IRQ mask.
> [   79.308330] rmi4_f01 rmi4-00.fn01: Failed to restore normal operation: -6.
> [   79.308335] rmi4_f01 rmi4-00.fn01: Resume failed with code -6.
> [   79.308339] rmi4_physical rmi4-00: Failed to suspend functions: -6
> [   79.308342] rmi4_smbus 6-002c: Failed to resume device: -6
> [   79.351967] rmi4_physical rmi4-00: Failed to read irqs, code=-6
>
> A resume failure that only shows up during asynchronous resume,
> typically means that the device is dependent on some other device to
> resume first, but this dependency is NOT established in a parent child
> relationship (which is wrong and needs to be fixed, perhaps using
> device_add_link()). Thus the kernel may be resuming these devices
> without first resuming some other device that these devices need to
> depend on.
>
> TBH, I'm not sure how to fix this. The only hint I see is that all of
> these devices seem to be attached to rmi4 device so perhaps something
> there? I see 6e4860410b828f recently fixed device hierarchy for rmi4,
> and so seemingly should have fixed this very issue (as also seen in
> commit message)?
>
> >
> > I will wait a few days if people come up with a fix. If not, I will
> > revert the offending commit.
>
> While I'll be sad because this means no i2c-client can now resume in
> parallel and increases resume latency by a *LOT* (hundreds of ms on
> all Linux systems), I understand that this needs to be done unless
> someone comes up with a fix.
>
> I wanted to confirm that the following patches shall continue to stay?
>
> d320ec7acc83 i2c: enable async suspend/resume for i2c adapters
> 7c5b3c158b38 i2c: designware: Enable async suspend / resume of
> designware devices
>
> Thanks & Best Regards,
>
> Rajat
>
>
> >
> > All the best,
> >
> >    Wolfram
> >

Re: 5.17-rc regression: rmi4 clients cannot deal with asynchronous suspend? (was: X1 Carbon touchpad not resumed)

[Dmitry Torokhov]

On Mon, Feb 07, 2022 at 01:41:36PM -0800, Rajat Jain wrote:
> +linux-input@vger.kernel.org
> 
> On Mon, Feb 7, 2022 at 1:09 PM Rajat Jain <rajatja@google.com> wrote:
> >
> > +Rafael (for any inputs on asynchronous suspend / resume)
> > +Dmitry Torokhov (since no other maintainer of rmi4 in MAINTAINERS file)
> > +loic.poulain@linaro.org (who fixed RMI device hierarchy recently)
> > + Some Synaptics folks (from recent commits - Vincent Huang, Andrew
> > Duggan, Cheiny)
> >
> > On Mon, Feb 7, 2022 at 12:23 PM Wolfram Sang <wsa@kernel.org> wrote:
> > >
> > > Hello Hugh,
> > >
> > > > Bisection led to 172d931910e1db800f4e71e8ed92281b6f8c6ee2
> > > > ("i2c: enable async suspend/resume on i2c client devices")
> > > > and reverting that fixes it for me.
> > >
> > > Thank you for the report plus bisection and sorry for the regression!
> >
> > +1, Thanks for the bisection, and apologies for the inconveniences.
> >
> > The problem here seems to be that for some reason, some devices (all
> > connected to rmi4 adapter) failed to resume, but only when
> > asynchronous suspend is enabled (by 172d931910e1):
> >
> > [   79.221064] rmi4_smbus 6-002c: failed to get SMBus version number!
> > [   79.265074] rmi4_physical rmi4-00: rmi_driver_reset_handler: Failed
> > to read current IRQ mask.
> > [   79.308330] rmi4_f01 rmi4-00.fn01: Failed to restore normal operation: -6.
> > [   79.308335] rmi4_f01 rmi4-00.fn01: Resume failed with code -6.
> > [   79.308339] rmi4_physical rmi4-00: Failed to suspend functions: -6
> > [   79.308342] rmi4_smbus 6-002c: Failed to resume device: -6
> > [   79.351967] rmi4_physical rmi4-00: Failed to read irqs, code=-6
> >
> > A resume failure that only shows up during asynchronous resume,
> > typically means that the device is dependent on some other device to
> > resume first, but this dependency is NOT established in a parent child
> > relationship (which is wrong and needs to be fixed, perhaps using
> > device_add_link()). Thus the kernel may be resuming these devices
> > without first resuming some other device that these devices need to
> > depend on.
> >
> > TBH, I'm not sure how to fix this. The only hint I see is that all of
> > these devices seem to be attached to rmi4 device so perhaps something
> > there? I see 6e4860410b828f recently fixed device hierarchy for rmi4,
> > and so seemingly should have fixed this very issue (as also seen in
> > commit message)?
> >
> > >
> > > I will wait a few days if people come up with a fix. If not, I will
> > > revert the offending commit.
> >
> > While I'll be sad because this means no i2c-client can now resume in
> > parallel and increases resume latency by a *LOT* (hundreds of ms on
> > all Linux systems), I understand that this needs to be done unless
> > someone comes up with a fix.

There is intricate dance happening switching touchpad from legacy PS/2
into RMI mode, with touchpad being dependent not only on SMbus
controller, but also on i8042 keyboard controller and its PS/2 port (or
rather their emulation by the system firmware).

I wonder if we could apply a little bit more targeted patch:

diff --git a/drivers/input/rmi4/rmi_smbus.c b/drivers/input/rmi4/rmi_smbus.c
index 2407ea43de59..3901d06d38ca 100644
--- a/drivers/input/rmi4/rmi_smbus.c
+++ b/drivers/input/rmi4/rmi_smbus.c
@@ -335,6 +335,7 @@ static int rmi_smb_probe(struct i2c_client *client,
 		return error;
 	}
 
+	device_disable_async_suspend(&client->dev);
 	return 0;
 }
 

... and if that works then we cant try to establish proper dependencies
via device links later.

Hugh, could you please try this out and see if it helps?

Thanks!

-- 
Dmitry

Re: 5.17-rc regression: rmi4 clients cannot deal with asynchronous suspend? (was: X1 Carbon touchpad not resumed)

[Hugh Dickins]

On Mon, 7 Feb 2022, Dmitry Torokhov wrote:
> On Mon, Feb 07, 2022 at 01:41:36PM -0800, Rajat Jain wrote:
> > +linux-input@vger.kernel.org
> > 
> > On Mon, Feb 7, 2022 at 1:09 PM Rajat Jain <rajatja@google.com> wrote:
> > >
> > > +Rafael (for any inputs on asynchronous suspend / resume)
> > > +Dmitry Torokhov (since no other maintainer of rmi4 in MAINTAINERS file)
> > > +loic.poulain@linaro.org (who fixed RMI device hierarchy recently)
> > > + Some Synaptics folks (from recent commits - Vincent Huang, Andrew
> > > Duggan, Cheiny)
> > >
> > > On Mon, Feb 7, 2022 at 12:23 PM Wolfram Sang <wsa@kernel.org> wrote:
> > > >
> > > > Hello Hugh,
> > > >
> > > > > Bisection led to 172d931910e1db800f4e71e8ed92281b6f8c6ee2
> > > > > ("i2c: enable async suspend/resume on i2c client devices")
> > > > > and reverting that fixes it for me.
> > > >
> > > > Thank you for the report plus bisection and sorry for the regression!
> > >
> > > +1, Thanks for the bisection, and apologies for the inconveniences.
> > >
> > > The problem here seems to be that for some reason, some devices (all
> > > connected to rmi4 adapter) failed to resume, but only when
> > > asynchronous suspend is enabled (by 172d931910e1):
> > >
> > > [   79.221064] rmi4_smbus 6-002c: failed to get SMBus version number!
> > > [   79.265074] rmi4_physical rmi4-00: rmi_driver_reset_handler: Failed
> > > to read current IRQ mask.
> > > [   79.308330] rmi4_f01 rmi4-00.fn01: Failed to restore normal operation: -6.
> > > [   79.308335] rmi4_f01 rmi4-00.fn01: Resume failed with code -6.
> > > [   79.308339] rmi4_physical rmi4-00: Failed to suspend functions: -6
> > > [   79.308342] rmi4_smbus 6-002c: Failed to resume device: -6
> > > [   79.351967] rmi4_physical rmi4-00: Failed to read irqs, code=-6
> > >
> > > A resume failure that only shows up during asynchronous resume,
> > > typically means that the device is dependent on some other device to
> > > resume first, but this dependency is NOT established in a parent child
> > > relationship (which is wrong and needs to be fixed, perhaps using
> > > device_add_link()). Thus the kernel may be resuming these devices
> > > without first resuming some other device that these devices need to
> > > depend on.
> > >
> > > TBH, I'm not sure how to fix this. The only hint I see is that all of
> > > these devices seem to be attached to rmi4 device so perhaps something
> > > there? I see 6e4860410b828f recently fixed device hierarchy for rmi4,
> > > and so seemingly should have fixed this very issue (as also seen in
> > > commit message)?
> > >
> > > >
> > > > I will wait a few days if people come up with a fix. If not, I will
> > > > revert the offending commit.
> > >
> > > While I'll be sad because this means no i2c-client can now resume in
> > > parallel and increases resume latency by a *LOT* (hundreds of ms on
> > > all Linux systems), I understand that this needs to be done unless
> > > someone comes up with a fix.
> 
> There is intricate dance happening switching touchpad from legacy PS/2
> into RMI mode, with touchpad being dependent not only on SMbus
> controller, but also on i8042 keyboard controller and its PS/2 port (or
> rather their emulation by the system firmware).
> 
> I wonder if we could apply a little bit more targeted patch:
> 
> diff --git a/drivers/input/rmi4/rmi_smbus.c b/drivers/input/rmi4/rmi_smbus.c
> index 2407ea43de59..3901d06d38ca 100644
> --- a/drivers/input/rmi4/rmi_smbus.c
> +++ b/drivers/input/rmi4/rmi_smbus.c
> @@ -335,6 +335,7 @@ static int rmi_smb_probe(struct i2c_client *client,
>  		return error;
>  	}
>  
> +	device_disable_async_suspend(&client->dev);
>  	return 0;
>  }
>  
> 
> ... and if that works then we cant try to establish proper dependencies
> via device links later.
> 
> Hugh, could you please try this out and see if it helps?

Yes, that works nicely, thanks Dmitry.

By the way, my memory's been jogged by "rmi4" and the discussion above:
I had a similar-ish problem with it a year ago, discussed with PM guys,

https://lore.kernel.org/linux-pm/alpine.LSU.2.11.2101102010200.25762@eggly.anvils/

I'm not saying you have to read through that thread, but you may find
some relevance in it - Saravana concluded rmi4 driver isn't capturing
parent/child relationship correctly (at that time, anyway).

Hugh

Re: 5.17-rc regression: X1 Carbon touchpad not resumed

[Thorsten Leemhuis]

[TLDR: I'm adding the regression report below to regzbot, the Linux
kernel regression tracking bot; all text you find below is compiled from
a few templates paragraphs you might have encountered already already
from similar mails.]

Hi, this is your Linux kernel regression tracker speaking.

On 07.02.22 20:45, Hugh Dickins wrote:
> 5.17-rc[1-3] on Lenovo ThinkPad X1 Carbon 5th gen: when lid closed
> and opened and system resumed, the touchpad cursor cannot be moved.
> 
> Some dmesg from bootup:
> [    2.211061] rmi4_smbus 6-002c: registering SMbus-connected sensor
> [    2.263809] ucsi_acpi USBC000:00: UCSI_GET_PDOS failed (-95)
> [    2.291782] rmi4_f01 rmi4-00.fn01: found RMI device, manufacturer: Synaptics, product: TM3289-002, fw id: 2492434
> [    2.371377] input: Synaptics TM3289-002 as /devices/pci0000:00/0000:00:1f.4/i2c-6/6-002c/rmi4-00/input/input8
> [    2.380820] serio: RMI4 PS/2 pass-through port at rmi4-00.fn03
> ...
> [    2.725471] input: PS/2 Generic Mouse as /devices/pci0000:00/0000:00:1f.4/i2c-6/6-002c/rmi4-00/rmi4-00.fn03/serio2/input/input9
> 
> Some dmesg from resume:
> [   79.221064] rmi4_smbus 6-002c: failed to get SMBus version number!
> [   79.265074] rmi4_physical rmi4-00: rmi_driver_reset_handler: Failed to read current IRQ mask.
> [   79.308330] rmi4_f01 rmi4-00.fn01: Failed to restore normal operation: -6.
> [   79.308335] rmi4_f01 rmi4-00.fn01: Resume failed with code -6.
> [   79.308339] rmi4_physical rmi4-00: Failed to suspend functions: -6
> [   79.308342] rmi4_smbus 6-002c: Failed to resume device: -6
> [   79.351967] rmi4_physical rmi4-00: Failed to read irqs, code=-6
> 
> Bisection led to 172d931910e1db800f4e71e8ed92281b6f8c6ee2
> ("i2c: enable async suspend/resume on i2c client devices")
> and reverting that fixes it for me.

Thanks for the report.

To be sure this issue doesn't fall through the cracks unnoticed, I'm
adding it to regzbot, my Linux kernel regression tracking bot:

#regzbot ^introduced 172d931910e1db800f4e71e8ed92281b6f8c6ee2
#regzbot title i2c/input/hid: X1 Carbon touchpad not resumed
#regzbot ignore-activity

Reminder for developers: when fixing the issue, please add a 'Link:'
tags pointing to the report (the mail quoted above) using
lore.kernel.org/r/, as explained in
'Documentation/process/submitting-patches.rst' and
'Documentation/process/5.Posting.rst'. This allows the bot to connect
the report with any patches posted or committed to fix the issue; this
again allows the bot to show the current status of regressions and
automatically resolve the issue when the fix hits the right tree.

I'm sending this to everyone that got the initial report, to make them
aware of the tracking. I also hope that messages like this motivate
people to directly get at least the regression mailing list and ideally
even regzbot involved when dealing with regressions, as messages like
this wouldn't be needed then.

Don't worry, I'll send further messages wrt to this regression just to
the lists (with a tag in the subject so people can filter them away), if
they are relevant just for regzbot. With a bit of luck no such messages
will be needed anyway.

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)

P.S.: As the Linux kernel's regression tracker I'm getting a lot of
reports on my table. I can only look briefly into most of them and lack
knowledge about most of the areas they concern. I thus unfortunately
will sometimes get things wrong or miss something important. I hope
that's not the case here; if you think it is, don't hesitate to tell me
in a public reply, it's in everyone's interest to set the public record
straight.

-- 
Additional information about regzbot:

If you want to know more about regzbot, check out its web-interface, the
getting start guide, and the references documentation:

https://linux-regtracking.leemhuis.info/regzbot/
https://gitlab.com/knurd42/regzbot/-/blob/main/docs/getting_started.md
https://gitlab.com/knurd42/regzbot/-/blob/main/docs/reference.md

The last two documents will explain how you can interact with regzbot
yourself if your want to.

Hint for reporters: when reporting a regression it's in your interest to
CC the regression list and tell regzbot about the issue, as that ensures
the regression makes it onto the radar of the Linux kernel's regression
tracker -- that's in your interest, as it ensures your report won't fall
through the cracks unnoticed.

Hint for developers: you normally don't need to care about regzbot once
it's involved. Fix the issue as you normally would, just remember to
include 'Link:' tag in the patch descriptions pointing to all reports
about the issue. This has been expected from developers even before
regzbot showed up for reasons explained in
'Documentation/process/submitting-patches.rst' and
'Documentation/process/5.Posting.rst'.

Re: 5.17-rc regression: rmi4 clients cannot deal with asynchronous suspend? (was: X1 Carbon touchpad not resumed)

[Loic Poulain]

Hi folks,

On Tue, 8 Feb 2022 at 03:50, Hugh Dickins <hughd@google.com> wrote:
>
> On Mon, 7 Feb 2022, Dmitry Torokhov wrote:
> > On Mon, Feb 07, 2022 at 01:41:36PM -0800, Rajat Jain wrote:
> > > +linux-input@vger.kernel.org
> > >
> > > On Mon, Feb 7, 2022 at 1:09 PM Rajat Jain <rajatja@google.com> wrote:
> > > >
> > > > +Rafael (for any inputs on asynchronous suspend / resume)
> > > > +Dmitry Torokhov (since no other maintainer of rmi4 in MAINTAINERS file)
> > > > +loic.poulain@linaro.org (who fixed RMI device hierarchy recently)
> > > > + Some Synaptics folks (from recent commits - Vincent Huang, Andrew
> > > > Duggan, Cheiny)
> > > >
> > > > On Mon, Feb 7, 2022 at 12:23 PM Wolfram Sang <wsa@kernel.org> wrote:
> > > > >
> > > > > Hello Hugh,
> > > > >
> > > > > > Bisection led to 172d931910e1db800f4e71e8ed92281b6f8c6ee2
> > > > > > ("i2c: enable async suspend/resume on i2c client devices")
> > > > > > and reverting that fixes it for me.
> > > > >
> > > > > Thank you for the report plus bisection and sorry for the regression!
> > > >
> > > > +1, Thanks for the bisection, and apologies for the inconveniences.
> > > >
> > > > The problem here seems to be that for some reason, some devices (all
> > > > connected to rmi4 adapter) failed to resume, but only when
> > > > asynchronous suspend is enabled (by 172d931910e1):
> > > >
> > > > [   79.221064] rmi4_smbus 6-002c: failed to get SMBus version number!

Looks like this is the initial issue. Does the rmi device lose power
while suspended? if so could it be that enabling async_suspend makes
the device resuming earlier, at a time it is not yet ready? What if
you simply start with a naive msleep(200) in rmi_smb_resume()?

The rmi4 bus does not rely on generic device suspend/resume
infrastructure for its subdevices, so async_suspend should only impact
the moment at which the smbus rmi4 root device is resumed, but not the
way it and its subdevices are resumed.

Would be interesting to get some pm_debug/pm_trace to compare the
good/bad cases.




> > > > [   79.265074] rmi4_physical rmi4-00: rmi_driver_reset_handler: Failed
> > > > to read current IRQ mask.
> > > > [   79.308330] rmi4_f01 rmi4-00.fn01: Failed to restore normal operation: -6.
> > > > [   79.308335] rmi4_f01 rmi4-00.fn01: Resume failed with code -6.
> > > > [   79.308339] rmi4_physical rmi4-00: Failed to suspend functions: -6
> > > > [   79.308342] rmi4_smbus 6-002c: Failed to resume device: -6
> > > > [   79.351967] rmi4_physical rmi4-00: Failed to read irqs, code=-6
> > > >
> > > > A resume failure that only shows up during asynchronous resume,
> > > > typically means that the device is dependent on some other device to
> > > > resume first, but this dependency is NOT established in a parent child
> > > > relationship (which is wrong and needs to be fixed, perhaps using
> > > > device_add_link()). Thus the kernel may be resuming these devices
> > > > without first resuming some other device that these devices need to
> > > > depend on.
> > > >
> > > > TBH, I'm not sure how to fix this. The only hint I see is that all of
> > > > these devices seem to be attached to rmi4 device so perhaps something
> > > > there? I see 6e4860410b828f recently fixed device hierarchy for rmi4,
> > > > and so seemingly should have fixed this very issue (as also seen in
> > > > commit message)?
> > > >
> > > > >
> > > > > I will wait a few days if people come up with a fix. If not, I will
> > > > > revert the offending commit.
> > > >
> > > > While I'll be sad because this means no i2c-client can now resume in
> > > > parallel and increases resume latency by a *LOT* (hundreds of ms on
> > > > all Linux systems), I understand that this needs to be done unless
> > > > someone comes up with a fix.
> >
> > There is intricate dance happening switching touchpad from legacy PS/2
> > into RMI mode, with touchpad being dependent not only on SMbus
> > controller, but also on i8042 keyboard controller and its PS/2 port (or
> > rather their emulation by the system firmware).
> >
> > I wonder if we could apply a little bit more targeted patch:
> >
> > diff --git a/drivers/input/rmi4/rmi_smbus.c b/drivers/input/rmi4/rmi_smbus.c
> > index 2407ea43de59..3901d06d38ca 100644
> > --- a/drivers/input/rmi4/rmi_smbus.c
> > +++ b/drivers/input/rmi4/rmi_smbus.c
> > @@ -335,6 +335,7 @@ static int rmi_smb_probe(struct i2c_client *client,
> >               return error;
> >       }
> >
> > +     device_disable_async_suspend(&client->dev);
> >       return 0;
> >  }
> >
> >
> > ... and if that works then we cant try to establish proper dependencies
> > via device links later.
> >
> > Hugh, could you please try this out and see if it helps?
>
> Yes, that works nicely, thanks Dmitry.
>
> By the way, my memory's been jogged by "rmi4" and the discussion above:
> I had a similar-ish problem with it a year ago, discussed with PM guys,
>
> https://lore.kernel.org/linux-pm/alpine.LSU.2.11.2101102010200.25762@eggly.anvils/
>
> I'm not saying you have to read through that thread, but you may find
> some relevance in it - Saravana concluded rmi4 driver isn't capturing
> parent/child relationship correctly (at that time, anyway).

Re: 5.17-rc regression: rmi4 clients cannot deal with asynchronous suspend? (was: X1 Carbon touchpad not resumed)

[Jarkko Nikula]

Hi

On 2/8/22 04:50, Hugh Dickins wrote:
> On Mon, 7 Feb 2022, Dmitry Torokhov wrote:
>> On Mon, Feb 07, 2022 at 01:41:36PM -0800, Rajat Jain wrote:
>>>>>> Bisection led to 172d931910e1db800f4e71e8ed92281b6f8c6ee2
>>>>>> ("i2c: enable async suspend/resume on i2c client devices")
>>>>>> and reverting that fixes it for me.
>>>>>
>>>>> Thank you for the report plus bisection and sorry for the regression!
>>>>
>>>> +1, Thanks for the bisection, and apologies for the inconveniences.
>>>>
>>>> The problem here seems to be that for some reason, some devices (all
>>>> connected to rmi4 adapter) failed to resume, but only when
>>>> asynchronous suspend is enabled (by 172d931910e1):
>>>>
>>>> [   79.221064] rmi4_smbus 6-002c: failed to get SMBus version number!
>>>> [   79.265074] rmi4_physical rmi4-00: rmi_driver_reset_handler: Failed
>>>> to read current IRQ mask.
>>>> [   79.308330] rmi4_f01 rmi4-00.fn01: Failed to restore normal operation: -6.
>>>> [   79.308335] rmi4_f01 rmi4-00.fn01: Resume failed with code -6.
>>>> [   79.308339] rmi4_physical rmi4-00: Failed to suspend functions: -6
>>>> [   79.308342] rmi4_smbus 6-002c: Failed to resume device: -6
>>>> [   79.351967] rmi4_physical rmi4-00: Failed to read irqs, code=-6
>>>>

v5.17-rc3 on Lenovo ThinkPad X1 Carbon 8th don't even suspend due the 
same commit 172d931910e1. Sadly I tested the original patch on other 
machine(s) but not on this one with rmi4 :-(

[   39.957293] PM: suspend entry (s2idle)
[   40.938666] Filesystems sync: 0.980 seconds
[   40.942751] Freezing user space processes ... (elapsed 0.001 seconds) 
done.
[   40.945511] OOM killer disabled.
[   40.946111] Freezing remaining freezable tasks ... (elapsed 0.001 
seconds) done.
[   40.948590] printk: Suspending console(s) (use no_console_suspend to 
debug)
[   40.993123] i801_smbus 0000:00:1f.4: No response
[   40.993218] rmi4_f01 rmi4-00.fn01: Failed to write sleep mode: -6.
[   40.993232] rmi4_f01 rmi4-00.fn01: Suspend failed with code -6.
[   40.993241] rmi4_physical rmi4-00: Failed to suspend functions: -6
[   40.993404] rmi4_smbus 1-002c: Failed to suspend device: -6
[   40.993414] PM: dpm_run_callback(): rmi_smb_suspend+0x0/0x30 
[rmi_smbus] returns -6
[   40.993438] rmi4_smbus 1-002c: PM: failed to suspend async: error -6
[   41.014198] PM: Some devices failed to suspend, or early wake event 
detected
[   41.021544] i801_smbus 0000:00:1f.4: No response
[   41.021612] rmi4_f03 rmi4-00.fn03: rmi_f03_pt_write: Failed to write 
to F03 TX register (-6).
[   41.022189] i801_smbus 0000:00:1f.4: No response
[   41.022230] rmi4_f03 rmi4-00.fn03: rmi_f03_pt_write: Failed to write 
to F03 TX register (-6).
[   41.023480] i801_smbus 0000:00:1f.4: No response
[   41.023542] rmi4_physical rmi4-00: rmi_driver_clear_irq_bits: Failed 
to change enabled interrupts!
[   41.033850] i801_smbus 0000:00:1f.4: No response
[   41.034006] OOM killer enabled.
[   41.035449] i801_smbus 0000:00:1f.4: No response
[   41.035722] Restarting tasks ...
[   41.036705] rmi4_physical rmi4-00: rmi_driver_set_irq_bits: Failed to 
change enabled interrupts!
[   41.038367] done.
[   41.039003] psmouse: probe of serio2 failed with error -1
[   41.071700] PM: suspend exit

>> I wonder if we could apply a little bit more targeted patch:
>>
>> diff --git a/drivers/input/rmi4/rmi_smbus.c b/drivers/input/rmi4/rmi_smbus.c
>> index 2407ea43de59..3901d06d38ca 100644
>> --- a/drivers/input/rmi4/rmi_smbus.c
>> +++ b/drivers/input/rmi4/rmi_smbus.c
>> @@ -335,6 +335,7 @@ static int rmi_smb_probe(struct i2c_client *client,
>>   		return error;
>>   	}
>>   
>> +	device_disable_async_suspend(&client->dev);
>>   	return 0;
>>   }
>>   
>>
>> ... and if that works then we cant try to establish proper dependencies
>> via device links later.
>>
>> Hugh, could you please try this out and see if it helps?
> 
> Yes, that works nicely, thanks Dmitry.
> 
Gladly fixes the issue on ThinkPad X1 Carbon 8th too:

Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>

Re: 5.17-rc regression: rmi4 clients cannot deal with asynchronous suspend? (was: X1 Carbon touchpad not resumed)

[Dmitry Torokhov]

Hi Hugh, Jarkko,

On Tue, Feb 08, 2022 at 04:57:53PM +0200, Jarkko Nikula wrote:
> Hi
> 
> On 2/8/22 04:50, Hugh Dickins wrote:
> > On Mon, 7 Feb 2022, Dmitry Torokhov wrote:
> > > On Mon, Feb 07, 2022 at 01:41:36PM -0800, Rajat Jain wrote:
> > > > > > > Bisection led to 172d931910e1db800f4e71e8ed92281b6f8c6ee2
> > > > > > > ("i2c: enable async suspend/resume on i2c client devices")
> > > > > > > and reverting that fixes it for me.
> > > > > > 
> > > > > > Thank you for the report plus bisection and sorry for the regression!
> > > > > 
> > > > > +1, Thanks for the bisection, and apologies for the inconveniences.
> > > > > 
> > > > > The problem here seems to be that for some reason, some devices (all
> > > > > connected to rmi4 adapter) failed to resume, but only when
> > > > > asynchronous suspend is enabled (by 172d931910e1):
> > > > > 
> > > > > [   79.221064] rmi4_smbus 6-002c: failed to get SMBus version number!
> > > > > [   79.265074] rmi4_physical rmi4-00: rmi_driver_reset_handler: Failed
> > > > > to read current IRQ mask.
> > > > > [   79.308330] rmi4_f01 rmi4-00.fn01: Failed to restore normal operation: -6.
> > > > > [   79.308335] rmi4_f01 rmi4-00.fn01: Resume failed with code -6.
> > > > > [   79.308339] rmi4_physical rmi4-00: Failed to suspend functions: -6
> > > > > [   79.308342] rmi4_smbus 6-002c: Failed to resume device: -6
> > > > > [   79.351967] rmi4_physical rmi4-00: Failed to read irqs, code=-6
> > > > > 
> 
> v5.17-rc3 on Lenovo ThinkPad X1 Carbon 8th don't even suspend due the same
> commit 172d931910e1. Sadly I tested the original patch on other machine(s)
> but not on this one with rmi4 :-(
> 
> [   39.957293] PM: suspend entry (s2idle)
> [   40.938666] Filesystems sync: 0.980 seconds
> [   40.942751] Freezing user space processes ... (elapsed 0.001 seconds)
> done.
> [   40.945511] OOM killer disabled.
> [   40.946111] Freezing remaining freezable tasks ... (elapsed 0.001
> seconds) done.
> [   40.948590] printk: Suspending console(s) (use no_console_suspend to
> debug)
> [   40.993123] i801_smbus 0000:00:1f.4: No response
> [   40.993218] rmi4_f01 rmi4-00.fn01: Failed to write sleep mode: -6.
> [   40.993232] rmi4_f01 rmi4-00.fn01: Suspend failed with code -6.
> [   40.993241] rmi4_physical rmi4-00: Failed to suspend functions: -6
> [   40.993404] rmi4_smbus 1-002c: Failed to suspend device: -6
> [   40.993414] PM: dpm_run_callback(): rmi_smb_suspend+0x0/0x30 [rmi_smbus]
> returns -6
> [   40.993438] rmi4_smbus 1-002c: PM: failed to suspend async: error -6
> [   41.014198] PM: Some devices failed to suspend, or early wake event
> detected
> [   41.021544] i801_smbus 0000:00:1f.4: No response
> [   41.021612] rmi4_f03 rmi4-00.fn03: rmi_f03_pt_write: Failed to write to
> F03 TX register (-6).
> [   41.022189] i801_smbus 0000:00:1f.4: No response
> [   41.022230] rmi4_f03 rmi4-00.fn03: rmi_f03_pt_write: Failed to write to
> F03 TX register (-6).
> [   41.023480] i801_smbus 0000:00:1f.4: No response
> [   41.023542] rmi4_physical rmi4-00: rmi_driver_clear_irq_bits: Failed to
> change enabled interrupts!
> [   41.033850] i801_smbus 0000:00:1f.4: No response
> [   41.034006] OOM killer enabled.
> [   41.035449] i801_smbus 0000:00:1f.4: No response
> [   41.035722] Restarting tasks ...
> [   41.036705] rmi4_physical rmi4-00: rmi_driver_set_irq_bits: Failed to
> change enabled interrupts!
> [   41.038367] done.
> [   41.039003] psmouse: probe of serio2 failed with error -1
> [   41.071700] PM: suspend exit

Sorry for the delay, but I wonder if you could try the patch below and
tell me if that also fixes the issue for you?

Also adding Hans as to make sure changes to psmouse_smbus make sense to
him.

Thanks!

diff --git a/drivers/input/mouse/psmouse-smbus.c b/drivers/input/mouse/psmouse-smbus.c
index a472489ccbad..164f6c757f6b 100644
--- a/drivers/input/mouse/psmouse-smbus.c
+++ b/drivers/input/mouse/psmouse-smbus.c
@@ -75,6 +75,8 @@ static void psmouse_smbus_detach_i2c_client(struct i2c_client *client)
 				    "Marking SMBus companion %s as gone\n",
 				    dev_name(&smbdev->client->dev));
 			smbdev->dead = true;
+			device_link_remove(&smbdev->client->dev,
+					   &smbdev->psmouse->ps2dev.serio->dev);
 			serio_rescan(smbdev->psmouse->ps2dev.serio);
 		} else {
 			list_del(&smbdev->node);
@@ -174,6 +176,8 @@ static void psmouse_smbus_disconnect(struct psmouse *psmouse)
 		kfree(smbdev);
 	} else {
 		smbdev->dead = true;
+		device_link_remove(&smbdev->client->dev,
+				   &psmouse->ps2dev.serio->dev);
 		psmouse_dbg(smbdev->psmouse,
 			    "posting removal request for SMBus companion %s\n",
 			    dev_name(&smbdev->client->dev));
@@ -270,6 +274,12 @@ int psmouse_smbus_init(struct psmouse *psmouse,
 
 	if (smbdev->client) {
 		/* We have our companion device */
+		if (!device_link_add(&smbdev->client->dev,
+				     &psmouse->ps2dev.serio->dev,
+				     DL_FLAG_STATELESS))
+			psmouse_warn(psmouse,
+				     "failed to set up link with iSMBus companion %s\n",
+				     dev_name(&smbdev->client->dev));
 		return 0;
 	}
 

-- 
Dmitry

Re: 5.17-rc regression: rmi4 clients cannot deal with asynchronous suspend? (was: X1 Carbon touchpad not resumed)

[Hugh Dickins]

On Sun, 13 Feb 2022, Dmitry Torokhov wrote:
> 
> Sorry for the delay, but I wonder if you could try the patch below and
> tell me if that also fixes the issue for you?

It fixes it for me, thanks Dmitry; with nothing unpleasant in dmesg.

Hugh

Re: 5.17-rc regression: rmi4 clients cannot deal with asynchronous suspend? (was: X1 Carbon touchpad not resumed)

[Hans de Goede]

Hi All,

On 2/14/22 03:31, Dmitry Torokhov wrote:
> Hi Hugh, Jarkko,
> 
> On Tue, Feb 08, 2022 at 04:57:53PM +0200, Jarkko Nikula wrote:
>> Hi
>>
>> On 2/8/22 04:50, Hugh Dickins wrote:
>>> On Mon, 7 Feb 2022, Dmitry Torokhov wrote:
>>>> On Mon, Feb 07, 2022 at 01:41:36PM -0800, Rajat Jain wrote:
>>>>>>>> Bisection led to 172d931910e1db800f4e71e8ed92281b6f8c6ee2
>>>>>>>> ("i2c: enable async suspend/resume on i2c client devices")
>>>>>>>> and reverting that fixes it for me.
>>>>>>>
>>>>>>> Thank you for the report plus bisection and sorry for the regression!
>>>>>>
>>>>>> +1, Thanks for the bisection, and apologies for the inconveniences.
>>>>>>
>>>>>> The problem here seems to be that for some reason, some devices (all
>>>>>> connected to rmi4 adapter) failed to resume, but only when
>>>>>> asynchronous suspend is enabled (by 172d931910e1):
>>>>>>
>>>>>> [   79.221064] rmi4_smbus 6-002c: failed to get SMBus version number!
>>>>>> [   79.265074] rmi4_physical rmi4-00: rmi_driver_reset_handler: Failed
>>>>>> to read current IRQ mask.
>>>>>> [   79.308330] rmi4_f01 rmi4-00.fn01: Failed to restore normal operation: -6.
>>>>>> [   79.308335] rmi4_f01 rmi4-00.fn01: Resume failed with code -6.
>>>>>> [   79.308339] rmi4_physical rmi4-00: Failed to suspend functions: -6
>>>>>> [   79.308342] rmi4_smbus 6-002c: Failed to resume device: -6
>>>>>> [   79.351967] rmi4_physical rmi4-00: Failed to read irqs, code=-6
>>>>>>
>>
>> v5.17-rc3 on Lenovo ThinkPad X1 Carbon 8th don't even suspend due the same
>> commit 172d931910e1. Sadly I tested the original patch on other machine(s)
>> but not on this one with rmi4 :-(
>>
>> [   39.957293] PM: suspend entry (s2idle)
>> [   40.938666] Filesystems sync: 0.980 seconds
>> [   40.942751] Freezing user space processes ... (elapsed 0.001 seconds)
>> done.
>> [   40.945511] OOM killer disabled.
>> [   40.946111] Freezing remaining freezable tasks ... (elapsed 0.001
>> seconds) done.
>> [   40.948590] printk: Suspending console(s) (use no_console_suspend to
>> debug)
>> [   40.993123] i801_smbus 0000:00:1f.4: No response
>> [   40.993218] rmi4_f01 rmi4-00.fn01: Failed to write sleep mode: -6.
>> [   40.993232] rmi4_f01 rmi4-00.fn01: Suspend failed with code -6.
>> [   40.993241] rmi4_physical rmi4-00: Failed to suspend functions: -6
>> [   40.993404] rmi4_smbus 1-002c: Failed to suspend device: -6
>> [   40.993414] PM: dpm_run_callback(): rmi_smb_suspend+0x0/0x30 [rmi_smbus]
>> returns -6
>> [   40.993438] rmi4_smbus 1-002c: PM: failed to suspend async: error -6
>> [   41.014198] PM: Some devices failed to suspend, or early wake event
>> detected
>> [   41.021544] i801_smbus 0000:00:1f.4: No response
>> [   41.021612] rmi4_f03 rmi4-00.fn03: rmi_f03_pt_write: Failed to write to
>> F03 TX register (-6).
>> [   41.022189] i801_smbus 0000:00:1f.4: No response
>> [   41.022230] rmi4_f03 rmi4-00.fn03: rmi_f03_pt_write: Failed to write to
>> F03 TX register (-6).
>> [   41.023480] i801_smbus 0000:00:1f.4: No response
>> [   41.023542] rmi4_physical rmi4-00: rmi_driver_clear_irq_bits: Failed to
>> change enabled interrupts!
>> [   41.033850] i801_smbus 0000:00:1f.4: No response
>> [   41.034006] OOM killer enabled.
>> [   41.035449] i801_smbus 0000:00:1f.4: No response
>> [   41.035722] Restarting tasks ...
>> [   41.036705] rmi4_physical rmi4-00: rmi_driver_set_irq_bits: Failed to
>> change enabled interrupts!
>> [   41.038367] done.
>> [   41.039003] psmouse: probe of serio2 failed with error -1
>> [   41.071700] PM: suspend exit
> 
> Sorry for the delay, but I wonder if you could try the patch below and
> tell me if that also fixes the issue for you?
> 
> Also adding Hans as to make sure changes to psmouse_smbus make sense to
> him.

I'm not really familiar with the whole psmouse intertouch code. I've added Benjamin
Tissoires to the Cc who knows this code a lot better then I do.

Regards,

Hans



> diff --git a/drivers/input/mouse/psmouse-smbus.c b/drivers/input/mouse/psmouse-smbus.c
> index a472489ccbad..164f6c757f6b 100644
> --- a/drivers/input/mouse/psmouse-smbus.c
> +++ b/drivers/input/mouse/psmouse-smbus.c
> @@ -75,6 +75,8 @@ static void psmouse_smbus_detach_i2c_client(struct i2c_client *client)
>  				    "Marking SMBus companion %s as gone\n",
>  				    dev_name(&smbdev->client->dev));
>  			smbdev->dead = true;
> +			device_link_remove(&smbdev->client->dev,
> +					   &smbdev->psmouse->ps2dev.serio->dev);
>  			serio_rescan(smbdev->psmouse->ps2dev.serio);
>  		} else {
>  			list_del(&smbdev->node);
> @@ -174,6 +176,8 @@ static void psmouse_smbus_disconnect(struct psmouse *psmouse)
>  		kfree(smbdev);
>  	} else {
>  		smbdev->dead = true;
> +		device_link_remove(&smbdev->client->dev,
> +				   &psmouse->ps2dev.serio->dev);
>  		psmouse_dbg(smbdev->psmouse,
>  			    "posting removal request for SMBus companion %s\n",
>  			    dev_name(&smbdev->client->dev));
> @@ -270,6 +274,12 @@ int psmouse_smbus_init(struct psmouse *psmouse,
>  
>  	if (smbdev->client) {
>  		/* We have our companion device */
> +		if (!device_link_add(&smbdev->client->dev,
> +				     &psmouse->ps2dev.serio->dev,
> +				     DL_FLAG_STATELESS))
> +			psmouse_warn(psmouse,
> +				     "failed to set up link with iSMBus companion %s\n",
> +				     dev_name(&smbdev->client->dev));
>  		return 0;
>  	}
>  
> 

Re: 5.17-rc regression: rmi4 clients cannot deal with asynchronous suspend? (was: X1 Carbon touchpad not resumed)

[Jarkko Nikula]

On 2/14/22 09:36, Hugh Dickins wrote:
> On Sun, 13 Feb 2022, Dmitry Torokhov wrote:
>>
>> Sorry for the delay, but I wonder if you could try the patch below and
>> tell me if that also fixes the issue for you?
> 
> It fixes it for me, thanks Dmitry; with nothing unpleasant in dmesg.
> 
Also for me.

Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>