2024-03-05 17:33:39

by Frank Li

[permalink] [raw]
Subject: [PATCH v4 0/4] arm64: dts: imx8qxp add asrc and sai

Update binding doc to avoid warning.
Change from v1 to v2
- Fixed dts DTB_CHECK warning

Signed-off-by: Frank Li <[email protected]>
---
Changes in v4:
Combine comments' from v2 and v3. I hope I address everythings.

- Krzysztof's comments
- add reson about why change

- rob's comments
using rob's suggest logic to restrict interrupt number
but for dma-names, still need use oneOf to cover 3 case
- [rx, tx]
- [rx]
- [tx]

oneOf
- items:
- tx
- rx
- enums: [rx, tx]

- Conor's comments
- add power-domains required for imx8qxp and imx8qm
- remove dmas descript, not allow use index to get dma-channel. Current
no user using this method.

- Link to v3: https://lore.kernel.org/r/[email protected]

Changes in v3:
- Fixed dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/sound/fsl,spdif.example.dtb: spdif@2004000: interrupts: [[0, 52, 4]] is too short
from schema $id: http://devicetree.org/schemas/sound/fsl,spdif.yaml#

- Link to v2: https://lore.kernel.org/r/[email protected]

---
Frank Li (4):
ASoC: dt-bindings: fsl,imx-asrc/spdif: Add power-domains property
ASoC: dt-bindings: fsl,imx-asrc: update max interrupt numbers
ASoC: dt-bindings: fsl-sai: allow only one dma-names
arm64: dts: imx8qxp: add asrc[0,1], esai0, spdif[0,1] and sai[4,5]

.../devicetree/bindings/sound/fsl,imx-asrc.yaml | 14 +
.../devicetree/bindings/sound/fsl,sai.yaml | 13 +-
.../devicetree/bindings/sound/fsl,spdif.yaml | 33 ++-
arch/arm64/boot/dts/freescale/imx8-ss-audio.dtsi | 304 +++++++++++++++++++++
4 files changed, 356 insertions(+), 8 deletions(-)
---
base-commit: ca301cf599a4eeafed8e3dd87bf8d2fe810e075e
change-id: 20240227-asrc_8qxp-25aa6783840f

Best regards,
--
Frank Li <[email protected]>



2024-03-05 17:33:51

by Frank Li

[permalink] [raw]
Subject: [PATCH v4 1/4] ASoC: dt-bindings: fsl,imx-asrc/spdif: Add power-domains property

Add power-domains property for asrc and spdif since fsl,imx8qm-asrc/spdif
and fsl,imx8qxp-asrc/spdif require 'power-domains'.

Set 'power-domains' as required property for compatible string
fsl,imx8qm-asrc/spdif and fsl,imx8qxp-asrc/spdif.

Signed-off-by: Frank Li <[email protected]>
---
Documentation/devicetree/bindings/sound/fsl,imx-asrc.yaml | 14 ++++++++++++++
Documentation/devicetree/bindings/sound/fsl,spdif.yaml | 15 +++++++++++++++
2 files changed, 29 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/fsl,imx-asrc.yaml b/Documentation/devicetree/bindings/sound/fsl,imx-asrc.yaml
index bfef2fcb75b14..76aa1f2484883 100644
--- a/Documentation/devicetree/bindings/sound/fsl,imx-asrc.yaml
+++ b/Documentation/devicetree/bindings/sound/fsl,imx-asrc.yaml
@@ -74,6 +74,9 @@ properties:
- const: asrck_f
- const: spba

+ power-domains:
+ maxItems: 1
+
fsl,asrc-rate:
$ref: /schemas/types.yaml#/definitions/uint32
description: The mutual sample rate used by DPCM Back Ends
@@ -131,6 +134,17 @@ allOf:
properties:
fsl,asrc-clk-map: false

+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - fsl,imx8qm-asrc
+ - fsl,imx8qxp-asrc
+ then:
+ required:
+ - power-domains
+
additionalProperties: false

examples:
diff --git a/Documentation/devicetree/bindings/sound/fsl,spdif.yaml b/Documentation/devicetree/bindings/sound/fsl,spdif.yaml
index 1d64e8337aa4b..56f8c0c8afdea 100644
--- a/Documentation/devicetree/bindings/sound/fsl,spdif.yaml
+++ b/Documentation/devicetree/bindings/sound/fsl,spdif.yaml
@@ -86,6 +86,9 @@ properties:
registers. Set this flag for HCDs with big endian descriptors and big
endian registers.

+ power-domains:
+ maxItems: 1
+
required:
- compatible
- reg
@@ -97,6 +100,18 @@ required:

additionalProperties: false

