Re: [PATCH net-next v9 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 2 potential
issue(s) to consider.

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

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] In Documentation/devicetree/bindings/net/wiznet,w5100.yaml the
  new machine-checkable constraint 'spi-max-frequency: maximum:…
- [Low] Undisclosed MAINTAINERS change: the commit adds a new "WIZNET
  W5100 ETHERNET DRIVER" section (MAINTAINERS:29442) that is never…

--

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

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

> diff --git a/Documentation/devicetree/bindings/net/wiznet,w5100.yaml b/Documentation/devicetree/bindings/net/wiznet,w5100.yaml
> new file mode 100644
> index 000000000000..25098308eb6e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/wiznet,w5100.yaml
> @@ -0,0 +1,68 @@

[ ... ]

> +properties:
> +  compatible:
> +    enum:
> +      - wiznet,w5100
> +      - wiznet,w5200
> +      - wiznet,w5500
> +
> +  interrupts:
> +    maxItems: 1
> +    description: Main interrupt (INT pin on the device).
> +
> +  reg:
> +    maxItems: 1
> +
> +  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: Medium]
Do the description and the constraint here disagree for wiznet,w5100?

The text says w5100 is limited to about 14 MHz, but the only
machine-checkable limit is a single unconditional maximum of 80000000 shared
by all three compatibles.  So this node passes dt_binding_check:

    ethernet@0 {
        compatible = "wiznet,w5100";
        reg = <0>;
        interrupts = <25 IRQ_TYPE_LEVEL_LOW>;
        spi-max-frequency = <80000000>;
    };

Nothing downstream clamps it either: w5100_spi_read() and w5100_spi_write()
in drivers/net/ethernet/wiznet/w5100-spi.c issue their transfers at the SPI
device's configured speed, with no per-chip ceiling, so the out-of-spec rate
is what actually gets driven for every register and packet buffer access.

Would an if-then block be preferable, keeping 80 MHz for the other two
parts?

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

Note the removed .txt scoped its 80 MHz statement to the w5500 only and had
no machine-checkable limit, so the unconditional bound is new here.  The
single maximum of 80000000 is still the only spi-max-frequency constraint at
the end of the series (9d871f2771b9), where the added allOf block constrains
only interrupts.

[ ... ]

> diff --git a/MAINTAINERS b/MAINTAINERS
> index cae5962045dd..e6a5a4e245a0 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -29439,6 +29439,12 @@ M:	Miloslav Trmac <mitr@volny.cz>
>  S:	Maintained
>  F:	drivers/input/misc/wistron_btns.c
>  
> +WIZNET W5100 ETHERNET DRIVER
> +M:	Arthur Crepin Leblond <arthur@marmottus.net>
> +L:	netdev@vger.kernel.org
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/net/wiznet,w5100.yaml

[Severity: Low]
Could the commit message mention this new MAINTAINERS section?  The
preserved log text only describes the .txt to YAML conversion and the
edge-falling to level-low trigger change; the new maintainership claim is
mentioned only in the changelog below the --- separator, which does not end
up in git history.

The section is named as a driver entry, but its only F: line matches the
binding file, so at this commit scripts/get_maintainer.pl will not route
drivers/net/ethernet/wiznet/ changes here.  That part is addressed later in
the same series: the commit "w5100: detect carrier state using link status
bit and optional interrupt" adds F: drivers/net/ethernet/wiznet/ to this
section, so the mismatch exists only between this patch and that one.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260918-wiznet-link-gpio-v9-0-5536f24d8d6d%40marmottus.net