diff mbox series

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

Message ID 20200930091412.8020-10-peter.ujfalusi@ti.com (mailing list archive)
State Changes Requested
Headers show
Series dmaengine/soc: k3-udma: Add support for BCDMA and PKTDMA | expand

Commit Message

Peter Ujfalusi Sept. 30, 2020, 9:14 a.m. UTC
New binding document for
Texas Instruments K3 Block Copy DMA (BCDMA).

BCDMA is introduced as part of AM64.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 .../devicetree/bindings/dma/ti/k3-bcdma.yaml  | 183 ++++++++++++++++++
 1 file changed, 183 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml

Comments

Peter Ujfalusi Oct. 1, 2020, 6:49 a.m. UTC | #1
Hi Rob,

On 30/09/2020 12.14, Peter Ujfalusi wrote:
> New binding document for
> Texas Instruments K3 Block Copy DMA (BCDMA).
> 
> BCDMA is introduced as part of AM64.
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> ---
>  .../devicetree/bindings/dma/ti/k3-bcdma.yaml  | 183 ++++++++++++++++++
>  1 file changed, 183 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml
> 
> diff --git a/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml b/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml
> new file mode 100644
> index 000000000000..c84fb641738f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml
> @@ -0,0 +1,183 @@

...

> +  compatible:
> +    enum:
> +      - ti,am64-dmss-bcdma

Would it be OK if I use ti,am64x-dmss-bcdma or should I stick with
am64-dmss-bcdma.
The TRM refers to the family as AM64x, but having the 'x' in the
compatible did not sounded right.

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Rob Herring (Arm) Oct. 6, 2020, 7:23 p.m. UTC | #2
On Thu, Oct 01, 2020 at 09:49:43AM +0300, Peter Ujfalusi wrote:
> Hi Rob,
> 
> On 30/09/2020 12.14, Peter Ujfalusi wrote:
> > New binding document for
> > Texas Instruments K3 Block Copy DMA (BCDMA).
> > 
> > BCDMA is introduced as part of AM64.
> > 
> > Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> > ---
> >  .../devicetree/bindings/dma/ti/k3-bcdma.yaml  | 183 ++++++++++++++++++
> >  1 file changed, 183 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml b/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml
> > new file mode 100644
> > index 000000000000..c84fb641738f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml
> > @@ -0,0 +1,183 @@
> 
> ...
> 
> > +  compatible:
> > +    enum:
> > +      - ti,am64-dmss-bcdma
> 
> Would it be OK if I use ti,am64x-dmss-bcdma or should I stick with
> am64-dmss-bcdma.

'ti,am654.*' was used pretty consistently, is this family different?

> The TRM refers to the family as AM64x, but having the 'x' in the
> compatible did not sounded right.

We generally don't want wildcards, but if the last digit is just pinout 
or fusing differences, then it's fine IMO.

Bottomline, just be consistent across all the compatible strings for 
this SoC.

Rob
Rob Herring (Arm) Oct. 6, 2020, 7:29 p.m. UTC | #3
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.
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> ---
>  .../devicetree/bindings/dma/ti/k3-bcdma.yaml  | 183 ++++++++++++++++++
>  1 file changed, 183 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml
> 
> diff --git a/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml b/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml
> new file mode 100644
> index 000000000000..c84fb641738f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml
> @@ -0,0 +1,183 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/dma/ti/k3-bcdma.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Texas Instruments K3 DMSS BCDMA Device Tree Bindings
> +
> +maintainers:
> +  - Peter Ujfalusi <peter.ujfalusi@ti.com>
> +
> +description: |
> +  The Block Copy DMA (BCDMA) is intended to perform similar functions as the TR
> +  mode channels of K3 UDMA-P.
> +  BCDMA includes block copy channels and Split channels.
> +
> +  Block copy channels mainly used for memory to memory transfers, but with
> +  optional triggers a block copy channel can service peripherals by accessing
> +  directly to memory mapped registers or area.
> +
> +  Split channels can be used to service PSI-L based peripherals.
> +  The peripherals can be PSI-L native or legacy, non PSI-L native peripherals
> +  with PDMAs. PDMA is tasked to act as a bridge between the PSI-L fabric and the
> +  legacy peripheral.
> +
> +  PDMAs can be configured via BCDMA split channel's peer registers to match with
> +  the configuration of the legacy peripheral.
> +
> +allOf:
> +  - $ref: /schemas/dma/dma-controller.yaml#
> +
> +properties:
> +  "#dma-cells":
> +    const: 3
> +    description: |
> +      cell 1: type of the BCDMA channel to be used to service the peripheral:
> +        0 - split channel
> +        1 - block copy channel using global trigger 1
> +        2 - block copy channel using global trigger 2
> +        3 - block copy channel using local trigger
> +
> +      cell 2: parameter for the channel:
> +        if cell 1 is 0 (split channel):
> +          PSI-L thread ID of the remote (to BCDMA) end.
> +          Valid ranges for thread ID depends on the data movement direction:
> +          for source thread IDs (rx): 0 - 0x7fff
> +          for destination thread IDs (tx): 0x8000 - 0xffff
> +
> +          Please refer to the device documentation for the PSI-L thread map and
> +          also the PSI-L peripheral chapter for the correct thread ID.
> +        if cell 1 is 1 or 2 (block copy channel using global trigger):
> +          Unused, ignored
> +
> +          The trigger must be configured for the channel externally to BCDMA,
> +          channels using global triggers should not be requested directly, but
> +          via DMA event router.
> +        if cell 1 is 3 (block copy channel using local trigger):
> +          bchan number of the locally triggered channel
> +
> +      cell 3: ASEL value for the channel
> +
> +  compatible:
> +    enum:
> +      - ti,am64-dmss-bcdma
> +
> +  "#address-cells":
> +    const: 2
> +
> +  "#size-cells":
> +    const: 2
> +
> +  reg:
> +    maxItems: 5
> +
> +  reg-names:
> +   items:
> +     - const: gcfg
> +     - const: bchanrt
> +     - const: rchanrt
> +     - const: tchanrt
> +     - const: ringrt
> +
> +  msi-parent: true
> +

