Re: [PATCH v2 2/2] dt-bindings: net: convert wiznet,w5x00 to YAML

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] The device tree example contains multiple syntax errors and unresolved references that will cause `make dt_binding_check` to fail fatally.
- [Low] The `pinctrl-names` property incorrectly uses `enum` alongside `minItems: 1` for a string array constraint.
--

--- Patch [2]: [PATCH v2 2/2] dt-bindings: net: convert wiznet,w5x00 to YAML ---
commit 03b1fc0266172468d4c1e28d10eec5fa26175fe7
Author: Arthur Crépin Leblond <[email protected]>

dt-bindings: net: convert wiznet,w5x00 to YAML
    
Convert the Wiznet w5x00 SPI Ethernet controller binding from
text to DT schema format.

> diff --git a/Documentation/devicetree/bindings/net/wiznet,w5x00.yaml b/Documentation/devicetree/bindings/net/wiznet,w5x00.yaml
> new file mode 100644
> index 0000000000000..8f45a5d773fe3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/wiznet,w5x00.yaml
> @@ -0,0 +1,103 @@
[ ... ]
> +  pinctrl-names:
> +    description:
> +      List of assigned state names, see pinctrl binding documentation.
> +    enum:
> +      - default
> +    minItems: 1

[Severity: Low]
Does this incorrectly restrict pinctrl-names to a scalar string instead of a
string-array?

Since pinctrl-names is inherently defined as a string-array, using
enum: [ default ] restricts the property to a scalar string, which will cause
schema validation to fail for valid device trees. The standard approach
requires:

  items:
    - const: default

[ ... ]
> +examples:
> +  - |
> +    /* Example (for Raspberry Pi with pin control stuff for GPIO irq) */
> +
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        ethernet@0: w5500@0 {

[Severity: Low]
Is the label ethernet@0 valid here?

DTS labels cannot contain the '@' character. This will cause a strict
compilation failure when this YAML example is parsed by dtc during
make dt_binding_check.

> +          compatible = "wiznet,w5500";
> +            reg = <0>;
> +            pinctrl-names = "default";
> +            pinctrl-0 = <&eth1_pins>;
> +            interrupt-parent = <&gpio>;

[Severity: Low]
Could this unresolved &gpio phandle reference cause a compiler error during
dt_binding_check?

Since &gpio is not defined in the scope of this standalone example, dtc will
fail to resolve it.

> +            interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
> +            spi-max-frequency = <30000000>;
> +            link-gpios = <&gpio 4 GPIO_ACTIVE_HIGH>;
> +            local-mac-address = [ a0 b0 c0 d0 e0 f0 ];
> +        };
> +    };
> +
> +    &gpio {
> +      eth1_pins: eth1_pins {
> +        brcm,pins = <25 4>;
> +        brcm,function = <0 0>; /* in */
> +        brcm,pull = <0 0>; /* none */
> +    };

[Severity: Low]
Are we missing a final closing brace for the &gpio node here?

This syntax error seems to have been introduced during the conversion from
the text binding, and it will prevent make dt_binding_check from compiling
the example.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2