RE: [net-next,10/14] net: dsa: netc: introduce NXP NETC switch driver for i.MX94

Wei Fang
2026-03-19T02:48:30Z

> > @@ -0,0 +1,698 @@
> > +static int netc_port_create_emdio_bus(struct netc_port *np,
> > + struct device_node *node)
> > +{
> > + struct netc_switch *priv = np->switch_priv;
> > + struct enetc_mdio_priv *mdio_priv;
> > + struct device *dev = priv->dev;
> > + struct enetc_hw *hw;
> > + struct mii_bus *bus;
> > + int err;
> > +
> > + hw = enetc_hw_alloc(dev, np->iobase);
> > + if (IS_ERR(hw))
> > + return dev_err_probe(dev, PTR_ERR(hw),
> > + "Failed to allocate enetc_hw\n");
> > +
> > + bus = devm_mdiobus_alloc_size(dev, sizeof(*mdio_priv));
> > + if (!bus)
> > + return -ENOMEM;
> > +
> > + bus->name = "NXP NETC switch external MDIO Bus";
> > + bus->read = enetc_mdio_read_c22;
> > + bus->write = enetc_mdio_write_c22;
> > + bus->read_c45 = enetc_mdio_read_c45;
> > + bus->write_c45 = enetc_mdio_write_c45;
> > + bus->parent = dev;
> > + mdio_priv = bus->priv;
> > + mdio_priv->hw = hw;
> > + mdio_priv->mdio_base = NETC_EMDIO_BASE;
> > + snprintf(bus->id, MII_BUS_ID_SIZE, "%s-p%d-emdio",
> > + dev_name(dev), np->dp->index);
> > +
> > + err = devm_of_mdiobus_register(dev, bus, node);
> > + if (err)
> > + return dev_err_probe(dev, err,
> > + "Cannot register EMDIO bus\n");
> > +
> > + np->emdio = bus;
> > +
> > + return 0;
> > +}
> > +
> > +static int netc_port_create_mdio_bus(struct netc_port *np,
> > + struct device_node *node)
> > +{
> > + struct device_node *mdio_node;
> > + int err;
> > +
> > + mdio_node = of_get_child_by_name(node, "mdio");
> > + if (mdio_node) {
> > + err = netc_port_create_emdio_bus(np, mdio_node);
> > + of_node_put(mdio_node);
> > + if (err)
> > + return err;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void netc_port_free_mdio_bus(struct netc_port *np)
> > +{
> > + if (np->emdio)
> > + mdiobus_unregister(np->emdio);
> ^^^^
>
> Does this cause a double unregister? The MDIO bus is registered with
> devm_of_mdiobus_register() in netc_port_create_emdio_bus(), which
> installs a devres action that automatically calls mdiobus_unregister()
> during device removal.
>
> However, netc_port_free_mdio_bus() also explicitly calls
> mdiobus_unregister(np->emdio).
>
> During device removal:
>
> netc_switch_remove()
> -> dsa_unregister_switch()
> -> dsa_switch_teardown()
> -> netc_teardown()
> -> netc_free_mdio_bus()
> -> netc_port_free_mdio_bus()
> -> mdiobus_unregister() // first call
>
> Then devres cleanup fires:
>
> devm_mdiobus_unregister()
> -> mdiobus_unregister() // second call
> -> WARN_ON_ONCE(bus->state != MDIOBUS_REGISTERED)
>
> The second call triggers a warning at drivers/net/phy/mdio_bus_provider.c:675
> because the bus state is already MDIOBUS_UNREGISTERED. With
> panic_on_warn=1, this causes a kernel panic.

Good catch, I changed mdiobus_register() to devm_of_mdiobus_register(), but I
forgot to remove mdiobus_unregister(). I will fix this issue.

>
> The same double-unregister occurs on the error path of
> netc_init_all_ports(), where netc_free_mdio_bus() is called, but devres
> will also fire during device teardown.
>
> > +}
>
> [ ... ]
>
> > +static int netc_switch_pci_init(struct pci_dev *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + struct netc_switch_regs *regs;
> > + struct netc_switch *priv;
> > + int err;
> > +
> > + pcie_flr(pdev);
> > + err = pci_enable_device_mem(pdev);
> > + if (err)
> > + return dev_err_probe(dev, err, "Failed to enable device\n");
> > +
> > + /* The command BD rings and NTMP tables need DMA */
> > + dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
> ^^^^
>
> Is the return value being checked? The comment states "command BD rings
> and NTMP tables need DMA", confirming DMA is essential for this driver.

No need to check the return value, because dma_set_mask_and_coherent()
never return fail when DMA_BIT_MASK(64).

See https://elixir.bootlin.com/linux/v7.0-rc4/source/Documentation/core-api/dma-api-howto.rst#L269

>
> The function can fail with -EIO when dma_supported() returns false for
> the 64-bit mask (from dma_set_mask() in kernel/dma/mapping.c:902).
>
> The sibling enetc driver from the same NXP NETC subsystem checks this
> return value in enetc.c:
>
> err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
> if (err) {
> ...
> goto err_dma;
> }
>
> If the 64-bit DMA mask setup fails, the DMA mask remains at the default,
> and subsequent DMA operations for NTMP table management (added in later
> commits) may silently fail or produce incorrect results on systems with
> memory above 4GB.
>