[PATCH devicetree 1/2] MIPS: mscc: ocelot: disable all switch ports by default

[PATCH devicetree 1/2] MIPS: mscc: ocelot: disable all switch ports by default

[Vladimir Oltean]

The ocelot switch driver used to ignore ports which do not have a
phy-handle property and not probe those, but this is not quite ok since
it is valid to not have a phy-handle property if there is a fixed-link.

It seems that checking for a phy-handle was a proxy for the proper check
which is for the status, but that doesn't make a lot of sense, since the
ocelot driver already iterates using for_each_available_child_of_node
which skips the disabled ports, so I have no idea.

Anyway, a widespread pattern in device trees is for a SoC dtsi to
disable by default all hardware, and let board dts files enable what is
used. So let's do that and enable only the ports with a phy-handle in
the pcb120 and pcb123 device tree files.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 arch/mips/boot/dts/mscc/ocelot.dtsi       | 11 +++++++++++
 arch/mips/boot/dts/mscc/ocelot_pcb120.dts |  8 ++++++++
 arch/mips/boot/dts/mscc/ocelot_pcb123.dts |  4 ++++
 3 files changed, 23 insertions(+)

diff --git a/arch/mips/boot/dts/mscc/ocelot.dtsi b/arch/mips/boot/dts/mscc/ocelot.dtsi
index 535a98284dcb..e51db651af13 100644
--- a/arch/mips/boot/dts/mscc/ocelot.dtsi
+++ b/arch/mips/boot/dts/mscc/ocelot.dtsi
@@ -150,36 +150,47 @@ ethernet-ports {
 
 				port0: port@0 {
 					reg = <0>;
+					status = "disabled";
 				};
 				port1: port@1 {
 					reg = <1>;
+					status = "disabled";
 				};
 				port2: port@2 {
 					reg = <2>;
+					status = "disabled";
 				};
 				port3: port@3 {
 					reg = <3>;
+					status = "disabled";
 				};
 				port4: port@4 {
 					reg = <4>;
+					status = "disabled";
 				};
 				port5: port@5 {
 					reg = <5>;
+					status = "disabled";
 				};
 				port6: port@6 {
 					reg = <6>;
+					status = "disabled";
 				};
 				port7: port@7 {
 					reg = <7>;
+					status = "disabled";
 				};
 				port8: port@8 {
 					reg = <8>;
+					status = "disabled";
 				};
 				port9: port@9 {
 					reg = <9>;
+					status = "disabled";
 				};
 				port10: port@10 {
 					reg = <10>;
+					status = "disabled";
 				};
 			};
 		};
