Re: [RFC PATCH 0/3] input: gpio-keys - fix pm ordering

[Saravana Kannan]

On Thu, Apr 27, 2023 at 3:18 PM Doug Berger <opendmb@xxxxxxxxx> wrote:
>
> Commit 52cdbdd49853 ("driver core: correct device's shutdown
> order") allowed for proper sequencing of the gpio-keys device
> shutdown callbacks by moving the device to the end of the
> devices_kset list at probe which was delayed by child
> dependencies.
>
> However, commit 722e5f2b1eec ("driver core: Partially revert
> "driver core: correct device's shutdown order"") removed this
> portion of that commit causing a reversion in the gpio-keys
> behavior which can prevent waking from shutdown.
>
> This RFC is an attempt to find a better solution for properly
> creating gpio-keys device links to ensure its suspend/resume and
> shutdown services are invoked before those of its suppliers.
>
> The first patch here is pretty brute force but simple and has
> the advantage that it should be easily backportable to the
> versions where the regression first occurred.

We really shouldn't be calling device_pm_move_to_tail() in drivers
because device link uses device_pm_move_to_tail() for ordering too.
And it becomes a "race" between device links and when the driver calls
device_pm_move_to_tail() and I'm not sure we'll get the same ordering
every time.

>
> The second patch is perhaps better in spirit though still a bit
> inelegant, but it can only be backported to versions of the
> kernel that contain the commit in its 'Fixes:' tag. That isn't
> really a valid 'Fixes:' tag since that commit did not cause the
> regression, but it does represent how far the patch could be
> backported.
>
> Both commits shouldn't really exist in the same kernel so the
> third patch reverts the first in an attempt to make that clear
> (though it may be a source of confusion for some).
>
> Hopefully someone with a better understanding of device links
> will see a less intrusive way to automatically capture these
> dependencies for parent device drivers that implement the
> functions of child node devices.

Can you give a summary of the issue on a real system? I took a look at
the two commits you've referenced above and it's not clear what's
still broken in the 6.3+

But I'd think that just teaching fw_devlink about some property should
be sufficient. If you are facing a real issue, have you made sure you
have fw_devlink=on (this is the default unless you turned it off in
the commandline when it had issues in the past).

-Saravana

>
> Doug Berger (3):
>   input: gpio-keys - use device_pm_move_to_tail
>   input: gpio-keys - add device links of children
>   Revert "input: gpio-keys - use device_pm_move_to_tail"
>
>  drivers/input/keyboard/gpio_keys.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> --
> 2.34.1
>