Re: [PATCH v2 - dt-bindings: maxim-ds1302: Convert to dt schema v2] dt-bindings: maxim-ds1302: Convert to dt schema

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] spi-max-frequency is incorrectly restricted to an enum of exact values, preventing valid lower frequencies.
- [Medium] spi-cs-high is unconditionally required, breaking setups with inverted hardware CS.
- [Medium] The binding defines an SPI peripheral and RTC but fails to reference standard core schemas.
- [Low] The YAML conversion silently removes the device tree example section.
--

commit 4b0cc4e2d5ae6befdb10a7457cde3f6698ee83c7
Author: Sudheendra Sampath <giveback4fun@gmail.com>

dt-bindings: maxim-ds1302: Convert to dt schema

This commit converts the Maxim DS1302 real-time clock text binding to the DT
schema format based on code review comments.

> diff --git a/Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml b/Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml
> new file mode 100644
> index 0000000000000..54a2fcf7f8e7b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml
> @@ -0,0 +1,50 @@
[ ... ]
> +  spi-max-frequency:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: DS-1302 has 500 kHz if powered at 2.2V, and 2MHz if powered at 5V
> +    enum:
> +      - 500000
> +      - 2000000

[Severity: Medium]
Does this restrict spi-max-frequency too strictly? Schema validation might
fail for perfectly valid device trees that specify frequencies other than
500kHz or 2MHz (e.g., 250000) to ensure signal integrity.

Should this use a constraint like maximum: 2000000 rather than an exact enum?

[ ... ]
> +required:
> +  - compatible
> +  - reg
> +  - spi-max-frequency
> +  - spi-3wire
> +  - spi-lsb-first
> +  - spi-cs-high

[Severity: Medium]
Will this unconditionally require spi-cs-high and break backwards
compatibility for existing setups?

The original text binding noted that this property is required unless inverted
in hardware. Forcing it globally in the required list might reject legitimate
hardware configurations where the chip select is hardware-inverted.

> +
> +additionalProperties: false

[Severity: Medium]
Does additionalProperties: false incorrectly reject standard SPI peripheral
properties here?

If a device tree uses properties like spi-cs-setup-delay-ns or
spi-rx-delay-us, dt_binding_check will throw an error. Should the schema
include references to /schemas/spi/spi-peripheral-props.yaml# and rtc.yaml#,
and then use unevaluatedProperties: false instead to properly inherit standard
capabilities?

[Severity: Low]
Is the examples block missing from the end of the schema file?

The original text file included an example demonstrating node placement.
Without it, dt_binding_check cannot validate the schema against a concrete
instance, and the usage documentation is lost.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908190137.1168904-1-giveback4fun@gmail.com?part=1