linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/4] arm64: dts: imx8qxp add asrc and sai
@ 2024-03-05 17:33 Frank Li
  2024-03-05 17:33 ` [PATCH v4 1/4] ASoC: dt-bindings: fsl,imx-asrc/spdif: Add power-domains property Frank Li
                   ` (3 more replies)
  0 siblings, 4 replies; 23+ messages in thread
From: Frank Li @ 2024-03-05 17:33 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Shengjiu Wang
  Cc: linux-sound, devicetree, imx, linux-arm-kernel, linux-kernel, Frank Li

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

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
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/20240228-asrc_8qxp-v3-0-d4d5935fd3aa@nxp.com

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/20240227-asrc_8qxp-v2-0-521bcc7eb1c0@nxp.com

---
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 <Frank.Li@nxp.com>


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v4 1/4] ASoC: dt-bindings: fsl,imx-asrc/spdif: Add power-domains property
  2024-03-05 17:33 [PATCH v4 0/4] arm64: dts: imx8qxp add asrc and sai Frank Li
@ 2024-03-05 17:33 ` Frank Li
  2024-03-06 18:53   ` Conor Dooley
  2024-03-05 17:33 ` [PATCH v4 2/4] ASoC: dt-bindings: fsl,imx-asrc: update max interrupt numbers Frank Li
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 23+ messages in thread
From: Frank Li @ 2024-03-05 17:33 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Shengjiu Wang
  Cc: linux-sound, devicetree, imx, linux-arm-kernel, linux-kernel, Frank Li

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 <Frank.Li@nxp.com>
---
 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


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v4 2/4] ASoC: dt-bindings: fsl,imx-asrc: update max interrupt numbers
  2024-03-05 17:33 [PATCH v4 0/4] arm64: dts: imx8qxp add asrc and sai Frank Li
  2024-03-05 17:33 ` [PATCH v4 1/4] ASoC: dt-bindings: fsl,imx-asrc/spdif: Add power-domains property Frank Li
@ 2024-03-05 17:33 ` Frank Li
  2024-03-06 18:47   ` Conor Dooley
  2024-03-06 20:25   ` Rob Herring
  2024-03-05 17:33 ` [PATCH v4 3/4] ASoC: dt-bindings: fsl-sai: allow only one dma-names Frank Li
  2024-03-05 17:33 ` [PATCH v4 4/4] arm64: dts: imx8qxp: add asrc[0,1], esai0, spdif[0,1] and sai[4,5] Frank Li
  3 siblings, 2 replies; 23+ messages in thread
From: Frank Li @ 2024-03-05 17:33 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Shengjiu Wang
  Cc: linux-sound, devicetree, imx, linux-arm-kernel, linux-kernel, Frank Li

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 <Frank.Li@nxp.com>
---
 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


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v4 3/4] ASoC: dt-bindings: fsl-sai: allow only one dma-names
  2024-03-05 17:33 [PATCH v4 0/4] arm64: dts: imx8qxp add asrc and sai Frank Li
  2024-03-05 17:33 ` [PATCH v4 1/4] ASoC: dt-bindings: fsl,imx-asrc/spdif: Add power-domains property Frank Li
  2024-03-05 17:33 ` [PATCH v4 2/4] ASoC: dt-bindings: fsl,imx-asrc: update max interrupt numbers Frank Li
@ 2024-03-05 17:33 ` Frank Li
  2024-03-06 18:45   ` Conor Dooley
  2024-03-05 17:33 ` [PATCH v4 4/4] arm64: dts: imx8qxp: add asrc[0,1], esai0, spdif[0,1] and sai[4,5] Frank Li
  3 siblings, 1 reply; 23+ messages in thread
