2023-05-25 12:32:37

by Srinivas Kandagatla

Subject: [PATCH v2 5/6] arm64: dts: qcom: sc8280xp: add resets for soundwire controllers

Soundwire controllers on sc8280xp needs an explicit reset, add
support for this.

Signed-off-by: Srinivas Kandagatla <[email protected]>
---
arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
index 6730349e34f4..39be2e89ce05 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
@@ -6,6 +6,7 @@

#include <dt-bindings/clock/qcom,dispcc-sc8280xp.h>
#include <dt-bindings/clock/qcom,gcc-sc8280xp.h>
+#include <dt-bindings/clock/qcom,lpasscc-sc8280xp.h>
#include <dt-bindings/clock/qcom,rpmh.h>
#include <dt-bindings/interconnect/qcom,osm-l3.h>
#include <dt-bindings/interconnect/qcom,sc8280xp.h>
@@ -2560,6 +2561,8 @@
interrupts = <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&rxmacro>;
clock-names = "iface";
+ resets = <&lpass_audiocc LPASS_AUDIO_SWR_RX_CGCR>;
+ reset-names = "swr_audio_cgcr";
label = "RX";

qcom,din-ports = <0>;
@@ -2634,6 +2637,8 @@
interrupts = <GIC_SPI 170 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&wsamacro>;
clock-names = "iface";
+ resets = <&lpass_audiocc LPASS_AUDIO_SWR_WSA_CGCR>;
+ reset-names = "swr_audio_cgcr";
label = "WSA";

qcom,din-ports = <2>;
@@ -2656,6 +2661,14 @@
status = "disabled";
};

+ lpass_audiocc: clock-controller@32a9000 {
+ compatible = "qcom,sc8280xp-lpassaudiocc";
+ reg = <0 0x032a9000 0 0x1000>;
+ qcom,adsp-pil-mode;
+ #clock-cells = <1>;
+ #reset-cells = <1>;
+ };
+
swr2: soundwire-controller@3330000 {
compatible = "qcom,soundwire-v1.6.0";
reg = <0 0x03330000 0 0x2000>;
@@ -2665,6 +2678,8 @@

clocks = <&txmacro>;
clock-names = "iface";
+ resets = <&lpasscc LPASS_AUDIO_SWR_TX_CGCR>;
+ reset-names = "swr_audio_cgcr";
label = "TX";
#sound-dai-cells = <1>;
#address-cells = <2>;
@@ -2901,6 +2916,14 @@
};
};

