2022-03-03 15:29:59

by Sergiu Moga

[permalink] [raw]
Subject: [PATCH v2 4/5] dt-bindings: rtc: convert at91sam9 bindings to json-schema

Convert RTC binding for Atmel/Microchip SoCs to Device Tree Schema
format.

Signed-off-by: Sergiu Moga <[email protected]>
---
.../bindings/rtc/atmel,at91sam9-rtc.txt | 25 --------
.../bindings/rtc/atmel,at91sam9-rtc.yaml | 61 +++++++++++++++++++
2 files changed, 61 insertions(+), 25 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/rtc/atmel,at91sam9-rtc.txt
create mode 100644 Documentation/devicetree/bindings/rtc/atmel,at91sam9-rtc.yaml

diff --git a/Documentation/devicetree/bindings/rtc/atmel,at91sam9-rtc.txt b/Documentation/devicetree/bindings/rtc/atmel,at91sam9-rtc.txt
deleted file mode 100644
index 3f0e2a5950eb..000000000000
--- a/Documentation/devicetree/bindings/rtc/atmel,at91sam9-rtc.txt
+++ /dev/null
@@ -1,25 +0,0 @@
-Atmel AT91SAM9260 Real Time Timer
-
-Required properties:
-- compatible: should be one of the following:
- - "atmel,at91sam9260-rtt"
- - "microchip,sam9x60-rtt", "atmel,at91sam9260-rtt"
-- reg: should encode the memory region of the RTT controller
-- interrupts: rtt alarm/event interrupt
-- clocks: should contain the 32 KHz slow clk that will drive the RTT block.
-- atmel,rtt-rtc-time-reg: should encode the GPBR register used to store
- the time base when the RTT is used as an RTC.
- The first cell should point to the GPBR node and the second one
- encode the offset within the GPBR block (or in other words, the
- GPBR register used to store the time base).
-
-
-Example:
-
-rtt@fffffd20 {
- compatible = "atmel,at91sam9260-rtt";
- reg = <0xfffffd20 0x10>;
- interrupts = <1 4 7>;
- clocks = <&clk32k>;
- atmel,rtt-rtc-time-reg = <&gpbr 0x0>;
-};
diff --git a/Documentation/devicetree/bindings/rtc/atmel,at91sam9-rtc.yaml b/Documentation/devicetree/bindings/rtc/atmel,at91sam9-rtc.yaml
new file mode 100644
index 000000000000..5a639c0ec2c0
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/atmel,at91sam9-rtc.yaml
@@ -0,0 +1,61 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2022 Microchip Technology, Inc. and its subsidiaries
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rtc/atmel,at91sam9-rtc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Atmel AT91 RTT Device Tree Bindings
+
+allOf:
+ - $ref: "rtc.yaml#"
+
+maintainers:
+ - Alexandre Belloni <[email protected]>
+
+properties:
+ compatible:
+ oneOf:
+ - items:
+ - const: atmel,at91sam9260-rtt
+ - items:
+ - const: microchip,sam9x60-rtt
+ - const: atmel,at91sam9260-rtt
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ atmel,rtt-rtc-time-reg:
+ $ref: /schemas/types.yaml#/definitions/phandle-array
+ description: |
+ Should encode the GPBR register used to store the time base when the
+ RTT is used as an RTC. The first cell should point to the GPBR node
+ and the second one encodes the offset within the GPBR block (or in
+ other words, the GPBR register used to store the time base).
+
+ start-year: true
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - atmel,rtt-rtc-time-reg
+
+additionalProperties: false
+
+examples:
+ - |
+ rtc@fffffd20 {
+ compatible = "atmel,at91sam9260-rtt";
+ reg = <0xfffffd20 0x10>;
+ interrupts = <1 4 7>;
+ clocks = <&clk32k>;
+ atmel,rtt-rtc-time-reg = <&gpbr 0x0>;
+ };
--
2.25.1


2022-03-03 17:40:16

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 4/5] dt-bindings: rtc: convert at91sam9 bindings to json-schema

