dt-bindings: maxim-ds1302: Convert to dt schema
Subsystems (2)
PATCH v2 1/1 dt-bindings: maxim-ds1302: Convert to dt schema
Convert Maxim DS1302 real-time clock binding to DT schema. Changes based on code review comments.
--- .../devicetree/bindings/rtc/maxim-ds1302.txt | 46 ----------------- .../devicetree/bindings/rtc/maxim-ds1302.yaml | 50 +++++++++++++++++++ 2 files changed, 50 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 +50 -0
50 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] spi-max-frequency is incorrectly restricted to an enum of exact values, preventing valid lower…
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 <[email protected]>
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.
Quoted 6 lines
Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml new file +0
-0@@ -0,0 +1,50 @@
[ ... ]
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: 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?
[ ... ]
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]
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.
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 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.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
Quoted 5 lines · Sudheendra Sampath
Convert Maxim DS1302 real-time clock binding to DT schema. Changes based on code review comments.
Signed-off-by Sudheendra Sampath <[email protected]>---
Your subject PATCH prefix is odd. Please use git format-patch -v2 or b4.
Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml
Quoted 63 lines
.../devicetree/bindings/rtc/maxim-ds1302.txt | 46 ----------------- .../devicetree/bindings/rtc/maxim-ds1302.yaml | 50 +++++++++++++++++++ 2 files changed, 50 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
Filename should match compatible.
Quoted 14 lines
Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml new file +13
-0@@ -0,0 +1,50 @@ +# 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]> + - Sergey Yanovich <[email protected]> + +description: |
Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml line 23
Quoted 10 lines
Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml new file +10
-0+ 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:
No need for items.
Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml line 30
Quoted 7 lines
Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml new file +7
-0+ - const: maxim,ds1302 + + reg: + maxItems: 1 + + spi-max-frequency: + $ref: /schemas/types.yaml#/definitions/uint32
Why do you need the type? Please look at other bindings.
Quoted 10 lines
Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml new file +10
-0+ description: DS-1302 has 500 kHz if powered at 2.2V, and 2MHz if powered at 5V + enum: + - 500000 + - 2000000 + + spi-3wire: true + + spi-lsb-first: true + + spi-cs-high: true
Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml line 50
Quoted 10 lines
Documentation/devicetree/bindings/rtc/maxim-ds1302.yaml new file +10
-0+ +required: + - compatible + - reg + - spi-max-frequency + - spi-3wire + - spi-lsb-first + - spi-cs-high + +additionalProperties: false
Missing example. Please follow standard rules, expressed in writing bindings and writing schema docs, DTS101 slides or just look how other people do it. Otherwise explain why doing this differently?
Best regards,
Krzysztof
Quoted 5 lines · Sudheendra Sampath
Convert Maxim DS1302 real-time clock binding to DT schema. Changes based on code review comments.
Signed-off-by Sudheendra Sampath <[email protected]>---
Where is changelog?
And more importantly, how did you figure out the address list without using get_maintainers.pl script?
Please use scripts/get_maintainers.pl to get a list of necessary people and lists to CC (and consider --no-git-fallback argument, so you will not CC people just because they made one commit years ago). It might happen, that command when run on an older kernel, gives you outdated entries. Therefore please be sure you base your patches on recent Linux kernel.
Tools like b4 or scripts/get_maintainer.pl provide you proper list of people, so fix your workflow. Tools might also fail if you work on some ancient tree (don't, instead use mainline) or work on fork of kernel (don't, instead use mainline). Just use b4 and everything should be fine, although remember about 'b4 prep --auto-to-cc' if you added new patches to the patchset.
Best regards,
Krzysztof