Convert at91 USART DT Binding for Atmel/Microchip SoCs to
json-schema format. Furthermore, move this binding to the
serial directory, since binding directories match hardware,
unlike the driver subsystems which match Linux convention.
Signed-off-by: Sergiu Moga <[email protected]>
---
v1 -> v2:
- only do what the commit says, split the addition of other compatibles and
properties in other patches
- remove unnecessary "|"'s
- mention header in `atmel,usart-mode`'s description
- place `if:` under `allOf:`
- respect order of spi0's DT properties: compatible, then reg then the reset of properties
.../devicetree/bindings/mfd/atmel-usart.txt | 98 ----------
.../bindings/serial/atmel,at91-usart.yaml | 183 ++++++++++++++++++
2 files changed, 183 insertions(+), 98 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/mfd/atmel-usart.txt
create mode 100644 Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml
diff --git a/Documentation/devicetree/bindings/mfd/atmel-usart.txt b/Documentation/devicetree/bindings/mfd/atmel-usart.txt
deleted file mode 100644
index a09133066aff..000000000000
--- a/Documentation/devicetree/bindings/mfd/atmel-usart.txt
+++ /dev/null
@@ -1,98 +0,0 @@
-* Atmel Universal Synchronous Asynchronous Receiver/Transmitter (USART)
-
-Required properties for USART:
-- compatible: Should be one of the following:
- - "atmel,at91rm9200-usart"
- - "atmel,at91sam9260-usart"
- - "microchip,sam9x60-usart"
- - "atmel,at91rm9200-dbgu", "atmel,at91rm9200-usart"
- - "atmel,at91sam9260-dbgu", "atmel,at91sam9260-usart"
- - "microchip,sam9x60-dbgu", "microchip,sam9x60-usart"
-- reg: Should contain registers location and length
-- interrupts: Should contain interrupt
-- clock-names: tuple listing input clock names.
- Required elements: "usart"
-- clocks: phandles to input clocks.
-
-Required properties for USART in SPI mode:
-- #size-cells : Must be <0>
-- #address-cells : Must be <1>
-- cs-gpios: chipselects (internal cs not supported)
-- atmel,usart-mode : Must be <AT91_USART_MODE_SPI> (found in dt-bindings/mfd/at91-usart.h)
-
-Optional properties in serial and SPI mode:
-- dma bindings for dma transfer:
- - dmas: DMA specifier, consisting of a phandle to DMA controller node,
- memory peripheral interface and USART DMA channel ID, FIFO configuration.
- The order of DMA channels is fixed. The first DMA channel must be TX
- associated channel and the second one must be RX associated channel.
- Refer to dma.txt and atmel-dma.txt for details.
- - dma-names: "tx" for TX channel.
- "rx" for RX channel.
- The order of dma-names is also fixed. The first name must be "tx"
- and the second one must be "rx" as in the examples below.
-
-Optional properties in serial mode:
-- atmel,use-dma-rx: use of PDC or DMA for receiving data
-- atmel,use-dma-tx: use of PDC or DMA for transmitting data
-- {rts,cts,dtr,dsr,rng,dcd}-gpios: specify a GPIO for RTS/CTS/DTR/DSR/RI/DCD line respectively.
- It will use specified PIO instead of the peripheral function pin for the USART feature.
- If unsure, don't specify this property.
-- atmel,fifo-size: maximum number of data the RX and TX FIFOs can store for FIFO
- capable USARTs.
-- rs485-rts-delay, rs485-rx-during-tx, linux,rs485-enabled-at-boot-time: see rs485.txt
-
-<chip> compatible description:
-- at91rm9200: legacy USART support
-- at91sam9260: generic USART implementation for SAM9 SoCs
-
-Example:
-- use PDC:
- usart0: serial@fff8c000 {
- 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>;
- };
-
-- use DMA:
- usart0: serial@f001c000 {
- compatible = "atmel,at91sam9260-usart";
- reg = <0xf001c000 0x100>;
- interrupts = <12 4 5>;
- clocks = <&usart0_clk>;
- clock-names = "usart";
- atmel,use-dma-rx;
- atmel,use-dma-tx;
- dmas = <&dma0 2 0x3>,
- <&dma0 2 0x204>;
- dma-names = "tx", "rx";
- atmel,fifo-size = <32>;
- };
-
-- SPI mode:
- #include <dt-bindings/mfd/at91-usart.h>
-
- spi0: spi@f001c000 {
- #address-cells = <1>;
- #size-cells = <0>;
- compatible = "atmel,at91rm9200-usart", "atmel,at91sam9260-usart";
- atmel,usart-mode = <AT91_USART_MODE_SPI>;
- reg = <0xf001c000 0x100>;
- 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 0>;
- };
diff --git a/Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml b/Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml
new file mode 100644
index 000000000000..b25535b7a4d2
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml
@@ -0,0 +1,183 @@
+# 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/serial/atmel,at91-usart.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Atmel Universal Synchronous Asynchronous Receiver/Transmitter (USART)
+
+maintainers:
+ - Richard Genoud <[email protected]>
+
+properties:
+ compatible:
+ oneOf:
+ - enum:
+ - atmel,at91rm9200-usart
+ - atmel,at91sam9260-usart
+ - microchip,sam9x60-usart
+ - 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
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ clock-names:
+ const: usart
+
+ clocks:
+ maxItems: 1
+
+ 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:
+ Must be either <AT91_USART_MODE_SPI> for SPI or
+ <AT91_USART_MODE_SERIAL> for USART (found in dt-bindings/mfd/at91-usart.h).
+ enum: [ 0, 1 ]
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clock-names
+ - clocks
+
+allOf:
+ - if:
+ 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:
+ 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@fff8c000 {
+ 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@f001c000 {
+ 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@f001c000 {
+ compatible = "atmel,at91sam9260-usart";
+ reg = <0xf001c000 0x100>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ atmel,usart-mode = <AT91_USART_MODE_SPI>;
+ 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>;
+ };
--
2.25.1
On 06/09/2022 15:55, Sergiu Moga wrote:
> Convert at91 USART DT Binding for Atmel/Microchip SoCs to
> json-schema format. Furthermore, move this binding to the
> serial directory, since binding directories match hardware,
> unlike the driver subsystems which match Linux convention.
>
> Signed-off-by: Sergiu Moga <[email protected]>
> ---
>
>
>
> v1 -> v2:
> - only do what the commit says, split the addition of other compatibles and
> properties in other patches
> - remove unnecessary "|"'s
> - mention header in `atmel,usart-mode`'s description
> - place `if:` under `allOf:`
> - respect order of spi0's DT properties: compatible, then reg then the reset of properties
>
>
>
>
>
> .../devicetree/bindings/mfd/atmel-usart.txt | 98 ----------
> .../bindings/serial/atmel,at91-usart.yaml | 183 ++++++++++++++++++
> 2 files changed, 183 insertions(+), 98 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/mfd/atmel-usart.txt
> create mode 100644 Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml
>
> diff --git a/Documentation/devicetree/bindings/mfd/atmel-usart.txt b/Documentation/devicetree/bindings/mfd/atmel-usart.txt
> deleted file mode 100644
> index a09133066aff..000000000000
> --- a/Documentation/devicetree/bindings/mfd/atmel-usart.txt
> +++ /dev/null
> @@ -1,98 +0,0 @@
> -* Atmel Universal Synchronous Asynchronous Receiver/Transmitter (USART)
> -
> -Required properties for USART:
> -- compatible: Should be one of the following:
> - - "atmel,at91rm9200-usart"
> - - "atmel,at91sam9260-usart"
> - - "microchip,sam9x60-usart"
> - - "atmel,at91rm9200-dbgu", "atmel,at91rm9200-usart"
> - - "atmel,at91sam9260-dbgu", "atmel,at91sam9260-usart"
> - - "microchip,sam9x60-dbgu", "microchip,sam9x60-usart"
> -- reg: Should contain registers location and length
> -- interrupts: Should contain interrupt
> -- clock-names: tuple listing input clock names.
> - Required elements: "usart"
> -- clocks: phandles to input clocks.
> -
> -Required properties for USART in SPI mode:
> -- #size-cells : Must be <0>
> -- #address-cells : Must be <1>
> -- cs-gpios: chipselects (internal cs not supported)
> -- atmel,usart-mode : Must be <AT91_USART_MODE_SPI> (found in dt-bindings/mfd/at91-usart.h)
> -
> -Optional properties in serial and SPI mode:
> -- dma bindings for dma transfer:
> - - dmas: DMA specifier, consisting of a phandle to DMA controller node,
> - memory peripheral interface and USART DMA channel ID, FIFO configuration.
> - The order of DMA channels is fixed. The first DMA channel must be TX
> - associated channel and the second one must be RX associated channel.
> - Refer to dma.txt and atmel-dma.txt for details.
> - - dma-names: "tx" for TX channel.
> - "rx" for RX channel.
> - The order of dma-names is also fixed. The first name must be "tx"
> - and the second one must be "rx" as in the examples below.
> -
> -Optional properties in serial mode:
> -- atmel,use-dma-rx: use of PDC or DMA for receiving data
> -- atmel,use-dma-tx: use of PDC or DMA for transmitting data
> -- {rts,cts,dtr,dsr,rng,dcd}-gpios: specify a GPIO for RTS/CTS/DTR/DSR/RI/DCD line respectively.
> - It will use specified PIO instead of the peripheral function pin for the USART feature.
> - If unsure, don't specify this property.
> -- atmel,fifo-size: maximum number of data the RX and TX FIFOs can store for FIFO
> - capable USARTs.
> -- rs485-rts-delay, rs485-rx-during-tx, linux,rs485-enabled-at-boot-time: see rs485.txt
> -
> -<chip> compatible description:
> -- at91rm9200: legacy USART support
> -- at91sam9260: generic USART implementation for SAM9 SoCs
> -
> -Example:
> -- use PDC:
> - usart0: serial@fff8c000 {
> - 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>;
> - };
> -
> -- use DMA:
> - usart0: serial@f001c000 {
> - compatible = "atmel,at91sam9260-usart";
> - reg = <0xf001c000 0x100>;
> - interrupts = <12 4 5>;
> - clocks = <&usart0_clk>;
> - clock-names = "usart";
> - atmel,use-dma-rx;
> - atmel,use-dma-tx;
> - dmas = <&dma0 2 0x3>,
> - <&dma0 2 0x204>;
> - dma-names = "tx", "rx";
> - atmel,fifo-size = <32>;
> - };
> -
> -- SPI mode:
> - #include <dt-bindings/mfd/at91-usart.h>
> -
> - spi0: spi@f001c000 {
> - #address-cells = <1>;
> - #size-cells = <0>;
> - compatible = "atmel,at91rm9200-usart", "atmel,at91sam9260-usart";
> - atmel,usart-mode = <AT91_USART_MODE_SPI>;
> - reg = <0xf001c000 0x100>;
> - 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 0>;
> - };
> diff --git a/Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml b/Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml
> new file mode 100644
> index 000000000000..b25535b7a4d2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml
> @@ -0,0 +1,183 @@
> +# 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/serial/atmel,at91-usart.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Atmel Universal Synchronous Asynchronous Receiver/Transmitter (USART)
> +
> +maintainers:
> + - Richard Genoud <[email protected]>
> +
> +properties:
> + compatible:
> + oneOf:
> + - enum:
> + - atmel,at91rm9200-usart
> + - atmel,at91sam9260-usart
> + - microchip,sam9x60-usart
> + - 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
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + clock-names:
> + const: usart
> +
> + clocks:
> + maxItems: 1
> +
> + 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:
> + Must be either <AT91_USART_MODE_SPI> for SPI or
> + <AT91_USART_MODE_SERIAL> for USART (found in dt-bindings/mfd/at91-usart.h).
> + enum: [ 0, 1 ]
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - clock-names
> + - clocks
> +
> +allOf:
> + - if:
> + properties:
> + $nodename:
> + pattern: "^serial@[0-9a-f]+$"
You should rather check value of atmel,usart-mode, because now you won't
properly match device nodes called "foobar". Since usart-mode has only
two possible values, this will nicely simplify you if-else.
> + 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:
> + Maximum number of data the RX and TX FIFOs can store for FIFO
> + capable USARTS.
> + enum: [ 16, 32 ]
I did not mention it last time, but I think it should follow generic
practice, so define all properties top-level and disallow them for other
type. This allows you to simply use additionalProperties:false at the end.
> +
> + 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
The same - top level and disallow them for uart.
> +
> + required:
> + - atmel,usart-mode
> + - "#size-cells"
> + - "#address-cells"
End else in this branch is what?
> +
> +unevaluatedProperties: false
> +
Best regards,
Krzysztof
On 08.09.2022 15:29, Krzysztof Kozlowski wrote:
> On 06/09/2022 15:55, Sergiu Moga wrote:
>> Convert at91 USART DT Binding for Atmel/Microchip SoCs to
>> json-schema format. Furthermore, move this binding to the
>> serial directory, since binding directories match hardware,
>> unlike the driver subsystems which match Linux convention.
>>
>> Signed-off-by: Sergiu Moga <[email protected]>
>> ---
>>
>>
>>
>> v1 -> v2:
>> - only do what the commit says, split the addition of other compatibles and
>> properties in other patches
>> - remove unnecessary "|"'s
>> - mention header in `atmel,usart-mode`'s description
>> - place `if:` under `allOf:`
>> - respect order of spi0's DT properties: compatible, then reg then the reset of properties
>>
>>
>>
>>
>>
>> .../devicetree/bindings/mfd/atmel-usart.txt | 98 ----------
>> .../bindings/serial/atmel,at91-usart.yaml | 183 ++++++++++++++++++
>> 2 files changed, 183 insertions(+), 98 deletions(-)
>> delete mode 100644 Documentation/devicetree/bindings/mfd/atmel-usart.txt
>> create mode 100644 Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/atmel-usart.txt b/Documentation/devicetree/bindings/mfd/atmel-usart.txt
>> deleted file mode 100644
>> index a09133066aff..000000000000
>> --- a/Documentation/devicetree/bindings/mfd/atmel-usart.txt
>> +++ /dev/null
>> @@ -1,98 +0,0 @@
>> -* Atmel Universal Synchronous Asynchronous Receiver/Transmitter (USART)
>> -
>> -Required properties for USART:
>> -- compatible: Should be one of the following:
>> - - "atmel,at91rm9200-usart"
>> - - "atmel,at91sam9260-usart"
>> - - "microchip,sam9x60-usart"
>> - - "atmel,at91rm9200-dbgu", "atmel,at91rm9200-usart"
>> - - "atmel,at91sam9260-dbgu", "atmel,at91sam9260-usart"
>> - - "microchip,sam9x60-dbgu", "microchip,sam9x60-usart"
>> -- reg: Should contain registers location and length
>> -- interrupts: Should contain interrupt
>> -- clock-names: tuple listing input clock names.
>> - Required elements: "usart"
>> -- clocks: phandles to input clocks.
>> -
>> -Required properties for USART in SPI mode:
>> -- #size-cells : Must be <0>
>> -- #address-cells : Must be <1>
>> -- cs-gpios: chipselects (internal cs not supported)
>> -- atmel,usart-mode : Must be <AT91_USART_MODE_SPI> (found in dt-bindings/mfd/at91-usart.h)
>> -
>> -Optional properties in serial and SPI mode:
>> -- dma bindings for dma transfer:
>> - - dmas: DMA specifier, consisting of a phandle to DMA controller node,
>> - memory peripheral interface and USART DMA channel ID, FIFO configuration.
>> - The order of DMA channels is fixed. The first DMA channel must be TX
>> - associated channel and the second one must be RX associated channel.
>> - Refer to dma.txt and atmel-dma.txt for details.
>> - - dma-names: "tx" for TX channel.
>> - "rx" for RX channel.
>> - The order of dma-names is also fixed. The first name must be "tx"
>> - and the second one must be "rx" as in the examples below.
>> -
>> -Optional properties in serial mode:
>> -- atmel,use-dma-rx: use of PDC or DMA for receiving data
>> -- atmel,use-dma-tx: use of PDC or DMA for transmitting data
>> -- {rts,cts,dtr,dsr,rng,dcd}-gpios: specify a GPIO for RTS/CTS/DTR/DSR/RI/DCD line respectively.
>> - It will use specified PIO instead of the peripheral function pin for the USART feature.
>> - If unsure, don't specify this property.
>> -- atmel,fifo-size: maximum number of data the RX and TX FIFOs can store for FIFO
>> - capable USARTs.
>> -- rs485-rts-delay, rs485-rx-during-tx, linux,rs485-enabled-at-boot-time: see rs485.txt
>> -
>> -<chip> compatible description:
>> -- at91rm9200: legacy USART support
>> -- at91sam9260: generic USART implementation for SAM9 SoCs
>> -
>> -Example:
>> -- use PDC:
>> - usart0: serial@fff8c000 {
>> - 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>;
>> - };
>> -
>> -- use DMA:
>> - usart0: serial@f001c000 {
>> - compatible = "atmel,at91sam9260-usart";
>> - reg = <0xf001c000 0x100>;
>> - interrupts = <12 4 5>;
>> - clocks = <&usart0_clk>;
>> - clock-names = "usart";
>> - atmel,use-dma-rx;
>> - atmel,use-dma-tx;
>> - dmas = <&dma0 2 0x3>,
>> - <&dma0 2 0x204>;
>> - dma-names = "tx", "rx";
>> - atmel,fifo-size = <32>;
>> - };
>> -
>> -- SPI mode:
>> - #include <dt-bindings/mfd/at91-usart.h>
>> -
>> - spi0: spi@f001c000 {
>> - #address-cells = <1>;
>> - #size-cells = <0>;
>> - compatible = "atmel,at91rm9200-usart", "atmel,at91sam9260-usart";
>> - atmel,usart-mode = <AT91_USART_MODE_SPI>;
>> - reg = <0xf001c000 0x100>;
>> - 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 0>;
>> - };
>> diff --git a/Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml b/Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml
>> new file mode 100644
>> index 000000000000..b25535b7a4d2
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml
>> @@ -0,0 +1,183 @@
>> +# 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/serial/atmel,at91-usart.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Atmel Universal Synchronous Asynchronous Receiver/Transmitter (USART)
>> +
>> +maintainers:
>> + - Richard Genoud <[email protected]>
>> +
>> +properties:
>> + compatible:
>> + oneOf:
>> + - enum:
>> + - atmel,at91rm9200-usart
>> + - atmel,at91sam9260-usart
>> + - microchip,sam9x60-usart
>> + - 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
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + interrupts:
>> + maxItems: 1
>> +
>> + clock-names:
>> + const: usart
>> +
>> + clocks:
>> + maxItems: 1
>> +
>> + 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:
>> + Must be either <AT91_USART_MODE_SPI> for SPI or
>> + <AT91_USART_MODE_SERIAL> for USART (found in dt-bindings/mfd/at91-usart.h).
>> + enum: [ 0, 1 ]
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - interrupts
>> + - clock-names
>> + - clocks
>> +
>> +allOf:
>> + - if:
>> + properties:
>> + $nodename:
>> + pattern: "^serial@[0-9a-f]+$"
>
> You should rather check value of atmel,usart-mode, because now you won't
> properly match device nodes called "foobar". Since usart-mode has only
> two possible values, this will nicely simplify you if-else.
>
>
I did think of that but the previous binding specifies that
atmel,usart-mode is required only for the SPI mode and it is optional
for the USART mode. That is why I went for the node's regex since I
thought it is something that both nodes would have.
>> + 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:
>> + Maximum number of data the RX and TX FIFOs can store for FIFO
>> + capable USARTS.
>> + enum: [ 16, 32 ]
>
> I did not mention it last time, but I think it should follow generic
> practice, so define all properties top-level and disallow them for other
> type. This allows you to simply use additionalProperties:false at the end.
>
What would be a good example binding in this case?
>> +
>> + 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
>
> The same - top level and disallow them for uart.
>
These values of #size-cells and #address-cells are only meant for the
SPI so I guess I would still have to specify their mandatory const
values here.
>> +
>> + required:
>> + - atmel,usart-mode
>> + - "#size-cells"
>> + - "#address-cells"
>
> End else in this branch is what?
>
You are right, I will remove the useless if: after else:
>> +
>> +unevaluatedProperties: false
>> +
>
> Best regards,
> Krzysztof
Regards,
Sergiu
On 08/09/2022 17:06, [email protected] wrote:
> On 08.09.2022 15:29, Krzysztof Kozlowski wrote:
>>> +required:
>>> + - compatible
>>> + - reg
>>> + - interrupts
>>> + - clock-names
>>> + - clocks
>>> +
>>> +allOf:
>>> + - if:
>>> + properties:
>>> + $nodename:
>>> + pattern: "^serial@[0-9a-f]+$"
>>
>> You should rather check value of atmel,usart-mode, because now you won't
>> properly match device nodes called "foobar". Since usart-mode has only
>> two possible values, this will nicely simplify you if-else.
>>
>>
>
>
> I did think of that but the previous binding specifies that
> atmel,usart-mode is required only for the SPI mode and it is optional
> for the USART mode. That is why I went for the node's regex since I
> thought it is something that both nodes would have.
I think it should be explicit - you configure node either to this or
that, so the property should be always present. The node name should not
be responsible for it, even though we want node names to match certain
patterns.
>
>
>>> + 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:
>>> + Maximum number of data the RX and TX FIFOs can store for FIFO
>>> + capable USARTS.
>>> + enum: [ 16, 32 ]
>>
>> I did not mention it last time, but I think it should follow generic
>> practice, so define all properties top-level and disallow them for other
>> type. This allows you to simply use additionalProperties:false at the end.
>>
>
>
> What would be a good example binding in this case?
The example binding.
https://elixir.bootlin.com/linux/v5.19/source/Documentation/devicetree/bindings/example-schema.yaml#L212
>
>
>>> +
>>> + 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
>>
>> The same - top level and disallow them for uart.
>>
>
>
> These values of #size-cells and #address-cells are only meant for the
> SPI so I guess I would still have to specify their mandatory const
> values here.
Sure, ok.
>
>
>>> +
>>> + required:
>>> + - atmel,usart-mode
>>> + - "#size-cells"
>>> + - "#address-cells"
>>
>> End else in this branch is what?
>>
>
>
> You are right, I will remove the useless if: after else:
Best regards,
Krzysztof
On 08.09.2022 18:10, Krzysztof Kozlowski wrote:
> On 08/09/2022 17:06, [email protected] wrote:
>> On 08.09.2022 15:29, Krzysztof Kozlowski wrote:
>
>>>> +required:
>>>> + - compatible
>>>> + - reg
>>>> + - interrupts
>>>> + - clock-names
>>>> + - clocks
>>>> +
>>>> +allOf:
>>>> + - if:
>>>> + properties:
>>>> + $nodename:
>>>> + pattern: "^serial@[0-9a-f]+$"
>>>
>>> You should rather check value of atmel,usart-mode, because now you won't
>>> properly match device nodes called "foobar". Since usart-mode has only
>>> two possible values, this will nicely simplify you if-else.
>>>
>>>
>>
>>
>> I did think of that but the previous binding specifies that
>> atmel,usart-mode is required only for the SPI mode and it is optional
>> for the USART mode. That is why I went for the node's regex since I
>> thought it is something that both nodes would have.
>
> I think it should be explicit - you configure node either to this or
> that, so the property should be always present.
No DT of ours has that property atm, since they are all on USART mode by
default. If I were to make it required. all nodes would fail so I would
have to add it to each of them.
> The node name should not
> be responsible for it, even though we want node names to match certain
> patterns.
>
Does checkig for the node's pattern not make it better then? Since it
imposes an additional check? If it would not have a conventional
pattern, it would fail through unevaluatedProperies:false at the end,
since it would have properties that were contained inside a branch that
the validation of the node would not have gone through since it contains
a pattern that does not match the conditions of that branch.
>>
>>
>>>> + 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:
>>>> + Maximum number of data the RX and TX FIFOs can store for FIFO
>>>> + capable USARTS.
>>>> + enum: [ 16, 32 ]
>>>
>>> I did not mention it last time, but I think it should follow generic
>>> practice, so define all properties top-level and disallow them for other
>>> type. This allows you to simply use additionalProperties:false at the end.
>>>
>>
>>
>> What would be a good example binding in this case?
>
> The example binding.
>
> https://elixir.bootlin.com/linux/v5.19/source/Documentation/devicetree/bindings/example-schema.yaml#L212
>
Ah, I understand now. I did not get what you meant by "disallow", I
guess it's just a "property-name: false".
Thanks!
>>
>>
>>>> +
>>>> + 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
>>>
>>> The same - top level and disallow them for uart.
>>>
>>
>>
>> These values of #size-cells and #address-cells are only meant for the
>> SPI so I guess I would still have to specify their mandatory const
>> values here.
>
> Sure, ok.
>
>>
>>
>>>> +
>>>> + required:
>>>> + - atmel,usart-mode
>>>> + - "#size-cells"
>>>> + - "#address-cells"
>>>
>>> End else in this branch is what?
>>>
>>
>>
>> You are right, I will remove the useless if: after else:
>
> Best regards,
> Krzysztof
Regards,
Sergiu
On 08/09/2022 17:27, [email protected] wrote:
> On 08.09.2022 18:10, Krzysztof Kozlowski wrote:
>> On 08/09/2022 17:06, [email protected] wrote:
>>> On 08.09.2022 15:29, Krzysztof Kozlowski wrote:
>>
>>>>> +required:
>>>>> + - compatible
>>>>> + - reg
>>>>> + - interrupts
>>>>> + - clock-names
>>>>> + - clocks
>>>>> +
>>>>> +allOf:
>>>>> + - if:
>>>>> + properties:
>>>>> + $nodename:
>>>>> + pattern: "^serial@[0-9a-f]+$"
>>>>
>>>> You should rather check value of atmel,usart-mode, because now you won't
>>>> properly match device nodes called "foobar". Since usart-mode has only
>>>> two possible values, this will nicely simplify you if-else.
>>>>
>>>>
>>>
>>>
>>> I did think of that but the previous binding specifies that
>>> atmel,usart-mode is required only for the SPI mode and it is optional
>>> for the USART mode. That is why I went for the node's regex since I
>>> thought it is something that both nodes would have.
>>
>> I think it should be explicit - you configure node either to this or
>> that, so the property should be always present.
>
>
>
> No DT of ours has that property atm, since they are all on USART mode by
> default. If I were to make it required. all nodes would fail so I would
> have to add it to each of them.
Which is a problem because...?
Have in mind that bindings can be changed. ABI here won't be broken.
>
>
>
>
>> The node name should not
>> be responsible for it, even though we want node names to match certain
>> patterns.
>>
>
>
> Does checkig for the node's pattern not make it better then? Since it
> imposes an additional check?
Not really, because if it is "foobar" your schema would not be applied
correctly.
> If it would not have a conventional
> pattern, it would fail through unevaluatedProperies:false at the end,
> since it would have properties that were contained inside a branch that
> the validation of the node would not have gone through since it contains
> a pattern that does not match the conditions of that branch.
Not for properties which are for example missing...
Best regards,
Krzysztof