| Message ID | 20260902233550.947218-1-giveback4fun@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | dt-bindings: maxim-ds1302: Convert to dt schema | expand |
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>;
On Wed, Sep 02, 2026 at 04:35:49PM -0700, Sudheendra Sampath wrote: > Convert Maxim DS1302 real-time clock binding to DT schema. > > Signed-off-by: Sudheendra Sampath <giveback4fun@gmail.com> > --- > .../devicetree/bindings/rtc/maxim-ds1302.txt | 46 ----------- > .../devicetree/bindings/rtc/maxim-ds1302.yaml | 81 +++++++++++++++++++ > 2 files changed, 81 insertions(+), 46 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/rtc/maxim-ds1302.txt > create mode 100644 Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml > > diff --git a/Documentation/devicetree/bindings/rtc/maxim-ds1302.txt b/Documentation/devicetree/bindings/rtc/maxim-ds1302.txt > deleted file mode 100644 > index ba470c56cdec..000000000000 > --- a/Documentation/devicetree/bindings/rtc/maxim-ds1302.txt > +++ /dev/null > @@ -1,46 +0,0 @@ > -* Maxim/Dallas Semiconductor DS-1302 RTC > - > -Simple device which could be used to store date/time between reboots. > - > -The device uses the standard MicroWire half-duplex transfer timing. > -Master output is set on low clock and sensed by the RTC on the rising > -edge. Master input is set by the RTC on the trailing edge and is sensed > -by the master on low clock. > - > -Required properties: > - > -- compatible : Should be "maxim,ds1302" > - > -Required SPI properties: > - > -- reg : Should be address of the device chip select within > - the controller. > - > -- spi-max-frequency : DS-1302 has 500 kHz if powered at 2.2V, > - and 2MHz if powered at 5V. > - > -- spi-3wire : The device has a shared signal IN/OUT line. > - > -- spi-lsb-first : DS-1302 requires least significant bit first > - transfers. > - > -- spi-cs-high: DS-1302 has active high chip select line. This is > - required unless inverted in hardware. > - > -Example: > - > -spi@901c { > - #address-cells = <1>; > - #size-cells = <0>; > - compatible = "icpdas,lp8841-spi-rtc"; > - reg = <0x901c 0x1>; > - > - rtc@0 { > - compatible = "maxim,ds1302"; > - reg = <0>; > - spi-max-frequency = <500000>; > - spi-3wire; > - spi-lsb-first; > - spi-cs-high; > - }; > -}; > diff --git a/Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml b/Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml > new file mode 100644 > index 000000000000..3e6264464e10 > --- /dev/null > +++ b/Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml s/-/,/ as the filename should match the compatible. > @@ -0,0 +1,81 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/rtc/maxim-ds1302.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Maxim/Dallas Semiconductor DS-1302 RTC > + > +maintainers: > + - Alexandre Belloni <alexandre.belloni@bootlin.com> This should be the original author of the text binding, provided they're still responsive. > + > +description: | > + Simple device which could be used to store date/time between reboots. > + > + The device uses the standard MicroWire half-duplex transfer timing. > + Master output is set on low clock and sensed by the RTC on the rising > + edge. Master input is set by the RTC on the trailing edge and is sensed > + by the master on low clock. > + > +properties: > + compatible: > + items: > + # Maxim DS1302 Real-time Clock This comment is obvious, just drop it. > + - const: maxim,ds1302 > + > + reg: > + maxItems: 1 > + > + spi-max-frequency: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: Maximum SPI clocking speed in Hz. This should retain the comment about the voltages, rather than state the obvious. > + enum: > + - 500000 > + - 2000000 > + > + spi-3wire: true > + > + spi-lsb-first: true > + > + spi-cs-high: true > + > +required: > + - compatible > + - reg > + - spi-max-frequency > + - spi-3wire > + - spi-lsb-first > + - spi-cs-high > + > +additionalProperties: false > + > +examples: > + - | > + spi1 { > + #address-cells = <1>; > + #size-cells = <0>; > + > + rtc@0 { > + compatible = "maxim,ds1302"; > + reg = <0>; > + spi-max-frequency = <500000>; > + spi-3wire; > + spi-lsb-first; > + spi-cs-high; > + }; > + }; > + > + - | > + spi2 { > + #address-cells = <1>; > + #size-cells = <0>; > + > + rtc@2000 { > + compatible = "maxim,ds1302"; > + reg = <0x2000>; > + spi-max-frequency = <2000000>; > + spi-3wire; > + spi-lsb-first; > + spi-cs-high; > + }; > + }; Where did this additional example come from? The chip select looks rather unlikely and the example has nothing meaningfully different in it. I think this should be dropped. pw-bot: changes-requested Thanks, Conor.
diff --git a/Documentation/devicetree/bindings/rtc/maxim-ds1302.txt b/Documentation/devicetree/bindings/rtc/maxim-ds1302.txt deleted file mode 100644 index ba470c56cdec..000000000000 --- a/Documentation/devicetree/bindings/rtc/maxim-ds1302.txt +++ /dev/null @@ -1,46 +0,0 @@ -* Maxim/Dallas Semiconductor DS-1302 RTC - -Simple device which could be used to store date/time between reboots. - -The device uses the standard MicroWire half-duplex transfer timing. -Master output is set on low clock and sensed by the RTC on the rising -edge. Master input is set by the RTC on the trailing edge and is sensed -by the master on low clock. - -Required properties: - -- compatible : Should be "maxim,ds1302" - -Required SPI properties: - -- reg : Should be address of the device chip select within - the controller. - -- spi-max-frequency : DS-1302 has 500 kHz if powered at 2.2V, - and 2MHz if powered at 5V. - -- spi-3wire : The device has a shared signal IN/OUT line. - -- spi-lsb-first : DS-1302 requires least significant bit first - transfers. - -- spi-cs-high: DS-1302 has active high chip select line. This is - required unless inverted in hardware. - -Example: - -spi@901c { - #address-cells = <1>; - #size-cells = <0>; - compatible = "icpdas,lp8841-spi-rtc"; - reg = <0x901c 0x1>; - - rtc@0 { - compatible = "maxim,ds1302"; - reg = <0>; - spi-max-frequency = <500000>; - spi-3wire; - spi-lsb-first; - spi-cs-high; - }; -}; diff --git a/Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml b/Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml new file mode 100644 index 000000000000..3e6264464e10 --- /dev/null +++ b/Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml @@ -0,0 +1,81 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/rtc/maxim-ds1302.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Maxim/Dallas Semiconductor DS-1302 RTC + +maintainers: + - Alexandre Belloni <alexandre.belloni@bootlin.com> + +description: | + Simple device which could be used to store date/time between reboots. + + The device uses the standard MicroWire half-duplex transfer timing. + Master output is set on low clock and sensed by the RTC on the rising + edge. Master input is set by the RTC on the trailing edge and is sensed + by the master on low clock. + +properties: + compatible: + items: + # Maxim DS1302 Real-time Clock + - const: maxim,ds1302 + + reg: + maxItems: 1 + + spi-max-frequency: + $ref: /schemas/types.yaml#/definitions/uint32 + description: Maximum SPI clocking speed in Hz. + enum: + - 500000 + - 2000000 + + spi-3wire: true + + spi-lsb-first: true + + spi-cs-high: true + +required: + - compatible + - reg + - spi-max-frequency + - spi-3wire + - spi-lsb-first + - spi-cs-high + +additionalProperties: false + +examples: + - | + spi1 { + #address-cells = <1>; + #size-cells = <0>; + + rtc@0 { + compatible = "maxim,ds1302"; + reg = <0>; + spi-max-frequency = <500000>; + spi-3wire; + spi-lsb-first; + spi-cs-high; + }; + }; + + - | + spi2 { + #address-cells = <1>; + #size-cells = <0>; + + rtc@2000 { + compatible = "maxim,ds1302"; + reg = <0x2000>; + spi-max-frequency = <2000000>; + spi-3wire; + spi-lsb-first; + spi-cs-high; + }; + };
Convert Maxim DS1302 real-time clock binding to DT schema. Signed-off-by: Sudheendra Sampath <giveback4fun@gmail.com> --- .../devicetree/bindings/rtc/maxim-ds1302.txt | 46 ----------- .../devicetree/bindings/rtc/maxim-ds1302.yaml | 81 +++++++++++++++++++ 2 files changed, 81 insertions(+), 46 deletions(-) delete mode 100644 Documentation/devicetree/bindings/rtc/maxim-ds1302.txt create mode 100644 Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml