[PATCH 0/5] Add updates for clock controllers to support QCM6490

[PATCH 0/5] Add updates for clock controllers to support QCM6490

[Taniya Das]

This series updates the clock controller drivers and devicetree nodes
for QCM6490 boards.

1. Adding the "qcom,adsp-skip-pll" property to dt bindings.
2. Fix to skip the lpass pll configuration on qcm6490 derivative boards.
3. Enable the force mem core for UFS ICE clock and update the gdsc
   transition delays.
4. Fix to add the camera titan top gdsc as parent to all camera gdscs.
5. Update protected clocks list and disable few of the lpass clock
   controller nodes for qcm6490-idp and qcs6490-rb3gen2 platforms.

Taniya Das (5):
  bindings: clock: qcom: Add "qcom,adsp-skip-pll"  property
  clk: qcom: lpassaudiocc-sc7280: Add support to skip PLL  configuration
  clk: qcom: sc7280: Update the transition delay for GDSC
  clk: qcom: camcc-sc7280: Add parent dependency to all camera GDSCs
  arm64: dts: qcom: Update protected clocks list for qcm6490 variants

 .../clock/qcom,sc7280-lpasscorecc.yaml        |  5 ++
 arch/arm64/boot/dts/qcom/qcm6490-idp.dts      | 54 ++++++++++++++++++-
 arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts  | 50 ++++++++++++++++-
 drivers/clk/qcom/camcc-sc7280.c               | 24 +++++++++
 drivers/clk/qcom/gcc-sc7280.c                 | 13 +++++
 drivers/clk/qcom/gpucc-sc7280.c               |  7 +++
 drivers/clk/qcom/lpassaudiocc-sc7280.c        | 14 +++--
 drivers/clk/qcom/videocc-sc7280.c             |  7 +++
 8 files changed, 168 insertions(+), 6 deletions(-)

--
2.17.1

[PATCH 1/5] bindings: clock: qcom: Add "qcom,adsp-skip-pll" property

[Taniya Das]

When remoteproc is used to boot the LPASS the ADSP PLL should not be
configured from the high level OS. Thus add support for property to
avoid configuring the LPASS PLL.

Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
---
 .../devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml   | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml b/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml
index deee5423d66e..358eb4a1cffd 100644
--- a/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml
@@ -49,6 +49,11 @@ properties:
       peripheral loader.
     type: boolean

+  qcom,adsp-skip-pll:
+    description:
+      Indicates if the LPASS PLL configuration is to be skipped.
+    type: boolean
+
 required:
   - compatible
   - reg
--
2.17.1

Re: [PATCH 1/5] bindings: clock: qcom: Add "qcom,adsp-skip-pll" property

[Dmitry Baryshkov]

On Thu, 8 Feb 2024 at 08:29, Taniya Das <quic_tdas@quicinc.com> wrote:
>
> When remoteproc is used to boot the LPASS the ADSP PLL should not be
> configured from the high level OS.

Why?

> Thus add support for property to
> avoid configuring the LPASS PLL.
>
> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
> ---
>  .../devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml   | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml b/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml
> index deee5423d66e..358eb4a1cffd 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml
> @@ -49,6 +49,11 @@ properties:
>        peripheral loader.
>      type: boolean
>
> +  qcom,adsp-skip-pll:
> +    description:
> +      Indicates if the LPASS PLL configuration is to be skipped.
> +    type: boolean

This property describes OS behaviour rather than the hardware. Such
things are generally not acceptable.

> +
>  required:
>    - compatible
>    - reg
> --
> 2.17.1
>
>


-- 
With best wishes
Dmitry

Re: [PATCH 1/5] bindings: clock: qcom: Add "qcom,adsp-skip-pll" property

[Taniya Das]

Thanks for your review, I will take care of the comments in my next 
patch series.

On 2/8/2024 12:29 PM, Dmitry Baryshkov wrote:
> On Thu, 8 Feb 2024 at 08:29, Taniya Das <quic_tdas@quicinc.com> wrote:
>>
>> When remoteproc is used to boot the LPASS the ADSP PLL should not be
>> configured from the high level OS.
> 
> Why?
> 
>> Thus add support for property to
>> avoid configuring the LPASS PLL.
>>
>> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
>> ---
>>   .../devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml   | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml b/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml
>> index deee5423d66e..358eb4a1cffd 100644
>> --- a/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml
>> +++ b/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml
>> @@ -49,6 +49,11 @@ properties:
>>         peripheral loader.
>>       type: boolean
>>
>> +  qcom,adsp-skip-pll:
>> +    description:
>> +      Indicates if the LPASS PLL configuration is to be skipped.
>> +    type: boolean
> 
> This property describes OS behaviour rather than the hardware. Such
> things are generally not acceptable.
> 
>> +
>>   required:
>>     - compatible
>>     - reg
>> --
>> 2.17.1
>>
>>
> 
> 

-- 
Thanks & Regards,
Taniya Das.

Re: [PATCH 1/5] bindings: clock: qcom: Add "qcom,adsp-skip-pll" property

[Krzysztof Kozlowski]

On 08/02/2024 07:28, Taniya Das wrote:
> When remoteproc is used to boot the LPASS the ADSP PLL should not be
> configured from the high level OS. Thus add support for property to
> avoid configuring the LPASS PLL.
> 
> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>

You nicely bypassed all my filters.

Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching.

Anyway, I don't understand point of this commit. Aren't you now
duplicating qcom,adsp-pil-mode?

Best regards,
Krzysztof

Re: [PATCH 1/5] bindings: clock: qcom: Add "qcom,adsp-skip-pll" property

[Taniya Das]

Thanks for your review Krzysztof.

On 2/8/2024 1:45 PM, Krzysztof Kozlowski wrote:
> On 08/02/2024 07:28, Taniya Das wrote:
>> When remoteproc is used to boot the LPASS the ADSP PLL should not be
>> configured from the high level OS. Thus add support for property to
>> avoid configuring the LPASS PLL.
>>
>> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
> 
> You nicely bypassed all my filters.
> 
> Please use subject prefixes matching the subsystem. You can get them for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching.
> 

My bad, I will update the commit subject.

> Anyway, I don't understand point of this commit. Aren't you now
> duplicating qcom,adsp-pil-mode?

No, the expectation with pil-mode was still certain level of 
configuration from HLOS LPASS clock drivers. But on the QCM6490 boards 
these clocks need to be completely reserved except the resets.

> 
> Best regards,
> Krzysztof
> 

-- 
Thanks & Regards,
Taniya Das.

Re: [PATCH 1/5] bindings: clock: qcom: Add "qcom,adsp-skip-pll" property

[Krzysztof Kozlowski]

On 06/03/2024 07:53, Taniya Das wrote:
> Thanks for your review Krzysztof.
> 
> On 2/8/2024 1:45 PM, Krzysztof Kozlowski wrote:
>> On 08/02/2024 07:28, Taniya Das wrote:
>>> When remoteproc is used to boot the LPASS the ADSP PLL should not be
>>> configured from the high level OS. Thus add support for property to
>>> avoid configuring the LPASS PLL.
>>>
>>> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
>>
>> You nicely bypassed all my filters.
>>
>> Please use subject prefixes matching the subsystem. You can get them for
>> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
>> your patch is touching.
>>
> 
> My bad, I will update the commit subject.
> 
>> Anyway, I don't understand point of this commit. Aren't you now
>> duplicating qcom,adsp-pil-mode?
> 
> No, the expectation with pil-mode was still certain level of 
> configuration from HLOS LPASS clock drivers. But on the QCM6490 boards 
> these clocks need to be completely reserved except the resets.

Sounds like qcom,adsp-pil-mode or qcom,controlled-remotely or one of
others. Stop inventing 10 properties describing similar case, one for
each binding.

Best regards,
Krzysztof

[PATCH 2/5] clk: qcom: lpassaudiocc-sc7280: Add support to skip PLL configuration

[Taniya Das]

The PLL configuration needs to be skipped when remoteproc brings the
LPASS out of reset.

Also update the lpassaudio_cc_reset regmap name and max register to handle
the regmap conflict warning between lpassaudio_cc_reset and lpassaudio_cc.

Fixes: a9dd26639d05 ("clk: qcom: lpass: Add support for LPASS clock controller for SC7280")
Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
---
 drivers/clk/qcom/lpassaudiocc-sc7280.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/qcom/lpassaudiocc-sc7280.c b/drivers/clk/qcom/lpassaudiocc-sc7280.c
index c43d0b1af7f7..2619a8ced9d5 100644
--- a/drivers/clk/qcom/lpassaudiocc-sc7280.c
+++ b/drivers/clk/qcom/lpassaudiocc-sc7280.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
  * Copyright (c) 2021, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved.
  */

 #include <linux/clk-provider.h>
@@ -766,11 +767,13 @@ static int lpass_audio_cc_sc7280_probe(struct platform_device *pdev)
 		goto exit;
 	}

-	clk_zonda_pll_configure(&lpass_audio_cc_pll, regmap, &lpass_audio_cc_pll_config);
+	if (!of_property_read_bool(pdev->dev.of_node, "qcom,adsp-skip-pll")) {
+		clk_zonda_pll_configure(&lpass_audio_cc_pll, regmap, &lpass_audio_cc_pll_config);

-	/* PLL settings */
-	regmap_write(regmap, 0x4, 0x3b);
-	regmap_write(regmap, 0x8, 0xff05);
+		/* PLL settings */
+		regmap_write(regmap, 0x4, 0x3b);
+		regmap_write(regmap, 0x8, 0xff05);
+	}

 	ret = qcom_cc_really_probe(pdev, &lpass_audio_cc_sc7280_desc, regmap);
 	if (ret) {
@@ -778,6 +781,9 @@ static int lpass_audio_cc_sc7280_probe(struct platform_device *pdev)
 		goto exit;
 	}

+	lpass_audio_cc_sc7280_regmap_config.name = "lpassaudio_cc_reset";
+	lpass_audio_cc_sc7280_regmap_config.max_register = 0xc8;
+
 	ret = qcom_cc_probe_by_index(pdev, 1, &lpass_audio_cc_reset_sc7280_desc);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to register LPASS AUDIO CC Resets\n");
--
2.17.1

Re: [PATCH 2/5] clk: qcom: lpassaudiocc-sc7280: Add support to skip PLL configuration

[Dmitry Baryshkov]

On Thu, 8 Feb 2024 at 08:29, Taniya Das <quic_tdas@quicinc.com> wrote:
>
> The PLL configuration needs to be skipped when remoteproc brings the
> LPASS out of reset.

Why?

>
> Also update the lpassaudio_cc_reset regmap name and max register to handle
> the regmap conflict warning between lpassaudio_cc_reset and lpassaudio_cc.

Separate patch, please.

