On 19/08/2022 12:25, Sergiu.Moga@xxxxxxxxxxxxx wrote: > On 18.08.2022 11:38, Krzysztof Kozlowski wrote: >> 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@xxxxxxxxxxxxx> >>> --- >>> .../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@xxxxxxxxx> >>> + >>> +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. > > > I missed this in the first read, but what do you mean by under allOf? > The only allOf's in this file are under the then:'s. > It means that "if:" is preferred to be under allOf, just like example schema is showing: https://elixir.bootlin.com/linux/v5.19/source/Documentation/devicetree/bindings/example-schema.yaml Not some other allOf, which could be many in your bindings. The one allOf in top-level, just like example schema. Best regards, Krzysztof
