diff mbox series

[2/2] arm64: dts: imx8qxp: Add EDMA0/EDMA1 nodes

Message ID 20190326094239.5910-3-daniel.baluta@nxp.com (mailing list archive)
State New, archived
Headers show
Series Add EDMA0/EDMA1 nodes | expand

Commit Message

Daniel Baluta March 26, 2019, 9:43 a.m. UTC
i.MX8QXP contains a total of 4 EDMA controllers of which
two are primarily for audio components and the other two
are for non-audio periperhals.

This patch adds the EDMA0/EDMA1 nodes used by audio peripherals.

EDMA0 contains channels for:
	* ASRC0
	* ESAI0
	* SPDIF0
	* SAI0, SAI1, SAI2, SAI3

EDMA1 contains channels for:
	* ASRC1
	* SAI4, SAI5

See chapter Audio DMA Memory Maps (2.2.3) from i.MX8QXP RM [1]

This patch is based on the dtsi file initially submitted by
Teo Hall in i.MX NXP internal tree.

[1] https://www.nxp.com/docs/en/reference-manual/IMX8DQXPRM.pdf

Cc: Teo Hall <teo.hall@nxp.com>
Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 72 ++++++++++++++++++++++
 1 file changed, 72 insertions(+)

Comments

Dong Aisheng March 26, 2019, 1:30 p.m. UTC | #1
> From: Daniel Baluta
> Sent: Tuesday, March 26, 2019 5:43 PM
> 
> i.MX8QXP contains a total of 4 EDMA controllers of which two are primarily
> for audio components and the other two are for non-audio periperhals.
> 
> This patch adds the EDMA0/EDMA1 nodes used by audio peripherals.
> 
> EDMA0 contains channels for:
> 	* ASRC0
> 	* ESAI0
> 	* SPDIF0
> 	* SAI0, SAI1, SAI2, SAI3
> 
> EDMA1 contains channels for:
> 	* ASRC1
> 	* SAI4, SAI5
> 
> See chapter Audio DMA Memory Maps (2.2.3) from i.MX8QXP RM [1]
> 
> This patch is based on the dtsi file initially submitted by Teo Hall in i.MX NXP
> internal tree.
> 
> [1] https://www.nxp.com/docs/en/reference-manual/IMX8DQXPRM.pdf
> 
> Cc: Teo Hall <teo.hall@nxp.com>
> Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
> ---
>  arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 72
> ++++++++++++++++++++++
>  1 file changed, 72 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> index 0cb939861a60..9e959deb2499 100644
> --- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> @@ -182,6 +182,78 @@
>  			#clock-cells = <1>;
>  		};
> 
> +		edma0: dma-controller@591f0000 {
> +			compatible = "fsl,imx8qm-edma";

Should this be "fsl,imx8qxp-edma"?
or
"fsl,imx8qxp-edma", "fsl,imx8qm-edma"?

Regards
Dong Aisheng

> +			reg = <0x59200000 0x10000>,   /* asrc0 pair A input req */
> +				<0x59210000 0x10000>, /* asrc0 pair B input req */
> +				<0x59220000 0x10000>, /* asrc0 pair C input req */
> +				<0x59230000 0x10000>, /* asrc0 pair A output req */
> +				<0x59240000 0x10000>, /* asrc0 pair B output req */
> +				<0x59250000 0x10000>, /* asrc0 pair C output req */
> +				<0x59260000 0x10000>, /* esai0 rx */
> +				<0x59270000 0x10000>, /* esai0 tx */
> +				<0x59280000 0x10000>, /* spdif0 rx */
> +				<0x59290000 0x10000>, /* spdif0 tx */
> +				<0x592c0000 0x10000>, /* sai0 rx */
> +				<0x592d0000 0x10000>, /* sai0 tx */
> +				<0x592e0000 0x10000>, /* sai1 rx */
> +				<0x592f0000 0x10000>, /* sai1 tx */
> +			#dma-cells = <3>;
> +			shared-interrupt;
> +			dma-channels = <14>;
> +			interrupts = <GIC_SPI 374 IRQ_TYPE_LEVEL_HIGH>,    /* asrc 0
> */
> +					<GIC_SPI 375 IRQ_TYPE_LEVEL_HIGH>,
> +					<GIC_SPI 376 IRQ_TYPE_LEVEL_HIGH>,
> +					<GIC_SPI 377 IRQ_TYPE_LEVEL_HIGH>,
> +					<GIC_SPI 378 IRQ_TYPE_LEVEL_HIGH>,
> +					<GIC_SPI 379 IRQ_TYPE_LEVEL_HIGH>,
> +					<GIC_SPI 410 IRQ_TYPE_LEVEL_HIGH>, /* esai0 */
> +					<GIC_SPI 410 IRQ_TYPE_LEVEL_HIGH>,
> +					<GIC_SPI 457 IRQ_TYPE_LEVEL_HIGH>, /* spdif0 */
> +					<GIC_SPI 459 IRQ_TYPE_LEVEL_HIGH>,
> +					<GIC_SPI 315 IRQ_TYPE_LEVEL_HIGH>, /* sai0 */
> +					<GIC_SPI 315 IRQ_TYPE_LEVEL_HIGH>,
> +					<GIC_SPI 317 IRQ_TYPE_LEVEL_HIGH>, /* sai1 */
> +					<GIC_SPI 317 IRQ_TYPE_LEVEL_HIGH>,
> +			interrupt-names = "edma0-chan0-rx", "edma0-chan1-rx", /*
> asrc0 */
> +					"edma0-chan2-rx", "edma0-chan3-tx",
> +					"edma0-chan4-tx", "edma0-chan5-tx",
> +					"edma0-chan6-rx", "edma0-chan7-tx",   /* esai0 */
> +					"edma0-chan8-rx", "edma0-chan9-tx",   /* spdif0 */
> +					"edma0-chan12-rx", "edma0-chan13-tx", /* sai0 */
> +					"edma0-chan14-rx", "edma0-chan15-tx", /* sai1 */
> +		};
> +
> +		edma1: dma-controller@599F0000 {
> +			compatible = "fsl,imx8qm-edma";
> +			reg = <0x0 0x59A00000 0x0 0x10000>, /* asrc1 */
> +				<0x0 0x59A10000 0x0 0x10000>,
> +				<0x0 0x59A20000 0x0 0x10000>,
> +				<0x0 0x59A30000 0x0 0x10000>,
> +				<0x0 0x59A40000 0x0 0x10000>,
> +				<0x0 0x59A50000 0x0 0x10000>,
> +				<0x0 0x59A80000 0x0 0x10000>, /* sai4 rx */
> +				<0x0 0x59A90000 0x0 0x10000>, /* sai4 tx */
> +				<0x0 0x59AA0000 0x0 0x10000>; /* sai5 tx */
> +			#dma-cells = <3>;
> +			shared-interrupt;
> +			dma-channels = <9>;
> +			interrupts = <GIC_SPI 382 IRQ_TYPE_LEVEL_HIGH>, /* asrc 1 */
> +					<GIC_SPI 383 IRQ_TYPE_LEVEL_HIGH>,
> +					<GIC_SPI 384 IRQ_TYPE_LEVEL_HIGH>,
> +					<GIC_SPI 385 IRQ_TYPE_LEVEL_HIGH>,
> +					<GIC_SPI 386 IRQ_TYPE_LEVEL_HIGH>,
> +					<GIC_SPI 387 IRQ_TYPE_LEVEL_HIGH>,
> +					<GIC_SPI 330 IRQ_TYPE_LEVEL_HIGH>, /* sai4 */
> +					<GIC_SPI 330 IRQ_TYPE_LEVEL_HIGH>,
> +					<GIC_SPI 332 IRQ_TYPE_LEVEL_HIGH>; /* sai5 */
> +			interrupt-names = "edma1-chan0-rx", "edma1-chan1-rx", /*
> asrc1 */
> +					"edma1-chan2-rx", "edma1-chan3-tx",
> +					"edma1-chan4-tx", "edma1-chan5-tx",
> +					"edma1-chan8-rx", "edma1-chan9-tx", /* sai4 */
> +					"edma1-chan10-tx"; /* sai5 */
> +		};
> +
>  		adma_lpuart0: serial@5a060000 {
>  			compatible = "fsl,imx8qxp-lpuart", "fsl,imx7ulp-lpuart";
>  			reg = <0x5a060000 0x1000>;
> --
> 2.17.1
Daniel Baluta March 27, 2019, 8:51 a.m. UTC | #2
On Tue, Mar 26, 2019 at 3:31 PM Aisheng Dong <aisheng.dong@nxp.com> wrote:
>
> > From: Daniel Baluta
> > Sent: Tuesday, March 26, 2019 5:43 PM
> >
> > i.MX8QXP contains a total of 4 EDMA controllers of which two are primarily
> > for audio components and the other two are for non-audio periperhals.
> >
> > This patch adds the EDMA0/EDMA1 nodes used by audio peripherals.
> >
> > EDMA0 contains channels for:
> >       * ASRC0
> >       * ESAI0
> >       * SPDIF0
> >       * SAI0, SAI1, SAI2, SAI3
> >
> > EDMA1 contains channels for:
> >       * ASRC1
> >       * SAI4, SAI5
> >
> > See chapter Audio DMA Memory Maps (2.2.3) from i.MX8QXP RM [1]
> >
> > This patch is based on the dtsi file initially submitted by Teo Hall in i.MX NXP
> > internal tree.
> >
> > [1] https://www.nxp.com/docs/en/reference-manual/IMX8DQXPRM.pdf
> >
> > Cc: Teo Hall <teo.hall@nxp.com>
> > Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
> > ---
> >  arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 72
> > ++++++++++++++++++++++
> >  1 file changed, 72 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > index 0cb939861a60..9e959deb2499 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > @@ -182,6 +182,78 @@
> >                       #clock-cells = <1>;
> >               };
> >
> > +             edma0: dma-controller@591f0000 {
> > +                     compatible = "fsl,imx8qm-edma";

Thanks for the comment Aisheng!

>
> Should this be "fsl,imx8qxp-edma"?

Yes, I think that's the more clear approach although in
our internal tree the edma driver uses only the "fsl,imx8qm-edma
compatible.

I will go with your suggestion.


> or
> "fsl,imx8qxp-edma", "fsl,imx8qm-edma"?

One thing that it is not clear for me is why there are places
where we use two compatible strings?

I understand the situation where are two distinct drivers, but is there
any other reason to add multiple compatible strings for a node in dts?

thanks,
Daniel.
Lucas Stach March 27, 2019, 9:33 a.m. UTC | #3
Hi Daniel,

Am Mittwoch, den 27.03.2019, 10:51 +0200 schrieb Daniel Baluta:
[...]
> 
> > or
> > "fsl,imx8qxp-edma", "fsl,imx8qm-edma"?
> 
> One thing that it is not clear for me is why there are places
> where we use two compatible strings?
> 
> I understand the situation where are two distinct drivers, but is there
> any other reason to add multiple compatible strings for a node in dts?

We use 2 compatible string where there should not be any differences
between the IP blocks of this SoC and a version the driver already
supports.

So if the eDMA driver already supports the software interface for
"fsl,imx8qm-edma" and the IP block is compatible with this, we add this
to the DT, so the we don't need any driver changes just to support a
new SoC. But as you can never be sure if there are subtle differences
in the IP block and/or SOC integration when adding the DT support, we
also add a more specific compatible to the DT. If it turns out that
there are software visible differences, we only need to adapt the
driver to check for the more specific compatible to trigger the changed
behavior, allowing to keep the DT stable.

Regards,
Lucas
Daniel Baluta March 27, 2019, 4:36 p.m. UTC | #4
On Wed, Mar 27, 2019 at 11:33 AM Lucas Stach <l.stach@pengutronix.de> wrote:
>
> Hi Daniel,
>
> Am Mittwoch, den 27.03.2019, 10:51 +0200 schrieb Daniel Baluta:
> [...]
> >
> > > or
> > > "fsl,imx8qxp-edma", "fsl,imx8qm-edma"?
> >
> > One thing that it is not clear for me is why there are places
> > where we use two compatible strings?
> >
> > I understand the situation where are two distinct drivers, but is there
> > any other reason to add multiple compatible strings for a node in dts?
>
> We use 2 compatible string where there should not be any differences
> between the IP blocks of this SoC and a version the driver already
> supports.
>
> So if the eDMA driver already supports the software interface for
> "fsl,imx8qm-edma" and the IP block is compatible with this, we add this
> to the DT, so the we don't need any driver changes just to support a
> new SoC. But as you can never be sure if there are subtle differences
> in the IP block and/or SOC integration when adding the DT support, we
> also add a more specific compatible to the DT. If it turns out that
> there are software visible differences, we only need to adapt the
> driver to check for the more specific compatible to trigger the changed
> behavior, allowing to keep the DT stable.

Excellent explanation Lucas! For the moment upstream eDMA driver doesn't have
support for any i.MX8 so I think using "fsl,imx8qxp-edma" should be acceptable.

Thanks,
Daniel.
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
index 0cb939861a60..9e959deb2499 100644
--- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
@@ -182,6 +182,78 @@ 
 			#clock-cells = <1>;
 		};
 
