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 [thread overview]
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
> >
WARNING: multiple messages have this Message-ID (diff)
From: Robin Gong <yibin.gong@nxp.com>
To: Rob Herring <robh@kernel.org>
Cc: "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
"will@kernel.org" <will@kernel.org>,
"angelo@sysam.it" <angelo@sysam.it>,
"shawnguo@kernel.org" <shawnguo@kernel.org>,
"s.hauer@pengutronix.de" <s.hauer@pengutronix.de>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"vkoul@kernel.org" <vkoul@kernel.org>,
dl-linux-imx <linux-imx@nxp.com>,
"kernel@pengutronix.de" <kernel@pengutronix.de>,
"catalin.marinas@arm.com" <catalin.marinas@arm.com>,
"dmaengine@vger.kernel.org" <dmaengine@vger.kernel.org>,
"dan.j.williams@intel.com" <dan.j.williams@intel.com>,
"festevam@gmail.com" <festevam@gmail.com>,
"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 [thread overview]
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
> >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-07-24 10:13 UTC|newest]
Thread overview: 24+ 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 ` 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 ` 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 ` 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 ` 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 ` 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 ` Robin Gong
2020-07-14 17:41 ` [PATCH v2 6/9] dt-bindings: dma: add fsl-edma3 yaml Robin Gong
2020-07-14 17:41 ` Robin Gong
2020-07-16 19:47 ` Rob Herring
2020-07-16 19:47 ` Rob Herring
2020-07-24 10:13 ` Robin Gong [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 ` Robin Gong
2020-07-14 17:41 ` [PATCH v2 8/9] arm64: dts: imx8qxp: add edma2 Robin Gong
2020-07-14 17:41 ` Robin Gong
2020-07-14 17:41 ` [PATCH v2 9/9] arm64: defconfig: add CONFIG_FSL_EDMA3 Robin Gong
2020-07-14 17:41 ` 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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.