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

[Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>]

On 08/09/2022 17:27, Sergiu.Moga@xxxxxxxxxxxxx wrote:
> On 08.09.2022 18:10, Krzysztof Kozlowski wrote:
>> On 08/09/2022 17:06, Sergiu.Moga@xxxxxxxxxxxxx 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