diff mbox series

[5/6] arm64: dts: qcom: sm8250: move wcd938x node out of soc node

Message ID 20220328143035.519909-6-vkoul@kernel.org (mailing list archive)
State Superseded
Headers show
Series arm64: dts: qcom: Get rid of some warnings | expand

Commit Message

Vinod Koul March 28, 2022, 2:30 p.m. UTC
The soc node expects all the nodes to have unit addresses. The wcd codec
node does not have that which causes warnings:

arch/arm64/boot/dts/qcom/sm8250-mtp.dts:631.17-648.4:
Warning (simple_bus_reg): /soc@0/codec: missing or empty reg/ranges property

Move wcd node out of soc to fix this

Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 arch/arm64/boot/dts/qcom/sm8250-mtp.dts | 40 ++++++++++++-------------
 1 file changed, 19 insertions(+), 21 deletions(-)

Comments

Krzysztof Kozlowski March 28, 2022, 3:21 p.m. UTC | #1
On 28/03/2022 16:30, Vinod Koul wrote:
> The soc node expects all the nodes to have unit addresses. The wcd codec
> node does not have that which causes warnings:
> 
> arch/arm64/boot/dts/qcom/sm8250-mtp.dts:631.17-648.4:
> Warning (simple_bus_reg): /soc@0/codec: missing or empty reg/ranges property
> 
> Move wcd node out of soc to fix this
> 
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> ---
>  arch/arm64/boot/dts/qcom/sm8250-mtp.dts | 40 ++++++++++++-------------
>  1 file changed, 19 insertions(+), 21 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8250-mtp.dts b/arch/arm64/boot/dts/qcom/sm8250-mtp.dts
> index fb99cc2827c7..3876a94b49a9 100644
> --- a/arch/arm64/boot/dts/qcom/sm8250-mtp.dts
> +++ b/arch/arm64/boot/dts/qcom/sm8250-mtp.dts
> @@ -156,6 +156,25 @@ vreg_s6c_0p88: smpc6-regulator {
>  		regulator-always-on;
>  		vin-supply = <&vph_pwr>;
>  	};
> +
> +	wcd938x: codec {

This probably should be under "sound" node. Anyway having it under soc
seemed incorrect.

I actually wonder where this wcd9380 sits? What type of bus?


> +		compatible = "qcom,wcd9380-codec";
> +		#sound-dai-cells = <1>;
> +		reset-gpios = <&tlmm 32 0>;
> +		vdd-buck-supply = <&vreg_s4a_1p8>;
> +		vdd-rxtx-supply = <&vreg_s4a_1p8>;
> +		vdd-io-supply = <&vreg_s4a_1p8>;
> +		vdd-mic-bias-supply = <&vreg_bob>;
> +		qcom,micbias1-microvolt = <1800000>;
> +		qcom,micbias2-microvolt = <1800000>;
> +		qcom,micbias3-microvolt = <1800000>;
> +		qcom,micbias4-microvolt = <1800000>;
> +		qcom,mbhc-buttons-vthreshold-microvolt = <75000 150000 237000 500000 500000 500000 500000 500000>;
> +		qcom,mbhc-headset-vthreshold-microvolt = <1700000>;
> +		qcom,mbhc-headphone-vthreshold-microvolt = <50000>;
> +		qcom,rx-device = <&wcd_rx>;
> +		qcom,tx-device = <&wcd_tx>;
> +	};
>  };
>  
>  &adsp {
> @@ -627,27 +646,6 @@ &slpi {
>  	firmware-name = "qcom/sm8250/slpi.mbn";
>  };
>  
> -&soc {
> -	wcd938x: codec {
> -		compatible = "qcom,wcd9380-codec";
> -		#sound-dai-cells = <1>;
> -		reset-gpios = <&tlmm 32 0>;
> -		vdd-buck-supply = <&vreg_s4a_1p8>;
> -		vdd-rxtx-supply = <&vreg_s4a_1p8>;
> -		vdd-io-supply = <&vreg_s4a_1p8>;
> -		vdd-mic-bias-supply = <&vreg_bob>;
> -		qcom,micbias1-microvolt = <1800000>;
> -		qcom,micbias2-microvolt = <1800000>;
> -		qcom,micbias3-microvolt = <1800000>;
> -		qcom,micbias4-microvolt = <1800000>;
> -		qcom,mbhc-buttons-vthreshold-microvolt = <75000 150000 237000 500000 500000 500000 500000 500000>;
> -		qcom,mbhc-headset-vthreshold-microvolt = <1700000>;
> -		qcom,mbhc-headphone-vthreshold-microvolt = <50000>;
> -		qcom,rx-device = <&wcd_rx>;
> -		qcom,tx-device = <&wcd_tx>;
> -	};
> -};
> -
>  &sound {
>  	compatible = "qcom,sm8250-sndcard";
>  	model = "SM8250-MTP-WCD9380-WSA8810-VA-DMIC";


Best regards,
Krzysztof
Vinod Koul March 28, 2022, 5:13 p.m. UTC | #2
On 28-03-22, 17:21, Krzysztof Kozlowski wrote:
> On 28/03/2022 16:30, Vinod Koul wrote:
> > The soc node expects all the nodes to have unit addresses. The wcd codec
> > node does not have that which causes warnings:
> > 
> > arch/arm64/boot/dts/qcom/sm8250-mtp.dts:631.17-648.4:
> > Warning (simple_bus_reg): /soc@0/codec: missing or empty reg/ranges property
> > 
> > Move wcd node out of soc to fix this
> > 
> > Signed-off-by: Vinod Koul <vkoul@kernel.org>
> > ---
> >  arch/arm64/boot/dts/qcom/sm8250-mtp.dts | 40 ++++++++++++-------------
> >  1 file changed, 19 insertions(+), 21 deletions(-)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/sm8250-mtp.dts b/arch/arm64/boot/dts/qcom/sm8250-mtp.dts
> > index fb99cc2827c7..3876a94b49a9 100644
> > --- a/arch/arm64/boot/dts/qcom/sm8250-mtp.dts
> > +++ b/arch/arm64/boot/dts/qcom/sm8250-mtp.dts
> > @@ -156,6 +156,25 @@ vreg_s6c_0p88: smpc6-regulator {
> >  		regulator-always-on;
> >  		vin-supply = <&vph_pwr>;
> >  	};
> > +
> > +	wcd938x: codec {
> 
> This probably should be under "sound" node. Anyway having it under soc
> seemed incorrect.

yeah it might make sense to be under sound. I think this is a slimbus
codec (right Srini..?) and this should be under slim node..

> 
> I actually wonder where this wcd9380 sits? What type of bus?
> 
> 
> > +		compatible = "qcom,wcd9380-codec";
> > +		#sound-dai-cells = <1>;
> > +		reset-gpios = <&tlmm 32 0>;
> > +		vdd-buck-supply = <&vreg_s4a_1p8>;
> > +		vdd-rxtx-supply = <&vreg_s4a_1p8>;
> > +		vdd-io-supply = <&vreg_s4a_1p8>;
> > +		vdd-mic-bias-supply = <&vreg_bob>;
> > +		qcom,micbias1-microvolt = <1800000>;
> > +		qcom,micbias2-microvolt = <1800000>;
> > +		qcom,micbias3-microvolt = <1800000>;
> > +		qcom,micbias4-microvolt = <1800000>;
> > +		qcom,mbhc-buttons-vthreshold-microvolt = <75000 150000 237000 500000 500000 500000 500000 500000>;
> > +		qcom,mbhc-headset-vthreshold-microvolt = <1700000>;
> > +		qcom,mbhc-headphone-vthreshold-microvolt = <50000>;
> > +		qcom,rx-device = <&wcd_rx>;
> > +		qcom,tx-device = <&wcd_tx>;
> > +	};
> >  };
> >  
> >  &adsp {
> > @@ -627,27 +646,6 @@ &slpi {
> >  	firmware-name = "qcom/sm8250/slpi.mbn";
> >  };
> >  
> > -&soc {
> > -	wcd938x: codec {
> > -		compatible = "qcom,wcd9380-codec";
> > -		#sound-dai-cells = <1>;
> > -		reset-gpios = <&tlmm 32 0>;
> > -		vdd-buck-supply = <&vreg_s4a_1p8>;
> > -		vdd-rxtx-supply = <&vreg_s4a_1p8>;
> > -		vdd-io-supply = <&vreg_s4a_1p8>;
> > -		vdd-mic-bias-supply = <&vreg_bob>;
> > -		qcom,micbias1-microvolt = <1800000>;
> > -		qcom,micbias2-microvolt = <1800000>;
> > -		qcom,micbias3-microvolt = <1800000>;
> > -		qcom,micbias4-microvolt = <1800000>;
> > -		qcom,mbhc-buttons-vthreshold-microvolt = <75000 150000 237000 500000 500000 500000 500000 500000>;
> > -		qcom,mbhc-headset-vthreshold-microvolt = <1700000>;
> > -		qcom,mbhc-headphone-vthreshold-microvolt = <50000>;
> > -		qcom,rx-device = <&wcd_rx>;
> > -		qcom,tx-device = <&wcd_tx>;
> > -	};
> > -};
> > -
> >  &sound {
> >  	compatible = "qcom,sm8250-sndcard";
> >  	model = "SM8250-MTP-WCD9380-WSA8810-VA-DMIC";
> 
> 
> Best regards,
> Krzysztof
Srinivas Kandagatla March 29, 2022, 8:44 a.m. UTC | #3
On 28/03/2022 18:13, Vinod Koul wrote:
> On 28-03-22, 17:21, Krzysztof Kozlowski wrote:
>> On 28/03/2022 16:30, Vinod Koul wrote:
>>> The soc node expects all the nodes to have unit addresses. The wcd codec
>>> node does not have that which causes warnings:
>>>
>>> arch/arm64/boot/dts/qcom/sm8250-mtp.dts:631.17-648.4:
>>> Warning (simple_bus_reg): /soc@0/codec: missing or empty reg/ranges property
>>>
>>> Move wcd node out of soc to fix this
>>>
>>> Signed-off-by: Vinod Koul <vkoul@kernel.org>
>>> ---
>>>   arch/arm64/boot/dts/qcom/sm8250-mtp.dts | 40 ++++++++++++-------------
>>>   1 file changed, 19 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/sm8250-mtp.dts b/arch/arm64/boot/dts/qcom/sm8250-mtp.dts
>>> index fb99cc2827c7..3876a94b49a9 100644
>>> --- a/arch/arm64/boot/dts/qcom/sm8250-mtp.dts
>>> +++ b/arch/arm64/boot/dts/qcom/sm8250-mtp.dts
>>> @@ -156,6 +156,25 @@ vreg_s6c_0p88: smpc6-regulator {
>>>   		regulator-always-on;
>>>   		vin-supply = <&vph_pwr>;
>>>   	};
>>> +
>>> +	wcd938x: codec {
>>
>> This probably should be under "sound" node. Anyway having it under soc
>> seemed incorrect.
> 
> yeah it might make sense to be under sound. I think this is a slimbus
> codec (right Srini..?) and this should be under slim node..

wcd938x can be moved out of soc node but not under sound as it is.

If we plan to move external codecs under sound node then sound node has 
to be converted in to a simple-bus I guess. If we do that we have to 
make sure that we are consistent across all the qcom dts. This does 
sound correct either.

Currently sound node is only used for sound-card, sound card uses LPASS 
IP which is part of SoC along with external or internal codecs.

I am not 100% sure moving aggregate devices like sound card which uses 
SoC components along with external components out of soc node is the 
right choice.

Moving sound out of soc node might also add some regressions as sound 
device is sometimes used to allocate dma memory, so we have to be 
careful with this move.

The reason why sound node is empty in SoC is because the wiring of dais 
are board specific. We could add compatible string to soc sound node if 
that helps clear some confusion.
> 
>>
>> I actually wonder where this wcd9380 sits? What type of bus?

WCD938x codec has two parts wcd938x-tx and wcd938x-rx which are under 
there respective SoundWire bus.

We can not move wcd938x-tx and wcd938x-rx out of there bus nodes which 
result with no device enumeration.

--srini



>>
>>
>>> +		compatible = "qcom,wcd9380-codec";
>>> +		#sound-dai-cells = <1>;
>>> +		reset-gpios = <&tlmm 32 0>;
>>> +		vdd-buck-supply = <&vreg_s4a_1p8>;
>>> +		vdd-rxtx-supply = <&vreg_s4a_1p8>;
>>> +		vdd-io-supply = <&vreg_s4a_1p8>;
>>> +		vdd-mic-bias-supply = <&vreg_bob>;
>>> +		qcom,micbias1-microvolt = <1800000>;
>>> +		qcom,micbias2-microvolt = <1800000>;
>>> +		qcom,micbias3-microvolt = <1800000>;
>>> +		qcom,micbias4-microvolt = <1800000>;
>>> +		qcom,mbhc-buttons-vthreshold-microvolt = <75000 150000 237000 500000 500000 500000 500000 500000>;
>>> +		qcom,mbhc-headset-vthreshold-microvolt = <1700000>;
>>> +		qcom,mbhc-headphone-vthreshold-microvolt = <50000>;
>>> +		qcom,rx-device = <&wcd_rx>;
>>> +		qcom,tx-device = <&wcd_tx>;
>>> +	};
>>>   };
>>>   
>>>   &adsp {
>>> @@ -627,27 +646,6 @@ &slpi {
>>>   	firmware-name = "qcom/sm8250/slpi.mbn";
>>>   };
>>>   
>>> -&soc {
>>> -	wcd938x: codec {
>>> -		compatible = "qcom,wcd9380-codec";
>>> -		#sound-dai-cells = <1>;
>>> -		reset-gpios = <&tlmm 32 0>;
>>> -		vdd-buck-supply = <&vreg_s4a_1p8>;
>>> -		vdd-rxtx-supply = <&vreg_s4a_1p8>;
>>> -		vdd-io-supply = <&vreg_s4a_1p8>;
>>> -		vdd-mic-bias-supply = <&vreg_bob>;
>>> -		qcom,micbias1-microvolt = <1800000>;
>>> -		qcom,micbias2-microvolt = <1800000>;
>>> -		qcom,micbias3-microvolt = <1800000>;
>>> -		qcom,micbias4-microvolt = <1800000>;
>>> -		qcom,mbhc-buttons-vthreshold-microvolt = <75000 150000 237000 500000 500000 500000 500000 500000>;
>>> -		qcom,mbhc-headset-vthreshold-microvolt = <1700000>;
>>> -		qcom,mbhc-headphone-vthreshold-microvolt = <50000>;
>>> -		qcom,rx-device = <&wcd_rx>;
>>> -		qcom,tx-device = <&wcd_tx>;
>>> -	};
>>> -};
>>> -
>>>   &sound {
>>>   	compatible = "qcom,sm8250-sndcard";
>>>   	model = "SM8250-MTP-WCD9380-WSA8810-VA-DMIC";
>>
>>
>> Best regards,
>> Krzysztof
>
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/sm8250-mtp.dts b/arch/arm64/boot/dts/qcom/sm8250-mtp.dts
index fb99cc2827c7..3876a94b49a9 100644
--- a/arch/arm64/boot/dts/qcom/sm8250-mtp.dts
+++ b/arch/arm64/boot/dts/qcom/sm8250-mtp.dts
@@ -156,6 +156,25 @@  vreg_s6c_0p88: smpc6-regulator {
 		regulator-always-on;
 		vin-supply = <&vph_pwr>;
 	};
+
+	wcd938x: codec {
+		compatible = "qcom,wcd9380-codec";
+		#sound-dai-cells = <1>;
+		reset-gpios = <&tlmm 32 0>;
+		vdd-buck-supply = <&vreg_s4a_1p8>;
+		vdd-rxtx-supply = <&vreg_s4a_1p8>;
+		vdd-io-supply = <&vreg_s4a_1p8>;
+		vdd-mic-bias-supply = <&vreg_bob>;
+		qcom,micbias1-microvolt = <1800000>;
+		qcom,micbias2-microvolt = <1800000>;
+		qcom,micbias3-microvolt = <1800000>;
+		qcom,micbias4-microvolt = <1800000>;
+		qcom,mbhc-buttons-vthreshold-microvolt = <75000 150000 237000 500000 500000 500000 500000 500000>;
+		qcom,mbhc-headset-vthreshold-microvolt = <1700000>;
+		qcom,mbhc-headphone-vthreshold-microvolt = <50000>;
+		qcom,rx-device = <&wcd_rx>;
+		qcom,tx-device = <&wcd_tx>;
+	};
 };
 
 &adsp {
@@ -627,27 +646,6 @@  &slpi {
 	firmware-name = "qcom/sm8250/slpi.mbn";
 };
 
-&soc {
-	wcd938x: codec {
-		compatible = "qcom,wcd9380-codec";
-		#sound-dai-cells = <1>;
-		reset-gpios = <&tlmm 32 0>;
-		vdd-buck-supply = <&vreg_s4a_1p8>;
-		vdd-rxtx-supply = <&vreg_s4a_1p8>;
-		vdd-io-supply = <&vreg_s4a_1p8>;
-		vdd-mic-bias-supply = <&vreg_bob>;
-		qcom,micbias1-microvolt = <1800000>;
-		qcom,micbias2-microvolt = <1800000>;
-		qcom,micbias3-microvolt = <1800000>;
-		qcom,micbias4-microvolt = <1800000>;
-		qcom,mbhc-buttons-vthreshold-microvolt = <75000 150000 237000 500000 500000 500000 500000 500000>;
-		qcom,mbhc-headset-vthreshold-microvolt = <1700000>;
-		qcom,mbhc-headphone-vthreshold-microvolt = <50000>;
-		qcom,rx-device = <&wcd_rx>;
-		qcom,tx-device = <&wcd_tx>;
-	};
-};
-
 &sound {
 	compatible = "qcom,sm8250-sndcard";
 	model = "SM8250-MTP-WCD9380-WSA8810-VA-DMIC";