>
> Fixes: a9dd26639d05 ("clk: qcom: lpass: Add support for LPASS clock controller for SC7280")
> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
> ---
>  drivers/clk/qcom/lpassaudiocc-sc7280.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/clk/qcom/lpassaudiocc-sc7280.c b/drivers/clk/qcom/lpassaudiocc-sc7280.c
> index c43d0b1af7f7..2619a8ced9d5 100644
> --- a/drivers/clk/qcom/lpassaudiocc-sc7280.c
> +++ b/drivers/clk/qcom/lpassaudiocc-sc7280.c
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  /*
>   * Copyright (c) 2021, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved.
>   */
>
>  #include <linux/clk-provider.h>
> @@ -766,11 +767,13 @@ static int lpass_audio_cc_sc7280_probe(struct platform_device *pdev)
>                 goto exit;
>         }
>
> -       clk_zonda_pll_configure(&lpass_audio_cc_pll, regmap, &lpass_audio_cc_pll_config);
> +       if (!of_property_read_bool(pdev->dev.of_node, "qcom,adsp-skip-pll")) {
> +               clk_zonda_pll_configure(&lpass_audio_cc_pll, regmap, &lpass_audio_cc_pll_config);
>
> -       /* PLL settings */
> -       regmap_write(regmap, 0x4, 0x3b);
> -       regmap_write(regmap, 0x8, 0xff05);
> +               /* PLL settings */
> +               regmap_write(regmap, 0x4, 0x3b);
> +               regmap_write(regmap, 0x8, 0xff05);
> +       }
>
>         ret = qcom_cc_really_probe(pdev, &lpass_audio_cc_sc7280_desc, regmap);
>         if (ret) {
> @@ -778,6 +781,9 @@ static int lpass_audio_cc_sc7280_probe(struct platform_device *pdev)
>                 goto exit;
>         }
>
> +       lpass_audio_cc_sc7280_regmap_config.name = "lpassaudio_cc_reset";
> +       lpass_audio_cc_sc7280_regmap_config.max_register = 0xc8;
> +
>         ret = qcom_cc_probe_by_index(pdev, 1, &lpass_audio_cc_reset_sc7280_desc);
>         if (ret) {
>                 dev_err(&pdev->dev, "Failed to register LPASS AUDIO CC Resets\n");
> --
> 2.17.1
>
>


-- 
With best wishes
Dmitry

Re: [PATCH 2/5] clk: qcom: lpassaudiocc-sc7280: Add support to skip PLL configuration

[Taniya Das]

Thanks for your review Dmitry.

On 2/8/2024 12:32 PM, Dmitry Baryshkov wrote:
> On Thu, 8 Feb 2024 at 08:29, Taniya Das <quic_tdas@quicinc.com> wrote:
>>
>> The PLL configuration needs to be skipped when remoteproc brings the
>> LPASS out of reset.
> 
> Why?
> 

On QCM6490 boards, the HLOS is not given access to program the PLL of 
Low Power Audio Subsystem. Also on these boards the per-requisite of a 
GDSC is not available. Thus we need to skip the PLL programming.

I will update the commit text in my next series.

>>
>> Also update the lpassaudio_cc_reset regmap name and max register to handle
>> the regmap conflict warning between lpassaudio_cc_reset and lpassaudio_cc.
> 
> Separate patch, please.
> 

Yes, I will take care.

>>
>> Fixes: a9dd26639d05 ("clk: qcom: lpass: Add support for LPASS clock controller for SC7280")
>> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
>> ---
>>   drivers/clk/qcom/lpassaudiocc-sc7280.c | 14 ++++++++++----
>>   1 file changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/clk/qcom/lpassaudiocc-sc7280.c b/drivers/clk/qcom/lpassaudiocc-sc7280.c
>> index c43d0b1af7f7..2619a8ced9d5 100644
>> --- a/drivers/clk/qcom/lpassaudiocc-sc7280.c
>> +++ b/drivers/clk/qcom/lpassaudiocc-sc7280.c
>> @@ -1,6 +1,7 @@
>>   // SPDX-License-Identifier: GPL-2.0-only
>>   /*
>>    * Copyright (c) 2021, The Linux Foundation. All rights reserved.
>> + * Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved.
>>    */
>>
>>   #include <linux/clk-provider.h>
>> @@ -766,11 +767,13 @@ static int lpass_audio_cc_sc7280_probe(struct platform_device *pdev)
>>                  goto exit;
>>          }
>>
>> -       clk_zonda_pll_configure(&lpass_audio_cc_pll, regmap, &lpass_audio_cc_pll_config);
>> +       if (!of_property_read_bool(pdev->dev.of_node, "qcom,adsp-skip-pll")) {
>> +               clk_zonda_pll_configure(&lpass_audio_cc_pll, regmap, &lpass_audio_cc_pll_config);
>>
>> -       /* PLL settings */
>> -       regmap_write(regmap, 0x4, 0x3b);
>> -       regmap_write(regmap, 0x8, 0xff05);
>> +               /* PLL settings */
>> +               regmap_write(regmap, 0x4, 0x3b);
>> +               regmap_write(regmap, 0x8, 0xff05);
>> +       }
>>
>>          ret = qcom_cc_really_probe(pdev, &lpass_audio_cc_sc7280_desc, regmap);
>>          if (ret) {
>> @@ -778,6 +781,9 @@ static int lpass_audio_cc_sc7280_probe(struct platform_device *pdev)
>>                  goto exit;
>>          }
>>
>> +       lpass_audio_cc_sc7280_regmap_config.name = "lpassaudio_cc_reset";
>> +       lpass_audio_cc_sc7280_regmap_config.max_register = 0xc8;
>> +
>>          ret = qcom_cc_probe_by_index(pdev, 1, &lpass_audio_cc_reset_sc7280_desc);
>>          if (ret) {
>>                  dev_err(&pdev->dev, "Failed to register LPASS AUDIO CC Resets\n");
>> --
>> 2.17.1
>>
>>
> 
> 

-- 
Thanks & Regards,
Taniya Das.

Re: [PATCH 2/5] clk: qcom: lpassaudiocc-sc7280: Add support to skip PLL configuration

[Dmitry Baryshkov]

On Wed, 6 Mar 2024 at 08:56, Taniya Das <quic_tdas@quicinc.com> wrote:
>
> Thanks for your review Dmitry.
>
> On 2/8/2024 12:32 PM, Dmitry Baryshkov wrote:
> > On Thu, 8 Feb 2024 at 08:29, Taniya Das <quic_tdas@quicinc.com> wrote:
> >>
> >> The PLL configuration needs to be skipped when remoteproc brings the
> >> LPASS out of reset.
> >
> > Why?
> >
>
> On QCM6490 boards, the HLOS is not given access to program the PLL of
> Low Power Audio Subsystem. Also on these boards the per-requisite of a
> GDSC is not available. Thus we need to skip the PLL programming.
>
> I will update the commit text in my next series.

Please use SoC-specific compatible instead of adding extra properties.

>
> >>
> >> Also update the lpassaudio_cc_reset regmap name and max register to handle
> >> the regmap conflict warning between lpassaudio_cc_reset and lpassaudio_cc.
> >
> > Separate patch, please.
> >
>
> Yes, I will take care.
>
> >>
> >> Fixes: a9dd26639d05 ("clk: qcom: lpass: Add support for LPASS clock controller for SC7280")
> >> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
> >> ---
> >>   drivers/clk/qcom/lpassaudiocc-sc7280.c | 14 ++++++++++----
> >>   1 file changed, 10 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/clk/qcom/lpassaudiocc-sc7280.c b/drivers/clk/qcom/lpassaudiocc-sc7280.c
> >> index c43d0b1af7f7..2619a8ced9d5 100644
> >> --- a/drivers/clk/qcom/lpassaudiocc-sc7280.c
> >> +++ b/drivers/clk/qcom/lpassaudiocc-sc7280.c
> >> @@ -1,6 +1,7 @@
> >>   // SPDX-License-Identifier: GPL-2.0-only
> >>   /*
> >>    * Copyright (c) 2021, The Linux Foundation. All rights reserved.
> >> + * Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved.
> >>    */
> >>
> >>   #include <linux/clk-provider.h>
> >> @@ -766,11 +767,13 @@ static int lpass_audio_cc_sc7280_probe(struct platform_device *pdev)
> >>                  goto exit;
> >>          }
> >>
> >> -       clk_zonda_pll_configure(&lpass_audio_cc_pll, regmap, &lpass_audio_cc_pll_config);
> >> +       if (!of_property_read_bool(pdev->dev.of_node, "qcom,adsp-skip-pll")) {
> >> +               clk_zonda_pll_configure(&lpass_audio_cc_pll, regmap, &lpass_audio_cc_pll_config);
> >>
> >> -       /* PLL settings */
> >> -       regmap_write(regmap, 0x4, 0x3b);
> >> -       regmap_write(regmap, 0x8, 0xff05);
> >> +               /* PLL settings */
> >> +               regmap_write(regmap, 0x4, 0x3b);
> >> +               regmap_write(regmap, 0x8, 0xff05);
> >> +       }
> >>
> >>          ret = qcom_cc_really_probe(pdev, &lpass_audio_cc_sc7280_desc, regmap);
> >>          if (ret) {
> >> @@ -778,6 +781,9 @@ static int lpass_audio_cc_sc7280_probe(struct platform_device *pdev)
> >>                  goto exit;
> >>          }
> >>
> >> +       lpass_audio_cc_sc7280_regmap_config.name = "lpassaudio_cc_reset";
> >> +       lpass_audio_cc_sc7280_regmap_config.max_register = 0xc8;
> >> +
> >>          ret = qcom_cc_probe_by_index(pdev, 1, &lpass_audio_cc_reset_sc7280_desc);
> >>          if (ret) {
> >>                  dev_err(&pdev->dev, "Failed to register LPASS AUDIO CC Resets\n");
> >> --
> >> 2.17.1
> >>
> >>
> >
> >
>
> --
> Thanks & Regards,
> Taniya Das.



-- 
With best wishes
Dmitry

Re: [PATCH 2/5] clk: qcom: lpassaudiocc-sc7280: Add support to skip PLL configuration

[Konrad Dybcio]

On 2/8/24 07:28, Taniya Das wrote:
> The PLL configuration needs to be skipped when remoteproc brings the
> LPASS out of reset.
> 
> Also update the lpassaudio_cc_reset regmap name and max register to handle
> the regmap conflict warning between lpassaudio_cc_reset and lpassaudio_cc.
> 
> Fixes: a9dd26639d05 ("clk: qcom: lpass: Add support for LPASS clock controller for SC7280")
> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
> ---
>   drivers/clk/qcom/lpassaudiocc-sc7280.c | 14 ++++++++++----
>   1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/clk/qcom/lpassaudiocc-sc7280.c b/drivers/clk/qcom/lpassaudiocc-sc7280.c
> index c43d0b1af7f7..2619a8ced9d5 100644
> --- a/drivers/clk/qcom/lpassaudiocc-sc7280.c
> +++ b/drivers/clk/qcom/lpassaudiocc-sc7280.c
> @@ -1,6 +1,7 @@
>   // SPDX-License-Identifier: GPL-2.0-only
>   /*
>    * Copyright (c) 2021, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved.
>    */
> 
>   #include <linux/clk-provider.h>
> @@ -766,11 +767,13 @@ static int lpass_audio_cc_sc7280_probe(struct platform_device *pdev)
>   		goto exit;
>   	}
> 
> -	clk_zonda_pll_configure(&lpass_audio_cc_pll, regmap, &lpass_audio_cc_pll_config);
> +	if (!of_property_read_bool(pdev->dev.of_node, "qcom,adsp-skip-pll")) {

Big no-no.

> +		clk_zonda_pll_configure(&lpass_audio_cc_pll, regmap, &lpass_audio_cc_pll_config);
> 
> -	/* PLL settings */
> -	regmap_write(regmap, 0x4, 0x3b);
> -	regmap_write(regmap, 0x8, 0xff05);
> +		/* PLL settings */
> +		regmap_write(regmap, 0x4, 0x3b);
> +		regmap_write(regmap, 0x8, 0xff05);

Model these properly and use the abstracted clock (re)configuration functions.
Add the unreachable clocks to `protected-clocks = <>` and make sure that the
aforementioned configure calls check if the PLL was really registered.

> +	}
> 
>   	ret = qcom_cc_really_probe(pdev, &lpass_audio_cc_sc7280_desc, regmap);
>   	if (ret) {
> @@ -778,6 +781,9 @@ static int lpass_audio_cc_sc7280_probe(struct platform_device *pdev)
>   		goto exit;
>   	}
> 
> +	lpass_audio_cc_sc7280_regmap_config.name = "lpassaudio_cc_reset";

Ugh.. are these really not contiguous, or were the register ranges misrepresented from
the start?

> +	lpass_audio_cc_sc7280_regmap_config.max_register = 0xc8;

Provide the real size of the block in .max_register instead, unconditionally

Konrad

[PATCH 3/5] clk: qcom: sc7280: Update the transition delay for GDSC

[Taniya Das]

The GDSC default values are being updated and they can cause issues
in the GDSC FSM state. While at it also update the force mem core
for UFS ICE clock.

Fixes: fae7617bb142 ("clk: qcom: Add video clock controller driver for SC7280")
Fixes: 1daec8cfebc2 ("clk: qcom: camcc: Add camera clock controller driver for SC7280")
Fixes: a3cc092196ef ("clk: qcom: Add Global Clock controller (GCC) driver for SC7280")
Fixes: 3e0f01d6c7e7 ("clk: qcom: Add graphics clock controller driver for SC7280")
Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
---
 drivers/clk/qcom/camcc-sc7280.c   | 19 +++++++++++++++++++
 drivers/clk/qcom/gcc-sc7280.c     | 13 +++++++++++++
 drivers/clk/qcom/gpucc-sc7280.c   |  7 +++++++
 drivers/clk/qcom/videocc-sc7280.c |  7 +++++++
 4 files changed, 46 insertions(+)

diff --git a/drivers/clk/qcom/camcc-sc7280.c b/drivers/clk/qcom/camcc-sc7280.c
index 49f046ea857c..4849b0e8c846 100644
--- a/drivers/clk/qcom/camcc-sc7280.c
+++ b/drivers/clk/qcom/camcc-sc7280.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
  * Copyright (c) 2021, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved.
  */

 #include <linux/clk-provider.h>
