Re: [PATCH net-next v3 1/7] net: usb: lan78xx: Convert to PHYlink for improved PHY and MAC management

[Simon Horman <horms@kernel.org>]

On Mon, Mar 10, 2025 at 12:57:31PM +0100, Oleksij Rempel wrote:

...

> +static struct phy_device *lan7801_phy_init(struct lan78xx_net *dev)
> +{
>  	struct phy_device *phydev;
> +	int ret;

nit: not strictly related to this patch as the problem already existed,
     but ret is set but otherwise unused in this function. Perhaps
     it can be removed at some point?

     Flagged by W=1 builds.

> +	u32 buf;
>  
>  	phydev = phy_find_first(dev->mdiobus);
>  	if (!phydev) {
> -		netdev_dbg(dev->net, "PHY Not Found!! Registering Fixed PHY\n");
> -		phydev = fixed_phy_register(PHY_POLL, &fphy_status, NULL);
> -		if (IS_ERR(phydev)) {
> -			netdev_err(dev->net, "No PHY/fixed_PHY found\n");
> -			return NULL;
> -		}
> -		netdev_dbg(dev->net, "Registered FIXED PHY\n");
> -		dev->interface = PHY_INTERFACE_MODE_RGMII;
> +		netdev_dbg(dev->net, "PHY Not Found!! Forcing RGMII configuration\n");
>  		ret = lan78xx_write_reg(dev, MAC_RGMII_ID,
>  					MAC_RGMII_ID_TXC_DELAY_EN_);
>  		ret = lan78xx_write_reg(dev, RGMII_TX_BYP_DLL, 0x3D00);

...

> @@ -4256,13 +4281,13 @@ static void lan78xx_delayedwork(struct work_struct *work)
>  		}
>  	}
>  
> -	if (test_bit(EVENT_LINK_RESET, &dev->flags)) {
> +	if (test_bit(EVENT_PHY_INT_ACK, &dev->flags)) {
>  		int ret = 0;
>  
> -		clear_bit(EVENT_LINK_RESET, &dev->flags);
> -		if (lan78xx_link_reset(dev) < 0) {
> -			netdev_info(dev->net, "link reset failed (%d)\n",
> -				    ret);
> +		clear_bit(EVENT_PHY_INT_ACK, &dev->flags);
> +		if (lan78xx_phy_int_ack(dev) < 0) {
> +			netdev_info(dev->net, "PHY INT ack failed (%pe)\n",
> +				    ERR_PTR(ret));

nit: ret is always 0 here. So I'm unsure both why it is useful to include
     in the error message, and why ERR_PTR() is being used.

     Flagged by Smatch.

>  		}
>  	}
>  

...