[PATCH v9 0/4] Add resets for ADSP based audio clock controller driver

[PATCH v9 0/4] Add resets for ADSP based audio clock controller driver

[Mohammad Rafi Shaik]

Add resets and remove qdsp6ss clcok controller for audioreach based platforms.
Changes since v8:
    -- Add the required gdsc's in lpass_cc_sc7280_desc structure.
    -- Modify qcom_cc_probe to qcom_cc_probe_by_index.
    -- Update the commit message for v8,4/5 patch, which is not required for new logic.
    -- Drop "Add binding headers for lpasscc" patch.
    -- Drop "Skip lpass_aon_cc_pll config" patch.
Changes since v7:
    -- Modiy AHB clock probing method in "Merge lpasscc into lpass_aon patch".
    -- Fix Typo errors in "Merge lpasscc into lpass_aon patch".
    -- Update commit message in "Merge lpasscc into lpass_aon patch" 
Changes since v6:
    -- Update commit message in "Merge lpasscc into lpass_aon patch" patch.
    -- Drop "Skip lpasscorecc registration" patch.
    -- Add comment in the code in "Skip lpass_aon_cc_pll config" patch.
Changes since v5:
    -- Fix compilation issue.
Changes since v4:
    -- Update Fixes tag in Merge lpasscc into lpass_aon patch.
    -- Revert removal of clk_regmap structure in Merge lpasscc into lpass_aon patch.

Changes since v3:
    -- Remove duplicate clock resets patch.
    -- Add binding headers for q6 clocks.
    -- Create new patch for merging lpasscc q6 clocks into lpass_aon.
    -- Create new patches for handling conflicts of ADSP and bypass solution.

Changes since v2:
    -- Revert removing qdsp6ss clock control.
    -- Add Conditional check for qdsp6ss clock registration.
Changes since v1:
    -- Update commit message.
    -- Remove qdsp6ss clock control.

Srinivasa Rao Mandadapu (2):
  dt-bindings: clock: qcom,sc7280-lpasscc: Add qcom,adsp-pil-mode
    property
  clk: qcom: lpasscc-sc7280: Skip qdsp6ss clock registration
Mohammad Rafi Shaik (2):
  clk: qcom: lpassaudiocc-sc7280: Add required gdsc power domain clks in required gdsc power domain clks
  clk: qcom: lpassaudiocc-sc7280: Modify qcom_cc_probe to qcom_cc_probe_by_index

 .../bindings/clock/qcom,sc7280-lpasscc.yaml          |  7 +++++++
 drivers/clk/qcom/lpassaudiocc-sc7280.c               |  4 +++-
 drivers/clk/qcom/lpasscc-sc7280.c                    | 12 +++++++-----
 3 files changed, 17 insertions(+), 6 deletions(-)

-- 
2.25.1

[PATCH v9 1/4] dt-bindings: clock: qcom,sc7280-lpasscc: Add qcom,adsp-pil-mode property

[Mohammad Rafi Shaik]

From: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>

When this property is set, the remoteproc is used to boot the
LPASS and therefore qdsp6ss clocks would be used to bring LPASS
out of reset, hence they are directly controlled by the remoteproc.

This is a cleanup done to handle overlap of regmap of lpasscc
and adsp remoteproc blocks.

Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
Signed-off-by: Mohammad Rafi Shaik <quic_mohs@quicinc.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
---
 .../devicetree/bindings/clock/qcom,sc7280-lpasscc.yaml     | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscc.yaml b/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscc.yaml
index 6151fdebbff8..97c6bd96e0cb 100644
--- a/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscc.yaml
@@ -41,6 +41,12 @@ properties:
       - const: qdsp6ss
       - const: top_cc
 
+  qcom,adsp-pil-mode:
+    description:
+      Indicates if the LPASS would be brought out of reset using
+      remoteproc peripheral loader.
+    type: boolean
+
 required:
   - compatible
   - reg
@@ -60,6 +66,7 @@ examples:
       reg-names = "qdsp6ss", "top_cc";
       clocks = <&gcc GCC_CFG_NOC_LPASS_CLK>;
       clock-names = "iface";