@@ -2247,6 +2248,9 @@ static struct clk_branch cam_cc_sleep_clk = {

 static struct gdsc cam_cc_titan_top_gdsc = {
 	.gdscr = 0xc194,
+	.en_rest_wait_val = 0x2,
+	.en_few_wait_val = 0x2,
+	.clk_dis_wait_val = 0xf,
 	.pd = {
 		.name = "cam_cc_titan_top_gdsc",
 	},
@@ -2256,6 +2260,9 @@ static struct gdsc cam_cc_titan_top_gdsc = {

 static struct gdsc cam_cc_bps_gdsc = {
 	.gdscr = 0x7004,
+	.en_rest_wait_val = 0x2,
+	.en_few_wait_val = 0x2,
+	.clk_dis_wait_val = 0xf,
 	.pd = {
 		.name = "cam_cc_bps_gdsc",
 	},
@@ -2265,6 +2272,9 @@ static struct gdsc cam_cc_bps_gdsc = {

 static struct gdsc cam_cc_ife_0_gdsc = {
 	.gdscr = 0xa004,
+	.en_rest_wait_val = 0x2,
+	.en_few_wait_val = 0x2,
+	.clk_dis_wait_val = 0xf,
 	.pd = {
 		.name = "cam_cc_ife_0_gdsc",
 	},
@@ -2274,6 +2284,9 @@ static struct gdsc cam_cc_ife_0_gdsc = {

 static struct gdsc cam_cc_ife_1_gdsc = {
 	.gdscr = 0xb004,
+	.en_rest_wait_val = 0x2,
+	.en_few_wait_val = 0x2,
+	.clk_dis_wait_val = 0xf,
 	.pd = {
 		.name = "cam_cc_ife_1_gdsc",
 	},
@@ -2283,6 +2296,9 @@ static struct gdsc cam_cc_ife_1_gdsc = {

 static struct gdsc cam_cc_ife_2_gdsc = {
 	.gdscr = 0xb070,
+	.en_rest_wait_val = 0x2,
+	.en_few_wait_val = 0x2,
+	.clk_dis_wait_val = 0xf,
 	.pd = {
 		.name = "cam_cc_ife_2_gdsc",
 	},
@@ -2292,6 +2308,9 @@ static struct gdsc cam_cc_ife_2_gdsc = {

 static struct gdsc cam_cc_ipe_0_gdsc = {
 	.gdscr = 0x8004,
+	.en_rest_wait_val = 0x2,
+	.en_few_wait_val = 0x2,
+	.clk_dis_wait_val = 0xf,
 	.pd = {
 		.name = "cam_cc_ipe_0_gdsc",
 	},
diff --git a/drivers/clk/qcom/gcc-sc7280.c b/drivers/clk/qcom/gcc-sc7280.c
index 2b661df5de26..5f3bfb6f4fbb 100644
--- a/drivers/clk/qcom/gcc-sc7280.c
+++ b/drivers/clk/qcom/gcc-sc7280.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
  * Copyright (c) 2020-2021, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved.
  */

 #include <linux/clk-provider.h>
@@ -3094,6 +3095,9 @@ static struct clk_branch gcc_wpss_rscp_clk = {

 static struct gdsc gcc_pcie_0_gdsc = {
 	.gdscr = 0x6b004,
+	.en_rest_wait_val = 0x2,
+	.en_few_wait_val = 0x2,
+	.clk_dis_wait_val = 0xf,
 	.pd = {
 		.name = "gcc_pcie_0_gdsc",
 	},
@@ -3112,6 +3116,9 @@ static struct gdsc gcc_pcie_1_gdsc = {

 static struct gdsc gcc_ufs_phy_gdsc = {
 	.gdscr = 0x77004,
+	.en_rest_wait_val = 0x2,
+	.en_few_wait_val = 0x2,
+	.clk_dis_wait_val = 0xf,
 	.pd = {
 		.name = "gcc_ufs_phy_gdsc",
 	},
@@ -3121,6 +3128,9 @@ static struct gdsc gcc_ufs_phy_gdsc = {

 static struct gdsc gcc_usb30_prim_gdsc = {
 	.gdscr = 0xf004,
+	.en_rest_wait_val = 0x2,
+	.en_few_wait_val = 0x2,
+	.clk_dis_wait_val = 0xf,
 	.pd = {
 		.name = "gcc_usb30_prim_gdsc",
 	},
@@ -3467,6 +3477,9 @@ static int gcc_sc7280_probe(struct platform_device *pdev)
 	regmap_update_bits(regmap, 0x71004, BIT(0), BIT(0));
 	regmap_update_bits(regmap, 0x7100C, BIT(13), BIT(13));

+	/* FORCE_MEM_CORE_ON for ufs phy ice core clocks */
+	qcom_branch_set_force_mem_core(regmap, gcc_ufs_phy_ice_core_clk, true);
+
 	ret = qcom_cc_register_rcg_dfs(regmap, gcc_dfs_clocks,
 			ARRAY_SIZE(gcc_dfs_clocks));
 	if (ret)
diff --git a/drivers/clk/qcom/gpucc-sc7280.c b/drivers/clk/qcom/gpucc-sc7280.c
index 1490cd45a654..0eea4cf7954d 100644
--- a/drivers/clk/qcom/gpucc-sc7280.c
+++ b/drivers/clk/qcom/gpucc-sc7280.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
  * Copyright (c) 2021, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved.
  */

 #include <linux/clk-provider.h>
@@ -379,6 +380,9 @@ static struct clk_branch gpu_cc_sleep_clk = {

 static struct gdsc cx_gdsc = {
 	.gdscr = 0x106c,
+	.en_rest_wait_val = 0x2,
+	.en_few_wait_val = 0x2,
+	.clk_dis_wait_val = 0x2,
 	.gds_hw_ctrl = 0x1540,
 	.pd = {
 		.name = "cx_gdsc",
@@ -389,6 +393,9 @@ static struct gdsc cx_gdsc = {

 static struct gdsc gx_gdsc = {
 	.gdscr = 0x100c,
+	.en_rest_wait_val = 0x2,
+	.en_few_wait_val = 0x2,
+	.clk_dis_wait_val = 0x2,
 	.clamp_io_ctrl = 0x1508,
 	.pd = {
 		.name = "gx_gdsc",
diff --git a/drivers/clk/qcom/videocc-sc7280.c b/drivers/clk/qcom/videocc-sc7280.c
index 615695d82319..b07895c459e8 100644
--- a/drivers/clk/qcom/videocc-sc7280.c
+++ b/drivers/clk/qcom/videocc-sc7280.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
  * Copyright (c) 2021, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved.
  */

 #include <linux/clk-provider.h>
@@ -232,6 +233,9 @@ static struct clk_branch video_cc_venus_ahb_clk = {

 static struct gdsc mvs0_gdsc = {
 	.gdscr = 0x3004,
+	.en_rest_wait_val = 0x2,
+	.en_few_wait_val = 0x2,
+	.clk_dis_wait_val = 0x6,
 	.pd = {
 		.name = "mvs0_gdsc",
 	},
@@ -241,6 +245,9 @@ static struct gdsc mvs0_gdsc = {

 static struct gdsc mvsc_gdsc = {
 	.gdscr = 0x2004,
+	.en_rest_wait_val = 0x2,
+	.en_few_wait_val = 0x2,
+	.clk_dis_wait_val = 0x6,
 	.pd = {
 		.name = "mvsc_gdsc",
 	},
--
2.17.1

Re: [PATCH 3/5] clk: qcom: sc7280: Update the transition delay for GDSC

[Dmitry Baryshkov]

On Thu, 8 Feb 2024 at 08:29, Taniya Das <quic_tdas@quicinc.com> wrote:
>
> The GDSC default values are being updated and they can cause issues
> in the GDSC FSM state.

It reads as if new values can cause issues with the FSM.

> While at it also update the force mem core
> for UFS ICE clock.

Separate patch, please.

>
> Fixes: fae7617bb142 ("clk: qcom: Add video clock controller driver for SC7280")
> Fixes: 1daec8cfebc2 ("clk: qcom: camcc: Add camera clock controller driver for SC7280")
> Fixes: a3cc092196ef ("clk: qcom: Add Global Clock controller (GCC) driver for SC7280")
> Fixes: 3e0f01d6c7e7 ("clk: qcom: Add graphics clock controller driver for SC7280")
> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
> ---
>  drivers/clk/qcom/camcc-sc7280.c   | 19 +++++++++++++++++++
>  drivers/clk/qcom/gcc-sc7280.c     | 13 +++++++++++++
>  drivers/clk/qcom/gpucc-sc7280.c   |  7 +++++++
>  drivers/clk/qcom/videocc-sc7280.c |  7 +++++++
>  4 files changed, 46 insertions(+)
>
> diff --git a/drivers/clk/qcom/camcc-sc7280.c b/drivers/clk/qcom/camcc-sc7280.c
> index 49f046ea857c..4849b0e8c846 100644
> --- a/drivers/clk/qcom/camcc-sc7280.c
> +++ b/drivers/clk/qcom/camcc-sc7280.c
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  /*
>   * Copyright (c) 2021, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved.
>   */
>
>  #include <linux/clk-provider.h>
> @@ -2247,6 +2248,9 @@ static struct clk_branch cam_cc_sleep_clk = {
>
>  static struct gdsc cam_cc_titan_top_gdsc = {
>         .gdscr = 0xc194,
> +       .en_rest_wait_val = 0x2,
> +       .en_few_wait_val = 0x2,
> +       .clk_dis_wait_val = 0xf,
>         .pd = {
>                 .name = "cam_cc_titan_top_gdsc",
>         },
> @@ -2256,6 +2260,9 @@ static struct gdsc cam_cc_titan_top_gdsc = {
>
>  static struct gdsc cam_cc_bps_gdsc = {
>         .gdscr = 0x7004,
> +       .en_rest_wait_val = 0x2,
> +       .en_few_wait_val = 0x2,
> +       .clk_dis_wait_val = 0xf,
>         .pd = {
>                 .name = "cam_cc_bps_gdsc",
>         },
> @@ -2265,6 +2272,9 @@ static struct gdsc cam_cc_bps_gdsc = {
>
>  static struct gdsc cam_cc_ife_0_gdsc = {
>         .gdscr = 0xa004,
> +       .en_rest_wait_val = 0x2,
> +       .en_few_wait_val = 0x2,
> +       .clk_dis_wait_val = 0xf,
>         .pd = {
>                 .name = "cam_cc_ife_0_gdsc",
>         },
> @@ -2274,6 +2284,9 @@ static struct gdsc cam_cc_ife_0_gdsc = {
>
>  static struct gdsc cam_cc_ife_1_gdsc = {
>         .gdscr = 0xb004,
> +       .en_rest_wait_val = 0x2,
> +       .en_few_wait_val = 0x2,
> +       .clk_dis_wait_val = 0xf,
>         .pd = {
>                 .name = "cam_cc_ife_1_gdsc",
>         },
> @@ -2283,6 +2296,9 @@ static struct gdsc cam_cc_ife_1_gdsc = {
>
>  static struct gdsc cam_cc_ife_2_gdsc = {
>         .gdscr = 0xb070,
> +       .en_rest_wait_val = 0x2,
> +       .en_few_wait_val = 0x2,
> +       .clk_dis_wait_val = 0xf,
>         .pd = {
>                 .name = "cam_cc_ife_2_gdsc",
>         },
> @@ -2292,6 +2308,9 @@ static struct gdsc cam_cc_ife_2_gdsc = {
>
>  static struct gdsc cam_cc_ipe_0_gdsc = {
>         .gdscr = 0x8004,
> +       .en_rest_wait_val = 0x2,
> +       .en_few_wait_val = 0x2,
> +       .clk_dis_wait_val = 0xf,
>         .pd = {
>                 .name = "cam_cc_ipe_0_gdsc",
>         },
> diff --git a/drivers/clk/qcom/gcc-sc7280.c b/drivers/clk/qcom/gcc-sc7280.c
> index 2b661df5de26..5f3bfb6f4fbb 100644
> --- a/drivers/clk/qcom/gcc-sc7280.c
> +++ b/drivers/clk/qcom/gcc-sc7280.c
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  /*
>   * Copyright (c) 2020-2021, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved.
>   */
>
>  #include <linux/clk-provider.h>
> @@ -3094,6 +3095,9 @@ static struct clk_branch gcc_wpss_rscp_clk = {
>
>  static struct gdsc gcc_pcie_0_gdsc = {
>         .gdscr = 0x6b004,
> +       .en_rest_wait_val = 0x2,
> +       .en_few_wait_val = 0x2,
> +       .clk_dis_wait_val = 0xf,
>         .pd = {
>                 .name = "gcc_pcie_0_gdsc",
>         },
> @@ -3112,6 +3116,9 @@ static struct gdsc gcc_pcie_1_gdsc = {
>
>  static struct gdsc gcc_ufs_phy_gdsc = {
>         .gdscr = 0x77004,
> +       .en_rest_wait_val = 0x2,
> +       .en_few_wait_val = 0x2,
> +       .clk_dis_wait_val = 0xf,
>         .pd = {
>                 .name = "gcc_ufs_phy_gdsc",
>         },
> @@ -3121,6 +3128,9 @@ static struct gdsc gcc_ufs_phy_gdsc = {
>
>  static struct gdsc gcc_usb30_prim_gdsc = {
>         .gdscr = 0xf004,
> +       .en_rest_wait_val = 0x2,
> +       .en_few_wait_val = 0x2,
> +       .clk_dis_wait_val = 0xf,
>         .pd = {
>                 .name = "gcc_usb30_prim_gdsc",
>         },
> @@ -3467,6 +3477,9 @@ static int gcc_sc7280_probe(struct platform_device *pdev)
>         regmap_update_bits(regmap, 0x71004, BIT(0), BIT(0));
>         regmap_update_bits(regmap, 0x7100C, BIT(13), BIT(13));
>
> +       /* FORCE_MEM_CORE_ON for ufs phy ice core clocks */
> +       qcom_branch_set_force_mem_core(regmap, gcc_ufs_phy_ice_core_clk, true);
> +
>         ret = qcom_cc_register_rcg_dfs(regmap, gcc_dfs_clocks,
>                         ARRAY_SIZE(gcc_dfs_clocks));
>         if (ret)
> diff --git a/drivers/clk/qcom/gpucc-sc7280.c b/drivers/clk/qcom/gpucc-sc7280.c
> index 1490cd45a654..0eea4cf7954d 100644
> --- a/drivers/clk/qcom/gpucc-sc7280.c
> +++ b/drivers/clk/qcom/gpucc-sc7280.c
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  /*
>   * Copyright (c) 2021, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved.
>   */
>
>  #include <linux/clk-provider.h>
> @@ -379,6 +380,9 @@ static struct clk_branch gpu_cc_sleep_clk = {
>
>  static struct gdsc cx_gdsc = {
>         .gdscr = 0x106c,
> +       .en_rest_wait_val = 0x2,
> +       .en_few_wait_val = 0x2,
> +       .clk_dis_wait_val = 0x2,
>         .gds_hw_ctrl = 0x1540,
>         .pd = {
>                 .name = "cx_gdsc",
> @@ -389,6 +393,9 @@ static struct gdsc cx_gdsc = {
>
>  static struct gdsc gx_gdsc = {
>         .gdscr = 0x100c,
> +       .en_rest_wait_val = 0x2,
> +       .en_few_wait_val = 0x2,
> +       .clk_dis_wait_val = 0x2,
>         .clamp_io_ctrl = 0x1508,
>         .pd = {
>                 .name = "gx_gdsc",
> diff --git a/drivers/clk/qcom/videocc-sc7280.c b/drivers/clk/qcom/videocc-sc7280.c
> index 615695d82319..b07895c459e8 100644
> --- a/drivers/clk/qcom/videocc-sc7280.c
> +++ b/drivers/clk/qcom/videocc-sc7280.c
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  /*
>   * Copyright (c) 2021, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved.
>   */
>
>  #include <linux/clk-provider.h>
> @@ -232,6 +233,9 @@ static struct clk_branch video_cc_venus_ahb_clk = {
>
>  static struct gdsc mvs0_gdsc = {
>         .gdscr = 0x3004,
> +       .en_rest_wait_val = 0x2,
> +       .en_few_wait_val = 0x2,
> +       .clk_dis_wait_val = 0x6,
>         .pd = {
>                 .name = "mvs0_gdsc",
>         },
> @@ -241,6 +245,9 @@ static struct gdsc mvs0_gdsc = {
>
>  static struct gdsc mvsc_gdsc = {
>         .gdscr = 0x2004,
> +       .en_rest_wait_val = 0x2,
> +       .en_few_wait_val = 0x2,
> +       .clk_dis_wait_val = 0x6,
>         .pd = {
>                 .name = "mvsc_gdsc",
>         },
> --
> 2.17.1
>
>


-- 
With best wishes
Dmitry

Re: [PATCH 3/5] clk: qcom: sc7280: Update the transition delay for GDSC

[Taniya Das]

Thanks for your review.

On 2/8/2024 12:33 PM, Dmitry Baryshkov wrote:
> On Thu, 8 Feb 2024 at 08:29, Taniya Das <quic_tdas@quicinc.com> wrote:
>>
>> The GDSC default values are being updated and they can cause issues
>> in the GDSC FSM state.
> 
> It reads as if new values can cause issues with the FSM.

Ah, sure, will update. The new values will resolve the FSM stability issues.

> 
>> While at it also update the force mem core
>> for UFS ICE clock.
> 
> Separate patch, please.
> 

Sure, will post in the next series.

>>
>> Fixes: fae7617bb142 ("clk: qcom: Add video clock controller driver for SC7280")
>> Fixes: 1daec8cfebc2 ("clk: qcom: camcc: Add camera clock controller driver for SC7280")
>> Fixes: a3cc092196ef ("clk: qcom: Add Global Clock controller (GCC) driver for SC7280")
>> Fixes: 3e0f01d6c7e7 ("clk: qcom: Add graphics clock controller driver for SC7280")
>> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
>> ---
>>   drivers/clk/qcom/camcc-sc7280.c   | 19 +++++++++++++++++++
>>   drivers/clk/qcom/gcc-sc7280.c     | 13 +++++++++++++
>>   drivers/clk/qcom/gpucc-sc7280.c   |  7 +++++++
>>   drivers/clk/qcom/videocc-sc7280.c |  7 +++++++
>>   4 files changed, 46 insertions(+)
>>
>> diff --git a/drivers/clk/qcom/camcc-sc7280.c b/drivers/clk/qcom/camcc-sc7280.c
>> index 49f046ea857c..4849b0e8c846 100644
>> --- a/drivers/clk/qcom/camcc-sc7280.c
>> +++ b/drivers/clk/qcom/camcc-sc7280.c
>> @@ -1,6 +1,7 @@
>>   // SPDX-License-Identifier: GPL-2.0-only
>>   /*
>>    * Copyright (c) 2021, The Linux Foundation. All rights reserved.
>> + * Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved.
>>    */
>>
>>   #include <linux/clk-provider.h>
>> @@ -2247,6 +2248,9 @@ static struct clk_branch cam_cc_sleep_clk = {
>>
>>   static struct gdsc cam_cc_titan_top_gdsc = {
>>          .gdscr = 0xc194,
>> +       .en_rest_wait_val = 0x2,
>> +       .en_few_wait_val = 0x2,
>> +       .clk_dis_wait_val = 0xf,
>>          .pd = {
>>                  .name = "cam_cc_titan_top_gdsc",
>>          },
>> @@ -2256,6 +2260,9 @@ static struct gdsc cam_cc_titan_top_gdsc = {
>>
>>   static struct gdsc cam_cc_bps_gdsc = {
>>          .gdscr = 0x7004,
>> +       .en_rest_wait_val = 0x2,
>> +       .en_few_wait_val = 0x2,
>> +       .clk_dis_wait_val = 0xf,
>>          .pd = {
>>                  .name = "cam_cc_bps_gdsc",
>>          },
>> @@ -2265,6 +2272,9 @@ static struct gdsc cam_cc_bps_gdsc = {
>>
>>   static struct gdsc cam_cc_ife_0_gdsc = {
>>          .gdscr = 0xa004,
>> +       .en_rest_wait_val = 0x2,
>> +       .en_few_wait_val = 0x2,
>> +       .clk_dis_wait_val = 0xf,
>>          .pd = {
>>                  .name = "cam_cc_ife_0_gdsc",
>>          },
>> @@ -2274,6 +2284,9 @@ static struct gdsc cam_cc_ife_0_gdsc = {
>>
>>   static struct gdsc cam_cc_ife_1_gdsc = {
>>          .gdscr = 0xb004,
>> +       .en_rest_wait_val = 0x2,
>> +       .en_few_wait_val = 0x2,
>> +       .clk_dis_wait_val = 0xf,
>>          .pd = {
>>                  .name = "cam_cc_ife_1_gdsc",
>>          },
>> @@ -2283,6 +2296,9 @@ static struct gdsc cam_cc_ife_1_gdsc = {
>>
>>   static struct gdsc cam_cc_ife_2_gdsc = {
>>          .gdscr = 0xb070,
>> +       .en_rest_wait_val = 0x2,
>> +       .en_few_wait_val = 0x2,
>> +       .clk_dis_wait_val = 0xf,
>>          .pd = {
>>                  .name = "cam_cc_ife_2_gdsc",
>>          },
>> @@ -2292,6 +2308,9 @@ static struct gdsc cam_cc_ife_2_gdsc = {
>>
>>   static struct gdsc cam_cc_ipe_0_gdsc = {
>>          .gdscr = 0x8004,
>> +       .en_rest_wait_val = 0x2,
>> +       .en_few_wait_val = 0x2,
>> +       .clk_dis_wait_val = 0xf,
>>          .pd = {
>>                  .name = "cam_cc_ipe_0_gdsc",
>>          },
>> diff --git a/drivers/clk/qcom/gcc-sc7280.c b/drivers/clk/qcom/gcc-sc7280.c
>> index 2b661df5de26..5f3bfb6f4fbb 100644
>> --- a/drivers/clk/qcom/gcc-sc7280.c
>> +++ b/drivers/clk/qcom/gcc-sc7280.c
>> @@ -1,6 +1,7 @@
>>   // SPDX-License-Identifier: GPL-2.0-only
>>   /*
>>    * Copyright (c) 2020-2021, The Linux Foundation. All rights reserved.
>> + * Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved.
>>    */
>>
>>   #include <linux/clk-provider.h>
>> @@ -3094,6 +3095,9 @@ static struct clk_branch gcc_wpss_rscp_clk = {
>>
>>   static struct gdsc gcc_pcie_0_gdsc = {
>>          .gdscr = 0x6b004,
>> +       .en_rest_wait_val = 0x2,
>> +       .en_few_wait_val = 0x2,
>> +       .clk_dis_wait_val = 0xf,
>>          .pd = {
>>                  .name = "gcc_pcie_0_gdsc",
>>          },
>> @@ -3112,6 +3116,9 @@ static struct gdsc gcc_pcie_1_gdsc = {
>>
>>   static struct gdsc gcc_ufs_phy_gdsc = {
>>          .gdscr = 0x77004,
>> +       .en_rest_wait_val = 0x2,
>> +       .en_few_wait_val = 0x2,
>> +       .clk_dis_wait_val = 0xf,
>>          .pd = {
>>                  .name = "gcc_ufs_phy_gdsc",
>>          },
>> @@ -3121,6 +3128,9 @@ static struct gdsc gcc_ufs_phy_gdsc = {
>>
>>   static struct gdsc gcc_usb30_prim_gdsc = {
>>          .gdscr = 0xf004,
>> +       .en_rest_wait_val = 0x2,
>> +       .en_few_wait_val = 0x2,
>> +       .clk_dis_wait_val = 0xf,
>>          .pd = {
>>                  .name = "gcc_usb30_prim_gdsc",
>>          },
>> @@ -3467,6 +3477,9 @@ static int gcc_sc7280_probe(struct platform_device *pdev)
>>          regmap_update_bits(regmap, 0x71004, BIT(0), BIT(0));
>>          regmap_update_bits(regmap, 0x7100C, BIT(13), BIT(13));
>>
>> +       /* FORCE_MEM_CORE_ON for ufs phy ice core clocks */
>> +       qcom_branch_set_force_mem_core(regmap, gcc_ufs_phy_ice_core_clk, true);
>> +
>>          ret = qcom_cc_register_rcg_dfs(regmap, gcc_dfs_clocks,
>>                          ARRAY_SIZE(gcc_dfs_clocks));
>>          if (ret)
>> diff --git a/drivers/clk/qcom/gpucc-sc7280.c b/drivers/clk/qcom/gpucc-sc7280.c
>> index 1490cd45a654..0eea4cf7954d 100644
>> --- a/drivers/clk/qcom/gpucc-sc7280.c
>> +++ b/drivers/clk/qcom/gpucc-sc7280.c
>> @@ -1,6 +1,7 @@
>>   // SPDX-License-Identifier: GPL-2.0-only
>>   /*
>>    * Copyright (c) 2021, The Linux Foundation. All rights reserved.
>> + * Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved.
>>    */
>>
>>   #include <linux/clk-provider.h>
>> @@ -379,6 +380,9 @@ static struct clk_branch gpu_cc_sleep_clk = {
>>
>>   static struct gdsc cx_gdsc = {
>>          .gdscr = 0x106c,
>> +       .en_rest_wait_val = 0x2,
>> +       .en_few_wait_val = 0x2,
>> +       .clk_dis_wait_val = 0x2,
>>          .gds_hw_ctrl = 0x1540,
>>          .pd = {
>>                  .name = "cx_gdsc",
>> @@ -389,6 +393,9 @@ static struct gdsc cx_gdsc = {
>>
>>   static struct gdsc gx_gdsc = {
>>          .gdscr = 0x100c,
>> +       .en_rest_wait_val = 0x2,
>> +       .en_few_wait_val = 0x2,
>> +       .clk_dis_wait_val = 0x2,
>>          .clamp_io_ctrl = 0x1508,
>>          .pd = {
>>                  .name = "gx_gdsc",
>> diff --git a/drivers/clk/qcom/videocc-sc7280.c b/drivers/clk/qcom/videocc-sc7280.c
>> index 615695d82319..b07895c459e8 100644
>> --- a/drivers/clk/qcom/videocc-sc7280.c
>> +++ b/drivers/clk/qcom/videocc-sc7280.c
>> @@ -1,6 +1,7 @@
>>   // SPDX-License-Identifier: GPL-2.0-only
>>   /*
>>    * Copyright (c) 2021, The Linux Foundation. All rights reserved.
>> + * Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved.
>>    */
>>
>>   #include <linux/clk-provider.h>
>> @@ -232,6 +233,9 @@ static struct clk_branch video_cc_venus_ahb_clk = {
>>
>>   static struct gdsc mvs0_gdsc = {
>>          .gdscr = 0x3004,
>> +       .en_rest_wait_val = 0x2,
>> +       .en_few_wait_val = 0x2,
>> +       .clk_dis_wait_val = 0x6,
>>          .pd = {
>>                  .name = "mvs0_gdsc",
>>          },
>> @@ -241,6 +245,9 @@ static struct gdsc mvs0_gdsc = {
>>
>>   static struct gdsc mvsc_gdsc = {
>>          .gdscr = 0x2004,
>> +       .en_rest_wait_val = 0x2,
>> +       .en_few_wait_val = 0x2,
>> +       .clk_dis_wait_val = 0x6,
>>          .pd = {
>>                  .name = "mvsc_gdsc",
>>          },
>> --
>> 2.17.1
>>
>>
> 
> 

-- 
Thanks & Regards,
Taniya Das.

[PATCH 4/5] clk: qcom: camcc-sc7280: Add parent dependency to all camera GDSCs

[Taniya Das]

Camera titan top GDSC is a parent supply to all other camera GDSCs. Titan
top GDSC is required to be enabled before enabling any other camera GDSCs
and it should be disabled only after all other camera GDSCs are disabled.
Ensure this behavior by marking titan top GDSC as parent of all other
camera GDSCs.

Fixes: 1daec8cfebc2 ("clk: qcom: camcc: Add camera clock controller driver for SC7280")
Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
---
 drivers/clk/qcom/camcc-sc7280.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/clk/qcom/camcc-sc7280.c b/drivers/clk/qcom/camcc-sc7280.c
index 4849b0e8c846..7f8ebcba30ce 100644
--- a/drivers/clk/qcom/camcc-sc7280.c
+++ b/drivers/clk/qcom/camcc-sc7280.c
@@ -2267,6 +2267,7 @@ static struct gdsc cam_cc_bps_gdsc = {
 		.name = "cam_cc_bps_gdsc",
 	},
 	.pwrsts = PWRSTS_OFF_ON,
+	.parent = &cam_cc_titan_top_gdsc.pd,
 	.flags = HW_CTRL | RETAIN_FF_ENABLE,
 };

@@ -2279,6 +2280,7 @@ static struct gdsc cam_cc_ife_0_gdsc = {
 		.name = "cam_cc_ife_0_gdsc",
 	},
 	.pwrsts = PWRSTS_OFF_ON,
+	.parent = &cam_cc_titan_top_gdsc.pd,
 	.flags = RETAIN_FF_ENABLE,
 };

@@ -2291,6 +2293,7 @@ static struct gdsc cam_cc_ife_1_gdsc = {
 		.name = "cam_cc_ife_1_gdsc",
 	},
 	.pwrsts = PWRSTS_OFF_ON,
+	.parent = &cam_cc_titan_top_gdsc.pd,
 	.flags = RETAIN_FF_ENABLE,
 };

@@ -2303,6 +2306,7 @@ static struct gdsc cam_cc_ife_2_gdsc = {
 		.name = "cam_cc_ife_2_gdsc",
 	},
 	.pwrsts = PWRSTS_OFF_ON,
+	.parent = &cam_cc_titan_top_gdsc.pd,
 	.flags = RETAIN_FF_ENABLE,
 };

@@ -2315,6 +2319,7 @@ static struct gdsc cam_cc_ipe_0_gdsc = {
 		.name = "cam_cc_ipe_0_gdsc",
 	},
 	.pwrsts = PWRSTS_OFF_ON,
+	.parent = &cam_cc_titan_top_gdsc.pd,
 	.flags = HW_CTRL | RETAIN_FF_ENABLE,
 };

--
2.17.1

Re: [PATCH 4/5] clk: qcom: camcc-sc7280: Add parent dependency to all camera GDSCs

[Dmitry Baryshkov]

On Thu, 8 Feb 2024 at 08:29, Taniya Das <quic_tdas@quicinc.com> wrote:
>
> Camera titan top GDSC is a parent supply to all other camera GDSCs. Titan
> top GDSC is required to be enabled before enabling any other camera GDSCs
> and it should be disabled only after all other camera GDSCs are disabled.
> Ensure this behavior by marking titan top GDSC as parent of all other
> camera GDSCs.
>
> Fixes: 1daec8cfebc2 ("clk: qcom: camcc: Add camera clock controller driver for SC7280")
> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

> ---
>  drivers/clk/qcom/camcc-sc7280.c | 5 +++++
>  1 file changed, 5 insertions(+)


-- 
With best wishes
Dmitry

[PATCH 5/5] arm64: dts: qcom: Update protected clocks list for qcm6490 variants

[Taniya Das]

Certain clocks are not accessible on QCM6490-IDP and QCS6490-RB3GEN2 boards
thus require them to be marked protected.

Also disable the LPASS nodes which are not to be used.

Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
---
 arch/arm64/boot/dts/qcom/qcm6490-idp.dts     | 54 +++++++++++++++++++-
 arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 50 +++++++++++++++++-
 2 files changed, 102 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
index 03e97e27d16d..425e4b87490b 100644
--- a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
+++ b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: BSD-3-Clause
 /*
- * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
+ * Copyright (c) 2023-2024 Qualcomm Innovation Center, Inc. All rights reserved.
  */

 /dts-v1/;
@@ -415,6 +415,58 @@
 	};
 };

+&gcc {
+	protected-clocks = <GCC_AGGRE_NOC_PCIE_1_AXI_CLK> ,<GCC_PCIE_1_AUX_CLK>,
+			<GCC_PCIE_1_AUX_CLK_SRC>, <GCC_PCIE_1_CFG_AHB_CLK>,
+			<GCC_PCIE_1_MSTR_AXI_CLK>, <GCC_PCIE_1_PHY_RCHNG_CLK_SRC>,
+			<GCC_PCIE_1_PIPE_CLK>, <GCC_PCIE_1_PIPE_CLK_SRC>,
+			<GCC_PCIE_1_SLV_AXI_CLK>, <GCC_PCIE_1_SLV_Q2A_AXI_CLK>,
+			<GCC_QSPI_CNOC_PERIPH_AHB_CLK>, <GCC_QSPI_CORE_CLK>,
+			<GCC_QSPI_CORE_CLK_SRC>,<GCC_USB30_SEC_MASTER_CLK>,
+			<GCC_USB30_SEC_MASTER_CLK_SRC>, <GCC_USB30_SEC_MOCK_UTMI_CLK>,
+			<GCC_USB30_SEC_MOCK_UTMI_CLK_SRC>,
+			<GCC_USB30_SEC_MOCK_UTMI_POSTDIV_CLK_SRC>, <GCC_USB30_SEC_SLEEP_CLK>,
+			<GCC_USB3_SEC_PHY_AUX_CLK>, <GCC_USB3_SEC_PHY_AUX_CLK_SRC>,
+			<GCC_USB3_SEC_PHY_COM_AUX_CLK>, <GCC_USB3_SEC_PHY_PIPE_CLK>,
+			<GCC_USB3_SEC_PHY_PIPE_CLK_SRC>, <GCC_CFG_NOC_LPASS_CLK>,
+			<GCC_MSS_GPLL0_MAIN_DIV_CLK_SRC>, <GCC_MSS_CFG_AHB_CLK>,
+			<GCC_MSS_OFFLINE_AXI_CLK>, <GCC_MSS_SNOC_AXI_CLK>,
+			<GCC_MSS_Q6_MEMNOC_AXI_CLK>, <GCC_MSS_Q6SS_BOOT_CLK_SRC>,
+			<GCC_SEC_CTRL_CLK_SRC>, <GCC_WPSS_AHB_CLK>,
+			<GCC_WPSS_AHB_BDG_MST_CLK>, <GCC_WPSS_RSCP_CLK>;
+};
+
+&lpasscc {
+	status = "disabled";
+};
+
+&lpass_audiocc {
+	qcom,adsp-skip-pll;
+	protected-clocks = <LPASS_AUDIO_CC_CDIV_RX_MCLK_DIV_CLK_SRC>,
+		<LPASS_AUDIO_CC_CODEC_MEM0_CLK>, <LPASS_AUDIO_CC_CODEC_MEM1_CLK>,
+		<LPASS_AUDIO_CC_CODEC_MEM2_CLK>, <LPASS_AUDIO_CC_CODEC_MEM_CLK>,
+		<LPASS_AUDIO_CC_EXT_MCLK0_CLK>, <LPASS_AUDIO_CC_EXT_MCLK0_CLK_SRC>,
+		<LPASS_AUDIO_CC_EXT_MCLK1_CLK>, <LPASS_AUDIO_CC_EXT_MCLK1_CLK_SRC>,
+		<LPASS_AUDIO_CC_PLL>, <LPASS_AUDIO_CC_PLL_OUT_AUX2>,
+		<LPASS_AUDIO_CC_PLL_OUT_AUX2_DIV_CLK_SRC>,
+		<LPASS_AUDIO_CC_PLL_OUT_MAIN_DIV_CLK_SRC>,
+		<LPASS_AUDIO_CC_RX_MCLK_2X_CLK>, <LPASS_AUDIO_CC_RX_MCLK_CLK>,
+		<LPASS_AUDIO_CC_RX_MCLK_CLK_SRC>;
+	/delete-property/ power-domains;
+};
+
+&lpass_aon {
+	status = "disabled";
+};
+
+&lpass_core {
+	status = "disabled";
+};
+
+&lpass_hm {
+	status = "disabled";
+};
+
 &qupv3_id_0 {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
index 8bb7d13d85f6..1398b84634c3 100644
--- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
+++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: BSD-3-Clause
 /*
- * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
+ * Copyright (c) 2023-2024 Qualcomm Innovation Center, Inc. All rights reserved.
  */

 /dts-v1/;
@@ -413,6 +413,54 @@
 	};
 };

+&gcc {
+	protected-clocks = <GCC_QSPI_CNOC_PERIPH_AHB_CLK>, <GCC_QSPI_CORE_CLK>,
+			<GCC_QSPI_CORE_CLK_SRC>,<GCC_USB30_SEC_MASTER_CLK>,
+			<GCC_USB30_SEC_MASTER_CLK_SRC>, <GCC_USB30_SEC_MOCK_UTMI_CLK>,
+			<GCC_USB30_SEC_MOCK_UTMI_CLK_SRC>,
+			<GCC_USB30_SEC_MOCK_UTMI_POSTDIV_CLK_SRC>, <GCC_USB30_SEC_SLEEP_CLK>,
+			<GCC_USB3_SEC_PHY_AUX_CLK>, <GCC_USB3_SEC_PHY_AUX_CLK_SRC>,
+			<GCC_USB3_SEC_PHY_COM_AUX_CLK>, <GCC_USB3_SEC_PHY_PIPE_CLK>,
+			<GCC_USB3_SEC_PHY_PIPE_CLK_SRC>, <GCC_CFG_NOC_LPASS_CLK>,
+			<GCC_MSS_GPLL0_MAIN_DIV_CLK_SRC>, <GCC_MSS_CFG_AHB_CLK>,
+			<GCC_MSS_OFFLINE_AXI_CLK>, <GCC_MSS_SNOC_AXI_CLK>,
+			<GCC_MSS_Q6_MEMNOC_AXI_CLK>, <GCC_MSS_Q6SS_BOOT_CLK_SRC>,
+			<GCC_SEC_CTRL_CLK_SRC>, <GCC_WPSS_AHB_CLK>,
+			<GCC_WPSS_AHB_BDG_MST_CLK>, <GCC_WPSS_RSCP_CLK>;
+};
+
+&lpasscc {
+	status = "disabled";
+};
+
+&lpass_audiocc {
+	qcom,adsp-skip-pll;
+	protected-clocks = <LPASS_AUDIO_CC_CDIV_RX_MCLK_DIV_CLK_SRC>,
+		<LPASS_AUDIO_CC_CODEC_MEM0_CLK>, <LPASS_AUDIO_CC_CODEC_MEM1_CLK>,
+		<LPASS_AUDIO_CC_CODEC_MEM2_CLK>, <LPASS_AUDIO_CC_CODEC_MEM_CLK>,
+		<LPASS_AUDIO_CC_EXT_MCLK0_CLK>, <LPASS_AUDIO_CC_EXT_MCLK0_CLK_SRC>,
+		<LPASS_AUDIO_CC_EXT_MCLK1_CLK>, <LPASS_AUDIO_CC_EXT_MCLK1_CLK_SRC>,
+		<LPASS_AUDIO_CC_PLL>, <LPASS_AUDIO_CC_PLL_OUT_AUX2>,
+		<LPASS_AUDIO_CC_PLL_OUT_AUX2_DIV_CLK_SRC>,
+		<LPASS_AUDIO_CC_PLL_OUT_MAIN_DIV_CLK_SRC>,
+		<LPASS_AUDIO_CC_RX_MCLK_2X_CLK>, <LPASS_AUDIO_CC_RX_MCLK_CLK>,
+		<LPASS_AUDIO_CC_RX_MCLK_CLK_SRC>;
+	/delete-property/ power-domains;
+};
+
+&lpass_aon {
+	status = "disabled";
+};
+
+&lpass_core {
+	status = "disabled";
+};
+
+&lpass_hm {
+	status = "disabled";
+};
+
+
 &qupv3_id_0 {
 	status = "okay";
 };
--
2.17.1

Re: [PATCH 5/5] arm64: dts: qcom: Update protected clocks list for qcm6490 variants

[Dmitry Baryshkov]

On Thu, 8 Feb 2024 at 08:29, Taniya Das <quic_tdas@quicinc.com> wrote:
>
> Certain clocks are not accessible on QCM6490-IDP and QCS6490-RB3GEN2 boards
> thus require them to be marked protected.
>
> Also disable the LPASS nodes which are not to be used.
>
> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/qcm6490-idp.dts     | 54 +++++++++++++++++++-
>  arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 50 +++++++++++++++++-
>  2 files changed, 102 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
> index 03e97e27d16d..425e4b87490b 100644
> --- a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
> +++ b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
> @@ -1,6 +1,6 @@
>  // SPDX-License-Identifier: BSD-3-Clause
>  /*
> - * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
> + * Copyright (c) 2023-2024 Qualcomm Innovation Center, Inc. All rights reserved.
>   */
>
>  /dts-v1/;
> @@ -415,6 +415,58 @@
>         };
>  };
>
> +&gcc {
> +       protected-clocks = <GCC_AGGRE_NOC_PCIE_1_AXI_CLK> ,<GCC_PCIE_1_AUX_CLK>,
> +                       <GCC_PCIE_1_AUX_CLK_SRC>, <GCC_PCIE_1_CFG_AHB_CLK>,
> +                       <GCC_PCIE_1_MSTR_AXI_CLK>, <GCC_PCIE_1_PHY_RCHNG_CLK_SRC>,
> +                       <GCC_PCIE_1_PIPE_CLK>, <GCC_PCIE_1_PIPE_CLK_SRC>,
> +                       <GCC_PCIE_1_SLV_AXI_CLK>, <GCC_PCIE_1_SLV_Q2A_AXI_CLK>,
> +                       <GCC_QSPI_CNOC_PERIPH_AHB_CLK>, <GCC_QSPI_CORE_CLK>,
> +                       <GCC_QSPI_CORE_CLK_SRC>,<GCC_USB30_SEC_MASTER_CLK>,
> +                       <GCC_USB30_SEC_MASTER_CLK_SRC>, <GCC_USB30_SEC_MOCK_UTMI_CLK>,
> +                       <GCC_USB30_SEC_MOCK_UTMI_CLK_SRC>,
> +                       <GCC_USB30_SEC_MOCK_UTMI_POSTDIV_CLK_SRC>, <GCC_USB30_SEC_SLEEP_CLK>,
> +                       <GCC_USB3_SEC_PHY_AUX_CLK>, <GCC_USB3_SEC_PHY_AUX_CLK_SRC>,
> +                       <GCC_USB3_SEC_PHY_COM_AUX_CLK>, <GCC_USB3_SEC_PHY_PIPE_CLK>,
> +                       <GCC_USB3_SEC_PHY_PIPE_CLK_SRC>, <GCC_CFG_NOC_LPASS_CLK>,
> +                       <GCC_MSS_GPLL0_MAIN_DIV_CLK_SRC>, <GCC_MSS_CFG_AHB_CLK>,
> +                       <GCC_MSS_OFFLINE_AXI_CLK>, <GCC_MSS_SNOC_AXI_CLK>,
> +                       <GCC_MSS_Q6_MEMNOC_AXI_CLK>, <GCC_MSS_Q6SS_BOOT_CLK_SRC>,
> +                       <GCC_SEC_CTRL_CLK_SRC>, <GCC_WPSS_AHB_CLK>,
> +                       <GCC_WPSS_AHB_BDG_MST_CLK>, <GCC_WPSS_RSCP_CLK>;

This looks like a huge variety of clocks. Are they really not
accessible or are you trying to make Linux stay away from all those
clocks to keep bootloader settings?

> +};
> +
> +&lpasscc {
> +       status = "disabled";
> +};
> +
> +&lpass_audiocc {
> +       qcom,adsp-skip-pll;
> +       protected-clocks = <LPASS_AUDIO_CC_CDIV_RX_MCLK_DIV_CLK_SRC>,
> +               <LPASS_AUDIO_CC_CODEC_MEM0_CLK>, <LPASS_AUDIO_CC_CODEC_MEM1_CLK>,
> +               <LPASS_AUDIO_CC_CODEC_MEM2_CLK>, <LPASS_AUDIO_CC_CODEC_MEM_CLK>,
> +               <LPASS_AUDIO_CC_EXT_MCLK0_CLK>, <LPASS_AUDIO_CC_EXT_MCLK0_CLK_SRC>,
> +               <LPASS_AUDIO_CC_EXT_MCLK1_CLK>, <LPASS_AUDIO_CC_EXT_MCLK1_CLK_SRC>,
> +               <LPASS_AUDIO_CC_PLL>, <LPASS_AUDIO_CC_PLL_OUT_AUX2>,
> +               <LPASS_AUDIO_CC_PLL_OUT_AUX2_DIV_CLK_SRC>,
> +               <LPASS_AUDIO_CC_PLL_OUT_MAIN_DIV_CLK_SRC>,
> +               <LPASS_AUDIO_CC_RX_MCLK_2X_CLK>, <LPASS_AUDIO_CC_RX_MCLK_CLK>,
> +               <LPASS_AUDIO_CC_RX_MCLK_CLK_SRC>;

This almost looks like a separate compatible.

> +       /delete-property/ power-domains;
> +};
> +
> +&lpass_aon {
> +       status = "disabled";

Should this be "reserved", controlled by ADSP? See how this was
implemented in sc7180.dtsi / sc7180-trogdor.dtsi.
Please consider inverting the logic. Generic ADSP implementation
should be present in sc7280.dtsi and then the non-default ChromeOS
implementation should be a part of sc7280-chrome-common.dtsi.

> +};
> +
> +&lpass_core {
> +       status = "disabled";
> +};
> +
> +&lpass_hm {
> +       status = "disabled";
> +};
> +
>  &qupv3_id_0 {
>         status = "okay";
>  };
> diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> index 8bb7d13d85f6..1398b84634c3 100644
> --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> @@ -1,6 +1,6 @@
>  // SPDX-License-Identifier: BSD-3-Clause
>  /*
> - * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
> + * Copyright (c) 2023-2024 Qualcomm Innovation Center, Inc. All rights reserved.
>   */
>
>  /dts-v1/;
> @@ -413,6 +413,54 @@
>         };
>  };
>
> +&gcc {
> +       protected-clocks = <GCC_QSPI_CNOC_PERIPH_AHB_CLK>, <GCC_QSPI_CORE_CLK>,
> +                       <GCC_QSPI_CORE_CLK_SRC>,<GCC_USB30_SEC_MASTER_CLK>,
> +                       <GCC_USB30_SEC_MASTER_CLK_SRC>, <GCC_USB30_SEC_MOCK_UTMI_CLK>,
> +                       <GCC_USB30_SEC_MOCK_UTMI_CLK_SRC>,
> +                       <GCC_USB30_SEC_MOCK_UTMI_POSTDIV_CLK_SRC>, <GCC_USB30_SEC_SLEEP_CLK>,
> +                       <GCC_USB3_SEC_PHY_AUX_CLK>, <GCC_USB3_SEC_PHY_AUX_CLK_SRC>,
> +                       <GCC_USB3_SEC_PHY_COM_AUX_CLK>, <GCC_USB3_SEC_PHY_PIPE_CLK>,
> +                       <GCC_USB3_SEC_PHY_PIPE_CLK_SRC>, <GCC_CFG_NOC_LPASS_CLK>,
> +                       <GCC_MSS_GPLL0_MAIN_DIV_CLK_SRC>, <GCC_MSS_CFG_AHB_CLK>,
> +                       <GCC_MSS_OFFLINE_AXI_CLK>, <GCC_MSS_SNOC_AXI_CLK>,
> +                       <GCC_MSS_Q6_MEMNOC_AXI_CLK>, <GCC_MSS_Q6SS_BOOT_CLK_SRC>,
> +                       <GCC_SEC_CTRL_CLK_SRC>, <GCC_WPSS_AHB_CLK>,
> +                       <GCC_WPSS_AHB_BDG_MST_CLK>, <GCC_WPSS_RSCP_CLK>;
> +};
> +
> +&lpasscc {
> +       status = "disabled";
> +};
> +
> +&lpass_audiocc {
> +       qcom,adsp-skip-pll;
> +       protected-clocks = <LPASS_AUDIO_CC_CDIV_RX_MCLK_DIV_CLK_SRC>,
> +               <LPASS_AUDIO_CC_CODEC_MEM0_CLK>, <LPASS_AUDIO_CC_CODEC_MEM1_CLK>,
> +               <LPASS_AUDIO_CC_CODEC_MEM2_CLK>, <LPASS_AUDIO_CC_CODEC_MEM_CLK>,
> +               <LPASS_AUDIO_CC_EXT_MCLK0_CLK>, <LPASS_AUDIO_CC_EXT_MCLK0_CLK_SRC>,
> +               <LPASS_AUDIO_CC_EXT_MCLK1_CLK>, <LPASS_AUDIO_CC_EXT_MCLK1_CLK_SRC>,
> +               <LPASS_AUDIO_CC_PLL>, <LPASS_AUDIO_CC_PLL_OUT_AUX2>,
> +               <LPASS_AUDIO_CC_PLL_OUT_AUX2_DIV_CLK_SRC>,
> +               <LPASS_AUDIO_CC_PLL_OUT_MAIN_DIV_CLK_SRC>,
> +               <LPASS_AUDIO_CC_RX_MCLK_2X_CLK>, <LPASS_AUDIO_CC_RX_MCLK_CLK>,
> +               <LPASS_AUDIO_CC_RX_MCLK_CLK_SRC>;
> +       /delete-property/ power-domains;
> +};
> +
> +&lpass_aon {
> +       status = "disabled";
> +};
> +
> +&lpass_core {
> +       status = "disabled";
> +};
> +
> +&lpass_hm {
> +       status = "disabled";
> +};
> +
> +
>  &qupv3_id_0 {
>         status = "okay";
>  };
> --
> 2.17.1
>
>


-- 
With best wishes
Dmitry

Re: [PATCH 5/5] arm64: dts: qcom: Update protected clocks list for qcm6490 variants

[Taniya Das]

Thanks for your review Dmitry.

On 2/8/2024 12:44 PM, Dmitry Baryshkov wrote:
> On Thu, 8 Feb 2024 at 08:29, Taniya Das <quic_tdas@quicinc.com> wrote:
>>
>> Certain clocks are not accessible on QCM6490-IDP and QCS6490-RB3GEN2 boards
>> thus require them to be marked protected.
>>
>> Also disable the LPASS nodes which are not to be used.
>>
>> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
>> ---
>>   arch/arm64/boot/dts/qcom/qcm6490-idp.dts     | 54 +++++++++++++++++++-
>>   arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 50 +++++++++++++++++-
>>   2 files changed, 102 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
>> index 03e97e27d16d..425e4b87490b 100644
>> --- a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
>> +++ b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
>> @@ -1,6 +1,6 @@
>>   // SPDX-License-Identifier: BSD-3-Clause
>>   /*
>> - * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
>> + * Copyright (c) 2023-2024 Qualcomm Innovation Center, Inc. All rights reserved.
>>    */
>>
>>   /dts-v1/;
>> @@ -415,6 +415,58 @@
>>          };
>>   };
>>
>> +&gcc {
>> +       protected-clocks = <GCC_AGGRE_NOC_PCIE_1_AXI_CLK> ,<GCC_PCIE_1_AUX_CLK>,
>> +                       <GCC_PCIE_1_AUX_CLK_SRC>, <GCC_PCIE_1_CFG_AHB_CLK>,
>> +                       <GCC_PCIE_1_MSTR_AXI_CLK>, <GCC_PCIE_1_PHY_RCHNG_CLK_SRC>,
>> +                       <GCC_PCIE_1_PIPE_CLK>, <GCC_PCIE_1_PIPE_CLK_SRC>,
>> +                       <GCC_PCIE_1_SLV_AXI_CLK>, <GCC_PCIE_1_SLV_Q2A_AXI_CLK>,
>> +                       <GCC_QSPI_CNOC_PERIPH_AHB_CLK>, <GCC_QSPI_CORE_CLK>,
>> +                       <GCC_QSPI_CORE_CLK_SRC>,<GCC_USB30_SEC_MASTER_CLK>,
>> +                       <GCC_USB30_SEC_MASTER_CLK_SRC>, <GCC_USB30_SEC_MOCK_UTMI_CLK>,
>> +                       <GCC_USB30_SEC_MOCK_UTMI_CLK_SRC>,
>> +                       <GCC_USB30_SEC_MOCK_UTMI_POSTDIV_CLK_SRC>, <GCC_USB30_SEC_SLEEP_CLK>,
>> +                       <GCC_USB3_SEC_PHY_AUX_CLK>, <GCC_USB3_SEC_PHY_AUX_CLK_SRC>,
>> +                       <GCC_USB3_SEC_PHY_COM_AUX_CLK>, <GCC_USB3_SEC_PHY_PIPE_CLK>,
>> +                       <GCC_USB3_SEC_PHY_PIPE_CLK_SRC>, <GCC_CFG_NOC_LPASS_CLK>,
>> +                       <GCC_MSS_GPLL0_MAIN_DIV_CLK_SRC>, <GCC_MSS_CFG_AHB_CLK>,
>> +                       <GCC_MSS_OFFLINE_AXI_CLK>, <GCC_MSS_SNOC_AXI_CLK>,
>> +                       <GCC_MSS_Q6_MEMNOC_AXI_CLK>, <GCC_MSS_Q6SS_BOOT_CLK_SRC>,
>> +                       <GCC_SEC_CTRL_CLK_SRC>, <GCC_WPSS_AHB_CLK>,
>> +                       <GCC_WPSS_AHB_BDG_MST_CLK>, <GCC_WPSS_RSCP_CLK>;
> 
> This looks like a huge variety of clocks. Are they really not
> accessible or are you trying to make Linux stay away from all those
> clocks to keep bootloader settings?
> 

These clocks are protected and accessing them from Linux will cause Bus 
error (NoC) and thus this list grows sometimes.

>> +};
>> +
>> +&lpasscc {
>> +       status = "disabled";
>> +};
>> +
>> +&lpass_audiocc {
>> +       qcom,adsp-skip-pll;
>> +       protected-clocks = <LPASS_AUDIO_CC_CDIV_RX_MCLK_DIV_CLK_SRC>,
>> +               <LPASS_AUDIO_CC_CODEC_MEM0_CLK>, <LPASS_AUDIO_CC_CODEC_MEM1_CLK>,
>> +               <LPASS_AUDIO_CC_CODEC_MEM2_CLK>, <LPASS_AUDIO_CC_CODEC_MEM_CLK>,
>> +               <LPASS_AUDIO_CC_EXT_MCLK0_CLK>, <LPASS_AUDIO_CC_EXT_MCLK0_CLK_SRC>,
>> +               <LPASS_AUDIO_CC_EXT_MCLK1_CLK>, <LPASS_AUDIO_CC_EXT_MCLK1_CLK_SRC>,
>> +               <LPASS_AUDIO_CC_PLL>, <LPASS_AUDIO_CC_PLL_OUT_AUX2>,
>> +               <LPASS_AUDIO_CC_PLL_OUT_AUX2_DIV_CLK_SRC>,
>> +               <LPASS_AUDIO_CC_PLL_OUT_MAIN_DIV_CLK_SRC>,
>> +               <LPASS_AUDIO_CC_RX_MCLK_2X_CLK>, <LPASS_AUDIO_CC_RX_MCLK_CLK>,
>> +               <LPASS_AUDIO_CC_RX_MCLK_CLK_SRC>;
> 
> This almost looks like a separate compatible.

We need to use the reset from this controller, rest all will be 
controlled via the Low Power Audio Firmware.

> 
>> +       /delete-property/ power-domains;
>> +};
>> +
>> +&lpass_aon {
>> +       status = "disabled";
> 
> Should this be "reserved", controlled by ADSP? See how this was
> implemented in sc7180.dtsi / sc7180-trogdor.dtsi.
> Please consider inverting the logic. Generic ADSP implementation
> should be present in sc7280.dtsi and then the non-default ChromeOS
> implementation should be a part of sc7280-chrome-common.dtsi.
> 

This I will take care in the next series.

>> +};
>> +
>> +&lpass_core {
>> +       status = "disabled";
>> +};
>> +
>> +&lpass_hm {
>> +       status = "disabled";
>> +};
>> +
>>   &qupv3_id_0 {
>>          status = "okay";
>>   };
>> diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
>> index 8bb7d13d85f6..1398b84634c3 100644
>> --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
>> +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
>> @@ -1,6 +1,6 @@
>>   // SPDX-License-Identifier: BSD-3-Clause
>>   /*
>> - * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
>> + * Copyright (c) 2023-2024 Qualcomm Innovation Center, Inc. All rights reserved.
>>    */
>>
>>   /dts-v1/;
>> @@ -413,6 +413,54 @@
>>          };
>>   };
>>
>> +&gcc {
>> +       protected-clocks = <GCC_QSPI_CNOC_PERIPH_AHB_CLK>, <GCC_QSPI_CORE_CLK>,
>> +                       <GCC_QSPI_CORE_CLK_SRC>,<GCC_USB30_SEC_MASTER_CLK>,
>> +                       <GCC_USB30_SEC_MASTER_CLK_SRC>, <GCC_USB30_SEC_MOCK_UTMI_CLK>,
>> +                       <GCC_USB30_SEC_MOCK_UTMI_CLK_SRC>,
>> +                       <GCC_USB30_SEC_MOCK_UTMI_POSTDIV_CLK_SRC>, <GCC_USB30_SEC_SLEEP_CLK>,
>> +                       <GCC_USB3_SEC_PHY_AUX_CLK>, <GCC_USB3_SEC_PHY_AUX_CLK_SRC>,
>> +                       <GCC_USB3_SEC_PHY_COM_AUX_CLK>, <GCC_USB3_SEC_PHY_PIPE_CLK>,
>> +                       <GCC_USB3_SEC_PHY_PIPE_CLK_SRC>, <GCC_CFG_NOC_LPASS_CLK>,
>> +                       <GCC_MSS_GPLL0_MAIN_DIV_CLK_SRC>, <GCC_MSS_CFG_AHB_CLK>,
>> +                       <GCC_MSS_OFFLINE_AXI_CLK>, <GCC_MSS_SNOC_AXI_CLK>,
>> +                       <GCC_MSS_Q6_MEMNOC_AXI_CLK>, <GCC_MSS_Q6SS_BOOT_CLK_SRC>,
>> +                       <GCC_SEC_CTRL_CLK_SRC>, <GCC_WPSS_AHB_CLK>,
>> +                       <GCC_WPSS_AHB_BDG_MST_CLK>, <GCC_WPSS_RSCP_CLK>;
>> +};
>> +
>> +&lpasscc {
>> +       status = "disabled";
>> +};
>> +
>> +&lpass_audiocc {
>> +       qcom,adsp-skip-pll;
>> +       protected-clocks = <LPASS_AUDIO_CC_CDIV_RX_MCLK_DIV_CLK_SRC>,
>> +               <LPASS_AUDIO_CC_CODEC_MEM0_CLK>, <LPASS_AUDIO_CC_CODEC_MEM1_CLK>,
>> +               <LPASS_AUDIO_CC_CODEC_MEM2_CLK>, <LPASS_AUDIO_CC_CODEC_MEM_CLK>,
>> +               <LPASS_AUDIO_CC_EXT_MCLK0_CLK>, <LPASS_AUDIO_CC_EXT_MCLK0_CLK_SRC>,
>> +               <LPASS_AUDIO_CC_EXT_MCLK1_CLK>, <LPASS_AUDIO_CC_EXT_MCLK1_CLK_SRC>,
>> +               <LPASS_AUDIO_CC_PLL>, <LPASS_AUDIO_CC_PLL_OUT_AUX2>,
>> +               <LPASS_AUDIO_CC_PLL_OUT_AUX2_DIV_CLK_SRC>,
>> +               <LPASS_AUDIO_CC_PLL_OUT_MAIN_DIV_CLK_SRC>,
>> +               <LPASS_AUDIO_CC_RX_MCLK_2X_CLK>, <LPASS_AUDIO_CC_RX_MCLK_CLK>,
>> +               <LPASS_AUDIO_CC_RX_MCLK_CLK_SRC>;
>> +       /delete-property/ power-domains;
>> +};
>> +
>> +&lpass_aon {
>> +       status = "disabled";
>> +};
>> +
>> +&lpass_core {
>> +       status = "disabled";
>> +};
>> +
>> +&lpass_hm {
>> +       status = "disabled";
>> +};
>> +
>> +
>>   &qupv3_id_0 {
>>          status = "okay";
>>   };
>> --
>> 2.17.1
>>
>>
> 
> 

-- 
Thanks & Regards,
Taniya Das.

Re: [PATCH 5/5] arm64: dts: qcom: Update protected clocks list for qcm6490 variants

[Dmitry Baryshkov]

On Wed, 6 Mar 2024 at 08:54, Taniya Das <quic_tdas@quicinc.com> wrote:
>
> Thanks for your review Dmitry.
>
> On 2/8/2024 12:44 PM, Dmitry Baryshkov wrote:
> > On Thu, 8 Feb 2024 at 08:29, Taniya Das <quic_tdas@quicinc.com> wrote:
> >>
> >> Certain clocks are not accessible on QCM6490-IDP and QCS6490-RB3GEN2 boards
> >> thus require them to be marked protected.
> >>
> >> Also disable the LPASS nodes which are not to be used.
> >>
> >> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
> >> ---
> >>   arch/arm64/boot/dts/qcom/qcm6490-idp.dts     | 54 +++++++++++++++++++-
> >>   arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 50 +++++++++++++++++-
> >>   2 files changed, 102 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
> >> index 03e97e27d16d..425e4b87490b 100644
> >> --- a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
> >> +++ b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
> >> @@ -1,6 +1,6 @@
> >>   // SPDX-License-Identifier: BSD-3-Clause
> >>   /*
> >> - * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
> >> + * Copyright (c) 2023-2024 Qualcomm Innovation Center, Inc. All rights reserved.
> >>    */
> >>
> >>   /dts-v1/;
> >> @@ -415,6 +415,58 @@
> >>          };
> >>   };
> >>
> >> +&gcc {
> >> +       protected-clocks = <GCC_AGGRE_NOC_PCIE_1_AXI_CLK> ,<GCC_PCIE_1_AUX_CLK>,
> >> +                       <GCC_PCIE_1_AUX_CLK_SRC>, <GCC_PCIE_1_CFG_AHB_CLK>,
> >> +                       <GCC_PCIE_1_MSTR_AXI_CLK>, <GCC_PCIE_1_PHY_RCHNG_CLK_SRC>,
> >> +                       <GCC_PCIE_1_PIPE_CLK>, <GCC_PCIE_1_PIPE_CLK_SRC>,
> >> +                       <GCC_PCIE_1_SLV_AXI_CLK>, <GCC_PCIE_1_SLV_Q2A_AXI_CLK>,
> >> +                       <GCC_QSPI_CNOC_PERIPH_AHB_CLK>, <GCC_QSPI_CORE_CLK>,
> >> +                       <GCC_QSPI_CORE_CLK_SRC>,<GCC_USB30_SEC_MASTER_CLK>,
> >> +                       <GCC_USB30_SEC_MASTER_CLK_SRC>, <GCC_USB30_SEC_MOCK_UTMI_CLK>,
> >> +                       <GCC_USB30_SEC_MOCK_UTMI_CLK_SRC>,
> >> +                       <GCC_USB30_SEC_MOCK_UTMI_POSTDIV_CLK_SRC>, <GCC_USB30_SEC_SLEEP_CLK>,
> >> +                       <GCC_USB3_SEC_PHY_AUX_CLK>, <GCC_USB3_SEC_PHY_AUX_CLK_SRC>,
> >> +                       <GCC_USB3_SEC_PHY_COM_AUX_CLK>, <GCC_USB3_SEC_PHY_PIPE_CLK>,
> >> +                       <GCC_USB3_SEC_PHY_PIPE_CLK_SRC>, <GCC_CFG_NOC_LPASS_CLK>,
> >> +                       <GCC_MSS_GPLL0_MAIN_DIV_CLK_SRC>, <GCC_MSS_CFG_AHB_CLK>,
> >> +                       <GCC_MSS_OFFLINE_AXI_CLK>, <GCC_MSS_SNOC_AXI_CLK>,
> >> +                       <GCC_MSS_Q6_MEMNOC_AXI_CLK>, <GCC_MSS_Q6SS_BOOT_CLK_SRC>,
> >> +                       <GCC_SEC_CTRL_CLK_SRC>, <GCC_WPSS_AHB_CLK>,
> >> +                       <GCC_WPSS_AHB_BDG_MST_CLK>, <GCC_WPSS_RSCP_CLK>;
> >
> > This looks like a huge variety of clocks. Are they really not
> > accessible or are you trying to make Linux stay away from all those
> > clocks to keep bootloader settings?
> >
>
> These clocks are protected and accessing them from Linux will cause Bus
> error (NoC) and thus this list grows sometimes.
>
> >> +};
> >> +
> >> +&lpasscc {
> >> +       status = "disabled";
> >> +};
> >> +
> >> +&lpass_audiocc {
> >> +       qcom,adsp-skip-pll;
> >> +       protected-clocks = <LPASS_AUDIO_CC_CDIV_RX_MCLK_DIV_CLK_SRC>,
> >> +               <LPASS_AUDIO_CC_CODEC_MEM0_CLK>, <LPASS_AUDIO_CC_CODEC_MEM1_CLK>,
> >> +               <LPASS_AUDIO_CC_CODEC_MEM2_CLK>, <LPASS_AUDIO_CC_CODEC_MEM_CLK>,
> >> +               <LPASS_AUDIO_CC_EXT_MCLK0_CLK>, <LPASS_AUDIO_CC_EXT_MCLK0_CLK_SRC>,
> >> +               <LPASS_AUDIO_CC_EXT_MCLK1_CLK>, <LPASS_AUDIO_CC_EXT_MCLK1_CLK_SRC>,
> >> +               <LPASS_AUDIO_CC_PLL>, <LPASS_AUDIO_CC_PLL_OUT_AUX2>,
> >> +               <LPASS_AUDIO_CC_PLL_OUT_AUX2_DIV_CLK_SRC>,
> >> +               <LPASS_AUDIO_CC_PLL_OUT_MAIN_DIV_CLK_SRC>,
> >> +               <LPASS_AUDIO_CC_RX_MCLK_2X_CLK>, <LPASS_AUDIO_CC_RX_MCLK_CLK>,
> >> +               <LPASS_AUDIO_CC_RX_MCLK_CLK_SRC>;
> >
> > This almost looks like a separate compatible.
>
> We need to use the reset from this controller, rest all will be
> controlled via the Low Power Audio Firmware.

Not "separate device", but "separate compatible".

>
> >
> >> +       /delete-property/ power-domains;
> >> +};
> >> +
> >> +&lpass_aon {
> >> +       status = "disabled";
> >
> > Should this be "reserved", controlled by ADSP? See how this was
> > implemented in sc7180.dtsi / sc7180-trogdor.dtsi.
> > Please consider inverting the logic. Generic ADSP implementation
> > should be present in sc7280.dtsi and then the non-default ChromeOS
> > implementation should be a part of sc7280-chrome-common.dtsi.
> >
>
> This I will take care in the next series.
>
> >> +};
> >> +
> >> +&lpass_core {
> >> +       status = "disabled";
> >> +};
> >> +
> >> +&lpass_hm {
> >> +       status = "disabled";
> >> +};
> >> +
> >>   &qupv3_id_0 {
> >>          status = "okay";
> >>   };
> >> diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> >> index 8bb7d13d85f6..1398b84634c3 100644
> >> --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> >> +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> >> @@ -1,6 +1,6 @@
> >>   // SPDX-License-Identifier: BSD-3-Clause
> >>   /*
> >> - * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
> >> + * Copyright (c) 2023-2024 Qualcomm Innovation Center, Inc. All rights reserved.
> >>    */
> >>
> >>   /dts-v1/;
> >> @@ -413,6 +413,54 @@
> >>          };
> >>   };
> >>
> >> +&gcc {
> >> +       protected-clocks = <GCC_QSPI_CNOC_PERIPH_AHB_CLK>, <GCC_QSPI_CORE_CLK>,
> >> +                       <GCC_QSPI_CORE_CLK_SRC>,<GCC_USB30_SEC_MASTER_CLK>,
> >> +                       <GCC_USB30_SEC_MASTER_CLK_SRC>, <GCC_USB30_SEC_MOCK_UTMI_CLK>,
> >> +                       <GCC_USB30_SEC_MOCK_UTMI_CLK_SRC>,
> >> +                       <GCC_USB30_SEC_MOCK_UTMI_POSTDIV_CLK_SRC>, <GCC_USB30_SEC_SLEEP_CLK>,
> >> +                       <GCC_USB3_SEC_PHY_AUX_CLK>, <GCC_USB3_SEC_PHY_AUX_CLK_SRC>,
> >> +                       <GCC_USB3_SEC_PHY_COM_AUX_CLK>, <GCC_USB3_SEC_PHY_PIPE_CLK>,
> >> +                       <GCC_USB3_SEC_PHY_PIPE_CLK_SRC>, <GCC_CFG_NOC_LPASS_CLK>,
> >> +                       <GCC_MSS_GPLL0_MAIN_DIV_CLK_SRC>, <GCC_MSS_CFG_AHB_CLK>,
> >> +                       <GCC_MSS_OFFLINE_AXI_CLK>, <GCC_MSS_SNOC_AXI_CLK>,
> >> +                       <GCC_MSS_Q6_MEMNOC_AXI_CLK>, <GCC_MSS_Q6SS_BOOT_CLK_SRC>,
> >> +                       <GCC_SEC_CTRL_CLK_SRC>, <GCC_WPSS_AHB_CLK>,
> >> +                       <GCC_WPSS_AHB_BDG_MST_CLK>, <GCC_WPSS_RSCP_CLK>;
> >> +};
> >> +
> >> +&lpasscc {
> >> +       status = "disabled";
> >> +};
> >> +
> >> +&lpass_audiocc {
> >> +       qcom,adsp-skip-pll;
> >> +       protected-clocks = <LPASS_AUDIO_CC_CDIV_RX_MCLK_DIV_CLK_SRC>,
> >> +               <LPASS_AUDIO_CC_CODEC_MEM0_CLK>, <LPASS_AUDIO_CC_CODEC_MEM1_CLK>,
> >> +               <LPASS_AUDIO_CC_CODEC_MEM2_CLK>, <LPASS_AUDIO_CC_CODEC_MEM_CLK>,
> >> +               <LPASS_AUDIO_CC_EXT_MCLK0_CLK>, <LPASS_AUDIO_CC_EXT_MCLK0_CLK_SRC>,
> >> +               <LPASS_AUDIO_CC_EXT_MCLK1_CLK>, <LPASS_AUDIO_CC_EXT_MCLK1_CLK_SRC>,
> >> +               <LPASS_AUDIO_CC_PLL>, <LPASS_AUDIO_CC_PLL_OUT_AUX2>,
> >> +               <LPASS_AUDIO_CC_PLL_OUT_AUX2_DIV_CLK_SRC>,
> >> +               <LPASS_AUDIO_CC_PLL_OUT_MAIN_DIV_CLK_SRC>,
> >> +               <LPASS_AUDIO_CC_RX_MCLK_2X_CLK>, <LPASS_AUDIO_CC_RX_MCLK_CLK>,
> >> +               <LPASS_AUDIO_CC_RX_MCLK_CLK_SRC>;
> >> +       /delete-property/ power-domains;
> >> +};
> >> +
> >> +&lpass_aon {
> >> +       status = "disabled";
> >> +};
> >> +
> >> +&lpass_core {
> >> +       status = "disabled";
> >> +};
> >> +
> >> +&lpass_hm {
> >> +       status = "disabled";
> >> +};
> >> +
> >> +
> >>   &qupv3_id_0 {
> >>          status = "okay";
> >>   };
> >> --
> >> 2.17.1
> >>
> >>
> >
> >
>
> --
> Thanks & Regards,
> Taniya Das.



-- 
With best wishes
Dmitry

Re: [PATCH 5/5] arm64: dts: qcom: Update protected clocks list for qcm6490 variants

[Bjorn Andersson]

On Thu, Feb 08, 2024 at 11:58:36AM +0530, Taniya Das wrote:
> Certain clocks are not accessible on QCM6490-IDP and QCS6490-RB3GEN2 boards
> thus require them to be marked protected.
> 
> Also disable the LPASS nodes which are not to be used.
> 

Please find a smaller patch, to make the board boot again here:

https://lore.kernel.org/linux-arm-msm/20240209-qcm6490-gcc-protected-clocks-v1-1-bd3487b2e7b1@quicinc.com/T/#u

> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/qcm6490-idp.dts     | 54 +++++++++++++++++++-
>  arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 50 +++++++++++++++++-
>  2 files changed, 102 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
> index 03e97e27d16d..425e4b87490b 100644
> --- a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
> +++ b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
> @@ -1,6 +1,6 @@
>  // SPDX-License-Identifier: BSD-3-Clause
>  /*
> - * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
> + * Copyright (c) 2023-2024 Qualcomm Innovation Center, Inc. All rights reserved.
>   */
> 
>  /dts-v1/;
> @@ -415,6 +415,58 @@
>  	};
>  };
> 
> +&gcc {
> +	protected-clocks = <GCC_AGGRE_NOC_PCIE_1_AXI_CLK> ,<GCC_PCIE_1_AUX_CLK>,
> +			<GCC_PCIE_1_AUX_CLK_SRC>, <GCC_PCIE_1_CFG_AHB_CLK>,
> +			<GCC_PCIE_1_MSTR_AXI_CLK>, <GCC_PCIE_1_PHY_RCHNG_CLK_SRC>,
> +			<GCC_PCIE_1_PIPE_CLK>, <GCC_PCIE_1_PIPE_CLK_SRC>,
> +			<GCC_PCIE_1_SLV_AXI_CLK>, <GCC_PCIE_1_SLV_Q2A_AXI_CLK>,

protected-clocks should mark the clocks that Linux must not access.

With that in mind, how is listing GCC_PCIE_1_* here compatible with
Krishna's patch [1], which clearly shows that Linux is expected to use
pcie1.

[1] https://lore.kernel.org/linux-arm-msm/20240207-enable_pcie-v1-1-b684afa6371c@quicinc.com/

Regards,
Bjorn

> +			<GCC_QSPI_CNOC_PERIPH_AHB_CLK>, <GCC_QSPI_CORE_CLK>,
> +			<GCC_QSPI_CORE_CLK_SRC>,<GCC_USB30_SEC_MASTER_CLK>,
> +			<GCC_USB30_SEC_MASTER_CLK_SRC>, <GCC_USB30_SEC_MOCK_UTMI_CLK>,
> +			<GCC_USB30_SEC_MOCK_UTMI_CLK_SRC>,
> +			<GCC_USB30_SEC_MOCK_UTMI_POSTDIV_CLK_SRC>, <GCC_USB30_SEC_SLEEP_CLK>,
> +			<GCC_USB3_SEC_PHY_AUX_CLK>, <GCC_USB3_SEC_PHY_AUX_CLK_SRC>,
> +			<GCC_USB3_SEC_PHY_COM_AUX_CLK>, <GCC_USB3_SEC_PHY_PIPE_CLK>,
> +			<GCC_USB3_SEC_PHY_PIPE_CLK_SRC>, <GCC_CFG_NOC_LPASS_CLK>,
> +			<GCC_MSS_GPLL0_MAIN_DIV_CLK_SRC>, <GCC_MSS_CFG_AHB_CLK>,
> +			<GCC_MSS_OFFLINE_AXI_CLK>, <GCC_MSS_SNOC_AXI_CLK>,
> +			<GCC_MSS_Q6_MEMNOC_AXI_CLK>, <GCC_MSS_Q6SS_BOOT_CLK_SRC>,
> +			<GCC_SEC_CTRL_CLK_SRC>, <GCC_WPSS_AHB_CLK>,
> +			<GCC_WPSS_AHB_BDG_MST_CLK>, <GCC_WPSS_RSCP_CLK>;
> +};
> +
> +&lpasscc {
> +	status = "disabled";
> +};
> +
> +&lpass_audiocc {
> +	qcom,adsp-skip-pll;
> +	protected-clocks = <LPASS_AUDIO_CC_CDIV_RX_MCLK_DIV_CLK_SRC>,
> +		<LPASS_AUDIO_CC_CODEC_MEM0_CLK>, <LPASS_AUDIO_CC_CODEC_MEM1_CLK>,
> +		<LPASS_AUDIO_CC_CODEC_MEM2_CLK>, <LPASS_AUDIO_CC_CODEC_MEM_CLK>,
> +		<LPASS_AUDIO_CC_EXT_MCLK0_CLK>, <LPASS_AUDIO_CC_EXT_MCLK0_CLK_SRC>,
> +		<LPASS_AUDIO_CC_EXT_MCLK1_CLK>, <LPASS_AUDIO_CC_EXT_MCLK1_CLK_SRC>,
> +		<LPASS_AUDIO_CC_PLL>, <LPASS_AUDIO_CC_PLL_OUT_AUX2>,
> +		<LPASS_AUDIO_CC_PLL_OUT_AUX2_DIV_CLK_SRC>,
> +		<LPASS_AUDIO_CC_PLL_OUT_MAIN_DIV_CLK_SRC>,
> +		<LPASS_AUDIO_CC_RX_MCLK_2X_CLK>, <LPASS_AUDIO_CC_RX_MCLK_CLK>,
> +		<LPASS_AUDIO_CC_RX_MCLK_CLK_SRC>;
> +	/delete-property/ power-domains;
> +};
> +
> +&lpass_aon {
> +	status = "disabled";
> +};
> +
> +&lpass_core {
> +	status = "disabled";
> +};
> +
> +&lpass_hm {
> +	status = "disabled";
> +};
> +
>  &qupv3_id_0 {
>  	status = "okay";
>  };
> diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> index 8bb7d13d85f6..1398b84634c3 100644
> --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> @@ -1,6 +1,6 @@
>  // SPDX-License-Identifier: BSD-3-Clause
>  /*
> - * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
> + * Copyright (c) 2023-2024 Qualcomm Innovation Center, Inc. All rights reserved.
>   */
> 
>  /dts-v1/;
> @@ -413,6 +413,54 @@
>  	};
>  };
> 
> +&gcc {
> +	protected-clocks = <GCC_QSPI_CNOC_PERIPH_AHB_CLK>, <GCC_QSPI_CORE_CLK>,
> +			<GCC_QSPI_CORE_CLK_SRC>,<GCC_USB30_SEC_MASTER_CLK>,
> +			<GCC_USB30_SEC_MASTER_CLK_SRC>, <GCC_USB30_SEC_MOCK_UTMI_CLK>,
> +			<GCC_USB30_SEC_MOCK_UTMI_CLK_SRC>,
> +			<GCC_USB30_SEC_MOCK_UTMI_POSTDIV_CLK_SRC>, <GCC_USB30_SEC_SLEEP_CLK>,
> +			<GCC_USB3_SEC_PHY_AUX_CLK>, <GCC_USB3_SEC_PHY_AUX_CLK_SRC>,
> +			<GCC_USB3_SEC_PHY_COM_AUX_CLK>, <GCC_USB3_SEC_PHY_PIPE_CLK>,
> +			<GCC_USB3_SEC_PHY_PIPE_CLK_SRC>, <GCC_CFG_NOC_LPASS_CLK>,
> +			<GCC_MSS_GPLL0_MAIN_DIV_CLK_SRC>, <GCC_MSS_CFG_AHB_CLK>,
> +			<GCC_MSS_OFFLINE_AXI_CLK>, <GCC_MSS_SNOC_AXI_CLK>,
> +			<GCC_MSS_Q6_MEMNOC_AXI_CLK>, <GCC_MSS_Q6SS_BOOT_CLK_SRC>,
> +			<GCC_SEC_CTRL_CLK_SRC>, <GCC_WPSS_AHB_CLK>,
> +			<GCC_WPSS_AHB_BDG_MST_CLK>, <GCC_WPSS_RSCP_CLK>;
> +};
> +
> +&lpasscc {
> +	status = "disabled";
> +};
> +
> +&lpass_audiocc {
> +	qcom,adsp-skip-pll;
> +	protected-clocks = <LPASS_AUDIO_CC_CDIV_RX_MCLK_DIV_CLK_SRC>,
> +		<LPASS_AUDIO_CC_CODEC_MEM0_CLK>, <LPASS_AUDIO_CC_CODEC_MEM1_CLK>,
> +		<LPASS_AUDIO_CC_CODEC_MEM2_CLK>, <LPASS_AUDIO_CC_CODEC_MEM_CLK>,
> +		<LPASS_AUDIO_CC_EXT_MCLK0_CLK>, <LPASS_AUDIO_CC_EXT_MCLK0_CLK_SRC>,
> +		<LPASS_AUDIO_CC_EXT_MCLK1_CLK>, <LPASS_AUDIO_CC_EXT_MCLK1_CLK_SRC>,
> +		<LPASS_AUDIO_CC_PLL>, <LPASS_AUDIO_CC_PLL_OUT_AUX2>,
> +		<LPASS_AUDIO_CC_PLL_OUT_AUX2_DIV_CLK_SRC>,
> +		<LPASS_AUDIO_CC_PLL_OUT_MAIN_DIV_CLK_SRC>,
> +		<LPASS_AUDIO_CC_RX_MCLK_2X_CLK>, <LPASS_AUDIO_CC_RX_MCLK_CLK>,
> +		<LPASS_AUDIO_CC_RX_MCLK_CLK_SRC>;
> +	/delete-property/ power-domains;
> +};
> +
> +&lpass_aon {
> +	status = "disabled";
> +};
> +
> +&lpass_core {
> +	status = "disabled";
> +};
> +
> +&lpass_hm {
> +	status = "disabled";
> +};
> +
> +
>  &qupv3_id_0 {
>  	status = "okay";
>  };
> --
> 2.17.1
> 

Re: [PATCH 5/5] arm64: dts: qcom: Update protected clocks list for qcm6490 variants

[Taniya Das]

Thanks Bjorn for your review.

On 2/9/2024 10:07 PM, Bjorn Andersson wrote:
> On Thu, Feb 08, 2024 at 11:58:36AM +0530, Taniya Das wrote:
>> Certain clocks are not accessible on QCM6490-IDP and QCS6490-RB3GEN2 boards
>> thus require them to be marked protected.
>>
>> Also disable the LPASS nodes which are not to be used.
>>
> 
> Please find a smaller patch, to make the board boot again here:
> 
> https://lore.kernel.org/linux-arm-msm/20240209-qcm6490-gcc-protected-clocks-v1-1-bd3487b2e7b1@quicinc.com/T/#u
> 
>> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
>> ---
>>   arch/arm64/boot/dts/qcom/qcm6490-idp.dts     | 54 +++++++++++++++++++-
>>   arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 50 +++++++++++++++++-
>>   2 files changed, 102 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
>> index 03e97e27d16d..425e4b87490b 100644
>> --- a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
>> +++ b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
>> @@ -1,6 +1,6 @@
>>   // SPDX-License-Identifier: BSD-3-Clause
>>   /*
>> - * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
>> + * Copyright (c) 2023-2024 Qualcomm Innovation Center, Inc. All rights reserved.
>>    */
>>
>>   /dts-v1/;
>> @@ -415,6 +415,58 @@
>>   	};
>>   };
>>
>> +&gcc {
>> +	protected-clocks = <GCC_AGGRE_NOC_PCIE_1_AXI_CLK> ,<GCC_PCIE_1_AUX_CLK>,
>> +			<GCC_PCIE_1_AUX_CLK_SRC>, <GCC_PCIE_1_CFG_AHB_CLK>,
>> +			<GCC_PCIE_1_MSTR_AXI_CLK>, <GCC_PCIE_1_PHY_RCHNG_CLK_SRC>,
>> +			<GCC_PCIE_1_PIPE_CLK>, <GCC_PCIE_1_PIPE_CLK_SRC>,
>> +			<GCC_PCIE_1_SLV_AXI_CLK>, <GCC_PCIE_1_SLV_Q2A_AXI_CLK>,
> 
> protected-clocks should mark the clocks that Linux must not access.
> 
> With that in mind, how is listing GCC_PCIE_1_* here compatible with
> Krishna's patch [1], which clearly shows that Linux is expected to use
> pcie1.
> 
> [1] https://lore.kernel.org/linux-arm-msm/20240207-enable_pcie-v1-1-b684afa6371c@quicinc.com/
> 
> Regards,
> Bjorn

I checked the patch and it is only expected to be used on RB3 boards and 
not on IDP platforms.

> 
>> +			<GCC_QSPI_CNOC_PERIPH_AHB_CLK>, <GCC_QSPI_CORE_CLK>,
>> +			<GCC_QSPI_CORE_CLK_SRC>,<GCC_USB30_SEC_MASTER_CLK>,
>> +			<GCC_USB30_SEC_MASTER_CLK_SRC>, <GCC_USB30_SEC_MOCK_UTMI_CLK>,
>> +			<GCC_USB30_SEC_MOCK_UTMI_CLK_SRC>,
>> +			<GCC_USB30_SEC_MOCK_UTMI_POSTDIV_CLK_SRC>, <GCC_USB30_SEC_SLEEP_CLK>,
>> +			<GCC_USB3_SEC_PHY_AUX_CLK>, <GCC_USB3_SEC_PHY_AUX_CLK_SRC>,
>> +			<GCC_USB3_SEC_PHY_COM_AUX_CLK>, <GCC_USB3_SEC_PHY_PIPE_CLK>,
>> +			<GCC_USB3_SEC_PHY_PIPE_CLK_SRC>, <GCC_CFG_NOC_LPASS_CLK>,
>> +			<GCC_MSS_GPLL0_MAIN_DIV_CLK_SRC>, <GCC_MSS_CFG_AHB_CLK>,
>> +			<GCC_MSS_OFFLINE_AXI_CLK>, <GCC_MSS_SNOC_AXI_CLK>,
>> +			<GCC_MSS_Q6_MEMNOC_AXI_CLK>, <GCC_MSS_Q6SS_BOOT_CLK_SRC>,
>> +			<GCC_SEC_CTRL_CLK_SRC>, <GCC_WPSS_AHB_CLK>,
>> +			<GCC_WPSS_AHB_BDG_MST_CLK>, <GCC_WPSS_RSCP_CLK>;
>> +};
>> +
>> +&lpasscc {
>> +	status = "disabled";
>> +};
>> +
>> +&lpass_audiocc {
>> +	qcom,adsp-skip-pll;
>> +	protected-clocks = <LPASS_AUDIO_CC_CDIV_RX_MCLK_DIV_CLK_SRC>,
>> +		<LPASS_AUDIO_CC_CODEC_MEM0_CLK>, <LPASS_AUDIO_CC_CODEC_MEM1_CLK>,
>> +		<LPASS_AUDIO_CC_CODEC_MEM2_CLK>, <LPASS_AUDIO_CC_CODEC_MEM_CLK>,
>> +		<LPASS_AUDIO_CC_EXT_MCLK0_CLK>, <LPASS_AUDIO_CC_EXT_MCLK0_CLK_SRC>,
>> +		<LPASS_AUDIO_CC_EXT_MCLK1_CLK>, <LPASS_AUDIO_CC_EXT_MCLK1_CLK_SRC>,
>> +		<LPASS_AUDIO_CC_PLL>, <LPASS_AUDIO_CC_PLL_OUT_AUX2>,
>> +		<LPASS_AUDIO_CC_PLL_OUT_AUX2_DIV_CLK_SRC>,
>> +		<LPASS_AUDIO_CC_PLL_OUT_MAIN_DIV_CLK_SRC>,
>> +		<LPASS_AUDIO_CC_RX_MCLK_2X_CLK>, <LPASS_AUDIO_CC_RX_MCLK_CLK>,
>> +		<LPASS_AUDIO_CC_RX_MCLK_CLK_SRC>;
>> +	/delete-property/ power-domains;
>> +};
>> +
>> +&lpass_aon {
>> +	status = "disabled";
>> +};
>> +
>> +&lpass_core {
>> +	status = "disabled";
>> +};
>> +
>> +&lpass_hm {
>> +	status = "disabled";
>> +};
>> +
>>   &qupv3_id_0 {
>>   	status = "okay";
>>   };
>> diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
>> index 8bb7d13d85f6..1398b84634c3 100644
>> --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
>> +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
>> @@ -1,6 +1,6 @@
>>   // SPDX-License-Identifier: BSD-3-Clause
>>   /*
>> - * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
>> + * Copyright (c) 2023-2024 Qualcomm Innovation Center, Inc. All rights reserved.
>>    */
>>
>>   /dts-v1/;
>> @@ -413,6 +413,54 @@
>>   	};
>>   };
>>
>> +&gcc {
>> +	protected-clocks = <GCC_QSPI_CNOC_PERIPH_AHB_CLK>, <GCC_QSPI_CORE_CLK>,
>> +			<GCC_QSPI_CORE_CLK_SRC>,<GCC_USB30_SEC_MASTER_CLK>,
>> +			<GCC_USB30_SEC_MASTER_CLK_SRC>, <GCC_USB30_SEC_MOCK_UTMI_CLK>,
>> +			<GCC_USB30_SEC_MOCK_UTMI_CLK_SRC>,
>> +			<GCC_USB30_SEC_MOCK_UTMI_POSTDIV_CLK_SRC>, <GCC_USB30_SEC_SLEEP_CLK>,
>> +			<GCC_USB3_SEC_PHY_AUX_CLK>, <GCC_USB3_SEC_PHY_AUX_CLK_SRC>,
>> +			<GCC_USB3_SEC_PHY_COM_AUX_CLK>, <GCC_USB3_SEC_PHY_PIPE_CLK>,
>> +			<GCC_USB3_SEC_PHY_PIPE_CLK_SRC>, <GCC_CFG_NOC_LPASS_CLK>,
>> +			<GCC_MSS_GPLL0_MAIN_DIV_CLK_SRC>, <GCC_MSS_CFG_AHB_CLK>,
>> +			<GCC_MSS_OFFLINE_AXI_CLK>, <GCC_MSS_SNOC_AXI_CLK>,
>> +			<GCC_MSS_Q6_MEMNOC_AXI_CLK>, <GCC_MSS_Q6SS_BOOT_CLK_SRC>,
>> +			<GCC_SEC_CTRL_CLK_SRC>, <GCC_WPSS_AHB_CLK>,
>> +			<GCC_WPSS_AHB_BDG_MST_CLK>, <GCC_WPSS_RSCP_CLK>;
>> +};
>> +
>> +&lpasscc {
>> +	status = "disabled";
>> +};
>> +
>> +&lpass_audiocc {
>> +	qcom,adsp-skip-pll;
>> +	protected-clocks = <LPASS_AUDIO_CC_CDIV_RX_MCLK_DIV_CLK_SRC>,
>> +		<LPASS_AUDIO_CC_CODEC_MEM0_CLK>, <LPASS_AUDIO_CC_CODEC_MEM1_CLK>,
>> +		<LPASS_AUDIO_CC_CODEC_MEM2_CLK>, <LPASS_AUDIO_CC_CODEC_MEM_CLK>,
>> +		<LPASS_AUDIO_CC_EXT_MCLK0_CLK>, <LPASS_AUDIO_CC_EXT_MCLK0_CLK_SRC>,
>> +		<LPASS_AUDIO_CC_EXT_MCLK1_CLK>, <LPASS_AUDIO_CC_EXT_MCLK1_CLK_SRC>,
>> +		<LPASS_AUDIO_CC_PLL>, <LPASS_AUDIO_CC_PLL_OUT_AUX2>,
>> +		<LPASS_AUDIO_CC_PLL_OUT_AUX2_DIV_CLK_SRC>,
>> +		<LPASS_AUDIO_CC_PLL_OUT_MAIN_DIV_CLK_SRC>,
>> +		<LPASS_AUDIO_CC_RX_MCLK_2X_CLK>, <LPASS_AUDIO_CC_RX_MCLK_CLK>,
>> +		<LPASS_AUDIO_CC_RX_MCLK_CLK_SRC>;
>> +	/delete-property/ power-domains;
>> +};
>> +
>> +&lpass_aon {
>> +	status = "disabled";
>> +};
>> +
>> +&lpass_core {
>> +	status = "disabled";
>> +};
>> +
>> +&lpass_hm {
>> +	status = "disabled";
>> +};
>> +
>> +
>>   &qupv3_id_0 {
>>   	status = "okay";
>>   };
>> --
>> 2.17.1
>>

-- 
Thanks & Regards,
Taniya Das.

Re: [PATCH 5/5] arm64: dts: qcom: Update protected clocks list for qcm6490 variants

[Konrad Dybcio]

On 8.02.2024 07:28, Taniya Das wrote:
> Certain clocks are not accessible on QCM6490-IDP and QCS6490-RB3GEN2 boards
> thus require them to be marked protected.
> 
> Also disable the LPASS nodes which are not to be used.
> 
> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
> ---

One patch per board, please

>  arch/arm64/boot/dts/qcom/qcm6490-idp.dts     | 54 +++++++++++++++++++-
>  arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 50 +++++++++++++++++-
>  2 files changed, 102 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
> index 03e97e27d16d..425e4b87490b 100644
> --- a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
> +++ b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
> @@ -1,6 +1,6 @@
>  // SPDX-License-Identifier: BSD-3-Clause
>  /*
> - * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
> + * Copyright (c) 2023-2024 Qualcomm Innovation Center, Inc. All rights reserved.
>   */
> 
>  /dts-v1/;
> @@ -415,6 +415,58 @@
>  	};
>  };
> 
> +&gcc {
> +	protected-clocks = <GCC_AGGRE_NOC_PCIE_1_AXI_CLK> ,<GCC_PCIE_1_AUX_CLK>,
> +			<GCC_PCIE_1_AUX_CLK_SRC>, <GCC_PCIE_1_CFG_AHB_CLK>,
> +			<GCC_PCIE_1_MSTR_AXI_CLK>, <GCC_PCIE_1_PHY_RCHNG_CLK_SRC>,
> +			<GCC_PCIE_1_PIPE_CLK>, <GCC_PCIE_1_PIPE_CLK_SRC>,
> +			<GCC_PCIE_1_SLV_AXI_CLK>, <GCC_PCIE_1_SLV_Q2A_AXI_CLK>,
> +			<GCC_QSPI_CNOC_PERIPH_AHB_CLK>, <GCC_QSPI_CORE_CLK>,
> +			<GCC_QSPI_CORE_CLK_SRC>,<GCC_USB30_SEC_MASTER_CLK>,
> +			<GCC_USB30_SEC_MASTER_CLK_SRC>, <GCC_USB30_SEC_MOCK_UTMI_CLK>,
> +			<GCC_USB30_SEC_MOCK_UTMI_CLK_SRC>,
> +			<GCC_USB30_SEC_MOCK_UTMI_POSTDIV_CLK_SRC>, <GCC_USB30_SEC_SLEEP_CLK>,
> +			<GCC_USB3_SEC_PHY_AUX_CLK>, <GCC_USB3_SEC_PHY_AUX_CLK_SRC>,
> +			<GCC_USB3_SEC_PHY_COM_AUX_CLK>, <GCC_USB3_SEC_PHY_PIPE_CLK>,
> +			<GCC_USB3_SEC_PHY_PIPE_CLK_SRC>, <GCC_CFG_NOC_LPASS_CLK>,
> +			<GCC_MSS_GPLL0_MAIN_DIV_CLK_SRC>, <GCC_MSS_CFG_AHB_CLK>,
> +			<GCC_MSS_OFFLINE_AXI_CLK>, <GCC_MSS_SNOC_AXI_CLK>,
> +			<GCC_MSS_Q6_MEMNOC_AXI_CLK>, <GCC_MSS_Q6SS_BOOT_CLK_SRC>,
> +			<GCC_SEC_CTRL_CLK_SRC>, <GCC_WPSS_AHB_CLK>,
> +			<GCC_WPSS_AHB_BDG_MST_CLK>, <GCC_WPSS_RSCP_CLK>;

Why on earth are clocks such as USB sleep reserved? How does it work?

> +};
> +
> +&lpasscc {
> +	status = "disabled";
> +};
> +
> +&lpass_audiocc {
> +	qcom,adsp-skip-pll;
> +	protected-clocks = <LPASS_AUDIO_CC_CDIV_RX_MCLK_DIV_CLK_SRC>,
> +		<LPASS_AUDIO_CC_CODEC_MEM0_CLK>, <LPASS_AUDIO_CC_CODEC_MEM1_CLK>,
> +		<LPASS_AUDIO_CC_CODEC_MEM2_CLK>, <LPASS_AUDIO_CC_CODEC_MEM_CLK>,
> +		<LPASS_AUDIO_CC_EXT_MCLK0_CLK>, <LPASS_AUDIO_CC_EXT_MCLK0_CLK_SRC>,
> +		<LPASS_AUDIO_CC_EXT_MCLK1_CLK>, <LPASS_AUDIO_CC_EXT_MCLK1_CLK_SRC>,
> +		<LPASS_AUDIO_CC_PLL>, <LPASS_AUDIO_CC_PLL_OUT_AUX2>,
> +		<LPASS_AUDIO_CC_PLL_OUT_AUX2_DIV_CLK_SRC>,
> +		<LPASS_AUDIO_CC_PLL_OUT_MAIN_DIV_CLK_SRC>,
> +		<LPASS_AUDIO_CC_RX_MCLK_2X_CLK>, <LPASS_AUDIO_CC_RX_MCLK_CLK>,
> +		<LPASS_AUDIO_CC_RX_MCLK_CLK_SRC>;
> +	/delete-property/ power-domains;
> +};

This literally disables all clocks that come out of this controller, why
not just set status = "reserved"?

> +
> +&lpass_aon {
> +	status = "disabled";
> +};
> +
> +&lpass_core {
> +	status = "disabled";
> +};
> +
> +&lpass_hm {
> +	status = "disabled";
> +};

These three are status = "reserved" by default already, so it's a NOP..

Konrad

Re: [PATCH 5/5] arm64: dts: qcom: Update protected clocks list for qcm6490 variants

[Taniya Das]

Thanks for the review.

On 2/9/2024 10:16 PM, Konrad Dybcio wrote:
> On 8.02.2024 07:28, Taniya Das wrote:
>> Certain clocks are not accessible on QCM6490-IDP and QCS6490-RB3GEN2 boards
>> thus require them to be marked protected.
>>
>> Also disable the LPASS nodes which are not to be used.
>>
>> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
>> ---
> 
> One patch per board, please

Sure, I will update the same in the next patch series.

> 
>>   arch/arm64/boot/dts/qcom/qcm6490-idp.dts     | 54 +++++++++++++++++++-
>>   arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 50 +++++++++++++++++-
>>   2 files changed, 102 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
>> index 03e97e27d16d..425e4b87490b 100644
>> --- a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
>> +++ b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
>> @@ -1,6 +1,6 @@
>>   // SPDX-License-Identifier: BSD-3-Clause
>>   /*
>> - * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
>> + * Copyright (c) 2023-2024 Qualcomm Innovation Center, Inc. All rights reserved.
>>    */
>>
>>   /dts-v1/;
>> @@ -415,6 +415,58 @@
>>   	};
>>   };
>>
>> +&gcc {
>> +	protected-clocks = <GCC_AGGRE_NOC_PCIE_1_AXI_CLK> ,<GCC_PCIE_1_AUX_CLK>,
>> +			<GCC_PCIE_1_AUX_CLK_SRC>, <GCC_PCIE_1_CFG_AHB_CLK>,
>> +			<GCC_PCIE_1_MSTR_AXI_CLK>, <GCC_PCIE_1_PHY_RCHNG_CLK_SRC>,
>> +			<GCC_PCIE_1_PIPE_CLK>, <GCC_PCIE_1_PIPE_CLK_SRC>,
>> +			<GCC_PCIE_1_SLV_AXI_CLK>, <GCC_PCIE_1_SLV_Q2A_AXI_CLK>,
>> +			<GCC_QSPI_CNOC_PERIPH_AHB_CLK>, <GCC_QSPI_CORE_CLK>,
>> +			<GCC_QSPI_CORE_CLK_SRC>,<GCC_USB30_SEC_MASTER_CLK>,
>> +			<GCC_USB30_SEC_MASTER_CLK_SRC>, <GCC_USB30_SEC_MOCK_UTMI_CLK>,
>> +			<GCC_USB30_SEC_MOCK_UTMI_CLK_SRC>,
>> +			<GCC_USB30_SEC_MOCK_UTMI_POSTDIV_CLK_SRC>, <GCC_USB30_SEC_SLEEP_CLK>,
>> +			<GCC_USB3_SEC_PHY_AUX_CLK>, <GCC_USB3_SEC_PHY_AUX_CLK_SRC>,
>> +			<GCC_USB3_SEC_PHY_COM_AUX_CLK>, <GCC_USB3_SEC_PHY_PIPE_CLK>,
>> +			<GCC_USB3_SEC_PHY_PIPE_CLK_SRC>, <GCC_CFG_NOC_LPASS_CLK>,
>> +			<GCC_MSS_GPLL0_MAIN_DIV_CLK_SRC>, <GCC_MSS_CFG_AHB_CLK>,
>> +			<GCC_MSS_OFFLINE_AXI_CLK>, <GCC_MSS_SNOC_AXI_CLK>,
>> +			<GCC_MSS_Q6_MEMNOC_AXI_CLK>, <GCC_MSS_Q6SS_BOOT_CLK_SRC>,
>> +			<GCC_SEC_CTRL_CLK_SRC>, <GCC_WPSS_AHB_CLK>,
>> +			<GCC_WPSS_AHB_BDG_MST_CLK>, <GCC_WPSS_RSCP_CLK>;
> 
> Why on earth are clocks such as USB sleep reserved? How does it work?

On IDP platform these clocks are to be kept protected and no exposure of 
the USB30/PCIE clocks. That was the reason to keep them in the list.

> 
>> +};
>> +
>> +&lpasscc {
>> +	status = "disabled";
>> +};
>> +
>> +&lpass_audiocc {
>> +	qcom,adsp-skip-pll;
>> +	protected-clocks = <LPASS_AUDIO_CC_CDIV_RX_MCLK_DIV_CLK_SRC>,
>> +		<LPASS_AUDIO_CC_CODEC_MEM0_CLK>, <LPASS_AUDIO_CC_CODEC_MEM1_CLK>,
>> +		<LPASS_AUDIO_CC_CODEC_MEM2_CLK>, <LPASS_AUDIO_CC_CODEC_MEM_CLK>,
>> +		<LPASS_AUDIO_CC_EXT_MCLK0_CLK>, <LPASS_AUDIO_CC_EXT_MCLK0_CLK_SRC>,
>> +		<LPASS_AUDIO_CC_EXT_MCLK1_CLK>, <LPASS_AUDIO_CC_EXT_MCLK1_CLK_SRC>,
>> +		<LPASS_AUDIO_CC_PLL>, <LPASS_AUDIO_CC_PLL_OUT_AUX2>,
>> +		<LPASS_AUDIO_CC_PLL_OUT_AUX2_DIV_CLK_SRC>,
>> +		<LPASS_AUDIO_CC_PLL_OUT_MAIN_DIV_CLK_SRC>,
>> +		<LPASS_AUDIO_CC_RX_MCLK_2X_CLK>, <LPASS_AUDIO_CC_RX_MCLK_CLK>,
>> +		<LPASS_AUDIO_CC_RX_MCLK_CLK_SRC>;
>> +	/delete-property/ power-domains;
>> +};
> 
> This literally disables all clocks that come out of this controller, why
> not just set status = "reserved"?
> 

We need the resets from the clock controller to be exposed for the audio 
SW driver to be able to perform resets and the rest of the clocks would 
be controlled from the Low Power Audio Firmware.

>> +
>> +&lpass_aon {
>> +	status = "disabled";
>> +};
>> +
>> +&lpass_core {
>> +	status = "disabled";
>> +};
>> +
>> +&lpass_hm {
>> +	status = "disabled";
>> +};
> 
> These three are status = "reserved" by default already, so it's a NOP..
> 

Yes, I see the patch to keep them reserved, I will remove them in the 
next series.

> Konrad

-- 
Thanks & Regards,
Taniya Das.