Re: [PATCH] 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] The `spi-max-frequency` property is improperly constrained using an `enum` instead of a `maximum` bound.
- [Medium] Missing `$ref` inclusions for common RTC and SPI properties, combined with the restrictive use of `additionalProperties: false`.
- [Medium] The `spi-cs-high` property is improperly set as unconditionally required, preventing validation of valid hardware inversions.
- [Low] The examples use non-standard SPI controller node names (`spi1` and `spi2`), which bypasses core SPI validation.
--

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

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

This commit converts the Maxim DS1302 real-time clock binding from a plain text
document to the YAML DT schema format.

> diff --git a/Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml b/Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml
> new file mode 100644
> index 0000000000000..3e6264464e109
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml
[ ... ]
> +  spi-max-frequency:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Maximum SPI clocking speed in Hz.
> +    enum:
> +      - 500000
> +      - 2000000

[Severity: Medium]
Does this enum restrict the property too tightly? The spi-max-frequency
property typically specifies the maximum limit the controller should use, not a
discrete set of allowed frequencies. 

Device trees specifying valid intermediate or lower maximum SPI frequencies
(e.g., 1000000 Hz or 200000 Hz) will fail schema validation. Should this use a
maximum bound instead?

> +
> +  spi-3wire: true
> +
> +  spi-lsb-first: true
> +
> +  spi-cs-high: true

[Severity: Medium]
Does defining these standard SPI properties as just true bypass their robust
type validation from the central SPI schema? This schema appears to be missing
$ref inclusions for common RTC and SPI properties (like
spi-peripheral-props.yaml# and rtc.yaml#).

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

[Severity: Medium]
Does making spi-cs-high unconditionally required prevent validation of boards
with inverted chip select hardware? 

The original text binding explicitly stated this property is required "unless
inverted in hardware". Forcing it here would cause valid configurations with
active-low chip selects to fail validation.

> +
> +additionalProperties: false

[Severity: Medium]
Does using additionalProperties: false without the required subsystem $refs
inappropriately reject valid common properties? 

Standard SPI and RTC properties (such as wakeup-source, spi-cpol, or spi-cpha)
will be strictly forbidden, causing valid real-world device trees using these
properties to fail validation. Should this be unevaluatedProperties: false
alongside the missing $refs?

> +
> +examples:
> +  - |
> +    spi1 {

[Severity: Low]
Does this non-standard node name (spi1) bypass core SPI validation? 

The core SPI controller schema (spi-controller.yaml) matches node names using
the pattern ^spi(@.*|-([0-9]|[1-9][0-9]+))?$. Using spi1 skips core SPI
validation for the example nodes, potentially masking invalid SPI property
usage in the example.

> +        #address-cells = <1>;
[ ... ]
> +  - |
> +    spi2 {

[Severity: Low]
Does this node name also bypass core SPI validation? Similarly to the first
example, spi2 will not match the SPI controller schema node name pattern.

> +        #address-cells = <1>;

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