+      qcom,adsp-pil-mode;
       #clock-cells = <1>;
     };
 ...
-- 
2.25.1

[PATCH v9 2/4] clk: qcom: lpasscc-sc7280: Skip qdsp6ss clock registration

[Mohammad Rafi Shaik]

From: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>

The qdsp6ss memory region is being shared by ADSP remoteproc device and
lpasscc clock device, hence causing memory conflict.
As the qdsp6ss clocks are being enabled in remoteproc driver, skip qdsp6ss
clock registration if "qcom,adsp-pil-mode" is enabled.

Fixes: 4ab43d171181 ("clk: qcom: Add lpass clock controller driver for SC7280")
Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
Signed-off-by: Mohammad Rafi Shaik <quic_mohs@quicinc.com>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
---
 drivers/clk/qcom/lpasscc-sc7280.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/qcom/lpasscc-sc7280.c b/drivers/clk/qcom/lpasscc-sc7280.c
index 5c1e17bd0d76..85dd5b9d64f0 100644
--- a/drivers/clk/qcom/lpasscc-sc7280.c
+++ b/drivers/clk/qcom/lpasscc-sc7280.c
@@ -118,12 +118,14 @@ static int lpass_cc_sc7280_probe(struct platform_device *pdev)
 		goto destroy_pm_clk;
 	}
 
-	lpass_regmap_config.name = "qdsp6ss";
-	desc = &lpass_qdsp6ss_sc7280_desc;
+	if (!of_property_read_bool(pdev->dev.of_node, "qcom,adsp-pil-mode")) {
+		lpass_regmap_config.name = "qdsp6ss";
+		desc = &lpass_qdsp6ss_sc7280_desc;
 
-	ret = qcom_cc_probe_by_index(pdev, 0, desc);
-	if (ret)
-		goto destroy_pm_clk;
+		ret = qcom_cc_probe_by_index(pdev, 0, desc);
+		if (ret)
+			goto destroy_pm_clk;
+	}
 
 	lpass_regmap_config.name = "top_cc";
 	desc = &lpass_cc_top_sc7280_desc;
-- 
2.25.1

Re: [PATCH v9 2/4] clk: qcom: lpasscc-sc7280: Skip qdsp6ss clock registration

[Stephen Boyd]

Quoting Mohammad Rafi Shaik (2023-03-17 07:16:20)
> From: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
> 
> The qdsp6ss memory region is being shared by ADSP remoteproc device and
> lpasscc clock device, hence causing memory conflict.
> As the qdsp6ss clocks are being enabled in remoteproc driver, skip qdsp6ss
> clock registration if "qcom,adsp-pil-mode" is enabled.
> 
> Fixes: 4ab43d171181 ("clk: qcom: Add lpass clock controller driver for SC7280")
> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
> Signed-off-by: Mohammad Rafi Shaik <quic_mohs@quicinc.com>
> Reviewed-by: Stephen Boyd <swboyd@chromium.org>
> ---
>  drivers/clk/qcom/lpasscc-sc7280.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/clk/qcom/lpasscc-sc7280.c b/drivers/clk/qcom/lpasscc-sc7280.c
> index 5c1e17bd0d76..85dd5b9d64f0 100644
> --- a/drivers/clk/qcom/lpasscc-sc7280.c
> +++ b/drivers/clk/qcom/lpasscc-sc7280.c
> @@ -118,12 +118,14 @@ static int lpass_cc_sc7280_probe(struct platform_device *pdev)
>                 goto destroy_pm_clk;
>         }
>  
> -       lpass_regmap_config.name = "qdsp6ss";
> -       desc = &lpass_qdsp6ss_sc7280_desc;
> +       if (!of_property_read_bool(pdev->dev.of_node, "qcom,adsp-pil-mode")) {
> +               lpass_regmap_config.name = "qdsp6ss";
> +               desc = &lpass_qdsp6ss_sc7280_desc;
>  
> -       ret = qcom_cc_probe_by_index(pdev, 0, desc);
> -       if (ret)
> -               goto destroy_pm_clk;
> +               ret = qcom_cc_probe_by_index(pdev, 0, desc);
> +               if (ret)
> +                       goto destroy_pm_clk;
> +       }

