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

["Marek Behún" <kabel@kernel.org>]

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