[v5,5/9] clk: qcom: apss-ipq6018: ipq5332: add safe source switch for a53pll
Commit Message
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>
---
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(-)
Comments
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;
> +
@@ -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 = {