netdev - Re: [PATCH net-next v1] lan78xx: lan7801 MAC support with lan8841

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: <20240612183309.01782254@kernel.org>
Date: Wed, 12 Jun 2024 18:33:09 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Rengarajan S <rengarajan.s@...rochip.com>
Cc: <woojung.huh@...rochip.com>, <UNGLinuxDriver@...rochip.com>,
 <davem@...emloft.net>, <edumazet@...gle.com>, <pabeni@...hat.com>,
 <netdev@...r.kernel.org>, <linux-usb@...r.kernel.org>,
 <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next v1] lan78xx: lan7801 MAC support with lan8841

On Tue, 11 Jun 2024 15:12:33 +0530 Rengarajan S wrote:
>  /* define external phy id */
>  #define	PHY_LAN8835			(0x0007C130)
> +#define PHY_LAN8841			(0x00221650)

For whatever reason the existing code uses a tab between define and its
name, so let's stick to that?

>  #define	PHY_KSZ9031RNX			(0x00221620)
>  
>  /* use ethtool to change the level for any given device */
> @@ -2327,6 +2328,13 @@ static struct phy_device *lan7801_phy_init(struct lan78xx_net *dev)
>  			netdev_err(dev->net, "Failed to register fixup for PHY_LAN8835\n");
>  			return NULL;
>  		}
> +		/* external PHY fixup for LAN8841 */
> +		ret = phy_register_fixup_for_uid(PHY_LAN8841, 0xfffffff0,
> +						 lan8835_fixup);
> +		if (ret < 0) {
> +			netdev_err(dev->net, "Failed to register fixup for PHY_LAN8841\n");

Don't you have to unregister the previous fixup on the error path here?
In fact the existing error path for PHY_LAN8835 is missing an unregsiter
for PHY_KSZ9031RNX.

Could you please send a separate fix for that with a Fixes tag?

> +			return NULL;
> +		}