| Message ID | 20210819170416.2252210-2-vladimir.oltean@nxp.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | eba54cbb92d28b4f6dc1ed5f73f5187b09d82c08 |
| Headers | show |
| Series | [devicetree,1/2] MIPS: mscc: ocelot: disable all switch ports by default | expand |
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?
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.
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.
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"; };
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(+)