Re: [PATCH net-next v5 2/3] dt-bindings: net: wiznet,w5100: add link status interrupt

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Removing the `interrupts` property from the top-level `properties` block without explicitly specifying `maxItems` in the conditional branches removes the upper bound on the array length, allowing invalid configurations to pass validation.
--

--- Patch [2]: [PATCH net-next v5 2/3] dt-bindings: net: wiznet,w5100: add link status interrupt ---
commit 27905addd6c59c9b61baecb1400ea1f5c8b0242f
Author: Arthur Crepin Leblond <[email protected]>

dt-bindings: net: wiznet,w5100: add link status interrupt

Add an optional link status interrupt which can be connected to the
LINKLED pin on the device to detect link status changes.

> diff --git a/Documentation/devicetree/bindings/net/wiznet,w5100.yaml b/Documentation/devicetree/bindings/net/wiznet,w5100.yaml
> index 217f301db182b..81f4382eb96c5 100644
> --- a/Documentation/devicetree/bindings/net/wiznet,w5100.yaml
> +++ b/Documentation/devicetree/bindings/net/wiznet,w5100.yaml
> @@ -15,6 +15,33 @@ description: |
>  allOf:
>    - $ref: /schemas/net/ethernet-controller.yaml#
>    - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - wiznet,w5500
> +    then:
> +      properties:
> +        interrupts:
> +          minItems: 1
> +          items:
> +            - description: Main interrupt (INT pin on the device).
> +            - description:
> +                Optional interrupt for link status changes (LINKLED pin on
> +                w5500).

[Severity: Medium]
Does this leave the array length unbounded for the wiznet,w5500 conditional
block?

Because the items keyword in standard JSON Schema only applies constraints to
the specified indices without capping the array size, it appears maxItems: 2
is missing here. 

This could allow device tree configurations with 3 or more extraneous
interrupts elements to silently pass schema validation.

> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - wiznet,w5100
> +              - wiznet,w5200
> +    then:
> +      properties:
> +        interrupts:
> +          items:
> +            - description: Main interrupt (INT pin on the device).

[Severity: Medium]
Does this branch for wiznet,w5100 and wiznet,w5200 also require an explicit
maxItems: 1 constraint?

Without an explicit cap, invalid device tree configurations specifying 2 or
more interrupts for these devices could similarly go undetected.

>  properties:
>    compatible:
> @@ -23,10 +50,6 @@ properties:
>        - wiznet,w5200
>        - wiznet,w5500
>  
> -  interrupts:
> -    items:
> -      - description: Main interrupt (INT pin on the device).
> -

[Severity: Medium]
Does removing the interrupts property from this top-level properties block
prevent dt-schema from automatically inferring and injecting a maxItems
constraint?

Leaving maxItems unspecified in the allOf conditional branches while removing
it from the top-level block removes the upper bound on the array length,
allowing invalid hardware descriptions to go undetected during dtbs_check.

[ ... ]

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