+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - fsl,imx8qm-spdif
+ - fsl,imx8qxp-spdif
+ then:
+ required:
+ - power-domains
+
examples:
- |
spdif@2004000 {

--
2.34.1


2024-03-05 17:34:12

by Frank Li

[permalink] [raw]
Subject: [PATCH v4 2/4] ASoC: dt-bindings: fsl,imx-asrc: update max interrupt numbers

fsl,imx8qxp-spdif and fsl,imx8qm-spdif have 2 interrupts. Other platforms
have 1 interrupt.

Increase max interrupt number to 2 and add restriction for platforms except
i.MX8QXP and i.MX8QM.

Signed-off-by: Frank Li <[email protected]>
---
Documentation/devicetree/bindings/sound/fsl,spdif.yaml | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/sound/fsl,spdif.yaml b/Documentation/devicetree/bindings/sound/fsl,spdif.yaml
index 56f8c0c8afdea..7f6590708e1ec 100644
--- a/Documentation/devicetree/bindings/sound/fsl,spdif.yaml
+++ b/Documentation/devicetree/bindings/sound/fsl,spdif.yaml
@@ -31,7 +31,8 @@ properties:
maxItems: 1

interrupts:
- maxItems: 1
+ minItems: 1
+ maxItems: 2

dmas:
items:
@@ -101,6 +102,21 @@ required:
additionalProperties: false

allOf:
+ - if:
+ properties:
+ compatible:
+ enum:
+ - fsl,imx8qm-spdif
+ - fsl,imx8qxp-spdif
+ then:
+ properties:
+ interrupts:
+ minItems: 2
+ else:
+ properties:
+ interrupts:
+ maxItems: 1
+
- if:
properties:
compatible:

--
2.34.1


2024-03-05 17:34:47

by Frank Li

[permalink] [raw]
Subject: [PATCH v4 4/4] arm64: dts: imx8qxp: add asrc[0,1], esai0, spdif[0,1] and sai[4,5]

Add asrc[0,1], esai0, spdif[0,1], sai[4,5] and related lpcg node for
imx8 audio subsystem.

Signed-off-by: Frank Li <[email protected]>
---
arch/arm64/boot/dts/freescale/imx8-ss-audio.dtsi | 304 +++++++++++++++++++++++
1 file changed, 304 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8-ss-audio.dtsi b/arch/arm64/boot/dts/freescale/imx8-ss-audio.dtsi
index 07afeb78ed564..78305559f15c9 100644
--- a/arch/arm64/boot/dts/freescale/imx8-ss-audio.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8-ss-audio.dtsi
@@ -6,6 +6,7 @@

#include <dt-bindings/clock/imx8-clock.h>
#include <dt-bindings/clock/imx8-lpcg.h>
+#include <dt-bindings/dma/fsl-edma.h>
#include <dt-bindings/firmware/imx/rsrc.h>

audio_ipg_clk: clock-audio-ipg {
@@ -481,4 +482,307 @@ acm: acm@59e00000 {
"sai3_rx_bclk",
"sai4_rx_bclk";
};
+
+ asrc0: asrc@59000000 {
+ compatible = "fsl,imx8qm-asrc";
+ reg = <0x59000000 0x10000>;
+ interrupts = <GIC_SPI 372 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&asrc0_lpcg 0>,
+ <&asrc0_lpcg 0>,
+ <&aud_pll_div0_lpcg 0>,
+ <&aud_pll_div1_lpcg 0>,
+ <&acm IMX_ADMA_ACM_AUD_CLK0_SEL>,
+ <&acm IMX_ADMA_ACM_AUD_CLK1_SEL>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>;
+ clock-names = "mem", "ipg",
+ "asrck_0", "asrck_1", "asrck_2", "asrck_3",
+ "asrck_4", "asrck_5", "asrck_6", "asrck_7",
+ "asrck_8", "asrck_9", "asrck_a", "asrck_b",
+ "asrck_c", "asrck_d", "asrck_e", "asrck_f",
+ "spba";
+ dmas = <&edma0 0 0 0>,
+ <&edma0 1 0 0>,
+ <&edma0 2 0 0>,
+ <&edma0 3 0 FSL_EDMA_RX>,
+ <&edma0 4 0 FSL_EDMA_RX>,
+ <&edma0 5 0 FSL_EDMA_RX>;
+ /* tx* is output channel of asrc, it is rx channel for eDMA */
+ dma-names = "rxa", "rxb", "rxc", "txa", "txb", "txc";
+ fsl,asrc-rate = <8000>;
+ fsl,asrc-width = <16>;
+ fsl,asrc-clk-map = <0>;
+ power-domains = <&pd IMX_SC_R_ASRC_0>;
+ status = "disabled";
+ };
+
+ esai0: esai@59010000 {
+ compatible = "fsl,imx8qm-esai", "fsl,imx6ull-esai";
+ reg = <0x59010000 0x10000>;
+ interrupts = <GIC_SPI 409 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&esai0_lpcg 1>, <&esai0_lpcg 0>, <&esai0_lpcg 1>, <&clk_dummy>;
+ clock-names = "core", "extal", "fsys", "spba";
+ dmas = <&edma0 6 0 FSL_EDMA_RX>, <&edma0 7 0 0>;
+ dma-names = "rx", "tx";
+ power-domains = <&pd IMX_SC_R_ESAI_0>;
+ status = "disabled";
+ };
+
+ spdif0: spdif@59020000 {
+ compatible = "fsl,imx8qm-spdif";
+ reg = <0x59020000 0x10000>;
+ interrupts = <GIC_SPI 456 IRQ_TYPE_LEVEL_HIGH>, /* rx */
+ <GIC_SPI 458 IRQ_TYPE_LEVEL_HIGH>; /* tx */
+ clocks = <&spdif0_lpcg 1>, /* core */
+ <&clk_dummy>, /* rxtx0 */
+ <&spdif0_lpcg 0>, /* rxtx1 */
+ <&clk_dummy>, /* rxtx2 */
+ <&clk_dummy>, /* rxtx3 */
+ <&clk_dummy>, /* rxtx4 */
+ <&audio_ipg_clk>, /* rxtx5 */
+ <&clk_dummy>, /* rxtx6 */
+ <&clk_dummy>, /* rxtx7 */
+ <&clk_dummy>; /* spba */
+ clock-names = "core", "rxtx0", "rxtx1", "rxtx2", "rxtx3", "rxtx4",
+ "rxtx5", "rxtx6", "rxtx7", "spba";
+ dmas = <&edma0 8 0 (FSL_EDMA_MULTI_FIFO | FSL_EDMA_RX)>,
+ <&edma0 9 0 FSL_EDMA_MULTI_FIFO>;
+ dma-names = "rx", "tx";
+ power-domains = <&pd IMX_SC_R_SPDIF_0>;
+ status = "disabled";
+ };
+
+ spdif1: spdif@59030000 {
+ compatible = "fsl,imx8qm-spdif";
+ reg = <0x59030000 0x10000>;
+ interrupts = <GIC_SPI 460 IRQ_TYPE_LEVEL_HIGH>, /* rx */
+ <GIC_SPI 462 IRQ_TYPE_LEVEL_HIGH>; /* tx */
+ clocks = <&spdif1_lpcg 1>, /* core */
+ <&clk_dummy>, /* rxtx0 */
+ <&spdif1_lpcg 0>, /* rxtx1 */
+ <&clk_dummy>, /* rxtx2 */
+ <&clk_dummy>, /* rxtx3 */
+ <&clk_dummy>, /* rxtx4 */
+ <&audio_ipg_clk>, /* rxtx5 */
+ <&clk_dummy>, /* rxtx6 */
+ <&clk_dummy>, /* rxtx7 */
+ <&clk_dummy>; /* spba */
+ clock-names = "core", "rxtx0", "rxtx1", "rxtx2", "rxtx3", "rxtx4",
+ "rxtx5", "rxtx6", "rxtx7", "spba";
+ dmas = <&edma0 10 0 (FSL_EDMA_MULTI_FIFO | FSL_EDMA_RX)>,
+ <&edma0 11 0 FSL_EDMA_MULTI_FIFO>;
+ dma-names = "rx", "tx";
+ power-domains = <&pd IMX_SC_R_SPDIF_1>;
+ status = "disabled";
+ };
+
+ asrc1: asrc@59800000 {
+ compatible = "fsl,imx8qm-asrc";
+ reg = <0x59800000 0x10000>;
+ interrupts = <GIC_SPI 380 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&asrc1_lpcg 0>,
+ <&asrc1_lpcg 0>,
+ <&aud_pll_div0_lpcg 0>,
+ <&aud_pll_div1_lpcg 0>,
+ <&acm IMX_ADMA_ACM_AUD_CLK0_SEL>,
+ <&acm IMX_ADMA_ACM_AUD_CLK1_SEL>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>;
+ clock-names = "mem", "ipg",
+ "asrck_0", "asrck_1", "asrck_2", "asrck_3",
+ "asrck_4", "asrck_5", "asrck_6", "asrck_7",
+ "asrck_8", "asrck_9", "asrck_a", "asrck_b",
+ "asrck_c", "asrck_d", "asrck_e", "asrck_f",
+ "spba";
+ dmas = <&edma1 0 0 0>,
+ <&edma1 1 0 0>,
+ <&edma1 2 0 0>,
+ <&edma1 3 0 FSL_EDMA_RX>,
+ <&edma1 4 0 FSL_EDMA_RX>,
+ <&edma1 5 0 FSL_EDMA_RX>;
+ /* tx* is output channel of asrc, it is rx channel for eDMA */
+ dma-names = "rxa", "rxb", "rxc", "txa", "txb", "txc";
+ fsl,asrc-rate = <8000>;
+ fsl,asrc-width = <16>;
+ fsl,asrc-clk-map = <1>;
+ power-domains = <&pd IMX_SC_R_ASRC_1>;
+ status = "disabled";
+ };
+
+ sai4: sai@59820000 {
+ compatible = "fsl,imx8qm-sai";
+ reg = <0x59820000 0x10000>;
+ interrupts = <GIC_SPI 329 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&sai4_lpcg 1>,
+ <&clk_dummy>,
+ <&sai4_lpcg 0>,
+ <&clk_dummy>,
+ <&clk_dummy>;
+ clock-names = "bus", "mclk0", "mclk1", "mclk2", "mclk3";
+ dmas = <&edma1 8 0 FSL_EDMA_RX>, <&edma1 9 0 0>;
+ dma-names = "rx", "tx";
+ power-domains = <&pd IMX_SC_R_SAI_4>;
+ status = "disabled";
+ };
+
+ sai5: sai@59830000 {
+ compatible = "fsl,imx8qm-sai";
+ reg = <0x59830000 0x10000>;
+ interrupts = <GIC_SPI 331 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&sai5_lpcg 1>,
+ <&clk_dummy>,
+ <&sai5_lpcg 0>,
+ <&clk_dummy>,
+ <&clk_dummy>;
+ clock-names = "bus", "mclk0", "mclk1", "mclk2", "mclk3";
+ dmas = <&edma1 10 0 0>;
+ dma-names = "tx";
+ power-domains = <&pd IMX_SC_R_SAI_5>;
+ status = "disabled";
+ };
+
+ amix: amix@59840000 {
+ compatible = "fsl,imx8qm-audmix";
+ reg = <0x59840000 0x10000>;
+ clocks = <&amix_lpcg 0>;
+ clock-names = "ipg";
+ power-domains = <&pd IMX_SC_R_AMIX>;
+ dais = <&sai4>, <&sai5>;
+ status = "disabled";
+ };
+
+ mqs: mqs@59850000 {
+ compatible = "fsl,imx8qm-mqs";
+ reg = <0x59850000 0x10000>;
+ clocks = <&mqs0_lpcg 0>,
+ <&mqs0_lpcg 1>;
+ clock-names = "mclk", "core";
+ power-domains = <&pd IMX_SC_R_MQS_0>;
+ status = "disabled";
+ };
+
+ asrc0_lpcg: clock-controller@59400000 {
+ compatible = "fsl,imx8qxp-lpcg";
+ reg = <0x59400000 0x10000>;
+ #clock-cells = <1>;
+ clocks = <&audio_ipg_clk>;
+ clock-indices = <IMX_LPCG_CLK_4>;
+ clock-output-names = "asrc0_lpcg_ipg_clk";
+ power-domains = <&pd IMX_SC_R_ASRC_0>;
+ };
+
+ esai0_lpcg: clock-controller@59410000 {
+ compatible = "fsl,imx8qxp-lpcg";
+ reg = <0x59410000 0x10000>;
+ #clock-cells = <1>;
+ clocks = <&acm IMX_ADMA_ACM_ESAI0_MCLK_SEL>,
+ <&audio_ipg_clk>;
+ clock-indices = <IMX_LPCG_CLK_0>, <IMX_LPCG_CLK_4>;
+ clock-output-names = "esai0_lpcg_extal_clk",
+ "esai0_lpcg_ipg_clk";
+ power-domains = <&pd IMX_SC_R_ESAI_0>;
+ };
+
+ spdif0_lpcg: clock-controller@59420000 {
+ compatible = "fsl,imx8qxp-lpcg";
+ reg = <0x59420000 0x10000>;
+ #clock-cells = <1>;
+ clocks = <&acm IMX_ADMA_ACM_SPDIF0_TX_CLK_SEL>,
+ <&audio_ipg_clk>;
+ clock-indices = <IMX_LPCG_CLK_0>, <IMX_LPCG_CLK_4>;
+ clock-output-names = "spdif0_lpcg_tx_clk",
+ "spdif0_lpcg_gclkw";
+ power-domains = <&pd IMX_SC_R_SPDIF_0>;
+ };
+
+ spdif1_lpcg: clock-controller@59430000 {
+ compatible = "fsl,imx8qxp-lpcg";
+ reg = <0x59430000 0x10000>;
+ #clock-cells = <1>;
+ clocks = <&acm IMX_ADMA_ACM_SPDIF1_TX_CLK_SEL>,
+ <&audio_ipg_clk>;
+ clock-indices = <IMX_LPCG_CLK_0>, <IMX_LPCG_CLK_4>;
+ clock-output-names = "spdif1_lpcg_tx_clk",
+ "spdif1_lpcg_gclkw";
+ power-domains = <&pd IMX_SC_R_SPDIF_1>;
+ status = "disabled";
+ };
+
+ asrc1_lpcg: clock-controller@59c00000 {
+ compatible = "fsl,imx8qxp-lpcg";
+ reg = <0x59c00000 0x10000>;
+ #clock-cells = <1>;
+ clocks = <&audio_ipg_clk>;
+ clock-indices = <IMX_LPCG_CLK_4>;
+ clock-output-names = "asrc1_lpcg_ipg_clk";
+ power-domains = <&pd IMX_SC_R_ASRC_1>;
+ };
+
+ sai4_lpcg: clock-controller@59c20000 {
+ compatible = "fsl,imx8qxp-lpcg";
+ reg = <0x59c20000 0x10000>;
+ #clock-cells = <1>;
+ clocks = <&acm IMX_ADMA_ACM_SAI4_MCLK_SEL>,
+ <&audio_ipg_clk>;
+ clock-indices = <IMX_LPCG_CLK_0>, <IMX_LPCG_CLK_4>;
+ clock-output-names = "sai4_lpcg_mclk",
+ "sai4_lpcg_ipg_clk";
+ power-domains = <&pd IMX_SC_R_SAI_4>;
+ };
+
+ sai5_lpcg: clock-controller@59c30000 {
+ compatible = "fsl,imx8qxp-lpcg";
+ reg = <0x59c30000 0x10000>;
+ #clock-cells = <1>;
+ clocks = <&acm IMX_ADMA_ACM_SAI5_MCLK_SEL>,
+ <&audio_ipg_clk>;
+ clock-indices = <IMX_LPCG_CLK_0>, <IMX_LPCG_CLK_4>;
+ clock-output-names = "sai5_lpcg_mclk",
+ "sai5_lpcg_ipg_clk";
+ power-domains = <&pd IMX_SC_R_SAI_5>;
+ };
+
+ amix_lpcg: clock-controller@59c40000 {
+ compatible = "fsl,imx8qxp-lpcg";
+ reg = <0x59c40000 0x10000>;
+ #clock-cells = <1>;
+ clocks = <&audio_ipg_clk>;
+ clock-indices = <IMX_LPCG_CLK_0>;
+ clock-output-names = "amix_lpcg_ipg_clk";
+ power-domains = <&pd IMX_SC_R_AMIX>;
+ };
+
+ mqs0_lpcg: clock-controller@59c50000 {
+ compatible = "fsl,imx8qxp-lpcg";
+ reg = <0x59c50000 0x10000>;
+ #clock-cells = <1>;
+ clocks = <&acm IMX_ADMA_ACM_MQS_TX_CLK_SEL>,
+ <&audio_ipg_clk>;
+ clock-indices = <IMX_LPCG_CLK_0>, <IMX_LPCG_CLK_4>;
+ clock-output-names = "mqs0_lpcg_mclk",
+ "mqs0_lpcg_ipg_clk";
+ power-domains = <&pd IMX_SC_R_MQS_0>;
+ };
};

--
2.34.1


2024-03-05 18:03:35

by Frank Li

[permalink] [raw]
Subject: [PATCH v4 3/4] ASoC: dt-bindings: fsl-sai: allow only one dma-names

Some sai only connect one direction dma (rx/tx) in SOC. For example:
imx8qxp sai5 only connect tx dma channel. So allow only one "rx" or "tx"
for dma-names.

Remove description under dmas because no user use index to get dma channel.
All user use 'dma-names' to get correct dma channel. dma-names already in
'required' list.

Signed-off-by: Frank Li <[email protected]>
---
Documentation/devicetree/bindings/sound/fsl,sai.yaml | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/fsl,sai.yaml b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
index 2456d958adeef..6f551c68d33db 100644
--- a/Documentation/devicetree/bindings/sound/fsl,sai.yaml
+++ b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
@@ -81,15 +81,14 @@ properties:

dmas:
minItems: 1
- items:
- - description: DMA controller phandle and request line for RX
- - description: DMA controller phandle and request line for TX
+ maxItems: 2

dma-names:
- minItems: 1
- items:
- - const: rx
- - const: tx
+ oneOf:
+ - items:
+ - const: rx
+ - const: tx
+ - enum: [ rx, tx ]

interrupts:
items:

--
2.34.1


2024-03-06 07:20:31

by Alexander Stein

[permalink] [raw]
Subject: Re: [PATCH v4 4/4] arm64: dts: imx8qxp: add asrc[0, 1], esai0, spdif[0, 1] and sai[4, 5]

Hi Frank,

thanks for the patch.

Am Dienstag, 5. M?rz 2024, 18:33:05 CET schrieb Frank Li:
> Add asrc[0,1], esai0, spdif[0,1], sai[4,5] and related lpcg node for
> imx8 audio subsystem.
>
> Signed-off-by: Frank Li <[email protected]>
> ---
> arch/arm64/boot/dts/freescale/imx8-ss-audio.dtsi | 304 +++++++++++++++++++++++
> 1 file changed, 304 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8-ss-audio.dtsi b/arch/arm64/boot/dts/freescale/imx8-ss-audio.dtsi
> index 07afeb78ed564..78305559f15c9 100644
> --- a/arch/arm64/boot/dts/freescale/imx8-ss-audio.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8-ss-audio.dtsi
> @@ -6,6 +6,7 @@
>
> #include <dt-bindings/clock/imx8-clock.h>
> #include <dt-bindings/clock/imx8-lpcg.h>
> +#include <dt-bindings/dma/fsl-edma.h>
> #include <dt-bindings/firmware/imx/rsrc.h>
>
> audio_ipg_clk: clock-audio-ipg {
> @@ -481,4 +482,307 @@ acm: acm@59e00000 {
> "sai3_rx_bclk",
> "sai4_rx_bclk";
> };
> +
> + asrc0: asrc@59000000 {

Please insert nodes sorted by address. ASRC0 is the very first node.

> + compatible = "fsl,imx8qm-asrc";
> + reg = <0x59000000 0x10000>;
> + interrupts = <GIC_SPI 372 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&asrc0_lpcg 0>,
> + <&asrc0_lpcg 0>,
> + <&aud_pll_div0_lpcg 0>,
> + <&aud_pll_div1_lpcg 0>,
> + <&acm IMX_ADMA_ACM_AUD_CLK0_SEL>,
> + <&acm IMX_ADMA_ACM_AUD_CLK1_SEL>,
> + <&clk_dummy>,
> + <&clk_dummy>,
> + <&clk_dummy>,
> + <&clk_dummy>,
> + <&clk_dummy>,
> + <&clk_dummy>,
> + <&clk_dummy>,
> + <&clk_dummy>,
> + <&clk_dummy>,
> + <&clk_dummy>,
> + <&clk_dummy>,
> + <&clk_dummy>,
> + <&clk_dummy>;
> + clock-names = "mem", "ipg",
> + "asrck_0", "asrck_1", "asrck_2", "asrck_3",
> + "asrck_4", "asrck_5", "asrck_6", "asrck_7",
> + "asrck_8", "asrck_9", "asrck_a", "asrck_b",
> + "asrck_c", "asrck_d", "asrck_e", "asrck_f",
> + "spba";
> + dmas = <&edma0 0 0 0>,
> + <&edma0 1 0 0>,
> + <&edma0 2 0 0>,
> + <&edma0 3 0 FSL_EDMA_RX>,
> + <&edma0 4 0 FSL_EDMA_RX>,
> + <&edma0 5 0 FSL_EDMA_RX>;
> + /* tx* is output channel of asrc, it is rx channel for eDMA */
> + dma-names = "rxa", "rxb", "rxc", "txa", "txb", "txc";
> + fsl,asrc-rate = <8000>;
> + fsl,asrc-width = <16>;
> + fsl,asrc-clk-map = <0>;
> + power-domains = <&pd IMX_SC_R_ASRC_0>;
> + status = "disabled";
> + };
> +
> + esai0: esai@59010000 {
> + compatible = "fsl,imx8qm-esai", "fsl,imx6ull-esai";
> + reg = <0x59010000 0x10000>;
> + interrupts = <GIC_SPI 409 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&esai0_lpcg 1>, <&esai0_lpcg 0>, <&esai0_lpcg 1>, <&clk_dummy>;
> + clock-names = "core", "extal", "fsys", "spba";
> + dmas = <&edma0 6 0 FSL_EDMA_RX>, <&edma0 7 0 0>;
> + dma-names = "rx", "tx";
> + power-domains = <&pd IMX_SC_R_ESAI_0>;
> + status = "disabled";
> + };
> +
> + spdif0: spdif@59020000 {
> + compatible = "fsl,imx8qm-spdif";
> + reg = <0x59020000 0x10000>;
> + interrupts = <GIC_SPI 456 IRQ_TYPE_LEVEL_HIGH>, /* rx */
> + <GIC_SPI 458 IRQ_TYPE_LEVEL_HIGH>; /* tx */
> + clocks = <&spdif0_lpcg 1>, /* core */
> + <&clk_dummy>, /* rxtx0 */
> + <&spdif0_lpcg 0>, /* rxtx1 */
> + <&clk_dummy>, /* rxtx2 */
> + <&clk_dummy>, /* rxtx3 */
> + <&clk_dummy>, /* rxtx4 */
> + <&audio_ipg_clk>, /* rxtx5 */
> + <&clk_dummy>, /* rxtx6 */
> + <&clk_dummy>, /* rxtx7 */
> + <&clk_dummy>; /* spba */
> + clock-names = "core", "rxtx0", "rxtx1", "rxtx2", "rxtx3", "rxtx4",
> + "rxtx5", "rxtx6", "rxtx7", "spba";
> + dmas = <&edma0 8 0 (FSL_EDMA_MULTI_FIFO | FSL_EDMA_RX)>,
> + <&edma0 9 0 FSL_EDMA_MULTI_FIFO>;
> + dma-names = "rx", "tx";
> + power-domains = <&pd IMX_SC_R_SPDIF_0>;
> + status = "disabled";
> + };
> +
> + spdif1: spdif@59030000 {

That's imx8qm only, no?

> + compatible = "fsl,imx8qm-spdif";
> + reg = <0x59030000 0x10000>;
> + interrupts = <GIC_SPI 460 IRQ_TYPE_LEVEL_HIGH>, /* rx */
> + <GIC_SPI 462 IRQ_TYPE_LEVEL_HIGH>; /* tx */
> + clocks = <&spdif1_lpcg 1>, /* core */
> + <&clk_dummy>, /* rxtx0 */
> + <&spdif1_lpcg 0>, /* rxtx1 */
> + <&clk_dummy>, /* rxtx2 */
> + <&clk_dummy>, /* rxtx3 */
> + <&clk_dummy>, /* rxtx4 */
> + <&audio_ipg_clk>, /* rxtx5 */
> + <&clk_dummy>, /* rxtx6 */
> + <&clk_dummy>, /* rxtx7 */
> + <&clk_dummy>; /* spba */
> + clock-names = "core", "rxtx0", "rxtx1", "rxtx2", "rxtx3", "rxtx4",
> + "rxtx5", "rxtx6", "rxtx7", "spba";
> + dmas = <&edma0 10 0 (FSL_EDMA_MULTI_FIFO | FSL_EDMA_RX)>,
> + <&edma0 11 0 FSL_EDMA_MULTI_FIFO>;
> + dma-names = "rx", "tx";
> + power-domains = <&pd IMX_SC_R_SPDIF_1>;
> + status = "disabled";
> + };
> +
> + asrc1: asrc@59800000 {

Insert this between dsp and edma1, sorted by address.

> + compatible = "fsl,imx8qm-asrc";
> + reg = <0x59800000 0x10000>;
> + interrupts = <GIC_SPI 380 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&asrc1_lpcg 0>,
> + <&asrc1_lpcg 0>,
> + <&aud_pll_div0_lpcg 0>,
> + <&aud_pll_div1_lpcg 0>,
> + <&acm IMX_ADMA_ACM_AUD_CLK0_SEL>,
> + <&acm IMX_ADMA_ACM_AUD_CLK1_SEL>,
> + <&clk_dummy>,
> + <&clk_dummy>,
> + <&clk_dummy>,
> + <&clk_dummy>,
> + <&clk_dummy>,
> + <&clk_dummy>,
> + <&clk_dummy>,
> + <&clk_dummy>,
> + <&clk_dummy>,
> + <&clk_dummy>,
> + <&clk_dummy>,
> + <&clk_dummy>,
> + <&clk_dummy>;
> + clock-names = "mem", "ipg",
> + "asrck_0", "asrck_1", "asrck_2", "asrck_3",
> + "asrck_4", "asrck_5", "asrck_6", "asrck_7",
> + "asrck_8", "asrck_9", "asrck_a", "asrck_b",
> + "asrck_c", "asrck_d", "asrck_e", "asrck_f",
> + "spba";
> + dmas = <&edma1 0 0 0>,
> + <&edma1 1 0 0>,
> + <&edma1 2 0 0>,
> + <&edma1 3 0 FSL_EDMA_RX>,
> + <&edma1 4 0 FSL_EDMA_RX>,
> + <&edma1 5 0 FSL_EDMA_RX>;
> + /* tx* is output channel of asrc, it is rx channel for eDMA */
> + dma-names = "rxa", "rxb", "rxc", "txa", "txb", "txc";
> + fsl,asrc-rate = <8000>;
> + fsl,asrc-width = <16>;
> + fsl,asrc-clk-map = <1>;
> + power-domains = <&pd IMX_SC_R_ASRC_1>;
> + status = "disabled";
> + };
> +
> + sai4: sai@59820000 {
> + compatible = "fsl,imx8qm-sai";
> + reg = <0x59820000 0x10000>;
> + interrupts = <GIC_SPI 329 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&sai4_lpcg 1>,
> + <&clk_dummy>,
> + <&sai4_lpcg 0>,
> + <&clk_dummy>,
> + <&clk_dummy>;
> + clock-names = "bus", "mclk0", "mclk1", "mclk2", "mclk3";
> + dmas = <&edma1 8 0 FSL_EDMA_RX>, <&edma1 9 0 0>;
> + dma-names = "rx", "tx";
> + power-domains = <&pd IMX_SC_R_SAI_4>;
> + status = "disabled";
> + };
> +
> + sai5: sai@59830000 {
> + compatible = "fsl,imx8qm-sai";
> + reg = <0x59830000 0x10000>;
> + interrupts = <GIC_SPI 331 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&sai5_lpcg 1>,
> + <&clk_dummy>,
> + <&sai5_lpcg 0>,
> + <&clk_dummy>,
> + <&clk_dummy>;
> + clock-names = "bus", "mclk0", "mclk1", "mclk2", "mclk3";
> + dmas = <&edma1 10 0 0>;
> + dma-names = "tx";
> + power-domains = <&pd IMX_SC_R_SAI_5>;
> + status = "disabled";
> + };
> +
> + amix: amix@59840000 {
> + compatible = "fsl,imx8qm-audmix";
> + reg = <0x59840000 0x10000>;
> + clocks = <&amix_lpcg 0>;
> + clock-names = "ipg";
> + power-domains = <&pd IMX_SC_R_AMIX>;
> + dais = <&sai4>, <&sai5>;
> + status = "disabled";
> + };
> +
> + mqs: mqs@59850000 {
> + compatible = "fsl,imx8qm-mqs";
> + reg = <0x59850000 0x10000>;
> + clocks = <&mqs0_lpcg 0>,
> + <&mqs0_lpcg 1>;
> + clock-names = "mclk", "core";
> + power-domains = <&pd IMX_SC_R_MQS_0>;
> + status = "disabled";
> + };
> +
> + asrc0_lpcg: clock-controller@59400000 {

Please insert he lpcg nodes according to their address.

> + compatible = "fsl,imx8qxp-lpcg";
> + reg = <0x59400000 0x10000>;
> + #clock-cells = <1>;
> + clocks = <&audio_ipg_clk>;
> + clock-indices = <IMX_LPCG_CLK_4>;
> + clock-output-names = "asrc0_lpcg_ipg_clk";
> + power-domains = <&pd IMX_SC_R_ASRC_0>;
> + };
> +
> + esai0_lpcg: clock-controller@59410000 {
> + compatible = "fsl,imx8qxp-lpcg";
> + reg = <0x59410000 0x10000>;
> + #clock-cells = <1>;
> + clocks = <&acm IMX_ADMA_ACM_ESAI0_MCLK_SEL>,
> + <&audio_ipg_clk>;
> + clock-indices = <IMX_LPCG_CLK_0>, <IMX_LPCG_CLK_4>;
> + clock-output-names = "esai0_lpcg_extal_clk",
> + "esai0_lpcg_ipg_clk";
> + power-domains = <&pd IMX_SC_R_ESAI_0>;
> + };
> +
> + spdif0_lpcg: clock-controller@59420000 {
> + compatible = "fsl,imx8qxp-lpcg";
> + reg = <0x59420000 0x10000>;
> + #clock-cells = <1>;
> + clocks = <&acm IMX_ADMA_ACM_SPDIF0_TX_CLK_SEL>,
> + <&audio_ipg_clk>;
> + clock-indices = <IMX_LPCG_CLK_0>, <IMX_LPCG_CLK_4>;
> + clock-output-names = "spdif0_lpcg_tx_clk",
> + "spdif0_lpcg_gclkw";
> + power-domains = <&pd IMX_SC_R_SPDIF_0>;
> + };
> +
> + spdif1_lpcg: clock-controller@59430000 {

That's imx8qm only as well, no?

Thanks and best regards,
Alexander

> + compatible = "fsl,imx8qxp-lpcg";
> + reg = <0x59430000 0x10000>;
> + #clock-cells = <1>;
> + clocks = <&acm IMX_ADMA_ACM_SPDIF1_TX_CLK_SEL>,
> + <&audio_ipg_clk>;
> + clock-indices = <IMX_LPCG_CLK_0>, <IMX_LPCG_CLK_4>;
> + clock-output-names = "spdif1_lpcg_tx_clk",
> + "spdif1_lpcg_gclkw";
> + power-domains = <&pd IMX_SC_R_SPDIF_1>;
> + status = "disabled";
> + };
> +
> + asrc1_lpcg: clock-controller@59c00000 {
> + compatible = "fsl,imx8qxp-lpcg";
> + reg = <0x59c00000 0x10000>;
> + #clock-cells = <1>;
> + clocks = <&audio_ipg_clk>;
> + clock-indices = <IMX_LPCG_CLK_4>;
> + clock-output-names = "asrc1_lpcg_ipg_clk";
> + power-domains = <&pd IMX_SC_R_ASRC_1>;
> + };
> +
> + sai4_lpcg: clock-controller@59c20000 {
> + compatible = "fsl,imx8qxp-lpcg";
> + reg = <0x59c20000 0x10000>;
> + #clock-cells = <1>;
> + clocks = <&acm IMX_ADMA_ACM_SAI4_MCLK_SEL>,
> + <&audio_ipg_clk>;
> + clock-indices = <IMX_LPCG_CLK_0>, <IMX_LPCG_CLK_4>;
> + clock-output-names = "sai4_lpcg_mclk",
> + "sai4_lpcg_ipg_clk";
> + power-domains = <&pd IMX_SC_R_SAI_4>;
> + };
> +
> + sai5_lpcg: clock-controller@59c30000 {
> + compatible = "fsl,imx8qxp-lpcg";
> + reg = <0x59c30000 0x10000>;
> + #clock-cells = <1>;
> + clocks = <&acm IMX_ADMA_ACM_SAI5_MCLK_SEL>,
> + <&audio_ipg_clk>;
> + clock-indices = <IMX_LPCG_CLK_0>, <IMX_LPCG_CLK_4>;
> + clock-output-names = "sai5_lpcg_mclk",
> + "sai5_lpcg_ipg_clk";
> + power-domains = <&pd IMX_SC_R_SAI_5>;
> + };
> +
> + amix_lpcg: clock-controller@59c40000 {
> + compatible = "fsl,imx8qxp-lpcg";
> + reg = <0x59c40000 0x10000>;
> + #clock-cells = <1>;
> + clocks = <&audio_ipg_clk>;
> + clock-indices = <IMX_LPCG_CLK_0>;
> + clock-output-names = "amix_lpcg_ipg_clk";
> + power-domains = <&pd IMX_SC_R_AMIX>;
> + };
> +
> + mqs0_lpcg: clock-controller@59c50000 {
> + compatible = "fsl,imx8qxp-lpcg";
> + reg = <0x59c50000 0x10000>;
> + #clock-cells = <1>;
> + clocks = <&acm IMX_ADMA_ACM_MQS_TX_CLK_SEL>,
> + <&audio_ipg_clk>;
> + clock-indices = <IMX_LPCG_CLK_0>, <IMX_LPCG_CLK_4>;
> + clock-output-names = "mqs0_lpcg_mclk",
> + "mqs0_lpcg_ipg_clk";
> + power-domains = <&pd IMX_SC_R_MQS_0>;
> + };
> };
>
>


--
TQ-Systems GmbH | M?hlstra?e 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht M?nchen, HRB 105018
Gesch?ftsf?hrer: Detlef Schneider, R?diger Stahl, Stefan Schneider
http://www.tq-group.com/



2024-03-06 15:21:18

by Frank Li

[permalink] [raw]
Subject: Re: [PATCH v4 4/4] arm64: dts: imx8qxp: add asrc[0, 1], esai0, spdif[0, 1] and sai[4, 5]

On Wed, Mar 06, 2024 at 08:20:00AM +0100, Alexander Stein wrote:
> Hi Frank,
>
> thanks for the patch.
>
> Am Dienstag, 5. M?rz 2024, 18:33:05 CET schrieb Frank Li:
> > Add asrc[0,1], esai0, spdif[0,1], sai[4,5] and related lpcg node for
> > imx8 audio subsystem.
> >
> > Signed-off-by: Frank Li <[email protected]>
> > ---
> > arch/arm64/boot/dts/freescale/imx8-ss-audio.dtsi | 304 +++++++++++++++++++++++
> > 1 file changed, 304 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8-ss-audio.dtsi b/arch/arm64/boot/dts/freescale/imx8-ss-audio.dtsi
> > index 07afeb78ed564..78305559f15c9 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8-ss-audio.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8-ss-audio.dtsi
> > @@ -6,6 +6,7 @@
> >
> > #include <dt-bindings/clock/imx8-clock.h>
> > #include <dt-bindings/clock/imx8-lpcg.h>
> > +#include <dt-bindings/dma/fsl-edma.h>
> > #include <dt-bindings/firmware/imx/rsrc.h>
> >
> > audio_ipg_clk: clock-audio-ipg {
> > @@ -481,4 +482,307 @@ acm: acm@59e00000 {
> > "sai3_rx_bclk",
> > "sai4_rx_bclk";
> > };
> > +
> > + asrc0: asrc@59000000 {
>
> Please insert nodes sorted by address. ASRC0 is the very first node.
>
> > + compatible = "fsl,imx8qm-asrc";
> > + reg = <0x59000000 0x10000>;
> > + interrupts = <GIC_SPI 372 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&asrc0_lpcg 0>,
> > + <&asrc0_lpcg 0>,
> > + <&aud_pll_div0_lpcg 0>,
> > + <&aud_pll_div1_lpcg 0>,
> > + <&acm IMX_ADMA_ACM_AUD_CLK0_SEL>,
> > + <&acm IMX_ADMA_ACM_AUD_CLK1_SEL>,
> > + <&clk_dummy>,
> > + <&clk_dummy>,
> > + <&clk_dummy>,
> > + <&clk_dummy>,
> > + <&clk_dummy>,
> > + <&clk_dummy>,
> > + <&clk_dummy>,
> > + <&clk_dummy>,
> > + <&clk_dummy>,
> > + <&clk_dummy>,
> > + <&clk_dummy>,
> > + <&clk_dummy>,
> > + <&clk_dummy>;
> > + clock-names = "mem", "ipg",
> > + "asrck_0", "asrck_1", "asrck_2", "asrck_3",
> > + "asrck_4", "asrck_5", "asrck_6", "asrck_7",
> > + "asrck_8", "asrck_9", "asrck_a", "asrck_b",
> > + "asrck_c", "asrck_d", "asrck_e", "asrck_f",
> > + "spba";
> > + dmas = <&edma0 0 0 0>,
> > + <&edma0 1 0 0>,
> > + <&edma0 2 0 0>,
> > + <&edma0 3 0 FSL_EDMA_RX>,
> > + <&edma0 4 0 FSL_EDMA_RX>,
> > + <&edma0 5 0 FSL_EDMA_RX>;
> > + /* tx* is output channel of asrc, it is rx channel for eDMA */
> > + dma-names = "rxa", "rxb", "rxc", "txa", "txb", "txc";
> > + fsl,asrc-rate = <8000>;
> > + fsl,asrc-width = <16>;
> > + fsl,asrc-clk-map = <0>;
> > + power-domains = <&pd IMX_SC_R_ASRC_0>;
> > + status = "disabled";
> > + };
> > +
> > + esai0: esai@59010000 {
> > + compatible = "fsl,imx8qm-esai", "fsl,imx6ull-esai";
> > + reg = <0x59010000 0x10000>;
> > + interrupts = <GIC_SPI 409 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&esai0_lpcg 1>, <&esai0_lpcg 0>, <&esai0_lpcg 1>, <&clk_dummy>;
> > + clock-names = "core", "extal", "fsys", "spba";
> > + dmas = <&edma0 6 0 FSL_EDMA_RX>, <&edma0 7 0 0>;
> > + dma-names = "rx", "tx";
> > + power-domains = <&pd IMX_SC_R_ESAI_0>;
> > + status = "disabled";
> > + };
> > +
> > + spdif0: spdif@59020000 {
> > + compatible = "fsl,imx8qm-spdif";
> > + reg = <0x59020000 0x10000>;
> > + interrupts = <GIC_SPI 456 IRQ_TYPE_LEVEL_HIGH>, /* rx */
> > + <GIC_SPI 458 IRQ_TYPE_LEVEL_HIGH>; /* tx */
> > + clocks = <&spdif0_lpcg 1>, /* core */
> > + <&clk_dummy>, /* rxtx0 */
> > + <&spdif0_lpcg 0>, /* rxtx1 */
> > + <&clk_dummy>, /* rxtx2 */
> > + <&clk_dummy>, /* rxtx3 */
> > + <&clk_dummy>, /* rxtx4 */
> > + <&audio_ipg_clk>, /* rxtx5 */
> > + <&clk_dummy>, /* rxtx6 */
> > + <&clk_dummy>, /* rxtx7 */
> > + <&clk_dummy>; /* spba */
> > + clock-names = "core", "rxtx0", "rxtx1", "rxtx2", "rxtx3", "rxtx4",
> > + "rxtx5", "rxtx6", "rxtx7", "spba";
> > + dmas = <&edma0 8 0 (FSL_EDMA_MULTI_FIFO | FSL_EDMA_RX)>,
> > + <&edma0 9 0 FSL_EDMA_MULTI_FIFO>;
> > + dma-names = "rx", "tx";
> > + power-domains = <&pd IMX_SC_R_SPDIF_0>;
> > + status = "disabled";
> > + };
> > +
> > + spdif1: spdif@59030000 {
>
> That's imx8qm only, no?

I am not sure what means. why do you think it is imx8qm only? It is for
imx8qm, imx8qxp, imx8dxl.

Frank

>
> > + compatible = "fsl,imx8qm-spdif";
> > + reg = <0x59030000 0x10000>;
> > + interrupts = <GIC_SPI 460 IRQ_TYPE_LEVEL_HIGH>, /* rx */
> > + <GIC_SPI 462 IRQ_TYPE_LEVEL_HIGH>; /* tx */
> > + clocks = <&spdif1_lpcg 1>, /* core */
> > + <&clk_dummy>, /* rxtx0 */
> > + <&spdif1_lpcg 0>, /* rxtx1 */
> > + <&clk_dummy>, /* rxtx2 */
> > + <&clk_dummy>, /* rxtx3 */
> > + <&clk_dummy>, /* rxtx4 */
> > + <&audio_ipg_clk>, /* rxtx5 */
> > + <&clk_dummy>, /* rxtx6 */
> > + <&clk_dummy>, /* rxtx7 */
> > + <&clk_dummy>; /* spba */
> > + clock-names = "core", "rxtx0", "rxtx1", "rxtx2", "rxtx3", "rxtx4",
> > + "rxtx5", "rxtx6", "rxtx7", "spba";
> > + dmas = <&edma0 10 0 (FSL_EDMA_MULTI_FIFO | FSL_EDMA_RX)>,
> > + <&edma0 11 0 FSL_EDMA_MULTI_FIFO>;
> > + dma-names = "rx", "tx";
> > + power-domains = <&pd IMX_SC_R_SPDIF_1>;
> > + status = "disabled";
> > + };
> > +
> > + asrc1: asrc@59800000 {
>
> Insert this between dsp and edma1, sorted by address.
>
> > + compatible = "fsl,imx8qm-asrc";
> > + reg = <0x59800000 0x10000>;
> > + interrupts = <GIC_SPI 380 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&asrc1_lpcg 0>,
> > + <&asrc1_lpcg 0>,
> > + <&aud_pll_div0_lpcg 0>,
> > + <&aud_pll_div1_lpcg 0>,
> > + <&acm IMX_ADMA_ACM_AUD_CLK0_SEL>,
> > + <&acm IMX_ADMA_ACM_AUD_CLK1_SEL>,
> > + <&clk_dummy>,
> > + <&clk_dummy>,
> > + <&clk_dummy>,
> > + <&clk_dummy>,
> > + <&clk_dummy>,
> > + <&clk_dummy>,
> > + <&clk_dummy>,
> > + <&clk_dummy>,
> > + <&clk_dummy>,
> > + <&clk_dummy>,
> > + <&clk_dummy>,
> > + <&clk_dummy>,
> > + <&clk_dummy>;
> > + clock-names = "mem", "ipg",
> > + "asrck_0", "asrck_1", "asrck_2", "asrck_3",
> > + "asrck_4", "asrck_5", "asrck_6", "asrck_7",
> > + "asrck_8", "asrck_9", "asrck_a", "asrck_b",
> > + "asrck_c", "asrck_d", "asrck_e", "asrck_f",
> > + "spba";
> > + dmas = <&edma1 0 0 0>,
> > + <&edma1 1 0 0>,
> > + <&edma1 2 0 0>,
> > + <&edma1 3 0 FSL_EDMA_RX>,
> > + <&edma1 4 0 FSL_EDMA_RX>,
> > + <&edma1 5 0 FSL_EDMA_RX>;
> > + /* tx* is output channel of asrc, it is rx channel for eDMA */
> > + dma-names = "rxa", "rxb", "rxc", "txa", "txb", "txc";
> > + fsl,asrc-rate = <8000>;
> > + fsl,asrc-width = <16>;
> > + fsl,asrc-clk-map = <1>;
> > + power-domains = <&pd IMX_SC_R_ASRC_1>;
> > + status = "disabled";
> > + };
> > +
> > + sai4: sai@59820000 {
> > + compatible = "fsl,imx8qm-sai";
> > + reg = <0x59820000 0x10000>;
> > + interrupts = <GIC_SPI 329 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&sai4_lpcg 1>,
> > + <&clk_dummy>,
> > + <&sai4_lpcg 0>,
> > + <&clk_dummy>,
> > + <&clk_dummy>;
> > + clock-names = "bus", "mclk0", "mclk1", "mclk2", "mclk3";
> > + dmas = <&edma1 8 0 FSL_EDMA_RX>, <&edma1 9 0 0>;
> > + dma-names = "rx", "tx";
> > + power-domains = <&pd IMX_SC_R_SAI_4>;
> > + status = "disabled";
> > + };
> > +
> > + sai5: sai@59830000 {
> > + compatible = "fsl,imx8qm-sai";
> > + reg = <0x59830000 0x10000>;
> > + interrupts = <GIC_SPI 331 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&sai5_lpcg 1>,
> > + <&clk_dummy>,
> > + <&sai5_lpcg 0>,
> > + <&clk_dummy>,
> > + <&clk_dummy>;
> > + clock-names = "bus", "mclk0", "mclk1", "mclk2", "mclk3";
> > + dmas = <&edma1 10 0 0>;
> > + dma-names = "tx";
> > + power-domains = <&pd IMX_SC_R_SAI_5>;
> > + status = "disabled";
> > + };
> > +
> > + amix: amix@59840000 {
> > + compatible = "fsl,imx8qm-audmix";
> > + reg = <0x59840000 0x10000>;
> > + clocks = <&amix_lpcg 0>;
> > + clock-names = "ipg";
> > + power-domains = <&pd IMX_SC_R_AMIX>;
> > + dais = <&sai4>, <&sai5>;
> > + status = "disabled";
> > + };
> > +
> > + mqs: mqs@59850000 {
> > + compatible = "fsl,imx8qm-mqs";
> > + reg = <0x59850000 0x10000>;
> > + clocks = <&mqs0_lpcg 0>,
> > + <&mqs0_lpcg 1>;
> > + clock-names = "mclk", "core";
> > + power-domains = <&pd IMX_SC_R_MQS_0>;
> > + status = "disabled";
> > + };
> > +
> > + asrc0_lpcg: clock-controller@59400000 {
>
> Please insert he lpcg nodes according to their address.
>
> > + compatible = "fsl,imx8qxp-lpcg";
> > + reg = <0x59400000 0x10000>;
> > + #clock-cells = <1>;
> > + clocks = <&audio_ipg_clk>;
> > + clock-indices = <IMX_LPCG_CLK_4>;
> > + clock-output-names = "asrc0_lpcg_ipg_clk";
> > + power-domains = <&pd IMX_SC_R_ASRC_0>;
> > + };
> > +
> > + esai0_lpcg: clock-controller@59410000 {
> > + compatible = "fsl,imx8qxp-lpcg";
> > + reg = <0x59410000 0x10000>;
> > + #clock-cells = <1>;
> > + clocks = <&acm IMX_ADMA_ACM_ESAI0_MCLK_SEL>,
> > + <&audio_ipg_clk>;
> > + clock-indices = <IMX_LPCG_CLK_0>, <IMX_LPCG_CLK_4>;
> > + clock-output-names = "esai0_lpcg_extal_clk",
> > + "esai0_lpcg_ipg_clk";
> > + power-domains = <&pd IMX_SC_R_ESAI_0>;
> > + };
> > +
> > + spdif0_lpcg: clock-controller@59420000 {
> > + compatible = "fsl,imx8qxp-lpcg";
> > + reg = <0x59420000 0x10000>;
> > + #clock-cells = <1>;
> > + clocks = <&acm IMX_ADMA_ACM_SPDIF0_TX_CLK_SEL>,
> > + <&audio_ipg_clk>;
> > + clock-indices = <IMX_LPCG_CLK_0>, <IMX_LPCG_CLK_4>;
> > + clock-output-names = "spdif0_lpcg_tx_clk",
> > + "spdif0_lpcg_gclkw";
> > + power-domains = <&pd IMX_SC_R_SPDIF_0>;
> > + };
> > +
> > + spdif1_lpcg: clock-controller@59430000 {
>
> That's imx8qm only as well, no?
>
> Thanks and best regards,
> Alexander
>
> > + compatible = "fsl,imx8qxp-lpcg";
> > + reg = <0x59430000 0x10000>;
> > + #clock-cells = <1>;
> > + clocks = <&acm IMX_ADMA_ACM_SPDIF1_TX_CLK_SEL>,
> > + <&audio_ipg_clk>;
> > + clock-indices = <IMX_LPCG_CLK_0>, <IMX_LPCG_CLK_4>;
> > + clock-output-names = "spdif1_lpcg_tx_clk",
> > + "spdif1_lpcg_gclkw";
> > + power-domains = <&pd IMX_SC_R_SPDIF_1>;
> > + status = "disabled";
> > + };
> > +
> > + asrc1_lpcg: clock-controller@59c00000 {
> > + compatible = "fsl,imx8qxp-lpcg";
> > + reg = <0x59c00000 0x10000>;
> > + #clock-cells = <1>;
> > + clocks = <&audio_ipg_clk>;
> > + clock-indices = <IMX_LPCG_CLK_4>;
> > + clock-output-names = "asrc1_lpcg_ipg_clk";
> > + power-domains = <&pd IMX_SC_R_ASRC_1>;
> > + };
> > +
> > + sai4_lpcg: clock-controller@59c20000 {
> > + compatible = "fsl,imx8qxp-lpcg";
> > + reg = <0x59c20000 0x10000>;
> > + #clock-cells = <1>;
> > + clocks = <&acm IMX_ADMA_ACM_SAI4_MCLK_SEL>,
> > + <&audio_ipg_clk>;
> > + clock-indices = <IMX_LPCG_CLK_0>, <IMX_LPCG_CLK_4>;
> > + clock-output-names = "sai4_lpcg_mclk",
> > + "sai4_lpcg_ipg_clk";
> > + power-domains = <&pd IMX_SC_R_SAI_4>;
> > + };
> > +
> > + sai5_lpcg: clock-controller@59c30000 {
> > + compatible = "fsl,imx8qxp-lpcg";
> > + reg = <0x59c30000 0x10000>;
> > + #clock-cells = <1>;
> > + clocks = <&acm IMX_ADMA_ACM_SAI5_MCLK_SEL>,
> > + <&audio_ipg_clk>;
> > + clock-indices = <IMX_LPCG_CLK_0>, <IMX_LPCG_CLK_4>;
> > + clock-output-names = "sai5_lpcg_mclk",
> > + "sai5_lpcg_ipg_clk";
> > + power-domains = <&pd IMX_SC_R_SAI_5>;
> > + };
> > +
> > + amix_lpcg: clock-controller@59c40000 {
> > + compatible = "fsl,imx8qxp-lpcg";
> > + reg = <0x59c40000 0x10000>;
> > + #clock-cells = <1>;
> > + clocks = <&audio_ipg_clk>;
> > + clock-indices = <IMX_LPCG_CLK_0>;
> > + clock-output-names = "amix_lpcg_ipg_clk";
> > + power-domains = <&pd IMX_SC_R_AMIX>;
> > + };
> > +
> > + mqs0_lpcg: clock-controller@59c50000 {
> > + compatible = "fsl,imx8qxp-lpcg";
> > + reg = <0x59c50000 0x10000>;
> > + #clock-cells = <1>;
> > + clocks = <&acm IMX_ADMA_ACM_MQS_TX_CLK_SEL>,
> > + <&audio_ipg_clk>;
> > + clock-indices = <IMX_LPCG_CLK_0>, <IMX_LPCG_CLK_4>;
> > + clock-output-names = "mqs0_lpcg_mclk",
> > + "mqs0_lpcg_ipg_clk";
> > + power-domains = <&pd IMX_SC_R_MQS_0>;
> > + };
> > };
> >
> >
>
>
> --
> TQ-Systems GmbH | M?hlstra?e 2, Gut Delling | 82229 Seefeld, Germany
> Amtsgericht M?nchen, HRB 105018
> Gesch?ftsf?hrer: Detlef Schneider, R?diger Stahl, Stefan Schneider
> http://www.tq-group.com/
>
>

2024-03-06 18:45:27

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v4 3/4] ASoC: dt-bindings: fsl-sai: allow only one dma-names

