[PATCH v6 0/3] clk: qcom: Add support for multiple power-domains for a clock controller.
Changes in v6: - Passes NULL to second parameter of devm_pm_domain_attach_list - Vlad - Link to v5: https://lore.kernel.org/r/20241128-b4-linux-next-24-11-18-clock-multiple-pow...
Changes in v5: - In-lines devm_pm_domain_attach_list() in probe() directly - Vlad - Link to v4: https://lore.kernel.org/r/20241127-b4-linux-next-24-11-18-clock-multiple-pow...
v4: - Adds Bjorn's RB to first patch - Bjorn - Drops the 'd' in "and int" - Bjorn - Amends commit log of patch 3 to capture a number of open questions - Bjorn - Link to v3: https://lore.kernel.org/r/20241126-b4-linux-next-24-11-18-clock-multiple-pow...
v3: - Fixes commit log "per which" - Bryan - Link to v2: https://lore.kernel.org/r/20241125-b4-linux-next-24-11-18-clock-multiple-pow...
v2: The main change in this version is Bjorn's pointing out that pm_runtime_* inside of the gdsc_enable/gdsc_disable path would be recursive and cause a lockdep splat. Dmitry alluded to this too.
Bjorn pointed to stuff being done lower in the gdsc_register() routine that might be a starting point.
I iterated around that idea and came up with patch #3. When a gdsc has no parent and the pd_list is non-NULL then attach that orphan GDSC to the clock controller power-domain list.
Existing subdomain code in gdsc_register() will connect the parent GDSCs in the clock-controller to the clock-controller subdomain, the new code here does that same job for a list of power-domains the clock controller depends on.
To Dmitry's point about MMCX and MCX dependencies for the registers inside of the clock controller, I have switched off all references in a test dtsi and confirmed that accessing the clock-controller regs themselves isn't required.
On the second point I also verified my test branch with lockdep on which was a concern with the pm_domain version of this solution but I wanted to cover it anyway with the new approach for completeness sake.
Here's the item-by-item list of changes:
- Adds a patch to capture pm_genpd_add_subdomain() result code - Bryan - Changes changelog of second patch to remove singleton and generally to make the commit log easier to understand - Bjorn - Uses demv_pm_domain_attach_list - Vlad - Changes error check to if (ret < 0 && ret != -EEXIST) - Vlad - Retains passing &pd_data instead of NULL - because NULL doesn't do the same thing - Bryan/Vlad - Retains standalone function qcom_cc_pds_attach() because the pd_data enumeration looks neater in a standalone function - Bryan/Vlad - Drops pm_runtime in favour of gdsc_add_subdomain_list() for each power-domain in the pd_list. The pd_list will be whatever is pointed to by power-domains = <> in the dtsi - Bjorn - Link to v1: https://lore.kernel.org/r/20241118-b4-linux-next-24-11-18-clock-multiple-pow...
v1: On x1e80100 and it's SKUs the Camera Clock Controller - CAMCC has multiple power-domains which power it. Usually with a single power-domain the core platform code will automatically switch on the singleton power-domain for you. If you have multiple power-domains for a device, in this case the clock controller, you need to switch those power-domains on/off yourself.
The clock controllers can also contain Global Distributed Switch Controllers - GDSCs which themselves can be referenced from dtsi nodes ultimately triggering a gdsc_en() in drivers/clk/qcom/gdsc.c.
As an example:
cci0: cci@ac4a000 { power-domains = <&camcc TITAN_TOP_GDSC>; };
This series adds the support to attach a power-domain list to the clock-controllers and the GDSCs those controllers provide so that in the case of the above example gdsc_toggle_logic() will trigger the power-domain list with pm_runtime_resume_and_get() and pm_runtime_put_sync() respectively.
Signed-off-by: Bryan O'Donoghue bryan.odonoghue@linaro.org --- Bryan O'Donoghue (3): clk: qcom: gdsc: Capture pm_genpd_add_subdomain result code clk: qcom: common: Add support for power-domain attachment clk: qcom: Support attaching GDSCs to multiple parents
drivers/clk/qcom/common.c | 6 ++++++ drivers/clk/qcom/gdsc.c | 41 +++++++++++++++++++++++++++++++++++++++-- drivers/clk/qcom/gdsc.h | 1 + 3 files changed, 46 insertions(+), 2 deletions(-) --- base-commit: 744cf71b8bdfcdd77aaf58395e068b7457634b2c change-id: 20241118-b4-linux-next-24-11-18-clock-multiple-power-domains-a5f994dc452a
Best regards,
Adding a new clause to this if/else I noticed the existing usage of pm_genpd_add_subdomain() wasn't capturing and returning the result code.
pm_genpd_add_subdomain() returns an int and can fail. Capture that result code and throw it up the call stack if something goes wrong.
Fixes: 1b771839de05 ("clk: qcom: gdsc: enable optional power domain support") Cc: stable@vger.kernel.org Reviewed-by: Bjorn Andersson andersson@kernel.org Signed-off-by: Bryan O'Donoghue bryan.odonoghue@linaro.org --- drivers/clk/qcom/gdsc.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c index fa5fe4c2a2ee7786c2e8858f3e41301f639e5d59..4fc6f957d0b846cc90e50ef243f23a7a27e66899 100644 --- a/drivers/clk/qcom/gdsc.c +++ b/drivers/clk/qcom/gdsc.c @@ -555,9 +555,11 @@ int gdsc_register(struct gdsc_desc *desc, if (!scs[i]) continue; if (scs[i]->parent) - pm_genpd_add_subdomain(scs[i]->parent, &scs[i]->pd); + ret = pm_genpd_add_subdomain(scs[i]->parent, &scs[i]->pd); else if (!IS_ERR_OR_NULL(dev->pm_domain)) - pm_genpd_add_subdomain(pd_to_genpd(dev->pm_domain), &scs[i]->pd); + ret = pm_genpd_add_subdomain(pd_to_genpd(dev->pm_domain), &scs[i]->pd); + if (ret) + return ret; }
return of_genpd_add_provider_onecell(dev->of_node, data);
linux-stable-mirror@lists.linaro.org
-
Bryan O'Donoghue