From: Frank Li @ 2024-03-05 17:33 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Shengjiu Wang
  Cc: linux-sound, devicetree, imx, linux-arm-kernel, linux-kernel, Frank Li

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 <Frank.Li@nxp.com>
---
 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


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v4 4/4] arm64: dts: imx8qxp: add asrc[0,1], esai0, spdif[0,1] and sai[4,5]
  2024-03-05 17:33 [PATCH v4 0/4] arm64: dts: imx8qxp add asrc and sai Frank Li
                   ` (2 preceding siblings ...)
  2024-03-05 17:33 ` [PATCH v4 3/4] ASoC: dt-bindings: fsl-sai: allow only one dma-names Frank Li
@ 2024-03-05 17:33 ` Frank Li
  2024-03-06  7:20   ` [PATCH v4 4/4] arm64: dts: imx8qxp: add asrc[0, 1], esai0, spdif[0, 1] and sai[4, 5] Alexander Stein
  3 siblings, 1 reply; 23+ messages in thread
From: Frank Li @ 2024-03-05 17:33 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Shengjiu Wang
  Cc: linux-sound, devicetree, imx, linux-arm-kernel, linux-kernel, 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 <Frank.Li@nxp.com>
---
 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


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH v4 4/4] arm64: dts: imx8qxp: add asrc[0, 1], esai0, spdif[0, 1] and sai[4, 5]
  2024-03-05 17:33 ` [PATCH v4 4/4] arm64: dts: imx8qxp: add asrc[0,1], esai0, spdif[0,1] and sai[4,5] Frank Li
@ 2024-03-06  7:20   ` Alexander Stein
  2024-03-06 15:19     ` Frank Li
  0 siblings, 1 reply; 23+ messages in thread
From: Alexander Stein @ 2024-03-06  7:20 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Shengjiu Wang, linux-arm-kernel, Frank Li
  Cc: linux-sound, devicetree, imx, linux-arm-kernel, linux-kernel, Frank Li

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



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v4 4/4] arm64: dts: imx8qxp: add asrc[0, 1], esai0, spdif[0, 1] and sai[4, 5]
  2024-03-06  7:20   ` [PATCH v4 4/4] arm64: dts: imx8qxp: add asrc[0, 1], esai0, spdif[0, 1] and sai[4, 5] Alexander Stein
@ 2024-03-06 15:19     ` Frank Li
  2024-03-07  7:22       ` Alexander Stein
  0 siblings, 1 reply; 23+ messages in thread
From: Frank Li @ 2024-03-06 15:19 UTC (permalink / raw)
  To: Alexander Stein
  Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Shengjiu Wang, linux-arm-kernel, linux-sound,
	devicetree, imx, linux-kernel

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v4 3/4] ASoC: dt-bindings: fsl-sai: allow only one dma-names
  2024-03-05 17:33 ` [PATCH v4 3/4] ASoC: dt-bindings: fsl-sai: allow only one dma-names Frank Li
@ 2024-03-06 18:45   ` Conor Dooley
  2024-03-06 19:25     ` Frank Li
  0 siblings, 1 reply; 23+ messages in thread
From: Conor Dooley @ 2024-03-06 18:45 UTC (permalink / raw)
  To: Frank Li
  Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Shengjiu Wang, linux-sound, devicetree, imx,
	linux-arm-kernel, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1657 bytes --]

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 <conor.dooley@microchip.com>

> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
>  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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v4 2/4] ASoC: dt-bindings: fsl,imx-asrc: update max interrupt numbers
  2024-03-05 17:33 ` [PATCH v4 2/4] ASoC: dt-bindings: fsl,imx-asrc: update max interrupt numbers Frank Li
@ 2024-03-06 18:47   ` Conor Dooley
  2024-03-06 20:25   ` Rob Herring
  1 sibling, 0 replies; 23+ messages in thread
From: Conor Dooley @ 2024-03-06 18:47 UTC (permalink / raw)
  To: Frank Li
  Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Shengjiu Wang, linux-sound, devicetree, imx,
	linux-arm-kernel, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 389 bytes --]

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 <Frank.Li@nxp.com>

