* [PATCH devicetree 1/2] MIPS: mscc: ocelot: disable all switch ports by default
@ 2021-08-19 17:04 Vladimir Oltean
2021-08-19 17:04 ` [PATCH devicetree 2/2] MIPS: mscc: ocelot: mark the phy-mode for internal PHY ports Vladimir Oltean
2021-08-21 8:41 ` [PATCH devicetree 1/2] MIPS: mscc: ocelot: disable all switch ports by default Thomas Bogendoerfer
0 siblings, 2 replies; 6+ messages in thread
From: Vladimir Oltean @ 2021-08-19 17:04 UTC (permalink / raw)
To: Alexandre Belloni, devicetree
Cc: UNGLinuxDriver, Horatiu Vultur, Rob Herring, Thomas Bogendoerfer,
linux-mips, linux-kernel
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH devicetree 2/2] MIPS: mscc: ocelot: mark the phy-mode for internal PHY ports
2021-08-19 17:04 [PATCH devicetree 1/2] MIPS: mscc: ocelot: disable all switch ports by default Vladimir Oltean
@ 2021-08-19 17:04 ` Vladimir Oltean
2021-08-19 17:17 ` Vladimir Oltean
2021-08-21 8:42 ` Thomas Bogendoerfer
2021-08-21 8:41 ` [PATCH devicetree 1/2] MIPS: mscc: ocelot: disable all switch ports by default Thomas Bogendoerfer
1 sibling, 2 replies; 6+ messages in thread
From: Vladimir Oltean @ 2021-08-19 17:04 UTC (permalink / raw)
To: Alexandre Belloni, devicetree
Cc: UNGLinuxDriver, Horatiu Vultur, Rob Herring, Thomas Bogendoerfer,
linux-mips, linux-kernel
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH devicetree 2/2] MIPS: mscc: ocelot: mark the phy-mode for internal PHY ports
2021-08-19 17:04 ` [PATCH devicetree 2/2] MIPS: mscc: ocelot: mark the phy-mode for internal PHY ports Vladimir Oltean
@ 2021-08-19 17:17 ` Vladimir Oltean
2021-08-19 17:58 ` Alexandre Belloni
2021-08-21 8:42 ` Thomas Bogendoerfer
1 sibling, 1 reply; 6+ messages in thread
From: Vladimir Oltean @ 2021-08-19 17:17 UTC (permalink / raw)
To: Alexandre Belloni, devicetree
Cc: UNGLinuxDriver, Horatiu Vultur, Rob Herring, Thomas Bogendoerfer,
linux-mips, linux-kernel
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?
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH devicetree 2/2] MIPS: mscc: ocelot: mark the phy-mode for internal PHY ports
2021-08-19 17:17 ` Vladimir Oltean
@ 2021-08-19 17:58 ` Alexandre Belloni
0 siblings, 0 replies; 6+ messages in thread
From: Alexandre Belloni @ 2021-08-19 17:58 UTC (permalink / raw)
To: Vladimir Oltean
Cc: devicetree, UNGLinuxDriver, Horatiu Vultur, Rob Herring,
Thomas Bogendoerfer, linux-mips, linux-kernel
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH devicetree 2/2] MIPS: mscc: ocelot: mark the phy-mode for internal PHY ports
2021-08-19 17:04 ` [PATCH devicetree 2/2] MIPS: mscc: ocelot: mark the phy-mode for internal PHY ports Vladimir Oltean
2021-08-19 17:17 ` Vladimir Oltean
@ 2021-08-21 8:42 ` Thomas Bogendoerfer
1 sibling, 0 replies; 6+ messages in thread
From: Thomas Bogendoerfer @ 2021-08-21 8:42 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Alexandre Belloni, devicetree, UNGLinuxDriver, Horatiu Vultur,
Rob Herring, linux-mips, linux-kernel
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 ]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH devicetree 1/2] MIPS: mscc: ocelot: disable all switch ports by default
2021-08-19 17:04 [PATCH devicetree 1/2] MIPS: mscc: ocelot: disable all switch ports by default Vladimir Oltean
2021-08-19 17:04 ` [PATCH devicetree 2/2] MIPS: mscc: ocelot: mark the phy-mode for internal PHY ports Vladimir Oltean
@ 2021-08-21 8:41 ` Thomas Bogendoerfer
1 sibling, 0 replies; 6+ messages in thread
From: Thomas Bogendoerfer @ 2021-08-21 8:41 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Alexandre Belloni, devicetree, UNGLinuxDriver, Horatiu Vultur,
Rob Herring, linux-mips, linux-kernel
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 ]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-08-21 9:25 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-19 17:04 [PATCH devicetree 1/2] MIPS: mscc: ocelot: disable all switch ports by default Vladimir Oltean
2021-08-19 17:04 ` [PATCH devicetree 2/2] MIPS: mscc: ocelot: mark the phy-mode for internal PHY ports Vladimir Oltean
2021-08-19 17:17 ` Vladimir Oltean
2021-08-19 17:58 ` Alexandre Belloni
2021-08-21 8:42 ` Thomas Bogendoerfer
2021-08-21 8:41 ` [PATCH devicetree 1/2] MIPS: mscc: ocelot: disable all switch ports by default Thomas Bogendoerfer
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.