lkml.org 
[lkml]   [2022]   [Sep]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH v2 05/13] dt-bindings: serial: atmel,at91-usart: convert to json-schema
    Date
    On 08.09.2022 18:10, Krzysztof Kozlowski wrote:
    > On 08/09/2022 17:06, Sergiu.Moga@microchip.com 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
    \
     
     \ /
      Last update: 2022-09-08 17:31    [W:6.354 / U:0.072 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site