Hi Jeffery! Your change 92e24e0e57f72e ("Input: psmouse - add delay when
deactivating for SMBus mode") [merged for v6.6-rc1] broke resume on my
T14s Gen1 (AMD): the system didn't really resume again at all (the
display almost always didn't re-initialize) and there was nothing in the
logs. I found your commit to be the culprit using a bisection and
confirmed that reverting it on top of Linux 6.6-rc3 makes thing work
again for me.
My dmesg from a kernel with the revert:
https://www.leemhuis.info/files/misc/dmesg
My config:
https://www.leemhuis.info/files/misc/config
Funny detail: this is the full-blown Fedora rawhide config, apart from
disabling UCSI (it caused another problem that was fixed after -rc1); I
first had tried to use localmodconfig to strip down the config, but then
the problem did not happen for some reason (or I did something stupid).
Ciao, Thorsten
#regzbot introduced 92e24e0e57f72e
#regzbot title Input: psmouse - Resume broken on T14s Gen1 (AMD) due to
a new delay when deactivating for SMBus mode
On 27.09.23 17:55, Jeffery Miller wrote:
> On Wed, Sep 27, 2023 at 10:43 AM Jeffery Miller
> <[email protected]> wrote:
>> On Wed, Sep 27, 2023 at 3:54 AM Thorsten Leemhuis <[email protected]> wrote:
>>>
>>> My dmesg from a kernel with the revert:
>>> https://www.leemhuis.info/files/misc/dmesg
Thx for looking into this!
>> In this dmesg output it shows that this is an elantech smbus device:
>> ```
>> [ 4.260415] psmouse serio1: elantech: assuming hardware version 4 (with firmware version 0x7f3001)
>> [ 4.279297] psmouse serio1: elantech: Synaptics capabilities query result 0x90, 0x18, 0x0f.
>> [ 4.292788] psmouse serio1: elantech: Elan sample query result 00, 80, c9
>> [ 4.319184] psmouse serio1: elantech: Elan ic body: 0x10, current fw version: 0x3
>> ...
>> [ 4.346951] psmouse serio1: elantech: Trying to set up SMBus access
>> [ 4.346986] psmouse serio1: elantech: SMbus companion is not ready yet
>> [ 4.369993] input: ETPS/2 Elantech TrackPoint as /devices/platform/i8042/serio1/input/input7
>> [ 4.376200] systemd[1]: bpf-lsm: LSM BPF program attached
>> [ 4.385192] input: ETPS/2 Elantech Touchpad as /devices/platform/i8042/serio1/input/input5
>> ```
>> The change in 92e24e0e57f72e shouldn't affect the elantouch device as elantech_setup_smbus
>> initializes `psmouse_smbus_init` with need_deactivate = false.
Hmmm. Wondering if I should warm up the compiler again to recheck my
result one more time[1].
>> Did you store dmesg logs from boot without the applied patch?
>
> I intended to ask if you have logs from a boot without 92e24e0e57f72e reverted.
https://www.leemhuis.info/files/misc/dmesg-6.6-rc3-vanilla
>> If the delay was being applied the timestamps should show the 30ms delay between
>> `psmouse serio1: elantech: Trying to set up SMBus access`
>> and
>> `psmouse serio1: elantech: SMbus companion is not ready yet`
Unless I missed something there is not difference. :-/
Ciao, Thorsten
[1] FWIW, this is my bisect log
"""
> git bisect start
> # status: waiting for both good and bad commits
> # bad: [6465e260f48790807eef06b583b38ca9789b6072] Linux 6.6-rc3
> git bisect bad 6465e260f48790807eef06b583b38ca9789b6072
> # status: waiting for good commit(s), bad commit known
> # good: [2dde18cd1d8fac735875f2e4987f11817cc0bc2c] Linux 6.5
> git bisect good 2dde18cd1d8fac735875f2e4987f11817cc0bc2c
> # good: [4fb0dacb78c6a041bbd38ddd998df806af5c2c69] Merge tag 'sound-6.6-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound
> git bisect good 4fb0dacb78c6a041bbd38ddd998df806af5c2c69
> # good: [307d59039fb26212a84a9aa6a134a7d2bdea34ca] Merge tag 'media/v6.6-1' of git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media
> git bisect good 307d59039fb26212a84a9aa6a134a7d2bdea34ca
> # bad: [4a0fc73da97efd23a383ca839e6fe86410268f6b] Merge tag 's390-6.6-2' of git://git.kernel.org/pub/scm/linux/kernel/git/s390/linux
> git bisect bad 4a0fc73da97efd23a383ca839e6fe86410268f6b
> # good: [e4f1b8202fb59c56a3de7642d50326923670513f] Merge tag 'for_linus' of git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost
> git bisect good e4f1b8202fb59c56a3de7642d50326923670513f
> # good: [5eea5820c7340d39e56e169e1b87199391105f6b] Merge tag 'mm-stable-2023-09-04-14-00' of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> git bisect good 5eea5820c7340d39e56e169e1b87199391105f6b
> # good: [65d6e954e37872fd9afb5ef3fc0481bb3c2f20f4] Merge tag 'gfs2-v6.5-rc5-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2
> git bisect good 65d6e954e37872fd9afb5ef3fc0481bb3c2f20f4
> # bad: [744a759492b5c57ff24a6e8aabe47b17ad8ee964] Merge tag 'input-for-v6.6-rc0' of git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input
> git bisect bad 744a759492b5c57ff24a6e8aabe47b17ad8ee964
> # good: [dbce1a7d5dce7318d8465b1e0d052ef1d2202237] Input: Explicitly include correct DT includes
> git bisect good dbce1a7d5dce7318d8465b1e0d052ef1d2202237
> # good: [29057cc5bddc785ea0a11534d7ad2546fa0872d3] Merge tag 'linux-watchdog-6.6-rc1' of git://http://www.linux-watchdog.org/linux-watchdog
> git bisect good 29057cc5bddc785ea0a11534d7ad2546fa0872d3
> # bad: [3e4bb047b23375a34dbf5885709ac3729d9cfb22] Input: qt2160 - convert to use devm_* api
> git bisect bad 3e4bb047b23375a34dbf5885709ac3729d9cfb22
> # good: [e175eae16c1bf92062f1f431a95f476a61a77c48] Input: mcs-touchkey - convert to use devm_* api
> git bisect good e175eae16c1bf92062f1f431a95f476a61a77c48
> # bad: [92e24e0e57f72e06c2df87116557331fd2d4dda2] Input: psmouse - add delay when deactivating for SMBus mode
> git bisect bad 92e24e0e57f72e06c2df87116557331fd2d4dda2
> # good: [8362bf82fb5441613aac7c6c9dbb6b83def6ad3b] Input: mcs-touchkey - fix uninitialized use of error in mcs_touchkey_probe()
> git bisect good 8362bf82fb5441613aac7c6c9dbb6b83def6ad3b
> # first bad commit: [92e24e0e57f72e06c2df87116557331fd2d4dda2] Input: psmouse - add delay when deactivating for SMBus mode
"""
Hi Thorsten,
On Wed, Sep 27, 2023 at 10:43 AM Jeffery Miller
<[email protected]> wrote:
>
>
> On Wed, Sep 27, 2023 at 3:54 AM Thorsten Leemhuis <[email protected]> wrote:
>>
>> My dmesg from a kernel with the revert:
>> https://www.leemhuis.info/files/misc/dmesg
>>
> In this dmesg output it shows that this is an elantech smbus device:
> ```
> [ 4.260415] psmouse serio1: elantech: assuming hardware version 4 (with firmware version 0x7f3001)
> [ 4.279297] psmouse serio1: elantech: Synaptics capabilities query result 0x90, 0x18, 0x0f.
> [ 4.292788] psmouse serio1: elantech: Elan sample query result 00, 80, c9
> [ 4.319184] psmouse serio1: elantech: Elan ic body: 0x10, current fw version: 0x3
> ...
> [ 4.346951] psmouse serio1: elantech: Trying to set up SMBus access
> [ 4.346986] psmouse serio1: elantech: SMbus companion is not ready yet
> [ 4.369993] input: ETPS/2 Elantech TrackPoint as /devices/platform/i8042/serio1/input/input7
> [ 4.376200] systemd[1]: bpf-lsm: LSM BPF program attached
> [ 4.385192] input: ETPS/2 Elantech Touchpad as /devices/platform/i8042/serio1/input/input5
> ```
> The change in 92e24e0e57f72e shouldn't affect the elantouch device as elantech_setup_smbus
> initializes `psmouse_smbus_init` with need_deactivate = false.
>
> Did you store dmesg logs from boot without the applied patch?
I intended to ask if you have logs from a boot without 92e24e0e57f72e reverted.
> If the delay was being applied the timestamps should show the 30ms delay between
> `psmouse serio1: elantech: Trying to set up SMBus access`
> and
> `psmouse serio1: elantech: SMbus companion is not ready yet`
>
Thank You,
Jeff
On 27.09.23 19:23, Thorsten Leemhuis wrote:
> On 27.09.23 17:55, Jeffery Miller wrote:
>> On Wed, Sep 27, 2023 at 10:43 AM Jeffery Miller
>> <[email protected]> wrote:
>>> On Wed, Sep 27, 2023 at 3:54 AM Thorsten Leemhuis <[email protected]> wrote:
>>>>
>>>> My dmesg from a kernel with the revert:
>>>> https://www.leemhuis.info/files/misc/dmesg
>
> Thx for looking into this!
>
>>> In this dmesg output it shows that this is an elantech smbus device:
>>> ```
>>> [ 4.260415] psmouse serio1: elantech: assuming hardware version 4 (with firmware version 0x7f3001)
>>> [ 4.279297] psmouse serio1: elantech: Synaptics capabilities query result 0x90, 0x18, 0x0f.
>>> [ 4.292788] psmouse serio1: elantech: Elan sample query result 00, 80, c9
>>> [ 4.319184] psmouse serio1: elantech: Elan ic body: 0x10, current fw version: 0x3
>>> ...
>>> [ 4.346951] psmouse serio1: elantech: Trying to set up SMBus access
>>> [ 4.346986] psmouse serio1: elantech: SMbus companion is not ready yet
>>> [ 4.369993] input: ETPS/2 Elantech TrackPoint as /devices/platform/i8042/serio1/input/input7
>>> [ 4.376200] systemd[1]: bpf-lsm: LSM BPF program attached
>>> [ 4.385192] input: ETPS/2 Elantech Touchpad as /devices/platform/i8042/serio1/input/input5
>>> ```
>>> The change in 92e24e0e57f72e shouldn't affect the elantouch device as elantech_setup_smbus
>>> initializes `psmouse_smbus_init` with need_deactivate = false.
>
> Hmmm. Wondering if I should warm up the compiler again to recheck my
> result one more time[1].
Just did that. Ran "make clean" and compiled mainline as of now
(633b47cb009d) and the machine does never resume from s2idle; then I
reverted 92e24e0e57f7 and compiled again (for completeness: without
running "make clean" beforehand) and with that kernel s2idle resume
works perfectly fine.
Wondering if I or the compiler is doing something stupid here -- or if
we missed some small but important detail somewhere.
Ciao, Thorsten
>>> Did you store dmesg logs from boot without the applied patch?
>> I intended to ask if you have logs from a boot without 92e24e0e57f72e reverted.
>
> https://www.leemhuis.info/files/misc/dmesg-6.6-rc3-vanilla
>
>>> If the delay was being applied the timestamps should show the 30ms delay between
>>> `psmouse serio1: elantech: Trying to set up SMBus access`
>>> and
>>> `psmouse serio1: elantech: SMbus companion is not ready yet`
>
> Unless I missed something there is not difference. :-/
>
> Ciao, Thorsten
>
> [1] FWIW, this is my bisect log
>
> """
>> git bisect start
>> # status: waiting for both good and bad commits
>> # bad: [6465e260f48790807eef06b583b38ca9789b6072] Linux 6.6-rc3
>> git bisect bad 6465e260f48790807eef06b583b38ca9789b6072
>> # status: waiting for good commit(s), bad commit known
>> # good: [2dde18cd1d8fac735875f2e4987f11817cc0bc2c] Linux 6.5
>> git bisect good 2dde18cd1d8fac735875f2e4987f11817cc0bc2c
>> # good: [4fb0dacb78c6a041bbd38ddd998df806af5c2c69] Merge tag 'sound-6.6-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound
>> git bisect good 4fb0dacb78c6a041bbd38ddd998df806af5c2c69
>> # good: [307d59039fb26212a84a9aa6a134a7d2bdea34ca] Merge tag 'media/v6.6-1' of git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media
>> git bisect good 307d59039fb26212a84a9aa6a134a7d2bdea34ca
>> # bad: [4a0fc73da97efd23a383ca839e6fe86410268f6b] Merge tag 's390-6.6-2' of git://git.kernel.org/pub/scm/linux/kernel/git/s390/linux
>> git bisect bad 4a0fc73da97efd23a383ca839e6fe86410268f6b
>> # good: [e4f1b8202fb59c56a3de7642d50326923670513f] Merge tag 'for_linus' of git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost
>> git bisect good e4f1b8202fb59c56a3de7642d50326923670513f
>> # good: [5eea5820c7340d39e56e169e1b87199391105f6b] Merge tag 'mm-stable-2023-09-04-14-00' of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
>> git bisect good 5eea5820c7340d39e56e169e1b87199391105f6b
>> # good: [65d6e954e37872fd9afb5ef3fc0481bb3c2f20f4] Merge tag 'gfs2-v6.5-rc5-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2
>> git bisect good 65d6e954e37872fd9afb5ef3fc0481bb3c2f20f4
>> # bad: [744a759492b5c57ff24a6e8aabe47b17ad8ee964] Merge tag 'input-for-v6.6-rc0' of git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input
>> git bisect bad 744a759492b5c57ff24a6e8aabe47b17ad8ee964
>> # good: [dbce1a7d5dce7318d8465b1e0d052ef1d2202237] Input: Explicitly include correct DT includes
>> git bisect good dbce1a7d5dce7318d8465b1e0d052ef1d2202237
>> # good: [29057cc5bddc785ea0a11534d7ad2546fa0872d3] Merge tag 'linux-watchdog-6.6-rc1' of git://http://www.linux-watchdog.org/linux-watchdog
>> git bisect good 29057cc5bddc785ea0a11534d7ad2546fa0872d3
>> # bad: [3e4bb047b23375a34dbf5885709ac3729d9cfb22] Input: qt2160 - convert to use devm_* api
>> git bisect bad 3e4bb047b23375a34dbf5885709ac3729d9cfb22
>> # good: [e175eae16c1bf92062f1f431a95f476a61a77c48] Input: mcs-touchkey - convert to use devm_* api
>> git bisect good e175eae16c1bf92062f1f431a95f476a61a77c48
>> # bad: [92e24e0e57f72e06c2df87116557331fd2d4dda2] Input: psmouse - add delay when deactivating for SMBus mode
>> git bisect bad 92e24e0e57f72e06c2df87116557331fd2d4dda2
>> # good: [8362bf82fb5441613aac7c6c9dbb6b83def6ad3b] Input: mcs-touchkey - fix uninitialized use of error in mcs_touchkey_probe()
>> git bisect good 8362bf82fb5441613aac7c6c9dbb6b83def6ad3b
>> # first bad commit: [92e24e0e57f72e06c2df87116557331fd2d4dda2] Input: psmouse - add delay when deactivating for SMBus mode
> """
>
>
Hello,
On Sat, Sep 30, 2023 at 4:04 AM Thorsten Leemhuis <[email protected]> wrote:
>
> [FWIW, in case sending this in private happened accidentally, feel free
> to make this public again.]
This was unintentional. Replying back to the list.
> """
> > diff --git a/drivers/input/mouse/psmouse-smbus.c b/drivers/input/mouse/psmouse-smbus.c
> > index 7b13de979908..fe12385bb856 100644
> > --- a/drivers/input/mouse/psmouse-smbus.c
> > +++ b/drivers/input/mouse/psmouse-smbus.c
> > @@ -121,11 +121,11 @@ static psmouse_ret_t psmouse_smbus_process_byte(struct psmouse *psmouse)
> >
> > static void psmouse_activate_smbus_mode(struct psmouse_smbus_dev *smbdev)
> > {
> > - if (smbdev->need_deactivate) {
> > - psmouse_deactivate(smbdev->psmouse);
> > - /* Give the device time to switch into SMBus mode */
> > - msleep(30);
> > - }
> > + if (smbdev->psmouse == NULL) {
> > + printk("XXX: smbdev->psmouse is null\n");
> > + } else {
> > + printk("XXX: smbdev->psmouse is set\n");
> > + }
> > }
> >
> > static int psmouse_smbus_reconnect(struct psmouse *psmouse)
> """
>
> During boot this prints "XXX: smbdev->psmouse is set". But it helped, as
> the machine now resumes from s2idle again -- while printing "XXX:
> smbdev->psmouse is null". And that should not be the case I assume. Or
> did my brute force test go sideways due to my limited skills?
This was a good test. You've identified where it is crashing.
Maybe we could confirm that `psmouse->private` is not-NULL?:
```
diff --git a/drivers/input/mouse/psmouse-smbus.c
b/drivers/input/mouse/psmouse-smbus.c
index 7b13de979908..432615df9ae8 100644
--- a/drivers/input/mouse/psmouse-smbus.c
+++ b/drivers/input/mouse/psmouse-smbus.c
@@ -130,7 +130,10 @@ static void psmouse_activate_smbus_mode(struct
psmouse_smbus_dev *smbdev)
static int psmouse_smbus_reconnect(struct psmouse *psmouse)
{
- psmouse_activate_smbus_mode(psmouse->private);
+ if (psmouse->private == NULL) {
+ printk("XXX smbdev is null");
+ }
+ //psmouse_activate_smbus_mode(psmouse->private);
return 0;
}
```
Thanks,
Jeff
On Thu, Sep 28, 2023 at 4:08 AM Thorsten Leemhuis <[email protected]> wrote:
>
> On 27.09.23 19:23, Thorsten Leemhuis wrote:
> > On 27.09.23 17:55, Jeffery Miller wrote:
> >> On Wed, Sep 27, 2023 at 10:43 AM Jeffery Miller
> >> <[email protected]> wrote:
> >>> On Wed, Sep 27, 2023 at 3:54 AM Thorsten Leemhuis <[email protected]> wrote:
> >>>>
> >>>> My dmesg from a kernel with the revert:
> >>>> https://www.leemhuis.info/files/misc/dmesg
> >
> > Thx for looking into this!
> >
> >>> In this dmesg output it shows that this is an elantech smbus device:
> >>> ```
> >>> [ 4.260415] psmouse serio1: elantech: assuming hardware version 4 (with firmware version 0x7f3001)
> >>> [ 4.279297] psmouse serio1: elantech: Synaptics capabilities query result 0x90, 0x18, 0x0f.
> >>> [ 4.292788] psmouse serio1: elantech: Elan sample query result 00, 80, c9
> >>> [ 4.319184] psmouse serio1: elantech: Elan ic body: 0x10, current fw version: 0x3
> >>> ...
> >>> [ 4.346951] psmouse serio1: elantech: Trying to set up SMBus access
> >>> [ 4.346986] psmouse serio1: elantech: SMbus companion is not ready yet
> >>> [ 4.369993] input: ETPS/2 Elantech TrackPoint as /devices/platform/i8042/serio1/input/input7
> >>> [ 4.376200] systemd[1]: bpf-lsm: LSM BPF program attached
> >>> [ 4.385192] input: ETPS/2 Elantech Touchpad as /devices/platform/i8042/serio1/input/input5
> >>> ```
> >>> The change in 92e24e0e57f72e shouldn't affect the elantouch device as elantech_setup_smbus
> >>> initializes `psmouse_smbus_init` with need_deactivate = false.
> >
> > Hmmm. Wondering if I should warm up the compiler again to recheck my
> > result one more time[1].
>
> Just did that. Ran "make clean" and compiled mainline as of now
> (633b47cb009d) and the machine does never resume from s2idle; then I
> reverted 92e24e0e57f7 and compiled again (for completeness: without
> running "make clean" beforehand) and with that kernel s2idle resume
> works perfectly fine.
>
> Wondering if I or the compiler is doing something stupid here -- or if
> we missed some small but important detail somewhere.
>
> Ciao, Thorsten
>
> >>> Did you store dmesg logs from boot without the applied patch?
> >> I intended to ask if you have logs from a boot without 92e24e0e57f72e reverted.
> >
> > https://www.leemhuis.info/files/misc/dmesg-6.6-rc3-vanilla
> >
> >>> If the delay was being applied the timestamps should show the 30ms delay between
> >>> `psmouse serio1: elantech: Trying to set up SMBus access`
> >>> and
> >>> `psmouse serio1: elantech: SMbus companion is not ready yet`
> >
> > Unless I missed something there is not difference. :-/
> >
> > Ciao, Thorsten
> >
> > [1] FWIW, this is my bisect log
> >
> > """
> >> git bisect start
> >> # status: waiting for both good and bad commits
> >> # bad: [6465e260f48790807eef06b583b38ca9789b6072] Linux 6.6-rc3
> >> git bisect bad 6465e260f48790807eef06b583b38ca9789b6072
> >> # status: waiting for good commit(s), bad commit known
> >> # good: [2dde18cd1d8fac735875f2e4987f11817cc0bc2c] Linux 6.5
> >> git bisect good 2dde18cd1d8fac735875f2e4987f11817cc0bc2c
> >> # good: [4fb0dacb78c6a041bbd38ddd998df806af5c2c69] Merge tag 'sound-6.6-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound
> >> git bisect good 4fb0dacb78c6a041bbd38ddd998df806af5c2c69
> >> # good: [307d59039fb26212a84a9aa6a134a7d2bdea34ca] Merge tag 'media/v6.6-1' of git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media
> >> git bisect good 307d59039fb26212a84a9aa6a134a7d2bdea34ca
> >> # bad: [4a0fc73da97efd23a383ca839e6fe86410268f6b] Merge tag 's390-6.6-2' of git://git.kernel.org/pub/scm/linux/kernel/git/s390/linux
> >> git bisect bad 4a0fc73da97efd23a383ca839e6fe86410268f6b
> >> # good: [e4f1b8202fb59c56a3de7642d50326923670513f] Merge tag 'for_linus' of git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost
> >> git bisect good e4f1b8202fb59c56a3de7642d50326923670513f
> >> # good: [5eea5820c7340d39e56e169e1b87199391105f6b] Merge tag 'mm-stable-2023-09-04-14-00' of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> >> git bisect good 5eea5820c7340d39e56e169e1b87199391105f6b
> >> # good: [65d6e954e37872fd9afb5ef3fc0481bb3c2f20f4] Merge tag 'gfs2-v6.5-rc5-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2
> >> git bisect good 65d6e954e37872fd9afb5ef3fc0481bb3c2f20f4
> >> # bad: [744a759492b5c57ff24a6e8aabe47b17ad8ee964] Merge tag 'input-for-v6.6-rc0' of git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input
> >> git bisect bad 744a759492b5c57ff24a6e8aabe47b17ad8ee964
> >> # good: [dbce1a7d5dce7318d8465b1e0d052ef1d2202237] Input: Explicitly include correct DT includes
> >> git bisect good dbce1a7d5dce7318d8465b1e0d052ef1d2202237
> >> # good: [29057cc5bddc785ea0a11534d7ad2546fa0872d3] Merge tag 'linux-watchdog-6.6-rc1' of git://http://www.linux-watchdog.org/linux-watchdog
> >> git bisect good 29057cc5bddc785ea0a11534d7ad2546fa0872d3
> >> # bad: [3e4bb047b23375a34dbf5885709ac3729d9cfb22] Input: qt2160 - convert to use devm_* api
> >> git bisect bad 3e4bb047b23375a34dbf5885709ac3729d9cfb22
> >> # good: [e175eae16c1bf92062f1f431a95f476a61a77c48] Input: mcs-touchkey - convert to use devm_* api
> >> git bisect good e175eae16c1bf92062f1f431a95f476a61a77c48
> >> # bad: [92e24e0e57f72e06c2df87116557331fd2d4dda2] Input: psmouse - add delay when deactivating for SMBus mode
> >> git bisect bad 92e24e0e57f72e06c2df87116557331fd2d4dda2
> >> # good: [8362bf82fb5441613aac7c6c9dbb6b83def6ad3b] Input: mcs-touchkey - fix uninitialized use of error in mcs_touchkey_probe()
> >> git bisect good 8362bf82fb5441613aac7c6c9dbb6b83def6ad3b
> >> # first bad commit: [92e24e0e57f72e06c2df87116557331fd2d4dda2] Input: psmouse - add delay when deactivating for SMBus mode
> > """
> >
> >
On 02.10.23 18:52, Jeffery Miller wrote:
> On Sat, Sep 30, 2023 at 4:04 AM Thorsten Leemhuis <[email protected]> wrote:
>> """
>>> diff --git a/drivers/input/mouse/psmouse-smbus.c b/drivers/input/mouse/psmouse-smbus.c
>>> index 7b13de979908..fe12385bb856 100644
>>> --- a/drivers/input/mouse/psmouse-smbus.c
>>> +++ b/drivers/input/mouse/psmouse-smbus.c
>>> @@ -121,11 +121,11 @@ static psmouse_ret_t psmouse_smbus_process_byte(struct psmouse *psmouse)
>>>
>>> static void psmouse_activate_smbus_mode(struct psmouse_smbus_dev *smbdev)
>>> {
>>> - if (smbdev->need_deactivate) {
>>> - psmouse_deactivate(smbdev->psmouse);
>>> - /* Give the device time to switch into SMBus mode */
>>> - msleep(30);
>>> - }
>>> + if (smbdev->psmouse == NULL) {
>>> + printk("XXX: smbdev->psmouse is null\n");
>>> + } else {
>>> + printk("XXX: smbdev->psmouse is set\n");
>>> + }
>>> }
>>>
>>> static int psmouse_smbus_reconnect(struct psmouse *psmouse)
>> """
>>
>> During boot this prints "XXX: smbdev->psmouse is set". But it helped, as
>> the machine now resumes from s2idle again -- while printing "XXX:
>> smbdev->psmouse is null". And that should not be the case I assume. Or
>> did my brute force test go sideways due to my limited skills?
>
> This was a good test. You've identified where it is crashing.
>
> Maybe we could confirm that `psmouse->private` is not-NULL?:
> ```
> diff --git a/drivers/input/mouse/psmouse-smbus.c
> b/drivers/input/mouse/psmouse-smbus.c
> index 7b13de979908..432615df9ae8 100644
> --- a/drivers/input/mouse/psmouse-smbus.c
> +++ b/drivers/input/mouse/psmouse-smbus.c
> @@ -130,7 +130,10 @@ static void psmouse_activate_smbus_mode(struct
> psmouse_smbus_dev *smbdev)
>
> static int psmouse_smbus_reconnect(struct psmouse *psmouse)
> {
> - psmouse_activate_smbus_mode(psmouse->private);
> + if (psmouse->private == NULL) {
> + printk("XXX smbdev is null");
> + }
> + //psmouse_activate_smbus_mode(psmouse->private);
> return 0;
> }
> ```
This didn't print anything on resume, so `psmouse->private` apparently
is set.
Tried brute force again afterwards to find what might unset
smbdev->psmouse by adding printk statements to
psmouse_smbus_disconnect() and psmouse_smbus_cleanup() but those didn't
fire, so it must be something else I didn't spot.
Ciao, Thorsten
> On Thu, Sep 28, 2023 at 4:08 AM Thorsten Leemhuis <[email protected]> wrote:
>>
>> On 27.09.23 19:23, Thorsten Leemhuis wrote:
>>> On 27.09.23 17:55, Jeffery Miller wrote:
>>>> On Wed, Sep 27, 2023 at 10:43 AM Jeffery Miller
>>>> <[email protected]> wrote:
>>>>> On Wed, Sep 27, 2023 at 3:54 AM Thorsten Leemhuis <[email protected]> wrote:
>>>>>>
>>>>>> My dmesg from a kernel with the revert:
>>>>>> https://www.leemhuis.info/files/misc/dmesg
>>>
>>> Thx for looking into this!
>>>
>>>>> In this dmesg output it shows that this is an elantech smbus device:
>>>>> ```
>>>>> [ 4.260415] psmouse serio1: elantech: assuming hardware version 4 (with firmware version 0x7f3001)
>>>>> [ 4.279297] psmouse serio1: elantech: Synaptics capabilities query result 0x90, 0x18, 0x0f.
>>>>> [ 4.292788] psmouse serio1: elantech: Elan sample query result 00, 80, c9
>>>>> [ 4.319184] psmouse serio1: elantech: Elan ic body: 0x10, current fw version: 0x3
>>>>> ...
>>>>> [ 4.346951] psmouse serio1: elantech: Trying to set up SMBus access
>>>>> [ 4.346986] psmouse serio1: elantech: SMbus companion is not ready yet
>>>>> [ 4.369993] input: ETPS/2 Elantech TrackPoint as /devices/platform/i8042/serio1/input/input7
>>>>> [ 4.376200] systemd[1]: bpf-lsm: LSM BPF program attached
>>>>> [ 4.385192] input: ETPS/2 Elantech Touchpad as /devices/platform/i8042/serio1/input/input5
>>>>> ```
>>>>> The change in 92e24e0e57f72e shouldn't affect the elantouch device as elantech_setup_smbus
>>>>> initializes `psmouse_smbus_init` with need_deactivate = false.
>>>
>>> Hmmm. Wondering if I should warm up the compiler again to recheck my
>>> result one more time[1].
>>
>> Just did that. Ran "make clean" and compiled mainline as of now
>> (633b47cb009d) and the machine does never resume from s2idle; then I
>> reverted 92e24e0e57f7 and compiled again (for completeness: without
>> running "make clean" beforehand) and with that kernel s2idle resume
>> works perfectly fine.
>>
>> Wondering if I or the compiler is doing something stupid here -- or if
>> we missed some small but important detail somewhere.
>>
>> Ciao, Thorsten
>>
>>>>> Did you store dmesg logs from boot without the applied patch?
>>>> I intended to ask if you have logs from a boot without 92e24e0e57f72e reverted.
>>>
>>> https://www.leemhuis.info/files/misc/dmesg-6.6-rc3-vanilla
>>>
>>>>> If the delay was being applied the timestamps should show the 30ms delay between
>>>>> `psmouse serio1: elantech: Trying to set up SMBus access`
>>>>> and
>>>>> `psmouse serio1: elantech: SMbus companion is not ready yet`
>>>
>>> Unless I missed something there is not difference. :-/
>>>
>>> Ciao, Thorsten
>>>
>>> [1] FWIW, this is my bisect log
>>>
>>> """
>>>> git bisect start
>>>> # status: waiting for both good and bad commits
>>>> # bad: [6465e260f48790807eef06b583b38ca9789b6072] Linux 6.6-rc3
>>>> git bisect bad 6465e260f48790807eef06b583b38ca9789b6072
>>>> # status: waiting for good commit(s), bad commit known
>>>> # good: [2dde18cd1d8fac735875f2e4987f11817cc0bc2c] Linux 6.5
>>>> git bisect good 2dde18cd1d8fac735875f2e4987f11817cc0bc2c
>>>> # good: [4fb0dacb78c6a041bbd38ddd998df806af5c2c69] Merge tag 'sound-6.6-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound
>>>> git bisect good 4fb0dacb78c6a041bbd38ddd998df806af5c2c69
>>>> # good: [307d59039fb26212a84a9aa6a134a7d2bdea34ca] Merge tag 'media/v6.6-1' of git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media
>>>> git bisect good 307d59039fb26212a84a9aa6a134a7d2bdea34ca
>>>> # bad: [4a0fc73da97efd23a383ca839e6fe86410268f6b] Merge tag 's390-6.6-2' of git://git.kernel.org/pub/scm/linux/kernel/git/s390/linux
>>>> git bisect bad 4a0fc73da97efd23a383ca839e6fe86410268f6b
>>>> # good: [e4f1b8202fb59c56a3de7642d50326923670513f] Merge tag 'for_linus' of git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost
>>>> git bisect good e4f1b8202fb59c56a3de7642d50326923670513f
>>>> # good: [5eea5820c7340d39e56e169e1b87199391105f6b] Merge tag 'mm-stable-2023-09-04-14-00' of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
>>>> git bisect good 5eea5820c7340d39e56e169e1b87199391105f6b
>>>> # good: [65d6e954e37872fd9afb5ef3fc0481bb3c2f20f4] Merge tag 'gfs2-v6.5-rc5-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2
>>>> git bisect good 65d6e954e37872fd9afb5ef3fc0481bb3c2f20f4
>>>> # bad: [744a759492b5c57ff24a6e8aabe47b17ad8ee964] Merge tag 'input-for-v6.6-rc0' of git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input
>>>> git bisect bad 744a759492b5c57ff24a6e8aabe47b17ad8ee964
>>>> # good: [dbce1a7d5dce7318d8465b1e0d052ef1d2202237] Input: Explicitly include correct DT includes
>>>> git bisect good dbce1a7d5dce7318d8465b1e0d052ef1d2202237
>>>> # good: [29057cc5bddc785ea0a11534d7ad2546fa0872d3] Merge tag 'linux-watchdog-6.6-rc1' of git://http://www.linux-watchdog.org/linux-watchdog
>>>> git bisect good 29057cc5bddc785ea0a11534d7ad2546fa0872d3
>>>> # bad: [3e4bb047b23375a34dbf5885709ac3729d9cfb22] Input: qt2160 - convert to use devm_* api
>>>> git bisect bad 3e4bb047b23375a34dbf5885709ac3729d9cfb22
>>>> # good: [e175eae16c1bf92062f1f431a95f476a61a77c48] Input: mcs-touchkey - convert to use devm_* api
>>>> git bisect good e175eae16c1bf92062f1f431a95f476a61a77c48
>>>> # bad: [92e24e0e57f72e06c2df87116557331fd2d4dda2] Input: psmouse - add delay when deactivating for SMBus mode
>>>> git bisect bad 92e24e0e57f72e06c2df87116557331fd2d4dda2
>>>> # good: [8362bf82fb5441613aac7c6c9dbb6b83def6ad3b] Input: mcs-touchkey - fix uninitialized use of error in mcs_touchkey_probe()
>>>> git bisect good 8362bf82fb5441613aac7c6c9dbb6b83def6ad3b
>>>> # first bad commit: [92e24e0e57f72e06c2df87116557331fd2d4dda2] Input: psmouse - add delay when deactivating for SMBus mode
>>> """
>>>
>>>
>
>
On Tue, Oct 3, 2023 at 2:30 AM Thorsten Leemhuis <[email protected]> wrote:
>
> This didn't print anything on resume, so `psmouse->private` apparently
> is set.
>
Thank you for reporting this and providing the information!
need_deactivate is never being set on the smbdev struct since it's elantouch.
On this machine SMBus is not used so it falls back to PS/2 mode.
When this occurs the psmouse->private pointer is being replaced but
psmouse_smbus_reconnect is still being called on resume expecting smbdev.
That explains why when it is setup needs_deactivate is false, but on resume it
has somehow changed to true.
I've submitted a fix for this at
https://lore.kernel.org/all/[email protected]/
and it should resolve this issue for you.
Thanks,
Jeff
[TLDR: This mail in primarily relevant for Linux kernel regression
tracking. See link in footer if these mails annoy you.]
On 27.09.23 10:54, Thorsten Leemhuis wrote:
> Hi Jeffery! Your change 92e24e0e57f72e ("Input: psmouse - add delay when
> deactivating for SMBus mode") [merged for v6.6-rc1] broke resume on my
> T14s Gen1 (AMD): the system didn't really resume again at all (the
> display almost always didn't re-initialize) and there was nothing in the
> logs. I found your commit to be the culprit using a bisection and
> confirmed that reverting it on top of Linux 6.6-rc3 makes thing work
> again for me.
>
> #regzbot introduced 92e24e0e57f72e
> #regzbot title Input: psmouse - Resume broken on T14s Gen1 (AMD) due to
> a new delay when deactivating for SMBus mode
#regzbot monitor:
https://lore.kernel.org/all/[email protected]/
#regzbot ignore-activity
Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
That page also explains what to do if mails like this annoy you.