[PATCH v3 1/4] arm64: dts: qcom: sa8155p-adp: Make compatible the first property

[PATCH v3 1/4] arm64: dts: qcom: sa8155p-adp: Make compatible the first property

[Andrew Halaney]

As stated at the below link in another review, compatible is always the
first property.

Follow suit here to avoid copying incorrectly in the future.

Link: https://lore.kernel.org/netdev/20230331215804.783439-1-ahalaney@redhat.com/T/#ma76b4116bbb9e49ee4bcf699e40935d80965b3f3
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Signed-off-by: Andrew Halaney <ahalaney@redhat.com>
---

Changes since v2:
    * Collect tags
Changes since v1:
    * None

 arch/arm64/boot/dts/qcom/sa8155p-adp.dts | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sa8155p-adp.dts b/arch/arm64/boot/dts/qcom/sa8155p-adp.dts
index 339fea522509..029b23198b3a 100644
--- a/arch/arm64/boot/dts/qcom/sa8155p-adp.dts
+++ b/arch/arm64/boot/dts/qcom/sa8155p-adp.dts
@@ -352,19 +352,18 @@ &ethernet {
 	max-speed = <1000>;
 
 	mdio {
+		compatible = "snps,dwmac-mdio";
 		#address-cells = <0x1>;
 		#size-cells = <0x0>;
 
-		compatible = "snps,dwmac-mdio";
-
 		/* Micrel KSZ9031RNZ PHY */
 		rgmii_phy: phy@7 {
+			compatible = "ethernet-phy-ieee802.3-c22";
 			reg = <0x7>;
 
 			interrupt-parent = <&tlmm>;
 			interrupts-extended = <&tlmm 124 IRQ_TYPE_EDGE_FALLING>; /* phy intr */
 			device_type = "ethernet-phy";
-			compatible = "ethernet-phy-ieee802.3-c22";
 		};
 	};
 };
-- 
2.40.0

[PATCH v3 2/4] arm64: dts: qcom: sa8155p-adp: Make -cells decimal

[Andrew Halaney]

The property logically makes sense in decimal, and is the standard used
elsewhere.

Suggested-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Signed-off-by: Andrew Halaney <ahalaney@redhat.com>
---

Changes since v2:
    * Collect tags
Changes since v1:
    * New patch (Konrad)

 arch/arm64/boot/dts/qcom/sa8155p-adp.dts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sa8155p-adp.dts b/arch/arm64/boot/dts/qcom/sa8155p-adp.dts