+		edma0: dma-controller@591f0000 {
+			compatible = "fsl,imx8qm-edma";
+			reg = <0x59200000 0x10000>,   /* asrc0 pair A input req */
+				<0x59210000 0x10000>, /* asrc0 pair B input req */
+				<0x59220000 0x10000>, /* asrc0 pair C input req */
+				<0x59230000 0x10000>, /* asrc0 pair A output req */
+				<0x59240000 0x10000>, /* asrc0 pair B output req */
+				<0x59250000 0x10000>, /* asrc0 pair C output req */
+				<0x59260000 0x10000>, /* esai0 rx */
+				<0x59270000 0x10000>, /* esai0 tx */
+				<0x59280000 0x10000>, /* spdif0 rx */
+				<0x59290000 0x10000>, /* spdif0 tx */
+				<0x592c0000 0x10000>, /* sai0 rx */
+				<0x592d0000 0x10000>, /* sai0 tx */
+				<0x592e0000 0x10000>, /* sai1 rx */
+				<0x592f0000 0x10000>, /* sai1 tx */
+			#dma-cells = <3>;
+			shared-interrupt;
+			dma-channels = <14>;
+			interrupts = <GIC_SPI 374 IRQ_TYPE_LEVEL_HIGH>,    /* asrc 0 */
+					<GIC_SPI 375 IRQ_TYPE_LEVEL_HIGH>,
+					<GIC_SPI 376 IRQ_TYPE_LEVEL_HIGH>,
+					<GIC_SPI 377 IRQ_TYPE_LEVEL_HIGH>,
+					<GIC_SPI 378 IRQ_TYPE_LEVEL_HIGH>,
+					<GIC_SPI 379 IRQ_TYPE_LEVEL_HIGH>,
+					<GIC_SPI 410 IRQ_TYPE_LEVEL_HIGH>, /* esai0 */
+					<GIC_SPI 410 IRQ_TYPE_LEVEL_HIGH>,
+					<GIC_SPI 457 IRQ_TYPE_LEVEL_HIGH>, /* spdif0 */
+					<GIC_SPI 459 IRQ_TYPE_LEVEL_HIGH>,
+					<GIC_SPI 315 IRQ_TYPE_LEVEL_HIGH>, /* sai0 */
+					<GIC_SPI 315 IRQ_TYPE_LEVEL_HIGH>,
+					<GIC_SPI 317 IRQ_TYPE_LEVEL_HIGH>, /* sai1 */
+					<GIC_SPI 317 IRQ_TYPE_LEVEL_HIGH>,
+			interrupt-names = "edma0-chan0-rx", "edma0-chan1-rx", /* asrc0 */
+					"edma0-chan2-rx", "edma0-chan3-tx",
+					"edma0-chan4-tx", "edma0-chan5-tx",
+					"edma0-chan6-rx", "edma0-chan7-tx",   /* esai0 */
+					"edma0-chan8-rx", "edma0-chan9-tx",   /* spdif0 */
+					"edma0-chan12-rx", "edma0-chan13-tx", /* sai0 */
+					"edma0-chan14-rx", "edma0-chan15-tx", /* sai1 */
+		};
+
+		edma1: dma-controller@599F0000 {
+			compatible = "fsl,imx8qm-edma";
+			reg = <0x0 0x59A00000 0x0 0x10000>, /* asrc1 */
+				<0x0 0x59A10000 0x0 0x10000>,
+				<0x0 0x59A20000 0x0 0x10000>,
+				<0x0 0x59A30000 0x0 0x10000>,
+				<0x0 0x59A40000 0x0 0x10000>,
+				<0x0 0x59A50000 0x0 0x10000>,
+				<0x0 0x59A80000 0x0 0x10000>, /* sai4 rx */
+				<0x0 0x59A90000 0x0 0x10000>, /* sai4 tx */
+				<0x0 0x59AA0000 0x0 0x10000>; /* sai5 tx */
+			#dma-cells = <3>;
+			shared-interrupt;
+			dma-channels = <9>;
+			interrupts = <GIC_SPI 382 IRQ_TYPE_LEVEL_HIGH>, /* asrc 1 */
+					<GIC_SPI 383 IRQ_TYPE_LEVEL_HIGH>,
+					<GIC_SPI 384 IRQ_TYPE_LEVEL_HIGH>,
+					<GIC_SPI 385 IRQ_TYPE_LEVEL_HIGH>,
+					<GIC_SPI 386 IRQ_TYPE_LEVEL_HIGH>,
+					<GIC_SPI 387 IRQ_TYPE_LEVEL_HIGH>,
+					<GIC_SPI 330 IRQ_TYPE_LEVEL_HIGH>, /* sai4 */
+					<GIC_SPI 330 IRQ_TYPE_LEVEL_HIGH>,
+					<GIC_SPI 332 IRQ_TYPE_LEVEL_HIGH>; /* sai5 */
+			interrupt-names = "edma1-chan0-rx", "edma1-chan1-rx", /* asrc1 */
+					"edma1-chan2-rx", "edma1-chan3-tx",
+					"edma1-chan4-tx", "edma1-chan5-tx",
+					"edma1-chan8-rx", "edma1-chan9-tx", /* sai4 */
+					"edma1-chan10-tx"; /* sai5 */
+		};
+
 		adma_lpuart0: serial@5a060000 {
 			compatible = "fsl,imx8qxp-lpuart", "fsl,imx7ulp-lpuart";
 			reg = <0x5a060000 0x1000>;