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

Jakub Kicinski
2024-06-13T01:33:10Z

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