Re: [PATCH net-next v9 10/11] net: phy: ncn26000: Support for onsemi's S2500 internal phy

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] phy: ncn26000: incorrect mask used in interrupt status check
--

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

net: phy: ncn26000: Support for onsemi's S2500 internal phy

This commit adds support for the internal PHY of the integrated media access
controller S2500. It configures the PLCA tx opportunity timer's default value
for this device, compared to the currently supported NCN26000 PHY.

> diff --git a/drivers/net/phy/ncn26000.c b/drivers/net/phy/ncn26000.c
> index 6d433f4e2c45d..a33ebb2a7ecef 100644
> --- a/drivers/net/phy/ncn26000.c
> +++ b/drivers/net/phy/ncn26000.c
[ ... ]
> @@ -163,6 +169,26 @@ static int ncn26000_config_intr(struct phy_device *phydev)
>  }
>  
>  static struct phy_driver ncn26000_driver[] = {
> +	{
> +		PHY_ID_MATCH_MODEL(PHY_ID_S2500),
> +		.name                  = "S2500",
> +		.features              = PHY_BASIC_T1S_P2MP_FEATURES,
> +		.config_init           = ncn26000_config_init,
> +		.config_intr           = ncn26000_config_intr,
> +		.config_aneg           = ncn26000_config_aneg,
> +		.read_status           = ncn26000_read_status,
> +		.handle_interrupt      = ncn26000_handle_interrupt,

[Severity: Medium]
Is ncn26000_handle_interrupt() working correctly here?

Looking at drivers/net/phy/ncn26000.c:ncn26000_handle_interrupt(), it uses the
register address macro (NCN26000_REG_IRQ_STATUS, which is 17 or 0x11) as a
bitmask instead of the correct interrupt bit (NCN26000_IRQ_LINKST_BIT, BIT(0)):

    if (ret < 0 || (ret & NCN26000_REG_IRQ_STATUS) == 0)
        return IRQ_NONE;

This inadvertently checks both bit 0 and bit 4 (NCN26000_IRQ_PLCAREC_BIT).

Consequently, if a PLCA receive interrupt occurs, wouldn't the driver
incorrectly trigger the PHY state machine?

While this logic existed previously in the driver, adding this callback
assignment newly exposes the S2500 device to the issue.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-s2500-mac-phy-support-v9-0-dcefe1d0bf0d@onsemi.com?part=10