Re: [PATCH net-next v11 1/3] dt-bindings: net: wiznet,w5100: convert to DT schema

[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] In Documentation/devicetree/bindings/net/wiznet,w5100.yaml, `spi-
  max-frequency` has a single `maximum: 80000000` that applies to all…

--

dt-bindings: net: wiznet,w5100: convert to DT schema

This commit converts the Wiznet w5x00 SPI Ethernet controller binding
from plain text to DT schema. It also changes the example interrupt
trigger type from edge-falling to level-low to match the driver, and
adds a MAINTAINERS entry for the new YAML file.

> diff --git a/Documentation/devicetree/bindings/net/wiznet,w5100.yaml b/Documentation/devicetree/bindings/net/wiznet,w5100.yaml
> new file mode 100644
> index 000000000000..df83f90b3919
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/wiznet,w5100.yaml

[ ... ]

> +allOf:
> +  - $ref: /schemas/net/ethernet-controller.yaml#
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - wiznet,w5100
> +      - wiznet,w5200
> +      - wiznet,w5500

[ ... ]

> +  spi-max-frequency:
> +    description:
> +      According to the datasheets, w5500 and w5200 allow a maximum of 80 MHz
> +      while w5100 is reported by users to allow a maximum of 14 MHz. However,
> +      board designs may need to limit this value.
> +    maximum: 80000000

[Severity: Low]
The description gives w5100 a lower limit of about 14 MHz, but the only
constraint here is maximum: 80000000, and it applies to all three
compatibles. Should the schema enforce the w5100 limit as well?

The allOf above only holds the two $refs. Nothing like this lowers the
limit for w5100:

  if:
    properties:
      compatible:
        contains:
          const: wiznet,w5100
  then:
    properties:
      spi-max-frequency:
        maximum: 14000000

As a result, a node with compatible = "wiznet,w5100" and, for example,
the 30 MHz value from the example would pass dt_binding_check and
dtbs_check.

The driver does not clamp the rate either. w5100_spi_probe() picks
w5100_spi_ops from the match data and passes the device on to
w5100_probe(), and the SPI core uses the max_speed_hz from DT as is.

The same gap remains at the end of the series. The allOf if/then block
added by "dt-bindings: net: wiznet,w5100: add link status interrupt"
only restricts interrupts and does not touch spi-max-frequency.

The old text binding had no limit at all, so this is not a regression
from the conversion. If the 14 MHz figure is left unenforced on purpose
because it comes from user reports rather than the datasheet, could the
binding say so?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260924-wiznet-link-gpio-v11-0-42a332517b30%40marmottus.net