diff --git a/arch/mips/boot/dts/mscc/ocelot_pcb120.dts b/arch/mips/boot/dts/mscc/ocelot_pcb120.dts
index 897de5025d7f..d2dc6b3d923c 100644
--- a/arch/mips/boot/dts/mscc/ocelot_pcb120.dts
+++ b/arch/mips/boot/dts/mscc/ocelot_pcb120.dts
@@ -69,40 +69,48 @@ phy4: ethernet-phy@3 {
 };
 
 &port0 {
+	status = "okay";
 	phy-handle = <&phy0>;
 };
 
 &port1 {
+	status = "okay";
 	phy-handle = <&phy1>;
 };
 
 &port2 {
+	status = "okay";
 	phy-handle = <&phy2>;
 };
 
 &port3 {
+	status = "okay";
 	phy-handle = <&phy3>;
 };
 
 &port4 {
+	status = "okay";
 	phy-handle = <&phy7>;
 	phy-mode = "sgmii";
 	phys = <&serdes 4 SERDES1G(2)>;
 };
 
 &port5 {
+	status = "okay";
 	phy-handle = <&phy4>;
 	phy-mode = "sgmii";
 	phys = <&serdes 5 SERDES1G(5)>;
 };
 
 &port6 {
+	status = "okay";
 	phy-handle = <&phy6>;
 	phy-mode = "sgmii";
 	phys = <&serdes 6 SERDES1G(3)>;
 };
 
 &port9 {
+	status = "okay";
 	phy-handle = <&phy5>;
 	phy-mode = "sgmii";
 	phys = <&serdes 9 SERDES1G(4)>;
diff --git a/arch/mips/boot/dts/mscc/ocelot_pcb123.dts b/arch/mips/boot/dts/mscc/ocelot_pcb123.dts
index ef852f382da8..7d7e638791dd 100644
--- a/arch/mips/boot/dts/mscc/ocelot_pcb123.dts
+++ b/arch/mips/boot/dts/mscc/ocelot_pcb123.dts
@@ -47,17 +47,21 @@ &mdio0 {
 };
 
 &port0 {
+	status = "okay";
 	phy-handle = <&phy0>;
 };
 
 &port1 {
+	status = "okay";
 	phy-handle = <&phy1>;
 };
 
 &port2 {
+	status = "okay";
 	phy-handle = <&phy2>;
 };
 
 &port3 {
+	status = "okay";
 	phy-handle = <&phy3>;
 };
-- 
2.25.1

[PATCH devicetree 2/2] MIPS: mscc: ocelot: mark the phy-mode for internal PHY ports

[Vladimir Oltean]

The ocelot driver was converted to phylink, and that expects a valid
phy_interface_t. Without a phy-mode, of_get_phy_mode returns
PHY_INTERFACE_MODE_NA, which is not ideal because phylink rejects that.

The ocelot driver was patched to treat PHY_INTERFACE_MODE_NA as
PHY_INTERFACE_MODE_INTERNAL to work with the broken DT blobs, but we
should fix the device trees and specify the phy-mode too.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 arch/mips/boot/dts/mscc/ocelot_pcb120.dts | 4 ++++
 arch/mips/boot/dts/mscc/ocelot_pcb123.dts | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/arch/mips/boot/dts/mscc/ocelot_pcb120.dts b/arch/mips/boot/dts/mscc/ocelot_pcb120.dts
index d2dc6b3d923c..bd240690cb37 100644
--- a/arch/mips/boot/dts/mscc/ocelot_pcb120.dts
+++ b/arch/mips/boot/dts/mscc/ocelot_pcb120.dts
@@ -71,21 +71,25 @@ phy4: ethernet-phy@3 {
 &port0 {
 	status = "okay";
 	phy-handle = <&phy0>;
+	phy-mode = "internal";
 };
 
 &port1 {
 	status = "okay";
 	phy-handle = <&phy1>;
+	phy-mode = "internal";
 };
 
 &port2 {
 	status = "okay";
 	phy-handle = <&phy2>;
+	phy-mode = "internal";
 };
 
 &port3 {
 	status = "okay";
 	phy-handle = <&phy3>;
+	phy-mode = "internal";
 };
 
 &port4 {
diff --git a/arch/mips/boot/dts/mscc/ocelot_pcb123.dts b/arch/mips/boot/dts/mscc/ocelot_pcb123.dts
index 7d7e638791dd..0185045c7630 100644
--- a/arch/mips/boot/dts/mscc/ocelot_pcb123.dts
+++ b/arch/mips/boot/dts/mscc/ocelot_pcb123.dts
@@ -49,19 +49,23 @@ &mdio0 {
 &port0 {
 	status = "okay";
 	phy-handle = <&phy0>;
+	phy-mode = "internal";
 };
 
 &port1 {
 	status = "okay";
 	phy-handle = <&phy1>;
+	phy-mode = "internal";
 };
 
 &port2 {
 	status = "okay";
 	phy-handle = <&phy2>;
+	phy-mode = "internal";
 };
 
 &port3 {
 	status = "okay";
 	phy-handle = <&phy3>;
+	phy-mode = "internal";
 };
-- 
2.25.1

Re: [PATCH devicetree 2/2] MIPS: mscc: ocelot: mark the phy-mode for internal PHY ports

[Vladimir Oltean]

On Thu, Aug 19, 2021 at 08:04:16PM +0300, Vladimir Oltean wrote:
> The ocelot driver was converted to phylink, and that expects a valid
> phy_interface_t. Without a phy-mode, of_get_phy_mode returns
> PHY_INTERFACE_MODE_NA, which is not ideal because phylink rejects that.
> 
> The ocelot driver was patched to treat PHY_INTERFACE_MODE_NA as
> PHY_INTERFACE_MODE_INTERNAL to work with the broken DT blobs, but we
> should fix the device trees and specify the phy-mode too.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---

Please note that the pre-phylink driver has this check:

		switch (ocelot_port->phy_mode) {
		case PHY_INTERFACE_MODE_NA:
			(...)
		case PHY_INTERFACE_MODE_SGMII:
			(...)
		case PHY_INTERFACE_MODE_QSGMII:
			(...)
		default:
			dev_err(ocelot->dev,
				"invalid phy mode for port%d, (Q)SGMII only\n",
				port);
			of_node_put(portnp);
			err = -EINVAL;
			goto out_teardown;
		}

So it does not actually expect PHY_INTERFACE_MODE_INTERNAL and will
error out.

Are we okay with the new device tree blobs breaking the old kernel?

Should we instead wait for a few more kernel release cycles before
changing the device tree in this regard?

Re: [PATCH devicetree 2/2] MIPS: mscc: ocelot: mark the phy-mode for internal PHY ports

[Alexandre Belloni]

On 19/08/2021 17:17:05+0000, Vladimir Oltean wrote:
> On Thu, Aug 19, 2021 at 08:04:16PM +0300, Vladimir Oltean wrote:
> > The ocelot driver was converted to phylink, and that expects a valid
> > phy_interface_t. Without a phy-mode, of_get_phy_mode returns
> > PHY_INTERFACE_MODE_NA, which is not ideal because phylink rejects that.
> > 
> > The ocelot driver was patched to treat PHY_INTERFACE_MODE_NA as
> > PHY_INTERFACE_MODE_INTERNAL to work with the broken DT blobs, but we
> > should fix the device trees and specify the phy-mode too.
> > 
> > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> > ---
> 
> Please note that the pre-phylink driver has this check:
> 
> 		switch (ocelot_port->phy_mode) {
> 		case PHY_INTERFACE_MODE_NA:
> 			(...)
> 		case PHY_INTERFACE_MODE_SGMII:
> 			(...)
> 		case PHY_INTERFACE_MODE_QSGMII:
> 			(...)
> 		default:
> 			dev_err(ocelot->dev,
> 				"invalid phy mode for port%d, (Q)SGMII only\n",
> 				port);
> 			of_node_put(portnp);
> 			err = -EINVAL;
> 			goto out_teardown;
> 		}
> 
> So it does not actually expect PHY_INTERFACE_MODE_INTERNAL and will
> error out.
> 
> Are we okay with the new device tree blobs breaking the old kernel?

From my point of view, newer device trees are not required to work on
older kernel, this would impose an unreasonable limitation and the use
case is very limited.

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH devicetree 1/2] MIPS: mscc: ocelot: disable all switch ports by default

[Thomas Bogendoerfer]

On Thu, Aug 19, 2021 at 08:04:15PM +0300, Vladimir Oltean wrote:
> The ocelot switch driver used to ignore ports which do not have a
> phy-handle property and not probe those, but this is not quite ok since
> it is valid to not have a phy-handle property if there is a fixed-link.
> 
> It seems that checking for a phy-handle was a proxy for the proper check
> which is for the status, but that doesn't make a lot of sense, since the
> ocelot driver already iterates using for_each_available_child_of_node
> which skips the disabled ports, so I have no idea.
> 
> Anyway, a widespread pattern in device trees is for a SoC dtsi to
> disable by default all hardware, and let board dts files enable what is
> used. So let's do that and enable only the ports with a phy-handle in
> the pcb120 and pcb123 device tree files.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  arch/mips/boot/dts/mscc/ocelot.dtsi       | 11 +++++++++++
>  arch/mips/boot/dts/mscc/ocelot_pcb120.dts |  8 ++++++++
>  arch/mips/boot/dts/mscc/ocelot_pcb123.dts |  4 ++++
>  3 files changed, 23 insertions(+)

applied to mips-next.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

Re: [PATCH devicetree 2/2] MIPS: mscc: ocelot: mark the phy-mode for internal PHY ports

[Thomas Bogendoerfer]

On Thu, Aug 19, 2021 at 08:04:16PM +0300, Vladimir Oltean wrote:
> The ocelot driver was converted to phylink, and that expects a valid
> phy_interface_t. Without a phy-mode, of_get_phy_mode returns
> PHY_INTERFACE_MODE_NA, which is not ideal because phylink rejects that.
> 
> The ocelot driver was patched to treat PHY_INTERFACE_MODE_NA as
> PHY_INTERFACE_MODE_INTERNAL to work with the broken DT blobs, but we
> should fix the device trees and specify the phy-mode too.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  arch/mips/boot/dts/mscc/ocelot_pcb120.dts | 4 ++++
>  arch/mips/boot/dts/mscc/ocelot_pcb123.dts | 4 ++++
>  2 files changed, 8 insertions(+)

applied to mips-next.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]