linux-kernel - Re: [PATCH 3/6] usb: cdns3-ti: add suspend/resume procedures for J7200

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <CX9MMPFL7HAY.NGULD1FN5WPN@tleb-bootlin-xps13-01>
Date:   Mon, 27 Nov 2023 14:25:58 +0100
From:   Théo Lebrun <theo.lebrun@...tlin.com>
To:     "Kevin Hilman" <khilman@...nel.org>,
        "Roger Quadros" <rogerq@...nel.org>,
        "Greg Kroah-Hartman" <gregkh@...uxfoundation.org>,
        "Rob Herring" <robh+dt@...nel.org>,
        "Krzysztof Kozlowski" <krzysztof.kozlowski+dt@...aro.org>,
        "Conor Dooley" <conor+dt@...nel.org>,
        "Peter Chen" <peter.chen@...nel.org>,
        "Pawel Laszczak" <pawell@...ence.com>,
        "Nishanth Menon" <nm@...com>,
        "Vignesh Raghavendra" <vigneshr@...com>,
        "Tero Kristo" <kristo@...nel.org>,
        "Vardhan, Vibhore" <vibhore@...com>
Cc:     <linux-usb@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        Grégory Clement <gregory.clement@...tlin.com>,
        "Thomas Petazzoni" <thomas.petazzoni@...tlin.com>
Subject: Re: [PATCH 3/6] usb: cdns3-ti: add suspend/resume procedures for
 J7200

Hello,

On Sun Nov 26, 2023 at 11:36 PM CET, Kevin Hilman wrote:
> Théo Lebrun <theo.lebrun@...tlin.com> writes:
> > On Wed Nov 22, 2023 at 11:23 PM CET, Kevin Hilman wrote:
> >> Théo Lebrun <theo.lebrun@...tlin.com> writes:
> >> The point is to signal to the power domain the device is in that it can
> >> power on/off.  These IP blocks are (re)used on many different SoCs, so
> >> the driver should not make any assumptions about what power domain it is
> >> in (if any.)
> >
> > On my platform, when the device is attached to the PD it gets turned on.
> > That feels logical to me: if a driver is not RPM aware it "just works".
>
> It "just works"... until the domain gets turned off.
>
> > Are there platforms where RPM must get enabled for the attached
> > power-domains to be turned on?
>
> Yes, but but more importantly, there are platforms where RPM must get
> enabled for the power domain to *stay* on.  For example, the power
> domain might get turned on due to devices probing etc, but as soon as
> all the RPM-enabled drivers drop their refcount, the domain will turn
> off.  If there is a device in that domain with a non-RPM enabled driver,
> that device will be powered off anc cause a crash.

OK, that makes sense, thanks for taking the time to explain. This topic
makes me see two things that I feel are close to being bugs. I'd be
curious to get your view on both.

 - If a device does not use RPM but its children do, it might get its
   associated power-domain turned off. That forces every single driver
   that want to stay alive to enable & increment RPM.

   What I naively expect: a genpd with a device attached to it that is
   not using RPM should mean that it should not be powered off at
   runtime_suspend. Benefit: no RPM calls in drivers that do not use
   it, and the behavior is that the genpd associated stays alive "as
   expected".

 - If a device uses RPM & has a refcount strictly positive, its
   associated power-domain gets turned off either way at suspend_noirq.
   That feels non-intuitive as well.

   What I naively expect: check for RPM refcounts of attached devices
   when doing suspend_noirq of power-domains. Benefit: control of what
   power-domains do from attached devices is done through the RPM API.

Regards,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com