Re: [PATCH v2] dt-bindings: rtc: epson,rx6110: Convert to DT Schema

Re: [PATCH v2] dt-bindings: rtc: epson,rx6110: Convert to DT Schema

[Conor Dooley]

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

On Sat, May 09, 2026 at 10:27:02AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] The YAML binding omits a reference to the core RTC subsystem schema (`rtc.yaml`).
> - [Medium] The strict requirement for SPI-specific properties was lost during the schema conversion.
> --
> 
> commit 83ad9493a42cbad1f11db8ac3498c9be50a357d1
> Author: Udaya Kiran Challa <challauday369@gmail.com>
> 
> dt-bindings: rtc: epson,rx6110: Convert to DT Schema
> 
> This commit converts the Epson RX6110 Real Time Clock devicetree binding
> from the legacy text format to DT schema.
> 
> > diff --git a/Documentation/devicetree/bindings/rtc/epson,rx6110.yaml b/Documentation/devicetree/bindings/rtc/epson,rx6110.yaml
> > new file mode 100644
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/rtc/epson,rx6110.yaml
> [ ... ]
> > +allOf:
> > +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> 
> Should this also include a reference to rtc.yaml# in the allOf block?
> Without it, since unevaluatedProperties is false, won't valid standard RTC
> properties like start-year or aux-voltage-chargeable cause validation
> errors? This omission appears to introduce a functional regression by
> bypassing the standard nodename checks provided by the core schema.
> 
> [ ... ]
> > +  spi-cpha: true
> > +  spi-cpol: true
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +unevaluatedProperties: false
> 
> The legacy text binding listed spi-cs-high, spi-cpha, and spi-cpol as
> mandatory required properties when the device operates in SPI mode.

+ the fact that cpha/cpol are listed with : true, and spi-cs-high isn't
which is confusing IMO.

> Because they are omitted from the required list to support I2C mode, won't
> misconfigured SPI nodes missing these hardware configuration flags now
> silently pass validation? Would adding a conditional allOf/if block be
> appropriate to enforce these properties for SPI configurations and avoid
> this regression?

Don't do this, since it requires knowing what the parent node is.
If anything, make it something like
if: oneOf: <> then: allOf: <>

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