dt-bindings: maxim-ds1302: Convert to dt schema
Subsystems (2)
PATCH 1/1 dt-bindings: maxim-ds1302: Convert to dt schema
Convert Maxim DS1302 real-time clock binding to DT schema.
--- .../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
Documentation/devicetree/bindings/rtc/maxim-ds1302.txt deleted +0 -46
@@ -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; - }; -};
Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml new file +81 -0
81 lines not on this page · raw message
Signature · 1 line
2.47.3
[email protected]
AI reviewThank 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…
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 <[email protected]>
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.
Quoted 5 lines
Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml new file +0
-0
[ ... ]
Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml line 34
Quoted 6 lines
Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml new file +6
-0+ 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?
Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml line 40
Quoted 6 lines
Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml new file +6
-0+ + 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#).
[ ... ]
Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml line 48
Quoted 7 lines
Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml new file +7
-0+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.
Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml
Quoted 2 lines
Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml new file +2
-0+ +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?
Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml line 54
Quoted 4 lines
Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml new file +4
-0+ +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.
Quoted 1 line
+ #address-cells = <1>;
[ ... ]
Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml
Quoted 2 lines
Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml new file +2
-0+ - | + 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.
Quoted 1 line
+ #address-cells = <1>;
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml
Quoted 67 lines · Sudheendra Sampath
Convert Maxim DS1302 real-time clock binding to DT schema.
Signed-off-by Sudheendra Sampath <[email protected]>--- .../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.yamlDocumentation/devicetree/bindings/rtc/maxim-ds1302.txt deleted +0
-46@@ -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; - }; -};Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml new file +0
-0
s/-/,/ as the filename should match the compatible.
Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml line 10
Quoted 11 lines
Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml new file +10
-0@@ -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 <[email protected]>
This should be the original author of the text binding, provided they're still responsive.
Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml line 23
Quoted 13 lines
Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml new file +13
-0+ +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.
Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml line 31
Quoted 8 lines
Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml new file +8
-0+ - 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.
Quoted 41 lines
Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml new file +40
-0+ 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>;
Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml line 81
Quoted 10 lines
Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml new file +10
-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.