Received-SPF: pass (google.com: domain of linux-kernel+bounces-212434-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Wed, 12 Jun 2024 18:33:09 -0700
From: Jakub Kicinski <kuba@kernel.org>
To: Rengarajan S <rengarajan.s@microchip.com>
Cc: <woojung.huh@microchip.com>, <UNGLinuxDriver@microchip.com>,
 <davem@davemloft.net>, <edumazet@google.com>, <pabeni@redhat.com>,
 <netdev@vger.kernel.org>, <linux-usb@vger.kernel.org>,
 <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH net-next v1] lan78xx: lan7801 MAC support with lan8841
Message-ID: <20240612183309.01782254@kernel.org>
In-Reply-To: <20240611094233.865234-1-rengarajan.s@microchip.com>
References: <20240611094233.865234-1-rengarajan.s@microchip.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

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