[PATCH 0/6] arm64: dts: qcom: Get rid of some warnings

[PATCH 0/6] arm64: dts: qcom: Get rid of some warnings

[Vinod Koul]

This attempts to make W=1 free from warnings for all SM* dts files.

Bunch of these are releated to node not having valid unit address and being
present under soc node. So moving it out fixed that. Interconnect node was
simple rename to remove unit address which was not really valid for these
nodes.

Vinod Koul (6):
  arm64: dts: qcom: sm8450: rename interconnect nodes
  arm64: dts: qcom: sm8350: move qup-opp-tables out of soc node
  arm64: dts: qcom: sm8250: move qup-opp-table out of soc node
  arm64: dts: qcom: sm8250: move sound node out of soc node
  arm64: dts: qcom: sm8250: move wcd938x node out of soc node
  arm64: dts: qcom: sm8250: remove address cells from dsi nodes

 arch/arm64/boot/dts/qcom/sm8250-mtp.dts | 40 +++++++------
 arch/arm64/boot/dts/qcom/sm8250.dtsi    | 50 +++++++---------
 arch/arm64/boot/dts/qcom/sm8350.dtsi    | 76 ++++++++++++-------------
 arch/arm64/boot/dts/qcom/sm8450.dtsi    |  4 +-
 4 files changed, 81 insertions(+), 89 deletions(-)

-- 
2.34.1

[PATCH 1/6] arm64: dts: qcom: sm8450: rename interconnect nodes

[Vinod Koul]

clk-virt and mc-virt interconnect nodes were named interconnect@0 and
interconnect@1. That is incorrect as we don't have unit addresses 0/1
for these node.

This causes warning:
arch/arm64/boot/dts/qcom/sm8450.dtsi:255.27-259.4:
	Warning (unit_address_vs_reg): /interconnect@0:
	node has a unit name, but no reg or ranges property
arch/arm64/boot/dts/qcom/sm8450.dtsi:261.26-265.4:
	Warning (unit_address_vs_reg): /interconnect@1:
	node has a unit name, but no reg or ranges property
arch/arm64/boot/dts/qcom/sm8450.dtsi:255.27-259.4:
	Warning (unique_unit_address_if_enabled): /interconnect@0:
	duplicate unit-address (also used in node /soc@0)

Rename the nodes to interconnect-0 and interconnect-1 to fix the
warning.

Signed-off-by: Vinod Koul <vkoul@kernel.org>
Suggested-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 arch/arm64/boot/dts/qcom/sm8450.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
index 934e29b9e153..b0128d115e7a 100644
--- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
@@ -252,13 +252,13 @@ scm: scm {
 		};
 	};
 
