Re: [PATCH v2 6/9] dt-bindings: dma: add fsl-edma3 yaml

From: Rob Herring
Date: Thu Jul 16 2020 - 15:47:52 EST

Next message: Peter Zijlstra: "Re: [PATCH] x86/split_lock: Enumerate split lock feature on Sapphire Rapids"
Previous message: Stephen Smalley: "Re: [PATCH v2 4/5] LSM: Define SELinux function to measure security state"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Wed, Jul 15, 2020 at 01:41:45AM +0800, Robin Gong wrote:
> Add device binding doc for fsl-edma3 driver.
>
> Signed-off-by: Robin Gong <yibin.gong@xxxxxxx>
> ---
> .../devicetree/bindings/dma/nxp,fsl-edma3.yaml | 134 +++++++++++++++++++++
> 1 file changed, 134 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/dma/nxp,fsl-edma3.yaml
>
> diff --git a/Documentation/devicetree/bindings/dma/nxp,fsl-edma3.yaml b/Documentation/devicetree/bindings/dma/nxp,fsl-edma3.yaml
> new file mode 100644
> index 00000000..ebdad90
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/nxp,fsl-edma3.yaml
> @@ -0,0 +1,134 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/dma/nxp,fsl-edma3.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP eDMA3 Controller
> +
> +maintainers:
> + - Robin Gong <yibin.gong@xxxxxxx>
> +
> +allOf:
> + - $ref: "dma-controller.yaml#"
> +
> +properties:
> + compatible:
> + items:
> + - enum:
> + - fsl,imx8qm-edma # i.mx8qm/qxp
> +
> + reg:
> + minItems: 2
> + maxItems: 32

Needs some sort of description as to what each region is.

> +
> + interrupts:
> + minItems: 2
> + maxItems: 32

ditto

> +
> + interrupt-names:
> + minItems: 2
> + maxItems: 32
> + items:
> + - pattern: "^edma[0-2]-chan([0-9]|[1-2][0-9]|3[0-1])-tx|rx+$"
> + - pattern: "^edma[0-2]-chan([0-9]|[1-2][0-9]|3[0-1])-tx|rx+$"
> + - pattern: "^edma[0-2]-chan([0-9]|[1-2][0-9]|3[0-1])-tx|rx+$"
> + - pattern: "^edma[0-2]-chan([0-9]|[1-2][0-9]|3[0-1])-tx|rx+$"
> + - pattern: "^edma[0-2]-chan([0-9]|[1-2][0-9]|3[0-1])-tx|rx+$"
> + - pattern: "^edma[0-2]-chan([0-9]|[1-2][0-9]|3[0-1])-tx|rx+$"
> + - pattern: "^edma[0-2]-chan([0-9]|[1-2][0-9]|3[0-1])-tx|rx+$"
> + - pattern: "^edma[0-2]-chan([0-9]|[1-2][0-9]|3[0-1])-tx|rx+$"

What about interrupts 8-31? If you want a pattern for all entries, you
do:

items:
pattern: "^edma[0-2]-chan([0-9]|[1-2][0-9]|3[0-1])-tx|rx+$"

(If 'items' is a schema instead of a list, then it applies to all
entries.)

What does edma[0-2] correspond to? The names should be local to the
instance.

Really, what's the point of names that just have the channel number in
them? The driver can create names dynamically if needed. We already have
properties to define how many channels and a mask of present channels.

> +
> + '#dma-cells':
> + const: 3
> + description: |
> + The 1st cell specifies the channel ID.
> +
> + The 2nd cell specifies the channel priority.
> +
> + The 3rd cell specifies the channel attributes:
> + bit0 transmit or receive.
> + 0 = transmit
> + 1 = receive
> + bit1 local or remote access.
> + 0 = local
> + 1 = remote
> + bit2 dualfifo case or not(only in Audio cyclic now).
> + 0 = not dual fifo case
> + 1 = dualfifo case.
> +
> + dma-channels:
> + minimum: 2
> + maximum: 32
> +
> + power-domains:
> + minItems: 2
> + maxItems: 32
> +
> + power-domain-names:
> + minItems: 2
> + maxItems: 32
> + items:
> + - pattern: "^edma[0-2]-chan([0-9]|[1-2][0-9]|3[0-1])+$"
> + - pattern: "^edma[0-2]-chan([0-9]|[1-2][0-9]|3[0-1])+$"
> + - pattern: "^edma[0-2]-chan([0-9]|[1-2][0-9]|3[0-1])+$"
> + - pattern: "^edma[0-2]-chan([0-9]|[1-2][0-9]|3[0-1])+$"
> + - pattern: "^edma[0-2]-chan([0-9]|[1-2][0-9]|3[0-1])+$"
> + - pattern: "^edma[0-2]-chan([0-9]|[1-2][0-9]|3[0-1])+$"
> + - pattern: "^edma[0-2]-chan([0-9]|[1-2][0-9]|3[0-1])+$"
> + - pattern: "^edma[0-2]-chan([0-9]|[1-2][0-9]|3[0-1])+$"

Again, why do you need names here?

> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - interrupt-names
> + - '#dma-cells'
> + - dma-channels
> + - power-domains
> + - power-domain-names
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/firmware/imx/rsrc.h>
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> + edma2: dma-controller@5a1f0000 {
> + compatible = "fsl,imx8qm-edma";
> + reg = <0x5a280000 0x10000>, /* channel8 UART0 rx */
> + <0x5a290000 0x10000>, /* channel9 UART0 tx */
> + <0x5a2a0000 0x10000>, /* channel10 UART1 rx */
> + <0x5a2b0000 0x10000>, /* channel11 UART1 tx */
> + <0x5a2c0000 0x10000>, /* channel12 UART2 rx */
> + <0x5a2d0000 0x10000>, /* channel13 UART2 tx */
> + <0x5a2e0000 0x10000>, /* channel14 UART3 rx */
> + <0x5a2f0000 0x10000>; /* channel15 UART3 tx */
> + #dma-cells = <3>;
> + dma-channels = <8>;
> + interrupts = <GIC_SPI 434 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 435 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 436 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 437 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 438 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 439 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 440 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 441 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "edma2-chan8-rx", "edma2-chan9-tx",
> + "edma2-chan10-rx", "edma2-chan11-tx",
> + "edma2-chan12-rx", "edma2-chan13-tx",
> + "edma2-chan14-rx", "edma2-chan15-tx";
> + power-domains = <&pd IMX_SC_R_DMA_2_CH8>,
> + <&pd IMX_SC_R_DMA_2_CH9>,
> + <&pd IMX_SC_R_DMA_2_CH10>,
> + <&pd IMX_SC_R_DMA_2_CH11>,
> + <&pd IMX_SC_R_DMA_2_CH12>,
> + <&pd IMX_SC_R_DMA_2_CH13>,
> + <&pd IMX_SC_R_DMA_2_CH14>,
> + <&pd IMX_SC_R_DMA_2_CH15>;
> + power-domain-names = "edma2-chan8", "edma2-chan9",
> + "edma2-chan10", "edma2-chan11",
> + "edma2-chan12", "edma2-chan13",
> + "edma2-chan14", "edma2-chan15";
> + };
> --
> 2.7.4
>

Next message: Peter Zijlstra: "Re: [PATCH] x86/split_lock: Enumerate split lock feature on Sapphire Rapids"
Previous message: Stephen Smalley: "Re: [PATCH v2 4/5] LSM: Define SELinux function to measure security state"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]