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

[Arthur Crepin Leblond <arthur@marmottus.net>]

On Thu, Sep 24, 2026 at 01:55:53AM +0000, netdev-bot+sashiko@kernel.org wrote:
>[Severity: Low]
>The description here says the w5100 tops out around 14 MHz, but the only
>machine-checkable constraint is the loosest value, 80000000, and it applies
>to all three compatibles.
>
>Should the w5100 limit be expressed as a conditional instead, so that it is
>actually validated?
>
>  allOf:
>    - if:
>        properties:
>          compatible:
>            contains:
>              const: wiznet,w5100
>      then:
>        properties:
>          spi-max-frequency:
>            maximum: 14000000
>
>As written, a node with:
>
>    compatible = "wiznet,w5100";
>    spi-max-frequency = <80000000>;
>
>passes dt_binding_check and dtbs_check with no diagnostic. Nothing in
>drivers/net/ethernet/wiznet/ looks at spi->max_speed_hz either, so the
>14 MHz figure is not enforced anywhere.
>
>This is still the case at the end of the series: the allOf if/then block
>added later by "dt-bindings: net: wiznet,w5100: add link status interrupt"
>only constrains interrupts, and spi-max-frequency keeps the unconditional
>80 MHz ceiling.
>
>Separately, the old wiznet,w5x00.txt documented the 80 MHz figure for the
>w5500 only:
>
>    - spi-max-frequency: Maximum frequency of the SPI bus when accessing
>      the w5500.
>
>The new schema extends that ceiling to w5100 and w5200 as well. Could the
>commit message mention that change?
I don't think it is really an issue here, the user must put a relevant
max spi frequency when creating their DT.

>
>[ ... ]
>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index df8ab9b824024..9f84e4c8163e4 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -29450,6 +29450,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]
>The commit message describes only the .txt to .yaml conversion and the
>trigger type change, but this hunk also creates a new maintainership entry.
>The "v8: Add a myself in MAINTAINERS" note is below the --- line and will
>not land in git history.
>
>Could the commit message describe the MAINTAINERS addition too?
>
>Also, at this commit the section is titled "WIZNET W5100 ETHERNET DRIVER"
>while the only F: pattern matches the YAML binding, so get_maintainer.pl
>still reports no maintainer for drivers/net/ethernet/wiznet/. That gap is
>closed later in the series: "w5100: detect carrier state using link status
>bit and optional interrupt" adds F: drivers/net/ethernet/wiznet/ to the
>same block. Would it make sense to add that F: line here, where the section
>is introduced?
>
>--
Will be fixed in v11

Arthur