[PATCH 0/2] Add resets for ADSP based audio clock controller driver.

[PATCH 0/2] Add resets for ADSP based audio clock controller driver.

[Srinivasa Rao Mandadapu]

Add resets for audio clock controller on audioreach based platforms. 

Srinivasa Rao Mandadapu (2):
  dt-bindings: clock: SC7280:  Add resets for LPASS audio clock
    controller
  clk: qcom: lpasscc: Add resets for SC7280 audioreach clock controller

 .../devicetree/bindings/clock/qcom,sc7280-lpasscc.yaml |  4 ++++
 drivers/clk/qcom/lpasscc-sc7280.c                      | 18 ++++++++++++++++++
 2 files changed, 22 insertions(+)

-- 
2.7.4

[PATCH 1/2] dt-bindings: clock: SC7280: Add resets for LPASS audio clock controller

[Srinivasa Rao Mandadapu]

Add support for LPASS audio clock gating for RX/TX/SWA core bus clocks
for audioreach based SC7280 platforms.

Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
---
 Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscc.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscc.yaml b/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscc.yaml
index 6151fde..b9be5f1 100644
--- a/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscc.yaml
@@ -31,6 +31,9 @@ properties:
   '#clock-cells':
     const: 1
 
+  '#reset-cells':
+    const: 1
+
   reg:
     items:
       - description: LPASS qdsp6ss register
@@ -61,5 +64,6 @@ examples:
       clocks = <&gcc GCC_CFG_NOC_LPASS_CLK>;
       clock-names = "iface";
       #clock-cells = <1>;
+      #reset-cells = <1>;
     };
 ...
-- 
2.7.4

Re: [PATCH 1/2] dt-bindings: clock: SC7280: Add resets for LPASS audio clock controller

[Krzysztof Kozlowski]

On 21/12/2022 11:21, Srinivasa Rao Mandadapu wrote:
> Add support for LPASS audio clock gating for RX/TX/SWA core bus clocks
> for audioreach based SC7280 platforms.

Use subject prefixes matching the subsystem (git log --oneline -- ...).
The final prefix should be "qcom,sc7280-lpasscc" and then the actual
subject should drop redundant pieces.

>  ...

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: clock: SC7280: Add resets for LPASS audio clock controller

[Srinivasa Rao Mandadapu]

On 12/21/2022 4:12 PM, Krzysztof Kozlowski wrote:
Thanks for your time Krzyszto!!!
> On 21/12/2022 11:21, Srinivasa Rao Mandadapu wrote:
>> Add support for LPASS audio clock gating for RX/TX/SWA core bus clocks
>> for audioreach based SC7280 platforms.
> Use subject prefixes matching the subsystem (git log --oneline -- ...).
> The final prefix should be "qcom,sc7280-lpasscc" and then the actual
> subject should drop redundant pieces.

Sorry. I didn't understand much from your statement.

Do you mean subject should something like below?

  dt-bindings: clock: qcom: sc7280-lpasscc: Add resets for audio clock 
controller

>
>>   ...
> Best regards,
> Krzysztof
>

Re: [PATCH 1/2] dt-bindings: clock: SC7280: Add resets for LPASS audio clock controller

[Krzysztof Kozlowski]

On 21/12/2022 13:22, Srinivasa Rao Mandadapu wrote:
> 
> On 12/21/2022 4:12 PM, Krzysztof Kozlowski wrote:
> Thanks for your time Krzyszto!!!
>> On 21/12/2022 11:21, Srinivasa Rao Mandadapu wrote:
>>> Add support for LPASS audio clock gating for RX/TX/SWA core bus clocks
>>> for audioreach based SC7280 platforms.
>> Use subject prefixes matching the subsystem (git log --oneline -- ...).
>> The final prefix should be "qcom,sc7280-lpasscc" and then the actual

                                ^^^^^^^ it's written here

>> subject should drop redundant pieces.
> 
> Sorry. I didn't understand much from your statement.
> 
> Do you mean subject should something like below?
> 
>   dt-bindings: clock: qcom: sc7280-lpasscc: Add resets for audio clock 
> controller

1. The last prefix should be "qcom,sc7280-lpasscc:".
2. And then drop "audio clock controller" because it is obvious, isn't it?

> 
>>
>>>   ...
>> Best regards,
>> Krzysztof
>>

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: clock: SC7280: Add resets for LPASS audio clock controller

[Srinivasa Rao Mandadapu]

