Re: [PATCH net-next] net: usb: name usb ethernet mdio buses reliably

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] net: usb: MDIO bus name collision via usb_make_path() on
  shared bus_name
- [Low] net: usb: missing usb_make_path() MDIO bus naming in
  smsc95xx_bind()
--

commit 29b6f41fcf0619fc5074a9aed4a761b5bff3fdf8
Author: Blake Alexander <blake.alexander@airbus.com>

net: usb: name usb ethernet mdio buses reliably

This commit replaces the "usb-%03d:%03d" MDIO bus naming in asix_devices,
ax88172a, and lan78xx with usb_make_path() so that USB Ethernet devices at
fixed topology positions receive deterministic MDIO bus names.

> diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
> index 293ef80c4e302..c752fa3868b81 100644
> --- a/drivers/net/usb/asix_devices.c
> +++ b/drivers/net/usb/asix_devices.c
> @@ -680,9 +680,8 @@ static int ax88772_init_mdio(struct usbnet *dev)
>  	priv->mdio->write = &asix_mdio_bus_write;
>  	priv->mdio->name = "Asix MDIO Bus";
>  	priv->mdio->phy_mask = ~(BIT(priv->phy_addr & 0x1f) | BIT(AX_EMBD_PHY_ADDR));
> -	/* mii bus name is usb-<usb bus number>-<usb device number> */
> -	snprintf(priv->mdio->id, MII_BUS_ID_SIZE, "usb-%03d:%03d",
> -		 dev->udev->bus->busnum, dev->udev->devnum);
> +
> +	usb_make_path(dev->udev, priv->mdio->id, MII_BUS_ID_SIZE);
>  
>  	ret = mdiobus_register(priv->mdio);
>  	if (ret) {
> diff --git a/drivers/net/usb/ax88172a.c b/drivers/net/usb/ax88172a.c
> index f5036451f0869..9f050de0cddbd 100644
> --- a/drivers/net/usb/ax88172a.c
> +++ b/drivers/net/usb/ax88172a.c
> @@ -73,9 +73,8 @@ static int ax88172a_init_mdio(struct usbnet *dev)
>  	priv->mdio->read = &asix_mdio_bus_read;
>  	priv->mdio->write = &asix_mdio_bus_write;
>  	priv->mdio->name = "Asix MDIO Bus";
> -	/* mii bus name is usb-<usb bus number>-<usb device number> */
> -	snprintf(priv->mdio->id, MII_BUS_ID_SIZE, "usb-%03d:%03d",
> -		 dev->udev->bus->busnum, dev->udev->devnum);
> +
> +	usb_make_path(dev->udev, priv->mdio->id, MII_BUS_ID_SIZE);
>  
>  	ret = mdiobus_register(priv->mdio);
>  	if (ret) {
> diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
> index d73002b2c21ac..153aa56b8e681 100644
> --- a/drivers/net/usb/lan78xx.c
> +++ b/drivers/net/usb/lan78xx.c
> @@ -2117,8 +2117,7 @@ static int lan78xx_mdio_init(struct lan78xx_net *dev)
>  	dev->mdiobus->name = "lan78xx-mdiobus";
>  	dev->mdiobus->parent = &dev->udev->dev;
>  
> -	snprintf(dev->mdiobus->id, MII_BUS_ID_SIZE, "usb-%03d:%03d",
> -		 dev->udev->bus->busnum, dev->udev->devnum);
> +	usb_make_path(dev->udev, dev->mdiobus->id, MII_BUS_ID_SIZE);
>  
>  	switch (dev->chipid) {
>  	case ID_REV_CHIP_ID_7800_:

[Severity: Medium]
Can usb_make_path() produce duplicate mii_bus->id strings across the USB
2.0 and USB 3.0 buses of an xHCI controller?

usb_make_path() formats "usb-%s-%s" from udev->bus->bus_name and
udev->devpath, omitting udev->bus->busnum.  Dual-roothub xHCI drivers
(such as xhci-pci and xhci-plat) register both the USB 2.0 main_hcd and
the USB 3.0 shared_hcd with the same controller device name as bus_name,
and number their roothub ports independently starting at 1:

drivers/usb/host/xhci-pci.c:xhci_pci_common_probe() {
	...
	xhci->shared_hcd = usb_create_shared_hcd(&xhci_pci_hc_driver, &dev->dev,
						 pci_name(dev), hcd);
}

When a USB 2.0 adapter on main_hcd and a SuperSpeed adapter (such as a
LAN7800) on shared_hcd sit at the same relative devpath (for example,
when USB 2.0 port N and USB 3.0 port N map to different physical
connectors or match behind downstream hubs), both receive the same
mii_bus->id.  __mdiobus_register() then fails with -EEXIST in
device_register(&bus->dev), causing probe or bind of the second network
interface to fail.

[Severity: Low]
Should smsc95xx_bind() also be updated to match the other USB Ethernet
MDIO bus drivers?

drivers/net/usb/smsc95xx.c:smsc95xx_bind() still formats
pdata->mdiobus->id using busnum and devnum, even though it already uses
usb_make_path() earlier in the same function for its IRQ domain fwnode:

drivers/net/usb/smsc95xx.c:smsc95xx_bind() {
	...
	snprintf(pdata->mdiobus->id, ARRAY_SIZE(pdata->mdiobus->id),
		 "usb-%03d:%03d", dev->udev->bus->busnum, dev->udev->devnum);
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109-net-usb-mdio-naming-v1-1-f4cca6737784@airbus.com?part=1