netdev - Re: [PATCH net-next 2/2] net: phy: marvell: implement cable-test for 88E308X/88E609X family

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: <88dc994b-a3da-4760-b7e0-c6da593b15d6@lunn.ch>
Date: Tue, 26 Mar 2024 15:32:50 +0100
From: Andrew Lunn <andrew@...n.ch>
To: Pawel Dembicki <paweldembicki@...il.com>
Cc: netdev@...r.kernel.org, Heiner Kallweit <hkallweit1@...il.com>,
	Russell King <linux@...linux.org.uk>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Stefan Eichenberger <eichest@...il.com>,
	Dimitri Fedrau <dima.fedrau@...il.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next 2/2] net: phy: marvell: implement cable-test for
 88E308X/88E609X family

> +		ret = phy_write(phydev, MII_BMCR, 0xa100);

BMCR_RESET | BMCR_ANENABLE | BMCR_FULLDPLX

You should check i have that correct, but no need for magic values
with this register.

> +static int m88e3082_vct_cable_test_report_trans(int result, u8 distance)
> +{
> +	switch (result) {
> +	case MII_VCT_TXRXPINS_VCTTST_OK:
> +		if (distance == MII_VCT_TXRXPINS_DISTRFLN_MAX)
> +			return ETHTOOL_A_CABLE_RESULT_CODE_OK;
> +		/* Impedance mismatch */
> +		return ETHTOOL_A_CABLE_RESULT_CODE_UNSPEC;

It is possible to add more results code, if you think Impedance
mismatch is useful.

> +	ret = phy_write(phydev, MII_BMCR, priv->m88e3082_vct_reg_backup);
> +	if (ret < 0)
> +		return ret;

I'm not sure this is required. When the cable test is finished, the
PHY state machine is moved to PHY_UP. That will cause
phy_config_aneg() to be called which should set BMCR back to the
correct value.

	Andrew