| 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: <1bb51aad80be4bb5e0413089e1b1bf747db4e123.camel@microchip.com>
Date: Mon, 10 Mar 2025 09:29:45 +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>,
<linux-kernel@...r.kernel.org>, <netdev@...r.kernel.org>,
<UNGLinuxDriver@...rochip.com>
Subject: Re: [PATCH net-next v2 1/7] net: usb: lan78xx: Convert to PHYlink for
improved PHY and MAC management
Hi Oleksji,
On Fri, 2025-03-07 at 19:24 +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`.
>
> Signed-off-by: Oleksij Rempel <o.rempel@...gutronix.de>
> ---
> 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
> ---
> drivers/net/usb/lan78xx.c | 565 ++++++++++++++++++++--------------
> ----
> 1 file changed, 300 insertions(+), 265 deletions(-)
>
> static int lan78xx_phy_init(struct lan78xx_net *dev)
> {
> - __ETHTOOL_DECLARE_LINK_MODE_MASK(fc) = { 0, };
> - int ret;
> - u32 mii_adv;
> struct phy_device *phydev;
> + int ret;
>
> switch (dev->chipid) {
> case ID_REV_CHIP_ID_7801_:
> phydev = lan7801_phy_init(dev);
> + /* If no PHY found, set fixed link, probably there is
> no
> + * device or some kind of different device like
> switch.
> + * For example: EVB-KSZ9897-1 (KSZ9897 switch
> evaluation board
> + * with LAN7801 & KSZ9031)
> + */
> if (!phydev) {
> - netdev_err(dev->net, "lan7801: PHY Init
> Failed");
> - return -EIO;
> + struct phylink_link_state state = {
> + .speed = SPEED_1000,
> + .duplex = DUPLEX_FULL,
> + .interface =
> PHY_INTERFACE_MODE_RGMII,
> + };
> +
> + ret = phylink_set_fixed_link(dev->phylink,
> &state);
> + if (ret)
> + netdev_err(dev->net, "Could not set
> fixed link\n");
> +
> + return ret;
> }
> +
> break;
>
> case ID_REV_CHIP_ID_7800_:
> @@ -2576,7 +2671,7 @@ static int lan78xx_phy_init(struct lan78xx_net
> *dev)
> return -EIO;
> }
> phydev->is_internal = true;
> - dev->interface = PHY_INTERFACE_MODE_GMII;
> + phydev->interface = PHY_INTERFACE_MODE_GMII;
> break;
>
> default:
> @@ -2591,36 +2686,14 @@ static int lan78xx_phy_init(struct
> lan78xx_net *dev)
> phydev->irq = PHY_POLL;
> netdev_dbg(dev->net, "phydev->irq = %d\n", phydev->irq);
>
> - /* set to AUTOMDIX */
> - phydev->mdix = ETH_TP_MDI_AUTO;
> -
> - ret = phy_connect_direct(dev->net, phydev,
> - lan78xx_link_status_change,
> - dev->interface);
> + ret = phylink_connect_phy(dev->phylink, phydev);
> if (ret) {
> netdev_err(dev->net, "can't attach PHY to %s\n",
> dev->mdiobus->id);
> - if (dev->chipid == ID_REV_CHIP_ID_7801_) {
> - if (phy_is_pseudo_fixed_link(phydev)) {
> - fixed_phy_unregister(phydev);
> - phy_device_free(phydev);
> - }
> - }
> return -EIO;
> }
>
> - /* MAC doesn't support 1000T Half */
> - phy_remove_link_mode(phydev,
> ETHTOOL_LINK_MODE_1000baseT_Half_BIT);
> -
> - /* support both flow controls */
> - dev->fc_request_control = (FLOW_CTRL_RX | FLOW_CTRL_TX);
> - linkmode_clear_bit(ETHTOOL_LINK_MODE_Pause_BIT,
> - phydev->advertising);
> - linkmode_clear_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
> - phydev->advertising);
> - mii_adv = (u32)mii_advertise_flowctrl(dev-
> >fc_request_control);
> - mii_adv_to_linkmode_adv_t(fc, mii_adv);
> - linkmode_or(phydev->advertising, fc, phydev->advertising);
> + phy_suspend(phydev);
>
Why phy_suspend called in the init? Is there any specific reason?
> phy_support_eee(phydev);
>
> @@ -2646,10 +2719,6 @@ static int lan78xx_phy_init(struct lan78xx_net
> *dev)
> }
> }
>
> - genphy_config_aneg(phydev);
> -
> - dev->fc_autoneg = phydev->autoneg;
> -
> return 0;
> }
>
>
Powered by blists - more mailing lists