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 for Android: free password hash cracker in your pocket

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

Message-Id: <CX15J7B8F8HH.1WZ10OOW31X1H@tleb-bootlin-xps13-01>
Date:   Fri, 17 Nov 2023 15:20:44 +0100
From:   Théo Lebrun <theo.lebrun@...tlin.com>
To:     "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

Hi Roger,

On Fri Nov 17, 2023 at 12:51 PM CET, Roger Quadros wrote:
> On 17/11/2023 12:17, Théo Lebrun wrote:
> > On Thu Nov 16, 2023 at 10:44 PM CET, Roger Quadros wrote:
> >> On 16/11/2023 20:56, Théo Lebrun wrote:
> >>> On Thu Nov 16, 2023 at 1:40 PM CET, Roger Quadros wrote:
> >>>> On 15/11/2023 17:02, Théo Lebrun wrote:
> >>>>> On Wed Nov 15, 2023 at 12:37 PM CET, Roger Quadros wrote:
> >>>>>> You might want to check suspend/resume ops in cdns3-plat and
> >>>>>> do something similar here.
> >>>>>
> >>>>> I'm unsure what you are referring to specifically in cdns3-plat?
> >>>>
> >>>> What I meant is, calling pm_runtime_get/put() from system suspend/resume
> >>>> hooks doesn't seem right.
> >>>>
> >>>> How about using something like pm_runtime_forbid(dev) on devices which
> >>>> loose USB context on runtime suspend e.g. J7200.
> >>>> So at probe we can get rid of the pm_runtime_get_sync() call.
> >>>
> >>> What is the goal of enabling PM runtime to then block (ie forbid) it in
> >>> its enabled state until system suspend?
> >>
> >> If USB controller retains context on runtime_suspend on some platforms
> >> then we don't want to forbid PM runtime.
> > 
> > What's the point of runtime PM if nothing is done based on it? This is
> > the current behavior of the driver.
>
> Even if driver doesn't have runtime_suspend/resume hooks, wouldn't 
> the USB Power domain turn off if we enable runtime PM and allow runtime
> autosuspend and all children have runtime suspended?

That cannot be the currently desired behavior as it would require a
runtime_resume implementation that restores this wrapper to its desired
state.

It could however be something that could be implemented. It would be a
matter of enabling PM runtime and that is it in the probe. No need to
even init the hardware in the probe. Then the runtime_resume
implementation would call the new cdns_ti_init_hw.

This is what the cdns3-imx wrapper is doing in a way, though what they
need is clocks rather than some registers to be written.

That all feels like outside the scope of the current patch series
though.

My suggestion for V2 would still therefore be to remove all PM runtime
as it has no impact. It could be added later down the road if cutting
the power-domain is a goal of yours.

Thanks,

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