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;
MIME-Version: 1.0
References: <20230427221625.116050-1-opendmb@gmail.com>
In-Reply-To: <20230427221625.116050-1-opendmb@gmail.com>
From:   Saravana Kannan <saravanak@google.com>
Date:   Mon, 1 May 2023 13:40:57 -0700
Message-ID: <CAGETcx-gDcqY7-_Ud0_rOtgvk7NbzevSa4UCV=NcqiV9zjAv7w@mail.gmail.com>
Subject: Re: [RFC PATCH 0/3] input: gpio-keys - fix pm ordering
To:     Doug Berger <opendmb@gmail.com>
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "Rafael J. Wysocki" <rafael@kernel.org>,
        Dmitry Torokhov <dmitry.torokhov@gmail.com>,
        Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>,
        Gergo Koteles <soyer@irl.hu>,
        Jonathan Cameron <Jonathan.Cameron@huawei.com>,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        Dan Williams <dan.j.williams@intel.com>,
        Hans de Goede <hdegoede@redhat.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Kees Cook <keescook@chromium.org>,
        Kishon Vijay Abraham I <kishon@ti.com>,
        Florian Fainelli <f.fainelli@gmail.com>,
        linux-kernel@vger.kernel.org, linux-input@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, Apr 27, 2023 at 3:18=E2=80=AFPM Doug Berger <opendmb@gmail.com> wro=
te:
>
> 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=3Don (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
>