index 029b23198b3a..ac52a8dfeba1 100644
--- a/arch/arm64/boot/dts/qcom/sa8155p-adp.dts
+++ b/arch/arm64/boot/dts/qcom/sa8155p-adp.dts
@@ -353,8 +353,8 @@ &ethernet {
 
 	mdio {
 		compatible = "snps,dwmac-mdio";
-		#address-cells = <0x1>;
-		#size-cells = <0x0>;
+		#address-cells = <1>;
+		#size-cells = <0>;
 
 		/* Micrel KSZ9031RNZ PHY */
 		rgmii_phy: phy@7 {
-- 
2.40.0

[PATCH v3 3/4] arm64: dts: qcom: sa8155p-adp: Remove unneeded rgmii_phy information

[Andrew Halaney]

Using interrupts-extended already indicates what the interrupt-parent
is, so drop the explicit interrupt-parent.

The comment about this being the phy-intr is not helpful either, since
this is the only interrupt in the phy node.

Suggested-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Signed-off-by: Andrew Halaney <ahalaney@redhat.com>
---

Changes since v2:
    * Collect tags
Changes since v1:
    * New patch (Konrad)

 arch/arm64/boot/dts/qcom/sa8155p-adp.dts | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sa8155p-adp.dts b/arch/arm64/boot/dts/qcom/sa8155p-adp.dts
index ac52a8dfeba1..b65e0203d783 100644
--- a/arch/arm64/boot/dts/qcom/sa8155p-adp.dts
+++ b/arch/arm64/boot/dts/qcom/sa8155p-adp.dts
@@ -361,8 +361,7 @@ rgmii_phy: phy@7 {
 			compatible = "ethernet-phy-ieee802.3-c22";
 			reg = <0x7>;
 
-			interrupt-parent = <&tlmm>;
-			interrupts-extended = <&tlmm 124 IRQ_TYPE_EDGE_FALLING>; /* phy intr */
+			interrupts-extended = <&tlmm 124 IRQ_TYPE_EDGE_FALLING>;
 			device_type = "ethernet-phy";
 		};
 	};
-- 
2.40.0

[PATCH v3 4/4] arm64: dts: qcom: sa8155p-adp: Move mtl nodes into ethernet node

[Andrew Halaney]

The mtl nodes aren't evaluated unless they're under the node with the
compatible. Move them so they're now evaluated in case future patchsets
modify them incorrectly.

An example of this can be seen in the link.

Link: https://lore.kernel.org/linux-arm-msm/20230414145844.wyg6pt623pzqwh5l@halaney-x13s/
Signed-off-by: Andrew Halaney <ahalaney@redhat.com>
---

Changes since v2:
    * New patch (me, after realizing this on another platform)

 arch/arm64/boot/dts/qcom/sa8155p-adp.dts | 46 ++++++++++++------------
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sa8155p-adp.dts b/arch/arm64/boot/dts/qcom/sa8155p-adp.dts
index b65e0203d783..02d04ce877c9 100644
--- a/arch/arm64/boot/dts/qcom/sa8155p-adp.dts
+++ b/arch/arm64/boot/dts/qcom/sa8155p-adp.dts
@@ -47,29 +47,6 @@ vreg_s4a_1p8: smps4 {
 
 		vin-supply = <&vreg_3p3>;
 	};
-
-	mtl_rx_setup: rx-queues-config {
-		snps,rx-queues-to-use = <1>;
-		snps,rx-sched-sp;
-
-		queue0 {
-			snps,dcb-algorithm;
-			snps,map-to-dma-channel = <0x0>;
-			snps,route-up;
-			snps,priority = <0x1>;
-		};
-	};
-
-	mtl_tx_setup: tx-queues-config {
-		snps,tx-queues-to-use = <1>;
-		snps,tx-sched-wrr;
-
-		queue0 {
-			snps,weight = <0x10>;
-			snps,dcb-algorithm;
-			snps,priority = <0x0>;
-		};
-	};
 };
 
 &apps_rsc {
@@ -365,6 +342,29 @@ rgmii_phy: phy@7 {
 			device_type = "ethernet-phy";
 		};
 	};
+
+	mtl_rx_setup: rx-queues-config {
+		snps,rx-queues-to-use = <1>;
+		snps,rx-sched-sp;
+
+		queue0 {
+			snps,dcb-algorithm;
+			snps,map-to-dma-channel = <0x0>;
+			snps,route-up;
+			snps,priority = <0x1>;
+		};
+	};
+
+	mtl_tx_setup: tx-queues-config {
+		snps,tx-queues-to-use = <1>;
+		snps,tx-sched-wrr;
+
+		queue0 {
+			snps,weight = <0x10>;
+			snps,dcb-algorithm;
+			snps,priority = <0x0>;
+		};
+	};
 };
 
 &qupv3_id_1 {
-- 
2.40.0

Re: [PATCH v3 2/4] arm64: dts: qcom: sa8155p-adp: Make -cells decimal

[Krzysztof Kozlowski]

On 21/04/2023 22:55, Andrew Halaney wrote:
> The property logically makes sense in decimal, and is the standard used
> elsewhere.
> 
> Suggested-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> Signed-off-by: Andrew Halaney <ahalaney@redhat.com>
> ---
> 
> Changes since v2:
>     * Collect tags
> Changes since v1:
>     * New patch (Konrad)

Doing such changes per one board will be a lot of churn...

Best regards,
Krzysztof

Re: [PATCH v3 2/4] arm64: dts: qcom: sa8155p-adp: Make -cells decimal

[Andrew Halaney]

On Mon, Apr 24, 2023 at 09:21:45AM +0200, Krzysztof Kozlowski wrote:
> On 21/04/2023 22:55, Andrew Halaney wrote:
> > The property logically makes sense in decimal, and is the standard used
> > elsewhere.
> > 
> > Suggested-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> > Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> > Signed-off-by: Andrew Halaney <ahalaney@redhat.com>
> > ---
> > 
> > Changes since v2:
> >     * Collect tags
> > Changes since v1:
> >     * New patch (Konrad)
> 
> Doing such changes per one board will be a lot of churn...
> 

Agreed, are you suggesting I drop this patch (and if we do this sort of
cleanup to do it in bulk)? Sorry, not entirely sure I am picking up what
that comment is implying.

Personally, I'd prefer to keeping that patch as this series is trying to
clean up the particular ethernet node.. but if the purpose of the series
was just to clean -cells up then I'd go with the bulk approach.

So as is, I'm going to leave it like so unless you have specific advice
against doing so.

Thanks,
Andrew

Re: [PATCH v3 2/4] arm64: dts: qcom: sa8155p-adp: Make -cells decimal

[Krzysztof Kozlowski]

On 24/04/2023 16:38, Andrew Halaney wrote:
> On Mon, Apr 24, 2023 at 09:21:45AM +0200, Krzysztof Kozlowski wrote:
>> On 21/04/2023 22:55, Andrew Halaney wrote:
>>> The property logically makes sense in decimal, and is the standard used
>>> elsewhere.
>>>
>>> Suggested-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>>> Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>>> Signed-off-by: Andrew Halaney <ahalaney@redhat.com>
>>> ---
>>>
>>> Changes since v2:
>>>     * Collect tags
>>> Changes since v1:
>>>     * New patch (Konrad)
>>
>> Doing such changes per one board will be a lot of churn...
>>
> 
> Agreed, are you suggesting I drop this patch (and if we do this sort of
> cleanup to do it in bulk)? Sorry, not entirely sure I am picking up what
> that comment is implying.

Yes, I suggest to fix all Qualcomm arm64 boards with one patch. There
are just few more occurrences of this. It's also pure cleanup, no
functional change.

> 
> Personally, I'd prefer to keeping that patch as this series is trying to
> clean up the particular ethernet node.. but if the purpose of the series
> was just to clean -cells up then I'd go with the bulk approach.
> 
> So as is, I'm going to leave it like so unless you have specific advice
> against doing so.

Best regards,
Krzysztof

Re: (subset) [PATCH v3 1/4] arm64: dts: qcom: sa8155p-adp: Make compatible the first property

[Bjorn Andersson]

On Fri, 21 Apr 2023 15:55:09 -0500, Andrew Halaney wrote:
> As stated at the below link in another review, compatible is always the
> first property.
> 
> Follow suit here to avoid copying incorrectly in the future.
> 
> 

Applied, thanks!

[1/4] arm64: dts: qcom: sa8155p-adp: Make compatible the first property
      commit: eee7369de0c3cca7aacab3874663d460fb7a5b06
[3/4] arm64: dts: qcom: sa8155p-adp: Remove unneeded rgmii_phy information
      commit: 0ff4f6a380214fcf85a5b5ad64ff481d9bd1dfc6
[4/4] arm64: dts: qcom: sa8155p-adp: Move mtl nodes into ethernet node
      commit: 93fe463652504bba298a68b56334729cdf92c0c3

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>