> +  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.

> +
> +  ti,asel:
> +    description: ASEL value for non slave channels
> +    allOf:

You no longer need 'allOf' here.

> +      - $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?

> +
> +  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'.

> +            #address-cells = <2>;
> +            #size-cells = <2>;
> +            dma-ranges;
> +            ranges;
> +
> +            ti,sci-dev-id = <25>;
> +
> +            main_bcdma: dma-controller@485c0100 {
> +                compatible = "ti,am64-dmss-bcdma";
> +                #address-cells = <2>;
> +                #size-cells = <2>;
> +
> +                reg = <0x0 0x485c0100 0x0 0x100>,
> +                      <0x0 0x4c000000 0x0 0x20000>,
> +                      <0x0 0x4a820000 0x0 0x20000>,
> +                      <0x0 0x4aa40000 0x0 0x20000>,
> +                      <0x0 0x4bc00000 0x0 0x100000>;
> +                reg-names = "gcfg", "bchanrt", "rchanrt", "tchanrt", "ringrt";
> +                msi-parent = <&inta_main_dmss>;
> +                #dma-cells = <3>;
> +
> +                ti,sci = <&dmsc>;
> +                ti,sci-dev-id = <26>;
> +
> +                ti,sci-rm-range-bchan = <0x20>; /* BLOCK_COPY_CHAN */
> +                ti,sci-rm-range-rchan = <0x21>; /* SPLIT_TR_RX_CHAN */
> +                ti,sci-rm-range-tchan = <0x22>; /* SPLIT_TR_TX_CHAN */
> +            };
> +        };
> +    };
> -- 
> Peter
> 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>
Peter Ujfalusi Oct. 7, 2020, 9:09 a.m. UTC | #4
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.

> 
>> +
>> +  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?

>> +
>> +  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.

> 
>> +            #address-cells = <2>;
>> +            #size-cells = <2>;
>> +            dma-ranges;
>> +            ranges;
>> +
>> +            ti,sci-dev-id = <25>;
>> +
>> +            main_bcdma: dma-controller@485c0100 {
>> +                compatible = "ti,am64-dmss-bcdma";
>> +                #address-cells = <2>;
>> +                #size-cells = <2>;
>> +
>> +                reg = <0x0 0x485c0100 0x0 0x100>,
>> +                      <0x0 0x4c000000 0x0 0x20000>,
>> +                      <0x0 0x4a820000 0x0 0x20000>,
>> +                      <0x0 0x4aa40000 0x0 0x20000>,
>> +                      <0x0 0x4bc00000 0x0 0x100000>;
>> +                reg-names = "gcfg", "bchanrt", "rchanrt", "tchanrt", "ringrt";
>> +                msi-parent = <&inta_main_dmss>;
>> +                #dma-cells = <3>;
>> +
>> +                ti,sci = <&dmsc>;
>> +                ti,sci-dev-id = <26>;
>> +
>> +                ti,sci-rm-range-bchan = <0x20>; /* BLOCK_COPY_CHAN */
>> +                ti,sci-rm-range-rchan = <0x21>; /* SPLIT_TR_RX_CHAN */
>> +                ti,sci-rm-range-tchan = <0x22>; /* SPLIT_TR_TX_CHAN */
>> +            };
>> +        };
>> +    };
>> -- 
>> Peter
>>
>> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
>> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>>

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Rob Herring (Arm) Oct. 7, 2020, 3:46 p.m. UTC | #5
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.


> >> +  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

> 
> >> +
> >> +  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
Peter Ujfalusi Oct. 8, 2020, 8:40 a.m. UTC | #6
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

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Rob Herring (Arm) Oct. 8, 2020, 7:15 p.m. UTC | #7
On Thu, Oct 8, 2020 at 3:40 AM Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:
>
>
>
> 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

Yes. I meant unevaluatedProperties instead of additionalProperties.

> 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.

> >>>> +  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.

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

Rob
Peter Ujfalusi Oct. 9, 2020, 8:06 a.m. UTC | #8
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

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

Patch

diff --git a/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml b/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml
new file mode 100644
index 000000000000..c84fb641738f
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml
@@ -0,0 +1,183 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/dma/ti/k3-bcdma.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments K3 DMSS BCDMA Device Tree Bindings
+
+maintainers:
+  - Peter Ujfalusi <peter.ujfalusi@ti.com>
+
+description: |
+  The Block Copy DMA (BCDMA) is intended to perform similar functions as the TR
+  mode channels of K3 UDMA-P.
+  BCDMA includes block copy channels and Split channels.
+
+  Block copy channels mainly used for memory to memory transfers, but with
+  optional triggers a block copy channel can service peripherals by accessing
+  directly to memory mapped registers or area.
+
+  Split channels can be used to service PSI-L based peripherals.
+  The peripherals can be PSI-L native or legacy, non PSI-L native peripherals
+  with PDMAs. PDMA is tasked to act as a bridge between the PSI-L fabric and the
+  legacy peripheral.
+
+  PDMAs can be configured via BCDMA split channel's peer registers to match with
+  the configuration of the legacy peripheral.
+
+allOf:
+  - $ref: /schemas/dma/dma-controller.yaml#
+
+properties:
+  "#dma-cells":
+    const: 3
+    description: |
+      cell 1: type of the BCDMA channel to be used to service the peripheral:
+        0 - split channel
+        1 - block copy channel using global trigger 1
+        2 - block copy channel using global trigger 2
+        3 - block copy channel using local trigger
+
+      cell 2: parameter for the channel:
+        if cell 1 is 0 (split channel):
+          PSI-L thread ID of the remote (to BCDMA) end.
+          Valid ranges for thread ID depends on the data movement direction:
+          for source thread IDs (rx): 0 - 0x7fff
+          for destination thread IDs (tx): 0x8000 - 0xffff
+
+          Please refer to the device documentation for the PSI-L thread map and
+          also the PSI-L peripheral chapter for the correct thread ID.
+        if cell 1 is 1 or 2 (block copy channel using global trigger):
+          Unused, ignored
+
+          The trigger must be configured for the channel externally to BCDMA,
+          channels using global triggers should not be requested directly, but
+          via DMA event router.
+        if cell 1 is 3 (block copy channel using local trigger):
+          bchan number of the locally triggered channel
+
+      cell 3: ASEL value for the channel
+
+  compatible:
+    enum:
+      - ti,am64-dmss-bcdma
+
+  "#address-cells":
+    const: 2
+
+  "#size-cells":
+    const: 2
+
+  reg:
+    maxItems: 5
+
+  reg-names:
+   items:
+     - const: gcfg
+     - const: bchanrt
+     - const: rchanrt
+     - const: tchanrt
+     - const: ringrt
+
+  msi-parent: true
+
+  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
+
+  ti,asel:
+    description: ASEL value for non slave channels
+    allOf:
+      - $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
+
+  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";
+            #address-cells = <2>;
+            #size-cells = <2>;
+            dma-ranges;
+            ranges;
+
+            ti,sci-dev-id = <25>;
+
+            main_bcdma: dma-controller@485c0100 {
+                compatible = "ti,am64-dmss-bcdma";
+                #address-cells = <2>;
+                #size-cells = <2>;
+
+                reg = <0x0 0x485c0100 0x0 0x100>,
+                      <0x0 0x4c000000 0x0 0x20000>,
+                      <0x0 0x4a820000 0x0 0x20000>,
+                      <0x0 0x4aa40000 0x0 0x20000>,
+                      <0x0 0x4bc00000 0x0 0x100000>;
+                reg-names = "gcfg", "bchanrt", "rchanrt", "tchanrt", "ringrt";
+                msi-parent = <&inta_main_dmss>;
+                #dma-cells = <3>;
+
+                ti,sci = <&dmsc>;
+                ti,sci-dev-id = <26>;
+
+                ti,sci-rm-range-bchan = <0x20>; /* BLOCK_COPY_CHAN */
+                ti,sci-rm-range-rchan = <0x21>; /* SPLIT_TR_RX_CHAN */
+                ti,sci-rm-range-tchan = <0x22>; /* SPLIT_TR_TX_CHAN */
+            };
+        };
+    };