| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <5de15da31f71e1bed8782375b402bcb5df2eb63a.camel@microchip.com>
Date: Tue, 25 Mar 2025 17:37:53 +0000
From: <Rengarajan.S@...rochip.com>
To: <andrew+netdev@...n.ch>, <rmk+kernel@...linux.org.uk>,
<davem@...emloft.net>, <Thangaraj.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 v5 2/6] net: usb: lan78xx: Convert to PHYlink for
improved PHY and MAC management
Hi Oleksji,
On Wed, 2025-03-19 at 09:49 +0100, Oleksij Rempel wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
>
> Convert the LAN78xx driver to use the PHYlink framework for managing
> PHY and MAC interactions.
>
> Key changes include:
> - Replace direct PHY operations with phylink equivalents (e.g.,
> phylink_start, phylink_stop).
> - Introduce lan78xx_phylink_setup for phylink initialization and
> configuration.
> - Add phylink MAC operations (lan78xx_mac_config,
> lan78xx_mac_link_down, lan78xx_mac_link_up) for managing link
> settings and flow control.
> - Remove redundant and now phylink-managed functions like
> `lan78xx_link_status_change`.
> - update lan78xx_get/set_pause to use phylink helpers
>
> Signed-off-by: Oleksij Rempel <o.rempel@...gutronix.de>
> ---
> changes v5:
> - merge ethtool pause interface changes to this patch
> changes v4:
> - add PHYLINK dependency
> - remove PHYLIB and FIXED_PHY, both are replaced by PHYLINK
> changes v3:
> - lan78xx_phy_init: drop phy_suspend()
> - lan78xx_phylink_setup: use phy_interface_set_rgmii()
> changes v2:
> - lan78xx_mac_config: remove unused rgmii_id
> - lan78xx_mac_config: PHY_INTERFACE_MODE_RGMII* variants
> - lan78xx_mac_config: remove auto-speed and duplex configuration
> - lan78xx_phylink_setup: set link_interface to
> PHY_INTERFACE_MODE_RGMII_ID
> instead of PHY_INTERFACE_MODE_NA.
> - lan78xx_phy_init: use phylink_set_fixed_link() instead of
> allocating
> fixed PHY.
> - lan78xx_configure_usb: move function values to separate variables
>
> 20220427_lukas_polling_be_gone_on_lan95xx.cover
> ---
> drivers/net/usb/Kconfig | 3 +-
> drivers/net/usb/lan78xx.c | 615 ++++++++++++++++++----------------
> ----
> 2 files changed, 298 insertions(+), 320 deletions(-)
>
> diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig
> index 3c360d4f0635..71168e47a9b1 100644
> --- a/drivers/net/usb/Kconfig
> +++ b/drivers/net/usb/Kconfig
> @@ -115,9 +115,8 @@ config USB_RTL8152
> config USB_LAN78XX
> tristate "Microchip LAN78XX Based USB Ethernet Adapters"
> select MII
> - select PHYLIB
> + select PHYLINK
> select MICROCHIP_PHY
> - select FIXED_PHY
> select CRC32
> help
> This option adds support for Microchip LAN78XX based USB 2
> diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
> index 13b5da18850a..fd6e80f9c35f 100644
> --- a/drivers/net/usb/lan78xx.c
> +++ b/drivers/net/usb/lan78xx.c
> +static int lan78xx_configure_usb(struct lan78xx_net *dev, int speed)
> +{
> + u32 mask, val;
> + int ret;
> +
> + /* Return early if USB speed is not SuperSpeed */
> + if (dev->udev->speed != USB_SPEED_SUPER)
> + return 0;
> +
> + /* Update U1 and U2 settings based on speed */
> + if (speed != SPEED_1000) {
> + mask = USB_CFG1_DEV_U2_INIT_EN_ |
> USB_CFG1_DEV_U1_INIT_EN_;
> + val = USB_CFG1_DEV_U2_INIT_EN_ |
> USB_CFG1_DEV_U1_INIT_EN_;
> + return lan78xx_update_reg(dev, USB_CFG1, mask, val);
> + }
> +
> + /* For 1000 Mbps: disable U2 and enable U1 */
> + mask = USB_CFG1_DEV_U2_INIT_EN_;
> + val = 0;
> + ret = lan78xx_update_reg(dev, USB_CFG1, mask, val);
> + if (ret < 0)
> + return ret;
> +
> + mask = USB_CFG1_DEV_U1_INIT_EN_;
> + val = USB_CFG1_DEV_U1_INIT_EN_;
> + return lan78xx_update_reg(dev, USB_CFG1, mask, val);
Is it possible to combine the mask and val used above into a single
statement and pass it as argument to lan78xx_update_reg. You can have
it as else part.
> +}
> +
> @@ -4352,32 +4329,29 @@ static void lan78xx_disconnect(struct
> usb_interface *intf)
> struct lan78xx_net *dev;
> struct usb_device *udev;
> struct net_device *net;
> - struct phy_device *phydev;
>
> dev = usb_get_intfdata(intf);
> usb_set_intfdata(intf, NULL);
> if (!dev)
> return;
>
> - netif_napi_del(&dev->napi);
> -
> udev = interface_to_usbdev(intf);
> net = dev->net;
>
> + rtnl_lock();
> + phylink_stop(dev->phylink);
> + phylink_disconnect_phy(dev->phylink);
> + rtnl_unlock();
> +
> + netif_napi_del(&dev->napi);
> +
> unregister_netdev(net);
>
> timer_shutdown_sync(&dev->stat_monitor);
> set_bit(EVENT_DEV_DISCONNECT, &dev->flags);
> cancel_delayed_work_sync(&dev->wq);
>
> - phydev = net->phydev;
> -
> - phy_disconnect(net->phydev);
> -
> - if (phy_is_pseudo_fixed_link(phydev)) {
> - fixed_phy_unregister(phydev);
> - phy_device_free(phydev);
> - }
> + phylink_destroy(dev->phylink);
Before destroying phylink here is it possible to add a check to make
sure dev->phylink is allocated properly.
>
> usb_scuttle_anchored_urbs(&dev->deferred);
>
>
> @@ -5158,7 +5137,7 @@ static int lan78xx_reset_resume(struct
> usb_interface *intf)
> if (ret < 0)
> return ret;
>
> - phy_start(dev->net->phydev);
> + phylink_start(dev->phylink);
>
> ret = lan78xx_resume(intf);
>
> --
> 2.39.5
>
Powered by blists - more mailing lists