Acked-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v4 1/4] ASoC: dt-bindings: fsl,imx-asrc/spdif: Add power-domains property
  2024-03-05 17:33 ` [PATCH v4 1/4] ASoC: dt-bindings: fsl,imx-asrc/spdif: Add power-domains property Frank Li
@ 2024-03-06 18:53   ` Conor Dooley
  0 siblings, 0 replies; 23+ messages in thread
From: Conor Dooley @ 2024-03-06 18:53 UTC (permalink / raw)
  To: Frank Li
  Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Shengjiu Wang, linux-sound, devicetree, imx,
	linux-arm-kernel, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 438 bytes --]

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 <Frank.Li@nxp.com>

Acked-by: Conor Dooley <conor.dooley@microchip.com>

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v4 3/4] ASoC: dt-bindings: fsl-sai: allow only one dma-names
  2024-03-06 18:45   ` Conor Dooley
@ 2024-03-06 19:25     ` Frank Li
  2024-03-06 19:45       ` Conor Dooley
  2024-03-06 20:55       ` Rob Herring
  0 siblings, 2 replies; 23+ messages in thread
From: Frank Li @ 2024-03-06 19:25 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Shengjiu Wang, linux-sound, devicetree, imx,
	linux-arm-kernel, linux-kernel

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 <conor.dooley@microchip.com>
> 
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> >  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.



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v4 3/4] ASoC: dt-bindings: fsl-sai: allow only one dma-names
  2024-03-06 19:25     ` Frank Li
@ 2024-03-06 19:45       ` Conor Dooley
  2024-03-06 20:25         ` Frank Li
  2024-03-06 20:55       ` Rob Herring
  1 sibling, 1 reply; 23+ messages in thread
From: Conor Dooley @ 2024-03-06 19:45 UTC (permalink / raw)
  To: Frank Li
  Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Shengjiu Wang, linux-sound, devicetree, imx,
	linux-arm-kernel, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 2308 bytes --]

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 <conor.dooley@microchip.com>
> > 
> > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > ---
> > >  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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v4 3/4] ASoC: dt-bindings: fsl-sai: allow only one dma-names
  2024-03-06 19:45       ` Conor Dooley
@ 2024-03-06 20:25         ` Frank Li
  0 siblings, 0 replies; 23+ messages in thread
From: Frank Li @ 2024-03-06 20:25 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Shengjiu Wang, linux-sound, devicetree, imx,
	linux-arm-kernel, linux-kernel

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 <conor.dooley@microchip.com>
> > > 
> > > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > > ---
> > > >  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


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v4 2/4] ASoC: dt-bindings: fsl,imx-asrc: update max interrupt numbers
  2024-03-05 17:33 ` [PATCH v4 2/4] ASoC: dt-bindings: fsl,imx-asrc: update max interrupt numbers Frank Li
  2024-03-06 18:47   ` Conor Dooley
@ 2024-03-06 20:25   ` Rob Herring
  2024-03-06 20:53     ` Frank Li
  1 sibling, 1 reply; 23+ messages in thread
