Re: [PATCH 09/18] dt-bindings: dma: ti: Add document for K3 BCDMA

Peter Ujfalusi
2020-10-09T08:06:44Z

On 08/10/2020 22.15, Rob Herring wrote:
> On Thu, Oct 8, 2020 at 3:40 AM Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:

>>> Yeah, you have to do 'unevaluatedProperties: false' which doesn't
>>> actually do anything yet, but can 'see' into $ref's.
>>
>> I see, but even if I add the unevaluatedProperties: false I will have
>> the same error as long as I have additionalProperties: false
>
> Yes. I meant unevaluatedProperties instead of additionalProperties.

OK, changed it to unevaluatedProperties.

>> If I remove the additionalProperties then it makes no difference if I
>> have the unevaluatedProperties: false or I don't.
>
> Not yet, but it will soon. Once I have the tree in a consistent state
> in 5.10-rc1, there will be a meta-schema to check all this (which is
> one of those must always be present).
>
> Though, as of now 'unevaluatedProperties' doesn't do anything because
> the underlying json-schema tool doesn't yet support it.

Understand, thanks for the details.

>>>>>> + ti,sci-rm-range-bchan:
>>>>>> + description: |
>>>>>> + Array of BCDMA block-copy channel resource subtypes for resource
>>>>>> + allocation for this host
>>>>>> + allOf:
>>>>>> + - $ref: /schemas/types.yaml#/definitions/uint32-array
>>>>>> + minItems: 1
>>>>>> + # Should be enough
>>>>>> + maxItems: 255
>>>>>
>>>>> Are there constraints for the individual elements?
>>>>
>>>> In practice the subtype ID is 6bits number.
>>>> Should I add limits to individual elements?
>>>
>>> Yes:
>>>
>>> items:
>>> maximum: 0x3f
>>
>> Right, I can just omit the minimum.
>>
>> It would be nice if I could use definitions for these ranges to avoid
>> duplicated lines by adding
>>
>> definitions:
>> ti,rm-range:
>> $ref: /schemas/types.yaml#/definitions/uint32-array
>> minItems: 1
>> # Should be enough
>> maxItems: 255
>> items:
>> minimum: 0
>> maximum: 0x3f
>>
>> to schemas/arm/keystone/ti,k3-sci-common.yaml
>>
>> and only have:
>>
>> ti,sci-rm-range-bchan:
>> $ref:
>> /schemas/arm/keystone/ti,k3-sci-common.yaml#/definitions/ti,rm-range
>> description: |
>> Array of BCDMA block-copy channel resource subtypes for resource
>> allocation for this host
>
> Just do:
>
> patternProperties:
> "^ti,sci-rm-range-[btr]chan$":
> ...
>
> If this is common for other bindings, then you can put it in
> ti,k3-sci-common.yaml.

Similar property (for RM ranges) also used by the ringacc, I have tried
to standardize us to use: ti,sci-rm-range-* in DT.

I will leave it as it is now for this series and we can simplify it
later with a wider series touching all involved yaml files.

>> but it results:
>> Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml:
>> properties:ti,sci-rm-range-bchan: {'$ref':
>> '/schemas/arm/keystone/ti,k3-sci-common.yaml#/definitions/ti,rm-range',
>> 'description': 'Array of BCDMA block-copy channel resource subtypes for
>> resource\nallocation for this host\n'} is not valid under any of the
>> given schemas (Possible causes of the failure):
>> Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml:
>> properties:ti,sci-rm-range-bchan: 'not' is a required property
>> Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml:
>> properties:ti,sci-rm-range-bchan:$ref:
>> '/schemas/arm/keystone/ti,k3-sci-common.yaml#/definitions/ti,rm-range'
>> does not match 'types.yaml#[/]{0,1}definitions/.*'
>
> We probably should allow for using 'definitions' which is pretty
> common json-schema practice, but don't primarily in order to keep
> folks within the lines. Things are optimized for not knowing
> json-schema and trying to minimize errors I have to check for.

I agree on these.

> Supporting it would complicate the meta-schema and the tools' fixup
> code. So far, the need for it has been pretty infrequent.

Sure, for the couple of duplication I have it is manageable without
sacrificing readability.

btw: I have made the similar changes to the k3-pktdma schema.

>
> Rob
>

- Péter