On Tue, Mar 05, 2024 at 12:33:04PM -0500, Frank Li wrote:
> Some sai only connect one direction dma (rx/tx) in SOC. For example:
> imx8qxp sai5 only connect tx dma channel. So allow only one "rx" or "tx"
> for dma-names.
>
> Remove description under dmas because no user use index to get dma channel.
> All user use 'dma-names' to get correct dma channel. dma-names already in
> 'required' list.

Acked-by: Conor Dooley <[email protected]>

> Signed-off-by: Frank Li <[email protected]>
> ---
> Documentation/devicetree/bindings/sound/fsl,sai.yaml | 13 ++++++-------
> 1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/sound/fsl,sai.yaml b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> index 2456d958adeef..6f551c68d33db 100644
> --- a/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> +++ b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> @@ -81,15 +81,14 @@ properties:
>
> dmas:
> minItems: 1
> - items:
> - - description: DMA controller phandle and request line for RX
> - - description: DMA controller phandle and request line for TX
> + maxItems: 2
>
> dma-names:
> - minItems: 1
> - items:
> - - const: rx
> - - const: tx
> + oneOf:
> + - items:
> + - const: rx
> + - const: tx
> + - enum: [ rx, tx ]

I'm not entirely sure if this was Rob's suggestion, I got the impression
he was suggesting that in the two items case we'd not care about the
order. But while I think this is different to that suggestion it's also
not wrong.

