Thread (98 messages) 98 messages, 7 authors, 2024-11-07

Re: [PATCH 00/16] Add initial USB support for the Renesas RZ/G3S SoC

From: Ulf Hansson <hidden>
Date: 2024-08-31 10:32:40
Also in: linux-arm-kernel, linux-clk, linux-phy, linux-pm, linux-renesas-soc, linux-usb, lkml

[...]
quoted
If not, there are two other options that can be considered I think.
*) Using the genpd on/off notifiers, to really allow the consumer
driver of the reset-control to know when the PM domain gets turned
on/off.
**) Move the entire reset handling into the PM domain provider, as it
obviously knows when the domain is getting turned on/off.
This option is what I've explored, tested on my side.

I explored it in 2 ways:

1/ SYSC modeled as an individual PM domain provider (this is more
   appropriate to how HW manual described the hardware) with this the PHY
   reset DT node would have to get 2 PM domains handlers (one for the
   current PM domain provider and the other one for SYSC):

+               phyrst: usbphy-ctrl@11e00000 {
+                       compatible = "renesas,r9a08g045-usbphy-ctrl";
+                       reg = <0 0x11e00000 0 0x10000>;
+                       clocks = <&cpg CPG_MOD R9A08G045_USB_PCLK>;
+                       resets = <&cpg R9A08G045_USB_PRESETN>;
+                       power-domain-names = "cpg", "sysc";
+                       power-domains = <&cpg R9A08G045_PD_USB_PHY>, <&sysc
R9A08G045_SYSC_PD_USB>;
+                       #reset-cells = <1>;
+                       status = "disabled";
+
+                       usb0_vbus_otg: regulator-vbus {
+                               regulator-name = "vbus";
+                       };
+               };
+
According to what you have described earlier/above, modelling the SYSC
as a PM domain provider seems like a better description of the HW to
me. Although, as I said earlier, if you prefer the reset approach, I
would not object to that.
and the PHY reset driver will get bulky with powering on/off both of these,
at least with my current implementation, something like (and the following
code is in probe()):

+       if (priv->set_power) {
+               priv->cpg_genpd_dev = dev_pm_domain_attach_by_name(dev, "cpg");
+               if (IS_ERR(priv->cpg_genpd_dev)) {
+                       dev_err_probe(dev, error, "Failed to attach CPG PM
domain!");
+                       error = PTR_ERR(priv->cpg_genpd_dev);
+                       goto err_pm_runtime_put;
+               }
+
+               priv->sysc_genpd_dev = dev_pm_domain_attach_by_name(dev,
"sysc");
+               if (IS_ERR(priv->sysc_genpd_dev)) {
+                       dev_err_probe(dev, error, "Failed to attach sysc PM
domain!");
+                       error = PTR_ERR(priv->sysc_genpd_dev);
+                       goto err_genpd_cpg_detach;
+               }
+
+               priv->cpg_genpd_dl = device_link_add(dev, priv->cpg_genpd_dev,
+                                                    DL_FLAG_PM_RUNTIME |
+                                                    DL_FLAG_STATELESS);
+               if (!priv->cpg_genpd_dl) {
+                       dev_err_probe(dev, -ENOMEM, "Failed to add CPG
genpd device link!");
+                       goto err_genpd_sysc_detach;
+               }
+
+               priv->sysc_genpd_dl = device_link_add(dev,
priv->sysc_genpd_dev,
+                                                     DL_FLAG_PM_RUNTIME |
+                                                     DL_FLAG_STATELESS);
+               if (!priv->sysc_genpd_dl) {
+                       dev_err_probe(dev, -ENOMEM, "Failed to add sysc
genpd device link!");
+                       goto err_genpd_cpg_dl_del;
+               }
+
+
+               error = pm_runtime_resume_and_get(priv->cpg_genpd_dev);
+               if (error) {
+                       dev_err_probe(dev, error, "Failed to runtime resume
cpg PM domain!");
+                       goto err_genpd_sysc_dl_del;
+               }
+
+               error = pm_runtime_resume_and_get(priv->sysc_genpd_dev);
+               if (error) {
+                       dev_err_probe(dev, error, "Failed to runtime resume
sysc PM domain!");
+                       goto err_genpd_cpg_off;
+               }
+       }
Indeed, the code above looks bulky.

Fortunately, we now have dev|devm_pm_domain_attach_list(), which
replaces all of the code above.

[...]

Kind regards
Uffe
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help