[PATCH v3 0/4] Update LPASS Audio clock driver for QCM6490 board

[PATCH v3 0/4] Update LPASS Audio clock driver for QCM6490 board

[Taniya Das]

This series updates the low pass audio clock controller driver for reset
functionality. The patches are split from the below series.
https://lore.kernel.org/all/20240318053555.20405-1-quic_tdas@quicinc.com/

The QCM6490 board requires only the reset functionality from the LPASS
subsystem. Thus separate out the driver probe to provide the same on the
QCM6490 boards.

[v3]:
Changes in [v3] compared to [v2]:
- update to Documentation bindings adding constraints. [Krzysztof]
- split the DT patch for "Update protected clocks list" for QCM6490 IDP
https://lore.kernel.org/linux-devicetree/20250206-protected_clock_qcm6490-v1-1-5923e8c47ab5@quicinc.com/

[v2]:
Changes in [v2] compared to [v1]:
 - Updated the lpass_audio_cc_sc7280 probe to get the match_data for both SC7280 and QCM6490.
 - Separate regmap for resets [Konrad]
 - Split the lpassaudiocc compatible and GCC protected clocks list changes. [Dmitry]
 - Link to V1: https://lore.kernel.org/all/20240531102252.26061-1-quic_tdas@quicinc.com/T/

[v1]
  - Add a separate platform driver for QCM6490 resets.
  - Add device tree changes for protected clocks for GCC and LPASS AudioCC
    compatible update.

Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
---
Taniya Das (4):
      dt-bindings: clock: qcom: Add compatible for QCM6490 boards
      clk: qcom: lpassaudiocc-sc7280: Add support for LPASS resets for QCM6490
      arm64: dts: qcom: qcm6490-idp: Update the LPASS audio node
      arm64: dts: qcom: qcs6490-rb3gen2: Update the LPASS audio node

 .../bindings/clock/qcom,sc7280-lpasscorecc.yaml    | 14 +++++++++++++
 arch/arm64/boot/dts/qcom/qcm6490-idp.dts           |  5 +++++
 arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts       |  5 +++++
 drivers/clk/qcom/lpassaudiocc-sc7280.c             | 23 ++++++++++++++++++----
 4 files changed, 43 insertions(+), 4 deletions(-)
---
base-commit: 808eb958781e4ebb6e9c0962af2e856767e20f45
change-id: 20250206-lpass_qcm6490_resets-204f86622a5c

Best regards,
-- 
Taniya Das <quic_tdas@quicinc.com>

[PATCH v3 1/4] dt-bindings: clock: qcom: Add compatible for QCM6490 boards

[Taniya Das]

Add the new QCM6490 compatible to support the reset functionality for
Low Power Audio subsystem.

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

diff --git a/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml b/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml
index 488d63959424088ede67835eb4dcf3feef6d0848..3217cd3c3b602e62b5cf2270fb351a6988d025aa 100644
--- a/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml
@@ -20,6 +20,7 @@ description: |
 properties:
   compatible:
     enum:
+      - qcom,qcm6490-lpassaudiocc
       - qcom,sc7280-lpassaoncc
       - qcom,sc7280-lpassaudiocc
       - qcom,sc7280-lpasscorecc
@@ -130,6 +131,19 @@ allOf:
         reg:
           maxItems: 1
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: qcom,qcm6490-lpassaudiocc
+
+    then:
+      properties:
+        reg:
+          items:
+            - description: lpass core cc register
+            - description: lpass audio csr register
+
 additionalProperties: false
 
 examples:

-- 
2.45.2

Re: [PATCH v3 1/4] dt-bindings: clock: qcom: Add compatible for QCM6490 boards

[Krzysztof Kozlowski]

On Wed, Feb 12, 2025 at 01:52:19PM +0530, Taniya Das wrote:
> @@ -130,6 +131,19 @@ allOf:
>          reg:
>            maxItems: 1
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: qcom,qcm6490-lpassaudiocc
> +
> +    then:

You need to constrain everything, also clocks. See all other examples.

Best regards,
Krzysztof

Re: [PATCH v3 1/4] dt-bindings: clock: qcom: Add compatible for QCM6490 boards

[Taniya Das]

On 2/13/2025 1:45 PM, Krzysztof Kozlowski wrote:
> On Wed, Feb 12, 2025 at 01:52:19PM +0530, Taniya Das wrote:
>> @@ -130,6 +131,19 @@ allOf:
>>          reg:
>>            maxItems: 1
>>  
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: qcom,qcm6490-lpassaudiocc
>> +
>> +    then:
> 
> You need to constrain everything, also clocks. See all other examples.
> 

Sure, I will take care to update in the next patch.

> Best regards,
> Krzysztof
> 

[PATCH v3 2/4] clk: qcom: lpassaudiocc-sc7280: Add support for LPASS resets for QCM6490

[Taniya Das]

On the QCM6490 boards the LPASS firmware controls the complete clock
controller functionalities. But the LPASS resets are required to be
controlled from the high level OS. The Audio SW driver should be able to
assert/deassert the audio resets as required. Thus in clock driver add
support for the resets.

Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
---
 drivers/clk/qcom/lpassaudiocc-sc7280.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/qcom/lpassaudiocc-sc7280.c b/drivers/clk/qcom/lpassaudiocc-sc7280.c
index 45e7264770866f929a3f4663c477330f0bf7aa84..b6439308926371891cc5f9a5e0d4e8393641560d 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>
@@ -713,14 +714,24 @@ static const struct qcom_reset_map lpass_audio_cc_sc7280_resets[] = {
 	[LPASS_AUDIO_SWR_WSA_CGCR] = { 0xb0, 1 },
 };
 
