[PATCH v2 0/5] Update LPASS Audio clock driver for QCM6490 board

[PATCH v2 0/5] 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.

[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 (5):
      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 protected clocks list
      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    |  1 +
 arch/arm64/boot/dts/qcom/qcm6490-idp.dts           | 28 +++++++++++++++++++++-
 arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts       |  5 ++++
 drivers/clk/qcom/lpassaudiocc-sc7280.c             | 23 ++++++++++++++----
 4 files changed, 52 insertions(+), 5 deletions(-)
---
base-commit: 367b5c3d53e57d51a5878816804652963da90950
change-id: 20240816-qcm6490-lpass-reset-e9a880a141f2

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

[PATCH v2 1/5] 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>
---
 Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml b/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml
index deee5423d66e..5d6f74cf9c43 100644
--- a/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml
@@ -26,6 +26,7 @@ properties:
 
   compatible:
     enum:
+      - qcom,qcm6490-lpassaudiocc
       - qcom,sc7280-lpassaoncc
       - qcom,sc7280-lpassaudiocc
       - qcom,sc7280-lpasscorecc

-- 
2.45.2

[PATCH v2 2/5] 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 45e726477086..b64393089263 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

[PATCH v2 3/5] arm64: dts: qcom: qcm6490-idp: Update protected clocks list

[Taniya Das]

Certain clocks are not accessible on QCM6490-IDP board,
thus mark them as protected.

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

diff --git a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
index a0668f767e4b..4b20b37d488d 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/;
@@ -688,3 +688,24 @@ &usb_1_qmpphy {
 &wifi {
 	memory-region = <&wlan_fw_mem>;
 };
+
+&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>;
+};

-- 
2.45.2

[PATCH v2 4/5] 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 4b20b37d488d..4eece564331a 100644
--- a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
+++ b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
@@ -709,3 +709,8 @@ &gcc {
 			<GCC_SEC_CTRL_CLK_SRC>, <GCC_WPSS_AHB_CLK>,
 			<GCC_WPSS_AHB_BDG_MST_CLK>, <GCC_WPSS_RSCP_CLK>;
 };
+
+&lpass_audiocc {
+	compatible = "qcom,qcm6490-lpassaudiocc";
+	/delete-property/ power-domains;
+};

-- 
2.45.2

[PATCH v2 5/5] 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 0d45662b8028..6e22c838eaff 100644
--- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
+++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
@@ -820,3 +820,8 @@ lt9611_irq_pin: lt9611-irq-state {
 		bias-disable;
 	};
 };
+
+&lpass_audiocc {
+	compatible = "qcom,qcm6490-lpassaudiocc";
+	/delete-property/ power-domains;
+};

-- 
2.45.2

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

[Krzysztof Kozlowski]

On 16/08/2024 10:32, Taniya Das wrote:
> Add the new QCM6490 compatible to support the reset functionality for
> Low Power Audio subsystem.
> 
> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>

Subject is odd - I do not see here anything related to boards.

Anyway, this is incomplete. Look at the rest of the binding - you did
not update any part related to proper clock constraints.

Best regards,
Krzysztof

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

[Krzysztof Kozlowski]

On 16/08/2024 10:32, 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 45e726477086..b64393089263 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 },

That's odd to see sc7280 reset added for qcm6490, but not used fot
sc7280 at all. Didn't you mean here lpass_audio_cc_qcm6409_desc?


Best regards,
Krzysztof

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

[Taniya Das]

On 8/17/2024 2:51 PM, Krzysztof Kozlowski wrote:
> On 16/08/2024 10:32, Taniya Das wrote:
>> Add the new QCM6490 compatible to support the reset functionality for
>> Low Power Audio subsystem.
>>
>> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
> 
> Subject is odd - I do not see here anything related to boards.
> 
> Anyway, this is incomplete. Look at the rest of the binding - you did
> not update any part related to proper clock constraints.
> 

Not sure if I understand the concern, but I was of the opinion that I 
have added a new compatible for QCM6490 board , but no new clock 
constraint added.


I see a patch from you 
https://lore.kernel.org/all/20240817094605.27185-1-krzysztof.kozlowski@linaro.org/ 
and I guess it fixes the constraints.


Please help with your comments further.
> Best regards,
> Krzysztof
> 

-- 
Thanks & Regards,
Taniya Das.

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

[Taniya Das]