-	clk_virt: interconnect@0 {
+	clk_virt: interconnect-0 {
 		compatible = "qcom,sm8450-clk-virt";
 		#interconnect-cells = <2>;
 		qcom,bcm-voters = <&apps_bcm_voter>;
 	};
 
-	mc_virt: interconnect@1 {
+	mc_virt: interconnect-1 {
 		compatible = "qcom,sm8450-mc-virt";
 		#interconnect-cells = <2>;
 		qcom,bcm-voters = <&apps_bcm_voter>;
-- 
2.34.1

Re: [PATCH 1/6] arm64: dts: qcom: sm8450: rename interconnect nodes

[Krzysztof Kozlowski]

On 28/03/2022 16:30, Vinod Koul wrote:
> clk-virt and mc-virt interconnect nodes were named interconnect@0 and
> interconnect@1. That is incorrect as we don't have unit addresses 0/1
> for these node.
> 
> This causes warning:
> arch/arm64/boot/dts/qcom/sm8450.dtsi:255.27-259.4:
> 	Warning (unit_address_vs_reg): /interconnect@0:
> 	node has a unit name, but no reg or ranges property
> arch/arm64/boot/dts/qcom/sm8450.dtsi:261.26-265.4:
> 	Warning (unit_address_vs_reg): /interconnect@1:
> 	node has a unit name, but no reg or ranges property
> arch/arm64/boot/dts/qcom/sm8450.dtsi:255.27-259.4:
> 	Warning (unique_unit_address_if_enabled): /interconnect@0:
> 	duplicate unit-address (also used in node /soc@0)
> 
> Rename the nodes to interconnect-0 and interconnect-1 to fix the
> warning.
> 
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> Suggested-by: Krzysztof Kozlowski <krzk@kernel.org>
> ---
>  arch/arm64/boot/dts/qcom/sm8450.dtsi | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

The DT schema will still need to be fixed because it expects 'reg' address.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

Re: [PATCH 1/6] arm64: dts: qcom: sm8450: rename interconnect nodes

[Vinod Koul]

On 28-03-22, 17:07, Krzysztof Kozlowski wrote:
> On 28/03/2022 16:30, Vinod Koul wrote:
> > clk-virt and mc-virt interconnect nodes were named interconnect@0 and
> > interconnect@1. That is incorrect as we don't have unit addresses 0/1
> > for these node.
> > 
> > This causes warning:
> > arch/arm64/boot/dts/qcom/sm8450.dtsi:255.27-259.4:
> > 	Warning (unit_address_vs_reg): /interconnect@0:
> > 	node has a unit name, but no reg or ranges property
> > arch/arm64/boot/dts/qcom/sm8450.dtsi:261.26-265.4:
> > 	Warning (unit_address_vs_reg): /interconnect@1:
> > 	node has a unit name, but no reg or ranges property
> > arch/arm64/boot/dts/qcom/sm8450.dtsi:255.27-259.4:
> > 	Warning (unique_unit_address_if_enabled): /interconnect@0:
> > 	duplicate unit-address (also used in node /soc@0)
> > 
> > Rename the nodes to interconnect-0 and interconnect-1 to fix the
> > warning.
> > 
> > Signed-off-by: Vinod Koul <vkoul@kernel.org>
> > Suggested-by: Krzysztof Kozlowski <krzk@kernel.org>
> > ---
> >  arch/arm64/boot/dts/qcom/sm8450.dtsi | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> The DT schema will still need to be fixed because it expects 'reg' address.

Yes will update that as well..

> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Thanks

-- 
~Vinod

[PATCH 2/6] arm64: dts: qcom: sm8350: move qup-opp-tables out of soc node

[Vinod Koul]

The soc node expects all the nodes to have unit addresses. The
qup-opp-tables do not have that which causes warnings:

arch/arm64/boot/dts/qcom/sm8350.dtsi:640.46-657.5:
	Warning (simple_bus_reg): /soc@0/qup-100mhz-opp-table:
	missing or empty reg/ranges property
arch/arm64/boot/dts/qcom/sm8350.dtsi:659.46-676.5:
	Warning (simple_bus_reg): /soc@0/qup-120mhz-opp-table:
	missing or empty reg/ranges property

Move the qup-opp-tables out of soc node to fix these warnings

Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 arch/arm64/boot/dts/qcom/sm8350.dtsi | 76 ++++++++++++++--------------
 1 file changed, 38 insertions(+), 38 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi b/arch/arm64/boot/dts/qcom/sm8350.dtsi
index d242bab69c2e..846ddd290d08 100644
--- a/arch/arm64/boot/dts/qcom/sm8350.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi
@@ -340,6 +340,44 @@ CLUSTER_PD: cpu-cluster0 {
 		};
 	};
 
+	qup_opp_table_100mhz: qup-100mhz-opp-table {
+		compatible = "operating-points-v2";
+
+		opp-50000000 {
+			opp-hz = /bits/ 64 <50000000>;
+			required-opps = <&rpmhpd_opp_min_svs>;
+		};
+
+		opp-75000000 {
+			opp-hz = /bits/ 64 <75000000>;
+			required-opps = <&rpmhpd_opp_low_svs>;
+		};
+
+		opp-100000000 {
+			opp-hz = /bits/ 64 <100000000>;
+			required-opps = <&rpmhpd_opp_svs>;
+		};
+	};
+
+	qup_opp_table_120mhz: qup-120mhz-opp-table {
+		compatible = "operating-points-v2";
+
+		opp-50000000 {
+			opp-hz = /bits/ 64 <50000000>;
+			required-opps = <&rpmhpd_opp_min_svs>;
+		};
+
+		opp-75000000 {
+			opp-hz = /bits/ 64 <75000000>;
+			required-opps = <&rpmhpd_opp_low_svs>;
+		};
+
+		opp-120000000 {
+			opp-hz = /bits/ 64 <120000000>;
+			required-opps = <&rpmhpd_opp_svs>;
+		};
+	};
+
 	reserved_memory: reserved-memory {
 		#address-cells = <2>;
 		#size-cells = <2>;
@@ -637,44 +675,6 @@ ipcc: mailbox@408000 {
 			#mbox-cells = <2>;
 		};
 
-		qup_opp_table_100mhz: qup-100mhz-opp-table {
-			compatible = "operating-points-v2";
-
-			opp-50000000 {
-				opp-hz = /bits/ 64 <50000000>;
-				required-opps = <&rpmhpd_opp_min_svs>;
-			};
-
-			opp-75000000 {
-				opp-hz = /bits/ 64 <75000000>;
-				required-opps = <&rpmhpd_opp_low_svs>;
-			};
-
-			opp-100000000 {
-				opp-hz = /bits/ 64 <100000000>;
-				required-opps = <&rpmhpd_opp_svs>;
-			};
-		};
-
-		qup_opp_table_120mhz: qup-120mhz-opp-table {
-			compatible = "operating-points-v2";
-
-			opp-50000000 {
-				opp-hz = /bits/ 64 <50000000>;
-				required-opps = <&rpmhpd_opp_min_svs>;
-			};
-
-			opp-75000000 {
-				opp-hz = /bits/ 64 <75000000>;
-				required-opps = <&rpmhpd_opp_low_svs>;
-			};
-
-			opp-120000000 {
-				opp-hz = /bits/ 64 <120000000>;
-				required-opps = <&rpmhpd_opp_svs>;
-			};
-		};
-
 		qupv3_id_2: geniqup@8c0000 {
 			compatible = "qcom,geni-se-qup";
 			reg = <0x0 0x008c0000 0x0 0x6000>;
-- 
2.34.1

Re: [PATCH 2/6] arm64: dts: qcom: sm8350: move qup-opp-tables out of soc node

[Krzysztof Kozlowski]

On 28/03/2022 16:30, Vinod Koul wrote:
> The soc node expects all the nodes to have unit addresses. The
> qup-opp-tables do not have that which causes warnings:
> 
> arch/arm64/boot/dts/qcom/sm8350.dtsi:640.46-657.5:
> 	Warning (simple_bus_reg): /soc@0/qup-100mhz-opp-table:
> 	missing or empty reg/ranges property
> arch/arm64/boot/dts/qcom/sm8350.dtsi:659.46-676.5:
> 	Warning (simple_bus_reg): /soc@0/qup-120mhz-opp-table:
> 	missing or empty reg/ranges property
> 
> Move the qup-opp-tables out of soc node to fix these warnings
> 
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> ---
>  arch/arm64/boot/dts/qcom/sm8350.dtsi | 76 ++++++++++++++--------------
>  1 file changed, 38 insertions(+), 38 deletions(-)
> 


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

[PATCH 3/6] arm64: dts: qcom: sm8250: move qup-opp-table out of soc node

[Vinod Koul]

The soc node expects all the nodes to have unit addresses. The
qup-opp-table does not have that which causes warnings:

arch/arm64/boot/dts/qcom/sm8250.dtsi:916.32-933.5:
	Warning (simple_bus_reg): /soc@0/qup-opp-table:
	missing or empty reg/ranges property

Move the qup-opp-table out of soc node to fix these warnings

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

diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index fdaf303ba047..b120babdb26c 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -701,6 +701,25 @@ CLUSTER_PD: cpu-cluster0 {
 		};
 	};
 
+	qup_opp_table: qup-opp-table {
+		compatible = "operating-points-v2";
+
+		opp-50000000 {
+			opp-hz = /bits/ 64 <50000000>;
+			required-opps = <&rpmhpd_opp_min_svs>;
+		};
+
+		opp-75000000 {
+			opp-hz = /bits/ 64 <75000000>;
+			required-opps = <&rpmhpd_opp_low_svs>;
+		};
+
+		opp-120000000 {
+			opp-hz = /bits/ 64 <120000000>;
+			required-opps = <&rpmhpd_opp_svs>;
+		};
+	};
+
 	reserved-memory {
 		#address-cells = <2>;
 		#size-cells = <2>;
@@ -913,25 +932,6 @@ rng: rng@793000 {
 			clock-names = "core";
 		};
 
-		qup_opp_table: qup-opp-table {
-			compatible = "operating-points-v2";
-
-			opp-50000000 {
-				opp-hz = /bits/ 64 <50000000>;
-				required-opps = <&rpmhpd_opp_min_svs>;
-			};
-
-			opp-75000000 {
-				opp-hz = /bits/ 64 <75000000>;
-				required-opps = <&rpmhpd_opp_low_svs>;
-			};
-
-			opp-120000000 {
-				opp-hz = /bits/ 64 <120000000>;
-				required-opps = <&rpmhpd_opp_svs>;
-			};
-		};
-
 		gpi_dma2: dma-controller@800000 {
 			compatible = "qcom,sm8250-gpi-dma";
 			reg = <0 0x00800000 0 0x70000>;
-- 
2.34.1

Re: [PATCH 3/6] arm64: dts: qcom: sm8250: move qup-opp-table out of soc node

[Krzysztof Kozlowski]

On 28/03/2022 16:30, Vinod Koul wrote:
> The soc node expects all the nodes to have unit addresses. The
> qup-opp-table does not have that which causes warnings:
> 
> arch/arm64/boot/dts/qcom/sm8250.dtsi:916.32-933.5:
> 	Warning (simple_bus_reg): /soc@0/qup-opp-table:
> 	missing or empty reg/ranges property
> 
> Move the qup-opp-table out of soc node to fix these warnings
> 
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> ---
>  arch/arm64/boot/dts/qcom/sm8250.dtsi | 38 ++++++++++++++--------------
>  1 file changed, 19 insertions(+), 19 deletions(-)
> 


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

[PATCH 4/6] arm64: dts: qcom: sm8250: move sound node out of soc

[Vinod Koul]

The soc node expects all the nodes to have unit addresses. The sound
node does not have that which causes warnings:

arch/arm64/boot/dts/qcom/sm8250.dtsi:2806.16-2807.5:
Warning (simple_bus_reg): /soc@0/sound: missing or empty reg/ranges property

Move sound node out of soc to fix this

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

diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index b120babdb26c..fb94b342f2d1 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -2803,9 +2803,6 @@ compute-cb@8 {
 			};
 		};
 
-		sound: sound {
-		};
-
 		usb_1_hsphy: phy@88e3000 {
 			compatible = "qcom,sm8250-usb-hs-phy",
 				     "qcom,usb-snps-hs-7nm-phy";
@@ -4684,6 +4681,9 @@ cpufreq_hw: cpufreq@18591000 {
 		};
 	};
 
+	sound: sound {
+	};
+
 	timer {
 		compatible = "arm,armv8-timer";
 		interrupts = <GIC_PPI 13
-- 
2.34.1

Re: [PATCH 4/6] arm64: dts: qcom: sm8250: move sound node out of soc

[Krzysztof Kozlowski]

On 28/03/2022 16:30, Vinod Koul wrote:
> The soc node expects all the nodes to have unit addresses. The sound
> node does not have that which causes warnings:
> 
> arch/arm64/boot/dts/qcom/sm8250.dtsi:2806.16-2807.5:
> Warning (simple_bus_reg): /soc@0/sound: missing or empty reg/ranges property
> 
> Move sound node out of soc to fix this
> 
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> ---
>  arch/arm64/boot/dts/qcom/sm8250.dtsi | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 

I don't know the SM8250, but usually the sound node (e.g. containing
audio-codec) is not part of SoC. I propose to remove it entirely from
DTSI and define in same place in each DTS. It makes more sense logically
in such case - one clearly see which board defines the sounds, which
does not.


Best regards,
Krzysztof

Re: [PATCH 4/6] arm64: dts: qcom: sm8250: move sound node out of soc

[Vinod Koul]

On 28-03-22, 17:14, Krzysztof Kozlowski wrote:
> On 28/03/2022 16:30, Vinod Koul wrote:
> > The soc node expects all the nodes to have unit addresses. The sound
> > node does not have that which causes warnings:
> > 
> > arch/arm64/boot/dts/qcom/sm8250.dtsi:2806.16-2807.5:
> > Warning (simple_bus_reg): /soc@0/sound: missing or empty reg/ranges property
> > 
> > Move sound node out of soc to fix this
> > 
> > Signed-off-by: Vinod Koul <vkoul@kernel.org>
> > ---
> >  arch/arm64/boot/dts/qcom/sm8250.dtsi | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> 
> I don't know the SM8250, but usually the sound node (e.g. containing
> audio-codec) is not part of SoC. I propose to remove it entirely from
> DTSI and define in same place in each DTS. It makes more sense logically
> in such case - one clearly see which board defines the sounds, which
> does not.

Most of our boards have sound, should we duplicate it in all the
boards..? Bjorn..?

-- 
~Vinod

Re: [PATCH 4/6] arm64: dts: qcom: sm8250: move sound node out of soc

[Krzysztof Kozlowski]

On 28/03/2022 19:14, Vinod Koul wrote:
> On 28-03-22, 17:14, Krzysztof Kozlowski wrote:
>> On 28/03/2022 16:30, Vinod Koul wrote:
>>> The soc node expects all the nodes to have unit addresses. The sound
>>> node does not have that which causes warnings:
>>>
>>> arch/arm64/boot/dts/qcom/sm8250.dtsi:2806.16-2807.5:
>>> Warning (simple_bus_reg): /soc@0/sound: missing or empty reg/ranges property
>>>
>>> Move sound node out of soc to fix this
>>>
>>> Signed-off-by: Vinod Koul <vkoul@kernel.org>
>>> ---
>>>  arch/arm64/boot/dts/qcom/sm8250.dtsi | 6 +++---
>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>
>> I don't know the SM8250, but usually the sound node (e.g. containing
>> audio-codec) is not part of SoC. I propose to remove it entirely from
>> DTSI and define in same place in each DTS. It makes more sense logically
>> in such case - one clearly see which board defines the sounds, which
>> does not.
> 
> Most of our boards have sound, should we duplicate it in all the
> boards..? Bjorn..?

It's anyway empty here, so not much of saving lines f code...


Best regards,
Krzysztof

Re: [PATCH 4/6] arm64: dts: qcom: sm8250: move sound node out of soc

[Bjorn Andersson]

On Mon 28 Mar 12:14 CDT 2022, Vinod Koul wrote:

> On 28-03-22, 17:14, Krzysztof Kozlowski wrote:
> > On 28/03/2022 16:30, Vinod Koul wrote:
> > > The soc node expects all the nodes to have unit addresses. The sound
> > > node does not have that which causes warnings:
> > > 
> > > arch/arm64/boot/dts/qcom/sm8250.dtsi:2806.16-2807.5:
> > > Warning (simple_bus_reg): /soc@0/sound: missing or empty reg/ranges property
> > > 
> > > Move sound node out of soc to fix this
> > > 
> > > Signed-off-by: Vinod Koul <vkoul@kernel.org>
> > > ---
> > >  arch/arm64/boot/dts/qcom/sm8250.dtsi | 6 +++---
> > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > > 
> > 
> > I don't know the SM8250, but usually the sound node (e.g. containing
> > audio-codec) is not part of SoC. I propose to remove it entirely from
> > DTSI and define in same place in each DTS. It makes more sense logically
> > in such case - one clearly see which board defines the sounds, which
> > does not.
> 
> Most of our boards have sound, should we duplicate it in all the
> boards..? Bjorn..?
> 

But is the sndcard platform or device specific? E.g. all the SDM845
boards seems to use either qcom,sdm845-sndcard or qcom,db845c-sndcard.

Are there room for some common properties in this node?


Otherwise it seems reasonable to skip it in the platform dtsi.

Regards,
Bjorn

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

[Vinod Koul]

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 {
+		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";
-- 
2.34.1

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

[Krzysztof Kozlowski]

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

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

[Vinod Koul]

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

-- 
~Vinod

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

[Srinivas Kandagatla]

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
> 

[PATCH 6/6] arm64: dts: qcom: sm8250: remove address cells from dsi nodes

[Vinod Koul]

The child of dsi nodes do not have unit address, this causes warnings:

arch/arm64/boot/dts/qcom/sm8250.dtsi:3249.22-3301.6:
	Warning (avoid_unnecessary_addr_size): /soc@0/mdss@ae00000/dsi@ae94000:
	unnecessary #address-cells/#size-cells without "ranges" or child "reg" property

arch/arm64/boot/dts/qcom/sm8250.dtsi:3322.22-3374.6:
	Warning (avoid_unnecessary_addr_size): /soc@0/mdss@ae00000/dsi@ae96000:
	unnecessary #address-cells/#size-cells without "ranges" or child "reg" property

So remove #address-cells/#size-cells for dsi nodes.

Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 arch/arm64/boot/dts/qcom/sm8250.dtsi | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index fb94b342f2d1..a76089ebf9ca 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -3275,9 +3275,6 @@ dsi0: dsi@ae94000 {
 
 				status = "disabled";
 
-				#address-cells = <1>;
-				#size-cells = <0>;
-
 				ports {
 					#address-cells = <1>;
 					#size-cells = <0>;
@@ -3348,9 +3345,6 @@ dsi1: dsi@ae96000 {
 
 				status = "disabled";
 
-				#address-cells = <1>;
-				#size-cells = <0>;
-
 				ports {
 					#address-cells = <1>;
 					#size-cells = <0>;
-- 
2.34.1

Re: [PATCH 6/6] arm64: dts: qcom: sm8250: remove address cells from dsi nodes

[Krzysztof Kozlowski]

On 28/03/2022 16:30, Vinod Koul wrote:
> The child of dsi nodes do not have unit address, this causes warnings:
> 
> arch/arm64/boot/dts/qcom/sm8250.dtsi:3249.22-3301.6:
> 	Warning (avoid_unnecessary_addr_size): /soc@0/mdss@ae00000/dsi@ae94000:
> 	unnecessary #address-cells/#size-cells without "ranges" or child "reg" property
> 
> arch/arm64/boot/dts/qcom/sm8250.dtsi:3322.22-3374.6:
> 	Warning (avoid_unnecessary_addr_size): /soc@0/mdss@ae00000/dsi@ae96000:
> 	unnecessary #address-cells/#size-cells without "ranges" or child "reg" property
> 
> So remove #address-cells/#size-cells for dsi nodes.

Looks good.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

Re: [PATCH 6/6] arm64: dts: qcom: sm8250: remove address cells from dsi nodes

[Krzysztof Kozlowski]

On 28/03/2022 16:30, Vinod Koul wrote:
> The child of dsi nodes do not have unit address, this causes warnings:
> 
> arch/arm64/boot/dts/qcom/sm8250.dtsi:3249.22-3301.6:
> 	Warning (avoid_unnecessary_addr_size): /soc@0/mdss@ae00000/dsi@ae94000:
> 	unnecessary #address-cells/#size-cells without "ranges" or child "reg" property
> 
> arch/arm64/boot/dts/qcom/sm8250.dtsi:3322.22-3374.6:
> 	Warning (avoid_unnecessary_addr_size): /soc@0/mdss@ae00000/dsi@ae96000:
> 	unnecessary #address-cells/#size-cells without "ranges" or child "reg" property
> 
> So remove #address-cells/#size-cells for dsi nodes.

I think this needs different fix and my review was not correct. MIPI DSI
is a bus, so it can have children with unit addresses.

This DTSI (and several others) should have address/size cells, so
downstream could add easily children (e.g. panel nodes).

However I don't know how to combine it with "ports" not having unit address.

Maybe Rob has some ideas?

Best regards,
Krzysztof