| Message ID | 20241128-b4-linux-next-24-11-18-clock-multiple-power-domains-v5-2-ca2826c46814@linaro.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | clk: qcom: Add support for multiple power-domains for a clock controller. | expand |
Hi Bryan, On 11/28/24 18:38, Bryan O'Donoghue wrote: > Right now we support one power-domain per clock controller. > These single power-domains are switched on by the driver platform logic. > > However when we have multiple power-domains attached to a clock-controller > that list of power-domains must be handled outside of driver platform > logic. > > Use devm_pm_domain_attach_list() to automatically hook the list of given > power-domains in the dtsi for the clock-controller driver. > > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > --- > drivers/clk/qcom/common.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c > index 33cc1f73c69d1f875a193aea0552902268dc8716..e6a024e95ab5f4b0776ffc6c7b3bebfbebb007fd 100644 > --- a/drivers/clk/qcom/common.c > +++ b/drivers/clk/qcom/common.c > @@ -22,6 +22,7 @@ struct qcom_cc { > struct qcom_reset_controller reset; > struct clk_regmap **rclks; > size_t num_rclks; > + struct dev_pm_domain_list *pd_list; > }; > > const > @@ -294,11 +295,19 @@ int qcom_cc_really_probe(struct device *dev, > struct clk_regmap **rclks = desc->clks; > size_t num_clk_hws = desc->num_clk_hws; > struct clk_hw **clk_hws = desc->clk_hws; > + struct dev_pm_domain_attach_data pd_data = { > + .pd_names = 0, > + .num_pd_names = 0, > + }; > > cc = devm_kzalloc(dev, sizeof(*cc), GFP_KERNEL); > if (!cc) > return -ENOMEM; > > + ret = devm_pm_domain_attach_list(dev, &pd_data, &cc->pd_list); Please make a call to the function like this: ret = devm_pm_domain_attach_list(dev, NULL, &cc->pd_list); Here pd_data is unused. > + if (ret < 0 && ret != -EEXIST) > + return ret; > + > reset = &cc->reset; > reset->rcdev.of_node = dev->of_node; > reset->rcdev.ops = &qcom_reset_ops; > -- Best wishes, Vladimir
On 29/11/2024 11:30, Vladimir Zapolskiy wrote: >> + ret = devm_pm_domain_attach_list(dev, &pd_data, &cc->pd_list); > > Please make a call to the function like this: > > ret = devm_pm_domain_attach_list(dev, NULL, &cc->pd_list); Passing &pd_data will cause devm_pd_domain_attach_list() to cycle through the power-domains listed and do dev_pm_domain_attach_by_id(); instead of dv_pm_domain_attach_by_name(); That's what &pd_data is passed here. You want to have that simple attachment of the power-domain list. --- bod
On 11/29/24 13:39, Bryan O'Donoghue wrote: > On 29/11/2024 11:30, Vladimir Zapolskiy wrote: >>> + ret = devm_pm_domain_attach_list(dev, &pd_data, &cc->pd_list); >> >> Please make a call to the function like this: >> >> ret = devm_pm_domain_attach_list(dev, NULL, &cc->pd_list); > > Passing &pd_data will cause devm_pd_domain_attach_list() to cycle > through the power-domains listed and do dev_pm_domain_attach_by_id(); Doesn't it cycle for pd_data.num_pd_names times? Which is zero. > instead of dv_pm_domain_attach_by_name(); > > That's what &pd_data is passed here. You want to have that simple > attachment of the power-domain list. I look at dev_pm_domain_attach_list() function with my best efforts to concentrate and see no functional difference between your version and the one proposed by me since v1. -- Best wishes, Vladimir
On 29/11/2024 11:44, Vladimir Zapolskiy wrote: > On 11/29/24 13:39, Bryan O'Donoghue wrote: >> On 29/11/2024 11:30, Vladimir Zapolskiy wrote: >>>> + ret = devm_pm_domain_attach_list(dev, &pd_data, &cc->pd_list); >>> >>> Please make a call to the function like this: >>> >>> ret = devm_pm_domain_attach_list(dev, NULL, &cc->pd_list); >> >> Passing &pd_data will cause devm_pd_domain_attach_list() to cycle >> through the power-domains listed and do dev_pm_domain_attach_by_id(); > > Doesn't it cycle for pd_data.num_pd_names times? Which is zero. > >> instead of dv_pm_domain_attach_by_name(); >> >> That's what &pd_data is passed here. You want to have that simple >> attachment of the power-domain list. > > I look at dev_pm_domain_attach_list() function with my best efforts > to concentrate and see no functional difference between your version > and the one proposed by me since v1. > > -- > Best wishes, > Vladimir You're right. I tested again, NULL works. I'll v6. --- bod
diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c index 33cc1f73c69d1f875a193aea0552902268dc8716..e6a024e95ab5f4b0776ffc6c7b3bebfbebb007fd 100644 --- a/drivers/clk/qcom/common.c +++ b/drivers/clk/qcom/common.c @@ -22,6 +22,7 @@ struct qcom_cc { struct qcom_reset_controller reset; struct clk_regmap **rclks; size_t num_rclks; + struct dev_pm_domain_list *pd_list; }; const @@ -294,11 +295,19 @@ int qcom_cc_really_probe(struct device *dev, struct clk_regmap **rclks = desc->clks; size_t num_clk_hws = desc->num_clk_hws; struct clk_hw **clk_hws = desc->clk_hws; + struct dev_pm_domain_attach_data pd_data = { + .pd_names = 0, + .num_pd_names = 0, + }; cc = devm_kzalloc(dev, sizeof(*cc), GFP_KERNEL); if (!cc) return -ENOMEM; + ret = devm_pm_domain_attach_list(dev, &pd_data, &cc->pd_list); + if (ret < 0 && ret != -EEXIST) + return ret; + reset = &cc->reset; reset->rcdev.of_node = dev->of_node; reset->rcdev.ops = &qcom_reset_ops;
Right now we support one power-domain per clock controller. These single power-domains are switched on by the driver platform logic. However when we have multiple power-domains attached to a clock-controller that list of power-domains must be handled outside of driver platform logic. Use devm_pm_domain_attach_list() to automatically hook the list of given power-domains in the dtsi for the clock-controller driver. Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> --- drivers/clk/qcom/common.c | 9 +++++++++ 1 file changed, 9 insertions(+)