Re: [PATCH net-next v9] net: mdio: Add RTL9300 MDIO driver

[Christophe JAILLET]

Le 10/03/2025 à 00:25, Chris Packham a écrit :
> Add a driver for the MDIO controller on the RTL9300 family of Ethernet
> switches with integrated SoC. There are 4 physical SMI interfaces on the
> RTL9300 however access is done using the switch ports. The driver takes
> the MDIO bus hierarchy from the DTS and uses this to configure the
> switch ports so they are associated with the correct PHY. This mapping
> is also used when dealing with software requests from phylib.

...

> +	fwnode_for_each_child_node(ports, port) {
> +		struct device_node *mdio_dn;
> +		u32 addr;
> +		u32 bus;
> +		u32 pn;
> +
> +		struct device_node *phy_dn __free(device_node) =
> +			of_parse_phandle(to_of_node(port), "phy-handle", 0);
> +		/* skip ports without phys */
> +		if (!phy_dn)
> +			continue;
> +
> +		mdio_dn = phy_dn->parent;
> +		/* only map ports that are connected to this mdio-controller */
> +		if (mdio_dn->parent != dev->of_node)
> +			continue;
> +
> +		err = fwnode_property_read_u32(port, "reg", &pn);
> +		if (err)
> +			return err;
> +
> +		if (pn >= MAX_PORTS)
> +			return dev_err_probe(dev, -EINVAL, "illegal port number %d\n", pn);

While checking for illegal port numbers, does the following make sense:

	if (test_bit(pn, priv->valid_ports)
		return dev_err_probe(dev, -EINVAL, "duplicated port number %d\n", pn);

> +
> +		err = of_property_read_u32(mdio_dn, "reg", &bus);
> +		if (err)
> +			return err;
> +
> +		if (bus >= MAX_SMI_BUSSES)
> +			return dev_err_probe(dev, -EINVAL, "illegal smi bus number %d\n", bus);
> +
> +		err = of_property_read_u32(phy_dn, "reg", &addr);
> +		if (err)
> +			return err;
> +
> +		bitmap_set(priv->valid_ports, pn, 1);

set_bit(pn, priv->valid_ports) ?

> +		priv->smi_bus[pn] = bus;
> +		priv->smi_addr[pn] = addr;
> +	}

...

CJ