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: <ZxD69GqiPcqOZK2w@makrotopia.org>
Date: Thu, 17 Oct 2024 12:54:28 +0100
From: Daniel Golle <daniel@...rotopia.org>
To: Paul Davey <paul.davey@...iedtelesis.co.nz>
Cc: Andrew Lunn <andrew@...n.ch>, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next] net: phy: aquantia: Add mdix config and
 reporting

On Thu, Oct 17, 2024 at 02:54:07PM +1300, Paul Davey wrote:
> Add support for configuring MDI-X state of PHY.
> Add reporting of resolved MDI-X state in status information.

> [...]
> +static int aqr_set_polarity(struct phy_device *phydev, int polarity)

"polarity" is not the right term here. This is not about the polarity
of copper pairs, but rather about pairs being swapped.
Please name the function accordingly, eg. aqr_set_mdix().

> +{
> +	u16 val = 0;
> +
> +	switch (polarity) {
> +	case ETH_TP_MDI:
> +		val = MDIO_AN_RESVD_VEND_PROV_MDIX_MDI;
> +		break;
> +	case ETH_TP_MDI_X:
> +		val = MDIO_AN_RESVD_VEND_PROV_MDIX_MDIX;
> +		break;
> +	case ETH_TP_MDI_AUTO:
> +	case ETH_TP_MDI_INVALID:
> +	default:
> +		val = MDIO_AN_RESVD_VEND_PROV_MDIX_AUTO;
> +		break;
> +	}
> +
> +	return phy_modify_mmd_changed(phydev, MDIO_MMD_AN, MDIO_AN_RESVD_VEND_PROV,
> +				      MDIO_AN_RESVD_VEND_PROV_MDIX_MASK, val);
> +}
> +
>  static int aqr_config_aneg(struct phy_device *phydev)
>  {
>  	bool changed = false;
>  	u16 reg;
>  	int ret;
>  
> +	ret = aqr_set_polarity(phydev, phydev->mdix_ctrl);
> +	if (ret < 0)
> +		return ret;
> +	if (ret > 0)
> +		changed = true;
> +
>  	if (phydev->autoneg == AUTONEG_DISABLE)
>  		return genphy_c45_pma_setup_forced(phydev);
>  
> @@ -278,6 +315,14 @@ static int aqr_read_status(struct phy_device *phydev)
>  				 val & MDIO_AN_RX_LP_STAT1_1000BASET_HALF);
>  	}
>  
> +	val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_RESVD_VEND_STATUS1);

According to the datasheet the MDI/MDI-X indication should only be
interpreted when autonegotiation has completed.
Hence this call should be protected by genphy_c45_aneg_done(phydev) and
phydev->mdix set to ETH_TP_MDI_INVALID in case auto-negotiation hasn't
completed.

> +	if (val < 0)
> +		return val;
> +	if (val & MDIO_AN_RESVD_VEND_STATUS1_MDIX)
> +		phydev->mdix = ETH_TP_MDI_X;
> +	else
> +		phydev->mdix = ETH_TP_MDI;
> +
>  	return genphy_c45_read_status(phydev);
>  }
>  
> -- 
> 2.47.0
> 
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