On 8/17/2024 2:55 PM, Krzysztof Kozlowski wrote:
> On 16/08/2024 10:32, 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 45e726477086..b64393089263 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 },
> 
> That's odd to see sc7280 reset added for qcm6490, but not used fot
> sc7280 at all. Didn't you mean here lpass_audio_cc_qcm6409_desc?
> 
> 
The resets descriptor(lpass_audio_cc_reset_sc7280_desc) is not part of 
the global clock descriptor(lpass_cc_sc7280_desc) as these are part of 
different regmaps.

On a non-QCM6490(SC7280) boards the resets are registered after the 
global descriptor is registered.

But on QCM6490 board we need to register only the reset descriptor and 
no clocks are to be handled/registered and thus passed the match data 
for QCM6490 boards only.

> Best regards,
> Krzysztof
> 

-- 
Thanks & Regards,
Taniya Das.

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

[Krzysztof Kozlowski]

On 13/09/2024 07:31, Taniya Das wrote:
> 
> 
> On 8/17/2024 2:55 PM, Krzysztof Kozlowski wrote:
>> On 16/08/2024 10:32, 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 45e726477086..b64393089263 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 },
>>
>> That's odd to see sc7280 reset added for qcm6490, but not used fot
>> sc7280 at all. Didn't you mean here lpass_audio_cc_qcm6409_desc?
>>
>>
> The resets descriptor(lpass_audio_cc_reset_sc7280_desc) is not part of 
> the global clock descriptor(lpass_cc_sc7280_desc) as these are part of 
> different regmaps.
> 
> On a non-QCM6490(SC7280) boards the resets are registered after the 
> global descriptor is registered.
> 
> But on QCM6490 board we need to register only the reset descriptor and 
> no clocks are to be handled/registered and thus passed the match data 
> for QCM6490 boards only.

Yeah, but why this is sc7280?

Best regards,
Krzysztof

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

[Krzysztof Kozlowski]

On 13/09/2024 07:18, Taniya Das wrote:
> 
> 
> On 8/17/2024 2:51 PM, Krzysztof Kozlowski wrote:
>> On 16/08/2024 10:32, Taniya Das wrote:
>>> Add the new QCM6490 compatible to support the reset functionality for
>>> Low Power Audio subsystem.
>>>
>>> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
>>
>> Subject is odd - I do not see here anything related to boards.
>>
>> Anyway, this is incomplete. Look at the rest of the binding - you did
>> not update any part related to proper clock constraints.
>>
> 
> Not sure if I understand the concern, but I was of the opinion that I 
> have added a new compatible for QCM6490 board , but no new clock 
> constraint added.
> 
> 
> I see a patch from you 
> https://lore.kernel.org/all/20240817094605.27185-1-krzysztof.kozlowski@linaro.org/ 
> and I guess it fixes the constraints.
> 

Look at entire file. This is incomplete.

Best regards,
Krzysztof

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

[Dmitry Baryshkov]

On Mon, Sep 16, 2024 at 10:33:21AM GMT, Krzysztof Kozlowski wrote:
> On 13/09/2024 07:31, Taniya Das wrote:
> > 
> > 
> > On 8/17/2024 2:55 PM, Krzysztof Kozlowski wrote:
> >> On 16/08/2024 10:32, 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 45e726477086..b64393089263 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 },
> >>
> >> That's odd to see sc7280 reset added for qcm6490, but not used fot
> >> sc7280 at all. Didn't you mean here lpass_audio_cc_qcm6409_desc?
> >>
> >>
> > The resets descriptor(lpass_audio_cc_reset_sc7280_desc) is not part of 
> > the global clock descriptor(lpass_cc_sc7280_desc) as these are part of 
> > different regmaps.
> > 
> > On a non-QCM6490(SC7280) boards the resets are registered after the 
> > global descriptor is registered.
> > 
> > But on QCM6490 board we need to register only the reset descriptor and 
> > no clocks are to be handled/registered and thus passed the match data 
> > for QCM6490 boards only.
> 
> Yeah, but why this is sc7280?

Because it's more or less the same HW, different TZ and hyp firmware?

-- 
With best wishes
Dmitry

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

[Krzysztof Kozlowski]

