| Message ID | 20250108012846.3275443-6-swboyd@chromium.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | qcom: Add an SoC PM driver for sc7180 using PM domains | expand |
On Tue, Jan 07, 2025 at 05:28:42PM -0800, Stephen Boyd wrote: > Find the watchdog device described as a child node of the sc7180 SoC > node and attach a generic pm domain to the device before registering the > device with the platform bus. The domain simply gets the clk and turns > it on when the pm domain is powered on and turns it off when the pm > domain is powered off. > > Cc: Rob Herring <robh@kernel.org> > Cc: Bjorn Andersson <andersson@kernel.org> > Cc: Konrad Dybcio <konradybcio@kernel.org> > Cc: <linux-arm-msm@vger.kernel.org> > Signed-off-by: Stephen Boyd <swboyd@chromium.org> > --- > drivers/bus/qcom/qcom-sc7180.c | 122 +++++++++++++++++++++++++++++++++ > 1 file changed, 122 insertions(+) > > diff --git a/drivers/bus/qcom/qcom-sc7180.c b/drivers/bus/qcom/qcom-sc7180.c > index a615cf5a2129..7dfe6b32efef 100644 > --- a/drivers/bus/qcom/qcom-sc7180.c > +++ b/drivers/bus/qcom/qcom-sc7180.c > @@ -3,18 +3,140 @@ > * SoC bus driver for Qualcomm SC7180 SoCs > */ > > +#include <linux/cleanup.h> > +#include <linux/clk.h> > #include <linux/device.h> > +#include <linux/dev_printk.h> > #include <linux/init.h> > #include <linux/mod_devicetable.h> > #include <linux/module.h> > #include <linux/of.h> > #include <linux/of_platform.h> > #include <linux/platform_device.h> > +#include <linux/pm.h> > +#include <linux/pm_domain.h> > + > +struct qcom_soc_pm_domain { > + struct clk *clk; > + struct generic_pm_domain pd; > +}; > + > +static struct qcom_soc_pm_domain * > +gpd_to_qcom_soc_pm_domain(struct generic_pm_domain *gpd) > +{ > + return container_of(gpd, struct qcom_soc_pm_domain, pd); > +} > + > +static struct qcom_soc_pm_domain *pd_to_qcom_soc_pm_domain(struct dev_pm_domain *pd) > +{ > + struct generic_pm_domain *gpd; > + > + gpd = container_of(pd, struct generic_pm_domain, domain); > + > + return gpd_to_qcom_soc_pm_domain(gpd); > +} > + > +static struct qcom_soc_pm_domain *dev_to_qcom_soc_pm_domain(struct device *dev) > +{ > + struct dev_pm_domain *pd; > + > + pd = dev->pm_domain; > + if (!pd) > + return NULL; > + > + return pd_to_qcom_soc_pm_domain(pd); > +} > + > +static struct platform_device * > +qcom_soc_alloc_device(struct platform_device *socdev, const char *compatible) > +{ > + struct device_node *np __free(device_node); > + > + np = of_get_compatible_child(socdev->dev.of_node, compatible); > + > + return of_platform_device_alloc(np, NULL, &socdev->dev); > +} > + > +static int qcom_soc_domain_activate(struct device *dev) > +{ > + struct qcom_soc_pm_domain *soc_domain; > + > + dev_info(dev, "Activating device\n"); > + soc_domain = dev_to_qcom_soc_pm_domain(dev); > + > + soc_domain->clk = devm_clk_get(dev, NULL); > + > + return PTR_ERR_OR_ZERO(soc_domain->clk); > +} > + > +static int qcom_soc_domain_power_on(struct generic_pm_domain *domain) > +{ > + struct qcom_soc_pm_domain *soc_domain; > + > + pr_info("Powering on device\n"); > + soc_domain = gpd_to_qcom_soc_pm_domain(domain); > + > + return clk_prepare_enable(soc_domain->clk); > +} > + > +static int qcom_soc_domain_power_off(struct generic_pm_domain *domain) > +{ > + struct qcom_soc_pm_domain *soc_domain; > + > + pr_info("Powering off device\n"); > + soc_domain = gpd_to_qcom_soc_pm_domain(domain); > + > + clk_disable_unprepare(soc_domain->clk); How's this going to scale when there are multiple clocks and it's not just turn on/off all the clocks in any order? Or when there's ordering requirements between different resources. I'm pretty sure I've seen attempts to order clock entries in DT based on the order they want to enable them. > + > + return 0; > +} > + > +static int qcom_soc_add_clk_domain(struct platform_device *socdev, > + struct platform_device *pdev) > +{ > + struct qcom_soc_pm_domain *domain; > + struct generic_pm_domain *pd; > + int ret; > + > + domain = devm_kzalloc(&socdev->dev, sizeof(*domain), GFP_KERNEL); > + if (!domain) > + return -ENOMEM; > + > + pd = &domain->pd; > + pd->name = "wdog"; > + ret = pm_genpd_init(pd, NULL, false); > + if (ret) > + return ret; > + > + /* TODO: Wrap this in a generic_pm_domain function similar to power_on() */ > + pd->domain.activate = qcom_soc_domain_activate; > + pd->power_on = qcom_soc_domain_power_on; > + pd->power_off = qcom_soc_domain_power_off; > + > + dev_info(&socdev->dev, "adding pm domain for %s\n", dev_name(&pdev->dev)); > + dev_pm_domain_set(&pdev->dev, &pd->domain); > + > + return 0; > +} > > static int qcom_soc_sc7180_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > struct device_node *np = dev->of_node; > + struct platform_device *sdev; > + int ret; > + > + sdev = qcom_soc_alloc_device(pdev, "qcom,apss-wdt-sc7180"); We're going to have to have an explicit call for every child node? > + if (!sdev) > + return dev_err_probe(dev, -ENODEV, "Failed to alloc sdev\n"); > + > + ret = qcom_soc_add_clk_domain(pdev, sdev); > + if (ret) > + return dev_err_probe(dev, ret, "Failed to add clk domain to sdev\n"); > + > + ret = of_platform_device_add(sdev); > + if (ret) > + return dev_err_probe(dev, ret, "Failed to add sdev to bus\n"); > > return of_platform_populate(np, NULL, NULL, dev); > } > -- > https://chromeos.dev >
Quoting Rob Herring (2025-01-10 06:09:34) > > diff --git a/drivers/bus/qcom/qcom-sc7180.c b/drivers/bus/qcom/qcom-sc7180.c > > index a615cf5a2129..7dfe6b32efef 100644 > > --- a/drivers/bus/qcom/qcom-sc7180.c > > +++ b/drivers/bus/qcom/qcom-sc7180.c > > @@ -3,18 +3,140 @@ > > * SoC bus driver for Qualcomm SC7180 SoCs > > */ > > > > +#include <linux/cleanup.h> > > +#include <linux/clk.h> > > #include <linux/device.h> > > +#include <linux/dev_printk.h> [...] > > + > > +static int qcom_soc_domain_power_on(struct generic_pm_domain *domain) > > +{ > > + struct qcom_soc_pm_domain *soc_domain; > > + > > + pr_info("Powering on device\n"); > > + soc_domain = gpd_to_qcom_soc_pm_domain(domain); > > + > > + return clk_prepare_enable(soc_domain->clk); > > +} > > + > > +static int qcom_soc_domain_power_off(struct generic_pm_domain *domain) > > +{ > > + struct qcom_soc_pm_domain *soc_domain; > > + > > + pr_info("Powering off device\n"); > > + soc_domain = gpd_to_qcom_soc_pm_domain(domain); > > + > > + clk_disable_unprepare(soc_domain->clk); > > How's this going to scale when there are multiple clocks and it's not > just turn on/off all the clocks in any order? Or when there's ordering > requirements between different resources. We'll need different power_on/power_off functions when the ordering requirements are different. It would be similar to how we have different clk_ops for different types of clks. > > I'm pretty sure I've seen attempts to order clock entries in DT based on > the order they want to enable them. Yes, I've seen that too. The order in DT shouldn't matter. The SoC PM driver will know what order of operations to take, including between different resources like resets, interconnects, power-domains, etc. That's the general idea of this driver, it will coordinate all the power for devices in the soc node, because it's written for that particular SoC. For example, if there are ordering requirements it can get clks by name and "do the right thing". Another goal I have is to power off devices that aren't bound to a driver, and/or never will be. If we can figure out the runtime PM state of devices before adding them to the platform bus it would allow us to power those devices off at runtime or during system suspend if userspace isn't actively trying to power down devices. Maybe to do that we'll have to be notified by subsystem frameworks when a provider is registered and then once all the providers are registered, get the PM resources like clks and interconnects, etc., figure out if they're on/off, set the runtime PM state of the device to match, and finally add the device to the bus. Then we can extend the driver PM core to allow userspace to turn off devices that aren't bound to a driver, because we've moved the SoC PM glue out of each driver into this one driver that both adds the devices to the system and manages the device power. If a node is status = "disabled" I'd like to still get all the PM resources for that device and either put them into one overall SoC PM domain, or in an "unused device" domain that we can have userspace tell the kernel it's safe to power down those devices that were left in a (semi-)powered state by the bootloader. Obviously I haven't gotten to this point yet, but it's another TODO item. We could also populate those devices but never let them be bound to a driver because they're marked disabled in DT. Then we don't have to do anything different from devices that are ok to use, but we waste kernel memory. Either way, the PM resources for disabled devices need to be dealt with. > > > + > > + return 0; > > +} > > + > > +static int qcom_soc_add_clk_domain(struct platform_device *socdev, > > + struct platform_device *pdev) > > +{ > > + struct qcom_soc_pm_domain *domain; > > + struct generic_pm_domain *pd; > > + int ret; > > + > > + domain = devm_kzalloc(&socdev->dev, sizeof(*domain), GFP_KERNEL); > > + if (!domain) > > + return -ENOMEM; > > + > > + pd = &domain->pd; > > + pd->name = "wdog"; > > + ret = pm_genpd_init(pd, NULL, false); > > + if (ret) > > + return ret; > > + > > + /* TODO: Wrap this in a generic_pm_domain function similar to power_on() */ > > + pd->domain.activate = qcom_soc_domain_activate; > > + pd->power_on = qcom_soc_domain_power_on; > > + pd->power_off = qcom_soc_domain_power_off; > > + > > + dev_info(&socdev->dev, "adding pm domain for %s\n", dev_name(&pdev->dev)); > > + dev_pm_domain_set(&pdev->dev, &pd->domain); > > + > > + return 0; > > +} > > > > static int qcom_soc_sc7180_probe(struct platform_device *pdev) > > { > > struct device *dev = &pdev->dev; > > struct device_node *np = dev->of_node; > > + struct platform_device *sdev; > > + int ret; > > + > > + sdev = qcom_soc_alloc_device(pdev, "qcom,apss-wdt-sc7180"); > > We're going to have to have an explicit call for every child node? Probably! Or we can populate the "complicated" ones that require something different and then populate the rest of the nodes that didn't get populated explicitly with some sort of loop over non-populated nodes that attaches a simple pm domain that does generic on/off sequencing. I haven't gotten that far to know if populating everything explicitly will be a problem.
diff --git a/drivers/bus/qcom/qcom-sc7180.c b/drivers/bus/qcom/qcom-sc7180.c index a615cf5a2129..7dfe6b32efef 100644 --- a/drivers/bus/qcom/qcom-sc7180.c +++ b/drivers/bus/qcom/qcom-sc7180.c @@ -3,18 +3,140 @@ * SoC bus driver for Qualcomm SC7180 SoCs */ +#include <linux/cleanup.h> +#include <linux/clk.h> #include <linux/device.h> +#include <linux/dev_printk.h> #include <linux/init.h> #include <linux/mod_devicetable.h> #include <linux/module.h> #include <linux/of.h> #include <linux/of_platform.h> #include <linux/platform_device.h> +#include <linux/pm.h> +#include <linux/pm_domain.h> + +struct qcom_soc_pm_domain { + struct clk *clk; + struct generic_pm_domain pd; +}; + +static struct qcom_soc_pm_domain * +gpd_to_qcom_soc_pm_domain(struct generic_pm_domain *gpd) +{ + return container_of(gpd, struct qcom_soc_pm_domain, pd); +} + +static struct qcom_soc_pm_domain *pd_to_qcom_soc_pm_domain(struct dev_pm_domain *pd) +{ + struct generic_pm_domain *gpd; + + gpd = container_of(pd, struct generic_pm_domain, domain); + + return gpd_to_qcom_soc_pm_domain(gpd); +} + +static struct qcom_soc_pm_domain *dev_to_qcom_soc_pm_domain(struct device *dev) +{ + struct dev_pm_domain *pd; + + pd = dev->pm_domain; + if (!pd) + return NULL; + + return pd_to_qcom_soc_pm_domain(pd); +} + +static struct platform_device * +qcom_soc_alloc_device(struct platform_device *socdev, const char *compatible) +{ + struct device_node *np __free(device_node); + + np = of_get_compatible_child(socdev->dev.of_node, compatible); + + return of_platform_device_alloc(np, NULL, &socdev->dev); +} + +static int qcom_soc_domain_activate(struct device *dev) +{ + struct qcom_soc_pm_domain *soc_domain; + + dev_info(dev, "Activating device\n"); + soc_domain = dev_to_qcom_soc_pm_domain(dev); + + soc_domain->clk = devm_clk_get(dev, NULL); + + return PTR_ERR_OR_ZERO(soc_domain->clk); +} + +static int qcom_soc_domain_power_on(struct generic_pm_domain *domain) +{ + struct qcom_soc_pm_domain *soc_domain; + + pr_info("Powering on device\n"); + soc_domain = gpd_to_qcom_soc_pm_domain(domain); + + return clk_prepare_enable(soc_domain->clk); +} + +static int qcom_soc_domain_power_off(struct generic_pm_domain *domain) +{ + struct qcom_soc_pm_domain *soc_domain; + + pr_info("Powering off device\n"); + soc_domain = gpd_to_qcom_soc_pm_domain(domain); + + clk_disable_unprepare(soc_domain->clk); + + return 0; +} + +static int qcom_soc_add_clk_domain(struct platform_device *socdev, + struct platform_device *pdev) +{ + struct qcom_soc_pm_domain *domain; + struct generic_pm_domain *pd; + int ret; + + domain = devm_kzalloc(&socdev->dev, sizeof(*domain), GFP_KERNEL); + if (!domain) + return -ENOMEM; + + pd = &domain->pd; + pd->name = "wdog"; + ret = pm_genpd_init(pd, NULL, false); + if (ret) + return ret; + + /* TODO: Wrap this in a generic_pm_domain function similar to power_on() */ + pd->domain.activate = qcom_soc_domain_activate; + pd->power_on = qcom_soc_domain_power_on; + pd->power_off = qcom_soc_domain_power_off; + + dev_info(&socdev->dev, "adding pm domain for %s\n", dev_name(&pdev->dev)); + dev_pm_domain_set(&pdev->dev, &pd->domain); + + return 0; +} static int qcom_soc_sc7180_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; struct device_node *np = dev->of_node; + struct platform_device *sdev; + int ret; + + sdev = qcom_soc_alloc_device(pdev, "qcom,apss-wdt-sc7180"); + if (!sdev) + return dev_err_probe(dev, -ENODEV, "Failed to alloc sdev\n"); + + ret = qcom_soc_add_clk_domain(pdev, sdev); + if (ret) + return dev_err_probe(dev, ret, "Failed to add clk domain to sdev\n"); + + ret = of_platform_device_add(sdev); + if (ret) + return dev_err_probe(dev, ret, "Failed to add sdev to bus\n"); return of_platform_populate(np, NULL, NULL, dev); }
Find the watchdog device described as a child node of the sc7180 SoC node and attach a generic pm domain to the device before registering the device with the platform bus. The domain simply gets the clk and turns it on when the pm domain is powered on and turns it off when the pm domain is powered off. Cc: Rob Herring <robh@kernel.org> Cc: Bjorn Andersson <andersson@kernel.org> Cc: Konrad Dybcio <konradybcio@kernel.org> Cc: <linux-arm-msm@vger.kernel.org> Signed-off-by: Stephen Boyd <swboyd@chromium.org> --- drivers/bus/qcom/qcom-sc7180.c | 122 +++++++++++++++++++++++++++++++++ 1 file changed, 122 insertions(+)