lkml.org 
[lkml]   [2022]   [Aug]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH 2/5] dt-bindings: mfd: atmel,at91-usart: convert to json-schema
    From
    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@microchip.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@gmail.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@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 {
    > + #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

    \
     
     \ /
      Last update: 2022-08-18 10:39    [W:4.447 / U:0.104 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site