[PATCH v3] net: dsa: mv88e6xxx: support RGMII cmode

[PATCH v3] net: dsa: mv88e6xxx: support RGMII cmode

[Marcus Carlberg]

Since the probe defaults all interfaces to the highest speed possible
(10GBASE-X in mv88e6393x) before the phy mode configuration from the
devicetree is considered it is currently impossible to use port 0 in
RGMII mode.

This change will allow RGMII modes to be configurable for port 0
enabling port 0 to be configured as RGMII as well as serial depending
on configuration.

Fixes: de776d0d316f ("net: dsa: mv88e6xxx: add support for mv88e6393x family")
Signed-off-by: Marcus Carlberg <marcus.carlberg@axis.com>
---

Notes:
    v2: add phy mode input validation for SERDES only ports
    
    v3: add RGMII phy interface types to supported phy modes list.
        add fixes tag.

 drivers/net/dsa/mv88e6xxx/chip.c |  8 ++++++++
 drivers/net/dsa/mv88e6xxx/port.c | 19 +++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 07e9a4da924c..6403f1f8bdbb 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -816,6 +816,14 @@ static void mv88e6393x_phylink_get_caps(struct mv88e6xxx_chip *chip, int port,
 				MAC_10000FD;
 		}
 	}
+
+	if (port == 0) {
+		__set_bit(PHY_INTERFACE_MODE_RMII, supported);
+		__set_bit(PHY_INTERFACE_MODE_RGMII, supported);
+		__set_bit(PHY_INTERFACE_MODE_RGMII_ID, supported);
+		__set_bit(PHY_INTERFACE_MODE_RGMII_RXID, supported);
+		__set_bit(PHY_INTERFACE_MODE_RGMII_TXID, supported);
+	}
 }
 
 static void mv88e6xxx_get_caps(struct dsa_switch *ds, int port,
diff --git a/drivers/net/dsa/mv88e6xxx/port.c b/drivers/net/dsa/mv88e6xxx/port.c
index 90c55f23b7c9..5c4195c635b0 100644
--- a/drivers/net/dsa/mv88e6xxx/port.c
+++ b/drivers/net/dsa/mv88e6xxx/port.c
@@ -517,6 +517,12 @@ static int mv88e6xxx_port_set_cmode(struct mv88e6xxx_chip *chip, int port,
 	case PHY_INTERFACE_MODE_RMII:
 		cmode = MV88E6XXX_PORT_STS_CMODE_RMII;
 		break;
+	case PHY_INTERFACE_MODE_RGMII:
+	case PHY_INTERFACE_MODE_RGMII_ID:
+	case PHY_INTERFACE_MODE_RGMII_RXID:
+	case PHY_INTERFACE_MODE_RGMII_TXID:
+		cmode = MV88E6XXX_PORT_STS_CMODE_RGMII;
+		break;
 	case PHY_INTERFACE_MODE_1000BASEX:
 		cmode = MV88E6XXX_PORT_STS_CMODE_1000BASEX;
 		break;
@@ -634,6 +640,19 @@ int mv88e6393x_port_set_cmode(struct mv88e6xxx_chip *chip, int port,
 	if (port != 0 && port != 9 && port != 10)
 		return -EOPNOTSUPP;
 
+	if (port == 9 || port == 10) {
+		switch (mode) {
+		case PHY_INTERFACE_MODE_RMII:
+		case PHY_INTERFACE_MODE_RGMII:
+		case PHY_INTERFACE_MODE_RGMII_ID:
+		case PHY_INTERFACE_MODE_RGMII_RXID:
+		case PHY_INTERFACE_MODE_RGMII_TXID:
+			return -EINVAL;
+		default:
+			break;
+		}
+	}
+
 	/* mv88e6393x errata 4.5: EEE should be disabled on SERDES ports */
 	err = mv88e6xxx_port_read(chip, port, MV88E6XXX_PORT_MAC_CTL, &reg);
 	if (err)
-- 
2.20.1

Re: [PATCH v3] net: dsa: mv88e6xxx: support RGMII cmode

[Jakub Kicinski]

On Mon, 22 Aug 2022 16:41:36 +0200 Marcus Carlberg wrote:
> Since the probe defaults all interfaces to the highest speed possible
> (10GBASE-X in mv88e6393x) before the phy mode configuration from the
> devicetree is considered it is currently impossible to use port 0 in
> RGMII mode.
> 
> This change will allow RGMII modes to be configurable for port 0
> enabling port 0 to be configured as RGMII as well as serial depending
> on configuration.
> 
> Fixes: de776d0d316f ("net: dsa: mv88e6xxx: add support for mv88e6393x family")
> Signed-off-by: Marcus Carlberg <marcus.carlberg@axis.com>

Seems like a new configuration which was not previously supported
rather than a regression, right? If so I'll drop the Fixes tag
when applying.

Re: [PATCH v3] net: dsa: mv88e6xxx: support RGMII cmode

[Marek Behún]

On Thu, 25 Aug 2022 12:38:07 -0700
Jakub Kicinski <kuba@kernel.org> wrote:

> On Mon, 22 Aug 2022 16:41:36 +0200 Marcus Carlberg wrote:
> > Since the probe defaults all interfaces to the highest speed possible
> > (10GBASE-X in mv88e6393x) before the phy mode configuration from the
> > devicetree is considered it is currently impossible to use port 0 in
> > RGMII mode.
> > 
> > This change will allow RGMII modes to be configurable for port 0
> > enabling port 0 to be configured as RGMII as well as serial depending
> > on configuration.
> > 
> > Fixes: de776d0d316f ("net: dsa: mv88e6xxx: add support for mv88e6393x family")
> > Signed-off-by: Marcus Carlberg <marcus.carlberg@axis.com>  
> 
> Seems like a new configuration which was not previously supported
> rather than a regression, right? If so I'll drop the Fixes tag
> when applying.

Please leave the fixes tag. This configuration should have been
supported from the beginning.

Marek

Re: [PATCH v3] net: dsa: mv88e6xxx: support RGMII cmode

[Jakub Kicinski]

On Fri, 26 Aug 2022 00:06:05 +0200 Marek Behún wrote:
> > On Mon, 22 Aug 2022 16:41:36 +0200 Marcus Carlberg wrote:  
> > > Since the probe defaults all interfaces to the highest speed possible
> > > (10GBASE-X in mv88e6393x) before the phy mode configuration from the
> > > devicetree is considered it is currently impossible to use port 0 in
> > > RGMII mode.
> > > 
> > > This change will allow RGMII modes to be configurable for port 0
> > > enabling port 0 to be configured as RGMII as well as serial depending
> > > on configuration.
> > > 
> > > Fixes: de776d0d316f ("net: dsa: mv88e6xxx: add support for mv88e6393x family")
> > > Signed-off-by: Marcus Carlberg <marcus.carlberg@axis.com>    
> > 
> > Seems like a new configuration which was not previously supported
> > rather than a regression, right? If so I'll drop the Fixes tag
> > when applying.  
> 
> Please leave the fixes tag. This configuration should have been
> supported from the beginning.

Could you explain why? Is there an upstream-supported platform
already in Linus's tree which doesn't boot or something?

Re: [PATCH v3] net: dsa: mv88e6xxx: support RGMII cmode

[Marek Behún]

On Thu, 25 Aug 2022 15:51:40 -0700
Jakub Kicinski <kuba@kernel.org> wrote:

> On Fri, 26 Aug 2022 00:06:05 +0200 Marek Behún wrote:
> > > On Mon, 22 Aug 2022 16:41:36 +0200 Marcus Carlberg wrote:    
> > > > Since the probe defaults all interfaces to the highest speed possible
> > > > (10GBASE-X in mv88e6393x) before the phy mode configuration from the
> > > > devicetree is considered it is currently impossible to use port 0 in
> > > > RGMII mode.
> > > > 
> > > > This change will allow RGMII modes to be configurable for port 0
> > > > enabling port 0 to be configured as RGMII as well as serial depending
> > > > on configuration.
> > > > 
> > > > Fixes: de776d0d316f ("net: dsa: mv88e6xxx: add support for mv88e6393x family")
> > > > Signed-off-by: Marcus Carlberg <marcus.carlberg@axis.com>      
> > > 
> > > Seems like a new configuration which was not previously supported
> > > rather than a regression, right? If so I'll drop the Fixes tag
> > > when applying.    
> > 
> > Please leave the fixes tag. This configuration should have been
> > supported from the beginning.  
> 
> Could you explain why? Is there an upstream-supported platform
> already in Linus's tree which doesn't boot or something?

If you mean whether there is a device-tree of such a device, they I
don't think so, because AFAIK there isn't a device-tree with 6393 in
upstream Linux other than CN9130-CRB.

But it is possible though that there is such a device which has
everything but the switch supported on older kernels, due to this RGMII
bug.

I think RGMII should have been supported on this switch when I send the
patch adding support for it, and it is a bug that it is not, becuase
RGMII is supported for similar switches driven by mv88e6xxx driver
(6390, for example). I don't know why I overlooked it then.

Note that I wouldn't consider adding support for USXGMII a fix, because
although the switch can do it, it was never done with this driver.

But if you think it doesn't apply anyway, remove the Fixes tag. This is
just my opinion that it should stay.

Marek

Re: [PATCH v3] net: dsa: mv88e6xxx: support RGMII cmode

[Jakub Kicinski]

On Fri, 26 Aug 2022 01:26:59 +0200 Marek Behún wrote:
> > Could you explain why? Is there an upstream-supported platform
> > already in Linus's tree which doesn't boot or something?  
> 
> If you mean whether there is a device-tree of such a device, they I
> don't think so, because AFAIK there isn't a device-tree with 6393 in
> upstream Linux other than CN9130-CRB.
> 
> But it is possible though that there is such a device which has
> everything but the switch supported on older kernels, due to this RGMII
> bug.
> 
> I think RGMII should have been supported on this switch when I send the
> patch adding support for it, and it is a bug that it is not, becuase
> RGMII is supported for similar switches driven by mv88e6xxx driver
> (6390, for example). I don't know why I overlooked it then.
> 
> Note that I wouldn't consider adding support for USXGMII a fix, because
> although the switch can do it, it was never done with this driver.
> 
> But if you think it doesn't apply anyway, remove the Fixes tag. This is
> just my opinion that it should stay.

I see, I can only go by our general guidance of not treating omissions 
as fixes, but I lack the knowledge to be certain what's right here.
Anyone willing to cast a tie-break vote? Andrew? net or net-next?

Re: [PATCH v3] net: dsa: mv88e6xxx: support RGMII cmode

[Andrew Lunn]

On Thu, Aug 25, 2022 at 04:42:06PM -0700, Jakub Kicinski wrote:
> On Fri, 26 Aug 2022 01:26:59 +0200 Marek Behún wrote:
> > > Could you explain why? Is there an upstream-supported platform
> > > already in Linus's tree which doesn't boot or something?  
> > 
> > If you mean whether there is a device-tree of such a device, they I
> > don't think so, because AFAIK there isn't a device-tree with 6393 in
> > upstream Linux other than CN9130-CRB.
> > 
> > But it is possible though that there is such a device which has
> > everything but the switch supported on older kernels, due to this RGMII
> > bug.
> > 
> > I think RGMII should have been supported on this switch when I send the
> > patch adding support for it, and it is a bug that it is not, becuase
> > RGMII is supported for similar switches driven by mv88e6xxx driver
> > (6390, for example). I don't know why I overlooked it then.
> > 
> > Note that I wouldn't consider adding support for USXGMII a fix, because
> > although the switch can do it, it was never done with this driver.
> > 
> > But if you think it doesn't apply anyway, remove the Fixes tag. This is
> > just my opinion that it should stay.
> 
> I see, I can only go by our general guidance of not treating omissions 
> as fixes, but I lack the knowledge to be certain what's right here.
> Anyone willing to cast a tie-break vote? Andrew? net or net-next?

Stable rules say: 

o It must fix a real bug that bothers people (not a, “This could be a problem…” type thing).

We know anything with a Fixes: tag pretty much gets considered as a
candidate for stable by the machine learning bot, even if we don't
mark it so. So i would say drop the Fixes: tag, it does not fulfil the
stable requirements.

    Andrew

Re: [PATCH v3] net: dsa: mv88e6xxx: support RGMII cmode

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (master)
by Jakub Kicinski <kuba@kernel.org>:

On Mon, 22 Aug 2022 16:41:36 +0200 you wrote:
> Since the probe defaults all interfaces to the highest speed possible
> (10GBASE-X in mv88e6393x) before the phy mode configuration from the
> devicetree is considered it is currently impossible to use port 0 in
> RGMII mode.
> 
> This change will allow RGMII modes to be configurable for port 0
> enabling port 0 to be configured as RGMII as well as serial depending
> on configuration.
> 
> [...]

Here is the summary with links:
  - [v3] net: dsa: mv88e6xxx: support RGMII cmode
    https://git.kernel.org/netdev/net-next/c/1d2577ab0f05

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html