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

[Jakub Kicinski <kuba@kernel.org>]

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;
> +		}