Re: [PATCH net-next v10 11/11] net: phy: ncn26000: Support for loopback

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] net: phy: ncn26000: ncn26000_config_aneg() clears
  BMCR_LOOPBACK in MII_BMCR
--

commit a6fa19dd96a071675ca905f2adbb2958359bdcce
Author: Selvamani Rajagopal <Selvamani.Rajagopal@onsemi.com>

net: phy: ncn26000: Support for loopback

This commit introduces a generic genphy_loopback_no_speed() helper that
toggles BMCR_LOOPBACK in MII_BMCR without setting speed, and wires it as
the .set_loopback callback for the S2500 and TI DP83867 PHY drivers.

> diff --git a/drivers/net/phy/ncn26000.c b/drivers/net/phy/ncn26000.c
> index 0006e157917df..c115af6eda33d 100644
> --- a/drivers/net/phy/ncn26000.c
> +++ b/drivers/net/phy/ncn26000.c
> @@ -176,6 +176,7 @@ static struct phy_driver ncn26000_driver[] = {
>  		.config_init           = ncn26000_config_init,
>  		.config_aneg           = ncn26000_config_aneg,
>  		.read_status           = ncn26000_read_status,
> +		.set_loopback          = genphy_loopback_no_speed,
>  		.set_plca_cfg          = ncn26000_c45_plca_set_cfg,
>  		.get_plca_cfg          = genphy_c45_plca_get_cfg,
>  		.get_plca_status       = genphy_c45_plca_get_status,

[Severity: Medium]
Does ncn26000_config_aneg() clear BMCR_LOOPBACK when phy_config_aneg() runs
while PHY loopback is enabled?

drivers/net/phy/ncn26000.c:ncn26000_config_aneg() {
	...
	// bring up the link
	return phy_write(phydev, MII_BMCR, NCN26000_BCMR_LINK_CTRL_BIT);
}

genphy_loopback_no_speed() sets BMCR_LOOPBACK in MII_BMCR, but any
subsequent phy_config_aneg() call (for example when phy_state_machine()
processes PHY_UP) overwrites the entire MII_BMCR register via phy_write().
This disables hardware loopback while phydev->loopback_enabled remains
true, causing loopback selftest frames to be transmitted onto the line
instead of looping back.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106-s2500-mac-phy-support-v10-0-31b5a69b85b7@onsemi.com?part=11