linux-kernel - Re: [PATCH net-next v7 2/7] net: pcs: xpcs: re-initiate clause 37 Auto-negotiation

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: <Z6TVmdCZeWerAZKP@shell.armlinux.org.uk>
Date: Thu, 6 Feb 2025 15:30:33 +0000
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Choong Yong Liang <yong.liang.choong@...ux.intel.com>
Cc: Simon Horman <horms@...nel.org>, Jose Abreu <joabreu@...opsys.com>,
	Jose Abreu <Jose.Abreu@...opsys.com>,
	David E Box <david.e.box@...ux.intel.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
	Dave Hansen <dave.hansen@...ux.intel.com>,
	"H . Peter Anvin" <hpa@...or.com>,
	Rajneesh Bhardwaj <irenic.rajneesh@...il.com>,
	David E Box <david.e.box@...el.com>,
	Andrew Lunn <andrew+netdev@...n.ch>,
	"David S . Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Maxime Coquelin <mcoquelin.stm32@...il.com>,
	Alexandre Torgue <alexandre.torgue@...s.st.com>,
	Jiawen Wu <jiawenwu@...stnetic.com>,
	Mengyuan Lou <mengyuanlou@...-swift.com>,
	Heiner Kallweit <hkallweit1@...il.com>,
	Hans de Goede <hdegoede@...hat.com>,
	Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
	Richard Cochran <richardcochran@...il.com>,
	Serge Semin <fancer.lancer@...il.com>, x86@...nel.org,
	linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
	platform-driver-x86@...r.kernel.org,
	linux-stm32@...md-mailman.stormreply.com,
	linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH net-next v7 2/7] net: pcs: xpcs: re-initiate clause 37
 Auto-negotiation

On Thu, Feb 06, 2025 at 09:18:54PM +0800, Choong Yong Liang wrote:
> The xpcs_switch_interface_mode function was introduced to handle
> interface switching.
> 
> According to the XPCS datasheet, a soft reset is required to initiate
> Clause 37 auto-negotiation when the XPCS switches interface modes.

Hmm. Given that description, taking it literally, claus 37
auto-negotiation is 1000BASE-X, not Cisco SGMII (which isn't an IEEE
802.3 standard.) Are you absolutely sure that this applies to Cisco
SGMII?

If the reset is required when switching to SGMII, should it be done
before or after configuring the XPCS for SGMII?

Also, if the reset is required, what happens if we're already using
SGMII, but AN has been disabled temporarily and is then re-enabled?
Is a reset required?

What about 1000BASE-X when AN is enabled or disabled and then switching
to SGMII?

> +static int xpcs_switch_to_aneg_c37_sgmii(const struct dw_xpcs_compat *compat,
> +					 struct dw_xpcs *xpcs,
> +					 unsigned int neg_mode)
> +{
> +	bool an_c37_enabled;
> +	int ret, mdio_ctrl;
> +
> +	if (neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED) {
> +		mdio_ctrl = xpcs_read(xpcs, MDIO_MMD_VEND2, MII_BMCR);
> +		if (mdio_ctrl < 0)
> +			return mdio_ctrl;
> +
> +		an_c37_enabled = mdio_ctrl & BMCR_ANENABLE;
> +		if (!an_c37_enabled) {

I don't think that we need "an_c37_enabled" here, I think simply:

		if (!(mdio_ctrl & BMCR_ANENABLE)) {

would be sufficient.

> +			//Perform soft reset to initiate C37 auto-negotiation
> +			ret = xpcs_soft_reset(xpcs, compat);
> +			if (ret)
> +				return ret;
> +		}
> +	}
> +	return 0;

I'm also wondering (as above) whether this soft reset needs to happen
_after_ xpcs_config_aneg_c37_sgmii() has done its work - this function
temporarily disables AN while it's doing its work.

I'm also wondering whether AN being disabled is really a deciding
factor (e.g. when switching from 1000BASE-X AN-enabled to SGMII, is a
reset required?)

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