+ lpasscc: clock-controller@33e0000 {
+ compatible = "qcom,sc8280xp-lpasscc";
+ reg = <0 0x033e0000 0 0x12000>;
+ qcom,adsp-pil-mode;
+ #clock-cells = <1>;
+ #reset-cells = <1>;
+ };
+
usb_0_qmpphy: phy@88eb000 {
compatible = "qcom,sc8280xp-qmp-usb43dp-phy";
reg = <0 0x088eb000 0 0x4000>;
--
2.21.0

2023-05-26 12:30:17

by Johan Hovold

Subject: Re: [PATCH v2 5/6] arm64: dts: qcom: sc8280xp: add resets for soundwire controllers

On Thu, May 25, 2023 at 01:29:29PM +0100, Srinivas Kandagatla wrote:
> Soundwire controllers on sc8280xp needs an explicit reset, add
> support for this.
>
> Signed-off-by: Srinivas Kandagatla <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> index 6730349e34f4..39be2e89ce05 100644
> --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi

> @@ -2901,6 +2916,14 @@
> };
> };
>
> + lpasscc: clock-controller@33e0000 {
> + compatible = "qcom,sc8280xp-lpasscc";
> + reg = <0 0x033e0000 0 0x12000>;
> + qcom,adsp-pil-mode;
> + #clock-cells = <1>;
> + #reset-cells = <1>;
> + };
> +
> usb_0_qmpphy: phy@88eb000 {
> compatible = "qcom,sc8280xp-qmp-usb43dp-phy";
> reg = <0 0x088eb000 0 0x4000>;

This one needs to be rebased on linux-next as the lpasscc node should go
before the new:

sdc2: mmc@8804000 {

node.

Looks good otherwise:

Reviewed-by: Johan Hovold <[email protected]>

Johan

2023-05-31 20:17:27

by Krzysztof Kozlowski

Subject: Re: [PATCH v2 5/6] arm64: dts: qcom: sc8280xp: add resets for soundwire controllers

On 25/05/2023 14:29, Srinivas Kandagatla wrote:
> Soundwire controllers on sc8280xp needs an explicit reset, add
> support for this.
>
> Signed-off-by: Srinivas Kandagatla <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> index 6730349e34f4..39be2e89ce05 100644
> --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> @@ -6,6 +6,7 @@
>
> #include <dt-bindings/clock/qcom,dispcc-sc8280xp.h>
> #include <dt-bindings/clock/qcom,gcc-sc8280xp.h>
> +#include <dt-bindings/clock/qcom,lpasscc-sc8280xp.h>
> #include <dt-bindings/clock/qcom,rpmh.h>
> #include <dt-bindings/interconnect/qcom,osm-l3.h>
> #include <dt-bindings/interconnect/qcom,sc8280xp.h>
> @@ -2560,6 +2561,8 @@
> interrupts = <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
> clocks = <&rxmacro>;
> clock-names = "iface";
> + resets = <&lpass_audiocc LPASS_AUDIO_SWR_RX_CGCR>;
> + reset-names = "swr_audio_cgcr";
> label = "RX";
>
> qcom,din-ports = <0>;
> @@ -2634,6 +2637,8 @@
> interrupts = <GIC_SPI 170 IRQ_TYPE_LEVEL_HIGH>;
> clocks = <&wsamacro>;
> clock-names = "iface";
> + resets = <&lpass_audiocc LPASS_AUDIO_SWR_WSA_CGCR>;
> + reset-names = "swr_audio_cgcr";
> label = "WSA";
>
> qcom,din-ports = <2>;
> @@ -2656,6 +2661,14 @@
> status = "disabled";
> };
>
> + lpass_audiocc: clock-controller@32a9000 {
> + compatible = "qcom,sc8280xp-lpassaudiocc";
> + reg = <0 0x032a9000 0 0x1000>;
> + qcom,adsp-pil-mode;

Here and...

> + #clock-cells = <1>;
> + #reset-cells = <1>;
> + };
> +
> swr2: soundwire-controller@3330000 {
> compatible = "qcom,soundwire-v1.6.0";
> reg = <0 0x03330000 0 0x2000>;
> @@ -2665,6 +2678,8 @@
>
> clocks = <&txmacro>;
> clock-names = "iface";
> + resets = <&lpasscc LPASS_AUDIO_SWR_TX_CGCR>;
> + reset-names = "swr_audio_cgcr";
> label = "TX";
> #sound-dai-cells = <1>;
> #address-cells = <2>;
> @@ -2901,6 +2916,14 @@
> };
> };
>
> + lpasscc: clock-controller@33e0000 {
> + compatible = "qcom,sc8280xp-lpasscc";
> + reg = <0 0x033e0000 0 0x12000>;
> + qcom,adsp-pil-mode;

... here - where do you use these properties? Driver ignores them.

AFAIR, these were only for sc7280 where we supported two variants for audio.

Best regards,
Krzysztof

2023-05-31 20:35:42

by Konrad Dybcio

Subject: Re: [PATCH v2 5/6] arm64: dts: qcom: sc8280xp: add resets for soundwire controllers

On 31.05.2023 22:03, Krzysztof Kozlowski wrote:
> On 25/05/2023 14:29, Srinivas Kandagatla wrote:
>> Soundwire controllers on sc8280xp needs an explicit reset, add
>> support for this.
>>
>> Signed-off-by: Srinivas Kandagatla <[email protected]>
>> ---
>> arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 23 +++++++++++++++++++++++
>> 1 file changed, 23 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
>> index 6730349e34f4..39be2e89ce05 100644
>> --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
>> @@ -6,6 +6,7 @@
>>
>> #include <dt-bindings/clock/qcom,dispcc-sc8280xp.h>
>> #include <dt-bindings/clock/qcom,gcc-sc8280xp.h>
>> +#include <dt-bindings/clock/qcom,lpasscc-sc8280xp.h>
>> #include <dt-bindings/clock/qcom,rpmh.h>
>> #include <dt-bindings/interconnect/qcom,osm-l3.h>
>> #include <dt-bindings/interconnect/qcom,sc8280xp.h>
>> @@ -2560,6 +2561,8 @@
>> interrupts = <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
>> clocks = <&rxmacro>;
>> clock-names = "iface";
>> + resets = <&lpass_audiocc LPASS_AUDIO_SWR_RX_CGCR>;
>> + reset-names = "swr_audio_cgcr";
>> label = "RX";
>>
>> qcom,din-ports = <0>;
>> @@ -2634,6 +2637,8 @@
>> interrupts = <GIC_SPI 170 IRQ_TYPE_LEVEL_HIGH>;
>> clocks = <&wsamacro>;
>> clock-names = "iface";
>> + resets = <&lpass_audiocc LPASS_AUDIO_SWR_WSA_CGCR>;
>> + reset-names = "swr_audio_cgcr";
>> label = "WSA";
>>
>> qcom,din-ports = <2>;
>> @@ -2656,6 +2661,14 @@
>> status = "disabled";
>> };
>>
>> + lpass_audiocc: clock-controller@32a9000 {
>> + compatible = "qcom,sc8280xp-lpassaudiocc";
>> + reg = <0 0x032a9000 0 0x1000>;
>> + qcom,adsp-pil-mode;
>
> Here and...
>
>> + #clock-cells = <1>;
>> + #reset-cells = <1>;
>> + };
>> +
>> swr2: soundwire-controller@3330000 {
>> compatible = "qcom,soundwire-v1.6.0";
>> reg = <0 0x03330000 0 0x2000>;
>> @@ -2665,6 +2678,8 @@
>>
>> clocks = <&txmacro>;
>> clock-names = "iface";
>> + resets = <&lpasscc LPASS_AUDIO_SWR_TX_CGCR>;
>> + reset-names = "swr_audio_cgcr";
>> label = "TX";
>> #sound-dai-cells = <1>;
>> #address-cells = <2>;
>> @@ -2901,6 +2916,14 @@
>> };
>> };
>>
>> + lpasscc: clock-controller@33e0000 {
>> + compatible = "qcom,sc8280xp-lpasscc";
>> + reg = <0 0x033e0000 0 0x12000>;
>> + qcom,adsp-pil-mode;
>
> ... here - where do you use these properties? Driver ignores them.
>
> AFAIR, these were only for sc7280 where we supported two variants for audio.
Smells like pure copypasta - we don't support any 8280xp chromebooks (not
even sure if any exist) and WP devices have a normal TZ+PAS setup

