Re: [PATCH net-next v25 03/10] dt-bindings: mfd: Document support for Airoha AN8855 Switch SoC

[netdev-bot+sashiko]

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

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

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The two AN8855 binding examples disagree on how the internal
  PHYs are accessed.

--

dt-bindings: mfd: Document support for Airoha AN8855 Switch SoC

This commit adds a YAML binding for the Airoha AN8855 Switch SoC. The SoC
exposes an Ethernet switch, an eFUSE NVMEM provider and internal Ethernet
PHYs. The example puts the internal PHYs on a nested mdio node.

> diff --git a/Documentation/devicetree/bindings/mfd/airoha,an8855.yaml b/Documentation/devicetree/bindings/mfd/airoha,an8855.yaml
> new file mode 100644
> index 0000000000000..48127e956c965
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/airoha,an8855.yaml
> @@ -0,0 +1,176 @@

[ ... ]

> +            mdio {
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +
> +                internal_phy1: ethernet-phy@1 {
> +                  compatible = "ethernet-phy-idc0ff.0410",
> +                               "ethernet-phy-ieee802.3-c22";

[Severity: Medium]
This example gives the internal PHYs the ethernet-phy-ieee802.3-c22
compatible. The PHY binding added earlier in this series uses the C45
compatible for the same PHY ID:

Documentation/devicetree/bindings/net/airoha,an8855-phy.yaml:
        ethernet-phy@1 {
            compatible = "ethernet-phy-idc0ff.0410",
                         "ethernet-phy-ieee802.3-c45";

Which of the two is correct?

The PHY binding selects on ethernet-phy-idc0ff.0410, so it also applies
to the PHY nodes nested here. Its $ref to ethernet-phy.yaml accepts both
c22 and c45, which means dt_binding_check won't catch the mismatch.

Later in the series, the MFD driver puts each PHY on a bus created with
devm_mdio_regmap_register(). That bus only provides C22 accessors:

drivers/net/mdio/mdio-regmap.c:devm_mdio_regmap_register()
    mii->read = mdio_regmap_read_c22;
    mii->write = mdio_regmap_write_c22;

Would a board DT that copies the PHY binding example end up on this path?

an8855_core_probe()
  fwnode_mdiobus_register_phy()      /* is_c45 is true */
    get_phy_device(bus, addr, true)
      get_phy_c45_ids()
        get_phy_c45_devs_in_pkg()
          mdiobus_c45_read()
            __mdiobus_c45_read()     /* -EOPNOTSUPP, no read_c45 */

That error turns into -EIO. an8855_core_probe() then jumps to
err_put_fwnodes, so the switch, eFUSE and PHYs never get registered.

The switch binding says its registers are reached by configuring the PHY
page. That also points to C22 access.

Should the PHY binding example use ethernet-phy-ieee802.3-c22, or drop the
C45 compatible, so the two bindings agree?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261009161102.38961-1-ansuelsmth%40gmail.com