Hi Jakub,

Apologies for the delay in reply. Thanks for reviewing the patch and
please find my comments inline.

On Wed, 2024-06-12 at 18:33 -0700, Jakub Kicinski wrote:

EXTERNAL EMAIL: Do not click links or open attachments unless you
know the content is safe

On Tue, 11 Jun 2024 15:12:33 +0530 Rengarajan S wrote:

quoted

 /* 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?

Sure. Will address the change in the next patch revision.

quoted

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

There is a seperate register and unregister done for PHY_LAN8835 and
PHY_KSZ9031RNX. Also, if the ret < 0 the fixup is not registered and
there is no need to unregister it further. Can you please elaborate on
what is missing in case of unregistering PHY_KSZ9031RNX.

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

quoted

+                     return NULL;
+             }