[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c1b45ed298748aafbe3557d637707820bc37e2d2.camel@microchip.com>
Date: Tue, 6 May 2025 05:31:30 +0000
From: <Thangaraj.S@...rochip.com>
To: <andrew+netdev@...n.ch>, <rmk+kernel@...linux.org.uk>,
<davem@...emloft.net>, <Rengarajan.S@...rochip.com>,
<Woojung.Huh@...rochip.com>, <pabeni@...hat.com>, <o.rempel@...gutronix.de>,
<edumazet@...gle.com>, <kuba@...nel.org>
CC: <phil@...pberrypi.org>, <kernel@...gutronix.de>, <horms@...nel.org>,
<linux-kernel@...r.kernel.org>, <netdev@...r.kernel.org>,
<UNGLinuxDriver@...rochip.com>, <maxime.chevallier@...tlin.com>
Subject: Re: [PATCH net-next v8 3/7] net: usb: lan78xx: refactor PHY init to
separate detection and MAC configuration
Hi Oleksj.
Thanks for the patch.
On Mon, 2025-05-05 at 10:43 +0200, Oleksij Rempel wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
>
> Split out PHY detection into lan78xx_get_phy() and MAC-side setup
> into
> lan78xx_mac_prepare_for_phy(), making the main lan78xx_phy_init()
> cleaner
> and easier to follow.
>
> This improves separation of concerns and prepares the code for a
> future
> transition to phylink. Fixed PHY registration and interface selection
> are now handled in lan78xx_get_phy(), while MAC-side delay
> configuration
> is done in lan78xx_mac_prepare_for_phy().
>
> The fixed PHY fallback is preserved for setups like EVB-KSZ9897-1,
> where LAN7801 connects directly to a KSZ switch without a standard
> PHY
> or device tree support.
>
> No functional changes intended.
>
> Signed-off-by: Oleksij Rempel <o.rempel@...gutronix.de>
> ---
> changes v6:
> - this patch is added in v6
> ---
> drivers/net/usb/lan78xx.c | 174 ++++++++++++++++++++++++++++------
> ----
> 1 file changed, 128 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
> index 9c0658227bde..7f1ecc415d53 100644
> --- a/drivers/net/usb/lan78xx.c
> +++ b/drivers/net/usb/lan78xx.c
> @@ -2508,53 +2508,145 @@ static void lan78xx_remove_irq_domain(struct
> lan78xx_net *dev)
> dev->domain_data.irqdomain = NULL;
> }
>
>
> static int lan78xx_phy_init(struct lan78xx_net *dev)
> @@ -2564,31 +2656,13 @@ static int lan78xx_phy_init(struct
> lan78xx_net *dev)
> u32 mii_adv;
> struct phy_device *phydev;
>
> - switch (dev->chipid) {
> - case ID_REV_CHIP_ID_7801_:
> - phydev = lan7801_phy_init(dev);
> - if (IS_ERR(phydev)) {
> - netdev_err(dev->net, "lan7801: failed to init
> PHY: %pe\n",
> - phydev);
> - return PTR_ERR(phydev);
> - }
> - break;
> -
> - case ID_REV_CHIP_ID_7800_:
> - case ID_REV_CHIP_ID_7850_:
> - phydev = phy_find_first(dev->mdiobus);
> - if (!phydev) {
> - netdev_err(dev->net, "no PHY found\n");
> - return -ENODEV;
> - }
> - phydev->is_internal = true;
> - dev->interface = PHY_INTERFACE_MODE_GMII;
> - break;
> + phydev = lan78xx_get_phy(dev);
> + if (IS_ERR(phydev))
> + return PTR_ERR(phydev);
>
> - default:
> - netdev_err(dev->net, "Unknown CHIP ID found\n");
> - return -ENODEV;
> - }
> + ret = lan78xx_mac_prepare_for_phy(dev);
> + if (ret < 0)
> + goto free_phy;
>
> /* if phyirq is not set, use polling mode in phylib */
> if (dev->domain_data.phyirq > 0)
> @@ -2662,6 +2736,14 @@ static int lan78xx_phy_init(struct lan78xx_net
> *dev)
> dev->fc_autoneg = phydev->autoneg;
>
> return 0;
> +
> +free_phy:
> + if (phy_is_pseudo_fixed_link(phydev)) {
> + fixed_phy_unregister(phydev);
> + phy_device_free(phydev);
> + }
> +
> + return ret;
> }
Could see as per implementation, this case might hit on normal phy
other than fixed-phy too. Should we not add any cleanup for phydev
here?
Thanks,
Thangaraj Samynathan
>
> static int lan78xx_set_rx_max_frame_length(struct lan78xx_net *dev,
> int size)
> --
> 2.39.5
>
Powered by blists - more mailing lists