Thanks,
Conor.


Attachments:
(No filename) (1.62 kB)
signature.asc (235.00 B)
Download all attachments

2024-03-06 18:53:43

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v4 1/4] ASoC: dt-bindings: fsl,imx-asrc/spdif: Add power-domains property

On Tue, Mar 05, 2024 at 12:33:02PM -0500, Frank Li wrote:
> Add power-domains property for asrc and spdif since fsl,imx8qm-asrc/spdif
> and fsl,imx8qxp-asrc/spdif require 'power-domains'.
>
> Set 'power-domains' as required property for compatible string
> fsl,imx8qm-asrc/spdif and fsl,imx8qxp-asrc/spdif.
>
> Signed-off-by: Frank Li <[email protected]>

Acked-by: Conor Dooley <[email protected]>

Cheers,
Conor.


Attachments:
(No filename) (438.00 B)
signature.asc (235.00 B)
Download all attachments

2024-03-06 18:57:20

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v4 2/4] ASoC: dt-bindings: fsl,imx-asrc: update max interrupt numbers

On Tue, Mar 05, 2024 at 12:33:03PM -0500, Frank Li wrote:
> fsl,imx8qxp-spdif and fsl,imx8qm-spdif have 2 interrupts. Other platforms
> have 1 interrupt.
>
> Increase max interrupt number to 2 and add restriction for platforms except
> i.MX8QXP and i.MX8QM.
>
> Signed-off-by: Frank Li <[email protected]>

Acked-by: Conor Dooley <[email protected]>

Thanks,
Conor.


Attachments:
(No filename) (389.00 B)
signature.asc (235.00 B)
Download all attachments

2024-03-06 19:26:32

by Frank Li

[permalink] [raw]
Subject: Re: [PATCH v4 3/4] ASoC: dt-bindings: fsl-sai: allow only one dma-names

On Wed, Mar 06, 2024 at 06:45:13PM +0000, Conor Dooley wrote:
> On Tue, Mar 05, 2024 at 12:33:04PM -0500, Frank Li wrote:
> > Some sai only connect one direction dma (rx/tx) in SOC. For example:
> > imx8qxp sai5 only connect tx dma channel. So allow only one "rx" or "tx"
> > for dma-names.
> >
> > Remove description under dmas because no user use index to get dma channel.
> > All user use 'dma-names' to get correct dma channel. dma-names already in
> > 'required' list.
>
> Acked-by: Conor Dooley <[email protected]>
>
> > Signed-off-by: Frank Li <[email protected]>
> > ---
> > Documentation/devicetree/bindings/sound/fsl,sai.yaml | 13 ++++++-------
> > 1 file changed, 6 insertions(+), 7 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/sound/fsl,sai.yaml b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > index 2456d958adeef..6f551c68d33db 100644
> > --- a/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > +++ b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > @@ -81,15 +81,14 @@ properties:
> >
> > dmas:
> > minItems: 1
> > - items:
> > - - description: DMA controller phandle and request line for RX
> > - - description: DMA controller phandle and request line for TX
> > + maxItems: 2
> >
> > dma-names:
> > - minItems: 1
> > - items:
> > - - const: rx
> > - - const: tx
> > + oneOf:
> > + - items:
> > + - const: rx
> > + - const: tx
> > + - enum: [ rx, tx ]
>
> I'm not entirely sure if this was Rob's suggestion, I got the impression
> he was suggesting that in the two items case we'd not care about the
> order. But while I think this is different to that suggestion it's also
> not wrong.

I log this at cover-letter. b4 can't support write change log at every
patch yet. Rob's suggest was not work. dt-binding check complain too long
if there are two dma-names = "rx", "tx".

Frank

>
> Thanks,
> Conor.



2024-03-06 19:45:45

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v4 3/4] ASoC: dt-bindings: fsl-sai: allow only one dma-names

On Wed, Mar 06, 2024 at 02:25:53PM -0500, Frank Li wrote:
> On Wed, Mar 06, 2024 at 06:45:13PM +0000, Conor Dooley wrote:
> > On Tue, Mar 05, 2024 at 12:33:04PM -0500, Frank Li wrote:
> > > Some sai only connect one direction dma (rx/tx) in SOC. For example:
> > > imx8qxp sai5 only connect tx dma channel. So allow only one "rx" or "tx"
> > > for dma-names.
> > >
> > > Remove description under dmas because no user use index to get dma channel.
> > > All user use 'dma-names' to get correct dma channel. dma-names already in
> > > 'required' list.
> >
> > Acked-by: Conor Dooley <[email protected]>
> >
> > > Signed-off-by: Frank Li <[email protected]>
> > > ---
> > > Documentation/devicetree/bindings/sound/fsl,sai.yaml | 13 ++++++-------
> > > 1 file changed, 6 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/sound/fsl,sai.yaml b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > index 2456d958adeef..6f551c68d33db 100644
> > > --- a/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > +++ b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > @@ -81,15 +81,14 @@ properties:
> > >
> > > dmas:
> > > minItems: 1
> > > - items:
> > > - - description: DMA controller phandle and request line for RX
> > > - - description: DMA controller phandle and request line for TX
> > > + maxItems: 2
> > >
> > > dma-names:
> > > - minItems: 1
> > > - items:
> > > - - const: rx
> > > - - const: tx
> > > + oneOf:
> > > + - items:
> > > + - const: rx
> > > + - const: tx
> > > + - enum: [ rx, tx ]
> >
> > I'm not entirely sure if this was Rob's suggestion, I got the impression
> > he was suggesting that in the two items case we'd not care about the
> > order. But while I think this is different to that suggestion it's also
> > not wrong.
>
> I log this at cover-letter. b4 can't support write change log at every
> patch yet. Rob's suggest was not work. dt-binding check complain too long
> if there are two dma-names = "rx", "tx".

Yeh, that is what happens if you just have
dma-names:
enum: [ rx, tx ]
since that only allows one entry. Take a look at st,stm32-uart.yaml
instead, for example.


Attachments:
(No filename) (2.25 kB)
signature.asc (235.00 B)
Download all attachments

2024-03-06 20:25:26

by Frank Li

[permalink] [raw]
Subject: Re: [PATCH v4 3/4] ASoC: dt-bindings: fsl-sai: allow only one dma-names

On Wed, Mar 06, 2024 at 07:45:34PM +0000, Conor Dooley wrote:
> On Wed, Mar 06, 2024 at 02:25:53PM -0500, Frank Li wrote:
> > On Wed, Mar 06, 2024 at 06:45:13PM +0000, Conor Dooley wrote:
> > > On Tue, Mar 05, 2024 at 12:33:04PM -0500, Frank Li wrote:
> > > > Some sai only connect one direction dma (rx/tx) in SOC. For example:
> > > > imx8qxp sai5 only connect tx dma channel. So allow only one "rx" or "tx"
> > > > for dma-names.
> > > >
> > > > Remove description under dmas because no user use index to get dma channel.
> > > > All user use 'dma-names' to get correct dma channel. dma-names already in
> > > > 'required' list.
> > >
> > > Acked-by: Conor Dooley <[email protected]>
> > >
> > > > Signed-off-by: Frank Li <[email protected]>
> > > > ---
> > > > Documentation/devicetree/bindings/sound/fsl,sai.yaml | 13 ++++++-------
> > > > 1 file changed, 6 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/sound/fsl,sai.yaml b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > > index 2456d958adeef..6f551c68d33db 100644
> > > > --- a/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > > +++ b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > > @@ -81,15 +81,14 @@ properties:
> > > >
> > > > dmas:
> > > > minItems: 1
> > > > - items:
> > > > - - description: DMA controller phandle and request line for RX
> > > > - - description: DMA controller phandle and request line for TX
> > > > + maxItems: 2
> > > >
> > > > dma-names:
> > > > - minItems: 1
> > > > - items:
> > > > - - const: rx
> > > > - - const: tx
> > > > + oneOf:
> > > > + - items:
> > > > + - const: rx
> > > > + - const: tx
> > > > + - enum: [ rx, tx ]
> > >
> > > I'm not entirely sure if this was Rob's suggestion, I got the impression
> > > he was suggesting that in the two items case we'd not care about the
> > > order. But while I think this is different to that suggestion it's also
> > > not wrong.
> >
> > I log this at cover-letter. b4 can't support write change log at every
> > patch yet. Rob's suggest was not work. dt-binding check complain too long
> > if there are two dma-names = "rx", "tx".
>
> Yeh, that is what happens if you just have
> dma-names:
> enum: [ rx, tx ]
> since that only allows one entry. Take a look at st,stm32-uart.yaml
> instead, for example.

