[v1,5/9] dt-bindings: dmaengine: convert Actions Semi Owl SoCs bindings to yaml

Converts the device tree bindings for the Actions Semi Owl SoCs DMA
Controller over to YAML schemas.

It also adds new compatible string "actions,s700-dma" to match
the driver.

Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
---
New patch, was not there in RFC.
---
 Documentation/devicetree/bindings/dma/owl-dma.txt  | 47 ------------
 Documentation/devicetree/bindings/dma/owl-dma.yaml | 84 ++++++++++++++++++++++
 2 files changed, 84 insertions(+), 47 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/dma/owl-dma.txt
 create mode 100644 Documentation/devicetree/bindings/dma/owl-dma.yaml

On 14/05/2020 17:10, Amit Singh Tomar wrote:

Hi,

> Converts the device tree bindings for the Actions Semi Owl SoCs DMA
> Controller over to YAML schemas.
> 
> It also adds new compatible string "actions,s700-dma" to match
> the driver.
> 
> Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
> ---
> New patch, was not there in RFC.
> ---
>  Documentation/devicetree/bindings/dma/owl-dma.txt  | 47 ------------
>  Documentation/devicetree/bindings/dma/owl-dma.yaml | 84 ++++++++++++++++++++++
>  2 files changed, 84 insertions(+), 47 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/dma/owl-dma.txt
>  create mode 100644 Documentation/devicetree/bindings/dma/owl-dma.yaml
> 
> diff --git a/Documentation/devicetree/bindings/dma/owl-dma.txt b/Documentation/devicetree/bindings/dma/owl-dma.txt
> deleted file mode 100644
> index 03e9bb12b75f..000000000000
> --- a/Documentation/devicetree/bindings/dma/owl-dma.txt
> +++ /dev/null
> @@ -1,47 +0,0 @@
> -* Actions Semi Owl SoCs DMA controller
> -
> -This binding follows the generic DMA bindings defined in dma.txt.
> -
> -Required properties:
> -- compatible: Should be "actions,s900-dma".
> -- reg: Should contain DMA registers location and length.
> -- interrupts: Should contain 4 interrupts shared by all channel.
> -- #dma-cells: Must be <1>. Used to represent the number of integer
> -              cells in the dmas property of client device.
> -- dma-channels: Physical channels supported.
> -- dma-requests: Number of DMA request signals supported by the controller.
> -                Refer to Documentation/devicetree/bindings/dma/dma.txt
> -- clocks: Phandle and Specifier of the clock feeding the DMA controller.
> -
> -Example:
> -
> -Controller:
> -                dma: dma-controller@e0260000 {
> -                        compatible = "actions,s900-dma";
> -                        reg = <0x0 0xe0260000 0x0 0x1000>;
> -                        interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>,
> -                                     <GIC_SPI 58 IRQ_TYPE_LEVEL_HIGH>,
> -                                     <GIC_SPI 59 IRQ_TYPE_LEVEL_HIGH>,
> -                                     <GIC_SPI 60 IRQ_TYPE_LEVEL_HIGH>;
> -                        #dma-cells = <1>;
> -                        dma-channels = <12>;
> -                        dma-requests = <46>;
> -                        clocks = <&clock CLK_DMAC>;
> -                };
> -
> -Client:
> -
> -DMA clients connected to the Actions Semi Owl SoCs DMA controller must
> -use the format described in the dma.txt file, using a two-cell specifier
> -for each channel.
> -
> -The two cells in order are:
> -1. A phandle pointing to the DMA controller.
> -2. The channel id.
> -
> -uart5: serial@e012a000 {
> -        ...
> -        dma-names = "tx", "rx";
> -        dmas = <&dma 26>, <&dma 27>;
> -        ...
> -};
> diff --git a/Documentation/devicetree/bindings/dma/owl-dma.yaml b/Documentation/devicetree/bindings/dma/owl-dma.yaml
> new file mode 100644
> index 000000000000..12e68c0ece67
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/owl-dma.yaml
> @@ -0,0 +1,84 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/dma/owl-dma.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Actions Semi Owl SoCs DMA controller
> +
> +description: |
> +  The OWL DMA is a general-purpose direct memory access controller capable of
> +  supporting 10 and 12 independent DMA channels for S700 and S900 SoCs
> +  respectively.

From here on the description is pretty much how DMA controller
references work in general, so I don't see the point of including this
in the description here.

> +  DMA clients connected to the Actions Semi Owl SoCs DMA controller must
> +  use the format described in the owl-dma.yaml file, using a two-cell specifier
> +  for each channel.
> +
> +  The two cells in order are:
> +  1. A phandle pointing to the DMA controller.
> +  2. The channel id.
> +
> +  uart5: serial@e012a000 {
> +    ...
> +    dma-names = "tx", "rx";
> +    dmas = <&dma 26>, <&dma 27>;
> +    ...
> +  };
> +
> +maintainers:
> +  - Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> +
> +allOf:
> +  - $ref: "dma-controller.yaml#"
> +
> +properties:
> +  compatible:
> +    enum:
> +      - actions,s900-dma
> +      - actions,s700-dma
> +
> +  reg:
> +    maxItems: 1

Could you replace those "maxItems: 1" here and below with:
  - description: ...., copying in the explanation from the .txt binding?
That should serve the same purpose as "maxItems: 1", but is more
descriptive.

> +
> +  interrupts:
> +    maxItems: 4

Please mention that the controller supports 4 interrupts, which are
freely assignable to the interrupt channels.
(The kernel chose to use only one, but that's nothing the binding is
concerned about).

> +
> +  "#dma-cells":
> +    const: 1
> +
> +  dma-channels:
> +    maxItems: 1
> +
> +  dma-requests:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - "#dma-cells"
> +  - dma-channels
> +  - dma-requests
> +  - clocks
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/actions,s700-cmu.h>

I would drop this line and replace CLK_DMAC with some number below, to
keep this *example* as independent as possible.

Cheers,
Andre

> +    dma: dma-controller@e0260000 {
> +        compatible = "actions,s900-dma";
> +        reg = <0x0 0xe0260000 0x0 0x1000>;
> +        interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>,
> +                     <GIC_SPI 58 IRQ_TYPE_LEVEL_HIGH>,
> +                     <GIC_SPI 59 IRQ_TYPE_LEVEL_HIGH>,
> +                     <GIC_SPI 60 IRQ_TYPE_LEVEL_HIGH>;
> +        #dma-cells = <1>;
> +        dma-channels = <12>;
> +        dma-requests = <46>;
> +        clocks = <&clock CLK_DMAC>;
> +    };
> +
> +...
>

Hi,

> Could you replace those "maxItems: 1" here and below with:
>   - description: ...., copying in the explanation from the .txt binding?
> That should serve the same purpose as "maxItems: 1", but is more
> descriptive.

But having it under reg: looks bit odd to me, no?

reg:
   - description: ...

Or did you mean something else ?

Thanks
-Amit

> But having it under reg: looks bit odd to me, no?
>
> reg:
>    - description: ...
>
> Or did you mean something else ?

Ah, I see you wanted to have description for individual items, for instance one
for reg, one for interrupts and clock etc..

Thanks
Amit

Hi,

> (The kernel chose to use only one, but that's nothing the binding is
> concerned about).
But there are four different interrupt events corresponds to these four
interrupts.
So, if Kernel chooses to have only one interrupt , how other events
would be notified ?

Thanks
-Amit