diff mbox series

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

Message ID 20240816-qcm6490-lpass-reset-v1-2-a11f33cad3c5@quicinc.com (mailing list archive)
State New
Headers show
Series Update LPASS Audio clock driver for QCM6490 board | expand

Commit Message

Taniya Das Aug. 16, 2024, 8:32 a.m. UTC
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(-)

Comments

Krzysztof Kozlowski Aug. 17, 2024, 9:25 a.m. UTC | #1
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
Taniya Das Sept. 13, 2024, 5:31 a.m. UTC | #2
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
>
Krzysztof Kozlowski Sept. 16, 2024, 8:33 a.m. UTC | #3
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
Dmitry Baryshkov Sept. 16, 2024, 8:55 a.m. UTC | #4
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?
Krzysztof Kozlowski Sept. 16, 2024, 9:03 a.m. UTC | #5
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
diff mbox series

Patch

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;