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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250317153611.GB688833@kernel.org>
Date: Mon, 17 Mar 2025 15:36:11 +0000
From: Simon Horman <horms@...nel.org>
To: Oleksij Rempel <o.rempel@...gutronix.de>
Cc: "David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Woojung Huh <woojung.huh@...rochip.com>,
	Andrew Lunn <andrew+netdev@...n.ch>,
	Russell King <rmk+kernel@...linux.org.uk>,
	Thangaraj Samynathan <Thangaraj.S@...rochip.com>,
	Rengarajan Sundararajan <Rengarajan.S@...rochip.com>,
	kernel@...gutronix.de, linux-kernel@...r.kernel.org,
	netdev@...r.kernel.org, UNGLinuxDriver@...rochip.com,
	Phil Elwell <phil@...pberrypi.org>
Subject: Re: [PATCH net-next v3 1/7] net: usb: lan78xx: Convert to PHYlink
 for improved PHY and MAC management

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.

>  		}
>  	}
>  

...