+static const struct regmap_config lpass_audio_cc_sc7280_reset_regmap_config = {
+	.name = "lpassaudio_cc_reset",
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.val_bits = 32,
+	.fast_io = true,
+	.max_register = 0xc8,
+};
+
 static const struct qcom_cc_desc lpass_audio_cc_reset_sc7280_desc = {
-	.config = &lpass_audio_cc_sc7280_regmap_config,
+	.config = &lpass_audio_cc_sc7280_reset_regmap_config,
 	.resets = lpass_audio_cc_sc7280_resets,
 	.num_resets = ARRAY_SIZE(lpass_audio_cc_sc7280_resets),
 };
 
 static const struct of_device_id lpass_audio_cc_sc7280_match_table[] = {
-	{ .compatible = "qcom,sc7280-lpassaudiocc" },
+	{ .compatible = "qcom,qcm6490-lpassaudiocc", .data = &lpass_audio_cc_reset_sc7280_desc },
+	{ .compatible = "qcom,sc7280-lpassaudiocc", .data = &lpass_audio_cc_sc7280_desc },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, lpass_audio_cc_sc7280_match_table);
@@ -752,13 +763,17 @@ static int lpass_audio_cc_sc7280_probe(struct platform_device *pdev)
 	struct regmap *regmap;
 	int ret;
 
+	desc = device_get_match_data(&pdev->dev);
+
+	if (desc->num_resets)
+		return qcom_cc_probe_by_index(pdev, 1, desc);
+
 	ret = lpass_audio_setup_runtime_pm(pdev);
 	if (ret)
 		return ret;
 
 	lpass_audio_cc_sc7280_regmap_config.name = "lpassaudio_cc";
 	lpass_audio_cc_sc7280_regmap_config.max_register = 0x2f000;
