[PATCH 0/3] Add missing DSA properties for marvell switches

[PATCH 0/3] Add missing DSA properties for marvell switches

[Andrew Lunn]

The DSA core has become more picky about DT properties. This patchset
add missing properties and removes some unused ones, for iMX boards.

Once all the missing properties are added, it should be possible to
simply phylink and the mv88e6xxx driver.

Andrew Lunn (3):
  ARM: dts: imx51: ZII: Add missing phy-mode
  ARM: dts: imx6qdl: Add missing phy-mode and fixed links
  ARM64: dts: freescale: ZII: Add missing phy-mode

 arch/arm/boot/dts/imx51-zii-rdu1.dts                | 2 +-
 arch/arm/boot/dts/imx51-zii-scu2-mezz.dts           | 2 +-
 arch/arm/boot/dts/imx51-zii-scu3-esb.dts            | 1 -
 arch/arm/boot/dts/imx6qdl-gw5904.dtsi               | 7 ++++++-
 arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi             | 2 +-
 arch/arm64/boot/dts/freescale/imx8mq-zii-ultra.dtsi | 2 +-
 6 files changed, 10 insertions(+), 6 deletions(-)

-- 
2.40.0

[PATCH 1/3] ARM: dts: imx51: ZII: Add missing phy-mode

[Andrew Lunn]

The DSA framework has got more picky about always having a phy-mode
for the CPU port. The imx51 Ethernet supports MII, and RMII. Set the
switch phy-mode based on how the SoC Ethernet port has been
configured.

Additionally, the cpu label has never actually been used in the
binding, so remove it.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 arch/arm/boot/dts/imx51-zii-rdu1.dts      | 2 +-
 arch/arm/boot/dts/imx51-zii-scu2-mezz.dts | 2 +-
 arch/arm/boot/dts/imx51-zii-scu3-esb.dts  | 1 -
 3 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/imx51-zii-rdu1.dts b/arch/arm/boot/dts/imx51-zii-rdu1.dts
