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] [day] [month] [year] [list]

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

On Mon, Mar 17, 2025 at 03:36:11PM +0000, Simon Horman wrote:
> 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.

Ack. This is addresses in 3. patch

> > +	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.

Thank you, i'll take a look on it.

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |