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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1f532784-c46d-6746-2511-466fd82c0809@ti.com>
Date:   Fri, 9 Oct 2020 11:06:49 +0300
From:   Peter Ujfalusi <peter.ujfalusi@...com>
To:     Rob Herring <robh@...nel.org>
CC:     Vinod <vkoul@...nel.org>, Nishanth Menon <nm@...com>,
        Santosh Shilimkar <ssantosh@...nel.org>,
        Vignesh R <vigneshr@...com>,
        Dan Williams <dan.j.williams@...el.com>,
        Tero Kristo <t-kristo@...com>,
        Lokesh Vutla <lokeshvutla@...com>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        <devicetree@...r.kernel.org>,
        "open list:DMA GENERIC OFFLOAD ENGINE SUBSYSTEM" 
        <dmaengine@...r.kernel.org>
Subject: Re: [PATCH 09/18] dt-bindings: dma: ti: Add document for K3 BCDMA



On 08/10/2020 22.15, Rob Herring wrote:
> On Thu, Oct 8, 2020 at 3:40 AM Peter Ujfalusi <peter.ujfalusi@...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

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki