Re: [PATCH v2 05/13] dt-bindings: serial: atmel,at91-usart: convert to json-schema

On 08/09/2022 17:06, Sergiu.Moga@microchip.com wrote:

On 08.09.2022 15:29, Krzysztof Kozlowski wrote:

quoted

quoted

+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.

quoted

quoted

+    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

quoted

quoted

+
+    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.

quoted

quoted

+
+        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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel