| Message ID | 49422d258d67d33a2547fbb7f4f6e72d489c2301.1697781921.git.quic_varada@quicinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | viresh kumar |
| Headers | show |
| Series | Enable cpufreq for IPQ5332 & IPQ9574 | expand |
On 10/20/23 08:19, Varadarajan Narayanan wrote: > Stromer Plus PLL found on IPQ53xx doesn't support dynamic > frequency scaling. To achieve the same, we need to park the APPS > PLL source to GPLL0, re configure the PLL and then switch the > source to APSS_PLL_EARLY. > > To support this, register a clock notifier to get the PRE_RATE > and POST_RATE notification. Change the APSS PLL source to GPLL0 > when PRE_RATE notification is received, then configure the PLL > and then change back the source to APSS_PLL_EARLY. > > Additionally, not all SKUs of IPQ53xx support scaling. Hence, > do the above to the SKUs that support scaling. > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com> > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com> > --- This looks good, however I'm not sure if registering the callback only on SoCs that are supposed to rate-change isn't overly-defensive. That said, I'm not particularly against it. Acked-by: Konrad Dybcio <konrad.dybcio@linaro.org> Konrad
Quoting Varadarajan Narayanan (2023-10-19 23:19:35) > Stromer Plus PLL found on IPQ53xx doesn't support dynamic > frequency scaling. To achieve the same, we need to park the APPS > PLL source to GPLL0, re configure the PLL and then switch the > source to APSS_PLL_EARLY. > > To support this, register a clock notifier to get the PRE_RATE > and POST_RATE notification. Change the APSS PLL source to GPLL0 > when PRE_RATE notification is received, then configure the PLL > and then change back the source to APSS_PLL_EARLY. > > Additionally, not all SKUs of IPQ53xx support scaling. Hence, > do the above to the SKUs that support scaling. > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com> > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com> The Kconfig change (patch#1) should be squashed into here. Otherwise there isn't a call to qcom_smem_get_soc_id() in the clk driver. > --- > v5: clk_notifier_register -> devm_clk_notifier_register > v3: devm_kzalloc for cpu_clk_notifier instead of global static > v2: Handle ABORT_RATE_CHANGE > Use local variable for apcs_alias0_clk_src.clkr.hw > Use single line comment instead of multi line style > --- > drivers/clk/qcom/apss-ipq6018.c | 58 ++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 57 insertions(+), 1 deletion(-) > > diff --git a/drivers/clk/qcom/apss-ipq6018.c b/drivers/clk/qcom/apss-ipq6018.c > index 4e13a08..e6295b8 100644 > --- a/drivers/clk/qcom/apss-ipq6018.c > +++ b/drivers/clk/qcom/apss-ipq6018.c > @@ -9,8 +9,11 @@ > #include <linux/clk-provider.h> > #include <linux/regmap.h> > #include <linux/module.h> > +#include <linux/clk.h> > +#include <linux/soc/qcom/smem.h> > > #include <dt-bindings/clock/qcom,apss-ipq.h> > +#include <dt-bindings/arm/qcom,ids.h> > > #include "common.h" > #include "clk-regmap.h" > @@ -84,15 +87,68 @@ static const struct qcom_cc_desc apss_ipq6018_desc = { > .num_clks = ARRAY_SIZE(apss_ipq6018_clks), > }; > > +static int cpu_clk_notifier_fn(struct notifier_block *nb, unsigned long action, > + void *data) > +{ > + struct clk_hw *hw; > + u8 index; > + int err; > + > + if (action == PRE_RATE_CHANGE) > + index = P_GPLL0; > + else if (action == POST_RATE_CHANGE || action == ABORT_RATE_CHANGE) > + index = P_APSS_PLL_EARLY; > + else > + return NOTIFY_OK; > + > + hw = &apcs_alias0_clk_src.clkr.hw; > + err = clk_rcg2_mux_closest_ops.set_parent(hw, index); > + > + return notifier_from_errno(err); > +} > + > static int apss_ipq6018_probe(struct platform_device *pdev) > { > + struct clk_hw *hw = &apcs_alias0_clk_src.clkr.hw; > + struct notifier_block *cpu_clk_notifier; > struct regmap *regmap; > + u32 soc_id; > + int ret; > + > + ret = qcom_smem_get_soc_id(&soc_id); You need to explain why this information can't come from the compatible string. > + if (ret) > + return ret; > > regmap = dev_get_regmap(pdev->dev.parent, NULL); > if (!regmap) > return -ENODEV; > > - return qcom_cc_really_probe(pdev, &apss_ipq6018_desc, regmap); > + ret = qcom_cc_really_probe(pdev, &apss_ipq6018_desc, regmap); > + if (ret) > + return ret; > + > + switch (soc_id) { > + /* Only below variants of IPQ53xx support scaling */ > + case QCOM_ID_IPQ5332: > + case QCOM_ID_IPQ5322: > + case QCOM_ID_IPQ5300: > + cpu_clk_notifier = devm_kzalloc(&pdev->dev, > + sizeof(*cpu_clk_notifier), > + GFP_KERNEL); > + if (!cpu_clk_notifier) > + return -ENOMEM; > +
diff --git a/drivers/clk/qcom/apss-ipq6018.c b/drivers/clk/qcom/apss-ipq6018.c index 4e13a08..e6295b8 100644 --- a/drivers/clk/qcom/apss-ipq6018.c +++ b/drivers/clk/qcom/apss-ipq6018.c @@ -9,8 +9,11 @@ #include <linux/clk-provider.h> #include <linux/regmap.h> #include <linux/module.h> +#include <linux/clk.h> +#include <linux/soc/qcom/smem.h> #include <dt-bindings/clock/qcom,apss-ipq.h> +#include <dt-bindings/arm/qcom,ids.h> #include "common.h" #include "clk-regmap.h" @@ -84,15 +87,68 @@ static const struct qcom_cc_desc apss_ipq6018_desc = { .num_clks = ARRAY_SIZE(apss_ipq6018_clks), }; +static int cpu_clk_notifier_fn(struct notifier_block *nb, unsigned long action, + void *data) +{ + struct clk_hw *hw; + u8 index; + int err; + + if (action == PRE_RATE_CHANGE) + index = P_GPLL0; + else if (action == POST_RATE_CHANGE || action == ABORT_RATE_CHANGE) + index = P_APSS_PLL_EARLY; + else + return NOTIFY_OK; + + hw = &apcs_alias0_clk_src.clkr.hw; + err = clk_rcg2_mux_closest_ops.set_parent(hw, index); + + return notifier_from_errno(err); +} + static int apss_ipq6018_probe(struct platform_device *pdev) { + struct clk_hw *hw = &apcs_alias0_clk_src.clkr.hw; + struct notifier_block *cpu_clk_notifier; struct regmap *regmap; + u32 soc_id; + int ret; + + ret = qcom_smem_get_soc_id(&soc_id); + if (ret) + return ret; regmap = dev_get_regmap(pdev->dev.parent, NULL); if (!regmap) return -ENODEV; - return qcom_cc_really_probe(pdev, &apss_ipq6018_desc, regmap); + ret = qcom_cc_really_probe(pdev, &apss_ipq6018_desc, regmap); + if (ret) + return ret; + + switch (soc_id) { + /* Only below variants of IPQ53xx support scaling */ + case QCOM_ID_IPQ5332: + case QCOM_ID_IPQ5322: + case QCOM_ID_IPQ5300: + cpu_clk_notifier = devm_kzalloc(&pdev->dev, + sizeof(*cpu_clk_notifier), + GFP_KERNEL); + if (!cpu_clk_notifier) + return -ENOMEM; + + cpu_clk_notifier->notifier_call = cpu_clk_notifier_fn; + + ret = devm_clk_notifier_register(&pdev->dev, hw->clk, cpu_clk_notifier); + if (ret) + return ret; + break; + default: + break; + } + + return 0; } static struct platform_driver apss_ipq6018_driver = {