Re: [PATCH net-next v25 1/3] dt-bindings: net: Document Motorcomm YT8824 PHY package

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The `reg` description in motorcomm,yt8824.yaml gives fixed
  hardware rules: 9 in internal mode, and member-PHY base + 4 in…

--

dt-bindings: net: Document Motorcomm YT8824 PHY package

This adds a YAML binding for the Motorcomm YT8824 PHY package, which holds
four PHYs. It defines the motorcomm,yt8824-package compatible, a
motorcomm,package-mode property (internal or 10g-qxgmii), and a reg that
points at the shared top extend register block. It also adds the binding
and a new maintainer to the MOTORCOMM PHY DRIVER entry in MAINTAINERS.

> diff --git a/Documentation/devicetree/bindings/net/motorcomm,yt8824.yaml b/Documentation/devicetree/bindings/net/motorcomm,yt8824.yaml
> new file mode 100644
> index 0000000000000..4c67f8afcf27a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/motorcomm,yt8824.yaml

[ ... ]

> +  motorcomm,package-mode:
> +    description: |
> +      PHY package can be configured in 2 modes:
> +      - internal: YT8824 is embedded in the switch, using
> +        an internal interface to the MAC.
> +      - 10g-qxgmii: YT8824 is a standalone external PHY,
> +        connected via 10G QXGMII.
> +    $ref: /schemas/types.yaml#/definitions/string
> +    enum: [ internal, 10g-qxgmii ]
> +
> +  reg:
> +    description:
> +      The absolute MDIO address of the YT8824 shared top extend
> +      register block.
> +      For internal mode, this is fixed at 9;
> +      For external mode, it is the base MDIO address of the four
> +      member PHYs plus 4, where base is typically 0x0.
> +      Note that this is NOT the reg of the first member PHY.

[Severity: Low]
This isn't a bug, but should the internal mode value be enforced by the
schema and not only described in prose?

As written, nothing ties reg to motorcomm,package-mode. The inherited
ethernet-phy-package.yaml only limits reg to the 0..31 range. So a node
with package-mode = "internal" and a reg other than 9 still passes
dt_binding_check.

The driver uses this value directly. of_phy_package_join() reads the
package reg as base_addr, and the YT8824 top extend accessors in
drivers/net/phy/motorcomm.c go to offset 0 from that base:

    ret = __phy_package_write(phydev, 0, YTPHY_PAGE_SELECT, regnum);

If the package reg is wrong, the top block page select and data accesses
would go to whatever MDIO address was given. Under the base schema
convention, that would usually be the first member PHY.

The external mode rule (child base plus 4) can't be expressed in
json-schema. The internal mode rule could be, though, with something
like:

    allOf:
      - if:
          properties:
            motorcomm,package-mode:
              const: internal
        then:
          properties:
            reg:
              const: 9

> +
> +required:
> +  - compatible
> +  - motorcomm,package-mode
> +  - reg

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261015145021.2216003-1-kyle.switch%40motor-comm.com