2022-03-04 17:38:18

by Krzysztof Kozlowski

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

On 04/03/2022 15:27, 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 | 63 +++++++++++++++++++
> 2 files changed, 63 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..af25cc275356
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rtc/atmel,at91sam9-rtc.yaml
> @@ -0,0 +1,63 @@
> +# 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
> + items:
> + - items:
> + - description: Phandle to the GPBR node.
> + - description: Offset within the GPBR block.
> + 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).

Don't duplicate in description what is already before, so entire part
from "The first cell" is not needed. I mentioned it last time - Instead
of describing cells here, use items. You have items, good, but you still
have description.

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

Could you start acknowledging comments instead of ignoring them?

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


Best regards,
Krzysztof