2024-04-02 15:43:34

by Michael Walle

[permalink] [raw]
Subject: [PATCH] arm64: dts: ti: k3-j722s: Disable ethernet ports by default

Device tree best practice is to disable any external interface in the
dtsi and just enable them if needed in the device tree. Thus, disable
both ethernet ports by default and just enable the one used by the EVM
in its device tree.

There is no functional change.

Signed-off-by: Michael Walle <[email protected]>
---
This should also be true for all the other SoCs. But I don't wanted to
touch all the (older) device trees. j722s is pretty new, so there we
should get it right.
---
arch/arm64/boot/dts/ti/k3-j722s-evm.dts | 5 +----
arch/arm64/boot/dts/ti/k3-j722s.dtsi | 8 ++++++++
2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/ti/k3-j722s-evm.dts b/arch/arm64/boot/dts/ti/k3-j722s-evm.dts
index d045dc7dde0c..afe7f68e6a4b 100644
--- a/arch/arm64/boot/dts/ti/k3-j722s-evm.dts
+++ b/arch/arm64/boot/dts/ti/k3-j722s-evm.dts
@@ -224,14 +224,11 @@ cpsw3g_phy0: ethernet-phy@0 {
};

&cpsw_port1 {
+ status = "okay";
phy-mode = "rgmii-rxid";
phy-handle = <&cpsw3g_phy0>;
};

-&cpsw_port2 {
- status = "disabled";
-};
-
&main_gpio1 {
status = "okay";
};
diff --git a/arch/arm64/boot/dts/ti/k3-j722s.dtsi b/arch/arm64/boot/dts/ti/k3-j722s.dtsi
index c75744edb143..d0451e6e7496 100644
--- a/arch/arm64/boot/dts/ti/k3-j722s.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j722s.dtsi
@@ -83,6 +83,14 @@ &inta_main_dmss {
ti,interrupt-ranges = <7 71 21>;
};

