Re: [PATCH net-next 1/2] dt-bindings: net: econet: add EN751221 ethernet

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] The compatible string 'econet,en7528-eth' is introduced in the schema without any documentation or mention in the commit message.
- [Low] The binding mandates xPON resets without explaining their relation to the Ethernet controller.
- [Low] Child nodes matching `^mac@[0-1]$` reference `ethernet-controller.yaml`, violating the strict `$nodename` constraint of the referenced schema.
--

commit 9471f88e7238b271d05f09311ad8d33d5d3b67fa
Author: Caleb James DeLisle <cjd@cjdns.fr>

dt-bindings: net: econet: add EN751221 ethernet

This commit adds device tree binding documentation for the EN751221 ethernet 
system. The controller features two QDMA modules, two GDM modules, and a packet 
processing engine to manage CPU communication and QoS prioritized forwarding.

> diff --git a/Documentation/devicetree/bindings/net/econet,en751221-eth.yaml b/Documentation/devicetree/bindings/net/econet,en751221-eth.yaml
> new file mode 100644
> index 0000000000000..5d953eaa16e52
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/econet,en751221-eth.yaml
> @@ -0,0 +1,127 @@
[ ... ]
> +properties:
> +  compatible:
> +    enum:
> +      - econet,en751221-eth
> +      - econet,en7528-eth

[Severity: Medium]
Is this compatible string meant to be included in this patch? The commit
message only mentions EN751221, but this adds bindings for a second SoC
variant (EN7528).

Since this variant has different hardware behavior (such as
dscp_byte_swap = false compared to true for EN751221), should its inclusion
be documented in the commit message?

[ ... ]
> +  reset-names:
> +    items:
> +      - const: fe
> +      - const: qdma0
> +      - const: qdma1
> +      - const: xpon-mac
> +      - const: xpon-phy

[Severity: Low]
Could the commit message provide some context on why the Ethernet Frame Engine
mandates xPON resets? While the hardware might require acquiring them, it is
unclear from the documentation why an Ethernet controller depends on xPON MAC
and PHY resets.

[ ... ]
> +patternProperties:
> +  "^mac@[0-1]$":
> +    type: object
> +    unevaluatedProperties: false
> +    $ref: ethernet-controller.yaml#

[Severity: Low]
Will this pattern result in a schema validation failure? The referenced
ethernet-controller.yaml schema explicitly restricts node names using the
pattern "^ethernet(@.*)?$".

By matching child nodes against "^mac@[0-1]$", any valid node for this binding
will fail the strict node naming constraints imposed by the referenced
ethernet schema.

[ ... ]
> +        gmac0: mac@0 {
> +          compatible = "econet,eth-mac";

[Severity: Low]
For example, will this "mac@0" example node trigger a dt_binding_check
failure because it does not begin with "ethernet@"?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909212146.3184077-1-cjd@cjdns.fr?part=1