Devicetree
 help / color / mirror / Atom feed
* [PATCH 0/3] SpacemiT K3: fix I2S DT integration and add controller nodes
@ 2026-07-22  2:35 Troy Mitchell
  2026-07-22  2:35 ` [PATCH 1/3] ASoC: spacemit: rename clock inputs to match binding Troy Mitchell
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Troy Mitchell @ 2026-07-22  2:35 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai,
	Yixun Lan, Jinmei Wei, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Alexandre Ghiti
  Cc: linux-sound, linux-riscv, spacemit, linux-kernel, devicetree,
	Troy Mitchell

The SpacemiT K3 SoC has six SSPA-based I2S controllers (i2s0-i2s5).
This series fixes two bugs that prevented writing a correct DT for any
of them and adds all six nodes.

Patch 1 fixes the ASoC driver's clock lookup names. The driver was
requesting "sspa_bus" and "sspa" but the published binding specifies
"bus" and "func", causing probe failures on any conformant DT node.

Patch 2 allows K3 i2s1 to use its 6-clock layout while preserving the
published 7-clock order used by the other K3 I2S controllers. It
describes the K1, K3 i2s1, and other K3 layouts as separate tuples.

Patch 3 adds the i2s0-i2s5 DTS nodes. i2s0 and i2s2-i2s5 use all 7
clocks including sysclk_div; i2s1 uses the 6-clock variant.

This series depends on the K3 I2S clock fixes posted at:

https://lore.kernel.org/all/20260717-k3-clk-fix-i2s-v1-0-e95001a692ee@linux.spacemit.com/

The prerequisite series contains:
  - dt-bindings: soc: spacemit: k3: add i2s_sysclk,
    i2s_bclk_factor and i2s1_sysclk_src IDs
  - clk: spacemit: k3: fix i2s clock topology
  - clk: spacemit: k3: fix missing /2 factor in i2s sysclk dividers

Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
---
Troy Mitchell (3):
      ASoC: spacemit: rename clock inputs to match binding
      dt-bindings: sound: spacemit,k1-i2s: allow 6 clocks for K3 i2s1
      riscv: dts: spacemit: k3: add i2s0-i2s5 nodes

 .../devicetree/bindings/sound/spacemit,k1-i2s.yaml |  64 +++++++----
 arch/riscv/boot/dts/spacemit/k3.dtsi               | 119 +++++++++++++++++++++
 sound/soc/spacemit/k1_i2s.c                        |  16 +--
 3 files changed, 171 insertions(+), 28 deletions(-)
---
base-commit: c425609d6ac4012c8bbf01ec2e10e801b1923a7b
change-id: 20260615-kx-i2s-dts-123764c9b2b2
prerequisite-message-id: <20260717-k3-clk-fix-i2s-v1-0-e95001a692ee@linux.spacemit.com>
prerequisite-patch-id: f8cbf4a26caa4b48d44328ec55288f19a5ea67fb
prerequisite-patch-id: 710ec05bdce3d01dcb7bb1fc2a5750666d02c20d
prerequisite-patch-id: 197bf1e162e5346f0282d4fd945ba631c68e0fad

Best regards,
--  
Troy Mitchell <troy.mitchell@linux.spacemit.com>


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

* [PATCH 1/3] ASoC: spacemit: rename clock inputs to match binding
  2026-07-22  2:35 [PATCH 0/3] SpacemiT K3: fix I2S DT integration and add controller nodes Troy Mitchell
@ 2026-07-22  2:35 ` Troy Mitchell
  2026-07-22  2:49   ` sashiko-bot
  2026-07-22  2:35 ` [PATCH 2/3] dt-bindings: sound: spacemit,k1-i2s: allow 6 clocks for K3 i2s1 Troy Mitchell
  2026-07-22  2:35 ` [PATCH 3/3] riscv: dts: spacemit: k3: add i2s0-i2s5 nodes Troy Mitchell
  2 siblings, 1 reply; 6+ messages in thread
From: Troy Mitchell @ 2026-07-22  2:35 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai,
	Yixun Lan, Jinmei Wei, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Alexandre Ghiti
  Cc: linux-sound, linux-riscv, spacemit, linux-kernel, devicetree,
	Troy Mitchell

The driver requests the per-controller SSPA bus and functional clocks
as "sspa_bus" and "sspa", but the device tree binding (spacemit,k1-i2s)
specifies them as "bus" and "func". As a result, any DT written against
the published binding fails to probe.

There are currently no in-tree DT users referencing these names, so
rename the clock inputs in the driver to match the binding rather than
changing the binding. While at it, rename the matching struct member
sspa_clk to func_clk for consistency with the new clock-names.

Fixes: fce217449075 ("ASoC: spacemit: add i2s support for K1 SoC")
Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
---
 sound/soc/spacemit/k1_i2s.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/sound/soc/spacemit/k1_i2s.c b/sound/soc/spacemit/k1_i2s.c
index 8871fc15b29c..4247d8d3a637 100644
--- a/sound/soc/spacemit/k1_i2s.c
+++ b/sound/soc/spacemit/k1_i2s.c
@@ -52,7 +52,7 @@ struct spacemit_i2s_dev {
 
 	struct clk *sysclk;
 	struct clk *bclk;
-	struct clk *sspa_clk;
+	struct clk *func_clk;
 	struct clk *sysclk_div;
 	struct clk *c_sysclk;
 	struct clk *c_bclk;
@@ -221,7 +221,7 @@ static int spacemit_i2s_hw_params(struct snd_pcm_substream *substream,
 	if (ret)
 		return ret;
 
-	return clk_set_rate(i2s->sspa_clk, bclk_rate);
+	return clk_set_rate(i2s->func_clk, bclk_rate);
 }
 
 static int spacemit_i2s_set_sysclk(struct snd_soc_dai *cpu_dai, int clk_id,
@@ -445,14 +445,14 @@ static int spacemit_i2s_probe(struct platform_device *pdev)
 	if (IS_ERR(i2s->bclk))
 		return dev_err_probe(i2s->dev, PTR_ERR(i2s->bclk), "failed to enable bit clock\n");
 
-	clk = devm_clk_get_enabled(i2s->dev, "sspa_bus");
+	clk = devm_clk_get_enabled(i2s->dev, "bus");
 	if (IS_ERR(clk))
-		return dev_err_probe(i2s->dev, PTR_ERR(clk), "failed to enable sspa_bus clock\n");
+		return dev_err_probe(i2s->dev, PTR_ERR(clk), "failed to enable bus clock\n");
 
-	i2s->sspa_clk = devm_clk_get_enabled(i2s->dev, "sspa");
-	if (IS_ERR(i2s->sspa_clk))
-		return dev_err_probe(i2s->dev, PTR_ERR(i2s->sspa_clk),
-				     "failed to enable sspa clock\n");
+	i2s->func_clk = devm_clk_get_enabled(i2s->dev, "func");
+	if (IS_ERR(i2s->func_clk))
+		return dev_err_probe(i2s->dev, PTR_ERR(i2s->func_clk),
+				     "failed to enable func clock\n");
 
 	i2s->sysclk_div = devm_clk_get_optional_enabled(i2s->dev, "sysclk_div");
 	if (IS_ERR(i2s->sysclk_div))

-- 
2.55.0


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

* [PATCH 2/3] dt-bindings: sound: spacemit,k1-i2s: allow 6 clocks for K3 i2s1
  2026-07-22  2:35 [PATCH 0/3] SpacemiT K3: fix I2S DT integration and add controller nodes Troy Mitchell
  2026-07-22  2:35 ` [PATCH 1/3] ASoC: spacemit: rename clock inputs to match binding Troy Mitchell