+&cpsw_port1 {
+ status = "disabled";
+};
+
+&cpsw_port2 {
+ status = "disabled";
+};
+
&oc_sram {
reg = <0x00 0x70000000 0x00 0x40000>;
ranges = <0x00 0x00 0x70000000 0x40000>;
--
2.39.2



2024-04-02 16:17:26

by Michael Walle

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: ti: k3-j722s: Disable ethernet ports by default

Hi,

> This is better at arch/arm64/boot/dts/ti/k3-am62p-main.dtsi.

I'm fine with that, but be aware that we'll also change the am62p
SoC dtsi in that case.

-michael


Attachments:
signature.asc (305.00 B)

2024-04-02 16:43:21

by Nishanth Menon

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: ti: k3-j722s: Disable ethernet ports by default

On 17:18-20240402, Michael Walle wrote:
> Device tree best practice is to disable any external interface in the
> dtsi and just enable them if needed in the device tree. Thus, disable
> both ethernet ports by default and just enable the one used by the EVM
> in its device tree.
>
> There is no functional change.
>
> Signed-off-by: Michael Walle <[email protected]>
> ---
> This should also be true for all the other SoCs. But I don't wanted to
> touch all the (older) device trees. j722s is pretty new, so there we
> should get it right.
> ---
> arch/arm64/boot/dts/ti/k3-j722s-evm.dts | 5 +----
> arch/arm64/boot/dts/ti/k3-j722s.dtsi | 8 ++++++++
> 2 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/ti/k3-j722s-evm.dts b/arch/arm64/boot/dts/ti/k3-j722s-evm.dts
> index d045dc7dde0c..afe7f68e6a4b 100644
> --- a/arch/arm64/boot/dts/ti/k3-j722s-evm.dts
> +++ b/arch/arm64/boot/dts/ti/k3-j722s-evm.dts
> @@ -224,14 +224,11 @@ cpsw3g_phy0: ethernet-phy@0 {
> };
>
> &cpsw_port1 {
> + status = "okay";
> phy-mode = "rgmii-rxid";
> phy-handle = <&cpsw3g_phy0>;
> };
>
> -&cpsw_port2 {
> - status = "disabled";
> -};
> -
> &main_gpio1 {
> status = "okay";
> };
> diff --git a/arch/arm64/boot/dts/ti/k3-j722s.dtsi b/arch/arm64/boot/dts/ti/k3-j722s.dtsi
> index c75744edb143..d0451e6e7496 100644
> --- a/arch/arm64/boot/dts/ti/k3-j722s.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-j722s.dtsi
> @@ -83,6 +83,14 @@ &inta_main_dmss {
> ti,interrupt-ranges = <7 71 21>;
> };
>
> +&cpsw_port1 {
> + status = "disabled";
> +};
> +
> +&cpsw_port2 {
> + status = "disabled";
> +};
> +
> &oc_sram {
> reg = <0x00 0x70000000 0x00 0x40000>;
> ranges = <0x00 0x00 0x70000000 0x40000>;
> --
> 2.39.2
>

Meant to respond to this thread:

This is better at arch/arm64/boot/dts/ti/k3-am62p-main.dtsi.

--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D

2024-04-02 16:47:41

by Nishanth Menon

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: ti: k3-j722s: Disable ethernet ports by default

On 18:16-20240402, Michael Walle wrote:
> Hi,
>
> > This is better at arch/arm64/boot/dts/ti/k3-am62p-main.dtsi.
>
> I'm fine with that, but be aware that we'll also change the am62p
> SoC dtsi in that case.

This change is valid to me (sigh.. it's been a whack-a-mole as you can expect).
62p and j722s/am67 are too closely related anyways.. but, this will need
to percolate consistently to all k3 SoCs.. (different problem).

--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D

2024-04-02 16:58:40

by Francesco Dolcini

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: ti: k3-j722s: Disable ethernet ports by default

Hello Michael,

On Tue, Apr 02, 2024 at 05:18:02PM +0200, Michael Walle wrote:
> Device tree best practice is to disable any external interface in the
> dtsi and just enable them if needed in the device tree. Thus, disable
> both ethernet ports by default and just enable the one used by the EVM
> in its device tree.
>
> There is no functional change.
>
> Signed-off-by: Michael Walle <[email protected]>
> ---
> This should also be true for all the other SoCs. But I don't wanted to
> touch all the (older) device trees. j722s is pretty new, so there we
> should get it right.
> ---
> arch/arm64/boot/dts/ti/k3-j722s-evm.dts | 5 +----
> arch/arm64/boot/dts/ti/k3-j722s.dtsi | 8 ++++++++
> 2 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/ti/k3-j722s-evm.dts b/arch/arm64/boot/dts/ti/k3-j722s-evm.dts
> index d045dc7dde0c..afe7f68e6a4b 100644
> --- a/arch/arm64/boot/dts/ti/k3-j722s-evm.dts
> +++ b/arch/arm64/boot/dts/ti/k3-j722s-evm.dts
> @@ -224,14 +224,11 @@ cpsw3g_phy0: ethernet-phy@0 {
> };
>
> &cpsw_port1 {
> + status = "okay";

status should be the last property, according to the dts coding guidelines.

> phy-mode = "rgmii-rxid";
> phy-handle = <&cpsw3g_phy0>;
> };

Francesco


2024-04-03 07:55:16

by Michael Walle

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: ti: k3-j722s: Disable ethernet ports by default

Hi Francesco,

On Tue Apr 2, 2024 at 6:58 PM CEST, Francesco Dolcini wrote:
> On Tue, Apr 02, 2024 at 05:18:02PM +0200, Michael Walle wrote:
> > Device tree best practice is to disable any external interface in the
> > dtsi and just enable them if needed in the device tree. Thus, disable
> > both ethernet ports by default and just enable the one used by the EVM
> > in its device tree.
> >
> > There is no functional change.
> >
> > Signed-off-by: Michael Walle <[email protected]>
> > ---
> > This should also be true for all the other SoCs. But I don't wanted to
> > touch all the (older) device trees. j722s is pretty new, so there we
> > should get it right.
> > ---
> > arch/arm64/boot/dts/ti/k3-j722s-evm.dts | 5 +----
> > arch/arm64/boot/dts/ti/k3-j722s.dtsi | 8 ++++++++
> > 2 files changed, 9 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/ti/k3-j722s-evm.dts b/arch/arm64/boot/dts/ti/k3-j722s-evm.dts
> > index d045dc7dde0c..afe7f68e6a4b 100644
> > --- a/arch/arm64/boot/dts/ti/k3-j722s-evm.dts
> > +++ b/arch/arm64/boot/dts/ti/k3-j722s-evm.dts
> > @@ -224,14 +224,11 @@ cpsw3g_phy0: ethernet-phy@0 {
> > };
> >
> > &cpsw_port1 {
> > + status = "okay";
>
> status should be the last property, according to the dts coding guidelines.

Thanks for pointing that out. There is
devicetree/bindings/dts-coding-style.rst, which is in fact new to
me. Up until now, I was under the impression that how this is
handled is up to the maintainer of the SoC. I know that for the NXP
Layerscape for example, the maintainer will have an eye esp. for
that. But here it seems kinda random/all over the place. That being
said, I tried to be consistent with the other cpsw* nodes.

Anyway, I'll change it to come last.

> > phy-mode = "rgmii-rxid";
> > phy-handle = <&cpsw3g_phy0>;
> > };

-michael


Attachments:
signature.asc (305.00 B)