On 16/09/2024 10:55, Dmitry Baryshkov wrote:
> On Mon, Sep 16, 2024 at 10:33:21AM GMT, Krzysztof Kozlowski wrote:
>> On 13/09/2024 07:31, Taniya Das wrote:
>>>
>>>
>>> On 8/17/2024 2:55 PM, Krzysztof Kozlowski wrote:
>>>> On 16/08/2024 10:32, 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 45e726477086..b64393089263 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 },
>>>>
>>>> That's odd to see sc7280 reset added for qcm6490, but not used fot
>>>> sc7280 at all. Didn't you mean here lpass_audio_cc_qcm6409_desc?
>>>>
>>>>
>>> The resets descriptor(lpass_audio_cc_reset_sc7280_desc) is not part of 
>>> the global clock descriptor(lpass_cc_sc7280_desc) as these are part of 
>>> different regmaps.
>>>
>>> On a non-QCM6490(SC7280) boards the resets are registered after the 
>>> global descriptor is registered.
>>>
>>> But on QCM6490 board we need to register only the reset descriptor and 
>>> no clocks are to be handled/registered and thus passed the match data 
>>> for QCM6490 boards only.
>>
>> Yeah, but why this is sc7280?
> 
> Because it's more or less the same HW, different TZ and hyp firmware?
> 

Hm, ok, probably I missed something from the context.

Best regards,
Krzysztof

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

[Taniya Das]

On 9/16/2024 2:33 PM, Krzysztof Kozlowski wrote:
> On 16/09/2024 10:55, Dmitry Baryshkov wrote:
>> On Mon, Sep 16, 2024 at 10:33:21AM GMT, Krzysztof Kozlowski wrote:
>>> On 13/09/2024 07:31, Taniya Das wrote:
>>>>
>>>>
>>>> On 8/17/2024 2:55 PM, Krzysztof Kozlowski wrote:
>>>>> On 16/08/2024 10:32, 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 45e726477086..b64393089263 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 },
>>>>>
>>>>> That's odd to see sc7280 reset added for qcm6490, but not used fot
>>>>> sc7280 at all. Didn't you mean here lpass_audio_cc_qcm6409_desc?
>>>>>
>>>>>
>>>> The resets descriptor(lpass_audio_cc_reset_sc7280_desc) is not part of
>>>> the global clock descriptor(lpass_cc_sc7280_desc) as these are part of
>>>> different regmaps.
>>>>
>>>> On a non-QCM6490(SC7280) boards the resets are registered after the
>>>> global descriptor is registered.
>>>>
>>>> But on QCM6490 board we need to register only the reset descriptor and
>>>> no clocks are to be handled/registered and thus passed the match data
>>>> for QCM6490 boards only.
>>>
>>> Yeah, but why this is sc7280?
>>
>> Because it's more or less the same HW, different TZ and hyp firmware?
>>
> 
> Hm, ok, probably I missed something from the context.
>

Apologies for responding on this thread later than expected.
Yes as Dmitry mentioned, it is the same HW, TZ and HYP firmware are 
different.

> Best regards,
> Krzysztof



-- 
Thanks & Regards,
Taniya Das.

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

[Taniya Das]

On 9/16/2024 2:10 PM, Krzysztof Kozlowski wrote:
> On 13/09/2024 07:18, Taniya Das wrote:
>>
>>
>> On 8/17/2024 2:51 PM, Krzysztof Kozlowski wrote:
>>> On 16/08/2024 10:32, Taniya Das wrote:
>>>> Add the new QCM6490 compatible to support the reset functionality for
>>>> Low Power Audio subsystem.
>>>>
>>>> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
>>>
>>> Subject is odd - I do not see here anything related to boards.
>>>
>>> Anyway, this is incomplete. Look at the rest of the binding - you did
>>> not update any part related to proper clock constraints.
>>>
>>
>> Not sure if I understand the concern, but I was of the opinion that I
>> have added a new compatible for QCM6490 board , but no new clock
>> constraint added.
>>
>>
>> I see a patch from you
>> https://lore.kernel.org/all/20240817094605.27185-1-krzysztof.kozlowski@linaro.org/
>> and I guess it fixes the constraints.
>>
> 
> Look at entire file. This is incomplete.
> 

Please accept apologies for taking longer than expected to respond on 
this thread.

Yes I think you meant something as below to be taken care as 
constraints, which I have overlooked in my earlier patch.


@@ -26,6 +26,7 @@ properties:

    compatible:
      enum:
+      - qcom,qcm6490-lpassaudiocc
        - qcom,sc7280-lpassaoncc
        - qcom,sc7280-lpassaudiocc
        - qcom,sc7280-lpasscorecc
@@ -125,6 +126,18 @@ 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

> Best regards,
> Krzysztof

-- 
Thanks & Regards,
Taniya Das.