[...]
> >
> > 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
