Re: [PATCH net-next v25 05/10] net: phy: Add Airoha AN8855 Internal Switch Gigabit PHY

[Christian Marangi <ansuelsmth@gmail.com>]

On Sat, Oct 10, 2026 at 04:11:39PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] net: phy: air_an8855: phy_write_mmd clobbers adjacent reserved fields
> - [Medium] net: phy: air_an8855: Array linear search underflow causes valid high efuse values to fall back to default
> --
> 
> commit e8cc9bb475e2accd40605a3774a8e659a3156338
> Author: Christian Marangi <ansuelsmth@gmail.com>
> 
> net: phy: Add Airoha AN8855 Internal Switch Gigabit PHY
> 
> Add support for Airoha AN8855 Internal Switch Gigabit PHY. This is a
> simple PHY driver to configure and calibrate the PHY for the AN8855
> Switch with the use of NVMEM cells.
> 
> > diff --git a/drivers/net/phy/air_an8855.c b/drivers/net/phy/air_an8855.c
> > new file mode 100644
> > index 0000000000000..bc22256fd5f95
> > --- /dev/null
> > +++ b/drivers/net/phy/air_an8855.c
> > @@ -0,0 +1,275 @@
> 
> [ ... ]
> 
> > +static const u8 dsa_r50ohm_table[] = {
> > +	127, 127, 127, 127, 127, 127, 127, 127, 127, 127,
> > +	127, 127, 127, 127, 127, 127, 127, 126, 122, 117,
> > +	112, 109, 104, 101,  97,  94,  90,  88,  84,  80,
> > +	78,  74,  72,  68,  66,  64,  61,  58,  56,  53,
> > +	51,  48,  47,  44,  42,  40,  38,  36,  34,  32,
> > +	31,  28,  27,  24,  24,  22,  20,  18,  16,  16,
> > +	14,  12,  11,   9
> > +};
> > +
> > +static int an8855_get_r50ohm_val(struct device *dev, const char *calib_name,
> > +				 u8 *dest)
> > +{
> > +	u32 shift_sel, val;
> > +	int ret;
> > +	int i;
> > +
> > +	ret = nvmem_cell_read_u32(dev, calib_name, &val);
> > +	if (ret)
> > +		return ret;
> > +
> > +	shift_sel = FIELD_GET(AN8855_SWITCH_EFUSE_R50O, val);
> > +	for (i = 0; i < ARRAY_SIZE(dsa_r50ohm_table); i++)
> > +		if (dsa_r50ohm_table[i] == shift_sel)
> > +			break;
> > +
> > +	if (i < 8 || i >= ARRAY_SIZE(dsa_r50ohm_table))
> > +		*dest = dsa_r50ohm_table[25];
> > +	else
> > +		*dest = dsa_r50ohm_table[i - 8];
> 
> [Severity: Medium]
> Can a valid high efuse value cause an array search underflow here?
> 
> Since dsa_r50ohm_table contains 17 occurrences of 127 at the beginning of the
> array, a forward linear search for 127 breaks at i = 0.
> 
> The subsequent logic if (i < 8) intercepts i = 0 as an error. Consequently, an
> input of 127 falls back to index 25 (94), whereas an input of 126 correctly
> matches at index 17 and maps to table[17-8] (127). The forward search breaks
> the - 8 shift algorithm for saturated values.
>

This is exaclty the logic used by the original driver and how EFUSE are
fused on the PHY.
 
> > +
> > +	return 0;
> > +}
> 
> [ ... ]
> 
> > +static int an8855_config_init(struct phy_device *phydev)
> > +{
> 
> [ ... ]
> 
> > +	/* Apply values to reduce signal noise */
> > +	ret = phy_modify_mmd(phydev, MDIO_MMD_VEND1, AN8855_PHY_TX_PAIR_DLY_SEL_GBE,
> > +			     AN8855_PHY_CR_DA_TX_PAIR_DELAY_SEL_A_GBE |
> > +			     AN8855_PHY_CR_DA_TX_PAIR_DELAY_SEL_C_GBE,
> > +			     FIELD_PREP(AN8855_PHY_CR_DA_TX_PAIR_DELAY_SEL_A_GBE, 0x4) |
> > +			     FIELD_PREP(AN8855_PHY_CR_DA_TX_PAIR_DELAY_SEL_C_GBE, 0x4));
> > +	if (ret)
> > +		return ret;
> > +	ret = phy_set_bits_mmd(phydev, MDIO_MMD_VEND1, AN8855_PHY_RXADC_CTRL,
> > +			       AN8855_PHY_RG_AD_SAMPLE_PHSEL_A |
> > +			       AN8855_PHY_RG_AD_SAMPLE_PHSEL_C);
> > +	if (ret)
> > +		return ret;
> > +	ret = phy_write_mmd(phydev, MDIO_MMD_VEND1, AN8855_PHY_RXADC_REV_0,
> > +			    FIELD_PREP(AN8855_PHY_RG_AD_RESERVE0_A, 0x1));
> 
> [Severity: Medium]
> Does this inadvertently clear the adjacent reserved fields for pair B?
> 
> phy_write_mmd() unconditionally overwrites the entire 16-bit register. By
> writing FIELD_PREP(AN8855_PHY_RG_AD_RESERVE0_A, 0x1), this inadvertently zeroes
> out the adjacent 8-bit field AN8855_PHY_RG_AD_RESERVE0_B (bits 7:0). Should
> phy_modify_mmd() be used here instead?
> 

No this is intended.

> > +	if (ret)
> > +		return ret;
> > +	ret = phy_write_mmd(phydev, MDIO_MMD_VEND1, AN8855_PHY_RXADC_REV_1,
> > +			    FIELD_PREP(AN8855_PHY_RG_AD_RESERVE0_C, 0x1));
> 
> [Severity: Medium]
> Similarly, does this inadvertently clear the adjacent reserved fields for
> pair D (AN8855_PHY_RG_AD_RESERVE0_D)?
> 

Intended.

> > +	if (ret)
> > +		return ret;
> > +
> > +	return 0;
> > +}
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261009161102.38961-1-ansuelsmth@gmail.com?part=5

-- 
	Ansuel