Re: [PATCH net-next v2 2/3] net: mdio: realtek-rtl9300: Convert "fwnode" left-overs to "of"

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 1 · Medium: 0 · Low: 0

- [High] Non-equivalent API conversion in otto_emdio_probe(): replacing
  `device_for_each_child_node_scoped(dev, child)` with…

--

net: mdio: realtek-rtl9300: Convert "fwnode" left-overs to "of"

This converts the remaining fwnode based property/child-node accesses in
the rtl9300 MDIO driver over to the OF API, and re-indents the
continuation line of otto_emdio_probe_one().

> diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
> index 11d078ef941cd..429321e9114fd 100644
> --- a/drivers/net/mdio/mdio-realtek-rtl9300.c
> +++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
> @@ -831,14 +831,14 @@ static void otto_emdio_notify_phy_detach(struct phy_device *phydev)
>  }
>  
>  static int otto_emdio_probe_one(struct device *dev, struct otto_emdio_priv *priv,
> -				 struct fwnode_handle *node)
> +				struct device_node *node)
>  {
>  	struct otto_emdio_chan *chan;
>  	struct mii_bus *bus;
>  	u32 mdio_bus;
>  	int err;
>  
> -	err = fwnode_property_read_u32(node, "reg", &mdio_bus);
> +	err = of_property_read_u32(node, "reg", &mdio_bus);
>  	if (err)
>  		return dev_err_probe(dev, err, "undefined smi bus number\n");
>  
> @@ -865,7 +865,7 @@ static int otto_emdio_probe_one(struct device *dev, struct otto_emdio_priv *priv
>  
>  	snprintf(bus->id, MII_BUS_ID_SIZE, "%s-%d", dev_name(dev), mdio_bus);
>  
> -	err = devm_of_mdiobus_register(dev, bus, to_of_node(node));
> +	err = devm_of_mdiobus_register(dev, bus, node);
>  	if (err)
>  		return dev_err_probe(dev, err, "cannot register MDIO bus\n");
>  
> @@ -1027,7 +1027,7 @@ static int otto_emdio_probe(struct platform_device *pdev)
>  			return dev_err_probe(dev, err, "failed to setup MDIO bus controller\n");
>  	}
>  
> -	device_for_each_child_node_scoped(dev, child) {
> +	for_each_child_of_node_scoped(dev->of_node, child) {

[Severity: High]

Is this iterator swap intended to be behaviour preserving?  The two
iterators differ in whether they honour the DT status property.

The old one dispatched into the OF fwnode ops:

drivers/of/property.c:of_fwnode_get_next_child_node() {
	return of_fwnode_handle(of_get_next_available_child(to_of_node(fwnode),
							    to_of_node(child)));
}

The new one does not filter:

include/linux/of.h
#define for_each_child_of_node_scoped(parent, child) \
	for (struct device_node *child __free(device_node) =		\
	     of_get_next_child(parent, NULL);				\
	     child != NULL;						\
	     child = of_get_next_child(parent, child))

So nodes with status = "disabled" now reach otto_emdio_probe_one().

Does that turn a disabled bus child into a probe failure for the whole
controller?  A disabled mdio-bus@N node still has a valid reg, so it
passes the reg and num_buses checks and ends up in:

	err = devm_of_mdiobus_register(dev, bus, node);
	if (err)
		return dev_err_probe(dev, err, "cannot register MDIO bus\n");

which reaches:

drivers/net/mdio/of_mdio.c:__of_mdiobus_register() {
	/* Do not continue if the node is disabled */
	if (!of_device_is_available(np))
		return -ENODEV;

and otto_emdio_probe() propagates that error immediately:

	for_each_child_of_node_scoped(dev->of_node, child) {
		err = otto_emdio_probe_one(dev, priv, child);
		if (err)
			return err;
	}

Previously the disabled child was skipped and the enabled buses still
came up.

This looks reachable with in-tree DT: arch/mips/boot/dts/realtek/rtl930x.dtsi
ships all four mdio-bus@0..3 children with status = "disabled", so boards
that enable the controller plus only the buses they use would now fail
probe entirely.

There is also a CONFIG_OF_MDIO=n variant of the same concern: the stub
devm_of_mdiobus_register() in include/linux/of_mdio.h forwards to
devm_mdiobus_register() with no availability check, so a DT-disabled bus
would be registered and address-scanned instead.

Finally, the sibling OF walk in the same driver still filters on
availability:

	for_each_available_child_of_node_scoped(ports_dn, port_dn) {

so otto_emdio_map_ports() and the bus enumeration would no longer agree on
which nodes are usable.  Would for_each_available_child_of_node_scoped(dev->of_node, child)
be the equivalent conversion here?

Note this is still present at the end of the series, in otto_emdio_probe()
at e4ad7bb3a35d ("net: mdio: realtek-rtl9300: Support non-default clock
frequency").

>  		err = otto_emdio_probe_one(dev, priv, child);
>  		if (err)
>  			return err;

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260906081946.3499898-1-markus.stockhausen%40gmx.de