From: Kevin Hilman <khilman@kernel.org> To: "Théo Lebrun" <theo.lebrun@bootlin.com>, "Roger Quadros" <rogerq@kernel.org>, "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>, "Rob Herring" <robh+dt@kernel.org>, "Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>, "Conor Dooley" <conor+dt@kernel.org>, "Peter Chen" <peter.chen@kernel.org>, "Pawel Laszczak" <pawell@cadence.com>, "Nishanth Menon" <nm@ti.com>, "Vignesh Raghavendra" <vigneshr@ti.com>, "Tero Kristo" <kristo@kernel.org>, "Vardhan, Vibhore" <vibhore@ti.com> Cc: linux-usb@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, "Grégory Clement" <gregory.clement@bootlin.com>, "Thomas Petazzoni" <thomas.petazzoni@bootlin.com> Subject: Re: [PATCH 3/6] usb: cdns3-ti: add suspend/resume procedures for J7200 Date: Sun, 26 Nov 2023 14:36:10 -0800 [thread overview] Message-ID: <7hil5odtwl.fsf@baylibre.com> (raw) In-Reply-To: <CX63KP2UPL1N.J9Q344Q06IGP@tleb-bootlin-xps13-01> Théo Lebrun <theo.lebrun@bootlin.com> writes: > Hello, > > On Wed Nov 22, 2023 at 11:23 PM CET, Kevin Hilman wrote: >> Théo Lebrun <theo.lebrun@bootlin.com> writes: >> > 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. >> >> 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. > The call chain that attaches & enables PD is platform_probe -> > dev_pm_domain_attach. That function takes a bool power_on which is > always true. In the genpd case, genpd_dev_pm_attach > calls __genpd_dev_pm_attach which does a genpd_power_on. > > Things I've not accounted for: > > - ACPI looks like it does the same but I've not checked. It gets passed > the power_on bool argument. > > - genpd_dev_pm_attach early returns with no error if there are multiple > PM domains attached to the device. There are many platforms in the > case according to some devicetree grepping. I can imagine a behavior > difference where dev_pm_domain callpaths handle that differently in > the RPM case. Is that what we are discussing? You're only looking at the attach, power-on phase. You need to think about when the domain powers off and powers back on. Kevin
WARNING: multiple messages have this Message-ID (diff)
From: Kevin Hilman <khilman@kernel.org> To: "Théo Lebrun" <theo.lebrun@bootlin.com>, "Roger Quadros" <rogerq@kernel.org>, "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>, "Rob Herring" <robh+dt@kernel.org>, "Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>, "Conor Dooley" <conor+dt@kernel.org>, "Peter Chen" <peter.chen@kernel.org>, "Pawel Laszczak" <pawell@cadence.com>, "Nishanth Menon" <nm@ti.com>, "Vignesh Raghavendra" <vigneshr@ti.com>, "Tero Kristo" <kristo@kernel.org>, "Vardhan, Vibhore" <vibhore@ti.com> Cc: linux-usb@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, "Grégory Clement" <gregory.clement@bootlin.com>, "Thomas Petazzoni" <thomas.petazzoni@bootlin.com> Subject: Re: [PATCH 3/6] usb: cdns3-ti: add suspend/resume procedures for J7200 Date: Sun, 26 Nov 2023 14:36:10 -0800 [thread overview] Message-ID: <7hil5odtwl.fsf@baylibre.com> (raw) In-Reply-To: <CX63KP2UPL1N.J9Q344Q06IGP@tleb-bootlin-xps13-01> Théo Lebrun <theo.lebrun@bootlin.com> writes: > Hello, > > On Wed Nov 22, 2023 at 11:23 PM CET, Kevin Hilman wrote: >> Théo Lebrun <theo.lebrun@bootlin.com> writes: >> > 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. >> >> 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. > The call chain that attaches & enables PD is platform_probe -> > dev_pm_domain_attach. That function takes a bool power_on which is > always true. In the genpd case, genpd_dev_pm_attach > calls __genpd_dev_pm_attach which does a genpd_power_on. > > Things I've not accounted for: > > - ACPI looks like it does the same but I've not checked. It gets passed > the power_on bool argument. > > - genpd_dev_pm_attach early returns with no error if there are multiple > PM domains attached to the device. There are many platforms in the > case according to some devicetree grepping. I can imagine a behavior > difference where dev_pm_domain callpaths handle that differently in > the RPM case. Is that what we are discussing? You're only looking at the attach, power-on phase. You need to think about when the domain powers off and powers back on. Kevin _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2023-11-26 22:36 UTC|newest] Thread overview: 70+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-11-13 14:26 [PATCH 0/6] usb: cdns: fix suspend on J7200 by assuming reset on resume Théo Lebrun 2023-11-13 14:26 ` Théo Lebrun 2023-11-13 14:26 ` [PATCH 1/6] dt-bindings: usb: ti,j721e-usb: add ti,j7200-usb compatible Théo Lebrun 2023-11-13 14:26 ` Théo Lebrun 2023-11-13 19:58 ` Conor Dooley 2023-11-13 19:58 ` Conor Dooley 2023-11-13 14:26 ` [PATCH 2/6] usb: cdns3-ti: move reg writes from probe into an init_hw helper Théo Lebrun 2023-11-13 14:26 ` Théo Lebrun 2023-11-15 11:33 ` Roger Quadros 2023-11-15 11:33 ` Roger Quadros 2023-11-15 14:23 ` Théo Lebrun 2023-11-15 14:23 ` Théo Lebrun 2023-11-16 12:00 ` Roger Quadros 2023-11-16 12:00 ` Roger Quadros 2023-11-13 14:26 ` [PATCH 3/6] usb: cdns3-ti: add suspend/resume procedures for J7200 Théo Lebrun 2023-11-13 14:26 ` Théo Lebrun 2023-11-13 15:39 ` Gregory CLEMENT 2023-11-13 15:39 ` Gregory CLEMENT 2023-11-14 11:13 ` Théo Lebrun 2023-11-14 11:13 ` Théo Lebrun 2023-11-15 11:37 ` Roger Quadros 2023-11-15 11:37 ` Roger Quadros 2023-11-15 15:02 ` Théo Lebrun 2023-11-15 15:02 ` Théo Lebrun 2023-11-16 12:40 ` Roger Quadros 2023-11-16 12:40 ` Roger Quadros 2023-11-16 18:56 ` Théo Lebrun 2023-11-16 18:56 ` Théo Lebrun 2023-11-16 21:44 ` Roger Quadros 2023-11-16 21:44 ` Roger Quadros 2023-11-17 10:17 ` Théo Lebrun 2023-11-17 10:17 ` Théo Lebrun 2023-11-17 11:51 ` Roger Quadros 2023-11-17 11:51 ` Roger Quadros 2023-11-17 14:20 ` Théo Lebrun 2023-11-17 14:20 ` Théo Lebrun 2023-11-18 10:41 ` Roger Quadros 2023-11-18 10:41 ` Roger Quadros 2023-11-22 22:23 ` Kevin Hilman 2023-11-22 22:23 ` Kevin Hilman 2023-11-23 9:51 ` Théo Lebrun 2023-11-23 9:51 ` Théo Lebrun 2023-11-26 22:36 ` Kevin Hilman [this message] 2023-11-26 22:36 ` Kevin Hilman 2023-11-27 13:25 ` Théo Lebrun 2023-11-27 13:25 ` Théo Lebrun 2023-12-12 18:26 ` Kevin Hilman 2023-12-12 18:26 ` Kevin Hilman 2023-12-12 19:31 ` Alan Stern 2023-12-12 19:31 ` Alan Stern 2023-11-13 14:26 ` [PATCH 4/6] usb: cdns3: support power-off of controller when in host role Théo Lebrun 2023-11-13 14:26 ` Théo Lebrun 2023-11-14 8:38 ` Peter Chen 2023-11-14 8:38 ` Peter Chen 2023-11-14 11:10 ` Théo Lebrun 2023-11-14 11:10 ` Théo Lebrun 2023-11-17 3:38 ` Peter Chen 2023-11-17 3:38 ` Peter Chen 2023-11-17 9:58 ` Théo Lebrun 2023-11-17 9:58 ` Théo Lebrun 2023-11-20 5:44 ` Peter Chen 2023-11-20 5:44 ` Peter Chen 2023-11-13 14:27 ` [PATCH 5/6] usb: cdns3-ti: notify cdns core that hardware resets across suspend on J7200 Théo Lebrun 2023-11-13 14:27 ` Théo Lebrun 2023-11-13 14:27 ` [PATCH 6/6] arm64: dts: ti: k3-j7200: use J7200-specific USB compatible Théo Lebrun 2023-11-13 14:27 ` Théo Lebrun 2023-11-14 10:01 ` Gregory CLEMENT 2023-11-14 10:01 ` Gregory CLEMENT 2023-11-14 11:14 ` Théo Lebrun 2023-11-14 11:14 ` Théo Lebrun
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=7hil5odtwl.fsf@baylibre.com \ --to=khilman@kernel.org \ --cc=conor+dt@kernel.org \ --cc=devicetree@vger.kernel.org \ --cc=gregkh@linuxfoundation.org \ --cc=gregory.clement@bootlin.com \ --cc=kristo@kernel.org \ --cc=krzysztof.kozlowski+dt@linaro.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-usb@vger.kernel.org \ --cc=nm@ti.com \ --cc=pawell@cadence.com \ --cc=peter.chen@kernel.org \ --cc=robh+dt@kernel.org \ --cc=rogerq@kernel.org \ --cc=theo.lebrun@bootlin.com \ --cc=thomas.petazzoni@bootlin.com \ --cc=vibhore@ti.com \ --cc=vigneshr@ti.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.