dmaengine Archive on lore.kernel.org
 help / color / Atom feed
From: Rob Herring <robh@kernel.org>
To: Robin Gong <yibin.gong@nxp.com>
Cc: vkoul@kernel.org, shawnguo@kernel.org, s.hauer@pengutronix.de,
	festevam@gmail.com, catalin.marinas@arm.com, will@kernel.org,
	dan.j.williams@intel.com, angelo@sysam.it, kernel@pengutronix.de,
	linux-imx@nxp.com, dmaengine@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 6/9] dt-bindings: dma: add fsl-edma3 yaml
Date: Thu, 16 Jul 2020 13:47:46 -0600
Message-ID: <20200716194746.GA2716872@bogus> (raw)
In-Reply-To: <1594748508-22179-7-git-send-email-yibin.gong@nxp.com>

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@nxp.com>
> ---
>  .../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@nxp.com>
> +
> +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
> 

  reply index

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-14 17:41 [PATCH v2 0/9] add fsl-edma3 support Robin Gong
2020-07-14 17:41 ` [PATCH v2 1/9] dmaengine: fsl-edma: move edma_request functions into drvdata Robin Gong
2020-07-14 17:41 ` [PATCH v2 2/9] dmaengine: fsl-edma-common: add condition check for fsl_edma_chan_mux Robin Gong
2020-07-14 17:41 ` [PATCH v2 3/9] dmaengine: fsl-edma-common: add fsl_chan into fsl_edma_fill_tcd Robin Gong
2020-07-14 17:41 ` [PATCH v2 4/9] dmaengine: fsl-edma-common: export fsl_edma_set_tcd_regs Robin Gong
2020-07-14 17:41 ` [PATCH v2 5/9] dmaengine: fsl-edma3: add fsl-edma3 driver Robin Gong
2020-07-14 17:41 ` [PATCH v2 6/9] dt-bindings: dma: add fsl-edma3 yaml Robin Gong
2020-07-16 19:47   ` Rob Herring [this message]
2020-07-24 10:13     ` Robin Gong
2020-07-14 17:41 ` [PATCH v2 7/9] firmware: imx: scu-pd: correct dma resource Robin Gong
2020-07-14 17:41 ` [PATCH v2 8/9] arm64: dts: imx8qxp: add edma2 Robin Gong
2020-07-14 17:41 ` [PATCH v2 9/9] arm64: defconfig: add CONFIG_FSL_EDMA3 Robin Gong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200716194746.GA2716872@bogus \
    --to=robh@kernel.org \
    --cc=angelo@sysam.it \
    --cc=catalin.marinas@arm.com \
    --cc=dan.j.williams@intel.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dmaengine@vger.kernel.org \
    --cc=festevam@gmail.com \
    --cc=kernel@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-imx@nxp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=s.hauer@pengutronix.de \
    --cc=shawnguo@kernel.org \
    --cc=vkoul@kernel.org \
    --cc=will@kernel.org \
    --cc=yibin.gong@nxp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

dmaengine Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/dmaengine/0 dmaengine/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dmaengine dmaengine/ https://lore.kernel.org/dmaengine \
		dmaengine@vger.kernel.org
	public-inbox-index dmaengine

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.dmaengine


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git