From: Rob Herring @ 2024-03-06 20:25 UTC (permalink / raw)
  To: Frank Li
  Cc: Liam Girdwood, Mark Brown, Krzysztof Kozlowski, Conor Dooley,
	Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	Shengjiu Wang, linux-sound, devicetree, imx, linux-arm-kernel,
	linux-kernel

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 <Frank.Li@nxp.com>
> ---
>  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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v4 2/4] ASoC: dt-bindings: fsl,imx-asrc: update max interrupt numbers
  2024-03-06 20:25   ` Rob Herring
@ 2024-03-06 20:53     ` Frank Li
  2024-03-06 23:00       ` Rob Herring
  0 siblings, 1 reply; 23+ messages in thread
From: Frank Li @ 2024-03-06 20:53 UTC (permalink / raw)
  To: Rob Herring
  Cc: Liam Girdwood, Mark Brown, Krzysztof Kozlowski, Conor Dooley,
	Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	Shengjiu Wang, linux-sound, devicetree, imx, linux-arm-kernel,
	linux-kernel

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 <Frank.Li@nxp.com>
> > ---
> >  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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v4 3/4] ASoC: dt-bindings: fsl-sai: allow only one dma-names
  2024-03-06 19:25     ` Frank Li
  2024-03-06 19:45       ` Conor Dooley
@ 2024-03-06 20:55       ` Rob Herring
  2024-03-06 21:19         ` Frank Li
  1 sibling, 1 reply; 23+ messages in thread
From: Rob Herring @ 2024-03-06 20:55 UTC (permalink / raw)
  To: Frank Li
  Cc: Conor Dooley, Liam Girdwood, Mark Brown, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Shengjiu Wang, linux-sound, devicetree, imx,
	linux-arm-kernel, linux-kernel

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 <conor.dooley@microchip.com>
> > 
> > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > ---
> > >  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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v4 3/4] ASoC: dt-bindings: fsl-sai: allow only one dma-names
  2024-03-06 20:55       ` Rob Herring
@ 2024-03-06 21:19         ` Frank Li
  2024-03-06 22:58           ` Rob Herring
  0 siblings, 1 reply; 23+ messages in thread
From: Frank Li @ 2024-03-06 21:19 UTC (permalink / raw)
  To: Rob Herring
  Cc: Conor Dooley, Liam Girdwood, Mark Brown, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Shengjiu Wang, linux-sound, devicetree, imx,
	linux-arm-kernel, linux-kernel

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 <conor.dooley@microchip.com>
> > > 
> > > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > > ---
> > > >  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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v4 3/4] ASoC: dt-bindings: fsl-sai: allow only one dma-names
  2024-03-06 21:19         ` Frank Li
@ 2024-03-06 22:58           ` Rob Herring
  2024-03-07  2:02             ` Frank Li
  0 siblings, 1 reply; 23+ messages in thread
From: Rob Herring @ 2024-03-06 22:58 UTC (permalink / raw)
  To: Frank Li
  Cc: Conor Dooley, Liam Girdwood, Mark Brown, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Shengjiu Wang, linux-sound, devicetree, imx,
	linux-arm-kernel, linux-kernel

On Wed, Mar 6, 2024 at 3:19 PM Frank Li <Frank.li@nxp.com> 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 <conor.dooley@microchip.com>
> > > >
> > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > > > ---
> > > > >  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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v4 2/4] ASoC: dt-bindings: fsl,imx-asrc: update max interrupt numbers
  2024-03-06 20:53     ` Frank Li
@ 2024-03-06 23:00       ` Rob Herring
  0 siblings, 0 replies; 23+ messages in thread
From: Rob Herring @ 2024-03-06 23:00 UTC (permalink / raw)
  To: Frank Li
  Cc: Liam Girdwood, Mark Brown, Krzysztof Kozlowski, Conor Dooley,
	Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	Shengjiu Wang, linux-sound, devicetree, imx, linux-arm-kernel,
	linux-kernel

On Wed, Mar 6, 2024 at 2:53 PM Frank Li <Frank.li@nxp.com> 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 <Frank.Li@nxp.com>
> > > ---
> > >  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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v4 3/4] ASoC: dt-bindings: fsl-sai: allow only one dma-names
  2024-03-06 22:58           ` Rob Herring
@ 2024-03-07  2:02             ` Frank Li
  2024-03-07 18:56               ` Conor Dooley
  0 siblings, 1 reply; 23+ messages in thread
From: Frank Li @ 2024-03-07  2:02 UTC (permalink / raw)
  To: Rob Herring
  Cc: Conor Dooley, Liam Girdwood, Mark Brown, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Shengjiu Wang, linux-sound, devicetree, imx,
	linux-arm-kernel, linux-kernel

