netdev - Re: Aquantia PHY in OCSGMII mode?

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: <aIzI5roBAaRgzXxH@shell.armlinux.org.uk>
Date: Fri, 1 Aug 2025 15:02:14 +0100
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Vladimir Oltean <olteanv@...il.com>
Cc: Alexander Wilhelm <alexander.wilhelm@...termo.com>,
	Andrew Lunn <andrew@...n.ch>,
	Heiner Kallweit <hkallweit1@...il.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: Aquantia PHY in OCSGMII mode?

On Fri, Aug 01, 2025 at 04:04:20PM +0300, Vladimir Oltean wrote:
> On Fri, Aug 01, 2025 at 01:23:44PM +0100, Russell King (Oracle) wrote:
> > It looks like memac_select_pcs() and memac_prepare() fail to
> > handle 2500BASEX despite memac_initialization() suggesting the
> > SGMII PCS supports 2500BASEX.
> 
> Thanks for pointing this out, it seems to be a regression introduced by
> commit 5d93cfcf7360 ("net: dpaa: Convert to phylink").
> 
> If there are no other volunteers, I can offer to submit a patch if
> Alexander confirms this fixes his setup.
> 
> > It would also be good if the driver can also use
> > pcs->supported_interfaces which states which modes the PCS layer
> > supports as well.
> 
> The current algorithm in lynx_pcs_create() is too optimistic and
> advertises host interfaces which the PCS may not actually support.
> 
> static const phy_interface_t lynx_interfaces[] = {
> 	PHY_INTERFACE_MODE_SGMII,
> 	PHY_INTERFACE_MODE_QSGMII,
> 	PHY_INTERFACE_MODE_1000BASEX,
> 	PHY_INTERFACE_MODE_2500BASEX,
> 	PHY_INTERFACE_MODE_10GBASER,
> 	PHY_INTERFACE_MODE_USXGMII,
> };
> 
> 	for (i = 0; i < ARRAY_SIZE(lynx_interfaces); i++)
> 		__set_bit(lynx_interfaces[i], lynx->pcs.supported_interfaces);
> 
> I am concerned that if we add logic to the MAC driver which does:
> 
> 		phy_interface_or(config->supported_interfaces,
> 				 config->supported_interfaces,
> 				 pcs->supported_interfaces);
> 
> then we depart from the physical reality of the board and may end up
> accepting a host interface which we should have rejected.
> 
> There is downstream code which refines lynx_pcs_create() to this:
> 
> 	/* In case we have access to the SerDes phy/lane, then ask the SerDes
> 	 * driver what interfaces are supported based on the current PLL
> 	 * configuration.
> 	 */
> 	for (int i = 0; i < ARRAY_SIZE(lynx_interfaces); i++) {
> 		phy_interface_t iface = lynx_interfaces[i];
> 
> 		err = phy_validate(lynx->serdes[PRIMARY_LANE],
> 				   PHY_MODE_ETHERNET, iface, NULL);
> 		if (err)
> 			continue;
> 
> 		__set_bit(iface, supported_interfaces);
> 	}
> 
> but the infrastructure (the SerDes driver) is currently lacking upstream.

It looks like the SerDes driver is managed by the MAC (it validates
each mode against the serdes PHY driver's validate function - serdes
being mac_dev->fman_mac->serdes. If this SerDes doesn't exist, then
only mac_dev->phy_if is supported.

So, I don't think there's any need for the Lynx to reach out to the
SerDes in mainline as it currently stands.

As the SerDes also dictates which modes and is managed by fman, I'd
suggest for mainline that the code needs to implement the following
pseudocode:

	config->supported_interfaces = mac_support |
				(pcs->supported_interfaces &
				serdes_supported_interfaces);

rather than the simple "or pcs->supported_interfaces into the
supported bitmap" that we can do in other drivers.

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