@ 2026-07-22  2:35 ` Troy Mitchell
  2026-07-22  2:35 ` [PATCH 3/3] riscv: dts: spacemit: k3: add i2s0-i2s5 nodes Troy Mitchell
  2 siblings, 0 replies; 6+ messages in thread
From: Troy Mitchell @ 2026-07-22  2:35 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai,
	Yixun Lan, Jinmei Wei, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Alexandre Ghiti
  Cc: linux-sound, linux-riscv, spacemit, linux-kernel, devicetree,
	Troy Mitchell

The K3 I2S controllers normally use the published 7-clock layout:

  sysclk, bclk, bus, func, sysclk_div, c_sysclk, c_bclk

However, K3 i2s1 has no dedicated sysclk divider and therefore uses a
6-clock layout:

  sysclk, bclk, bus, func, c_sysclk, c_bclk

Describe the 4-clock K1 and both K3 layouts as separate tuples. Lower
the K3 minimum from 7 to 6 clocks while preserving the existing
7-clock ordering.

Fixes: 6bc6b28c0314 ("ASoC: dt-bindings: add SpacemiT K3 SoC compatible")
Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
---
 .../devicetree/bindings/sound/spacemit,k1-i2s.yaml | 64 +++++++++++++++-------
 1 file changed, 44 insertions(+), 20 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/spacemit,k1-i2s.yaml b/Documentation/devicetree/bindings/sound/spacemit,k1-i2s.yaml