Konrad
>
> Best regards,
> Krzysztof
>

2023-06-02 13:30:50

by Krzysztof Kozlowski

Subject: Re: [PATCH v2 5/6] arm64: dts: qcom: sc8280xp: add resets for soundwire controllers

Resending as my previous email probably got lost. If you got it twice,
apologies.

On 25/05/2023 14:29, Srinivas Kandagatla wrote:
> Soundwire controllers on sc8280xp needs an explicit reset, add
> support for this.
>
> Signed-off-by: Srinivas Kandagatla <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> index 6730349e34f4..39be2e89ce05 100644
> --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> @@ -6,6 +6,7 @@
>
> #include <dt-bindings/clock/qcom,dispcc-sc8280xp.h>
> #include <dt-bindings/clock/qcom,gcc-sc8280xp.h>
> +#include <dt-bindings/clock/qcom,lpasscc-sc8280xp.h>
> #include <dt-bindings/clock/qcom,rpmh.h>
> #include <dt-bindings/interconnect/qcom,osm-l3.h>
> #include <dt-bindings/interconnect/qcom,sc8280xp.h>
> @@ -2560,6 +2561,8 @@
> interrupts = <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
> clocks = <&rxmacro>;
> clock-names = "iface";
> + resets = <&lpass_audiocc LPASS_AUDIO_SWR_RX_CGCR>;
> + reset-names = "swr_audio_cgcr";
> label = "RX";
>
> qcom,din-ports = <0>;
> @@ -2634,6 +2637,8 @@
> interrupts = <GIC_SPI 170 IRQ_TYPE_LEVEL_HIGH>;
> clocks = <&wsamacro>;
> clock-names = "iface";
> + resets = <&lpass_audiocc LPASS_AUDIO_SWR_WSA_CGCR>;
> + reset-names = "swr_audio_cgcr";
> label = "WSA";
>
> qcom,din-ports = <2>;
> @@ -2656,6 +2661,14 @@
> status = "disabled";
> };
>
> + lpass_audiocc: clock-controller@32a9000 {
> + compatible = "qcom,sc8280xp-lpassaudiocc";
> + reg = <0 0x032a9000 0 0x1000>;
> + qcom,adsp-pil-mode;

Here and...

> + #clock-cells = <1>;
> + #reset-cells = <1>;
> + };
> +
> swr2: soundwire-controller@3330000 {
> compatible = "qcom,soundwire-v1.6.0";
> reg = <0 0x03330000 0 0x2000>;
> @@ -2665,6 +2678,8 @@
>
> clocks = <&txmacro>;
> clock-names = "iface";
> + resets = <&lpasscc LPASS_AUDIO_SWR_TX_CGCR>;
> + reset-names = "swr_audio_cgcr";
> label = "TX";
> #sound-dai-cells = <1>;
> #address-cells = <2>;
> @@ -2901,6 +2916,14 @@
> };
> };
>
> + lpasscc: clock-controller@33e0000 {
> + compatible = "qcom,sc8280xp-lpasscc";
> + reg = <0 0x033e0000 0 0x12000>;
> + qcom,adsp-pil-mode;

... here - where do you use these properties? Driver ignores them.

AFAIR, these were only for sc7280 where we supported two variants for audio.

Best regards,
Krzysztof

2023-06-07 14:16:02

by Srinivas Kandagatla

Subject: Re: [PATCH v2 5/6] arm64: dts: qcom: sc8280xp: add resets for soundwire controllers

On 31/05/2023 21:03, Krzysztof Kozlowski wrote:
> On 25/05/2023 14:29, Srinivas Kandagatla wrote:
>> Soundwire controllers on sc8280xp needs an explicit reset, add
>> support for this.
>>
>> Signed-off-by: Srinivas Kandagatla <[email protected]>
>> ---
>> arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 23 +++++++++++++++++++++++
>> 1 file changed, 23 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
>> index 6730349e34f4..39be2e89ce05 100644
>> --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
>> @@ -6,6 +6,7 @@
>>
>> #include <dt-bindings/clock/qcom,dispcc-sc8280xp.h>
>> #include <dt-bindings/clock/qcom,gcc-sc8280xp.h>
>> +#include <dt-bindings/clock/qcom,lpasscc-sc8280xp.h>
>> #include <dt-bindings/clock/qcom,rpmh.h>
>> #include <dt-bindings/interconnect/qcom,osm-l3.h>
>> #include <dt-bindings/interconnect/qcom,sc8280xp.h>
>> @@ -2560,6 +2561,8 @@
>> interrupts = <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
>> clocks = <&rxmacro>;
>> clock-names = "iface";
>> + resets = <&lpass_audiocc LPASS_AUDIO_SWR_RX_CGCR>;
>> + reset-names = "swr_audio_cgcr";
>> label = "RX";
>>
>> qcom,din-ports = <0>;
>> @@ -2634,6 +2637,8 @@
>> interrupts = <GIC_SPI 170 IRQ_TYPE_LEVEL_HIGH>;
>> clocks = <&wsamacro>;
>> clock-names = "iface";
>> + resets = <&lpass_audiocc LPASS_AUDIO_SWR_WSA_CGCR>;
>> + reset-names = "swr_audio_cgcr";
>> label = "WSA";
>>
>> qcom,din-ports = <2>;
>> @@ -2656,6 +2661,14 @@
>> status = "disabled";
>> };
>>
>> + lpass_audiocc: clock-controller@32a9000 {
>> + compatible = "qcom,sc8280xp-lpassaudiocc";
>> + reg = <0 0x032a9000 0 0x1000>;
>> + qcom,adsp-pil-mode;
>
> Here and...
>
>> + #clock-cells = <1>;
>> + #reset-cells = <1>;
>> + };
>> +
>> swr2: soundwire-controller@3330000 {
>> compatible = "qcom,soundwire-v1.6.0";
>> reg = <0 0x03330000 0 0x2000>;
>> @@ -2665,6 +2678,8 @@
>>
>> clocks = <&txmacro>;
>> clock-names = "iface";
>> + resets = <&lpasscc LPASS_AUDIO_SWR_TX_CGCR>;
>> + reset-names = "swr_audio_cgcr";
>> label = "TX";
>> #sound-dai-cells = <1>;
>> #address-cells = <2>;
>> @@ -2901,6 +2916,14 @@
>> };
>> };
>>
>> + lpasscc: clock-controller@33e0000 {
>> + compatible = "qcom,sc8280xp-lpasscc";
>> + reg = <0 0x033e0000 0 0x12000>;
>> + qcom,adsp-pil-mode;
>
> ... here - where do you use these properties? Driver ignores them.
Wil add a check in driver.

>
> AFAIR, these were only for sc7280 where we supported two variants for audio.
>
we can drop this property altogether, But having this flag will make
sure that the bindings are in a ready state to accommodate a new variant
of sc8280xp with dsp bypass.

--srini



> Best regards,
> Krzysztof
>