| Message ID | 20250327-videocc-pll-multi-pd-voting-v3-5-895fafd62627@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | clk: qcom: Add support to attach multiple power domains in cc probe | expand |
On Thu, Mar 27, 2025 at 03:22:25PM +0530, Jagadeesh Kona wrote: > Add support for runtime power management in qcom_cc_really_probe() to > commonize it across all the clock controllers. The runtime power management > is not required for all clock controllers, hence handle the rpm based on > use_rpm flag in clock controller descriptor. > > Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com> > --- > drivers/clk/qcom/common.c | 37 ++++++++++++++++++++++++++++--------- > drivers/clk/qcom/common.h | 1 + > 2 files changed, 29 insertions(+), 9 deletions(-) > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
On 27/03/2025 09:52, Jagadeesh Kona wrote: > - return ret; > + goto put_rpm; > + > + ret = qcom_cc_icc_register(dev, desc); > + > +put_rpm: > + if (desc->use_rpm) > + pm_runtime_put(dev); > > - return qcom_cc_icc_register(dev, desc); > + return ret; > } > EXPORT_SYMBOL_GPL(qcom_cc_really_probe); Doesn't look right you're missing the put if register goes wrong ret = qcom_cc_icc_register(dev, desc); if (ret) goto put_rpm; return 0; put_rpm: if (desc->us_rpm) pm_runtime_put(); return ret;
On 3/27/2025 9:28 PM, Bryan O'Donoghue wrote: > On 27/03/2025 09:52, Jagadeesh Kona wrote: >> - return ret; >> + goto put_rpm; >> + >> + ret = qcom_cc_icc_register(dev, desc); >> + >> +put_rpm: >> + if (desc->use_rpm) >> + pm_runtime_put(dev); >> - return qcom_cc_icc_register(dev, desc); >> + return ret; >> } >> EXPORT_SYMBOL_GPL(qcom_cc_really_probe); > > Doesn't look right you're missing the put if register goes wrong > The intention is to call pm_runtime_put() regardless of the return value from qcom_cc_icc_register(), as it is the final API call. Therefore, the return type is not checked, and pm_runtime_put() is called in both success and failure cases before returning the final return code. Thanks, Jagadeesh > ret = qcom_cc_icc_register(dev, desc); > > if (ret) > goto put_rpm; > > return 0; > > put_rpm: > if (desc->us_rpm) > pm_runtime_put(); > > return ret;
diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c index 9e3380fd718198c9fe63d7361615a91c3ecb3d60..9cbf1c5296dad3ee5477a2f5a445488707663b9d 100644 --- a/drivers/clk/qcom/common.c +++ b/drivers/clk/qcom/common.c @@ -9,6 +9,7 @@ #include <linux/platform_device.h> #include <linux/clk-provider.h> #include <linux/interconnect-clk.h> +#include <linux/pm_runtime.h> #include <linux/reset-controller.h> #include <linux/of.h> @@ -304,6 +305,16 @@ int qcom_cc_really_probe(struct device *dev, if (ret < 0 && ret != -EEXIST) return ret; + if (desc->use_rpm) { + ret = devm_pm_runtime_enable(dev); + if (ret) + return ret; + + ret = pm_runtime_resume_and_get(dev); + if (ret) + return ret; + } + reset = &cc->reset; reset->rcdev.of_node = dev->of_node; reset->rcdev.ops = &qcom_reset_ops; @@ -314,23 +325,25 @@ int qcom_cc_really_probe(struct device *dev, ret = devm_reset_controller_register(dev, &reset->rcdev); if (ret) - return ret; + goto put_rpm; if (desc->gdscs && desc->num_gdscs) { scd = devm_kzalloc(dev, sizeof(*scd), GFP_KERNEL); - if (!scd) - return -ENOMEM; + if (!scd) { + ret = -ENOMEM; + goto put_rpm; + } scd->dev = dev; scd->scs = desc->gdscs; scd->num = desc->num_gdscs; scd->pd_list = cc->pd_list; ret = gdsc_register(scd, &reset->rcdev, regmap); if (ret) - return ret; + goto put_rpm; ret = devm_add_action_or_reset(dev, qcom_cc_gdsc_unregister, scd); if (ret) - return ret; + goto put_rpm; } cc->rclks = rclks; @@ -341,7 +354,7 @@ int qcom_cc_really_probe(struct device *dev, for (i = 0; i < num_clk_hws; i++) { ret = devm_clk_hw_register(dev, clk_hws[i]); if (ret) - return ret; + goto put_rpm; } for (i = 0; i < num_clks; i++) { @@ -350,14 +363,20 @@ int qcom_cc_really_probe(struct device *dev, ret = devm_clk_register_regmap(dev, rclks[i]); if (ret) - return ret; + goto put_rpm; } ret = devm_of_clk_add_hw_provider(dev, qcom_cc_clk_hw_get, cc); if (ret) - return ret; + goto put_rpm; + + ret = qcom_cc_icc_register(dev, desc); + +put_rpm: + if (desc->use_rpm) + pm_runtime_put(dev); - return qcom_cc_icc_register(dev, desc); + return ret; } EXPORT_SYMBOL_GPL(qcom_cc_really_probe); diff --git a/drivers/clk/qcom/common.h b/drivers/clk/qcom/common.h index 7ace5d7f5836aa81431153ba92d8f14f2ffe8147..9c10bc8c197cd7dfa25ccd245763ad6acb081523 100644 --- a/drivers/clk/qcom/common.h +++ b/drivers/clk/qcom/common.h @@ -38,6 +38,7 @@ struct qcom_cc_desc { const struct qcom_icc_hws_data *icc_hws; size_t num_icc_hws; unsigned int icc_first_node_id; + bool use_rpm; }; /**
Add support for runtime power management in qcom_cc_really_probe() to commonize it across all the clock controllers. The runtime power management is not required for all clock controllers, hence handle the rpm based on use_rpm flag in clock controller descriptor. Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com> --- drivers/clk/qcom/common.c | 37 ++++++++++++++++++++++++++++--------- drivers/clk/qcom/common.h | 1 + 2 files changed, 29 insertions(+), 9 deletions(-)