Re: [PATCH 1/3] dt-bindings: hsi: hsi-client: convert to YAML

From: Krzysztof Kozlowski
Date: Tue Mar 26 2024 - 12:08:48 EST


On 26/03/2024 16:15, Sebastian Reichel wrote:
> Hi,
>
> On Tue, Mar 26, 2024 at 01:56:22PM +0100, Krzysztof Kozlowski wrote:
>>>>> +allOf:
>>>>> + - if:
>>>>> + required:
>>>>> + - hsi-mode
>>>>> + then:
>>>>> + properties:
>>>>> + hsi-rx-mode: false
>>>>> + hsi-tx-mode: false
>>>>
>>>> I don't understand what you are trying to achieve here and with anyOf.
>>>> It looks like just oneOf. OTOH, old binding did not exclude these
>>>> properties.
>>>
>>> So the anyOf ensures, that either hsi-mode or hsi-rx-mode +
>>> hsi-tx-mode are specified. Those properties were previously
>>
>> Not entirely. anyOf should succeed also when none of them are present,
>> which is not what you want in such case.
>
> Right, this should be oneOf instead of anyOf. I fixed that for v2.
>
>>> listed as required and they are indeed mandatory by the Linux
>>> kernel implementation.
>>>
>>> The old binding also has this:
>>>
>>> hsi-mode: May be used ***instead*** hsi-rx-mode and hsi-tx-mode
>>>
>>> So it's either hsi-rx-mode + hsi-tx-mode OR hsi-mode, but not
>>> all properties at the same time. That's what the allOf ensures:
>>> if hsi-mode is specified, then hsi-rx-mode and hsi-tx-mode may
>>> not be specified.
>>
>> Then wouldn't this work for you:
>> https://elixir.bootlin.com/linux/v5.17-rc2/source/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml#L91
>
> I suppose you mean using "then: not: required: PROPERTY" instead of
> "then: PROPERTY: false"? The variant using "PROPERTY: false" is what
> is being used in example-schema.yaml:

No, I pointed to specific line with code for you.

>
> https://elixir.bootlin.com/linux/v6.8/source/Documentation/devicetree/bindings/example-schema.yaml#L225
>
> IMHO the "not: required: property" is harder to understand. I would
> expect that to mean "the property is not required (i.e. optional)"
> instead of "the property is not allowed".


Best regards,
Krzysztof