index 240d90402e4f..72723f067b2a 100644
--- a/Documentation/devicetree/bindings/sound/spacemit,k1-i2s.yaml
+++ b/Documentation/devicetree/bindings/sound/spacemit,k1-i2s.yaml
@@ -23,9 +23,9 @@ allOf:
     then:
       properties:
         clocks:
-          minItems: 7
+          minItems: 6
         clock-names:
-          minItems: 7
+          minItems: 6
     else:
       properties:
         clocks:
@@ -43,26 +43,50 @@ properties:
     maxItems: 1
 
   clocks:
-    minItems: 4
-    items:
-      - description: clock for I2S sysclk
-      - description: clock for I2S bclk
-      - description: clock for I2S bus
-      - description: clock for I2S controller
-      - description: clock for I2S sysclk divider
-      - description: clock for I2S common sysclk
-      - description: clock for I2S common bclk
+    oneOf:
+      - items:
+          - description: clock for I2S sysclk
+          - description: clock for I2S bclk
+          - description: clock for I2S bus
+          - description: clock for I2S controller
+      - items:
+          - description: clock for I2S sysclk
+          - description: clock for I2S bclk
+          - description: clock for I2S bus
+          - description: clock for I2S controller
+          - description: clock for I2S common sysclk
+          - description: clock for I2S common bclk
+      - items:
+          - description: clock for I2S sysclk
+          - description: clock for I2S bclk
+          - description: clock for I2S bus
+          - description: clock for I2S controller
+          - description: clock for I2S sysclk divider
+          - description: clock for I2S common sysclk
+          - description: clock for I2S common bclk
 
   clock-names:
-    minItems: 4
-    items:
-      - const: sysclk
-      - const: bclk
-      - const: bus
-      - const: func
-      - const: sysclk_div
-      - const: c_sysclk
-      - const: c_bclk
+    oneOf:
+      - items:
+          - const: sysclk
+          - const: bclk
+          - const: bus
+          - const: func
+      - items:
+          - const: sysclk
+          - const: bclk
+          - const: bus
+          - const: func
+          - const: c_sysclk
+          - const: c_bclk
+      - items:
+          - const: sysclk
+          - const: bclk
+          - const: bus
+          - const: func
+          - const: sysclk_div
+          - const: c_sysclk
+          - const: c_bclk
 
   dmas:
     minItems: 1

-- 
2.55.0


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

* [PATCH 3/3] riscv: dts: spacemit: k3: add i2s0-i2s5 nodes
  2026-07-22  2:35 [PATCH 0/3] SpacemiT K3: fix I2S DT integration and add controller nodes Troy Mitchell
  2026-07-22  2:35 ` [PATCH 1/3] ASoC: spacemit: rename clock inputs to match binding Troy Mitchell
  2026-07-22  2:35 ` [PATCH 2/3] dt-bindings: sound: spacemit,k1-i2s: allow 6 clocks for K3 i2s1 Troy Mitchell
@ 2026-07-22  2:35 ` Troy Mitchell
  2026-07-22  2:47   ` sashiko-bot
  2 siblings, 1 reply; 6+ messages in thread
From: Troy Mitchell @ 2026-07-22  2:35 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai,
	Yixun Lan, Jinmei Wei, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Alexandre Ghiti
  Cc: linux-sound, linux-riscv, spacemit, linux-kernel, devicetree,
	Troy Mitchell

