dmaengine Archive on lore.kernel.org
 help / color / Atom feed
From: Robin Gong <yibin.gong@nxp.com>
To: Rob Herring <robh@kernel.org>
Cc: "vkoul@kernel.org" <vkoul@kernel.org>,
	"shawnguo@kernel.org" <shawnguo@kernel.org>,
	"s.hauer@pengutronix.de" <s.hauer@pengutronix.de>,
	"festevam@gmail.com" <festevam@gmail.com>,
	"catalin.marinas@arm.com" <catalin.marinas@arm.com>,
	"will@kernel.org" <will@kernel.org>,
	"dan.j.williams@intel.com" <dan.j.williams@intel.com>,
	"angelo@sysam.it" <angelo@sysam.it>,
	"kernel@pengutronix.de" <kernel@pengutronix.de>,
	dl-linux-imx <linux-imx@nxp.com>,
	"dmaengine@vger.kernel.org" <dmaengine@vger.kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>
Subject: RE: [PATCH v2 6/9] dt-bindings: dma: add fsl-edma3 yaml
Date: Fri, 24 Jul 2020 10:13:41 +0000
Message-ID: <VE1PR04MB6638C3006D405D9C8E13547C89770@VE1PR04MB6638.eurprd04.prod.outlook.com> (raw)
In-Reply-To: <20200716194746.GA2716872@bogus>

On 2020/07/17 3:48 Rob Herring <robh@kernel.org> wrote:
> > +
> > +  reg:
> > +    minItems: 2
> > +    maxItems: 32
> 
> Needs some sort of description as to what each region is.
Okay, will add it in v3.
> 
> > +
> > +  interrupts:
> > +    minItems: 2
> > +    maxItems: 32
> 
> ditto
Will add it in v3.
> 
> > +
> > +  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
Here 8 lines matches with the 8 channel used in dts (same example as the
bottom of this yaml file below), otherwise, below warning comes out with
'make dt_binding_check' if I remove the above last line:
dma-controller@5a1f0000: interrupt-names: Additional items are not allowed ('edma2-chan15-tx' was unexpected)

Maybe there something I miss, please correct me if I'm wrong, thanks.

> 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.
There are 3 edma controllers on i.mx8QXP/QM, [0-2] is controller index.
Clear controller index here may make dts readable since every edma channel
has unique register map, interrupt number, power domain.

> 
> 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.
interrupt-names/power-domain-names here just for better readable in dts, besides, the clear interrupt name for every channel including edma controller
id which passed by dts could be used to register irq.

> 
> > +
> > +  '#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
2020-07-24 10:13     ` Robin Gong [this message]
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=VE1PR04MB6638C3006D405D9C8E13547C89770@VE1PR04MB6638.eurprd04.prod.outlook.com \
    --to=yibin.gong@nxp.com \
    --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=robh@kernel.org \
    --cc=s.hauer@pengutronix.de \
    --cc=shawnguo@kernel.org \
    --cc=vkoul@kernel.org \
    --cc=will@kernel.org \
    /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