Yes, it works. Thank you very much. I will update it at next version.

Frank


2024-03-06 20:31:26

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v4 2/4] ASoC: dt-bindings: fsl,imx-asrc: update max interrupt numbers

On Tue, Mar 05, 2024 at 12:33:03PM -0500, Frank Li wrote:
> fsl,imx8qxp-spdif and fsl,imx8qm-spdif have 2 interrupts. Other platforms
> have 1 interrupt.
>
> Increase max interrupt number to 2 and add restriction for platforms except
> i.MX8QXP and i.MX8QM.
>
> Signed-off-by: Frank Li <[email protected]>
> ---
> Documentation/devicetree/bindings/sound/fsl,spdif.yaml | 18 +++++++++++++++++-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/sound/fsl,spdif.yaml b/Documentation/devicetree/bindings/sound/fsl,spdif.yaml
> index 56f8c0c8afdea..7f6590708e1ec 100644
> --- a/Documentation/devicetree/bindings/sound/fsl,spdif.yaml
> +++ b/Documentation/devicetree/bindings/sound/fsl,spdif.yaml
> @@ -31,7 +31,8 @@ properties:
> maxItems: 1
>
> interrupts:
> - maxItems: 1
> + minItems: 1
> + maxItems: 2
>
> dmas:
> items:
> @@ -101,6 +102,21 @@ required:
> additionalProperties: false
>
> allOf:
> + - if:
> + properties:
> + compatible:
> + enum:
> + - fsl,imx8qm-spdif
> + - fsl,imx8qxp-spdif
> + then:
> + properties:
> + interrupts:
> + minItems: 2

Nowhere is it explained what the 2 interrupts are or what the order of
them is. Is the first interrupt the same as before, and there is a new
interrupt for something else? Or it's the same interrupts, but they've
just split up into 2?

Rob

2024-03-06 20:54:01

by Frank Li

[permalink] [raw]
Subject: Re: [PATCH v4 2/4] ASoC: dt-bindings: fsl,imx-asrc: update max interrupt numbers

On Wed, Mar 06, 2024 at 02:25:37PM -0600, Rob Herring wrote:
> On Tue, Mar 05, 2024 at 12:33:03PM -0500, Frank Li wrote:
> > fsl,imx8qxp-spdif and fsl,imx8qm-spdif have 2 interrupts. Other platforms
> > have 1 interrupt.
> >
> > Increase max interrupt number to 2 and add restriction for platforms except
> > i.MX8QXP and i.MX8QM.
> >
> > Signed-off-by: Frank Li <[email protected]>
> > ---
> > Documentation/devicetree/bindings/sound/fsl,spdif.yaml | 18 +++++++++++++++++-
> > 1 file changed, 17 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/sound/fsl,spdif.yaml b/Documentation/devicetree/bindings/sound/fsl,spdif.yaml
> > index 56f8c0c8afdea..7f6590708e1ec 100644
> > --- a/Documentation/devicetree/bindings/sound/fsl,spdif.yaml
> > +++ b/Documentation/devicetree/bindings/sound/fsl,spdif.yaml
> > @@ -31,7 +31,8 @@ properties:
> > maxItems: 1
> >
> > interrupts:
> > - maxItems: 1
> > + minItems: 1
> > + maxItems: 2
> >
> > dmas:
> > items:
> > @@ -101,6 +102,21 @@ required:
> > additionalProperties: false
> >
> > allOf:
> > + - if:
> > + properties:
> > + compatible:
> > + enum:
> > + - fsl,imx8qm-spdif
> > + - fsl,imx8qxp-spdif
> > + then:
> > + properties:
> > + interrupts:
> > + minItems: 2
>
> Nowhere is it explained what the 2 interrupts are or what the order of
> them is. Is the first interrupt the same as before, and there is a new
> interrupt for something else? Or it's the same interrupts, but they've
> just split up into 2?

It is same one and split to tx and rx. Order doesn't matter. They uses one
irq handle.

Frank

>
> Rob

2024-03-06 20:55:34

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v4 3/4] ASoC: dt-bindings: fsl-sai: allow only one dma-names

On Wed, Mar 06, 2024 at 02:25:53PM -0500, Frank Li wrote:
> On Wed, Mar 06, 2024 at 06:45:13PM +0000, Conor Dooley wrote:
> > On Tue, Mar 05, 2024 at 12:33:04PM -0500, Frank Li wrote:
> > > Some sai only connect one direction dma (rx/tx) in SOC. For example:
> > > imx8qxp sai5 only connect tx dma channel. So allow only one "rx" or "tx"
> > > for dma-names.
> > >
> > > Remove description under dmas because no user use index to get dma channel.
> > > All user use 'dma-names' to get correct dma channel. dma-names already in
> > > 'required' list.
> >
> > Acked-by: Conor Dooley <[email protected]>
> >
> > > Signed-off-by: Frank Li <[email protected]>
> > > ---
> > > Documentation/devicetree/bindings/sound/fsl,sai.yaml | 13 ++++++-------
> > > 1 file changed, 6 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/sound/fsl,sai.yaml b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > index 2456d958adeef..6f551c68d33db 100644
> > > --- a/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > +++ b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > @@ -81,15 +81,14 @@ properties:
> > >
> > > dmas:
> > > minItems: 1
> > > - items:
> > > - - description: DMA controller phandle and request line for RX
> > > - - description: DMA controller phandle and request line for TX
> > > + maxItems: 2
> > >
> > > dma-names:
> > > - minItems: 1
> > > - items:
> > > - - const: rx
> > > - - const: tx
> > > + oneOf:
> > > + - items:
> > > + - const: rx
> > > + - const: tx
> > > + - enum: [ rx, tx ]
> >
> > I'm not entirely sure if this was Rob's suggestion, I got the impression
> > he was suggesting that in the two items case we'd not care about the
> > order. But while I think this is different to that suggestion it's also
> > not wrong.
>
> I log this at cover-letter. b4 can't support write change log at every
> patch yet.

It never will (probably). That's because it doesn't need to. You can
just do it with git. When you edit the commit message, then after the
tags, Add '---' and put whatever you want after. That works as long as
the commit is applied from a patch as 'git am' will drop it.

> Rob's suggest was not work. dt-binding check complain too long
> if there are two dma-names = "rx", "tx".

So I'm wrong or you didn't have it correct? No way to tell with your
explanation. Let me give you the exact schema:

dma-names:
minItems: 1
items:
- enum: [ rx, tx ]
- const: tx

This says we can have 1 or 2 entries. The first entry can be either rx
or tx. The 2nd entry must be tx. That's what you want. However, '"tx",
"tx"' is allowed with the above, but we enforce items to be unique
elsewhere. Or I thought we did, but we relaxed '.*-names$' at some
point. I'm going to fix that now.

Rob

2024-03-06 21:19:35

by Frank Li

[permalink] [raw]
Subject: Re: [PATCH v4 3/4] ASoC: dt-bindings: fsl-sai: allow only one dma-names

On Wed, Mar 06, 2024 at 02:55:24PM -0600, Rob Herring wrote:
> On Wed, Mar 06, 2024 at 02:25:53PM -0500, Frank Li wrote:
> > On Wed, Mar 06, 2024 at 06:45:13PM +0000, Conor Dooley wrote:
> > > On Tue, Mar 05, 2024 at 12:33:04PM -0500, Frank Li wrote:
> > > > Some sai only connect one direction dma (rx/tx) in SOC. For example:
> > > > imx8qxp sai5 only connect tx dma channel. So allow only one "rx" or "tx"
> > > > for dma-names.
> > > >
> > > > Remove description under dmas because no user use index to get dma channel.
> > > > All user use 'dma-names' to get correct dma channel. dma-names already in
> > > > 'required' list.
> > >
> > > Acked-by: Conor Dooley <[email protected]>
> > >
> > > > Signed-off-by: Frank Li <[email protected]>
> > > > ---
> > > > Documentation/devicetree/bindings/sound/fsl,sai.yaml | 13 ++++++-------
> > > > 1 file changed, 6 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/sound/fsl,sai.yaml b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > > index 2456d958adeef..6f551c68d33db 100644
> > > > --- a/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > > +++ b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > > @@ -81,15 +81,14 @@ properties:
> > > >
> > > > dmas:
> > > > minItems: 1
> > > > - items:
> > > > - - description: DMA controller phandle and request line for RX
> > > > - - description: DMA controller phandle and request line for TX
> > > > + maxItems: 2
> > > >
> > > > dma-names:
> > > > - minItems: 1
> > > > - items:
> > > > - - const: rx
> > > > - - const: tx
> > > > + oneOf:
> > > > + - items:
> > > > + - const: rx
> > > > + - const: tx
> > > > + - enum: [ rx, tx ]
> > >
> > > I'm not entirely sure if this was Rob's suggestion, I got the impression
> > > he was suggesting that in the two items case we'd not care about the
> > > order. But while I think this is different to that suggestion it's also
> > > not wrong.
> >
> > I log this at cover-letter. b4 can't support write change log at every
> > patch yet.
>
> It never will (probably). That's because it doesn't need to. You can
> just do it with git. When you edit the commit message, then after the
> tags, Add '---' and put whatever you want after. That works as long as
> the commit is applied from a patch as 'git am' will drop it.

The key problem is that I don't want to lost notes when respin patches. It
is easy to make mistake when I copy old serise change logs.

Previously I use git notes + git-rebase, it work fine. Notes can be kept
when I do rebase and git commit --amend. But one thing is not good.
git send-email --to-cmd=./script/get_maintainer.sh *.patch. It can't
combine all patches's maitainer to a list. It looks like difference patch
will be difference --to list.

b4 can help some case. But can't keep git-notes information when rebase.
It should be git bugs or feature missed.

>
> > Rob's suggest was not work. dt-binding check complain too long
> > if there are two dma-names = "rx", "tx".
>
> So I'm wrong or you didn't have it correct? No way to tell with your
> explanation. Let me give you the exact schema:
>
> dma-names:
> minItems: 1
> items:
> - enum: [ rx, tx ]
> - const: tx
>
> This says we can have 1 or 2 entries. The first entry can be either rx
> or tx. The 2nd entry must be tx. That's what you want. However, '"tx",
> "tx"' is allowed with the above, but we enforce items to be unique
> elsewhere. Or I thought we did, but we relaxed '.*-names$' at some
> point. I'm going to fix that now.

Conor find out my problem. The below code works. I missed maxItems.

dma-names:
minItems: 1
maxItems: 2
items
enum: [ rx, tx ]

Frank

>
> Rob

2024-03-06 22:58:50

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v4 3/4] ASoC: dt-bindings: fsl-sai: allow only one dma-names

On Wed, Mar 6, 2024 at 3:19 PM Frank Li <[email protected]> wrote:
>
> On Wed, Mar 06, 2024 at 02:55:24PM -0600, Rob Herring wrote:
> > On Wed, Mar 06, 2024 at 02:25:53PM -0500, Frank Li wrote:
> > > On Wed, Mar 06, 2024 at 06:45:13PM +0000, Conor Dooley wrote:
> > > > On Tue, Mar 05, 2024 at 12:33:04PM -0500, Frank Li wrote:
> > > > > Some sai only connect one direction dma (rx/tx) in SOC. For example:
> > > > > imx8qxp sai5 only connect tx dma channel. So allow only one "rx" or "tx"
> > > > > for dma-names.
> > > > >
> > > > > Remove description under dmas because no user use index to get dma channel.
> > > > > All user use 'dma-names' to get correct dma channel. dma-names already in
> > > > > 'required' list.
> > > >
> > > > Acked-by: Conor Dooley <[email protected]>
> > > >
> > > > > Signed-off-by: Frank Li <[email protected]>
> > > > > ---
> > > > > Documentation/devicetree/bindings/sound/fsl,sai.yaml | 13 ++++++-------
> > > > > 1 file changed, 6 insertions(+), 7 deletions(-)
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/sound/fsl,sai.yaml b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > > > index 2456d958adeef..6f551c68d33db 100644
> > > > > --- a/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > > > +++ b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > > > @@ -81,15 +81,14 @@ properties:
> > > > >
> > > > > dmas:
> > > > > minItems: 1
> > > > > - items:
> > > > > - - description: DMA controller phandle and request line for RX
> > > > > - - description: DMA controller phandle and request line for TX
> > > > > + maxItems: 2
> > > > >
> > > > > dma-names:
> > > > > - minItems: 1
> > > > > - items:
> > > > > - - const: rx
> > > > > - - const: tx
> > > > > + oneOf:
> > > > > + - items:
> > > > > + - const: rx
> > > > > + - const: tx
> > > > > + - enum: [ rx, tx ]
> > > >
> > > > I'm not entirely sure if this was Rob's suggestion, I got the impression
> > > > he was suggesting that in the two items case we'd not care about the
> > > > order. But while I think this is different to that suggestion it's also
> > > > not wrong.
> > >
> > > I log this at cover-letter. b4 can't support write change log at every
> > > patch yet.
> >
> > It never will (probably). That's because it doesn't need to. You can
> > just do it with git. When you edit the commit message, then after the
> > tags, Add '---' and put whatever you want after. That works as long as
> > the commit is applied from a patch as 'git am' will drop it.
>
> The key problem is that I don't want to lost notes when respin patches. It
> is easy to make mistake when I copy old serise change logs.

You don't. It is all saved in the commit. When you rebase for the next
version, you just add to the changes history.

> Previously I use git notes + git-rebase, it work fine. Notes can be kept
> when I do rebase and git commit --amend. But one thing is not good.
> git send-email --to-cmd=./script/get_maintainer.sh *.patch. It can't
> combine all patches's maitainer to a list. It looks like difference patch
> will be difference --to list.
>
> b4 can help some case. But can't keep git-notes information when rebase.
> It should be git bugs or feature missed.

git-notes always seemed clunky and to be extra work to me. Just having
the changelog in the commit msg is a lot easier.

> > > Rob's suggest was not work. dt-binding check complain too long
> > > if there are two dma-names = "rx", "tx".
> >
> > So I'm wrong or you didn't have it correct? No way to tell with your
> > explanation. Let me give you the exact schema:
> >
> > dma-names:
> > minItems: 1
> > items:
> > - enum: [ rx, tx ]
> > - const: tx
> >
> > This says we can have 1 or 2 entries. The first entry can be either rx
> > or tx. The 2nd entry must be tx. That's what you want. However, '"tx",
> > "tx"' is allowed with the above, but we enforce items to be unique
> > elsewhere. Or I thought we did, but we relaxed '.*-names$' at some
> > point. I'm going to fix that now.
>
> Conor find out my problem. The below code works. I missed maxItems.
>
> dma-names:
> minItems: 1
> maxItems: 2
> items

Missing ':'

> enum: [ rx, tx ]

That is not my suggestion. This would be my 3rd choice after what I
proposed or what you had already. Please plug in exactly what I told
you and report back what doesn't work.

Rob

2024-03-06 23:01:40

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v4 2/4] ASoC: dt-bindings: fsl,imx-asrc: update max interrupt numbers

On Wed, Mar 6, 2024 at 2:53 PM Frank Li <[email protected]> wrote:
>
> On Wed, Mar 06, 2024 at 02:25:37PM -0600, Rob Herring wrote:
> > On Tue, Mar 05, 2024 at 12:33:03PM -0500, Frank Li wrote:
> > > fsl,imx8qxp-spdif and fsl,imx8qm-spdif have 2 interrupts. Other platforms
> > > have 1 interrupt.
> > >
> > > Increase max interrupt number to 2 and add restriction for platforms except
> > > i.MX8QXP and i.MX8QM.
> > >
> > > Signed-off-by: Frank Li <[email protected]>
> > > ---
> > > Documentation/devicetree/bindings/sound/fsl,spdif.yaml | 18 +++++++++++++++++-
> > > 1 file changed, 17 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/sound/fsl,spdif.yaml b/Documentation/devicetree/bindings/sound/fsl,spdif.yaml
> > > index 56f8c0c8afdea..7f6590708e1ec 100644
> > > --- a/Documentation/devicetree/bindings/sound/fsl,spdif.yaml
> > > +++ b/Documentation/devicetree/bindings/sound/fsl,spdif.yaml
> > > @@ -31,7 +31,8 @@ properties:
> > > maxItems: 1
> > >
> > > interrupts:
> > > - maxItems: 1
> > > + minItems: 1
> > > + maxItems: 2
> > >
> > > dmas:
> > > items:
> > > @@ -101,6 +102,21 @@ required:
> > > additionalProperties: false
> > >
> > > allOf:
> > > + - if:
> > > + properties:
> > > + compatible:
> > > + enum:
> > > + - fsl,imx8qm-spdif
> > > + - fsl,imx8qxp-spdif
> > > + then:
> > > + properties:
> > > + interrupts:
> > > + minItems: 2
> >
> > Nowhere is it explained what the 2 interrupts are or what the order of
> > them is. Is the first interrupt the same as before, and there is a new
> > interrupt for something else? Or it's the same interrupts, but they've
> > just split up into 2?
>
> It is same one and split to tx and rx. Order doesn't matter.