On Wed, Mar 06, 2024 at 04:58:22PM -0600, Rob Herring wrote:
> On Wed, Mar 6, 2024 at 3:19 PM Frank Li <Frank.li@nxp.com> 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 <conor.dooley@microchip.com>
> > > > >
> > > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > > > > ---
> > > > > >  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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v4 4/4] arm64: dts: imx8qxp: add asrc[0, 1], esai0, spdif[0, 1] and sai[4, 5]
  2024-03-06 15:19     ` Frank Li
@ 2024-03-07  7:22       ` Alexander Stein
  2024-03-07 16:26         ` Frank Li
  0 siblings, 1 reply; 23+ messages in thread
From: Alexander Stein @ 2024-03-07  7:22 UTC (permalink / raw)
  To: Frank Li
  Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Shengjiu Wang, linux-arm-kernel, linux-sound,
	devicetree, imx, linux-kernel

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



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v4 4/4] arm64: dts: imx8qxp: add asrc[0, 1], esai0, spdif[0, 1] and sai[4, 5]
  2024-03-07  7:22       ` Alexander Stein
@ 2024-03-07 16:26         ` Frank Li
  0 siblings, 0 replies; 23+ messages in thread
From: Frank Li @ 2024-03-07 16:26 UTC (permalink / raw)
  To: Alexander Stein
  Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Shengjiu Wang, linux-arm-kernel, linux-sound,
	devicetree, imx, linux-kernel

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v4 3/4] ASoC: dt-bindings: fsl-sai: allow only one dma-names
  2024-03-07  2:02             ` Frank Li
@ 2024-03-07 18:56               ` Conor Dooley
  0 siblings, 0 replies; 23+ messages in thread
From: Conor Dooley @ 2024-03-07 18:56 UTC (permalink / raw)
  To: Frank Li
  Cc: Rob Herring, Liam Girdwood, Mark Brown, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Shengjiu Wang, linux-sound, devicetree, imx,
	linux-arm-kernel, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 4139 bytes --]

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 <Frank.li@nxp.com> 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 <conor.dooley@microchip.com>
> > > > > >
> > > > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > > > > > ---
> > > > > > >  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"

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2024-03-07 18:56 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-05 17:33 [PATCH v4 0/4] arm64: dts: imx8qxp add asrc and sai Frank Li
2024-03-05 17:33 ` [PATCH v4 1/4] ASoC: dt-bindings: fsl,imx-asrc/spdif: Add power-domains property Frank Li
2024-03-06 18:53   ` Conor Dooley
2024-03-05 17:33 ` [PATCH v4 2/4] ASoC: dt-bindings: fsl,imx-asrc: update max interrupt numbers Frank Li
2024-03-06 18:47   ` Conor Dooley
2024-03-06 20:25   ` Rob Herring
2024-03-06 20:53     ` Frank Li
2024-03-06 23:00       ` Rob Herring
2024-03-05 17:33 ` [PATCH v4 3/4] ASoC: dt-bindings: fsl-sai: allow only one dma-names Frank Li
2024-03-06 18:45   ` Conor Dooley
2024-03-06 19:25     ` Frank Li
2024-03-06 19:45       ` Conor Dooley
2024-03-06 20:25         ` Frank Li
2024-03-06 20:55       ` Rob Herring
2024-03-06 21:19         ` Frank Li
2024-03-06 22:58           ` Rob Herring
2024-03-07  2:02             ` Frank Li
2024-03-07 18:56               ` Conor Dooley
2024-03-05 17:33 ` [PATCH v4 4/4] arm64: dts: imx8qxp: add asrc[0,1], esai0, spdif[0,1] and sai[4,5] Frank Li
2024-03-06  7:20   ` [PATCH v4 4/4] arm64: dts: imx8qxp: add asrc[0, 1], esai0, spdif[0, 1] and sai[4, 5] Alexander Stein
2024-03-06 15:19     ` Frank Li
2024-03-07  7:22       ` Alexander Stein
2024-03-07 16:26         ` Frank Li

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).