Re: [PATCH net-next v7 01/10] dt-bindings: net: airoha: Add EN7581 ethernet-ports properties

[Lorenzo Bianconi <lorenzo@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3453 bytes --]

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] Unconditionally requiring `#address-cells` and `#size-cells` for `ethernet@3` and `ethernet@4` breaks backward compatibility and causes `dtc` warnings.
> - [Low] The `reg` property in the `airoha,eth-port` node incorrectly uses `maxItems: 5` instead of `maximum: 5`.
> --
> 
> commit f8c9c25c14697c9de91b47f1ea5850c1b33f44a9
> Author: Lorenzo Bianconi <lorenzo@kernel.org>
> 
> dt-bindings: net: airoha: Add EN7581 ethernet-ports properties
> 
> The EN7581 and AN7583 SoCs support connecting multiple external SerDes to GDM3
> or GDM4 ports via a hw arbiter that manages the traffic in a TDM manner. This
> commit introduces the ethernet-port property in order to model a given
> net_device connected via the external arbiter.
> 
> > diff --git a/Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml b/Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml
> > index fbe2ddcdd909c..7144d6da7394b 100644
> > --- a/Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml
> > +++ b/Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml
> > @@ -130,6 +130,46 @@ patternProperties:
> >          maximum: 4
> >          description: GMAC port identifier
> >  
> > +    allOf:
> > +      - if:
> > +          properties:
> > +            reg:
> > +              contains:
> > +                items:
> > +                  - enum:
> > +                      - 3
> > +                      - 4
> > +        then:
> > +          properties:
> > +            '#address-cells':
> > +              const: 1
> > +
> > +            '#size-cells':
> > +              const: 0
> > +
> > +          patternProperties:
> > +            "^ethernet@[0-5]$":
> > +              type: object
> > +              unevaluatedProperties: false
> > +              $ref: ethernet-controller.yaml#
> > +              description: External ethernet port ID available on the GDM port
> > +
> > +              properties:
> > +                compatible:
> > +                  const: airoha,eth-port
> > +
> > +                reg:
> > +                  maxItems: 5
> 
> Could this maxItems constraint allow the reg array to contain up to 5 elements
> (e.g., reg = <0 1 2 3 4>)? Since the parent node enforces #address-cells = <1>
> and #size-cells = <0>, shouldn't this use maximum: 5 instead to restrict the
> numeric value of a single port ID?

ack, I will fix it in v8.

> 
> > +                  description: External ethernet port identifier
> > +
> > +              required:
> > +                - reg
> > +                - compatible
> > +
> > +          required:
> > +            - "#address-cells"
> > +            - "#size-cells"
> 
> Will unconditionally requiring #address-cells and #size-cells for ethernet@3
> and ethernet@4 break configurations that don't use the external arbiter?
> 
> If a board connects the GDM3 or GDM4 port directly to a PHY without child nodes,
> it seems these properties would fail schema validation.
> 
> Also, if added merely to satisfy dtbs_check, wouldn't dtc emit an
> avoid_unnecessary_addr_size warning when the node has no children?

ack, I will fix it in v8.

Regards,
Lorenzo

> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260516-airoha-eth-multi-serdes-v7-0-99e0093303e2@kernel.org?part=1

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]