linux-kernel - Re: [PATCH net-next v1 1/7] net: usb: lan78xx: Convert to PHYlink for improved PHY and MAC management

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z35y8r32QFvZKQLI@shell.armlinux.org.uk>
Date: Wed, 8 Jan 2025 12:43:30 +0000
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Oleksij Rempel <o.rempel@...gutronix.de>
Cc: "David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Woojung Huh <woojung.huh@...rochip.com>,
	Andrew Lunn <andrew+netdev@...n.ch>, kernel@...gutronix.de,
	linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
	UNGLinuxDriver@...rochip.com, Phil Elwell <phil@...pberrypi.org>
Subject: Re: [PATCH net-next v1 1/7] net: usb: lan78xx: Convert to PHYlink
 for improved PHY and MAC management

On Wed, Jan 08, 2025 at 01:13:35PM +0100, Oleksij Rempel wrote:
> @@ -2508,6 +2369,207 @@ static void lan78xx_remove_irq_domain(struct lan78xx_net *dev)
>  	dev->domain_data.irqdomain = NULL;
>  }
>  
> +static void lan78xx_mac_config(struct phylink_config *config, unsigned int mode,
> +			       const struct phylink_link_state *state)
> +{
> +	struct net_device *net = to_net_dev(config->dev);
> +	struct lan78xx_net *dev = netdev_priv(net);
> +	u32 rgmii_id = 0;
> +	u32 mac_cr = 0;
> +	int ret;
> +
> +	/* Check if the mode is supported */
> +	if (mode != MLO_AN_FIXED && mode != MLO_AN_PHY) {
> +		netdev_err(net, "Unsupported negotiation mode: %u\n", mode);
> +		return;
> +	}
> +
> +	switch (state->interface) {
> +	case PHY_INTERFACE_MODE_INTERNAL:
> +	case PHY_INTERFACE_MODE_GMII:
> +			mac_cr |= MAC_CR_GMII_EN_;
> +		break;

The indentation has gone a bit weird here.

> +	case PHY_INTERFACE_MODE_RGMII_ID:
> +		break;

Normally, a MAC should support all RGMII interface modes, because these
define the RGMII delays at the PHY and have little dependence on the
MAC.

> +static int lan78xx_phylink_setup(struct lan78xx_net *dev)
> +{
> +	phy_interface_t link_interface;
> +	struct phylink *phylink;
> +
> +	dev->phylink_config.dev = &dev->net->dev;
> +	dev->phylink_config.type = PHYLINK_NETDEV;
> +	dev->phylink_config.mac_capabilities = MAC_SYM_PAUSE | MAC_ASYM_PAUSE |
> +		MAC_10 | MAC_100 | MAC_1000FD;
> +	dev->phylink_config.mac_managed_pm = true;
> +
> +	if (dev->chipid == ID_REV_CHIP_ID_7801_) {
> +		__set_bit(PHY_INTERFACE_MODE_RGMII,
> +			  dev->phylink_config.supported_interfaces);
> +		__set_bit(PHY_INTERFACE_MODE_RGMII_ID,
> +			  dev->phylink_config.supported_interfaces);
> +		__set_bit(PHY_INTERFACE_MODE_RGMII_RXID,
> +			  dev->phylink_config.supported_interfaces);
> +		__set_bit(PHY_INTERFACE_MODE_RGMII_TXID,
> +			  dev->phylink_config.supported_interfaces);

The mac_config implementation conflicts with this.

> +		link_interface = PHY_INTERFACE_MODE_NA;

This is supposed to be for DSA, not for general use. Is there any reason
you can't pass the real mode that is being used here?

> @@ -2576,7 +2673,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_INTERNAL;

This needs to be explained.

As for continuing to use fixed-phy, please instead use
phylink_set_fixed_link() instead.

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!