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

[Wei Fang]

> > @@ -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.
>