Add the six SSPA-based I2S controller nodes for the K3 SoC.

i2s0 and i2s2-i2s5 each have a dedicated per-controller sysclk
divider, so they use the published 7-clock layout (sysclk, bclk, bus,
func, sysclk_div, c_sysclk, c_bclk).

i2s1 uses 6 clocks (sysclk, bclk, bus, func, c_sysclk, c_bclk) because
its sysclk is driven directly by a DDN (ISCCR0) and has no separate
divider stage.

Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
---
 arch/riscv/boot/dts/spacemit/k3.dtsi | 119 +++++++++++++++++++++++++++++++++++
 1 file changed, 119 insertions(+)

diff --git a/arch/riscv/boot/dts/spacemit/k3.dtsi b/arch/riscv/boot/dts/spacemit/k3.dtsi
index 19fc9b49668e..967e5d87527a 100644
--- a/arch/riscv/boot/dts/spacemit/k3.dtsi
+++ b/arch/riscv/boot/dts/spacemit/k3.dtsi
@@ -1081,6 +1081,125 @@ uart10: serial@d401f000 {
 			status = "disabled";
 		};
 
+		i2s0: i2s@d4026000 {
+			compatible = "spacemit,k3-i2s";
+			reg = <0x0 0xd4026000 0x0 0x30>;
+			clocks = <&syscon_mpmu CLK_MPMU_I2S0_SYSCLK>,
+				 <&syscon_apbc CLK_APBC_I2S0_BCLK>,
+				 <&syscon_apbc CLK_APBC_I2S0_BUS>,
+				 <&syscon_apbc CLK_APBC_I2S0>,
+				 <&syscon_mpmu CLK_MPMU_I2S0_SYSCLK_DIV>,
+				 <&syscon_mpmu CLK_MPMU_I2S_SYSCLK>,
+				 <&syscon_mpmu CLK_MPMU_I2S_BCLK>;
+			clock-names = "sysclk", "bclk", "bus", "func",
+				      "sysclk_div", "c_sysclk", "c_bclk";
+			dmas = <&pdma 21>, <&pdma 22>;
+			dma-names = "tx", "rx";
+			resets = <&syscon_apbc RESET_APBC_I2S0>;
+			#sound-dai-cells = <0>;
+
+			status = "disabled";
+		};
+
+		i2s1: i2s@d4026800 {
+			compatible = "spacemit,k3-i2s";
+			reg = <0x0 0xd4026800 0x0 0x30>;
+			clocks = <&syscon_mpmu CLK_MPMU_I2S1_SYSCLK>,
+				 <&syscon_apbc CLK_APBC_I2S1_BCLK>,
+				 <&syscon_apbc CLK_APBC_I2S1_BUS>,
+				 <&syscon_apbc CLK_APBC_I2S1>,
+				 <&syscon_mpmu CLK_MPMU_I2S_SYSCLK>,
+				 <&syscon_mpmu CLK_MPMU_I2S_BCLK>;
+			clock-names = "sysclk", "bclk", "bus", "func",
+				      "c_sysclk", "c_bclk";
+			dmas = <&pdma 23>, <&pdma 24>;
+			dma-names = "tx", "rx";
+			resets = <&syscon_apbc RESET_APBC_I2S1>;
+			#sound-dai-cells = <0>;
+
+			status = "disabled";
+		};
+
+		i2s2: i2s@d4027000 {
+			compatible = "spacemit,k3-i2s";
+			reg = <0x0 0xd4027000 0x0 0x30>;
+			clocks = <&syscon_mpmu CLK_MPMU_I2S2_SYSCLK>,
+				 <&syscon_apbc CLK_APBC_I2S2_BCLK>,
+				 <&syscon_apbc CLK_APBC_I2S2_BUS>,
+				 <&syscon_apbc CLK_APBC_I2S2>,
+				 <&syscon_mpmu CLK_MPMU_I2S2_SYSCLK_DIV>,
+				 <&syscon_mpmu CLK_MPMU_I2S_SYSCLK>,
+				 <&syscon_mpmu CLK_MPMU_I2S_BCLK>;
+			clock-names = "sysclk", "bclk", "bus", "func",
+				      "sysclk_div", "c_sysclk", "c_bclk";
+			dmas = <&pdma 56>, <&pdma 57>;
+			dma-names = "tx", "rx";
+			resets = <&syscon_apbc RESET_APBC_I2S2>;
+			#sound-dai-cells = <0>;
+
+			status = "disabled";
+		};
+
+		i2s3: i2s@d4027800 {
+			compatible = "spacemit,k3-i2s";
+			reg = <0x0 0xd4027800 0x0 0x30>;
+			clocks = <&syscon_mpmu CLK_MPMU_I2S3_SYSCLK>,
+				 <&syscon_apbc CLK_APBC_I2S3_BCLK>,
+				 <&syscon_apbc CLK_APBC_I2S3_BUS>,
+				 <&syscon_apbc CLK_APBC_I2S3>,
+				 <&syscon_mpmu CLK_MPMU_I2S3_SYSCLK_DIV>,
+				 <&syscon_mpmu CLK_MPMU_I2S_SYSCLK>,
+				 <&syscon_mpmu CLK_MPMU_I2S_BCLK>;
+			clock-names = "sysclk", "bclk", "bus", "func",
+				      "sysclk_div", "c_sysclk", "c_bclk";
+			dmas = <&pdma 58>, <&pdma 59>;
+			dma-names = "tx", "rx";
+			resets = <&syscon_apbc RESET_APBC_I2S3>;
+			#sound-dai-cells = <0>;
+
+			status = "disabled";
+		};
+
+		i2s4: i2s@d4041000 {
+			compatible = "spacemit,k3-i2s";
+			reg = <0x0 0xd4041000 0x0 0x30>;
+			clocks = <&syscon_mpmu CLK_MPMU_I2S4_SYSCLK>,
+				 <&syscon_apbc CLK_APBC_I2S4_BCLK>,
+				 <&syscon_apbc CLK_APBC_I2S4_BUS>,
+				 <&syscon_apbc CLK_APBC_I2S4>,
+				 <&syscon_mpmu CLK_MPMU_I2S4_SYSCLK_DIV>,
+				 <&syscon_mpmu CLK_MPMU_I2S_SYSCLK>,
+				 <&syscon_mpmu CLK_MPMU_I2S_BCLK>;
+			clock-names = "sysclk", "bclk", "bus", "func",
+				      "sysclk_div", "c_sysclk", "c_bclk";
+			dmas = <&pdma 60>, <&pdma 61>;
+			dma-names = "tx", "rx";
+			resets = <&syscon_apbc RESET_APBC_I2S4>;
+			#sound-dai-cells = <0>;
+
+			status = "disabled";
+		};
+
+		i2s5: i2s@d4041800 {
+			compatible = "spacemit,k3-i2s";
+			reg = <0x0 0xd4041800 0x0 0x30>;
+			clocks = <&syscon_mpmu CLK_MPMU_I2S5_SYSCLK>,
+				 <&syscon_apbc CLK_APBC_I2S5_BCLK>,
+				 <&syscon_apbc CLK_APBC_I2S5_BUS>,
+				 <&syscon_apbc CLK_APBC_I2S5>,
+				 <&syscon_mpmu CLK_MPMU_I2S5_SYSCLK_DIV>,
+				 <&syscon_mpmu CLK_MPMU_I2S_SYSCLK>,
+				 <&syscon_mpmu CLK_MPMU_I2S_BCLK>;
+			clock-names = "sysclk", "bclk", "bus", "func",
+				      "sysclk_div", "c_sysclk", "c_bclk";
+			dmas = <&pdma 62>, <&pdma 63>;
+			dma-names = "tx", "rx";
+			resets = <&syscon_apbc RESET_APBC_I2S5>;
+			#sound-dai-cells = <0>;
+
+			status = "disabled";
+		};
+
 		syscon_mpmu: system-controller@d4050000 {
 			compatible = "spacemit,k3-syscon-mpmu";
 			reg = <0x0 0xd4050000 0x0 0x10000>;

-- 
2.55.0


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

* Re: [PATCH 3/3] riscv: dts: spacemit: k3: add i2s0-i2s5 nodes
  2026-07-22  2:35 ` [PATCH 3/3] riscv: dts: spacemit: k3: add i2s0-i2s5 nodes Troy Mitchell
@ 2026-07-22  2:47   ` sashiko-bot
  0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-07-22  2:47 UTC (permalink / raw)
  To: Troy Mitchell; +Cc: devicetree, conor+dt, robh

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The device tree uses the undeclared macro `CLK_MPMU_I2S_SYSCLK`, which will cause a fatal syntax error during DT compilation.
--

--- Patch [3]: [PATCH 3/3] riscv: dts: spacemit: k3: add i2s0-i2s5 nodes ---
commit 89d824064dd5e05a6a01752a3478d73808fc8c1f
Author: Troy Mitchell <troy.mitchell@linux.spacemit.com>

riscv: dts: spacemit: k3: add i2s0-i2s5 nodes

This commit adds the six SSPA-based I2S controller nodes for the K3 SoC,
configuring their clocks, DMAs, and resets.

> diff --git a/arch/riscv/boot/dts/spacemit/k3.dtsi b/arch/riscv/boot/dts/spacemit/k3.dtsi
> index 19fc9b49668ed..967e5d87527a1 100644
> --- a/arch/riscv/boot/dts/spacemit/k3.dtsi
> +++ b/arch/riscv/boot/dts/spacemit/k3.dtsi
> @@ -1081,6 +1081,125 @@ uart10: serial@d401f000 {
>  			status = "disabled";
>  		};
>  
> +		i2s0: i2s@d4026000 {
> +			compatible = "spacemit,k3-i2s";
> +			reg = <0x0 0xd4026000 0x0 0x30>;
> +			clocks = <&syscon_mpmu CLK_MPMU_I2S0_SYSCLK>,
> +				 <&syscon_apbc CLK_APBC_I2S0_BCLK>,
> +				 <&syscon_apbc CLK_APBC_I2S0_BUS>,
> +				 <&syscon_apbc CLK_APBC_I2S0>,
> +				 <&syscon_mpmu CLK_MPMU_I2S0_SYSCLK_DIV>,
> +				 <&syscon_mpmu CLK_MPMU_I2S_SYSCLK>,

[Severity: Low]
Is CLK_MPMU_I2S_SYSCLK defined anywhere?

It appears this macro is undeclared, which will cause a fatal syntax error
during device tree compilation.

This same undeclared macro is also used in the i2s1 through i2s5 nodes added
by this patch.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721-kx-i2s-dts-v1-0-d22cb6cfaab5@linux.spacemit.com?part=3

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

* Re: [PATCH 1/3] ASoC: spacemit: rename clock inputs to match binding
  2026-07-22  2:35 ` [PATCH 1/3] ASoC: spacemit: rename clock inputs to match binding Troy Mitchell
@ 2026-07-22  2:49   ` sashiko-bot
  0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-07-22  2:49 UTC (permalink / raw)
  To: Troy Mitchell; +Cc: devicetree, robh, conor+dt

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing spinlock in trigger and hw_params callbacks for shared registers and state variables.
- [High] Unchecked stream capabilities lead to sound card probe failure if a DMA channel is missing from the Device Tree.
--

--- Patch [1]: [PATCH 1/3] ASoC: spacemit: rename clock inputs to match binding ---
commit 3f9179a8e0f739a34984ca4b3cbbfefcad4578c2
Author: Troy Mitchell <troy.mitchell@linux.spacemit.com>
Date:   Wed Jul 22 02:35:55 2026 +0000

    ASoC: spacemit: rename clock inputs to match binding

    The driver requests the per-controller SSPA bus and functional clocks
    as "sspa_bus" and "sspa", but the device tree binding specifies them as
    "bus" and "func". This patch renames the clock inputs in the driver to
    match the binding, and updates the internal struct member for consistency.

> diff --git a/sound/soc/spacemit/k1_i2s.c b/sound/soc/spacemit/k1_i2s.c
> index 8871fc15b29cc..4247d8d3a6370 100644
> --- a/sound/soc/spacemit/k1_i2s.c
> +++ b/sound/soc/spacemit/k1_i2s.c

[ ... ]

> @@ -221,7 +221,7 @@ static int spacemit_i2s_hw_params(struct snd_pcm_substream *substream,
>  	if (ret)
>  		return ret;
>  

[Severity: High]
This is a pre-existing issue, but missing a spinlock in the hw_params and
trigger callbacks might lead to hardware state corruption.

The spacemit_i2s_hw_params() and spacemit_i2s_trigger() callbacks can be
invoked concurrently on different CPUs for playback and capture streams on a
full-duplex setup.

Both functions perform unprotected read-modify-write operations on the shared
SSCR register. For example, in spacemit_i2s_hw_params():

spacemit_i2s_hw_params() {
    ...
    val = readl(i2s->base + SSCR);
    if (val & SSCR_SSE)
        return 0;
    val &= ~SSCR_DW_32BYTE;
    val |= data_width;
    writel(val, i2s->base + SSCR);
    ...
}

Could concurrent execution cause the hw_params callback to overwrite the
SSCR_SSE enable bit set by the trigger callback, or corrupt the
i2s->started_count reference? Should a driver-level spinlock be added to
protect these shared registers and state variables?

> -	return clk_set_rate(i2s->sspa_clk, bclk_rate);
> +	return clk_set_rate(i2s->func_clk, bclk_rate);
>  }

[ ... ]

> @@ -445,14 +445,14 @@ static int spacemit_i2s_probe(struct platform_device *pdev)
>  	if (IS_ERR(i2s->bclk))
>  		return dev_err_probe(i2s->dev, PTR_ERR(i2s->bclk), "failed to enable bit clock\n");
>  
> -	clk = devm_clk_get_enabled(i2s->dev, "sspa_bus");
> +	clk = devm_clk_get_enabled(i2s->dev, "bus");
>  	if (IS_ERR(clk))
> -		return dev_err_probe(i2s->dev, PTR_ERR(clk), "failed to enable sspa_bus clock\n");
> +		return dev_err_probe(i2s->dev, PTR_ERR(clk), "failed to enable bus clock\n");
>  
> -	i2s->sspa_clk = devm_clk_get_enabled(i2s->dev, "sspa");
> -	if (IS_ERR(i2s->sspa_clk))
> -		return dev_err_probe(i2s->dev, PTR_ERR(i2s->sspa_clk),
> -				     "failed to enable sspa clock\n");
> +	i2s->func_clk = devm_clk_get_enabled(i2s->dev, "func");
> +	if (IS_ERR(i2s->func_clk))
> +		return dev_err_probe(i2s->dev, PTR_ERR(i2s->func_clk),
> +				     "failed to enable func clock\n");

[Severity: High]
This is a pre-existing issue, but unchecked stream capabilities might cause
the sound card probe to fail if a DMA channel is missing from the Device Tree.

When spacemit_i2s_probe() calls spacemit_i2s_init_dai(), the driver
allocates the dai driver struct by duplicating spacemit_i2s_dai, which
statically advertises both playback and capture capabilities with
channels_min = 1.

If the Device Tree lacks a DMA channel (for example, the rx channel), the
driver correctly sets has_capture to false but fails to clear
dai->capture.channels_min.

Would this cause the ALSA core to instantiate an unsupported capture
substream, leading dmaengine_pcm_new() to fail and abort the sound card
probe when it requests the missing DMA channel? Should the capabilities for
unsupported streams be explicitly cleared when has_playback or has_capture
is false?

>  	i2s->sysclk_div = devm_clk_get_optional_enabled(i2s->dev, "sysclk_div");

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721-kx-i2s-dts-v1-0-d22cb6cfaab5@linux.spacemit.com?part=1

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

end of thread, other threads:[~2026-07-22  2:49 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-22  2:35 [PATCH 0/3] SpacemiT K3: fix I2S DT integration and add controller nodes Troy Mitchell
2026-07-22  2:35 ` [PATCH 1/3] ASoC: spacemit: rename clock inputs to match binding Troy Mitchell
2026-07-22  2:49   ` sashiko-bot
2026-07-22  2:35 ` [PATCH 2/3] dt-bindings: sound: spacemit,k1-i2s: allow 6 clocks for K3 i2s1 Troy Mitchell
2026-07-22  2:35 ` [PATCH 3/3] riscv: dts: spacemit: k3: add i2s0-i2s5 nodes Troy Mitchell
2026-07-22  2:47   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox