[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c1a98a3e-609e-7783-b1b7-3bb39caa8c65@linaro.org>
Date: Thu, 18 Aug 2022 11:38:15 +0300
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Sergiu Moga <sergiu.moga@...rochip.com>, lee@...nel.org,
robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org,
nicolas.ferre@...rochip.com, alexandre.belloni@...tlin.com,
claudiu.beznea@...rochip.com, radu_nicolae.pirea@....ro,
richard.genoud@...il.com, mturquette@...libre.com,
sboyd@...nel.org, gregkh@...uxfoundation.org, jirislaby@...nel.org,
admin@...iphile.com, kavyasree.kotagiri@...rochip.com
Cc: devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org, linux-spi@...r.kernel.org,
linux-clk@...r.kernel.org, linux-serial@...r.kernel.org
Subject: Re: [PATCH 2/5] dt-bindings: mfd: atmel,at91-usart: convert to
json-schema
On 17/08/2022 10:55, Sergiu Moga wrote:
> Convert at91 USART DT Binding for Atmel/Microchip SoCs to
> json-schema format.
>
> Signed-off-by: Sergiu Moga <sergiu.moga@...rochip.com>
> ---
> .../bindings/mfd/atmel,at91-usart.yaml | 190 ++++++++++++++++++
> .../devicetree/bindings/mfd/atmel-usart.txt | 98 ---------
> 2 files changed, 190 insertions(+), 98 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/mfd/atmel,at91-usart.yaml
> delete mode 100644 Documentation/devicetree/bindings/mfd/atmel-usart.txt
>
> diff --git a/Documentation/devicetree/bindings/mfd/atmel,at91-usart.yaml b/Documentation/devicetree/bindings/mfd/atmel,at91-usart.yaml
> new file mode 100644
> index 000000000000..cf15d73fa1e8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/atmel,at91-usart.yaml
> @@ -0,0 +1,190 @@
> +# 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/mfd/atmel,at91-usart.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Atmel Universal Synchronous Asynchronous Receiver/Transmitter (USART)
> +
> +maintainers:
> + - Richard Genoud <richard.genoud@...il.com>
> +
> +properties:
> + compatible:
> + oneOf:
This looks quite different than bindings and you commit msg is saying it
is only a conversion. Mention any changes against original bindings.
> + - const: atmel,at91rm9200-usart
> + - const: atmel,at91sam9260-usart
> + - const: microchip,sam9x60-usart
That's an enum
> + - items:
> + - const: atmel,at91rm9200-dbgu
> + - const: atmel,at91rm9200-usart
> + - items:
> + - const: atmel,at91sam9260-dbgu
> + - const: atmel,at91sam9260-usart
> + - items:
> + - const: microchip,sam9x60-dbgu
> + - const: microchip,sam9x60-usart
> + - items:
> + - const: microchip,sam9x60-usart
> + - const: atmel,at91sam9260-usart
This is not correct - contradicts earlier one.
> + - items:
> + - const: microchip,sam9x60-dbgu
> + - const: microchip,sam9x60-usart
> + - const: atmel,at91sam9260-dbgu
> + - const: atmel,at91sam9260-usart
What? You wrote above that microchip,sam9x60-dbgu is compatible only
with microchip,sam9x60-usart. Now you write it is also compatible with
other ones?
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + clock-names:
> + contains:
> + const: usart
No, this has to be specific/fixed list.
> +
> + clocks:
> + minItems: 1
> + maxItems: 2
Not really - define the items. One item could be optional, though.
> +
> + dmas:
> + items:
> + - description: TX DMA Channel
> + - description: RX DMA Channel
> +
> + dma-names:
> + items:
> + - const: tx
> + - const: rx
> +
> + atmel,usart-mode:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: |
No need for |
> + Must be either 1 for SPI or 0 for USART.
Mention the header.
> + enum: [ 0, 1 ]
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - clock-names
> + - clocks
> +
> +if:
Put it under allOf.
> + properties:
> + $nodename:
> + pattern: "^serial@[0-9a-f]+$"
> +then:
> + allOf:
> + - $ref: /schemas/serial/serial.yaml#
> + - $ref: /schemas/serial/rs485.yaml#
> +
> + properties:
> + atmel,use-dma-rx:
> + type: boolean
> + description: use of PDC or DMA for receiving data
> +
> + atmel,use-dma-tx:
> + type: boolean
> + description: use of PDC or DMA for transmitting data
> +
> + atmel,fifo-size:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: |
No need for |
> + Maximum number of data the RX and TX FIFOs can store for FIFO
> + capable USARTS.
> + enum: [ 16, 32 ]
> +
> +else:
> + if:
> + properties:
> + $nodename:
> + pattern: "^spi@[0-9a-f]+$"
> + then:
> + allOf:
> + - $ref: /schemas/spi/spi-controller.yaml#
> +
> + properties:
> + atmel,usart-mode:
> + const: 1
> +
> + "#size-cells":
> + const: 0
> +
> + "#address-cells":
> + const: 1
> +
> + required:
> + - atmel,usart-mode
> + - "#size-cells"
> + - "#address-cells"
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> + #include <dt-bindings/interrupt-controller/irq.h>
> + #include <dt-bindings/mfd/at91-usart.h>
> + #include <dt-bindings/dma/at91.h>
> +
> + /* use PDC */
> + usart0: serial@...8c000 {
> + compatible = "atmel,at91sam9260-usart";
> + reg = <0xfff8c000 0x4000>;
> + interrupts = <7>;
> + clocks = <&usart0_clk>;
> + clock-names = "usart";
> + atmel,use-dma-rx;
> + atmel,use-dma-tx;
> + rts-gpios = <&pioD 15 GPIO_ACTIVE_LOW>;
> + cts-gpios = <&pioD 16 GPIO_ACTIVE_LOW>;
> + dtr-gpios = <&pioD 17 GPIO_ACTIVE_LOW>;
> + dsr-gpios = <&pioD 18 GPIO_ACTIVE_LOW>;
> + dcd-gpios = <&pioD 20 GPIO_ACTIVE_LOW>;
> + rng-gpios = <&pioD 19 GPIO_ACTIVE_LOW>;
> + };
> +
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> + #include <dt-bindings/interrupt-controller/irq.h>
> + #include <dt-bindings/mfd/at91-usart.h>
> + #include <dt-bindings/dma/at91.h>
> +
> + /* use DMA */
> + usart1: serial@...1c000 {
> + compatible = "atmel,at91sam9260-usart";
> + reg = <0xf001c000 0x100>;
> + interrupts = <12 IRQ_TYPE_LEVEL_HIGH 5>;
> + clocks = <&usart0_clk>;
> + clock-names = "usart";
> + atmel,use-dma-rx;
> + atmel,use-dma-tx;
> + dmas = <&dma0 2 AT91_DMA_CFG_PER_ID(3)>,
> + <&dma0 2 (AT91_DMA_CFG_PER_ID(4) | AT91_DMA_CFG_FIFOCFG_ASAP)>;
> + dma-names = "tx", "rx";
> + atmel,fifo-size = <32>;
> + };
> +
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> + #include <dt-bindings/interrupt-controller/irq.h>
> + #include <dt-bindings/mfd/at91-usart.h>
> + #include <dt-bindings/dma/at91.h>
> +
> + /* SPI mode */
> + spi0: spi@...1c000 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "atmel,at91sam9260-usart";
> + atmel,usart-mode = <AT91_USART_MODE_SPI>;
> + reg = <0xf001c000 0x100>;
compatible, then reg then the reset of properties
> + interrupts = <12 IRQ_TYPE_LEVEL_HIGH 5>;
> + clocks = <&usart0_clk>;
> + clock-names = "usart";
> + dmas = <&dma0 2 AT91_DMA_CFG_PER_ID(3)>,
> + <&dma0 2 (AT91_DMA_CFG_PER_ID(4) | AT91_DMA_CFG_FIFOCFG_ASAP)>;
> + dma-names = "tx", "rx";
> + cs-gpios = <&pioB 3 GPIO_ACTIVE_HIGH>;
> + };
Best regards,
Krzysztof
Powered by blists - more mailing lists