On 12/21/2022 5:54 PM, Krzysztof Kozlowski wrote:
> On 21/12/2022 13:22, Srinivasa Rao Mandadapu wrote:
>> On 12/21/2022 4:12 PM, Krzysztof Kozlowski wrote:
>> Thanks for your time Krzyszto!!!
>>> On 21/12/2022 11:21, Srinivasa Rao Mandadapu wrote:
>>>> Add support for LPASS audio clock gating for RX/TX/SWA core bus clocks
>>>> for audioreach based SC7280 platforms.
>>> Use subject prefixes matching the subsystem (git log --oneline -- ...).
>>> The final prefix should be "qcom,sc7280-lpasscc" and then the actual
>                                  ^^^^^^^ it's written here
>
>>> subject should drop redundant pieces.
>> Sorry. I didn't understand much from your statement.
>>
>> Do you mean subject should something like below?
>>
>>    dt-bindings: clock: qcom: sc7280-lpasscc: Add resets for audio clock
>> controller
> 1. The last prefix should be "qcom,sc7280-lpasscc:".
> 2. And then drop "audio clock controller" because it is obvious, isn't it?
Okay!. Thanks for clarifying. I will change accordingly. Actually I 
followed previous similar commits.
>
>>>>    ...
>>> Best regards,
>>> Krzysztof
>>>
> Best regards,
> Krzysztof
>

[PATCH 2/2] clk: qcom: lpasscc: Add resets for SC7280 audioreach clock controller

[Srinivasa Rao Mandadapu]

The clock gating control for TX/RX/WSA core bus clocks would be required
to be reset(moved from hardware control) from audio core driver. Thus
add the support for the reset clocks in audioreach based clock driver.

Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
---
 drivers/clk/qcom/lpasscc-sc7280.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/clk/qcom/lpasscc-sc7280.c b/drivers/clk/qcom/lpasscc-sc7280.c
index 5c1e17b..d81d81b 100644
--- a/drivers/clk/qcom/lpasscc-sc7280.c
+++ b/drivers/clk/qcom/lpasscc-sc7280.c
@@ -12,10 +12,12 @@
 #include <linux/regmap.h>
 
 #include <dt-bindings/clock/qcom,lpass-sc7280.h>
+#include <dt-bindings/clock/qcom,lpassaudiocc-sc7280.h>
 
 #include "clk-regmap.h"
 #include "clk-branch.h"
 #include "common.h"
