netdev - Re: [RFC PATCH 2/2] net: phy: mxl-gpy: add new device tree property to disable SGMII autoneg

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Zikd+GxuwMRC+5Ae@shell.armlinux.org.uk>
Date: Wed, 24 Apr 2024 15:58:00 +0100
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Stefan Eichenberger <eichest@...il.com>
Cc: Andrew Lunn <andrew@...n.ch>, davem@...emloft.net, edumazet@...gle.com,
	kuba@...nel.org, pabeni@...hat.com, robh@...nel.org,
	krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org,
	lxu@...linear.com, hkallweit1@...il.com, michael@...le.cc,
	netdev@...r.kernel.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 2/2] net: phy: mxl-gpy: add new device tree property
 to disable SGMII autoneg

On Thu, Apr 18, 2024 at 04:01:59PM +0100, Russell King (Oracle) wrote:
> On Wed, Apr 17, 2024 at 09:22:50AM +0200, Stefan Eichenberger wrote:
> > I also checked the datasheet and you are right about the 1000base-X mode
> > and in-band AN. What worked for us so far was to use SGMII mode even for
> > 2.5Gbps and disable in-band AN (which is possible for SGMII). I think
> > this works because as you wrote, the genphy just multiplies the clock by
> > 2.5 and doesn't care if it's 1000base-X or SGMII. With your patches we
> > might even be able to use in-band autonegoation for 10,100 and 1000Mbps
> > and then just disable it for 2.5Gbps. I need to test it, but I have hope
> > that this should work.
> 
> There is another way we could address this. If the querying support
> had a means to identify that the endpoint supports bypass mode, we
> could then have phylink identify that, and arrange to program the
> mvpp2 end to be in 1000base-X + x2.5 clock + AN bypass, which would
> mean it wouldn't require the inband 16-bit word to be present.
> 
> I haven't fully thought it through yet - for example, I haven't
> considered how we should indicate to the PCS that AN bypass mode
> should be enabled or disabled via the pcs_config() method.

Okay, I've been trying to put more effort into this, but it's been slow
progress (sorry).

My thoughts from a design point of view were that we could just switch
to PHYLINK_PCS_NEG_OUTBAND instead of PHYLINK_PCS_NEG_INBAND_* and
everything at the PCS layer should be able to cope, but this is not the
case, especially with mvneta/mvpp2.

The problem is that mvneta/mvpp2 (and probably more) expect that

1) MLO_AN_INBAND means that the PCS will be using inband, and that
   means the link up/down state won't be forced. This basically implies
   that only PHYLINK_PCS_NEG_INBAND_* can be used can be used for the
   PCS.

2) !MLO_AN_INBAND means that an out-of-band mechanism will be used and
   that means that the link needs to be forced (since there's no way
   for the hardware to know whether the link should be up or down.)
   It's therefore expected that only PHYLINK_PCS_NEG_OUTBAND will be
   used for the PCS.

So, attempting to put a resolution of the PHY and PCS abilities into
phylink_pcs_neg_mode() and select the appropriate PHYLINK_PCS_NEG_*
mode alone just doesn't work. Yet... we need to do that in there when
considering whether inband can be enabled or not for non-PHY links.

Basically, it needs a re-think how to solve this...

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