index e537e06e11d7..8621760af1af 100644
--- a/arch/arm/boot/dts/imx51-zii-rdu1.dts
+++ b/arch/arm/boot/dts/imx51-zii-rdu1.dts
@@ -181,7 +181,7 @@ ports {
 
 				port@0 {
 					reg = <0>;
-					label = "cpu";
+					phy-mode = "mii";
 					ethernet = <&fec>;
 
 					fixed-link {
diff --git a/arch/arm/boot/dts/imx51-zii-scu2-mezz.dts b/arch/arm/boot/dts/imx51-zii-scu2-mezz.dts
index 21dd3f7abd48..883e80d92ef0 100644
--- a/arch/arm/boot/dts/imx51-zii-scu2-mezz.dts
+++ b/arch/arm/boot/dts/imx51-zii-scu2-mezz.dts
@@ -82,7 +82,7 @@ port@3 {
 
 				port@4 {
 					reg = <4>;
-					label = "cpu";
+					phy-mode = "mii";
 					ethernet = <&fec>;
 
 					fixed-link {
diff --git a/arch/arm/boot/dts/imx51-zii-scu3-esb.dts b/arch/arm/boot/dts/imx51-zii-scu3-esb.dts
index 9f857eb44bf7..19a3b142c964 100644
--- a/arch/arm/boot/dts/imx51-zii-scu3-esb.dts
+++ b/arch/arm/boot/dts/imx51-zii-scu3-esb.dts
@@ -267,7 +267,6 @@ fixed-link {
 
 				port@6 {
 					reg = <6>;
-					label = "cpu";
 					phy-mode = "mii";
 					ethernet = <&fec>;
 
-- 
2.40.0

[PATCH 2/3] ARM: dts: imx6qdl: Add missing phy-mode and fixed links

[Andrew Lunn]

The DSA framework has got more picky about always having a phy-mode
for the CPU port. Add a phy-mode based on what the SoC ethernet is
using. For RGMII mode, have the switch add the delays.

Additionally, the cpu label has never actually been used in the
binding, so remove it.

Lastly add a fixed-link node indicating the expected speed/duplex of
the link to the SoC.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 arch/arm/boot/dts/imx6qdl-gw5904.dtsi   | 7 ++++++-
 arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi | 2 +-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/imx6qdl-gw5904.dtsi b/arch/arm/boot/dts/imx6qdl-gw5904.dtsi
index 9fc79af2bc9a..9594bc5745ed 100644
--- a/arch/arm/boot/dts/imx6qdl-gw5904.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw5904.dtsi
@@ -238,8 +238,13 @@ port@3 {
 
 				port@5 {
 					reg = <5>;
-					label = "cpu";
 					ethernet = <&fec>;
+					phy-mode = "rgmii-id";
+
+					fixed-link {
+						speed = <1000>;
+						full-duplex;
+					};
 				};
 			};
 		};
diff --git a/arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi b/arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi
index 5bb47c79a4da..826a9d6cb4d8 100644
--- a/arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi
@@ -757,7 +757,7 @@ port@1 {
 
 				port@2 {
 					reg = <2>;
-					label = "cpu";
+					phy-mode = "rmii";
 					ethernet = <&fec>;
 
 					fixed-link {
-- 
2.40.0

[PATCH 3/3] ARM64: dts: freescale: ZII: Add missing phy-mode

[Andrew Lunn]

The DSA framework has got more picky about always having a phy-mode
for the CPU port. The imx8mq Ethernet is being configured to RMII. Set
the switch phy-mode based on this.

Additionally, the cpu label has never actually been used in the
binding, so remove it.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 arch/arm64/boot/dts/freescale/imx8mq-zii-ultra.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mq-zii-ultra.dtsi b/arch/arm64/boot/dts/freescale/imx8mq-zii-ultra.dtsi
index 3a52679ecd68..3bf7850fbe9c 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq-zii-ultra.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq-zii-ultra.dtsi
@@ -177,7 +177,7 @@ port@1 {
 
 				port@2 {
 					reg = <2>;
-					label = "cpu";
+					phy-mode = "rmii";
 					ethernet = <&fec1>;
 
 					fixed-link {
-- 
2.40.0

Re: [PATCH 1/3] ARM: dts: imx51: ZII: Add missing phy-mode

[Vladimir Oltean]

On Fri, Apr 07, 2023 at 05:25:01PM +0200, Andrew Lunn wrote:
> The DSA framework has got more picky about always having a phy-mode
> for the CPU port. The imx51 Ethernet supports MII, and RMII. Set the
> switch phy-mode based on how the SoC Ethernet port has been
> configured.
> 
> Additionally, the cpu label has never actually been used in the
> binding, so remove it.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---

In theory, an MII MAC-to-MAC connection should have phy-mode = "mii" on
one end and phy-mode = "rev-mii" on the other, right?

Re: [PATCH 1/3] ARM: dts: imx51: ZII: Add missing phy-mode

[Andrew Lunn]

On Fri, Apr 07, 2023 at 06:41:59PM +0300, Vladimir Oltean wrote:
> On Fri, Apr 07, 2023 at 05:25:01PM +0200, Andrew Lunn wrote:
> > The DSA framework has got more picky about always having a phy-mode
> > for the CPU port. The imx51 Ethernet supports MII, and RMII. Set the
> > switch phy-mode based on how the SoC Ethernet port has been
> > configured.
> > 
> > Additionally, the cpu label has never actually been used in the
> > binding, so remove it.
> > 
> > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> > ---
> 
> In theory, an MII MAC-to-MAC connection should have phy-mode = "mii" on
> one end and phy-mode = "rev-mii" on the other, right?

In theory, yes. As far as i understand, it makes a difference to where
the clock comes from. rev-mii is a clock provider i think.

But from what i understand of the code, and the silicon, this property
is going to be ignored, whatever value you give it. phy-mode is only
used and respected when the port can support 1000Base-X, SGMII, and
above, or use its built in PHY. For MII, GMII, RMII, RGMII the port
setting is determined by strapping resistors.

The DSA core however does care that there is a phy-mode, even if it is
ignored. I hope after these patches land we can turn that check into
enforce mode, and that then unlocks Russell to make phylink
improvement.

	Andrew

Re: [PATCH 1/3] ARM: dts: imx51: ZII: Add missing phy-mode

[Vladimir Oltean]

On Fri, Apr 07, 2023 at 06:10:31PM +0200, Andrew Lunn wrote:
> On Fri, Apr 07, 2023 at 06:41:59PM +0300, Vladimir Oltean wrote:
> > On Fri, Apr 07, 2023 at 05:25:01PM +0200, Andrew Lunn wrote:
> > > The DSA framework has got more picky about always having a phy-mode
> > > for the CPU port. The imx51 Ethernet supports MII, and RMII. Set the
> > > switch phy-mode based on how the SoC Ethernet port has been
> > > configured.
> > > 
> > > Additionally, the cpu label has never actually been used in the
> > > binding, so remove it.
> > > 
> > > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> > > ---
> > 
> > In theory, an MII MAC-to-MAC connection should have phy-mode = "mii" on
> > one end and phy-mode = "rev-mii" on the other, right?
> 
> In theory, yes. As far as i understand, it makes a difference to where
> the clock comes from. rev-mii is a clock provider i think.
> 
> But from what i understand of the code, and the silicon, this property
> is going to be ignored, whatever value you give it. phy-mode is only
> used and respected when the port can support 1000Base-X, SGMII, and
> above, or use its built in PHY. For MII, GMII, RMII, RGMII the port
> setting is determined by strapping resistors.

If it's ignored, even better, one more reason to make it rev-mii and
not mii, no compatibility concerns with the driver not understanding the
difference...

> The DSA core however does care that there is a phy-mode, even if it is
> ignored. I hope after these patches land we can turn that check into
> enforce mode, and that then unlocks Russell to make phylink
> improvement.
> 
> 	Andrew
>

Re: [PATCH 1/3] ARM: dts: imx51: ZII: Add missing phy-mode

[Andrew Lunn]

> If it's ignored, even better, one more reason to make it rev-mii and
> not mii, no compatibility concerns with the driver not understanding the
> difference...

O.K. i will respin the patch tomorrow.

     Andrew

Re: [PATCH 1/3] ARM: dts: imx51: ZII: Add missing phy-mode

[Vladimir Oltean]

On Fri, Apr 07, 2023 at 06:10:31PM +0200, Andrew Lunn wrote:
> On Fri, Apr 07, 2023 at 06:41:59PM +0300, Vladimir Oltean wrote:
> > In theory, an MII MAC-to-MAC connection should have phy-mode = "mii" on
> > one end and phy-mode = "rev-mii" on the other, right?
> 
> In theory, yes. As far as i understand, it makes a difference to where
> the clock comes from. rev-mii is a clock provider i think.
> 
> But from what i understand of the code, and the silicon, this property
> is going to be ignored, whatever value you give it. phy-mode is only
> used and respected when the port can support 1000Base-X, SGMII, and
> above, or use its built in PHY. For MII, GMII, RMII, RGMII the port
> setting is determined by strapping resistors.
> 
> The DSA core however does care that there is a phy-mode, even if it is
> ignored. I hope after these patches land we can turn that check into
> enforce mode, and that then unlocks Russell to make phylink
> improvement.

Actually, looking at mv88e6xxx_translate_cmode() right now, I guess it's
not exactly true that the value is going to be ignored, whatever it is.
A CMODE of MV88E6XXX_PORT_STS_CMODE_MII_PHY is not going to be translated
into "rev-mii", but into "mii", same as MV88E6XXX_PORT_STS_CMODE_MII.
Same for MV88E6XXX_PORT_STS_CMODE_RMII_PHY ("rmii" and not "rev-rmii").
So, when given "rev-mii" or "rev-rmii" as phy modes in the device tree,
the generic phylink validation procedure should reject them for being
unsupported.

This means either the patch set moves forward with v1, or the driver is
fixed to accept the dedicated PHY modes for PHY roles.

Russell, what do you think?

Re: [PATCH 1/3] ARM: dts: imx51: ZII: Add missing phy-mode

[Russell King (Oracle)]

On Mon, Apr 10, 2023 at 01:00:12PM +0300, Vladimir Oltean wrote:
> On Fri, Apr 07, 2023 at 06:10:31PM +0200, Andrew Lunn wrote:
> > On Fri, Apr 07, 2023 at 06:41:59PM +0300, Vladimir Oltean wrote:
> > > In theory, an MII MAC-to-MAC connection should have phy-mode = "mii" on
> > > one end and phy-mode = "rev-mii" on the other, right?
> > 
> > In theory, yes. As far as i understand, it makes a difference to where
> > the clock comes from. rev-mii is a clock provider i think.
> > 
> > But from what i understand of the code, and the silicon, this property
> > is going to be ignored, whatever value you give it. phy-mode is only
> > used and respected when the port can support 1000Base-X, SGMII, and
> > above, or use its built in PHY. For MII, GMII, RMII, RGMII the port
> > setting is determined by strapping resistors.
> > 
> > The DSA core however does care that there is a phy-mode, even if it is
> > ignored. I hope after these patches land we can turn that check into
> > enforce mode, and that then unlocks Russell to make phylink
> > improvement.
> 
> Actually, looking at mv88e6xxx_translate_cmode() right now, I guess it's
> not exactly true that the value is going to be ignored, whatever it is.
> A CMODE of MV88E6XXX_PORT_STS_CMODE_MII_PHY is not going to be translated
> into "rev-mii", but into "mii", same as MV88E6XXX_PORT_STS_CMODE_MII.
> Same for MV88E6XXX_PORT_STS_CMODE_RMII_PHY ("rmii" and not "rev-rmii").
> So, when given "rev-mii" or "rev-rmii" as phy modes in the device tree,
> the generic phylink validation procedure should reject them for being
> unsupported.
> 
> This means either the patch set moves forward with v1, or the driver is
> fixed to accept the dedicated PHY modes for PHY roles.
> 
> Russell, what do you think?

I'm afraid I didn't bother trying to understand all the *MII modes
that Marvell document poorly in their functional specification,
especially when they document that e.g. RMII_PHY mode can also be
used to connect to a PHY. I took their table with a pinch of salt
and did what I thought was best.

It is entirely possible that some are wrong, especially as we don't
document what the various PHY_INTERFACE_MODE_* mean in the kernel
(remember that I started doing that).

As far as phylink, it treats REV*MII and the corresponding *MII
mode the same way in terms of their capabilities, but as you say
it will determine which mode will be acceptable.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH 1/3] ARM: dts: imx51: ZII: Add missing phy-mode

[Andrew Lunn]

> Actually, looking at mv88e6xxx_translate_cmode() right now, I guess it's
> not exactly true that the value is going to be ignored, whatever it is.
> A CMODE of MV88E6XXX_PORT_STS_CMODE_MII_PHY is not going to be translated
> into "rev-mii", but into "mii", same as MV88E6XXX_PORT_STS_CMODE_MII.
> Same for MV88E6XXX_PORT_STS_CMODE_RMII_PHY ("rmii" and not "rev-rmii").
> So, when given "rev-mii" or "rev-rmii" as phy modes in the device tree,
> the generic phylink validation procedure should reject them for being
> unsupported.

Ah. I did not actually test this version on hardware. I was expecting
it to be ignored, since the cmode cannot be changed. I did not think
about phylink performing validation.

I will boot of one these DT files on hardware to see what happens.

	Andrew

Re: [PATCH 1/3] ARM: dts: imx51: ZII: Add missing phy-mode

[Andrew Lunn]

On Mon, Apr 10, 2023 at 01:00:12PM +0300, Vladimir Oltean wrote:
> On Fri, Apr 07, 2023 at 06:10:31PM +0200, Andrew Lunn wrote:
> > On Fri, Apr 07, 2023 at 06:41:59PM +0300, Vladimir Oltean wrote:
> > > In theory, an MII MAC-to-MAC connection should have phy-mode = "mii" on
> > > one end and phy-mode = "rev-mii" on the other, right?
> > 
> > In theory, yes. As far as i understand, it makes a difference to where
> > the clock comes from. rev-mii is a clock provider i think.
> > 
> > But from what i understand of the code, and the silicon, this property
> > is going to be ignored, whatever value you give it. phy-mode is only
> > used and respected when the port can support 1000Base-X, SGMII, and
> > above, or use its built in PHY. For MII, GMII, RMII, RGMII the port
> > setting is determined by strapping resistors.
> > 
> > The DSA core however does care that there is a phy-mode, even if it is
> > ignored. I hope after these patches land we can turn that check into
> > enforce mode, and that then unlocks Russell to make phylink
> > improvement.
> 
> Actually, looking at mv88e6xxx_translate_cmode() right now, I guess it's
> not exactly true that the value is going to be ignored, whatever it is.
> A CMODE of MV88E6XXX_PORT_STS_CMODE_MII_PHY is not going to be translated
> into "rev-mii", but into "mii", same as MV88E6XXX_PORT_STS_CMODE_MII.
> Same for MV88E6XXX_PORT_STS_CMODE_RMII_PHY ("rmii" and not "rev-rmii").
> So, when given "rev-mii" or "rev-rmii" as phy modes in the device tree,
> the generic phylink validation procedure should reject them for being
> unsupported.

I tested vf610-zii-devel-rev-c.dtb:

[    2.307416] mv88e6085 mdio_mux-0.1:00: configuring for fixed/rev-rmii link mode
[    2.314459] mv88e6085 mdio_mux-0.1:00: configuring for fixed/xaui link mode
[    2.320588] mv88e6085 mdio_mux-0.1:00: Link is Up - 100Mbps/Full - flow control off
[    2.327722] mv88e6085 mdio_mux-0.2:00: configuring for fixed/xaui link mode
[    2.334729] mv88e6085 mdio_mux-0.2:00: Link is Up - 10Gbps/Full - flow control off
[    2.343263] mv88e6085 mdio_mux-0.1:00: Link is Up - 10Gbps/Full - flow control off
[    2.396110] mv88e6085 mdio_mux-0.1:00 lan1 (uninitialized): PHY [!mdio-mux!mdio@1!switch@0!mdio:01] driver [Marvell 88E6390 Fa)
[    2.498137] mv88e6085 mdio_mux-0.1:00 lan2 (uninitialized): PHY [!mdio-mux!mdio@1!switch@0!mdio:02] driver [Marvell 88E6390 Fa)
[    2.566028] mv88e6085 mdio_mux-0.1:00 lan3 (uninitialized): PHY [!mdio-mux!mdio@1!switch@0!mdio:03] driver [Marvell 88E6390 Fa)
[    2.663995] mv88e6085 mdio_mux-0.1:00 lan4 (uninitialized): PHY [!mdio-mux!mdio@1!switch@0!mdio:04] driver [Marvell 88E6390 Fa)
[    2.746064] mv88e6085 mdio_mux-0.2:00 lan5 (uninitialized): PHY [!mdio-mux!mdio@2!switch@0!mdio:01] driver [Marvell 88E6390 Fa)
[    2.834000] mv88e6085 mdio_mux-0.2:00 lan6 (uninitialized): PHY [!mdio-mux!mdio@2!switch@0!mdio:02] driver [Marvell 88E6390 Fa)
[    2.933998] mv88e6085 mdio_mux-0.2:00 lan7 (uninitialized): PHY [!mdio-mux!mdio@2!switch@0!mdio:03] driver [Marvell 88E6390 Fa)
[    3.016031] mv88e6085 mdio_mux-0.2:00 lan8 (uninitialized): PHY [!mdio-mux!mdio@2!switch@0!mdio:04] driver [Marvell 88E6390 Fa)
[    3.033976] mv88e6085 mdio_mux-0.2:00 sff2 (uninitialized): switched to inband/2500base-x link mode

So we can see link mode rev-rmii and there are no errors.

Testing a board using mii, not rmii is going to be a bit harder. I
don't have one set up right now.

      Andrew

Re: [PATCH 1/3] ARM: dts: imx51: ZII: Add missing phy-mode

[Vladimir Oltean]

On Mon, Apr 10, 2023 at 02:35:28PM +0200, Andrew Lunn wrote:
> I tested vf610-zii-devel-rev-c.dtb:
> 
> [    2.307416] mv88e6085 mdio_mux-0.1:00: configuring for fixed/rev-rmii link mode
> [    2.314459] mv88e6085 mdio_mux-0.1:00: configuring for fixed/xaui link mode
> [    2.320588] mv88e6085 mdio_mux-0.1:00: Link is Up - 100Mbps/Full - flow control off
> [    2.327722] mv88e6085 mdio_mux-0.2:00: configuring for fixed/xaui link mode
> [    2.334729] mv88e6085 mdio_mux-0.2:00: Link is Up - 10Gbps/Full - flow control off
> [    2.343263] mv88e6085 mdio_mux-0.1:00: Link is Up - 10Gbps/Full - flow control off
> [    2.396110] mv88e6085 mdio_mux-0.1:00 lan1 (uninitialized): PHY [!mdio-mux!mdio@1!switch@0!mdio:01] driver [Marvell 88E6390 Fa)
> [    2.498137] mv88e6085 mdio_mux-0.1:00 lan2 (uninitialized): PHY [!mdio-mux!mdio@1!switch@0!mdio:02] driver [Marvell 88E6390 Fa)
> [    2.566028] mv88e6085 mdio_mux-0.1:00 lan3 (uninitialized): PHY [!mdio-mux!mdio@1!switch@0!mdio:03] driver [Marvell 88E6390 Fa)
> [    2.663995] mv88e6085 mdio_mux-0.1:00 lan4 (uninitialized): PHY [!mdio-mux!mdio@1!switch@0!mdio:04] driver [Marvell 88E6390 Fa)
> [    2.746064] mv88e6085 mdio_mux-0.2:00 lan5 (uninitialized): PHY [!mdio-mux!mdio@2!switch@0!mdio:01] driver [Marvell 88E6390 Fa)
> [    2.834000] mv88e6085 mdio_mux-0.2:00 lan6 (uninitialized): PHY [!mdio-mux!mdio@2!switch@0!mdio:02] driver [Marvell 88E6390 Fa)
> [    2.933998] mv88e6085 mdio_mux-0.2:00 lan7 (uninitialized): PHY [!mdio-mux!mdio@2!switch@0!mdio:03] driver [Marvell 88E6390 Fa)
> [    3.016031] mv88e6085 mdio_mux-0.2:00 lan8 (uninitialized): PHY [!mdio-mux!mdio@2!switch@0!mdio:04] driver [Marvell 88E6390 Fa)
> [    3.033976] mv88e6085 mdio_mux-0.2:00 sff2 (uninitialized): switched to inband/2500base-x link mode
> 
> So we can see link mode rev-rmii and there are no errors.
> 
> Testing a board using mii, not rmii is going to be a bit harder. I
> don't have one set up right now.

hmmm... why does this work?

would you mind adding this small debug print and booting again?

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index a4111f1be375..0b9754d4db80 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -695,6 +695,10 @@ static int phylink_validate(struct phylink *pl, unsigned long *supported,
 {
 	const unsigned long *interfaces = pl->config->supported_interfaces;
 
+	phylink_err(pl, "%s: supported_interfaces=%*pbl, interface %s\n",
+		    __func__, (int)PHY_INTERFACE_MODE_MAX,
+		    interfaces, phy_modes(state->interface));
+
 	if (!phy_interface_empty(interfaces)) {
 		if (state->interface == PHY_INTERFACE_MODE_NA)
 			return phylink_validate_mask(pl, supported, state,

I would've thought this check below would fail:

		if (!test_bit(state->interface, interfaces))
			return -EINVAL;

Re: [PATCH 1/3] ARM: dts: imx51: ZII: Add missing phy-mode

[Andrew Lunn]

> hmmm... why does this work?
> 
> would you mind adding this small debug print and booting again?
 
mv88e6xxx_translate_cmode: cmode 4, supported: supported=7

CMODE 4 is 'RMII PHY' or 'RMII to PHY', depending on if it has found a
PHY or not.

mv88e6085 mdio_mux-0.1:00: phylink_validate: supported_interfaces=1,3,7, interface rev-rmii
mv88e6085 mdio_mux-0.1:00: phylink_validate: supported_interfaces=1,3,7, interface rev-rmii
mv88e6085 mdio_mux-0.1:00: configuring for fixed/rev-rmii link mode

Both calls to phylink_validate() then returning -EINVAL.

The first call is from phylink_create(), which does not check the
return value.

The second call is from phylink_parse_fixedlink(), which also does not
check the return code.

This is a bit fragile, all it would need is for these return values to
be checked and it would break. So i think cmode 4 should return
REVRMII and cmode 5 shoud be RMII. I will submit a patch making this
change.

	Andrew

Re: [PATCH 1/3] ARM: dts: imx51: ZII: Add missing phy-mode

[Vladimir Oltean]

On Mon, Apr 10, 2023 at 11:37:15AM +0100, Russell King (Oracle) wrote:
> It is entirely possible that some are wrong, especially as we don't
> document what the various PHY_INTERFACE_MODE_* mean in the kernel
> (remember that I started doing that).

include/linux/phy.h does give a small hint about what is expected:

 * @PHY_INTERFACE_MODE_REVRMII: Reduced Media Independent Interface in PHY role

and the commit which added that mode does reference, in its description,
the conversation that led to its creation:
https://lore.kernel.org/netdev/20210201214515.cx6ivvme2tlquge2@skbuf/