Make that clear in the schema. Order does matter and you have to
define the order in the schema.

> They uses one
> irq handle.

What does that mean?

Rob

2024-03-07 02:03:00

by Frank Li

[permalink] [raw]
Subject: Re: [PATCH v4 3/4] ASoC: dt-bindings: fsl-sai: allow only one dma-names

On Wed, Mar 06, 2024 at 04:58:22PM -0600, Rob Herring wrote:
> On Wed, Mar 6, 2024 at 3:19 PM Frank Li <[email protected]> wrote:
> >
> > On Wed, Mar 06, 2024 at 02:55:24PM -0600, Rob Herring wrote:
> > > On Wed, Mar 06, 2024 at 02:25:53PM -0500, Frank Li wrote:
> > > > On Wed, Mar 06, 2024 at 06:45:13PM +0000, Conor Dooley wrote:
> > > > > On Tue, Mar 05, 2024 at 12:33:04PM -0500, Frank Li wrote:
> > > > > > Some sai only connect one direction dma (rx/tx) in SOC. For example:
> > > > > > imx8qxp sai5 only connect tx dma channel. So allow only one "rx" or "tx"
> > > > > > for dma-names.
> > > > > >
> > > > > > Remove description under dmas because no user use index to get dma channel.
> > > > > > All user use 'dma-names' to get correct dma channel. dma-names already in
> > > > > > 'required' list.
> > > > >
> > > > > Acked-by: Conor Dooley <[email protected]>
> > > > >
> > > > > > Signed-off-by: Frank Li <[email protected]>
> > > > > > ---
> > > > > > Documentation/devicetree/bindings/sound/fsl,sai.yaml | 13 ++++++-------
> > > > > > 1 file changed, 6 insertions(+), 7 deletions(-)
> > > > > >
> > > > > > diff --git a/Documentation/devicetree/bindings/sound/fsl,sai.yaml b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > > > > index 2456d958adeef..6f551c68d33db 100644
> > > > > > --- a/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > > > > +++ b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > > > > @@ -81,15 +81,14 @@ properties:
> > > > > >
> > > > > > dmas:
> > > > > > minItems: 1
> > > > > > - items:
> > > > > > - - description: DMA controller phandle and request line for RX
> > > > > > - - description: DMA controller phandle and request line for TX
> > > > > > + maxItems: 2
> > > > > >
> > > > > > dma-names:
> > > > > > - minItems: 1
> > > > > > - items:
> > > > > > - - const: rx
> > > > > > - - const: tx
> > > > > > + oneOf:
> > > > > > + - items:
> > > > > > + - const: rx
> > > > > > + - const: tx
> > > > > > + - enum: [ rx, tx ]
> > > > >
> > > > > I'm not entirely sure if this was Rob's suggestion, I got the impression
> > > > > he was suggesting that in the two items case we'd not care about the
> > > > > order. But while I think this is different to that suggestion it's also
> > > > > not wrong.
> > > >
> > > > I log this at cover-letter. b4 can't support write change log at every
> > > > patch yet.
> > >
> > > It never will (probably). That's because it doesn't need to. You can
> > > just do it with git. When you edit the commit message, then after the
> > > tags, Add '---' and put whatever you want after. That works as long as
> > > the commit is applied from a patch as 'git am' will drop it.
> >
> > The key problem is that I don't want to lost notes when respin patches. It
> > is easy to make mistake when I copy old serise change logs.
>
> You don't. It is all saved in the commit. When you rebase for the next
> version, you just add to the changes history.
>
> > Previously I use git notes + git-rebase, it work fine. Notes can be kept
> > when I do rebase and git commit --amend. But one thing is not good.
> > git send-email --to-cmd=./script/get_maintainer.sh *.patch. It can't
> > combine all patches's maitainer to a list. It looks like difference patch
> > will be difference --to list.
> >
> > b4 can help some case. But can't keep git-notes information when rebase.
> > It should be git bugs or feature missed.
>
> git-notes always seemed clunky and to be extra work to me. Just having
> the changelog in the commit msg is a lot easier.
>
> > > > Rob's suggest was not work. dt-binding check complain too long
> > > > if there are two dma-names = "rx", "tx".
> > >
> > > So I'm wrong or you didn't have it correct? No way to tell with your
> > > explanation. Let me give you the exact schema:
> > >
> > > dma-names:
> > > minItems: 1
> > > items:
> > > - enum: [ rx, tx ]
> > > - const: tx
> > >
> > > This says we can have 1 or 2 entries. The first entry can be either rx
> > > or tx. The 2nd entry must be tx. That's what you want. However, '"tx",
> > > "tx"' is allowed with the above, but we enforce items to be unique
> > > elsewhere. Or I thought we did, but we relaxed '.*-names$' at some
> > > point. I'm going to fix that now.
> >
> > Conor find out my problem. The below code works. I missed maxItems.
> >
> > dma-names:
> > minItems: 1
> > maxItems: 2
> > items
>
> Missing ':'
>
> > enum: [ rx, tx ]
>
> That is not my suggestion. This would be my 3rd choice after what I
> proposed or what you had already. Please plug in exactly what I told
> you and report back what doesn't work.

It also works. look likes conor's method is a little bit easy to be
understood.

Frank
>
> Rob

2024-03-07 07:22:43

by Alexander Stein

[permalink] [raw]
Subject: Re: [PATCH v4 4/4] arm64: dts: imx8qxp: add asrc[0, 1], esai0, spdif[0, 1] and sai[4, 5]

Hi Frank,

Am Mittwoch, 6. M?rz 2024, 16:19:18 CET schrieb Frank Li:
> On Wed, Mar 06, 2024 at 08:20:00AM +0100, Alexander Stein wrote:
> > Hi Frank,
> >
> > thanks for the patch.
> >
> > Am Dienstag, 5. M?rz 2024, 18:33:05 CET schrieb Frank Li:
> > > Add asrc[0,1], esai0, spdif[0,1], sai[4,5] and related lpcg node for
> > > imx8 audio subsystem.
> > >
> > > Signed-off-by: Frank Li <[email protected]>
> > > ---
> > > arch/arm64/boot/dts/freescale/imx8-ss-audio.dtsi | 304 +++++++++++++++++++++++
> > > 1 file changed, 304 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/freescale/imx8-ss-audio.dtsi b/arch/arm64/boot/dts/freescale/imx8-ss-audio.dtsi
> > > index 07afeb78ed564..78305559f15c9 100644
> > > --- a/arch/arm64/boot/dts/freescale/imx8-ss-audio.dtsi
> > > +++ b/arch/arm64/boot/dts/freescale/imx8-ss-audio.dtsi
> > > @@ -6,6 +6,7 @@
> > >
> > > #include <dt-bindings/clock/imx8-clock.h>
> > > #include <dt-bindings/clock/imx8-lpcg.h>
> > > +#include <dt-bindings/dma/fsl-edma.h>
> > > #include <dt-bindings/firmware/imx/rsrc.h>
> > >
> > > audio_ipg_clk: clock-audio-ipg {
> > > @@ -481,4 +482,307 @@ acm: acm@59e00000 {
> > > "sai3_rx_bclk",
> > > "sai4_rx_bclk";
> > > };
> > > +
> > > + asrc0: asrc@59000000 {
> >
> > Please insert nodes sorted by address. ASRC0 is the very first node.
> >
> > > + compatible = "fsl,imx8qm-asrc";
> > > + reg = <0x59000000 0x10000>;
> > > + interrupts = <GIC_SPI 372 IRQ_TYPE_LEVEL_HIGH>;
> > > + clocks = <&asrc0_lpcg 0>,
> > > + <&asrc0_lpcg 0>,
> > > + <&aud_pll_div0_lpcg 0>,
> > > + <&aud_pll_div1_lpcg 0>,
> > > + <&acm IMX_ADMA_ACM_AUD_CLK0_SEL>,
> > > + <&acm IMX_ADMA_ACM_AUD_CLK1_SEL>,
> > > + <&clk_dummy>,
> > > + <&clk_dummy>,
> > > + <&clk_dummy>,
> > > + <&clk_dummy>,
> > > + <&clk_dummy>,
> > > + <&clk_dummy>,
> > > + <&clk_dummy>,
> > > + <&clk_dummy>,
> > > + <&clk_dummy>,
> > > + <&clk_dummy>,
> > > + <&clk_dummy>,
> > > + <&clk_dummy>,
> > > + <&clk_dummy>;
> > > + clock-names = "mem", "ipg",
> > > + "asrck_0", "asrck_1", "asrck_2", "asrck_3",
> > > + "asrck_4", "asrck_5", "asrck_6", "asrck_7",
> > > + "asrck_8", "asrck_9", "asrck_a", "asrck_b",
> > > + "asrck_c", "asrck_d", "asrck_e", "asrck_f",
> > > + "spba";
> > > + dmas = <&edma0 0 0 0>,
> > > + <&edma0 1 0 0>,
> > > + <&edma0 2 0 0>,
> > > + <&edma0 3 0 FSL_EDMA_RX>,
> > > + <&edma0 4 0 FSL_EDMA_RX>,
> > > + <&edma0 5 0 FSL_EDMA_RX>;
> > > + /* tx* is output channel of asrc, it is rx channel for eDMA */
> > > + dma-names = "rxa", "rxb", "rxc", "txa", "txb", "txc";
> > > + fsl,asrc-rate = <8000>;
> > > + fsl,asrc-width = <16>;
> > > + fsl,asrc-clk-map = <0>;
> > > + power-domains = <&pd IMX_SC_R_ASRC_0>;
> > > + status = "disabled";
> > > + };
> > > +
> > > + esai0: esai@59010000 {
> > > + compatible = "fsl,imx8qm-esai", "fsl,imx6ull-esai";
> > > + reg = <0x59010000 0x10000>;
> > > + interrupts = <GIC_SPI 409 IRQ_TYPE_LEVEL_HIGH>;
> > > + clocks = <&esai0_lpcg 1>, <&esai0_lpcg 0>, <&esai0_lpcg 1>, <&clk_dummy>;
> > > + clock-names = "core", "extal", "fsys", "spba";
> > > + dmas = <&edma0 6 0 FSL_EDMA_RX>, <&edma0 7 0 0>;
> > > + dma-names = "rx", "tx";
> > > + power-domains = <&pd IMX_SC_R_ESAI_0>;
> > > + status = "disabled";
> > > + };
> > > +
> > > + spdif0: spdif@59020000 {
> > > + compatible = "fsl,imx8qm-spdif";
> > > + reg = <0x59020000 0x10000>;
> > > + interrupts = <GIC_SPI 456 IRQ_TYPE_LEVEL_HIGH>, /* rx */
> > > + <GIC_SPI 458 IRQ_TYPE_LEVEL_HIGH>; /* tx */
> > > + clocks = <&spdif0_lpcg 1>, /* core */
> > > + <&clk_dummy>, /* rxtx0 */
> > > + <&spdif0_lpcg 0>, /* rxtx1 */
> > > + <&clk_dummy>, /* rxtx2 */
> > > + <&clk_dummy>, /* rxtx3 */
> > > + <&clk_dummy>, /* rxtx4 */
> > > + <&audio_ipg_clk>, /* rxtx5 */
> > > + <&clk_dummy>, /* rxtx6 */
> > > + <&clk_dummy>, /* rxtx7 */
> > > + <&clk_dummy>; /* spba */
> > > + clock-names = "core", "rxtx0", "rxtx1", "rxtx2", "rxtx3", "rxtx4",
> > > + "rxtx5", "rxtx6", "rxtx7", "spba";
> > > + dmas = <&edma0 8 0 (FSL_EDMA_MULTI_FIFO | FSL_EDMA_RX)>,
> > > + <&edma0 9 0 FSL_EDMA_MULTI_FIFO>;
> > > + dma-names = "rx", "tx";
> > > + power-domains = <&pd IMX_SC_R_SPDIF_0>;
> > > + status = "disabled";
> > > + };
> > > +
> > > + spdif1: spdif@59030000 {
> >
> > That's imx8qm only, no?
>
> I am not sure what means. why do you think it is imx8qm only? It is for
> imx8qm, imx8qxp, imx8dxl.

According to Table- 2-6 (Audio DMA memory Map) in i.MX8X RM Rev. 0 05/2020,
the lasted one available on the webpage, address 0x59030000 is reserved.
I read that as there is no periphery available. This matches the feature list
in 1.1.2 Features, where "1x SPDIF" is stated.

So spdif1 is only for imx8qm (no idea about imx8dxl though) and should be
listed in a file called imx8qm-ss-audio.dtsi which is only included in
imx8qm.dtsi.

Thanks and best regards
Alexander

>
> Frank
>
> >
> > > + compatible = "fsl,imx8qm-spdif";
> > > + reg = <0x59030000 0x10000>;
> > > + interrupts = <GIC_SPI 460 IRQ_TYPE_LEVEL_HIGH>, /* rx */
> > > + <GIC_SPI 462 IRQ_TYPE_LEVEL_HIGH>; /* tx */
> > > + clocks = <&spdif1_lpcg 1>, /* core */
> > > + <&clk_dummy>, /* rxtx0 */
> > > + <&spdif1_lpcg 0>, /* rxtx1 */
> > > + <&clk_dummy>, /* rxtx2 */
> > > + <&clk_dummy>, /* rxtx3 */
> > > + <&clk_dummy>, /* rxtx4 */
> > > + <&audio_ipg_clk>, /* rxtx5 */
> > > + <&clk_dummy>, /* rxtx6 */
> > > + <&clk_dummy>, /* rxtx7 */
> > > + <&clk_dummy>; /* spba */
> > > + clock-names = "core", "rxtx0", "rxtx1", "rxtx2", "rxtx3", "rxtx4",
> > > + "rxtx5", "rxtx6", "rxtx7", "spba";
> > > + dmas = <&edma0 10 0 (FSL_EDMA_MULTI_FIFO | FSL_EDMA_RX)>,
> > > + <&edma0 11 0 FSL_EDMA_MULTI_FIFO>;
> > > + dma-names = "rx", "tx";
> > > + power-domains = <&pd IMX_SC_R_SPDIF_1>;
> > > + status = "disabled";
> > > + };
> > > +
> > > + asrc1: asrc@59800000 {
> >
> > Insert this between dsp and edma1, sorted by address.
> >
> > > + compatible = "fsl,imx8qm-asrc";
> > > + reg = <0x59800000 0x10000>;
> > > + interrupts = <GIC_SPI 380 IRQ_TYPE_LEVEL_HIGH>;
> > > + clocks = <&asrc1_lpcg 0>,
> > > + <&asrc1_lpcg 0>,
> > > + <&aud_pll_div0_lpcg 0>,
> > > + <&aud_pll_div1_lpcg 0>,
> > > + <&acm IMX_ADMA_ACM_AUD_CLK0_SEL>,
> > > + <&acm IMX_ADMA_ACM_AUD_CLK1_SEL>,
> > > + <&clk_dummy>,
> > > + <&clk_dummy>,
> > > + <&clk_dummy>,
> > > + <&clk_dummy>,
> > > + <&clk_dummy>,
> > > + <&clk_dummy>,
> > > + <&clk_dummy>,
> > > + <&clk_dummy>,
> > > + <&clk_dummy>,
> > > + <&clk_dummy>,
> > > + <&clk_dummy>,
> > > + <&clk_dummy>,
> > > + <&clk_dummy>;
> > > + clock-names = "mem", "ipg",
> > > + "asrck_0", "asrck_1", "asrck_2", "asrck_3",
> > > + "asrck_4", "asrck_5", "asrck_6", "asrck_7",
> > > + "asrck_8", "asrck_9", "asrck_a", "asrck_b",
> > > + "asrck_c", "asrck_d", "asrck_e", "asrck_f",
> > > + "spba";
> > > + dmas = <&edma1 0 0 0>,
> > > + <&edma1 1 0 0>,
> > > + <&edma1 2 0 0>,
> > > + <&edma1 3 0 FSL_EDMA_RX>,
> > > + <&edma1 4 0 FSL_EDMA_RX>,
> > > + <&edma1 5 0 FSL_EDMA_RX>;
> > > + /* tx* is output channel of asrc, it is rx channel for eDMA */
> > > + dma-names = "rxa", "rxb", "rxc", "txa", "txb", "txc";
> > > + fsl,asrc-rate = <8000>;
> > > + fsl,asrc-width = <16>;
> > > + fsl,asrc-clk-map = <1>;
> > > + power-domains = <&pd IMX_SC_R_ASRC_1>;
> > > + status = "disabled";
> > > + };
> > > +
> > > + sai4: sai@59820000 {
> > > + compatible = "fsl,imx8qm-sai";
> > > + reg = <0x59820000 0x10000>;
> > > + interrupts = <GIC_SPI 329 IRQ_TYPE_LEVEL_HIGH>;
> > > + clocks = <&sai4_lpcg 1>,
> > > + <&clk_dummy>,
> > > + <&sai4_lpcg 0>,
> > > + <&clk_dummy>,
> > > + <&clk_dummy>;
> > > + clock-names = "bus", "mclk0", "mclk1", "mclk2", "mclk3";
> > > + dmas = <&edma1 8 0 FSL_EDMA_RX>, <&edma1 9 0 0>;
> > > + dma-names = "rx", "tx";
> > > + power-domains = <&pd IMX_SC_R_SAI_4>;
> > > + status = "disabled";
> > > + };
> > > +
> > > + sai5: sai@59830000 {
> > > + compatible = "fsl,imx8qm-sai";
> > > + reg = <0x59830000 0x10000>;
> > > + interrupts = <GIC_SPI 331 IRQ_TYPE_LEVEL_HIGH>;
> > > + clocks = <&sai5_lpcg 1>,
> > > + <&clk_dummy>,
> > > + <&sai5_lpcg 0>,
> > > + <&clk_dummy>,
> > > + <&clk_dummy>;
> > > + clock-names = "bus", "mclk0", "mclk1", "mclk2", "mclk3";
> > > + dmas = <&edma1 10 0 0>;
> > > + dma-names = "tx";
> > > + power-domains = <&pd IMX_SC_R_SAI_5>;
> > > + status = "disabled";
> > > + };
> > > +
> > > + amix: amix@59840000 {
> > > + compatible = "fsl,imx8qm-audmix";
> > > + reg = <0x59840000 0x10000>;
> > > + clocks = <&amix_lpcg 0>;
> > > + clock-names = "ipg";
> > > + power-domains = <&pd IMX_SC_R_AMIX>;
> > > + dais = <&sai4>, <&sai5>;
> > > + status = "disabled";
> > > + };
> > > +
> > > + mqs: mqs@59850000 {
> > > + compatible = "fsl,imx8qm-mqs";
> > > + reg = <0x59850000 0x10000>;
> > > + clocks = <&mqs0_lpcg 0>,
> > > + <&mqs0_lpcg 1>;
> > > + clock-names = "mclk", "core";
> > > + power-domains = <&pd IMX_SC_R_MQS_0>;
> > > + status = "disabled";
> > > + };
> > > +
> > > + asrc0_lpcg: clock-controller@59400000 {
> >
> > Please insert he lpcg nodes according to their address.
> >
> > > + compatible = "fsl,imx8qxp-lpcg";
> > > + reg = <0x59400000 0x10000>;
> > > + #clock-cells = <1>;
> > > + clocks = <&audio_ipg_clk>;
> > > + clock-indices = <IMX_LPCG_CLK_4>;
> > > + clock-output-names = "asrc0_lpcg_ipg_clk";
> > > + power-domains = <&pd IMX_SC_R_ASRC_0>;
> > > + };
> > > +
> > > + esai0_lpcg: clock-controller@59410000 {
> > > + compatible = "fsl,imx8qxp-lpcg";
> > > + reg = <0x59410000 0x10000>;
> > > + #clock-cells = <1>;
> > > + clocks = <&acm IMX_ADMA_ACM_ESAI0_MCLK_SEL>,
> > > + <&audio_ipg_clk>;
> > > + clock-indices = <IMX_LPCG_CLK_0>, <IMX_LPCG_CLK_4>;
> > > + clock-output-names = "esai0_lpcg_extal_clk",
> > > + "esai0_lpcg_ipg_clk";
> > > + power-domains = <&pd IMX_SC_R_ESAI_0>;
> > > + };
> > > +
> > > + spdif0_lpcg: clock-controller@59420000 {
> > > + compatible = "fsl,imx8qxp-lpcg";
> > > + reg = <0x59420000 0x10000>;
> > > + #clock-cells = <1>;
> > > + clocks = <&acm IMX_ADMA_ACM_SPDIF0_TX_CLK_SEL>,
> > > + <&audio_ipg_clk>;
> > > + clock-indices = <IMX_LPCG_CLK_0>, <IMX_LPCG_CLK_4>;
> > > + clock-output-names = "spdif0_lpcg_tx_clk",
> > > + "spdif0_lpcg_gclkw";
> > > + power-domains = <&pd IMX_SC_R_SPDIF_0>;
> > > + };
> > > +
> > > + spdif1_lpcg: clock-controller@59430000 {
> >
> > That's imx8qm only as well, no?
> >
> > Thanks and best regards,
> > Alexander
> >
> > > + compatible = "fsl,imx8qxp-lpcg";
> > > + reg = <0x59430000 0x10000>;
> > > + #clock-cells = <1>;
> > > + clocks = <&acm IMX_ADMA_ACM_SPDIF1_TX_CLK_SEL>,
> > > + <&audio_ipg_clk>;
> > > + clock-indices = <IMX_LPCG_CLK_0>, <IMX_LPCG_CLK_4>;
> > > + clock-output-names = "spdif1_lpcg_tx_clk",
> > > + "spdif1_lpcg_gclkw";
> > > + power-domains = <&pd IMX_SC_R_SPDIF_1>;
> > > + status = "disabled";
> > > + };
> > > +
> > > + asrc1_lpcg: clock-controller@59c00000 {
> > > + compatible = "fsl,imx8qxp-lpcg";
> > > + reg = <0x59c00000 0x10000>;
> > > + #clock-cells = <1>;
> > > + clocks = <&audio_ipg_clk>;
> > > + clock-indices = <IMX_LPCG_CLK_4>;
> > > + clock-output-names = "asrc1_lpcg_ipg_clk";
> > > + power-domains = <&pd IMX_SC_R_ASRC_1>;
> > > + };
> > > +
> > > + sai4_lpcg: clock-controller@59c20000 {
> > > + compatible = "fsl,imx8qxp-lpcg";
> > > + reg = <0x59c20000 0x10000>;
> > > + #clock-cells = <1>;
> > > + clocks = <&acm IMX_ADMA_ACM_SAI4_MCLK_SEL>,
> > > + <&audio_ipg_clk>;
> > > + clock-indices = <IMX_LPCG_CLK_0>, <IMX_LPCG_CLK_4>;
> > > + clock-output-names = "sai4_lpcg_mclk",
> > > + "sai4_lpcg_ipg_clk";
> > > + power-domains = <&pd IMX_SC_R_SAI_4>;
> > > + };
> > > +
> > > + sai5_lpcg: clock-controller@59c30000 {
> > > + compatible = "fsl,imx8qxp-lpcg";
> > > + reg = <0x59c30000 0x10000>;
> > > + #clock-cells = <1>;
> > > + clocks = <&acm IMX_ADMA_ACM_SAI5_MCLK_SEL>,
> > > + <&audio_ipg_clk>;
> > > + clock-indices = <IMX_LPCG_CLK_0>, <IMX_LPCG_CLK_4>;
> > > + clock-output-names = "sai5_lpcg_mclk",
> > > + "sai5_lpcg_ipg_clk";
> > > + power-domains = <&pd IMX_SC_R_SAI_5>;
> > > + };
> > > +
> > > + amix_lpcg: clock-controller@59c40000 {
> > > + compatible = "fsl,imx8qxp-lpcg";
> > > + reg = <0x59c40000 0x10000>;
> > > + #clock-cells = <1>;
> > > + clocks = <&audio_ipg_clk>;
> > > + clock-indices = <IMX_LPCG_CLK_0>;
> > > + clock-output-names = "amix_lpcg_ipg_clk";
> > > + power-domains = <&pd IMX_SC_R_AMIX>;
> > > + };
> > > +
> > > + mqs0_lpcg: clock-controller@59c50000 {
> > > + compatible = "fsl,imx8qxp-lpcg";
> > > + reg = <0x59c50000 0x10000>;
> > > + #clock-cells = <1>;
> > > + clocks = <&acm IMX_ADMA_ACM_MQS_TX_CLK_SEL>,
> > > + <&audio_ipg_clk>;
> > > + clock-indices = <IMX_LPCG_CLK_0>, <IMX_LPCG_CLK_4>;
> > > + clock-output-names = "mqs0_lpcg_mclk",
> > > + "mqs0_lpcg_ipg_clk";
> > > + power-domains = <&pd IMX_SC_R_MQS_0>;
> > > + };
> > > };
> > >
> > >
> >
> >
>
>


