Re: [PATCH net-next v3 13/14] dt-bindings: net: add onsemi's S2500

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing `spi-max-frequency` in the device tree binding properties, required list, and example for an SPI peripheral.
--

commit ad5dbf88af08e186c5df6e55d01443c7fc4840ab
Author: Selvamani Rajagopal <Selvamani.Rajagopal@onsemi.com>

dt-bindings: net: add onsemi's S2500

Add YAML device tree binding for the onsemi S2500 IEEE 802.3cg
compliant Ethernet transceiver device.

> diff --git a/Documentation/devicetree/bindings/net/onnn,s2500.yaml b/Documentation/devicetree/bindings/net/onnn,s2500.yaml
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/onnn,s2500.yaml
[ ... ]
> +properties:
> +  compatible:
> +    const: onnn,s2500
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    description:
> +      Interrupt from MAC-PHY asserted in the event of Receive Chunks
> +      Available, Transmit Chunk Credits Available and Extended Status
> +      Event.
> +    maxItems: 1

[Severity: Medium]
Since this is an SPI peripheral, should spi-max-frequency be defined in the
properties section with device-specific maximum frequency constraints?

> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts

[Severity: Medium]
Would it be appropriate to add spi-max-frequency to this required list?
Omitting it allows incomplete device tree nodes to pass schema validation,
even though the SPI host controller needs it to configure the bus clock
speed.

> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    spi {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      ethernet@0 {
> +        compatible = "onnn,s2500";
> +        reg = <0>;
> +        pinctrl-names = "default";
> +        pinctrl-0 = <&eth0_pins>;
> +        interrupt-parent = <&gpio>;
> +        interrupts = <25 IRQ_TYPE_LEVEL_LOW>;

[Severity: Medium]
Is spi-max-frequency missing from this example? If developers copy this node
configuration, could it fail to configure the SPI host properly at runtime
without the max frequency property?

> +      };
> +    };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/CY8PR02MB92494D02A1209BC9BC72EAC883162@CY8PR02MB9249.namprd02.prod.outlook.com?part=13