Re: [PATCH net-next] net: phy: broadcom: Save PHY counters during suspend

[Jakub Kicinski <kuba@kernel.org>]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: phy: broadcom: Save PHY counters during suspend

The PHY counters can be lost if the PHY is reset during suspend. We
need to save the values into the shadow counters or the accounting
will be incorrect over multiple suspend and resume cycles.

> diff --git a/drivers/net/phy/bcm7xxx.c b/drivers/net/phy/bcm7xxx.c
> --- a/drivers/net/phy/bcm7xxx.c
> +++ b/drivers/net/phy/bcm7xxx.c
> @@ -733,6 +733,7 @@ static int bcm7xxx_config_init(struct phy_device *phydev)
>   */
>  static int bcm7xxx_suspend(struct phy_device *phydev)
>  {
> +	struct bcm7xxx_phy_priv *priv = phydev->priv;
>  	int ret;

[ ... ]

>  	unsigned int i;
>  
> +	mutex_lock(&phydev->lock);
> +	bcm_phy_update_stats_shadow(phydev, priv->stats);

Does this cause a NULL pointer dereference for 40nm PHYs?

The bcm7xxx_suspend() function is used as the suspend callback for PHY
models defined by the BCM7XXX_40NM_EPHY() macro (like BCM7346, BCM7362,
and BCM7425).

Unlike the 28nm and 16nm PHY macros which assign a probe callback to
allocate phydev->priv, the BCM7XXX_40NM_EPHY() macro does not appear to
define a probe callback. This would leave phydev->priv as NULL and cause a
crash when trying to access priv->stats here.

> +	mutex_unlock(&phydev->lock);
>  
>  	for (i = 0; i < ARRAY_SIZE(bcm7xxx_suspend_cfg); i++) {
>  		ret = phy_write(phydev,
-- 
pw-bot: cr