--
TQ-Systems GmbH | M?hlstra?e 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht M?nchen, HRB 105018
Gesch?ftsf?hrer: Detlef Schneider, R?diger Stahl, Stefan Schneider
http://www.tq-group.com/



2024-03-07 16:27:44

by Frank Li

[permalink] [raw]
Subject: Re: [PATCH v4 4/4] arm64: dts: imx8qxp: add asrc[0, 1], esai0, spdif[0, 1] and sai[4, 5]

On Thu, Mar 07, 2024 at 08:22:25AM +0100, Alexander Stein wrote:
> Hi Frank,
>
> Am Mittwoch, 6. M?rz 2024, 16:19:18 CET schrieb Frank Li:
> > On Wed, Mar 06, 2024 at 08:20:00AM +0100, Alexander Stein wrote:
> > > Hi Frank,
> > >
> > > thanks for the patch.
> > >
> > > Am Dienstag, 5. M?rz 2024, 18:33:05 CET schrieb Frank Li:
> > > > Add asrc[0,1], esai0, spdif[0,1], sai[4,5] and related lpcg node for
> > > > imx8 audio subsystem.
> > > >
> > > > Signed-off-by: Frank Li <[email protected]>
> > > > ---
> > > > arch/arm64/boot/dts/freescale/imx8-ss-audio.dtsi | 304 +++++++++++++++++++++++
> > > > 1 file changed, 304 insertions(+)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/freescale/imx8-ss-audio.dtsi b/arch/arm64/boot/dts/freescale/imx8-ss-audio.dtsi
> > > > index 07afeb78ed564..78305559f15c9 100644
> > > > --- a/arch/arm64/boot/dts/freescale/imx8-ss-audio.dtsi
> > > > +++ b/arch/arm64/boot/dts/freescale/imx8-ss-audio.dtsi
> > > > @@ -6,6 +6,7 @@
> > > >
> > > > #include <dt-bindings/clock/imx8-clock.h>
> > > > #include <dt-bindings/clock/imx8-lpcg.h>
> > > > +#include <dt-bindings/dma/fsl-edma.h>
> > > > #include <dt-bindings/firmware/imx/rsrc.h>
> > > >
> > > > audio_ipg_clk: clock-audio-ipg {
> > > > @@ -481,4 +482,307 @@ acm: acm@59e00000 {
> > > > "sai3_rx_bclk",
> > > > "sai4_rx_bclk";
> > > > };
> > > > +
> > > > + asrc0: asrc@59000000 {
> > >
> > > Please insert nodes sorted by address. ASRC0 is the very first node.
> > >
> > > > + compatible = "fsl,imx8qm-asrc";
> > > > + reg = <0x59000000 0x10000>;
> > > > + interrupts = <GIC_SPI 372 IRQ_TYPE_LEVEL_HIGH>;
> > > > + clocks = <&asrc0_lpcg 0>,
> > > > + <&asrc0_lpcg 0>,
> > > > + <&aud_pll_div0_lpcg 0>,
> > > > + <&aud_pll_div1_lpcg 0>,
> > > > + <&acm IMX_ADMA_ACM_AUD_CLK0_SEL>,
> > > > + <&acm IMX_ADMA_ACM_AUD_CLK1_SEL>,
> > > > + <&clk_dummy>,
> > > > + <&clk_dummy>,
> > > > + <&clk_dummy>,
> > > > + <&clk_dummy>,
> > > > + <&clk_dummy>,
> > > > + <&clk_dummy>,
> > > > + <&clk_dummy>,
> > > > + <&clk_dummy>,
> > > > + <&clk_dummy>,
> > > > + <&clk_dummy>,
> > > > + <&clk_dummy>,
> > > > + <&clk_dummy>,
> > > > + <&clk_dummy>;
> > > > + clock-names = "mem", "ipg",
> > > > + "asrck_0", "asrck_1", "asrck_2", "asrck_3",
> > > > + "asrck_4", "asrck_5", "asrck_6", "asrck_7",
> > > > + "asrck_8", "asrck_9", "asrck_a", "asrck_b",
> > > > + "asrck_c", "asrck_d", "asrck_e", "asrck_f",
> > > > + "spba";
> > > > + dmas = <&edma0 0 0 0>,
> > > > + <&edma0 1 0 0>,
> > > > + <&edma0 2 0 0>,
> > > > + <&edma0 3 0 FSL_EDMA_RX>,
> > > > + <&edma0 4 0 FSL_EDMA_RX>,
> > > > + <&edma0 5 0 FSL_EDMA_RX>;
> > > > + /* tx* is output channel of asrc, it is rx channel for eDMA */
> > > > + dma-names = "rxa", "rxb", "rxc", "txa", "txb", "txc";
> > > > + fsl,asrc-rate = <8000>;
> > > > + fsl,asrc-width = <16>;
> > > > + fsl,asrc-clk-map = <0>;
> > > > + power-domains = <&pd IMX_SC_R_ASRC_0>;
> > > > + status = "disabled";
> > > > + };
> > > > +
> > > > + esai0: esai@59010000 {
> > > > + compatible = "fsl,imx8qm-esai", "fsl,imx6ull-esai";
> > > > + reg = <0x59010000 0x10000>;
> > > > + interrupts = <GIC_SPI 409 IRQ_TYPE_LEVEL_HIGH>;
> > > > + clocks = <&esai0_lpcg 1>, <&esai0_lpcg 0>, <&esai0_lpcg 1>, <&clk_dummy>;
> > > > + clock-names = "core", "extal", "fsys", "spba";
> > > > + dmas = <&edma0 6 0 FSL_EDMA_RX>, <&edma0 7 0 0>;
> > > > + dma-names = "rx", "tx";
> > > > + power-domains = <&pd IMX_SC_R_ESAI_0>;
> > > > + status = "disabled";
> > > > + };
> > > > +
> > > > + spdif0: spdif@59020000 {
> > > > + compatible = "fsl,imx8qm-spdif";
> > > > + reg = <0x59020000 0x10000>;
> > > > + interrupts = <GIC_SPI 456 IRQ_TYPE_LEVEL_HIGH>, /* rx */
> > > > + <GIC_SPI 458 IRQ_TYPE_LEVEL_HIGH>; /* tx */
> > > > + clocks = <&spdif0_lpcg 1>, /* core */
> > > > + <&clk_dummy>, /* rxtx0 */
> > > > + <&spdif0_lpcg 0>, /* rxtx1 */
> > > > + <&clk_dummy>, /* rxtx2 */
> > > > + <&clk_dummy>, /* rxtx3 */
> > > > + <&clk_dummy>, /* rxtx4 */
> > > > + <&audio_ipg_clk>, /* rxtx5 */
> > > > + <&clk_dummy>, /* rxtx6 */
> > > > + <&clk_dummy>, /* rxtx7 */
> > > > + <&clk_dummy>; /* spba */
> > > > + clock-names = "core", "rxtx0", "rxtx1", "rxtx2", "rxtx3", "rxtx4",
> > > > + "rxtx5", "rxtx6", "rxtx7", "spba";
> > > > + dmas = <&edma0 8 0 (FSL_EDMA_MULTI_FIFO | FSL_EDMA_RX)>,
> > > > + <&edma0 9 0 FSL_EDMA_MULTI_FIFO>;
> > > > + dma-names = "rx", "tx";
> > > > + power-domains = <&pd IMX_SC_R_SPDIF_0>;
> > > > + status = "disabled";
> > > > + };
> > > > +
> > > > + spdif1: spdif@59030000 {
> > >
> > > That's imx8qm only, no?
> >
> > I am not sure what means. why do you think it is imx8qm only? It is for
> > imx8qm, imx8qxp, imx8dxl.
>
> According to Table- 2-6 (Audio DMA memory Map) in i.MX8X RM Rev. 0 05/2020,
> the lasted one available on the webpage, address 0x59030000 is reserved.
> I read that as there is no periphery available. This matches the feature list
> in 1.1.2 Features, where "1x SPDIF" is stated.
>
> So spdif1 is only for imx8qm (no idea about imx8dxl though) and should be
> listed in a file called imx8qm-ss-audio.dtsi which is only included in
> imx8qm.dtsi.
>

Thanks, I will do that.

> Thanks and best regards
> Alexander
>
> >
> > Frank
> >
> > >
> > > > + compatible = "fsl,imx8qm-spdif";
> > > > + reg = <0x59030000 0x10000>;
> > > > + interrupts = <GIC_SPI 460 IRQ_TYPE_LEVEL_HIGH>, /* rx */
> > > > + <GIC_SPI 462 IRQ_TYPE_LEVEL_HIGH>; /* tx */
> > > > + clocks = <&spdif1_lpcg 1>, /* core */
> > > > + <&clk_dummy>, /* rxtx0 */
> > > > + <&spdif1_lpcg 0>, /* rxtx1 */
> > > > + <&clk_dummy>, /* rxtx2 */
> > > > + <&clk_dummy>, /* rxtx3 */
> > > > + <&clk_dummy>, /* rxtx4 */
> > > > + <&audio_ipg_clk>, /* rxtx5 */
> > > > + <&clk_dummy>, /* rxtx6 */
> > > > + <&clk_dummy>, /* rxtx7 */
> > > > + <&clk_dummy>; /* spba */
> > > > + clock-names = "core", "rxtx0", "rxtx1", "rxtx2", "rxtx3", "rxtx4",
> > > > + "rxtx5", "rxtx6", "rxtx7", "spba";
> > > > + dmas = <&edma0 10 0 (FSL_EDMA_MULTI_FIFO | FSL_EDMA_RX)>,
> > > > + <&edma0 11 0 FSL_EDMA_MULTI_FIFO>;
> > > > + dma-names = "rx", "tx";
> > > > + power-domains = <&pd IMX_SC_R_SPDIF_1>;
> > > > + status = "disabled";
> > > > + };
> > > > +
> > > > + asrc1: asrc@59800000 {
> > >
> > > Insert this between dsp and edma1, sorted by address.
> > >
> > > > + compatible = "fsl,imx8qm-asrc";
> > > > + reg = <0x59800000 0x10000>;
> > > > + interrupts = <GIC_SPI 380 IRQ_TYPE_LEVEL_HIGH>;
> > > > + clocks = <&asrc1_lpcg 0>,
> > > > + <&asrc1_lpcg 0>,
> > > > + <&aud_pll_div0_lpcg 0>,
> > > > + <&aud_pll_div1_lpcg 0>,
> > > > + <&acm IMX_ADMA_ACM_AUD_CLK0_SEL>,
> > > > + <&acm IMX_ADMA_ACM_AUD_CLK1_SEL>,
> > > > + <&clk_dummy>,
> > > > + <&clk_dummy>,
> > > > + <&clk_dummy>,
> > > > + <&clk_dummy>,
> > > > + <&clk_dummy>,
> > > > + <&clk_dummy>,
> > > > + <&clk_dummy>,
> > > > + <&clk_dummy>,
> > > > + <&clk_dummy>,
> > > > + <&clk_dummy>,
> > > > + <&clk_dummy>,
> > > > + <&clk_dummy>,
> > > > + <&clk_dummy>;
> > > > + clock-names = "mem", "ipg",
> > > > + "asrck_0", "asrck_1", "asrck_2", "asrck_3",
> > > > + "asrck_4", "asrck_5", "asrck_6", "asrck_7",
> > > > + "asrck_8", "asrck_9", "asrck_a", "asrck_b",
> > > > + "asrck_c", "asrck_d", "asrck_e", "asrck_f",
> > > > + "spba";
> > > > + dmas = <&edma1 0 0 0>,
> > > > + <&edma1 1 0 0>,
> > > > + <&edma1 2 0 0>,
> > > > + <&edma1 3 0 FSL_EDMA_RX>,
> > > > + <&edma1 4 0 FSL_EDMA_RX>,
> > > > + <&edma1 5 0 FSL_EDMA_RX>;
> > > > + /* tx* is output channel of asrc, it is rx channel for eDMA */
> > > > + dma-names = "rxa", "rxb", "rxc", "txa", "txb", "txc";
> > > > + fsl,asrc-rate = <8000>;
> > > > + fsl,asrc-width = <16>;
> > > > + fsl,asrc-clk-map = <1>;
> > > > + power-domains = <&pd IMX_SC_R_ASRC_1>;
> > > > + status = "disabled";
> > > > + };
> > > > +
> > > > + sai4: sai@59820000 {
> > > > + compatible = "fsl,imx8qm-sai";
> > > > + reg = <0x59820000 0x10000>;
> > > > + interrupts = <GIC_SPI 329 IRQ_TYPE_LEVEL_HIGH>;
> > > > + clocks = <&sai4_lpcg 1>,
> > > > + <&clk_dummy>,
> > > > + <&sai4_lpcg 0>,
> > > > + <&clk_dummy>,
> > > > + <&clk_dummy>;
> > > > + clock-names = "bus", "mclk0", "mclk1", "mclk2", "mclk3";
> > > > + dmas = <&edma1 8 0 FSL_EDMA_RX>, <&edma1 9 0 0>;
> > > > + dma-names = "rx", "tx";
> > > > + power-domains = <&pd IMX_SC_R_SAI_4>;
> > > > + status = "disabled";
> > > > + };
> > > > +
> > > > + sai5: sai@59830000 {
> > > > + compatible = "fsl,imx8qm-sai";
> > > > + reg = <0x59830000 0x10000>;
> > > > + interrupts = <GIC_SPI 331 IRQ_TYPE_LEVEL_HIGH>;
> > > > + clocks = <&sai5_lpcg 1>,
> > > > + <&clk_dummy>,
> > > > + <&sai5_lpcg 0>,
> > > > + <&clk_dummy>,
> > > > + <&clk_dummy>;
> > > > + clock-names = "bus", "mclk0", "mclk1", "mclk2", "mclk3";
> > > > + dmas = <&edma1 10 0 0>;
> > > > + dma-names = "tx";
> > > > + power-domains = <&pd IMX_SC_R_SAI_5>;
> > > > + status = "disabled";
> > > > + };
> > > > +
> > > > + amix: amix@59840000 {
> > > > + compatible = "fsl,imx8qm-audmix";
> > > > + reg = <0x59840000 0x10000>;
> > > > + clocks = <&amix_lpcg 0>;
> > > > + clock-names = "ipg";
> > > > + power-domains = <&pd IMX_SC_R_AMIX>;
> > > > + dais = <&sai4>, <&sai5>;
> > > > + status = "disabled";
> > > > + };
> > > > +
> > > > + mqs: mqs@59850000 {
> > > > + compatible = "fsl,imx8qm-mqs";
> > > > + reg = <0x59850000 0x10000>;
> > > > + clocks = <&mqs0_lpcg 0>,
> > > > + <&mqs0_lpcg 1>;
> > > > + clock-names = "mclk", "core";
> > > > + power-domains = <&pd IMX_SC_R_MQS_0>;
> > > > + status = "disabled";
> > > > + };
> > > > +
> > > > + asrc0_lpcg: clock-controller@59400000 {
> > >
> > > Please insert he lpcg nodes according to their address.
> > >
> > > > + compatible = "fsl,imx8qxp-lpcg";
> > > > + reg = <0x59400000 0x10000>;
> > > > + #clock-cells = <1>;
> > > > + clocks = <&audio_ipg_clk>;
> > > > + clock-indices = <IMX_LPCG_CLK_4>;
> > > > + clock-output-names = "asrc0_lpcg_ipg_clk";
> > > > + power-domains = <&pd IMX_SC_R_ASRC_0>;
> > > > + };
> > > > +
> > > > + esai0_lpcg: clock-controller@59410000 {
> > > > + compatible = "fsl,imx8qxp-lpcg";
> > > > + reg = <0x59410000 0x10000>;
> > > > + #clock-cells = <1>;
> > > > + clocks = <&acm IMX_ADMA_ACM_ESAI0_MCLK_SEL>,
> > > > + <&audio_ipg_clk>;
> > > > + clock-indices = <IMX_LPCG_CLK_0>, <IMX_LPCG_CLK_4>;
> > > > + clock-output-names = "esai0_lpcg_extal_clk",
> > > > + "esai0_lpcg_ipg_clk";
> > > > + power-domains = <&pd IMX_SC_R_ESAI_0>;
> > > > + };
> > > > +
> > > > + spdif0_lpcg: clock-controller@59420000 {
> > > > + compatible = "fsl,imx8qxp-lpcg";
> > > > + reg = <0x59420000 0x10000>;
> > > > + #clock-cells = <1>;
> > > > + clocks = <&acm IMX_ADMA_ACM_SPDIF0_TX_CLK_SEL>,
> > > > + <&audio_ipg_clk>;
> > > > + clock-indices = <IMX_LPCG_CLK_0>, <IMX_LPCG_CLK_4>;
> > > > + clock-output-names = "spdif0_lpcg_tx_clk",
> > > > + "spdif0_lpcg_gclkw";
> > > > + power-domains = <&pd IMX_SC_R_SPDIF_0>;
> > > > + };
> > > > +
> > > > + spdif1_lpcg: clock-controller@59430000 {
> > >
> > > That's imx8qm only as well, no?
> > >
> > > Thanks and best regards,
> > > Alexander
> > >
> > > > + compatible = "fsl,imx8qxp-lpcg";
> > > > + reg = <0x59430000 0x10000>;
> > > > + #clock-cells = <1>;
> > > > + clocks = <&acm IMX_ADMA_ACM_SPDIF1_TX_CLK_SEL>,
> > > > + <&audio_ipg_clk>;
> > > > + clock-indices = <IMX_LPCG_CLK_0>, <IMX_LPCG_CLK_4>;
> > > > + clock-output-names = "spdif1_lpcg_tx_clk",
> > > > + "spdif1_lpcg_gclkw";
> > > > + power-domains = <&pd IMX_SC_R_SPDIF_1>;
> > > > + status = "disabled";
> > > > + };
> > > > +
> > > > + asrc1_lpcg: clock-controller@59c00000 {
> > > > + compatible = "fsl,imx8qxp-lpcg";
> > > > + reg = <0x59c00000 0x10000>;
> > > > + #clock-cells = <1>;
> > > > + clocks = <&audio_ipg_clk>;
> > > > + clock-indices = <IMX_LPCG_CLK_4>;
> > > > + clock-output-names = "asrc1_lpcg_ipg_clk";
> > > > + power-domains = <&pd IMX_SC_R_ASRC_1>;
> > > > + };
> > > > +
> > > > + sai4_lpcg: clock-controller@59c20000 {
> > > > + compatible = "fsl,imx8qxp-lpcg";
> > > > + reg = <0x59c20000 0x10000>;
> > > > + #clock-cells = <1>;
> > > > + clocks = <&acm IMX_ADMA_ACM_SAI4_MCLK_SEL>,
> > > > + <&audio_ipg_clk>;
> > > > + clock-indices = <IMX_LPCG_CLK_0>, <IMX_LPCG_CLK_4>;
> > > > + clock-output-names = "sai4_lpcg_mclk",
> > > > + "sai4_lpcg_ipg_clk";
> > > > + power-domains = <&pd IMX_SC_R_SAI_4>;
> > > > + };
> > > > +
> > > > + sai5_lpcg: clock-controller@59c30000 {
> > > > + compatible = "fsl,imx8qxp-lpcg";
> > > > + reg = <0x59c30000 0x10000>;
> > > > + #clock-cells = <1>;
> > > > + clocks = <&acm IMX_ADMA_ACM_SAI5_MCLK_SEL>,
> > > > + <&audio_ipg_clk>;
> > > > + clock-indices = <IMX_LPCG_CLK_0>, <IMX_LPCG_CLK_4>;
> > > > + clock-output-names = "sai5_lpcg_mclk",
> > > > + "sai5_lpcg_ipg_clk";
> > > > + power-domains = <&pd IMX_SC_R_SAI_5>;
> > > > + };
> > > > +
> > > > + amix_lpcg: clock-controller@59c40000 {
> > > > + compatible = "fsl,imx8qxp-lpcg";
> > > > + reg = <0x59c40000 0x10000>;
> > > > + #clock-cells = <1>;
> > > > + clocks = <&audio_ipg_clk>;
> > > > + clock-indices = <IMX_LPCG_CLK_0>;
> > > > + clock-output-names = "amix_lpcg_ipg_clk";
> > > > + power-domains = <&pd IMX_SC_R_AMIX>;
> > > > + };
> > > > +
> > > > + mqs0_lpcg: clock-controller@59c50000 {
> > > > + compatible = "fsl,imx8qxp-lpcg";
> > > > + reg = <0x59c50000 0x10000>;
> > > > + #clock-cells = <1>;
> > > > + clocks = <&acm IMX_ADMA_ACM_MQS_TX_CLK_SEL>,
> > > > + <&audio_ipg_clk>;
> > > > + clock-indices = <IMX_LPCG_CLK_0>, <IMX_LPCG_CLK_4>;
> > > > + clock-output-names = "mqs0_lpcg_mclk",
> > > > + "mqs0_lpcg_ipg_clk";
> > > > + power-domains = <&pd IMX_SC_R_MQS_0>;
> > > > + };
> > > > };
> > > >
> > > >
> > >
> > >
> >
> >
>
>
> --
> TQ-Systems GmbH | M?hlstra?e 2, Gut Delling | 82229 Seefeld, Germany
> Amtsgericht M?nchen, HRB 105018
> Gesch?ftsf?hrer: Detlef Schneider, R?diger Stahl, Stefan Schneider
> http://www.tq-group.com/
>
>

