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

Peter Ujfalusi
2020-10-08T08:40:13Z

On 07/10/2020 18.46, Rob Herring wrote:
> On Wed, Oct 07, 2020 at 12:09:06PM +0300, Peter Ujfalusi wrote:
>>
>>
>> On 06/10/2020 22.29, Rob Herring wrote:
>>> On Wed, Sep 30, 2020 at 12:14:03PM +0300, Peter Ujfalusi wrote:
>>>> New binding document for
>>>> Texas Instruments K3 Block Copy DMA (BCDMA).
>>>>
>>>> BCDMA is introduced as part of AM64.
>>>>
>>
>> ...
>>
>>>
>>>> + ti,sci:
>>>> + description: phandle to TI-SCI compatible System controller node
>>>> + allOf:
>>>> + - $ref: /schemas/types.yaml#/definitions/phandle
>>>> +
>>>> + ti,sci-dev-id:
>>>> + description: TI-SCI device id of BCDMA
>>>> + allOf:
>>>> + - $ref: /schemas/types.yaml#/definitions/uint32
>>>
>>> We have a common definition for these.
>>
>> Yes, in arm/keystone/ti,k3-sci-common.yaml, but I could not get to use
>> that as reference.
>>
>> I can not list it under the topmost allOf and drop the ti,sci and
>> ti,sci-dev-id like this:
>>
>> allOf:
>> - $ref: /schemas/dma/dma-controller.yaml#
>> - $ref: /schemas/arm/keystone/ti,k3-sci-common.yaml#
>>
>> It results:
>> CHKDT Documentation/devicetree/bindings/processed-schema-examples.json
>> DTEX Documentation/devicetree/bindings/dma/ti/k3-bcdma.example.dts
>> SCHEMA Documentation/devicetree/bindings/processed-schema-examples.json
>> DTC Documentation/devicetree/bindings/dma/ti/k3-bcdma.example.dt.yaml
>> CHECK Documentation/devicetree/bindings/dma/ti/k3-bcdma.example.dt.yaml
>> Documentation/devicetree/bindings/dma/ti/k3-bcdma.example.dt.yaml:
>> dma-controller@485c0100: 'ti,sci', 'ti,sci-dev-id' do not match any of
>> the regexes: 'pinctrl-[0-9]+'
>> From schema: Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml
>>
>> If I remove the "additionalProperties: false" from the schema file, then
>> it compiles fine.
>
> 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

If I remove the additionalProperties then it makes no difference if I
have the unevaluatedProperties: false or I don't.

>
>>>> + ti,asel:
>>>> + description: ASEL value for non slave channels
>>>> + allOf:
>>>
>>> You no longer need 'allOf' here.
>>
>> OK, I changed it in all instances.
>>
>>>
>>>> + - $ref: /schemas/types.yaml#/definitions/uint32
>>>> +
>>>> + 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

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/.*'

SCHEMA Documentation/devicetree/bindings/processed-schema-examples.json
Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml: ignoring, error
in schema: properties: ti,sci-rm-range-bchan
warning: no schema found in file:
Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml

So, obviously I'm looking at it from a wrong angle. It is not urgent, I
can spend time to figure out later and switch all cases where the RM
ranges are used.

>
>>
>>>> +
>>>> + ti,sci-rm-range-tchan:
>>>> + description: |
>>>> + Array of BCDMA split tx channel resource subtypes for resource allocation
>>>> + for this host
>>>> + allOf:
>>>> + - $ref: /schemas/types.yaml#/definitions/uint32-array
>>>> + minItems: 1
>>>> + # Should be enough
>>>> + maxItems: 255
>>>> +
>>>> + ti,sci-rm-range-rchan:
>>>> + description: |
>>>> + Array of BCDMA split rx channel resource subtypes for resource allocation
>>>> + for this host
>>>> + allOf:
>>>> + - $ref: /schemas/types.yaml#/definitions/uint32-array
>>>> + minItems: 1
>>>> + # Should be enough
>>>> + maxItems: 255
>>>> +
>>>> +required:
>>>> + - compatible
>>>> + - "#address-cells"
>>>> + - "#size-cells"
>>>> + - "#dma-cells"
>>>> + - reg
>>>> + - reg-names
>>>> + - msi-parent
>>>> + - ti,sci
>>>> + - ti,sci-dev-id
>>>> + - ti,sci-rm-range-bchan
>>>> + - ti,sci-rm-range-tchan
>>>> + - ti,sci-rm-range-rchan
>>>> +
>>>> +additionalProperties: false
>>>> +
>>>> +examples:
>>>> + - |+
>>>> + cbass_main {
>>>> + #address-cells = <2>;
>>>> + #size-cells = <2>;
>>>> +
>>>> + main_dmss {
>>>> + compatible = "simple-mfd";
>>>
>>> IMO, if it is memory-mapped, then you should be using 'simple-bus'.
>>
>> We had the same discussion when I introduced the k3-udma binding and we
>> have concluded on the simple-mfd as DMSS is not a bus, but contains
>> different peripherals.
>
> Ok.
>
> Rob
>

- Péter