I noticed that we don't set any max_register on the regmap_config. Can
you also do that in a patch before this patch? I think without that set
you can oops the kernel by reading the debugfs file for the regmap.

>  
>         lpass_regmap_config.name = "top_cc";
>         desc = &lpass_cc_top_sc7280_desc;

[PATCH v9 3/4] clk: qcom: lpassaudiocc-sc7280: Add required gdsc power domain clks in lpass_cc_sc7280_desc strcuture

[Mohammad Rafi Shaik]

Add the gdsc's in lpass_cc_sc7280_desc strcuture,
When qcom,adsp-pil-mode is enabled, gdsc's required to solve
dependencies in lpass_audiocc probe.

Fixes: 4ab43d171181 ("clk: qcom: Add lpass clock controller driver for SC7280")
Signed-off-by: Mohammad Rafi Shaik <quic_mohs@quicinc.com>
---
 drivers/clk/qcom/lpassaudiocc-sc7280.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/clk/qcom/lpassaudiocc-sc7280.c b/drivers/clk/qcom/lpassaudiocc-sc7280.c
index 1339f9211a14..134eb1529ede 100644
--- a/drivers/clk/qcom/lpassaudiocc-sc7280.c
+++ b/drivers/clk/qcom/lpassaudiocc-sc7280.c
@@ -696,6 +696,8 @@ static const struct qcom_cc_desc lpass_cc_sc7280_desc = {
 	.config = &lpass_audio_cc_sc7280_regmap_config,
 	.clks = lpass_cc_sc7280_clocks,
 	.num_clks = ARRAY_SIZE(lpass_cc_sc7280_clocks),
+	.gdscs = lpass_aon_cc_sc7280_gdscs,
+	.num_gdscs = ARRAY_SIZE(lpass_aon_cc_sc7280_gdscs),
 };
 
 static const struct qcom_cc_desc lpass_audio_cc_sc7280_desc = {
-- 
2.25.1

Re: [PATCH v9 3/4] clk: qcom: lpassaudiocc-sc7280: Add required gdsc power domain clks in lpass_cc_sc7280_desc strcuture

[Stephen Boyd]

Just remove strcuture from the subject.

Quoting Mohammad Rafi Shaik (2023-03-17 07:16:21)
> Add the gdsc's in lpass_cc_sc7280_desc strcuture,

s/strcuture/struct/

> When qcom,adsp-pil-mode is enabled, gdsc's required to solve

Just 'GDSCs', because gdsc isn't owning anything.

> dependencies in lpass_audiocc probe.

I think you're saying that we need to register the GDSCs when
qcom,adsp-pil-mode is set in the qcom,sc7280-lpassaoncc node. Otherwise,
we don't get any GDSCs provided by that node when we should always be
providing them.

> 
> Fixes: 4ab43d171181 ("clk: qcom: Add lpass clock controller driver for SC7280")

If the above is right, then this fixes tag is wrong. It should really be 

Fixes: 0cbcfbe50cbf ("clk: qcom: lpass: Handle the regmap overlap of lpasscc and lpass_aon")

right?

Re: [PATCH v9 3/4] clk: qcom: lpassaudiocc-sc7280: Add required gdsc power domain clks in lpass_cc_sc7280_desc strcuture

[Mohammad Rafi Shaik]

On 3/18/2023 12:13 AM, Stephen Boyd wrote:
> Just remove strcuture from the subject.
>
> Quoting Mohammad Rafi Shaik (2023-03-17 07:16:21)
>> Add the gdsc's in lpass_cc_sc7280_desc strcuture,
> s/strcuture/struct/
>
>> When qcom,adsp-pil-mode is enabled, gdsc's required to solve
> Just 'GDSCs', because gdsc isn't owning anything.
>
>> dependencies in lpass_audiocc probe.
> I think you're saying that we need to register the GDSCs when
> qcom,adsp-pil-mode is set in the qcom,sc7280-lpassaoncc node. Otherwise,
> we don't get any GDSCs provided by that node when we should always be
> providing them.

Yes, need to register GDSCs when qcom,adsp-pil-mode is set in the 
qcom,sc7280-lpassaoncc node.

The lpass_audiocc requires GDSCs to probe become success.

>> Fixes: 4ab43d171181 ("clk: qcom: Add lpass clock controller driver for SC7280")
> If the above is right, then this fixes tag is wrong. It should really be
>
> Fixes: 0cbcfbe50cbf ("clk: qcom: lpass: Handle the regmap overlap of lpasscc and lpass_aon")
>
> right?
Okay, will change fixes tag.

[PATCH v9 4/4] clk: qcom: lpassaudiocc-sc7280: Modify qcom_cc_probe to qcom_cc_probe_by_index

[Mohammad Rafi Shaik]

Modify AHB clocks explicit registration from qcom_cc_probe to
qcom_cc_probe_by_index.

Signed-off-by: Mohammad Rafi Shaik <quic_mohs@quicinc.com>
---
 drivers/clk/qcom/lpassaudiocc-sc7280.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/qcom/lpassaudiocc-sc7280.c b/drivers/clk/qcom/lpassaudiocc-sc7280.c
index 134eb1529ede..b7f4688a61f4 100644
--- a/drivers/clk/qcom/lpassaudiocc-sc7280.c
+++ b/drivers/clk/qcom/lpassaudiocc-sc7280.c
@@ -830,7 +830,7 @@ static int lpass_aon_cc_sc7280_probe(struct platform_device *pdev)
 	if (of_property_read_bool(pdev->dev.of_node, "qcom,adsp-pil-mode")) {
 		lpass_audio_cc_sc7280_regmap_config.name = "cc";
 		desc = &lpass_cc_sc7280_desc;
-		ret = qcom_cc_probe(pdev, desc);
+		ret = qcom_cc_probe_by_index(pdev, 0, desc);
 		goto exit;
 	}
 
-- 
2.25.1

Re: [PATCH v9 4/4] clk: qcom: lpassaudiocc-sc7280: Modify qcom_cc_probe to qcom_cc_probe_by_index

[Stephen Boyd]

Quoting Mohammad Rafi Shaik (2023-03-17 07:16:22)
> Modify AHB clocks explicit registration from qcom_cc_probe to
> qcom_cc_probe_by_index.

Why? Isn't it the same?

Re: [PATCH v9 4/4] clk: qcom: lpassaudiocc-sc7280: Modify qcom_cc_probe to qcom_cc_probe_by_index

[Mohammad Rafi Shaik]

On 3/17/2023 11:57 PM, Stephen Boyd wrote:
> Quoting Mohammad Rafi Shaik (2023-03-17 07:16:22)
>> Modify AHB clocks explicit registration from qcom_cc_probe to
>> qcom_cc_probe_by_index.
> Why? Isn't it the same?
yes, it is same.

As per the previous version review comment, modified qcom_cc_probe to 
qcom_cc_probe_by_index.

Re: [PATCH v9 4/4] clk: qcom: lpassaudiocc-sc7280: Modify qcom_cc_probe to qcom_cc_probe_by_index

[Stephen Boyd]

Quoting Mohammad Rafi Shaik (2023-03-23 00:45:26)
> 
> On 3/17/2023 11:57 PM, Stephen Boyd wrote:
> > Quoting Mohammad Rafi Shaik (2023-03-17 07:16:22)
> >> Modify AHB clocks explicit registration from qcom_cc_probe to
> >> qcom_cc_probe_by_index.
> > Why? Isn't it the same?
> yes, it is same.
> 
> As per the previous version review comment, modified qcom_cc_probe to 
> qcom_cc_probe_by_index.

Previously you were probing with index 1. That necessitated using
qcom_cc_probe_by_index() instead of qcom_cc_probe(). That is no longer
the case though, so this patch is unnecessary.