Re: [net-next,v15,12/13] net: mdio: realtek-rtl9300: Add support for RTL839x

[netdev-bot+sashiko@kernel.org]

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

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

- [Low] The binding schema
  Documentation/devicetree/bindings/net/realtek,rtl9301-mdio.yaml…

--

net: mdio: realtek-rtl9300: Add support for RTL839x

This adds the register definitions, C22/C45 accessors and the
otto_emdio_info instance for the RTL839x (cypress) family, plus the
realtek,rtl8391-mdio compatible to otto_emdio_ids[]. The new device
declares 2 SMI buses, 8192 pages and 52 ports.

> diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
> index 57ab89afb9e5e..86b2040fa3335 100644
> --- a/drivers/net/mdio/mdio-realtek-rtl9300.c
> +++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
> @@ -148,6 +148,28 @@
>  #define RTL8380_SMI_POLL_CTRL			0xa17c
>  #define RTL8380_SMI_PORT0_5_ADDR_CTRL		0xa1c8
>  
> +#define RTL8390_NUM_BUSES			2

[ ... ]

> @@ -980,6 +1058,29 @@ static const struct otto_emdio_info otto_emdio_8380_info = {
>  	.write_c45 = otto_emdio_8380_write_c45,
>  };
>  
> +static const struct otto_emdio_info otto_emdio_8390_info = {
> +	.cmd_fail = RTL8390_PHY_CTRL_FAIL,
> +	.cmd_read = RTL8390_PHY_CTRL_READ,
> +	.cmd_write = RTL8390_PHY_CTRL_WRITE,
> +	.cmd_regs = {
> +		.broadcast = RTL8390_BCAST_PHYID_CTRL,
> +		.c22_data = RTL8390_PHYREG_ACCESS_CTRL,
> +		.c45_data = RTL8390_PHYREG_MMD_CTRL,
> +		.ext_page = RTL8390_PHYREG_CTRL,
> +		.io_data = RTL8390_PHYREG_DATA_CTRL,
> +		.port_mask_low = RTL8390_PHYREG_PORT_CTRL_LOW,
> +		.port_mask_high = RTL8390_PHYREG_PORT_CTRL_HIGH,
> +	},
> +	.num_buses = RTL8390_NUM_BUSES,

[Severity: Low]
Should the binding schema be narrowed for this compatible too?

Documentation/devicetree/bindings/net/realtek,rtl9301-mdio.yaml has a
single unconditional child-node pattern that applies to every compatible
in the file:

    patternProperties:
      '^mdio-bus@[0-3]$':
        $ref: mdio.yaml#

There is no allOf/if-then block that restricts the index per compatible,
so a realtek,rtl8391-mdio node containing mdio-bus@2 or mdio-bus@3 passes
dt_binding_check, while the driver data added here says the part only has
two buses.

The driver then rejects such a tree in otto_emdio_probe_one():

	if (mdio_bus >= priv->info->num_buses)
		return dev_err_probe(dev, -EINVAL,
				     "illegal (dangling) smi bus number %d\n", mdio_bus);

and again in otto_emdio_map_ports() when a phy-handle resolves to a bus
node with reg = <2>:

	if (bus >= priv->info->num_buses) {
		err = dev_err_probe(dev, -EINVAL, "illegal smi bus number %d\n", bus);

Since the whole probe fails in that case, no MDIO bus is registered at all
for the switch. Would it make sense to add if-then blocks keying on
realtek,rtl8391-mdio (2 buses) and realtek,rtl8380-mdio (1 bus) so the
schema enforces the same limit the driver does?

> +	.num_pages = RTL8390_NUM_PAGES,
> +	.num_ports = RTL8390_NUM_PORTS,
> +	.poll_ctrl = RTL8390_SMI_PORT_POLLING_CTRL,
> +	.read_c22 = otto_emdio_8390_read_c22,
> +	.read_c45 = otto_emdio_8390_read_c45,
> +	.write_c22 = otto_emdio_8390_write_c22,
> +	.write_c45 = otto_emdio_8390_write_c45,
> +};
> +
>  static const struct otto_emdio_info otto_emdio_9300_info = {

[ ... ]

> @@ -1031,6 +1132,7 @@ static const struct otto_emdio_info otto_emdio_9310_info = {
>  
>  static const struct of_device_id otto_emdio_ids[] = {
>  	{ .compatible = "realtek,rtl8380-mdio", .data = &otto_emdio_8380_info },
> +	{ .compatible = "realtek,rtl8391-mdio", .data = &otto_emdio_8390_info },
>  	{ .compatible = "realtek,rtl9301-mdio", .data = &otto_emdio_9300_info },
>  	{ .compatible = "realtek,rtl9311-mdio", .data = &otto_emdio_9310_info },
>  	{}

For reference, the lax [0-3] pattern predates this series, but until now it
only covered realtek,rtl9301-mdio and realtek,rtl9311-mdio, which do have
four buses (RTL9300_NUM_BUSES and RTL9310_NUM_BUSES are both 4), so the
schema matched the driver. The mismatch appears with the 2-bus RTL839x
compatible claimed here.

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