-	desc = &lpass_audio_cc_sc7280_desc;
 
 	regmap = qcom_cc_map(pdev, desc);
 	if (IS_ERR(regmap)) {
@@ -772,7 +787,7 @@ static int lpass_audio_cc_sc7280_probe(struct platform_device *pdev)
 	regmap_write(regmap, 0x4, 0x3b);
 	regmap_write(regmap, 0x8, 0xff05);
 
-	ret = qcom_cc_really_probe(&pdev->dev, &lpass_audio_cc_sc7280_desc, regmap);
+	ret = qcom_cc_really_probe(&pdev->dev, desc, regmap);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to register LPASS AUDIO CC clocks\n");
 		goto exit;

-- 
2.45.2

Re: [PATCH v3 2/4] clk: qcom: lpassaudiocc-sc7280: Add support for LPASS resets for QCM6490

[Dmitry Baryshkov]

On Wed, Feb 12, 2025 at 01:52:20PM +0530, Taniya Das wrote:
> On the QCM6490 boards the LPASS firmware controls the complete clock
> controller functionalities. But the LPASS resets are required to be
> controlled from the high level OS. The Audio SW driver should be able to
> assert/deassert the audio resets as required. Thus in clock driver add
> support for the resets.
> 
> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
> ---
>  drivers/clk/qcom/lpassaudiocc-sc7280.c | 23 +++++++++++++++++++----
>  1 file changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/clk/qcom/lpassaudiocc-sc7280.c b/drivers/clk/qcom/lpassaudiocc-sc7280.c
> index 45e7264770866f929a3f4663c477330f0bf7aa84..b6439308926371891cc5f9a5e0d4e8393641560d 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>
> @@ -713,14 +714,24 @@ static const struct qcom_reset_map lpass_audio_cc_sc7280_resets[] = {
>  	[LPASS_AUDIO_SWR_WSA_CGCR] = { 0xb0, 1 },
>  };
>  
> +static const struct regmap_config lpass_audio_cc_sc7280_reset_regmap_config = {
> +	.name = "lpassaudio_cc_reset",
> +	.reg_bits = 32,
> +	.reg_stride = 4,
> +	.val_bits = 32,
> +	.fast_io = true,
> +	.max_register = 0xc8,
> +};
> +
>  static const struct qcom_cc_desc lpass_audio_cc_reset_sc7280_desc = {
> -	.config = &lpass_audio_cc_sc7280_regmap_config,
> +	.config = &lpass_audio_cc_sc7280_reset_regmap_config,
>  	.resets = lpass_audio_cc_sc7280_resets,
>  	.num_resets = ARRAY_SIZE(lpass_audio_cc_sc7280_resets),
>  };
>  
>  static const struct of_device_id lpass_audio_cc_sc7280_match_table[] = {
> -	{ .compatible = "qcom,sc7280-lpassaudiocc" },
> +	{ .compatible = "qcom,qcm6490-lpassaudiocc", .data = &lpass_audio_cc_reset_sc7280_desc },
> +	{ .compatible = "qcom,sc7280-lpassaudiocc", .data = &lpass_audio_cc_sc7280_desc },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(of, lpass_audio_cc_sc7280_match_table);
> @@ -752,13 +763,17 @@ static int lpass_audio_cc_sc7280_probe(struct platform_device *pdev)
>  	struct regmap *regmap;
>  	int ret;
>  
> +	desc = device_get_match_data(&pdev->dev);
> +
> +	if (desc->num_resets)
> +		return qcom_cc_probe_by_index(pdev, 1, desc);

Won't this break SC7280 support by causing an early return?

> +
>  	ret = lpass_audio_setup_runtime_pm(pdev);
>  	if (ret)
>  		return ret;
>  
>  	lpass_audio_cc_sc7280_regmap_config.name = "lpassaudio_cc";
>  	lpass_audio_cc_sc7280_regmap_config.max_register = 0x2f000;
> -	desc = &lpass_audio_cc_sc7280_desc;
>  
>  	regmap = qcom_cc_map(pdev, desc);
>  	if (IS_ERR(regmap)) {
> @@ -772,7 +787,7 @@ static int lpass_audio_cc_sc7280_probe(struct platform_device *pdev)
>  	regmap_write(regmap, 0x4, 0x3b);
>  	regmap_write(regmap, 0x8, 0xff05);
>  
> -	ret = qcom_cc_really_probe(&pdev->dev, &lpass_audio_cc_sc7280_desc, regmap);
> +	ret = qcom_cc_really_probe(&pdev->dev, desc, regmap);
>  	if (ret) {
>  		dev_err(&pdev->dev, "Failed to register LPASS AUDIO CC clocks\n");
>  		goto exit;
> 
> -- 
> 2.45.2
> 

-- 
With best wishes
Dmitry

Re: [PATCH v3 2/4] clk: qcom: lpassaudiocc-sc7280: Add support for LPASS resets for QCM6490

[Taniya Das]

On 2/12/2025 4:39 PM, Dmitry Baryshkov wrote:
> On Wed, Feb 12, 2025 at 01:52:20PM +0530, Taniya Das wrote:
>> On the QCM6490 boards the LPASS firmware controls the complete clock
>> controller functionalities. But the LPASS resets are required to be
>> controlled from the high level OS. The Audio SW driver should be able to
>> assert/deassert the audio resets as required. Thus in clock driver add
>> support for the resets.
>>
>> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
>> ---
>>  drivers/clk/qcom/lpassaudiocc-sc7280.c | 23 +++++++++++++++++++----
>>  1 file changed, 19 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/clk/qcom/lpassaudiocc-sc7280.c b/drivers/clk/qcom/lpassaudiocc-sc7280.c
>> index 45e7264770866f929a3f4663c477330f0bf7aa84..b6439308926371891cc5f9a5e0d4e8393641560d 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>
>> @@ -713,14 +714,24 @@ static const struct qcom_reset_map lpass_audio_cc_sc7280_resets[] = {
>>  	[LPASS_AUDIO_SWR_WSA_CGCR] = { 0xb0, 1 },
>>  };
>>  
>> +static const struct regmap_config lpass_audio_cc_sc7280_reset_regmap_config = {
>> +	.name = "lpassaudio_cc_reset",
>> +	.reg_bits = 32,
>> +	.reg_stride = 4,
>> +	.val_bits = 32,
>> +	.fast_io = true,
>> +	.max_register = 0xc8,
>> +};
>> +
>>  static const struct qcom_cc_desc lpass_audio_cc_reset_sc7280_desc = {
>> -	.config = &lpass_audio_cc_sc7280_regmap_config,
>> +	.config = &lpass_audio_cc_sc7280_reset_regmap_config,
>>  	.resets = lpass_audio_cc_sc7280_resets,
>>  	.num_resets = ARRAY_SIZE(lpass_audio_cc_sc7280_resets),
>>  };
>>  
>>  static const struct of_device_id lpass_audio_cc_sc7280_match_table[] = {
>> -	{ .compatible = "qcom,sc7280-lpassaudiocc" },
>> +	{ .compatible = "qcom,qcm6490-lpassaudiocc", .data = &lpass_audio_cc_reset_sc7280_desc },
>> +	{ .compatible = "qcom,sc7280-lpassaudiocc", .data = &lpass_audio_cc_sc7280_desc },
>>  	{ }
>>  };
>>  MODULE_DEVICE_TABLE(of, lpass_audio_cc_sc7280_match_table);
>> @@ -752,13 +763,17 @@ static int lpass_audio_cc_sc7280_probe(struct platform_device *pdev)
>>  	struct regmap *regmap;
>>  	int ret;
>>  
>> +	desc = device_get_match_data(&pdev->dev);
>> +
>> +	if (desc->num_resets)
>> +		return qcom_cc_probe_by_index(pdev, 1, desc);
> 
> Won't this break SC7280 support by causing an early return?
> 

The resets are not defined for SC7280.
static const struct qcom_cc_desc lpass_audio_cc_sc7280_desc = {
        .config = &lpass_audio_cc_sc7280_regmap_config,
        .clks = lpass_audio_cc_sc7280_clocks,
        .num_clks = ARRAY_SIZE(lpass_audio_cc_sc7280_clocks),
};

The reset get registered for SC7280 after the clocks are registered.
qcom_cc_probe_by_index(pdev, 1,  &lpass_audio_cc_reset_sc7280_desc);

>> +
>>  	ret = lpass_audio_setup_runtime_pm(pdev);
>>  	if (ret)
>>  		return ret;
>>  
>>  	lpass_audio_cc_sc7280_regmap_config.name = "lpassaudio_cc";
>>  	lpass_audio_cc_sc7280_regmap_config.max_register = 0x2f000;
>> -	desc = &lpass_audio_cc_sc7280_desc;
>>  
>>  	regmap = qcom_cc_map(pdev, desc);
>>  	if (IS_ERR(regmap)) {
>> @@ -772,7 +787,7 @@ static int lpass_audio_cc_sc7280_probe(struct platform_device *pdev)
>>  	regmap_write(regmap, 0x4, 0x3b);
>>  	regmap_write(regmap, 0x8, 0xff05);
>>  
>> -	ret = qcom_cc_really_probe(&pdev->dev, &lpass_audio_cc_sc7280_desc, regmap);
>> +	ret = qcom_cc_really_probe(&pdev->dev, desc, regmap);
>>  	if (ret) {
>>  		dev_err(&pdev->dev, "Failed to register LPASS AUDIO CC clocks\n");
>>  		goto exit;
>>
>> -- 
>> 2.45.2
>>
> 

Re: [PATCH v3 2/4] clk: qcom: lpassaudiocc-sc7280: Add support for LPASS resets for QCM6490

[Dmitry Baryshkov]

On Wed, 12 Feb 2025 at 19:15, Taniya Das <quic_tdas@quicinc.com> wrote:
>
>
>
> On 2/12/2025 4:39 PM, Dmitry Baryshkov wrote:
> > On Wed, Feb 12, 2025 at 01:52:20PM +0530, Taniya Das wrote:
> >> On the QCM6490 boards the LPASS firmware controls the complete clock
> >> controller functionalities. But the LPASS resets are required to be
> >> controlled from the high level OS. The Audio SW driver should be able to
> >> assert/deassert the audio resets as required. Thus in clock driver add
> >> support for the resets.
> >>
> >> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
> >> ---
> >>  drivers/clk/qcom/lpassaudiocc-sc7280.c | 23 +++++++++++++++++++----
> >>  1 file changed, 19 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/clk/qcom/lpassaudiocc-sc7280.c b/drivers/clk/qcom/lpassaudiocc-sc7280.c
> >> index 45e7264770866f929a3f4663c477330f0bf7aa84..b6439308926371891cc5f9a5e0d4e8393641560d 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>
> >> @@ -713,14 +714,24 @@ static const struct qcom_reset_map lpass_audio_cc_sc7280_resets[] = {
> >>      [LPASS_AUDIO_SWR_WSA_CGCR] = { 0xb0, 1 },
> >>  };
> >>
> >> +static const struct regmap_config lpass_audio_cc_sc7280_reset_regmap_config = {
> >> +    .name = "lpassaudio_cc_reset",
> >> +    .reg_bits = 32,
> >> +    .reg_stride = 4,
> >> +    .val_bits = 32,
> >> +    .fast_io = true,
> >> +    .max_register = 0xc8,
> >> +};
> >> +
> >>  static const struct qcom_cc_desc lpass_audio_cc_reset_sc7280_desc = {
> >> -    .config = &lpass_audio_cc_sc7280_regmap_config,
> >> +    .config = &lpass_audio_cc_sc7280_reset_regmap_config,
> >>      .resets = lpass_audio_cc_sc7280_resets,
> >>      .num_resets = ARRAY_SIZE(lpass_audio_cc_sc7280_resets),
> >>  };
> >>
> >>  static const struct of_device_id lpass_audio_cc_sc7280_match_table[] = {
> >> -    { .compatible = "qcom,sc7280-lpassaudiocc" },
> >> +    { .compatible = "qcom,qcm6490-lpassaudiocc", .data = &lpass_audio_cc_reset_sc7280_desc },
> >> +    { .compatible = "qcom,sc7280-lpassaudiocc", .data = &lpass_audio_cc_sc7280_desc },
> >>      { }
> >>  };
> >>  MODULE_DEVICE_TABLE(of, lpass_audio_cc_sc7280_match_table);
> >> @@ -752,13 +763,17 @@ static int lpass_audio_cc_sc7280_probe(struct platform_device *pdev)
> >>      struct regmap *regmap;
> >>      int ret;
> >>
> >> +    desc = device_get_match_data(&pdev->dev);
> >> +
> >> +    if (desc->num_resets)
> >> +            return qcom_cc_probe_by_index(pdev, 1, desc);
> >
> > Won't this break SC7280 support by causing an early return?
> >
>
> The resets are not defined for SC7280.
> static const struct qcom_cc_desc lpass_audio_cc_sc7280_desc = {
>         .config = &lpass_audio_cc_sc7280_regmap_config,
>         .clks = lpass_audio_cc_sc7280_clocks,
>         .num_clks = ARRAY_SIZE(lpass_audio_cc_sc7280_clocks),
> };
>
> The reset get registered for SC7280 after the clocks are registered.
> qcom_cc_probe_by_index(pdev, 1,  &lpass_audio_cc_reset_sc7280_desc);

Could you please make this condition more obvious and error-prone
rather than checking one particular non-obvious property?

>
> >> +
> >>      ret = lpass_audio_setup_runtime_pm(pdev);
> >>      if (ret)
> >>              return ret;
> >>
> >>      lpass_audio_cc_sc7280_regmap_config.name = "lpassaudio_cc";
> >>      lpass_audio_cc_sc7280_regmap_config.max_register = 0x2f000;
> >> -    desc = &lpass_audio_cc_sc7280_desc;
> >>
> >>      regmap = qcom_cc_map(pdev, desc);
> >>      if (IS_ERR(regmap)) {
> >> @@ -772,7 +787,7 @@ static int lpass_audio_cc_sc7280_probe(struct platform_device *pdev)
> >>      regmap_write(regmap, 0x4, 0x3b);
> >>      regmap_write(regmap, 0x8, 0xff05);
> >>
> >> -    ret = qcom_cc_really_probe(&pdev->dev, &lpass_audio_cc_sc7280_desc, regmap);
> >> +    ret = qcom_cc_really_probe(&pdev->dev, desc, regmap);
> >>      if (ret) {
> >>              dev_err(&pdev->dev, "Failed to register LPASS AUDIO CC clocks\n");
> >>              goto exit;
> >>
> >> --
> >> 2.45.2
> >>
> >
>


-- 
With best wishes
Dmitry

Re: [PATCH v3 2/4] clk: qcom: lpassaudiocc-sc7280: Add support for LPASS resets for QCM6490

[Taniya Das]

On 2/13/2025 1:30 AM, Dmitry Baryshkov wrote:
> On Wed, 12 Feb 2025 at 19:15, Taniya Das <quic_tdas@quicinc.com> wrote:
>>
>>
>>
>> On 2/12/2025 4:39 PM, Dmitry Baryshkov wrote:
>>> On Wed, Feb 12, 2025 at 01:52:20PM +0530, Taniya Das wrote:
>>>> On the QCM6490 boards the LPASS firmware controls the complete clock
>>>> controller functionalities. But the LPASS resets are required to be
>>>> controlled from the high level OS. The Audio SW driver should be able to
>>>> assert/deassert the audio resets as required. Thus in clock driver add
>>>> support for the resets.
>>>>
>>>> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
>>>> ---
>>>>  drivers/clk/qcom/lpassaudiocc-sc7280.c | 23 +++++++++++++++++++----
>>>>  1 file changed, 19 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/clk/qcom/lpassaudiocc-sc7280.c b/drivers/clk/qcom/lpassaudiocc-sc7280.c
>>>> index 45e7264770866f929a3f4663c477330f0bf7aa84..b6439308926371891cc5f9a5e0d4e8393641560d 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>
>>>> @@ -713,14 +714,24 @@ static const struct qcom_reset_map lpass_audio_cc_sc7280_resets[] = {
>>>>      [LPASS_AUDIO_SWR_WSA_CGCR] = { 0xb0, 1 },
>>>>  };
>>>>
>>>> +static const struct regmap_config lpass_audio_cc_sc7280_reset_regmap_config = {
>>>> +    .name = "lpassaudio_cc_reset",
>>>> +    .reg_bits = 32,
>>>> +    .reg_stride = 4,
>>>> +    .val_bits = 32,
>>>> +    .fast_io = true,
>>>> +    .max_register = 0xc8,
>>>> +};
>>>> +
>>>>  static const struct qcom_cc_desc lpass_audio_cc_reset_sc7280_desc = {
>>>> -    .config = &lpass_audio_cc_sc7280_regmap_config,
>>>> +    .config = &lpass_audio_cc_sc7280_reset_regmap_config,
>>>>      .resets = lpass_audio_cc_sc7280_resets,
>>>>      .num_resets = ARRAY_SIZE(lpass_audio_cc_sc7280_resets),
>>>>  };
>>>>
>>>>  static const struct of_device_id lpass_audio_cc_sc7280_match_table[] = {
>>>> -    { .compatible = "qcom,sc7280-lpassaudiocc" },
>>>> +    { .compatible = "qcom,qcm6490-lpassaudiocc", .data = &lpass_audio_cc_reset_sc7280_desc },
>>>> +    { .compatible = "qcom,sc7280-lpassaudiocc", .data = &lpass_audio_cc_sc7280_desc },
>>>>      { }
>>>>  };
>>>>  MODULE_DEVICE_TABLE(of, lpass_audio_cc_sc7280_match_table);
>>>> @@ -752,13 +763,17 @@ static int lpass_audio_cc_sc7280_probe(struct platform_device *pdev)
>>>>      struct regmap *regmap;
>>>>      int ret;
>>>>
>>>> +    desc = device_get_match_data(&pdev->dev);
>>>> +
>>>> +    if (desc->num_resets)
>>>> +            return qcom_cc_probe_by_index(pdev, 1, desc);
>>>
>>> Won't this break SC7280 support by causing an early return?
>>>
>>
>> The resets are not defined for SC7280.
>> static const struct qcom_cc_desc lpass_audio_cc_sc7280_desc = {
>>         .config = &lpass_audio_cc_sc7280_regmap_config,
>>         .clks = lpass_audio_cc_sc7280_clocks,
>>         .num_clks = ARRAY_SIZE(lpass_audio_cc_sc7280_clocks),
>> };
>>
>> The reset get registered for SC7280 after the clocks are registered.
>> qcom_cc_probe_by_index(pdev, 1,  &lpass_audio_cc_reset_sc7280_desc);
> 
> Could you please make this condition more obvious and error-prone
> rather than checking one particular non-obvious property?
> 

Dmitry, we had earlier tried [1], but seems like we could not align on
this patchset.

If you are aligned, please let me know I can fall back on the approach.

[1]:
https://lore.kernel.org/all/20240318053555.20405-3-quic_tdas@quicinc.com/

Do you have any suggestions that we could consider?

>>
>>>> +
>>>>      ret = lpass_audio_setup_runtime_pm(pdev);
>>>>      if (ret)
>>>>              return ret;
>>>>
>>>>      lpass_audio_cc_sc7280_regmap_config.name = "lpassaudio_cc";
>>>>      lpass_audio_cc_sc7280_regmap_config.max_register = 0x2f000;
>>>> -    desc = &lpass_audio_cc_sc7280_desc;
>>>>
>>>>      regmap = qcom_cc_map(pdev, desc);
>>>>      if (IS_ERR(regmap)) {
>>>> @@ -772,7 +787,7 @@ static int lpass_audio_cc_sc7280_probe(struct platform_device *pdev)
>>>>      regmap_write(regmap, 0x4, 0x3b);
>>>>      regmap_write(regmap, 0x8, 0xff05);
>>>>
>>>> -    ret = qcom_cc_really_probe(&pdev->dev, &lpass_audio_cc_sc7280_desc, regmap);
>>>> +    ret = qcom_cc_really_probe(&pdev->dev, desc, regmap);
>>>>      if (ret) {
>>>>              dev_err(&pdev->dev, "Failed to register LPASS AUDIO CC clocks\n");
>>>>              goto exit;
>>>>
>>>> --
>>>> 2.45.2
>>>>
>>>
>>
> 
> 

Re: [PATCH v3 2/4] clk: qcom: lpassaudiocc-sc7280: Add support for LPASS resets for QCM6490

[Dmitry Baryshkov]

On Thu, 13 Feb 2025 at 08:52, Taniya Das <quic_tdas@quicinc.com> wrote:
>
>
>
> On 2/13/2025 1:30 AM, Dmitry Baryshkov wrote:
> > On Wed, 12 Feb 2025 at 19:15, Taniya Das <quic_tdas@quicinc.com> wrote:
> >>
> >>
> >>
> >> On 2/12/2025 4:39 PM, Dmitry Baryshkov wrote:
> >>> On Wed, Feb 12, 2025 at 01:52:20PM +0530, Taniya Das wrote:
> >>>> On the QCM6490 boards the LPASS firmware controls the complete clock
> >>>> controller functionalities. But the LPASS resets are required to be
> >>>> controlled from the high level OS. The Audio SW driver should be able to
> >>>> assert/deassert the audio resets as required. Thus in clock driver add
> >>>> support for the resets.
> >>>>
> >>>> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
> >>>> ---
> >>>>  drivers/clk/qcom/lpassaudiocc-sc7280.c | 23 +++++++++++++++++++----
> >>>>  1 file changed, 19 insertions(+), 4 deletions(-)
> >>>>
> >>>> diff --git a/drivers/clk/qcom/lpassaudiocc-sc7280.c b/drivers/clk/qcom/lpassaudiocc-sc7280.c
> >>>> index 45e7264770866f929a3f4663c477330f0bf7aa84..b6439308926371891cc5f9a5e0d4e8393641560d 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>
> >>>> @@ -713,14 +714,24 @@ static const struct qcom_reset_map lpass_audio_cc_sc7280_resets[] = {
> >>>>      [LPASS_AUDIO_SWR_WSA_CGCR] = { 0xb0, 1 },
> >>>>  };
> >>>>
> >>>> +static const struct regmap_config lpass_audio_cc_sc7280_reset_regmap_config = {
> >>>> +    .name = "lpassaudio_cc_reset",
> >>>> +    .reg_bits = 32,
> >>>> +    .reg_stride = 4,
> >>>> +    .val_bits = 32,
> >>>> +    .fast_io = true,
> >>>> +    .max_register = 0xc8,
> >>>> +};
> >>>> +
> >>>>  static const struct qcom_cc_desc lpass_audio_cc_reset_sc7280_desc = {
> >>>> -    .config = &lpass_audio_cc_sc7280_regmap_config,
> >>>> +    .config = &lpass_audio_cc_sc7280_reset_regmap_config,
> >>>>      .resets = lpass_audio_cc_sc7280_resets,
> >>>>      .num_resets = ARRAY_SIZE(lpass_audio_cc_sc7280_resets),
> >>>>  };
> >>>>
> >>>>  static const struct of_device_id lpass_audio_cc_sc7280_match_table[] = {
> >>>> -    { .compatible = "qcom,sc7280-lpassaudiocc" },
> >>>> +    { .compatible = "qcom,qcm6490-lpassaudiocc", .data = &lpass_audio_cc_reset_sc7280_desc },
> >>>> +    { .compatible = "qcom,sc7280-lpassaudiocc", .data = &lpass_audio_cc_sc7280_desc },
> >>>>      { }
> >>>>  };
> >>>>  MODULE_DEVICE_TABLE(of, lpass_audio_cc_sc7280_match_table);
> >>>> @@ -752,13 +763,17 @@ static int lpass_audio_cc_sc7280_probe(struct platform_device *pdev)
> >>>>      struct regmap *regmap;
> >>>>      int ret;
> >>>>
> >>>> +    desc = device_get_match_data(&pdev->dev);
> >>>> +
> >>>> +    if (desc->num_resets)
> >>>> +            return qcom_cc_probe_by_index(pdev, 1, desc);
> >>>
> >>> Won't this break SC7280 support by causing an early return?
> >>>
> >>
> >> The resets are not defined for SC7280.
> >> static const struct qcom_cc_desc lpass_audio_cc_sc7280_desc = {
> >>         .config = &lpass_audio_cc_sc7280_regmap_config,
> >>         .clks = lpass_audio_cc_sc7280_clocks,
> >>         .num_clks = ARRAY_SIZE(lpass_audio_cc_sc7280_clocks),
> >> };
> >>
> >> The reset get registered for SC7280 after the clocks are registered.
> >> qcom_cc_probe_by_index(pdev, 1,  &lpass_audio_cc_reset_sc7280_desc);
> >
> > Could you please make this condition more obvious and error-prone
> > rather than checking one particular non-obvious property?
> >
>
> Dmitry, we had earlier tried [1], but seems like we could not align on
> this patchset.
>
> If you are aligned, please let me know I can fall back on the approach.

You have been using of_device_is_compatible(). Krzysztof suggested
using mach data. Both approaches are fine with me (I'm sorry,
Krzysztof, this is a clock driver for a single platform, it doesn't
need to scale).

You've settled on the second one. So far so good.

But! The problem is in readability. Checking for desc->num_resets is a
_hidden_ or cryptic way of checking whether to register only a first
controller or both.

BTW: the commit message also tells nothing about the dropped power
domain and skipped PM code. Is it not required anymore? Is it handled
automatically by the firmware? But I see that audio codecs still use
that power domain.

>
> [1]:
> https://lore.kernel.org/all/20240318053555.20405-3-quic_tdas@quicinc.com/
>
> Do you have any suggestions that we could consider?
>
> >>
> >>>> +
> >>>>      ret = lpass_audio_setup_runtime_pm(pdev);
> >>>>      if (ret)
> >>>>              return ret;
> >>>>
> >>>>      lpass_audio_cc_sc7280_regmap_config.name = "lpassaudio_cc";
> >>>>      lpass_audio_cc_sc7280_regmap_config.max_register = 0x2f000;
> >>>> -    desc = &lpass_audio_cc_sc7280_desc;
> >>>>
> >>>>      regmap = qcom_cc_map(pdev, desc);
> >>>>      if (IS_ERR(regmap)) {
> >>>> @@ -772,7 +787,7 @@ static int lpass_audio_cc_sc7280_probe(struct platform_device *pdev)
> >>>>      regmap_write(regmap, 0x4, 0x3b);
> >>>>      regmap_write(regmap, 0x8, 0xff05);
> >>>>
> >>>> -    ret = qcom_cc_really_probe(&pdev->dev, &lpass_audio_cc_sc7280_desc, regmap);
> >>>> +    ret = qcom_cc_really_probe(&pdev->dev, desc, regmap);
> >>>>      if (ret) {
> >>>>              dev_err(&pdev->dev, "Failed to register LPASS AUDIO CC clocks\n");
> >>>>              goto exit;
> >>>>
> >>>> --
> >>>> 2.45.2
> >>>>
> >>>
> >>
> >
> >
>


-- 
With best wishes
Dmitry

Re: [PATCH v3 2/4] clk: qcom: lpassaudiocc-sc7280: Add support for LPASS resets for QCM6490

[Taniya Das]

On 2/13/2025 7:58 PM, Dmitry Baryshkov wrote:
>>>>>>  static const struct qcom_cc_desc lpass_audio_cc_reset_sc7280_desc = {
>>>>>> -    .config = &lpass_audio_cc_sc7280_regmap_config,
>>>>>> +    .config = &lpass_audio_cc_sc7280_reset_regmap_config,
>>>>>>      .resets = lpass_audio_cc_sc7280_resets,
>>>>>>      .num_resets = ARRAY_SIZE(lpass_audio_cc_sc7280_resets),
>>>>>>  };
>>>>>>
>>>>>>  static const struct of_device_id lpass_audio_cc_sc7280_match_table[] = {
>>>>>> -    { .compatible = "qcom,sc7280-lpassaudiocc" },
>>>>>> +    { .compatible = "qcom,qcm6490-lpassaudiocc", .data = &lpass_audio_cc_reset_sc7280_desc },
>>>>>> +    { .compatible = "qcom,sc7280-lpassaudiocc", .data = &lpass_audio_cc_sc7280_desc },
>>>>>>      { }
>>>>>>  };
>>>>>>  MODULE_DEVICE_TABLE(of, lpass_audio_cc_sc7280_match_table);
>>>>>> @@ -752,13 +763,17 @@ static int lpass_audio_cc_sc7280_probe(struct platform_device *pdev)
>>>>>>      struct regmap *regmap;
>>>>>>      int ret;
>>>>>>
>>>>>> +    desc = device_get_match_data(&pdev->dev);
>>>>>> +
>>>>>> +    if (desc->num_resets)
>>>>>> +            return qcom_cc_probe_by_index(pdev, 1, desc);
>>>>> Won't this break SC7280 support by causing an early return?
>>>>>
>>>> The resets are not defined for SC7280.
>>>> static const struct qcom_cc_desc lpass_audio_cc_sc7280_desc = {
>>>>         .config = &lpass_audio_cc_sc7280_regmap_config,
>>>>         .clks = lpass_audio_cc_sc7280_clocks,
>>>>         .num_clks = ARRAY_SIZE(lpass_audio_cc_sc7280_clocks),
>>>> };
>>>>
>>>> The reset get registered for SC7280 after the clocks are registered.
>>>> qcom_cc_probe_by_index(pdev, 1,  &lpass_audio_cc_reset_sc7280_desc);
>>> Could you please make this condition more obvious and error-prone
>>> rather than checking one particular non-obvious property?
>>>
>> Dmitry, we had earlier tried [1], but seems like we could not align on
>> this patchset.
>>
>> If you are aligned, please let me know I can fall back on the approach.
> You have been using of_device_is_compatible(). Krzysztof suggested
> using mach data. Both approaches are fine with me (I'm sorry,
> Krzysztof, this is a clock driver for a single platform, it doesn't
> need to scale).
> 
> You've settled on the second one. So far so good.

Sure, I will go ahead with the existing approach, but ensure I replace
the num_resets check with the of_device_is_compatible(), so it is more
readable. Hope this aligns with your thoughts as well.

> 
> But! The problem is in readability. Checking for desc->num_resets is a
> _hidden_ or cryptic way of checking whether to register only a first
> controller or both.
> 
> BTW: the commit message also tells nothing about the dropped power
> domain and skipped PM code. Is it not required anymore? Is it handled
> automatically by the firmware? But I see that audio codecs still use
> that power domain.
Yes, it will be taken care in the firmware and I will update in the
commit text.


Thanks,
Taniya.

Re: [PATCH v3 2/4] clk: qcom: lpassaudiocc-sc7280: Add support for LPASS resets for QCM6490

[Dmitry Baryshkov]

On Tue, Feb 18, 2025 at 10:53:05AM +0530, Taniya Das wrote:
> 
> 
> On 2/13/2025 7:58 PM, Dmitry Baryshkov wrote:
> >>>>>>  static const struct qcom_cc_desc lpass_audio_cc_reset_sc7280_desc = {
> >>>>>> -    .config = &lpass_audio_cc_sc7280_regmap_config,
> >>>>>> +    .config = &lpass_audio_cc_sc7280_reset_regmap_config,
> >>>>>>      .resets = lpass_audio_cc_sc7280_resets,
> >>>>>>      .num_resets = ARRAY_SIZE(lpass_audio_cc_sc7280_resets),
> >>>>>>  };
> >>>>>>
> >>>>>>  static const struct of_device_id lpass_audio_cc_sc7280_match_table[] = {
> >>>>>> -    { .compatible = "qcom,sc7280-lpassaudiocc" },
> >>>>>> +    { .compatible = "qcom,qcm6490-lpassaudiocc", .data = &lpass_audio_cc_reset_sc7280_desc },
> >>>>>> +    { .compatible = "qcom,sc7280-lpassaudiocc", .data = &lpass_audio_cc_sc7280_desc },
> >>>>>>      { }
> >>>>>>  };
> >>>>>>  MODULE_DEVICE_TABLE(of, lpass_audio_cc_sc7280_match_table);
> >>>>>> @@ -752,13 +763,17 @@ static int lpass_audio_cc_sc7280_probe(struct platform_device *pdev)
> >>>>>>      struct regmap *regmap;
> >>>>>>      int ret;
> >>>>>>
> >>>>>> +    desc = device_get_match_data(&pdev->dev);
> >>>>>> +
> >>>>>> +    if (desc->num_resets)
> >>>>>> +            return qcom_cc_probe_by_index(pdev, 1, desc);
> >>>>> Won't this break SC7280 support by causing an early return?
> >>>>>
> >>>> The resets are not defined for SC7280.
> >>>> static const struct qcom_cc_desc lpass_audio_cc_sc7280_desc = {
> >>>>         .config = &lpass_audio_cc_sc7280_regmap_config,
> >>>>         .clks = lpass_audio_cc_sc7280_clocks,
> >>>>         .num_clks = ARRAY_SIZE(lpass_audio_cc_sc7280_clocks),
> >>>> };
> >>>>
> >>>> The reset get registered for SC7280 after the clocks are registered.
> >>>> qcom_cc_probe_by_index(pdev, 1,  &lpass_audio_cc_reset_sc7280_desc);
> >>> Could you please make this condition more obvious and error-prone
> >>> rather than checking one particular non-obvious property?
> >>>
> >> Dmitry, we had earlier tried [1], but seems like we could not align on
> >> this patchset.
> >>
> >> If you are aligned, please let me know I can fall back on the approach.
> > You have been using of_device_is_compatible(). Krzysztof suggested
> > using mach data. Both approaches are fine with me (I'm sorry,
> > Krzysztof, this is a clock driver for a single platform, it doesn't
> > need to scale).
> > 
> > You've settled on the second one. So far so good.
> 
> Sure, I will go ahead with the existing approach, but ensure I replace
> the num_resets check with the of_device_is_compatible(), so it is more
> readable. Hope this aligns with your thoughts as well.

Ack, thanks.

> 
> > 
> > But! The problem is in readability. Checking for desc->num_resets is a
> > _hidden_ or cryptic way of checking whether to register only a first
> > controller or both.
> > 
> > BTW: the commit message also tells nothing about the dropped power
> > domain and skipped PM code. Is it not required anymore? Is it handled
> > automatically by the firmware? But I see that audio codecs still use
> > that power domain.
> Yes, it will be taken care in the firmware and I will update in the
> commit text.

Ack, thanks.

> 
> 
> Thanks,
> Taniya.

-- 
With best wishes
Dmitry

[PATCH v3 3/4] arm64: dts: qcom: qcm6490-idp: Update the LPASS audio node

[Taniya Das]

Update the lpassaudio node to support the new compatible as the
lpassaudio needs to support the reset functionality on the
QCM6490 IDP board and the rest of the Audio functionality would be
provided from the LPASS firmware.

Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
---
 arch/arm64/boot/dts/qcom/qcm6490-idp.dts | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
index 9209efcc49b57a853c4dd55ac52cd4dc98d7fe86..86d58f625d0e16a1eccd9925609b496fbd066051 100644
--- a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
+++ b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
@@ -759,3 +759,8 @@ &wifi {
 
 	status = "okay";
 };
+
+&lpass_audiocc {
+	compatible = "qcom,qcm6490-lpassaudiocc";
+	/delete-property/ power-domains;
+};

-- 
2.45.2

[PATCH v3 4/4] arm64: dts: qcom: qcs6490-rb3gen2: Update the LPASS audio node

[Taniya Das]

Update the lpassaudio node to support the new compatible as the
lpassaudio needs to support the reset functionality on the
QCS6490 RB3Gen2 board and the rest of the Audio functionality would be
provided from the LPASS firmware.

Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
---
 arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
index 7a36c90ad4ec8b52f30b22b1621404857d6ef336..057bbb187ab35a7e6cca9cff6a3e4569ae045fe2 100644
--- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
+++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
@@ -986,3 +986,8 @@ sd_cd: sd-cd-state {
 		bias-pull-up;
 	};
 };
+
+&lpass_audiocc {
+	compatible = "qcom,qcm6490-lpassaudiocc";
+	/delete-property/ power-domains;
+};

-- 
2.45.2