+#include "reset.h"
 
 static struct clk_branch lpass_top_cc_lpi_q6_axim_hs_clk = {
 	.halt_reg = 0x0,
@@ -102,6 +104,18 @@ static const struct qcom_cc_desc lpass_qdsp6ss_sc7280_desc = {
 	.num_clks = ARRAY_SIZE(lpass_qdsp6ss_sc7280_clocks),
 };
 
+static const struct qcom_reset_map lpass_cc_sc7280_resets[] = {
+	[LPASS_AUDIO_SWR_RX_CGCR] =  { 0xa0, 1 },
+	[LPASS_AUDIO_SWR_TX_CGCR] =  { 0xa8, 1 },
+	[LPASS_AUDIO_SWR_WSA_CGCR] = { 0xb0, 1 },
+};
+
+static const struct qcom_cc_desc lpass_audio_cc_reset_sc7280_desc = {
+	.config = &lpass_regmap_config,
+	.resets = lpass_cc_sc7280_resets,
+	.num_resets = ARRAY_SIZE(lpass_cc_sc7280_resets),
+};
+
 static int lpass_cc_sc7280_probe(struct platform_device *pdev)
 {
 	const struct qcom_cc_desc *desc;
@@ -132,6 +146,10 @@ static int lpass_cc_sc7280_probe(struct platform_device *pdev)
 	if (ret)
 		goto destroy_pm_clk;
 
+	ret = qcom_cc_probe_by_index(pdev, 1, &lpass_audio_cc_reset_sc7280_desc);
+	if (ret)
+		goto destroy_pm_clk;
+
 	return 0;
 
 destroy_pm_clk:
-- 
2.7.4

Re: [PATCH 2/2] clk: qcom: lpasscc: Add resets for SC7280 audioreach clock controller

[Krzysztof Kozlowski]

On 21/12/2022 11:21, Srinivasa Rao Mandadapu wrote:
> The clock gating control for TX/RX/WSA core bus clocks would be required
> to be reset(moved from hardware control) from audio core driver. Thus
> add the support for the reset clocks in audioreach based clock driver.
> 
> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
> ---
>  drivers/clk/qcom/lpasscc-sc7280.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/clk/qcom/lpasscc-sc7280.c b/drivers/clk/qcom/lpasscc-sc7280.c
> index 5c1e17b..d81d81b 100644
> --- a/drivers/clk/qcom/lpasscc-sc7280.c
> +++ b/drivers/clk/qcom/lpasscc-sc7280.c
> @@ -12,10 +12,12 @@
>  #include <linux/regmap.h>
>  
>  #include <dt-bindings/clock/qcom,lpass-sc7280.h>
> +#include <dt-bindings/clock/qcom,lpassaudiocc-sc7280.h>

These are bindings for different device.

>  
>  #include "clk-regmap.h"
>  #include "clk-branch.h"
>  #include "common.h"
> +#include "reset.h"
>  
>  static struct clk_branch lpass_top_cc_lpi_q6_axim_hs_clk = {
>  	.halt_reg = 0x0,
> @@ -102,6 +104,18 @@ static const struct qcom_cc_desc lpass_qdsp6ss_sc7280_desc = {
>  	.num_clks = ARRAY_SIZE(lpass_qdsp6ss_sc7280_clocks),
>  };
>  
> +static const struct qcom_reset_map lpass_cc_sc7280_resets[] = {
> +	[LPASS_AUDIO_SWR_RX_CGCR] =  { 0xa0, 1 },
> +	[LPASS_AUDIO_SWR_TX_CGCR] =  { 0xa8, 1 },
> +	[LPASS_AUDIO_SWR_WSA_CGCR] = { 0xb0, 1 },

These are example the same - IDs and values - as
qcom,sc7280-lpassaudiocc. Aren't you duplicating same control?

Best regards,
Krzysztof

Re: [PATCH 2/2] clk: qcom: lpasscc: Add resets for SC7280 audioreach clock controller

[Srinivasa Rao Mandadapu]

On 12/21/2022 4:09 PM, Krzysztof Kozlowski wrote:
Thanks for your time Krzysztof!!!
> On 21/12/2022 11:21, Srinivasa Rao Mandadapu wrote:
>> The clock gating control for TX/RX/WSA core bus clocks would be required
>> to be reset(moved from hardware control) from audio core driver. Thus
>> add the support for the reset clocks in audioreach based clock driver.
>>
>> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
>> ---
>>   drivers/clk/qcom/lpasscc-sc7280.c | 18 ++++++++++++++++++
>>   1 file changed, 18 insertions(+)
>>
>> diff --git a/drivers/clk/qcom/lpasscc-sc7280.c b/drivers/clk/qcom/lpasscc-sc7280.c
>> index 5c1e17b..d81d81b 100644
>> --- a/drivers/clk/qcom/lpasscc-sc7280.c
>> +++ b/drivers/clk/qcom/lpasscc-sc7280.c
>> @@ -12,10 +12,12 @@
>>   #include <linux/regmap.h>
>>   
>>   #include <dt-bindings/clock/qcom,lpass-sc7280.h>
>> +#include <dt-bindings/clock/qcom,lpassaudiocc-sc7280.h>
> These are bindings for different device.

They are not exactly for different device. It's for same device with 
ADSP enabled platforms.

Basically lpassaudiocc-sc7280.c and lpasscorecc-sc7280.c are for legacy 
path.

lpasscc-sc7280.c is for ADSP based AudioReach Solution.

>
>>   
>>   #include "clk-regmap.h"
>>   #include "clk-branch.h"
>>   #include "common.h"
>> +#include "reset.h"
>>   
>>   static struct clk_branch lpass_top_cc_lpi_q6_axim_hs_clk = {
>>   	.halt_reg = 0x0,
>> @@ -102,6 +104,18 @@ static const struct qcom_cc_desc lpass_qdsp6ss_sc7280_desc = {
>>   	.num_clks = ARRAY_SIZE(lpass_qdsp6ss_sc7280_clocks),
>>   };
>>   
>> +static const struct qcom_reset_map lpass_cc_sc7280_resets[] = {
>> +	[LPASS_AUDIO_SWR_RX_CGCR] =  { 0xa0, 1 },
>> +	[LPASS_AUDIO_SWR_TX_CGCR] =  { 0xa8, 1 },
>> +	[LPASS_AUDIO_SWR_WSA_CGCR] = { 0xb0, 1 },
> These are example the same - IDs and values - as
> qcom,sc7280-lpassaudiocc. Aren't you duplicating same control?

As explained above legacy path drivers and ADSP path drivers are 
enabled/used exclusively,

adding reset controls here.

>
> Best regards,
> Krzysztof
>

Re: [PATCH 2/2] clk: qcom: lpasscc: Add resets for SC7280 audioreach clock controller

[Krzysztof Kozlowski]

On 21/12/2022 14:18, Srinivasa Rao Mandadapu wrote:
> 
> On 12/21/2022 4:09 PM, Krzysztof Kozlowski wrote:
> Thanks for your time Krzysztof!!!
>> On 21/12/2022 11:21, Srinivasa Rao Mandadapu wrote:
>>> The clock gating control for TX/RX/WSA core bus clocks would be required
>>> to be reset(moved from hardware control) from audio core driver. Thus
>>> add the support for the reset clocks in audioreach based clock driver.
>>>
>>> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
>>> ---
>>>   drivers/clk/qcom/lpasscc-sc7280.c | 18 ++++++++++++++++++
>>>   1 file changed, 18 insertions(+)
>>>
>>> diff --git a/drivers/clk/qcom/lpasscc-sc7280.c b/drivers/clk/qcom/lpasscc-sc7280.c
>>> index 5c1e17b..d81d81b 100644
>>> --- a/drivers/clk/qcom/lpasscc-sc7280.c
>>> +++ b/drivers/clk/qcom/lpasscc-sc7280.c
>>> @@ -12,10 +12,12 @@
>>>   #include <linux/regmap.h>
>>>   
>>>   #include <dt-bindings/clock/qcom,lpass-sc7280.h>
>>> +#include <dt-bindings/clock/qcom,lpassaudiocc-sc7280.h>
>> These are bindings for different device.
> 
> They are not exactly for different device. It's for same device with 
> ADSP enabled platforms.
> 
> Basically lpassaudiocc-sc7280.c and lpasscorecc-sc7280.c are for legacy 
> path.
> 
> lpasscc-sc7280.c is for ADSP based AudioReach Solution.

I see two different devices:
lpasscc@3000000
clock-controller@3300000

clock inputs and outputs are different, so it does not look like for
same device.

Best regards,
Krzysztof

Re: [PATCH 2/2] clk: qcom: lpasscc: Add resets for SC7280 audioreach clock controller

[Srinivasa Rao Mandadapu]

On 12/22/2022 4:14 PM, Krzysztof Kozlowski wrote:
Thanks for Your Time Krzyszto!!!
> On 21/12/2022 14:18, Srinivasa Rao Mandadapu wrote:
>> On 12/21/2022 4:09 PM, Krzysztof Kozlowski wrote:
>> Thanks for your time Krzysztof!!!
>>> On 21/12/2022 11:21, Srinivasa Rao Mandadapu wrote:
>>>> The clock gating control for TX/RX/WSA core bus clocks would be required
>>>> to be reset(moved from hardware control) from audio core driver. Thus
>>>> add the support for the reset clocks in audioreach based clock driver.
>>>>
>>>> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
>>>> ---
>>>>    drivers/clk/qcom/lpasscc-sc7280.c | 18 ++++++++++++++++++
>>>>    1 file changed, 18 insertions(+)
>>>>
>>>> diff --git a/drivers/clk/qcom/lpasscc-sc7280.c b/drivers/clk/qcom/lpasscc-sc7280.c
>>>> index 5c1e17b..d81d81b 100644
>>>> --- a/drivers/clk/qcom/lpasscc-sc7280.c
>>>> +++ b/drivers/clk/qcom/lpasscc-sc7280.c
>>>> @@ -12,10 +12,12 @@
>>>>    #include <linux/regmap.h>
>>>>    
>>>>    #include <dt-bindings/clock/qcom,lpass-sc7280.h>
>>>> +#include <dt-bindings/clock/qcom,lpassaudiocc-sc7280.h>
>>> These are bindings for different device.
>> They are not exactly for different device. It's for same device with
>> ADSP enabled platforms.
>>
>> Basically lpassaudiocc-sc7280.c and lpasscorecc-sc7280.c are for legacy
>> path.
>>
>> lpasscc-sc7280.c is for ADSP based AudioReach Solution.
> I see two different devices:
> lpasscc@3000000
> clock-controller@3300000
>
> clock inputs and outputs are different, so it does not look like for
> same device.

Actually, even though there are 2 different device nodes, they are being 
used exclusively.

In ADSP enabled path(on same sc7280 based platform it's enabled for some 
vendors) only lpasscc node is being used

and legacy path nodes are being disabled due to register region conflicts.

Below is the patch for the same:
https://patchwork.kernel.org/project/linux-arm-msm/patch/1671702170-24781-8-git-send-email-quic_srivasam@quicinc.com/

>
> Best regards,
> Krzysztof
>