2020-10-14 06:21:36

by Taniya Das

Subject: [PATCH v0] LPASSCC: Configure the PLL in case lost for SC7180

In the case where the LPASSCC PLL loses the PLL configuration it would fail
to lock. Thus allow reconfigure the PLL from pm_resume.

Taniya Das (1):
clk: qcom: lpasscc: Re-configure the PLL in case lost

drivers/clk/qcom/lpasscorecc-sc7180.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)

--
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member
of the Code Aurora Forum, hosted by the Linux Foundation.

2020-10-14 06:21:37

by Taniya Das

Subject: [PATCH v0] clk: qcom: lpasscc: Re-configure the PLL in case lost

In the case where the PLL configuration is lost, then the pm runtime
resume will reconfigure before usage.

Fixes: edab812d802d ("clk: qcom: lpass: Add support for LPASS clock controller for SC7180")
Signed-off-by: Taniya Das <[email protected]>
---
drivers/clk/qcom/lpasscorecc-sc7180.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)

diff --git a/drivers/clk/qcom/lpasscorecc-sc7180.c b/drivers/clk/qcom/lpasscorecc-sc7180.c
index 228d08f..5804a93 100644
--- a/drivers/clk/qcom/lpasscorecc-sc7180.c
+++ b/drivers/clk/qcom/lpasscorecc-sc7180.c
@@ -356,6 +356,25 @@ static const struct qcom_cc_desc lpass_audio_hm_sc7180_desc = {
.num_gdscs = ARRAY_SIZE(lpass_audio_hm_sc7180_gdscs),
};

+static int lpass_core_cc_pm_clk_resume(struct device *dev)
+{
+ struct regmap *regmap = dev_get_drvdata(dev);
+ int l_val;
+
+ pm_clk_resume(dev);
+
+ /* Read PLL_L_VAL */
+ regmap_read(regmap, 0x1004, &l_val);
+ if (!l_val)
+ clk_fabia_pll_configure(&lpass_lpaaudio_dig_pll, regmap,
+ &lpass_lpaaudio_dig_pll_config);
+ return 0;
+}
+
+static const struct dev_pm_ops lpass_core_pm_ops = {
+ SET_RUNTIME_PM_OPS(pm_clk_suspend, lpass_core_cc_pm_clk_resume, NULL)
+};
+
static int lpass_core_cc_sc7180_probe(struct platform_device *pdev)
{
const struct qcom_cc_desc *desc;
@@ -386,6 +405,9 @@ static int lpass_core_cc_sc7180_probe(struct platform_device *pdev)
clk_fabia_pll_configure(&lpass_lpaaudio_dig_pll, regmap,
&lpass_lpaaudio_dig_pll_config);

+ pdev->dev.driver->pm = &lpass_core_pm_ops;
+ dev_set_drvdata(&pdev->dev, regmap);
+
return qcom_cc_really_probe(pdev, &lpass_core_cc_sc7180_desc, regmap);
}

--
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member
of the Code Aurora Forum, hosted by the Linux Foundation.

2020-10-14 09:28:55

by Doug Anderson

Subject: Re: [PATCH v0] clk: qcom: lpasscc: Re-configure the PLL in case lost

Hi,

On Tue, Oct 13, 2020 at 11:33 AM Taniya Das <[email protected]> wrote:
>
> In the case where the PLL configuration is lost, then the pm runtime
> resume will reconfigure before usage.
>
> Fixes: edab812d802d ("clk: qcom: lpass: Add support for LPASS clock controller for SC7180")
> Signed-off-by: Taniya Das <[email protected]>
> ---
> drivers/clk/qcom/lpasscorecc-sc7180.c | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/drivers/clk/qcom/lpasscorecc-sc7180.c b/drivers/clk/qcom/lpasscorecc-sc7180.c
> index 228d08f..5804a93 100644
> --- a/drivers/clk/qcom/lpasscorecc-sc7180.c
> +++ b/drivers/clk/qcom/lpasscorecc-sc7180.c
> @@ -356,6 +356,25 @@ static const struct qcom_cc_desc lpass_audio_hm_sc7180_desc = {
> .num_gdscs = ARRAY_SIZE(lpass_audio_hm_sc7180_gdscs),
> };
>
> +static int lpass_core_cc_pm_clk_resume(struct device *dev)
> +{
> + struct regmap *regmap = dev_get_drvdata(dev);
> + int l_val;

nit: technically "unsigned int" to match what regmap_read takes.

> +
> + pm_clk_resume(dev);

Even though pm_clk_resume() doesn't currently return any errors, it
would be good form to check. AKA:

ret = pm_clk_resume(dev);
if (ret)
return ret;


> + /* Read PLL_L_VAL */
> + regmap_read(regmap, 0x1004, &l_val);
> + if (!l_val)
> + clk_fabia_pll_configure(&lpass_lpaaudio_dig_pll, regmap,
> + &lpass_lpaaudio_dig_pll_config);
> + return 0;
> +}
> +
> +static const struct dev_pm_ops lpass_core_pm_ops = {
> + SET_RUNTIME_PM_OPS(pm_clk_suspend, lpass_core_cc_pm_clk_resume, NULL)
> +};
> +
> static int lpass_core_cc_sc7180_probe(struct platform_device *pdev)
> {
> const struct qcom_cc_desc *desc;
> @@ -386,6 +405,9 @@ static int lpass_core_cc_sc7180_probe(struct platform_device *pdev)
> clk_fabia_pll_configure(&lpass_lpaaudio_dig_pll, regmap,
> &lpass_lpaaudio_dig_pll_config);
>
> + pdev->dev.driver->pm = &lpass_core_pm_ops;
> + dev_set_drvdata(&pdev->dev, regmap);

I'm kinda confused. Why not just change "lpass_core_cc_pm_ops"? Then
you can get rid of the above two lines of code and get rid of the
whole "lpass_core_pm_ops" structure?


-Doug