On 03/03/2022 15:06, Sergiu Moga wrote:
> Convert RTC binding for Atmel/Microchip SoCs to Device Tree Schema
> format.
>
> Signed-off-by: Sergiu Moga <[email protected]>
> ---
> .../bindings/rtc/atmel,at91sam9-rtc.txt | 25 --------
> .../bindings/rtc/atmel,at91sam9-rtc.yaml | 61 +++++++++++++++++++
> 2 files changed, 61 insertions(+), 25 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/rtc/atmel,at91sam9-rtc.txt
> create mode 100644 Documentation/devicetree/bindings/rtc/atmel,at91sam9-rtc.yaml
>
> diff --git a/Documentation/devicetree/bindings/rtc/atmel,at91sam9-rtc.txt b/Documentation/devicetree/bindings/rtc/atmel,at91sam9-rtc.txt
> deleted file mode 100644
> index 3f0e2a5950eb..000000000000
> --- a/Documentation/devicetree/bindings/rtc/atmel,at91sam9-rtc.txt
> +++ /dev/null
> @@ -1,25 +0,0 @@
> -Atmel AT91SAM9260 Real Time Timer
> -
> -Required properties:
> -- compatible: should be one of the following:
> - - "atmel,at91sam9260-rtt"
> - - "microchip,sam9x60-rtt", "atmel,at91sam9260-rtt"
> -- reg: should encode the memory region of the RTT controller
> -- interrupts: rtt alarm/event interrupt
> -- clocks: should contain the 32 KHz slow clk that will drive the RTT block.
> -- atmel,rtt-rtc-time-reg: should encode the GPBR register used to store
> - the time base when the RTT is used as an RTC.
> - The first cell should point to the GPBR node and the second one
> - encode the offset within the GPBR block (or in other words, the
> - GPBR register used to store the time base).
> -
> -
> -Example:
> -
> -rtt@fffffd20 {
> - compatible = "atmel,at91sam9260-rtt";
> - reg = <0xfffffd20 0x10>;
> - interrupts = <1 4 7>;
> - clocks = <&clk32k>;
> - atmel,rtt-rtc-time-reg = <&gpbr 0x0>;
> -};
> diff --git a/Documentation/devicetree/bindings/rtc/atmel,at91sam9-rtc.yaml b/Documentation/devicetree/bindings/rtc/atmel,at91sam9-rtc.yaml
> new file mode 100644
> index 000000000000..5a639c0ec2c0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rtc/atmel,at91sam9-rtc.yaml
> @@ -0,0 +1,61 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (C) 2022 Microchip Technology, Inc. and its subsidiaries
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/rtc/atmel,at91sam9-rtc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Atmel AT91 RTT Device Tree Bindings
> +
> +allOf:
> + - $ref: "rtc.yaml#"
> +
> +maintainers:
> + - Alexandre Belloni <[email protected]>
> +
> +properties:
> + compatible:
> + oneOf:
> + - items:
> + - const: atmel,at91sam9260-rtt
> + - items:
> + - const: microchip,sam9x60-rtt
> + - const: atmel,at91sam9260-rtt
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + clocks:
> + maxItems: 1
> +
> + atmel,rtt-rtc-time-reg:
> + $ref: /schemas/types.yaml#/definitions/phandle-array
> + description: |
> + Should encode the GPBR register used to store the time base when the
> + RTT is used as an RTC. The first cell should point to the GPBR node
> + and the second one encodes the offset within the GPBR block (or in
> + other words, the GPBR register used to store the time base).

Instead of describing cells here, you need items with description. I
gave you the example last time, so instead of ignoring it, please
implement it.

> +
> + start-year: true
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - clocks
> + - atmel,rtt-rtc-time-reg
> +
> +additionalProperties: false

This will disallow any other properties mentioned in rtc.yaml, e.g.
popular wakeup-source. Is it really intended? If core schema is
extended, the driver would need to be updated to support new features.
Any reason to choose such approach? The other way is to remove
start-year and have here unevaluatedProperties.

> +
> +examples:
> + - |
> + rtc@fffffd20 {
> + compatible = "atmel,at91sam9260-rtt";
> + reg = <0xfffffd20 0x10>;
> + interrupts = <1 4 7>;

At least one number above looks like known macro, so use it.

> + clocks = <&clk32k>;
> + atmel,rtt-rtc-time-reg = <&gpbr 0x0>;
> + };


Best regards,
Krzysztof

2022-03-04 14:18:19

by Sergiu Moga

[permalink] [raw]
Subject: Re: [PATCH v2 4/5] dt-bindings: rtc: convert at91sam9 bindings to json-schema