2024-03-07 18:56:43

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v4 3/4] ASoC: dt-bindings: fsl-sai: allow only one dma-names

On Wed, Mar 06, 2024 at 09:02:38PM -0500, Frank Li wrote:
> On Wed, Mar 06, 2024 at 04:58:22PM -0600, Rob Herring wrote:
> > On Wed, Mar 6, 2024 at 3:19 PM Frank Li <[email protected]> wrote:
> > >
> > > On Wed, Mar 06, 2024 at 02:55:24PM -0600, Rob Herring wrote:
> > > > On Wed, Mar 06, 2024 at 02:25:53PM -0500, Frank Li wrote:
> > > > > On Wed, Mar 06, 2024 at 06:45:13PM +0000, Conor Dooley wrote:
> > > > > > On Tue, Mar 05, 2024 at 12:33:04PM -0500, Frank Li wrote:
> > > > > > > Some sai only connect one direction dma (rx/tx) in SOC. For example:
> > > > > > > imx8qxp sai5 only connect tx dma channel. So allow only one "rx" or "tx"
> > > > > > > for dma-names.
> > > > > > >
> > > > > > > Remove description under dmas because no user use index to get dma channel.
> > > > > > > All user use 'dma-names' to get correct dma channel. dma-names already in
> > > > > > > 'required' list.
> > > > > >
> > > > > > Acked-by: Conor Dooley <[email protected]>
> > > > > >
> > > > > > > Signed-off-by: Frank Li <[email protected]>
> > > > > > > ---
> > > > > > > Documentation/devicetree/bindings/sound/fsl,sai.yaml | 13 ++++++-------
> > > > > > > 1 file changed, 6 insertions(+), 7 deletions(-)
> > > > > > >
> > > > > > > diff --git a/Documentation/devicetree/bindings/sound/fsl,sai.yaml b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > > > > > index 2456d958adeef..6f551c68d33db 100644
> > > > > > > --- a/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > > > > > +++ b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > > > > > @@ -81,15 +81,14 @@ properties:
> > > > > > >
> > > > > > > dmas:
> > > > > > > minItems: 1
> > > > > > > - items:
> > > > > > > - - description: DMA controller phandle and request line for RX
> > > > > > > - - description: DMA controller phandle and request line for TX
> > > > > > > + maxItems: 2
> > > > > > >
> > > > > > > dma-names:
> > > > > > > - minItems: 1
> > > > > > > - items:
> > > > > > > - - const: rx
> > > > > > > - - const: tx
> > > > > > > + oneOf:
> > > > > > > + - items:
> > > > > > > + - const: rx
> > > > > > > + - const: tx
> > > > > > > + - enum: [ rx, tx ]
> > > > > >
> > > > > > I'm not entirely sure if this was Rob's suggestion, I got the impression
> > > > > > he was suggesting that in the two items case we'd not care about the
> > > > > > order. But while I think this is different to that suggestion it's also
> > > > > > not wrong.

> > > > > Rob's suggest was not work. dt-binding check complain too long
> > > > > if there are two dma-names = "rx", "tx".
> > > >
> > > > So I'm wrong or you didn't have it correct? No way to tell with your
> > > > explanation. Let me give you the exact schema:
> > > >
> > > > dma-names:
> > > > minItems: 1
> > > > items:
> > > > - enum: [ rx, tx ]
> > > > - const: tx
> > > >
> > > > This says we can have 1 or 2 entries. The first entry can be either rx
> > > > or tx. The 2nd entry must be tx. That's what you want. However, '"tx",
> > > > "tx"' is allowed with the above, but we enforce items to be unique
> > > > elsewhere. Or I thought we did, but we relaxed '.*-names$' at some
> > > > point. I'm going to fix that now.
> > >
> > > Conor find out my problem. The below code works. I missed maxItems.
> > >
> > > dma-names:
> > > minItems: 1
> > > maxItems: 2
> > > items
> >
> > Missing ':'
> >
> > > enum: [ rx, tx ]
> >
> > That is not my suggestion. This would be my 3rd choice after what I
> > proposed or what you had already. Please plug in exactly what I told
> > you and report back what doesn't work.
>
> It also works. look likes conor's method is a little bit easy to be
> understood.

What I suggested is different, it is more permissive than what you have
or what Rob suggested. Your original one allows
"rx", "tx" OR "rx" OR "tx"
Rob's allows the same but with a nicer syntax. What that stm binding I
mentioned allows is
"rx", "tx" OR "tx", "rx" OR "rx" OR "tx"


Attachments:
(No filename) (4.04 kB)
signature.asc (235.00 B)
Download all attachments