On 03.03.2022 16:21, Krzysztof Kozlowski wrote:
> On 03/03/2022 15:06, Sergiu Moga wrote:
>> Convert RTC binding for Atmel/Microchip SoCs to Device Tree Schema
>> format.
>>
>> Signed-off-by: Sergiu Moga <[email protected]>
>> ---
>> .../bindings/rtc/atmel,at91sam9-rtc.txt | 25 --------
>> .../bindings/rtc/atmel,at91sam9-rtc.yaml | 61 +++++++++++++++++++
>> 2 files changed, 61 insertions(+), 25 deletions(-)
>> delete mode 100644 Documentation/devicetree/bindings/rtc/atmel,at91sam9-rtc.txt
>> create mode 100644 Documentation/devicetree/bindings/rtc/atmel,at91sam9-rtc.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/rtc/atmel,at91sam9-rtc.txt b/Documentation/devicetree/bindings/rtc/atmel,at91sam9-rtc.txt
>> deleted file mode 100644
>> index 3f0e2a5950eb..000000000000
>> --- a/Documentation/devicetree/bindings/rtc/atmel,at91sam9-rtc.txt
>> +++ /dev/null
>> @@ -1,25 +0,0 @@
>> -Atmel AT91SAM9260 Real Time Timer
>> -
>> -Required properties:
>> -- compatible: should be one of the following:
>> - - "atmel,at91sam9260-rtt"
>> - - "microchip,sam9x60-rtt", "atmel,at91sam9260-rtt"
>> -- reg: should encode the memory region of the RTT controller
>> -- interrupts: rtt alarm/event interrupt
>> -- clocks: should contain the 32 KHz slow clk that will drive the RTT block.
>> -- atmel,rtt-rtc-time-reg: should encode the GPBR register used to store
>> - the time base when the RTT is used as an RTC.
>> - The first cell should point to the GPBR node and the second one
>> - encode the offset within the GPBR block (or in other words, the
>> - GPBR register used to store the time base).
>> -
>> -
>> -Example:
>> -
>> -rtt@fffffd20 {
>> - compatible = "atmel,at91sam9260-rtt";
>> - reg = <0xfffffd20 0x10>;
>> - interrupts = <1 4 7>;
>> - clocks = <&clk32k>;
>> - atmel,rtt-rtc-time-reg = <&gpbr 0x0>;
>> -};
>> diff --git a/Documentation/devicetree/bindings/rtc/atmel,at91sam9-rtc.yaml b/Documentation/devicetree/bindings/rtc/atmel,at91sam9-rtc.yaml
>> new file mode 100644
>> index 000000000000..5a639c0ec2c0
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/rtc/atmel,at91sam9-rtc.yaml
>> @@ -0,0 +1,61 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +# Copyright (C) 2022 Microchip Technology, Inc. and its subsidiaries
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/rtc/atmel,at91sam9-rtc.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Atmel AT91 RTT Device Tree Bindings
>> +
>> +allOf:
>> + - $ref: "rtc.yaml#"
>> +
>> +maintainers:
>> + - Alexandre Belloni <[email protected]>
>> +
>> +properties:
>> + compatible:
>> + oneOf:
>> + - items:
>> + - const: atmel,at91sam9260-rtt
>> + - items:
>> + - const: microchip,sam9x60-rtt
>> + - const: atmel,at91sam9260-rtt
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + interrupts:
>> + maxItems: 1
>> +
>> + clocks:
>> + maxItems: 1
>> +
>> + atmel,rtt-rtc-time-reg:
>> + $ref: /schemas/types.yaml#/definitions/phandle-array
>> + description: |
>> + Should encode the GPBR register used to store the time base when the
>> + RTT is used as an RTC. The first cell should point to the GPBR node
>> + and the second one encodes the offset within the GPBR block (or in
>> + other words, the GPBR register used to store the time base).
> Instead of describing cells here, you need items with description. I
> gave you the example last time, so instead of ignoring it, please
> implement it.
>
Noted.

>> +
>> + start-year: true
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - interrupts
>> + - clocks
>> + - atmel,rtt-rtc-time-reg
>> +
>> +additionalProperties: false
> This will disallow any other properties mentioned in rtc.yaml, e.g.
> popular wakeup-source. Is it really intended? If core schema is
> extended, the driver would need to be updated to support new features.
> Any reason to choose such approach? The other way is to remove
> start-year and have here unevaluatedProperties.
>
Ah, I see. Removing start-year and using "unevaluatedProperties: false"
here is better indeed.

>> +
>> +examples:
>> + - |
>> + rtc@fffffd20 {
>> + compatible = "atmel,at91sam9260-rtt";
>> + reg = <0xfffffd20 0x10>;
>> + interrupts = <1 4 7>;
> At least one number above looks like known macro, so use it.
>
Noted. Will add another patch to the series for this.

>> + clocks = <&clk32k>;
>> + atmel,rtt-rtc-time-reg = <&gpbr 0x0>;
>> + };
>
> Best regards,
> Krzysztof

Thank you for the feedback.

Sergiu