[PATCH v3 00/10] ASoC: add Renesas MSIOF sound driver

[PATCH v3 00/10] ASoC: add Renesas MSIOF sound driver

[Kuninori Morimoto]

Hi Mark, Rob, Krzysztof
Cc Geert

Renesas MSIOF can work as both SPI and I2S.
Current Linux supports MSIOF-SPI. This patch-set adds new MSIOF-I2S.

Because it is using same HW-IP, we want to share same compatible for both
MSIOF-SPI/I2S case. MSIOF-I2S (Sound) will use Audio-Graph-Card/Card2 which
uses Of-Graph, but  MSIOF-SPI is not use Of-Graph.
So, this patch-set assumes it was used as MSIOF-I2S if DT is using Of-Graph,
otherwise, it is MSIOF-SPI (This assumption will works if SPI *never*
use Of-Graph in the future).

Link: https://lore.kernel.org/r/87h62vh5mj.wl-kuninori.morimoto.gx@renesas.com
Link: https://lore.kernel.org/r/875xjeb0wu.wl-kuninori.morimoto.gx@renesas.com

v2 -> v3
	- Tidyup MSIOF dt-bindings schema ([1/10])
	- Add SPI tidyup patch ([02/10])
	- Tidyup git-log comment ([04/10)
	- Tidyup ADG handling ([05/10])
	- Tidyup typo ([07/10])
	- Add Reviewed-by ([10/10])

v1 -> v2
	- Merge I2S DT bindings into SPI DT
	- add "dt-bindings: " in Subject ([1/9])
	- use dev instead of pdev->dev ([2/9])
	- use __free() ([3/9])
	- Add Geert's Reviewed-by ([3/9])
	- Tidyup top NOTE ([6/9])
	- Remove unused defines ([6/9])
	- remove "probed" ([6/9])
	- remove MODULE_ALIAS([6/9])
	- rename "spi" to "serial-engine" ([7/9])
	- add MSIOF on defconfig ([9/9])


Kuninori Morimoto (10):
  dt-bindings: renesas,sh-msiof: Add MSIOF I2S Sound support
  spi: sh-msiof: use dev in sh_msiof_spi_probe()
  spi: sh-msiof: ignore driver probing if it was MSIOF Sound
  ASoC: renesas: rsnd: allow to use ADG as standalone
  ASoC: renesas: rsnd: care BRGA/BRGB select in rsnd_adg_clk_enable()
  ASoC: renesas: rsnd: enable to use "adg" clock
  ASoC: renesas: add MSIOF sound support
  arm64: dts: renesas: r8a779g0: tidyup MSIOF node name
  arm64: dts: renesas: sparrow hawk: Add MSIOF Sound support
  arm64: defconfig: add Renesas MSIOF sound support

 .../bindings/spi/renesas,sh-msiof.yaml        |  43 +-
 arch/arm64/boot/dts/renesas/r8a779g0.dtsi     |  12 +-
 .../dts/renesas/r8a779g3-sparrow-hawk.dts     |  98 +++
 arch/arm64/configs/defconfig                  |   1 +
 drivers/spi/spi-sh-msiof.c                    |  42 +-
 sound/soc/renesas/Kconfig                     |   7 +
 sound/soc/renesas/rcar/Makefile               |   3 +
 sound/soc/renesas/rcar/adg.c                  |  32 +-
 sound/soc/renesas/rcar/core.c                 |   7 +-
 sound/soc/renesas/rcar/msiof.c                | 566 ++++++++++++++++++
 10 files changed, 766 insertions(+), 45 deletions(-)
 create mode 100644 sound/soc/renesas/rcar/msiof.c

-- 
2.43.0

[PATCH v3 01/10] dt-bindings: renesas,sh-msiof: Add MSIOF I2S Sound support

[Kuninori Morimoto]

Renesas MSIOF (Clock-Synchronized Serial Interface with FIFO) can work as
both SPI and I2S. MSIOF-I2S will use Audio Graph Card/Card2 driver which
uses Of-Graph in DT.

MSIOF-SPI/I2S are using same DT compatible properties.
MSIOF-I2S         uses Of-Graph for Audio-Graph-Card/Card2,
MSIOF-SPI doesn't use  Of-Graph.

Adds schema for MSIOF-I2S (= Sound).

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 .../bindings/spi/renesas,sh-msiof.yaml        | 43 ++++++++++++-------
 1 file changed, 27 insertions(+), 16 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/renesas,sh-msiof.yaml b/Documentation/devicetree/bindings/spi/renesas,sh-msiof.yaml
index 49649fc3f95a..880bd854d196 100644
--- a/Documentation/devicetree/bindings/spi/renesas,sh-msiof.yaml
+++ b/Documentation/devicetree/bindings/spi/renesas,sh-msiof.yaml
@@ -4,14 +4,11 @@
 $id: http://devicetree.org/schemas/spi/renesas,sh-msiof.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Renesas MSIOF SPI controller
+title: Renesas MSIOF SPI / I2S controller
 
 maintainers:
   - Geert Uytterhoeven <geert+renesas@glider.be>
 
-allOf:
-  - $ref: spi-controller.yaml#
-
 properties:
   compatible:
     oneOf:
@@ -146,24 +143,38 @@ properties:
     $ref: /schemas/types.yaml#/definitions/uint32
     default: 64
 
+  # for MSIOF-I2S
+  port:
+    $ref: ../sound/audio-graph-port.yaml#
+    unevaluatedProperties: false
+
 required:
   - compatible
   - reg
   - interrupts
   - clocks
   - power-domains
-  - '#address-cells'
-  - '#size-cells'
-
-if:
-  not:
-    properties:
-      compatible:
-        contains:
-          const: renesas,sh-mobile-msiof
-then:
-  required:
-    - resets
+
+allOf:
+  # additional "required""
+  - if:
+      not:
+        properties:
+          compatible:
+            contains:
+              const: renesas,sh-mobile-msiof
+    then:
+      required:
+        - resets
+
+  # "MSIOF-SPI" specific
+  - if:
+      properties:
+        $nodename:
+          pattern: '^spi@'
+    then:
+      allOf:
+        - $ref: spi-controller.yaml#
 
 unevaluatedProperties: false
 
-- 
2.43.0

[PATCH v3 02/10] spi: sh-msiof: use dev in sh_msiof_spi_probe()

[Kuninori Morimoto]

sh_msiof_spi_probe() is using priv->dev everywhare,
but makes code long. Create struct device *dev and use it.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 drivers/spi/spi-sh-msiof.c | 36 +++++++++++++++++-------------------
 1 file changed, 17 insertions(+), 19 deletions(-)

diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index 8a98c313548e..581cec19cb77 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -1276,20 +1276,21 @@ static int sh_msiof_spi_probe(struct platform_device *pdev)
 	const struct sh_msiof_chipdata *chipdata;
 	struct sh_msiof_spi_info *info;
 	struct sh_msiof_spi_priv *p;
+	struct device *dev = &pdev->dev;
 	unsigned long clksrc;
 	int i;
 	int ret;
 
-	chipdata = of_device_get_match_data(&pdev->dev);
+	chipdata = of_device_get_match_data(dev);
 	if (chipdata) {
-		info = sh_msiof_spi_parse_dt(&pdev->dev);
+		info = sh_msiof_spi_parse_dt(dev);
 	} else {
 		chipdata = (const void *)pdev->id_entry->driver_data;
-		info = dev_get_platdata(&pdev->dev);
+		info = dev_get_platdata(dev);
 	}
 
 	if (!info) {
-		dev_err(&pdev->dev, "failed to obtain device info\n");
+		dev_err(dev, "failed to obtain device info\n");
 		return -ENXIO;
 	}
 
@@ -1297,11 +1298,9 @@ static int sh_msiof_spi_probe(struct platform_device *pdev)
 		info->dtdl = 200;
 
 	if (info->mode == MSIOF_SPI_TARGET)
-		ctlr = spi_alloc_target(&pdev->dev,
-				        sizeof(struct sh_msiof_spi_priv));
+		ctlr = spi_alloc_target(dev, sizeof(struct sh_msiof_spi_priv));
 	else
-		ctlr = spi_alloc_host(&pdev->dev,
-				      sizeof(struct sh_msiof_spi_priv));
+		ctlr = spi_alloc_host(dev, sizeof(struct sh_msiof_spi_priv));
 	if (ctlr == NULL)
 		return -ENOMEM;
 
@@ -1315,9 +1314,9 @@ static int sh_msiof_spi_probe(struct platform_device *pdev)
 	init_completion(&p->done);
 	init_completion(&p->done_txdma);
 
-	p->clk = devm_clk_get(&pdev->dev, NULL);
+	p->clk = devm_clk_get(dev, NULL);
 	if (IS_ERR(p->clk)) {
-		dev_err(&pdev->dev, "cannot get clock\n");
+		dev_err(dev, "cannot get clock\n");
 		ret = PTR_ERR(p->clk);
 		goto err1;
 	}
@@ -1334,15 +1333,14 @@ static int sh_msiof_spi_probe(struct platform_device *pdev)
 		goto err1;
 	}
 
-	ret = devm_request_irq(&pdev->dev, i, sh_msiof_spi_irq, 0,
-			       dev_name(&pdev->dev), p);
+	ret = devm_request_irq(dev, i, sh_msiof_spi_irq, 0, dev_name(&pdev->dev), p);
 	if (ret) {
-		dev_err(&pdev->dev, "unable to request irq\n");
+		dev_err(dev, "unable to request irq\n");
 		goto err1;
 	}
 
 	p->pdev = pdev;
-	pm_runtime_enable(&pdev->dev);
+	pm_runtime_enable(dev);
 
 	/* Platform data may override FIFO sizes */
 	p->tx_fifo_size = chipdata->tx_fifo_size;
@@ -1361,7 +1359,7 @@ static int sh_msiof_spi_probe(struct platform_device *pdev)
 	ctlr->flags = chipdata->ctlr_flags;
 	ctlr->bus_num = pdev->id;
 	ctlr->num_chipselect = p->info->num_chipselect;
-	ctlr->dev.of_node = pdev->dev.of_node;
+	ctlr->dev.of_node = dev->of_node;
 	ctlr->setup = sh_msiof_spi_setup;
 	ctlr->prepare_message = sh_msiof_prepare_message;
 	ctlr->target_abort = sh_msiof_target_abort;
@@ -1373,11 +1371,11 @@ static int sh_msiof_spi_probe(struct platform_device *pdev)
 
 	ret = sh_msiof_request_dma(p);
 	if (ret < 0)
-		dev_warn(&pdev->dev, "DMA not available, using PIO\n");
+		dev_warn(dev, "DMA not available, using PIO\n");
 
-	ret = devm_spi_register_controller(&pdev->dev, ctlr);
+	ret = devm_spi_register_controller(dev, ctlr);
 	if (ret < 0) {
-		dev_err(&pdev->dev, "devm_spi_register_controller error.\n");
+		dev_err(dev, "devm_spi_register_controller error.\n");
 		goto err2;
 	}
 
@@ -1385,7 +1383,7 @@ static int sh_msiof_spi_probe(struct platform_device *pdev)
 
  err2:
 	sh_msiof_release_dma(p);
-	pm_runtime_disable(&pdev->dev);
+	pm_runtime_disable(dev);
  err1:
 	spi_controller_put(ctlr);
 	return ret;
-- 
2.43.0

[PATCH v3 03/10] spi: sh-msiof: ignore driver probing if it was MSIOF Sound

[Kuninori Morimoto]

Renesas MSIOF (Clock-Synchronized Serial Interface with FIFO) can work as
both SPI and I2S. MSIOF-I2S will use Audio Graph Card/Card2 driver which
Of-Graph in DT.

MSIOF-SPI/I2S are using same DT compatible properties.
MSIOF-I2S         uses Of-Graph for Audio-Graph-Card/Card2,
MSIOF-SPI doesn't use  Of-Graph.

Check "port" node when driver probing

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/spi/spi-sh-msiof.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index 581cec19cb77..d9e3d83fc7e8 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -20,6 +20,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_graph.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/sh_dma.h>
@@ -1281,6 +1282,11 @@ static int sh_msiof_spi_probe(struct platform_device *pdev)
 	int i;
 	int ret;
 
+	/* Check whether MSIOF is used as I2S mode or SPI mode by checking "port" node */
+	struct device_node *port __free(device_node) = of_graph_get_next_port(dev->of_node, NULL);
+	if (port) /* It was MSIOF-I2S */
+		return -ENODEV;
+
 	chipdata = of_device_get_match_data(dev);
 	if (chipdata) {
 		info = sh_msiof_spi_parse_dt(dev);
-- 
2.43.0

[PATCH v3 04/10] ASoC: renesas: rsnd: allow to use ADG as standalone

[Kuninori Morimoto]

Audio clock generator (= ADG) can be used standalone (without DAI),
but current driver indicates error if it doesn't have DAI on DT.
It is not error, allow to use ADG as standalone.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/renesas/rcar/core.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/sound/soc/renesas/rcar/core.c b/sound/soc/renesas/rcar/core.c
index 30afc942d381..4f4ed24cb361 100644
--- a/sound/soc/renesas/rcar/core.c
+++ b/sound/soc/renesas/rcar/core.c
@@ -1482,8 +1482,13 @@ static int rsnd_dai_probe(struct rsnd_priv *priv)
 	int dai_i;
 
 	nr = rsnd_dai_of_node(priv, &is_graph);
+
+	/*
+	 * There is a case that it is used only for ADG (Sound Clock).
+	 * No DAI is not error
+	 */
 	if (!nr)
-		return -EINVAL;
+		return 0;
 
 	rdrv = devm_kcalloc(dev, nr, sizeof(*rdrv), GFP_KERNEL);
 	rdai = devm_kcalloc(dev, nr, sizeof(*rdai), GFP_KERNEL);
-- 
2.43.0

[PATCH v3 05/10] ASoC: renesas: rsnd: care BRGA/BRGB select in rsnd_adg_clk_enable()

[Kuninori Morimoto]

Renesas rsnd related clocks are enabled by rsnd_adg_clk_enable(),
but it doesn't care about BRGA/BRGB selection (It is handled when
SSI was started) (BRGA is used for 44.1kHz lineage, BRGB is used for
48kHz lineage in this driver).

But it should be handled since probe time.
Includes BRGCKR_31 in adg->ckr to handle it since boot time.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/renesas/rcar/adg.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/sound/soc/renesas/rcar/adg.c b/sound/soc/renesas/rcar/adg.c
index 191f212d338c..e6b7273b27ad 100644
--- a/sound/soc/renesas/rcar/adg.c
+++ b/sound/soc/renesas/rcar/adg.c
@@ -19,6 +19,7 @@
 #define CLKOUT3	3
 #define CLKOUTMAX 4
 
+#define BRGCKR_31	(1 << 31)
 #define BRRx_MASK(x) (0x3FF & x)
 
 static struct rsnd_mod_ops adg_ops = {
@@ -361,10 +362,13 @@ int rsnd_adg_ssi_clk_try_start(struct rsnd_mod *ssi_mod, unsigned int rate)
 
 	rsnd_adg_set_ssi_clk(ssi_mod, data);
 
+	ckr = adg->ckr & ~BRGCKR_31;
 	if (0 == (rate % 8000))
-		ckr = 0x80000000; /* BRGB output = 48kHz */
-
-	rsnd_mod_bset(adg_mod, BRGCKR, 0x80770000, adg->ckr | ckr);
+		ckr |= BRGCKR_31; /* use BRGB output = 48kHz */
+	if (ckr != adg->ckr) {
+		rsnd_mod_bset(adg_mod, BRGCKR, 0x80770000, adg->ckr);
+		adg->ckr = ckr;
+	}
 
 	dev_dbg(dev, "CLKOUT is based on BRG%c (= %dHz)\n",
 		(ckr) ? 'B' : 'A',
@@ -683,6 +687,9 @@ static int rsnd_adg_get_clkout(struct rsnd_priv *priv)
 	}
 
 rsnd_adg_get_clkout_end:
+	if (0 == (req_rate[0] % 8000))
+		ckr |= BRGCKR_31; /* use BRGB output = 48kHz */
+
 	adg->ckr = ckr;
 	adg->brga = brga;
 	adg->brgb = brgb;
-- 
2.43.0

[PATCH v3 06/10] ASoC: renesas: rsnd: enable to use "adg" clock

[Kuninori Morimoto]

ADG needs its MSTP to use it, and it was handled as "clk_i" before.
R-Car Gen2/Gen3 are using it, but Gen4 doesn't have it.
"clk_i" is not intuitive for ADG MSTP.
Let's enable to use "adg" clock. It can keep compatible with R-Car
Gen2/Gen3 and Gen4.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/renesas/rcar/adg.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/sound/soc/renesas/rcar/adg.c b/sound/soc/renesas/rcar/adg.c
index e6b7273b27ad..8641b73d1f77 100644
--- a/sound/soc/renesas/rcar/adg.c
+++ b/sound/soc/renesas/rcar/adg.c
@@ -31,6 +31,7 @@ static struct rsnd_mod_ops adg_ops = {
 #define ADG_HZ_SIZE	2
 
 struct rsnd_adg {
+	struct clk *adg;
 	struct clk *clkin[CLKINMAX];
 	struct clk *clkout[CLKOUTMAX];
 	struct clk *null_clk;
@@ -386,6 +387,10 @@ int rsnd_adg_clk_control(struct rsnd_priv *priv, int enable)
 	int ret = 0, i;
 
 	if (enable) {
+		ret = clk_prepare_enable(adg->adg);
+		if (ret < 0)
+			return ret;
+
 		rsnd_mod_bset(adg_mod, BRGCKR, 0x80770000, adg->ckr);
 		rsnd_mod_write(adg_mod, BRRA,  adg->brga);
 		rsnd_mod_write(adg_mod, BRRB,  adg->brgb);
@@ -419,6 +424,10 @@ int rsnd_adg_clk_control(struct rsnd_priv *priv, int enable)
 	if (ret < 0)
 		rsnd_adg_clk_disable(priv);
 
+	/* disable adg */
+	if (!enable)
+		clk_disable_unprepare(adg->adg);
+
 	return ret;
 }
 
@@ -475,6 +484,16 @@ static int rsnd_adg_get_clkin(struct rsnd_priv *priv)
 		clkin_size = ARRAY_SIZE(clkin_name_gen4);
 	}
 
+	/*
+	 * get adg
+	 * No "adg" is not error
+	 */
+	clk = devm_clk_get(dev, "adg");
+	if (IS_ERR_OR_NULL(clk))
+		clk = rsnd_adg_null_clk_get(priv);
+	adg->adg = clk;
+
+	/* get clkin */
 	for (i = 0; i < clkin_size; i++) {
 		clk = devm_clk_get(dev, clkin_name[i]);
 
-- 
2.43.0

[PATCH v3 07/10] ASoC: renesas: add MSIOF sound support

[Kuninori Morimoto]

Renesas MSIOF (Clock-Synchronized Serial Interface with FIFO) can work as
both SPI and I2S. Adds MSIOF-I2S driver.

MSIOF-SPI/I2S are using same DT compatible properties.
MSIOF-I2S         uses Of-Graph for Audio-Graph-Card/Card2,
MSIOF-SPI doesn't use  Of-Graph.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/renesas/Kconfig       |   7 +
 sound/soc/renesas/rcar/Makefile |   3 +
 sound/soc/renesas/rcar/msiof.c  | 566 ++++++++++++++++++++++++++++++++
 3 files changed, 576 insertions(+)
 create mode 100644 sound/soc/renesas/rcar/msiof.c

diff --git a/sound/soc/renesas/Kconfig b/sound/soc/renesas/Kconfig
index cb01fb36355f..dabf02a955ca 100644
--- a/sound/soc/renesas/Kconfig
+++ b/sound/soc/renesas/Kconfig
@@ -46,6 +46,13 @@ config SND_SOC_RCAR
 	help
 	  This option enables R-Car SRU/SCU/SSIU/SSI sound support
 
+config SND_SOC_MSIOF
+	tristate "R-Car series MSIOF support"
+	depends on OF
+	select SND_DMAENGINE_PCM
+	help
+	  This option enables R-Car MSIOF sound support
+
 config SND_SOC_RZ
 	tristate "RZ/G2L series SSIF-2 support"
 	depends on ARCH_RZG2L || COMPILE_TEST
diff --git a/sound/soc/renesas/rcar/Makefile b/sound/soc/renesas/rcar/Makefile
index 45eb875a912a..3a2c875595bd 100644
--- a/sound/soc/renesas/rcar/Makefile
+++ b/sound/soc/renesas/rcar/Makefile
@@ -1,3 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 snd-soc-rcar-y		:= core.o gen.o dma.o adg.o ssi.o ssiu.o src.o ctu.o mix.o dvc.o cmd.o debugfs.o
 obj-$(CONFIG_SND_SOC_RCAR)	+= snd-soc-rcar.o
+
+snd-soc-msiof-y			:= msiof.o
+obj-$(CONFIG_SND_SOC_MSIOF)	+= snd-soc-msiof.o
diff --git a/sound/soc/renesas/rcar/msiof.c b/sound/soc/renesas/rcar/msiof.c
new file mode 100644
index 000000000000..2c9c7eebfd39
--- /dev/null
+++ b/sound/soc/renesas/rcar/msiof.c
@@ -0,0 +1,566 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// Renesas R-Car MSIOF (Clock-Synchronized Serial Interface with FIFO) I2S driver
+//
+// Copyright (C) 2025 Renesas Solutions Corp.
+// Author: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
+//
+
+/*
+ * [NOTE]
+ *
+ * This driver doesn't support Clock/Frame Provider Mode
+ *
+ * Basically MSIOF is created for SPI, but we can use it as I2S (Sound), etc. Because of it, when
+ * we use it as I2S (Sound) with Provider Mode, we need to send dummy TX data even though it was
+ * used for RX. Because SPI HW needs TX Clock/Frame output for RX purpose.
+ * But it makes driver code complex in I2S (Sound).
+ *
+ * And when we use it as I2S (Sound) as Provider Mode, the clock source is [MSO clock] (= 133.33MHz)
+ * SoC internal clock. It is not for 48kHz/44.1kHz base clock. Thus the output/input will not be
+ * accurate sound.
+ *
+ * Because of these reasons, this driver doesn't support Clock/Frame Provider Mode. Use it as
+ * Clock/Frame Consumer Mode.
+ */
+
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_dma.h>
+#include <linux/of_graph.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <sound/dmaengine_pcm.h>
+#include <sound/soc.h>
+
+/* register */
+#define SITMDR1		0x00
+#define SITMDR2		0x04
+#define SITMDR3		0x08
+#define SIRMDR1		0x10
+#define SIRMDR2		0x14
+#define SIRMDR3		0x18
+#define SICTR		0x28
+#define SISTR		0x40
+#define SIIER		0x44
+#define SITFDR		0x50
+#define SIRFDR		0x60
+
+/* SITMDR1/ SIRMDR1 */
+#define PCON		(1 << 30)		/* Transfer Signal Connection */
+#define SYNCMD_LR	(3 << 28)		/* L/R mode */
+#define SYNCAC		(1 << 25)		/* Sync Polarity (Active-low) */
+#define DTDL_1		(1 << 20)		/* 1-clock-cycle delay */
+#define TXSTP		(1 <<  0)		/* Transmission/Reception Stop on FIFO */
+
+/* SITMDR2 and SIRMDR2 */
+#define BITLEN1(x)	(((x) - 1) << 24)	/* Data Size (8-32 bits) */
+#define GRP		(1 << 30)		/* Group count */
+
+/* SICTR */
+#define TEDG		(1 << 27)		/* Transmit Timing (1 = falling edge) */
+#define REDG		(1 << 26)		/* Receive  Timing (1 = rising  edge) */
+#define TXE		(1 <<  9)		/* Transmit Enable */
+#define RXE		(1 <<  8)		/* Receive Enable */
+
+/* SISTR */
+#define TFSERR		(1 << 21)		/* Transmit Frame Synchronization Error */
+#define TFOVF		(1 << 20)		/* Transmit FIFO Overflow */
+#define TFUDF		(1 << 19)		/* Transmit FIFO Underflow */
+#define RFSERR		(1 <<  5)		/* Receive Frame Synchronization Error */
+#define RFUDF		(1 <<  4)		/* Receive FIFO Underflow */
+#define RFOVF		(1 <<  3)		/* Receive FIFO Overflow */
+#define SISTR_ERR_TX	(TFSERR | TFOVF | TFUDF)
+#define SISTR_ERR_RX	(RFSERR | RFOVF | RFUDF)
+#define SISTR_ERR	(SISTR_ERR_TX | SISTR_ERR_RX)
+
+/* SIIER */
+#define TDMAE		(1 << 31)		/* Transmit Data DMA Transfer Req. Enable */
+#define TDREQE		(1 << 28)		/* Transmit Data Transfer Request Enable */
+#define RDMAE		(1 << 15)		/* Receive Data DMA Transfer Req. Enable */
+#define RDREQE		(1 << 12)		/* Receive Data Transfer Request Enable */
+
+/*
+ * The data on memory in 24bit case is located at <right> side
+ *	[  xxxxxx]
+ *	[  xxxxxx]
+ *	[  xxxxxx]
+ *
+ * HW assuming signal in 24bit case is located at <left> side
+ *	---+         +---------+
+ *	   +---------+         +---------+...
+ *	   [xxxxxx  ][xxxxxx  ][xxxxxx  ]
+ *
+ * When we use 24bit data, it will be transferred via 32bit width via DMA,
+ * and MSIOF/DMA doesn't support data shift, we can't use 24bit data correctly.
+ * There is no such issue on 16/32bit data case.
+ */
+#define MSIOF_RATES	SNDRV_PCM_RATE_8000_192000
+#define MSIOF_FMTS	(SNDRV_PCM_FMTBIT_S16_LE |\
+			 SNDRV_PCM_FMTBIT_S32_LE)
+
+struct msiof_priv {
+	struct device *dev;
+	struct snd_pcm_substream *substream[SNDRV_PCM_STREAM_LAST + 1];
+	spinlock_t lock;
+	void __iomem *base;
+	resource_size_t phy_addr;
+
+	/* for error */
+	int err_syc[SNDRV_PCM_STREAM_LAST + 1];
+	int err_ovf[SNDRV_PCM_STREAM_LAST + 1];
+	int err_udf[SNDRV_PCM_STREAM_LAST + 1];
+
+	/* bit field */
+	u32 flags;
+#define MSIOF_FLAGS_NEED_DELAY		(1 << 0)
+};
+#define msiof_flag_has(priv, flag)	(priv->flags &  flag)
+#define msiof_flag_set(priv, flag)	(priv->flags |= flag)
+
+#define msiof_is_play(substream)	((substream)->stream == SNDRV_PCM_STREAM_PLAYBACK)
+#define msiof_read(priv, reg)		ioread32((priv)->base + reg)
+#define msiof_write(priv, reg, val)	iowrite32(val, (priv)->base + reg)
+#define msiof_status_clear(priv)	msiof_write(priv, SISTR, SISTR_ERR)
+
+static void msiof_update(struct msiof_priv *priv, u32 reg, u32 mask, u32 val)
+{
+	u32 old = msiof_read(priv, reg);
+	u32 new = (old & ~mask) | (val & mask);
+
+	if (old != new)
+		msiof_write(priv, reg, new);
+}
+
+static void msiof_update_and_wait(struct msiof_priv *priv, u32 reg, u32 mask, u32 val, u32 expect)
+{
+	u32 data;
+	int ret;
+
+	msiof_update(priv, reg, mask, val);
+
+	ret = readl_poll_timeout_atomic(priv->base + reg, data,
+					(data & mask) == expect, 1, 128);
+	if (ret)
+		dev_warn(priv->dev, "write timeout [0x%02x] 0x%08x / 0x%08x\n",
+			 reg, data, expect);
+}
+
+static int msiof_hw_start(struct snd_soc_component *component,
+			  struct snd_pcm_substream *substream, int cmd)
+{
+	struct msiof_priv *priv = snd_soc_component_get_drvdata(component);
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	int is_play = msiof_is_play(substream);
+	int width = snd_pcm_format_width(runtime->format);
+	u32 val;
+
+	/*
+	 * see
+	 *	[NOTE] on top of this driver
+	 */
+	/*
+	 * see
+	 *	Datasheet 109.3.6 [Transmit and Receive Procedures]
+	 *
+	 *	TX: Fig 109.14	- Fig 109.23
+	 *	RX: Fig 109.15
+	 */
+
+	/* reset errors */
+	priv->err_syc[substream->stream] =
+	priv->err_ovf[substream->stream] =
+	priv->err_udf[substream->stream] = 0;
+
+	/* SITMDRx */
+	if (is_play) {
+		val = PCON | SYNCMD_LR | SYNCAC | TXSTP;
+		if (msiof_flag_has(priv, MSIOF_FLAGS_NEED_DELAY))
+			val |= DTDL_1;
+
+		msiof_write(priv, SITMDR1, val);
+
+		val = BITLEN1(width);
+		msiof_write(priv, SITMDR2, val | GRP);
+		msiof_write(priv, SITMDR3, val);
+
+	}
+	/* SIRMDRx */
+	else {
+		val = SYNCMD_LR | SYNCAC;
+		if (msiof_flag_has(priv, MSIOF_FLAGS_NEED_DELAY))
+			val |= DTDL_1;
+
+		msiof_write(priv, SIRMDR1, val);
+
+		val = BITLEN1(width);
+		msiof_write(priv, SIRMDR2, val | GRP);
+		msiof_write(priv, SIRMDR3, val);
+	}
+
+	/* SIIER */
+	if (is_play)
+		val = TDREQE | TDMAE | SISTR_ERR_TX;
+	else
+		val = RDREQE | RDMAE | SISTR_ERR_RX;
+	msiof_update(priv, SIIER, val, val);
+
+	/* SICTR */
+	if (is_play)
+		val = TXE | TEDG;
+	else
+		val = RXE | REDG;
+	msiof_update_and_wait(priv, SICTR, val, val, val);
+
+	msiof_status_clear(priv);
+
+	/* Start DMAC */
+	snd_dmaengine_pcm_trigger(substream, cmd);
+
+	return 0;
+}
+
+static int msiof_hw_stop(struct snd_soc_component *component,
+			 struct snd_pcm_substream *substream, int cmd)
+{
+	struct msiof_priv *priv = snd_soc_component_get_drvdata(component);
+	struct device *dev = component->dev;
+	int is_play = msiof_is_play(substream);
+	u32 val;
+
+	/* SIIER */
+	if (is_play)
+		val = TDREQE | TDMAE | SISTR_ERR_TX;
+	else
+		val = RDREQE | RDMAE | SISTR_ERR_RX;
+	msiof_update(priv, SIIER, val, 0);
+
+	/* Stop DMAC */
+	snd_dmaengine_pcm_trigger(substream, cmd);
+
+	/* SICTR */
+	if (is_play)
+		val = TXE;
+	else
+		val = RXE;
+	msiof_update_and_wait(priv, SICTR, val, 0, 0);
+
+	/* indicate error status if exist */
+	if (priv->err_syc[substream->stream] ||
+	    priv->err_ovf[substream->stream] ||
+	    priv->err_udf[substream->stream])
+		dev_warn(dev, "FSERR(%s) = %d, FOVF = %d, FUDF = %d\n",
+			 snd_pcm_direction_name(substream->stream),
+			 priv->err_syc[substream->stream],
+			 priv->err_ovf[substream->stream],
+			 priv->err_udf[substream->stream]);
+
+	return 0;
+}
+
+static int msiof_dai_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
+{
+	struct msiof_priv *priv = snd_soc_dai_get_drvdata(dai);
+
+	switch (fmt & SND_SOC_DAIFMT_CLOCK_PROVIDER_MASK) {
+	/*
+	 * It supports Clock/Frame Consumer Mode only
+	 * see
+	 *	[NOTE] on top of this driver
+	 */
+	case SND_SOC_DAIFMT_BC_FC:
+		break;
+	/* others are error */
+	default:
+		return -EINVAL;
+	}
+
+	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
+	/* it supports NB_NF only */
+	case SND_SOC_DAIFMT_NB_NF:
+	default:
+		break;
+	/* others are error */
+	case SND_SOC_DAIFMT_NB_IF:
+	case SND_SOC_DAIFMT_IB_NF:
+	case SND_SOC_DAIFMT_IB_IF:
+		return -EINVAL;
+	}
+
+	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
+	case SND_SOC_DAIFMT_I2S:
+		msiof_flag_set(priv, MSIOF_FLAGS_NEED_DELAY);
+		break;
+	case SND_SOC_DAIFMT_LEFT_J:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+/*
+ * Select below from Sound Card, not auto
+ *	SND_SOC_DAIFMT_CBC_CFC
+ *	SND_SOC_DAIFMT_CBP_CFP
+ */
+static const u64 msiof_dai_formats = SND_SOC_POSSIBLE_DAIFMT_I2S	|
+				     SND_SOC_POSSIBLE_DAIFMT_LEFT_J	|
+				     SND_SOC_POSSIBLE_DAIFMT_NB_NF;
+
+static const struct snd_soc_dai_ops msiof_dai_ops = {
+	.set_fmt			= msiof_dai_set_fmt,
+	.auto_selectable_formats	= &msiof_dai_formats,
+	.num_auto_selectable_formats	= 1,
+};
+
+static struct snd_soc_dai_driver msiof_dai_driver = {
+	.name = "msiof-dai",
+	.playback = {
+		.rates		= MSIOF_RATES,
+		.formats	= MSIOF_FMTS,
+		.channels_min	= 2,
+		.channels_max	= 2,
+	},
+	.capture = {
+		.rates		= MSIOF_RATES,
+		.formats	= MSIOF_FMTS,
+		.channels_min	= 2,
+		.channels_max	= 2,
+	},
+	.ops = &msiof_dai_ops,
+};
+
+static struct snd_pcm_hardware msiof_pcm_hardware = {
+	.info =	SNDRV_PCM_INFO_INTERLEAVED	|
+		SNDRV_PCM_INFO_MMAP		|
+		SNDRV_PCM_INFO_MMAP_VALID,
+	.buffer_bytes_max	= 64 * 1024,
+	.period_bytes_min	= 32,
+	.period_bytes_max	= 8192,
+	.periods_min		= 1,
+	.periods_max		= 32,
+	.fifo_size		= 64,
+};
+
+static int msiof_open(struct snd_soc_component *component,
+		      struct snd_pcm_substream *substream)
+{
+	struct device *dev = component->dev;
+	struct dma_chan *chan;
+	static const char * const dma_names[] = {"rx", "tx"};
+	int is_play = msiof_is_play(substream);
+	int ret;
+
+	chan = of_dma_request_slave_channel(dev->of_node, dma_names[is_play]);
+	if (IS_ERR(chan))
+		return PTR_ERR(chan);
+
+	ret = snd_dmaengine_pcm_open(substream, chan);
+	if (ret < 0)
+		goto open_err_dma;
+
+	snd_soc_set_runtime_hwparams(substream, &msiof_pcm_hardware);
+
+	ret = snd_pcm_hw_constraint_integer(substream->runtime, SNDRV_PCM_HW_PARAM_PERIODS);
+
+open_err_dma:
+	if (ret < 0)
+		dma_release_channel(chan);
+
+	return ret;
+}
+
+static int msiof_close(struct snd_soc_component *component,
+		       struct snd_pcm_substream *substream)
+{
+	return snd_dmaengine_pcm_close_release_chan(substream);
+}
+
+static snd_pcm_uframes_t msiof_pointer(struct snd_soc_component *component,
+				       struct snd_pcm_substream *substream)
+{
+	return snd_dmaengine_pcm_pointer(substream);
+}
+
+#define PREALLOC_BUFFER		(32 * 1024)
+#define PREALLOC_BUFFER_MAX	(32 * 1024)
+static int msiof_new(struct snd_soc_component *component,
+		     struct snd_soc_pcm_runtime *rtd)
+{
+	snd_pcm_set_managed_buffer_all(rtd->pcm, SNDRV_DMA_TYPE_DEV,
+				       rtd->card->snd_card->dev,
+				       PREALLOC_BUFFER, PREALLOC_BUFFER_MAX);
+	return 0;
+}
+
+static int msiof_trigger(struct snd_soc_component *component,
+			 struct snd_pcm_substream *substream, int cmd)
+{
+	struct device *dev = component->dev;
+	struct msiof_priv *priv = dev_get_drvdata(dev);
+	unsigned long flags;
+	int ret = -EINVAL;
+
+	spin_lock_irqsave(&priv->lock, flags);
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+		priv->substream[substream->stream] = substream;
+		fallthrough;
+	case SNDRV_PCM_TRIGGER_RESUME:
+		ret = msiof_hw_start(component, substream, cmd);
+		break;
+	case SNDRV_PCM_TRIGGER_STOP:
+		priv->substream[substream->stream] = NULL;
+		fallthrough;
+	case SNDRV_PCM_TRIGGER_SUSPEND:
+		ret = msiof_hw_stop(component, substream, cmd);
+		break;
+	}
+
+	spin_unlock_irqrestore(&priv->lock, flags);
+
+	return ret;
+}
+
+static int msiof_hw_params(struct snd_soc_component *component,
+			   struct snd_pcm_substream *substream,
+			   struct snd_pcm_hw_params *params)
+{
+	struct msiof_priv *priv = dev_get_drvdata(component->dev);
+	struct dma_chan *chan = snd_dmaengine_pcm_get_chan(substream);
+	struct dma_slave_config cfg = {};
+	unsigned long flags;
+	int ret;
+
+	spin_lock_irqsave(&priv->lock, flags);
+
+	ret = snd_hwparams_to_dma_slave_config(substream, params, &cfg);
+	if (ret < 0)
+		goto hw_params_out;
+
+	cfg.dst_addr = priv->phy_addr + SITFDR;
+	cfg.src_addr = priv->phy_addr + SIRFDR;
+
+	ret = dmaengine_slave_config(chan, &cfg);
+hw_params_out:
+	spin_unlock_irqrestore(&priv->lock, flags);
+
+	return ret;
+}
+
+static const struct snd_soc_component_driver msiof_component_driver = {
+	.name		= "msiof",
+	.open		= msiof_open,
+	.close		= msiof_close,
+	.pointer	= msiof_pointer,
+	.pcm_construct	= msiof_new,
+	.trigger	= msiof_trigger,
+	.hw_params	= msiof_hw_params,
+};
+
+static irqreturn_t msiof_interrupt(int irq, void *data)
+{
+	struct msiof_priv *priv = data;
+	struct snd_pcm_substream *substream;
+	u32 sistr;
+
+	spin_lock(&priv->lock);
+
+	sistr = msiof_read(priv, SISTR);
+	msiof_status_clear(priv);
+
+	spin_unlock(&priv->lock);
+
+	/* overflow/underflow error */
+	substream = priv->substream[SNDRV_PCM_STREAM_PLAYBACK];
+	if (substream && (sistr & SISTR_ERR_TX)) {
+		// snd_pcm_stop_xrun(substream);
+		if (sistr & TFSERR)
+			priv->err_syc[SNDRV_PCM_STREAM_PLAYBACK]++;
+		if (sistr & TFOVF)
+			priv->err_ovf[SNDRV_PCM_STREAM_PLAYBACK]++;
+		if (sistr & TFUDF)
+			priv->err_udf[SNDRV_PCM_STREAM_PLAYBACK]++;
+	}
+
+	substream = priv->substream[SNDRV_PCM_STREAM_CAPTURE];
+	if (substream && (sistr & SISTR_ERR_RX)) {
+		// snd_pcm_stop_xrun(substream);
+		if (sistr & RFSERR)
+			priv->err_syc[SNDRV_PCM_STREAM_CAPTURE]++;
+		if (sistr & RFOVF)
+			priv->err_ovf[SNDRV_PCM_STREAM_CAPTURE]++;
+		if (sistr & RFUDF)
+			priv->err_udf[SNDRV_PCM_STREAM_CAPTURE]++;
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int msiof_probe(struct platform_device *pdev)
+{
+	struct msiof_priv *priv;
+	struct device *dev = &pdev->dev;
+	struct resource *res;
+	int irq, ret;
+
+	/* Check MSIOF as Sound mode or SPI mode */
+	struct device_node *port __free(device_node) = of_graph_get_next_port(dev->of_node, NULL);
+	if (!port)
+		return -ENODEV;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -ENODEV;
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq <= 0)
+		return -ENODEV;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENODEV;
+
+	priv->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(priv->base))
+		return PTR_ERR(priv->base);
+
+	ret = devm_request_irq(dev, irq, msiof_interrupt, 0, dev_name(dev), priv);
+	if (ret)
+		return ret;
+
+	priv->dev	= dev;
+	priv->phy_addr	= res->start;
+
+	spin_lock_init(&priv->lock);
+	platform_set_drvdata(pdev, priv);
+
+	devm_pm_runtime_enable(dev);
+
+	ret = devm_snd_soc_register_component(dev, &msiof_component_driver,
+					      &msiof_dai_driver, 1);
+
+	return ret;
+}
+
+static const struct of_device_id msiof_of_match[] = {
+	{ .compatible = "renesas,rcar-gen4-msiof", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, msiof_of_match);
+
+static struct platform_driver msiof_driver = {
+	.driver	= {
+		.name	= "msiof-pcm-audio",
+		.of_match_table = msiof_of_match,
+	},
+	.probe		= msiof_probe,
+};
+module_platform_driver(msiof_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Renesas R-Car MSIOF I2S audio driver");
+MODULE_AUTHOR("Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>");
-- 
2.43.0

[PATCH v3 08/10] arm64: dts: renesas: r8a779g0: tidyup MSIOF node name

[Kuninori Morimoto]

MSIOF will be used as both SPI or I2S. The node name is not "spi".
Rename it to "serial-engine".

Suggested-by: Krzysztof Kozlowski <krzk@kernel.org>
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 arch/arm64/boot/dts/renesas/r8a779g0.dtsi | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a779g0.dtsi b/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
index 1760720b7128..0b85fa0d1f41 100644
--- a/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
@@ -1250,7 +1250,7 @@ tpu: pwm@e6e80000 {
 			status = "disabled";
 		};
 
-		msiof0: spi@e6e90000 {
+		msiof0: serial-engine@e6e90000 {
 			compatible = "renesas,msiof-r8a779g0",
 				     "renesas,rcar-gen4-msiof";
 			reg = <0 0xe6e90000 0 0x0064>;
@@ -1266,7 +1266,7 @@ msiof0: spi@e6e90000 {
 			status = "disabled";
 		};
 
-		msiof1: spi@e6ea0000 {
+		msiof1: serial-engine@e6ea0000 {
 			compatible = "renesas,msiof-r8a779g0",
 				     "renesas,rcar-gen4-msiof";
 			reg = <0 0xe6ea0000 0 0x0064>;
@@ -1282,7 +1282,7 @@ msiof1: spi@e6ea0000 {
 			status = "disabled";
 		};
 
-		msiof2: spi@e6c00000 {
+		msiof2: serial-engine@e6c00000 {
 			compatible = "renesas,msiof-r8a779g0",
 				     "renesas,rcar-gen4-msiof";
 			reg = <0 0xe6c00000 0 0x0064>;
@@ -1298,7 +1298,7 @@ msiof2: spi@e6c00000 {
 			status = "disabled";
 		};
 
-		msiof3: spi@e6c10000 {
+		msiof3: serial-engine@e6c10000 {
 			compatible = "renesas,msiof-r8a779g0",
 				     "renesas,rcar-gen4-msiof";
 			reg = <0 0xe6c10000 0 0x0064>;
@@ -1314,7 +1314,7 @@ msiof3: spi@e6c10000 {
 			status = "disabled";
 		};
 
-		msiof4: spi@e6c20000 {
+		msiof4: serial-engine@e6c20000 {
 			compatible = "renesas,msiof-r8a779g0",
 				     "renesas,rcar-gen4-msiof";
 			reg = <0 0xe6c20000 0 0x0064>;
@@ -1330,7 +1330,7 @@ msiof4: spi@e6c20000 {
 			status = "disabled";
 		};
 
-		msiof5: spi@e6c28000 {
+		msiof5: serial-engine@e6c28000 {
 			compatible = "renesas,msiof-r8a779g0",
 				     "renesas,rcar-gen4-msiof";
 			reg = <0 0xe6c28000 0 0x0064>;
-- 
2.43.0

[PATCH v3 09/10] arm64: dts: renesas: sparrow hawk: Add MSIOF Sound support

[Kuninori Morimoto]

Sparrow Hawk has Headset (CONN3) AUX_IN (CONN4) for Sound input/output
which is using MSIOF. Support it.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 .../dts/renesas/r8a779g3-sparrow-hawk.dts     | 98 +++++++++++++++++++
 1 file changed, 98 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a779g3-sparrow-hawk.dts b/arch/arm64/boot/dts/renesas/r8a779g3-sparrow-hawk.dts
index b54d45115a85..0a4a9e1f85a5 100644
--- a/arch/arm64/boot/dts/renesas/r8a779g3-sparrow-hawk.dts
+++ b/arch/arm64/boot/dts/renesas/r8a779g3-sparrow-hawk.dts
@@ -4,6 +4,29 @@
  *
  * Copyright (C) 2025 Marek Vasut <marek.vasut+renesas@mailbox.org>
  */
+/*
+ * DA7212 Codec settings
+ *
+ * for Playback
+ *	> amixer set "Headphone" 40%
+ *	> amixer set "Headphone" on
+ *	> amixer set "Mixout Left DAC Left"  on
+ *	> amixer set "Mixout Right DAC Right" on
+ *
+ * for Capture (Aux/Mic)
+ *	> amixer set "Aux" on
+ *	> amixer set "Aux" 80%
+ *	> amixer set "Mixin PGA" on
+ *	> amixer set "Mixin PGA" 50%
+ *	> amixer set "ADC" on
+ *	> amixer set "ADC" 80%
+ *	> amixer set "Mixin Left Aux Left" on
+ *	> amixer set "Mixin Right Aux Right" on
+ *	> amixer set "Mic 1" on
+ *	> amixer set "Mic 1" 80%
+ *	> amixer set "Mixin Left Mic 1" on
+ *	> amixer set "Mixin Right Mic 1" on
+ */
 
 /dts-v1/;
 #include <dt-bindings/gpio/gpio.h>
@@ -150,6 +173,12 @@ vcc_sdhi: regulator-vcc-sdhi {
 		gpios-states = <1>;
 		states = <3300000 0>, <1800000 1>;
 	};
+
+	/* Page 30 / Audio_Codec */
+	sound_card: sound {
+		compatible = "audio-graph-card2";
+		links = <&msiof1_snd>;
+	};
 };
 
 /* Page 22 / Ether_AVB0 */
@@ -341,6 +370,29 @@ i2c0_mux1: i2c@1 {
 			reg = <1>;
 			#address-cells = <1>;
 			#size-cells = <0>;
+
+			/* Page 30 / Audio_Codec */
+			codec@1a {
+				compatible = "dlg,da7212";
+
+				#sound-dai-cells = <0>;
+				reg = <0x1a>;
+
+				clocks = <&rcar_sound>;
+				clock-names = "mclk";
+
+				VDDA-supply   = <&reg_1p8v>;
+				VDDMIC-supply = <&reg_3p3v>;
+				VDDIO-supply  = <&reg_3p3v>;
+
+				port {
+					da7212_endpoint: endpoint {
+						bitclock-master;
+						frame-master;
+						remote-endpoint = <&msiof1_snd_endpoint>;
+					};
+				};
+			};
 		};
 
 		i2c0_mux2: i2c@2 {
@@ -603,6 +655,52 @@ sd_uhs_pins: sd-uhs {
 		function = "mmc";
 		power-source = <1800>;
 	};
+
+	/* Page 30 / Audio_Codec */
+	msiof1_pins: sound {
+		groups = "msiof1_clk", "msiof1_sync", "msiof1_txd", "msiof1_rxd";
+		function = "msiof1";
+	};
+
+	/* Page 30 / Audio_Codec */
+	sound_clk_pins: sound-clk {
+		groups = "audio_clkin", "audio_clkout";
+		function = "audio_clk";
+	};
+};
+
+&audio_clkin {
+	clock-frequency = <24576000>;
+};
+
+/* Page 30 / Audio_Codec */
+&rcar_sound {
+	pinctrl-0 = <&sound_clk_pins>;
+	pinctrl-names = "default";
+
+	/* It is used for ADG output as DA7212_MCLK */
+
+	/* audio_clkout */
+	clock-frequency = <12288000>; /* 48 kHz groups */
+
+	status = "okay";
+};
+
+&msiof1 {
+	pinctrl-0 = <&msiof1_pins>;
+	pinctrl-names = "default";
+
+	status = "okay";
+
+	/* ignore DT warning */
+	/delete-property/#address-cells;
+	/delete-property/#size-cells;
+
+	msiof1_snd: port {
+		msiof1_snd_endpoint: endpoint {
+			remote-endpoint = <&da7212_endpoint>;
+		};
+	};
 };
 
 /* Page 31 / FAN */
-- 
2.43.0

[PATCH v3 10/10] arm64: defconfig: add Renesas MSIOF sound support

[Kuninori Morimoto]

Renesas V4H Sparrow Hawk board needs MSIOF Sound driver.
Support it.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 5bb8f09422a2..b74f5810e0e6 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -1010,6 +1010,7 @@ CONFIG_SND_SOC_ROCKCHIP_RT5645=m
 CONFIG_SND_SOC_RK3399_GRU_SOUND=m
 CONFIG_SND_SOC_SAMSUNG=y
 CONFIG_SND_SOC_RCAR=m
+CONFIG_SND_SOC_MSIOF=m
 CONFIG_SND_SOC_RZ=m
 CONFIG_SND_SOC_SOF_TOPLEVEL=y
 CONFIG_SND_SOC_SOF_OF=y
-- 
2.43.0

Re: [PATCH v3 01/10] dt-bindings: renesas,sh-msiof: Add MSIOF I2S Sound support

[Geert Uytterhoeven]

Hi Morimoto-san,

On Tue, 15 Apr 2025 at 03:33, Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
> Renesas MSIOF (Clock-Synchronized Serial Interface with FIFO) can work as
> both SPI and I2S. MSIOF-I2S will use Audio Graph Card/Card2 driver which
> uses Of-Graph in DT.
>
> MSIOF-SPI/I2S are using same DT compatible properties.
> MSIOF-I2S         uses Of-Graph for Audio-Graph-Card/Card2,
> MSIOF-SPI doesn't use  Of-Graph.
>
> Adds schema for MSIOF-I2S (= Sound).
>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Thanks for the update!

> --- a/Documentation/devicetree/bindings/spi/renesas,sh-msiof.yaml
> +++ b/Documentation/devicetree/bindings/spi/renesas,sh-msiof.yaml
> @@ -4,14 +4,11 @@
>  $id: http://devicetree.org/schemas/spi/renesas,sh-msiof.yaml#
>  $schema: http://devicetree.org/meta-schemas/core.yaml#
>
> -title: Renesas MSIOF SPI controller
> +title: Renesas MSIOF SPI / I2S controller
>
>  maintainers:
>    - Geert Uytterhoeven <geert+renesas@glider.be>
>
> -allOf:
> -  - $ref: spi-controller.yaml#
> -
>  properties:
>    compatible:
>      oneOf:
> @@ -146,24 +143,38 @@ properties:
>      $ref: /schemas/types.yaml#/definitions/uint32
>      default: 64
>
> +  # for MSIOF-I2S
> +  port:
> +    $ref: ../sound/audio-graph-port.yaml#
> +    unevaluatedProperties: false
> +
>  required:
>    - compatible
>    - reg
>    - interrupts
>    - clocks
>    - power-domains
> -  - '#address-cells'
> -  - '#size-cells'
> -
> -if:
> -  not:
> -    properties:
> -      compatible:
> -        contains:
> -          const: renesas,sh-mobile-msiof
> -then:
> -  required:
> -    - resets
> +
> +allOf:
> +  # additional "required""
> +  - if:
> +      not:
> +        properties:
> +          compatible:
> +            contains:
> +              const: renesas,sh-mobile-msiof
> +    then:
> +      required:
> +        - resets
> +
> +  # "MSIOF-SPI" specific
> +  - if:
> +      properties:
> +        $nodename:
> +          pattern: '^spi@'

This condition does not match what you wrote in the cover letter:
the controller is used in I2S mode when a port(s) subnode is present,
and in SPI mode when no port(s) subnode is present.

> +    then:
> +      allOf:
> +        - $ref: spi-controller.yaml#

Documentation/devicetree/bindings/spi/spi-controller.yaml indeed
requires that the node-name matches "^spi(@.*|-([0-9]|[1-9][0-9]+))?$".
The controller's node is located in the SoC-specific .dtsi, where its
intended use case is not yet known, and its node name cannot easily be
overridden in the board .dts that specifies the use case.  Hence the
node name must always be "spi" (and cannot be e.g. "serial-engine").
Let's hope there is no other use case for MSIOF that requires using
a different node name...

>
>  unevaluatedProperties: false

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v3 02/10] spi: sh-msiof: use dev in sh_msiof_spi_probe()

[Geert Uytterhoeven]

Hi Morimoto-san,

Thanks for your patch!

On Tue, 15 Apr 2025 at 03:34, Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
> sh_msiof_spi_probe() is using priv->dev everywhare,

everywhere

> but makes code long. Create struct device *dev and use it.
>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

> --- a/drivers/spi/spi-sh-msiof.c
> +++ b/drivers/spi/spi-sh-msiof.c

> @@ -1334,15 +1333,14 @@ static int sh_msiof_spi_probe(struct platform_device *pdev)
>                 goto err1;
>         }
>
> -       ret = devm_request_irq(&pdev->dev, i, sh_msiof_spi_irq, 0,
> -                              dev_name(&pdev->dev), p);
> +       ret = devm_request_irq(dev, i, sh_msiof_spi_irq, 0, dev_name(&pdev->dev), p);

Looks like you missed one instance ;-)

>         if (ret) {
> -               dev_err(&pdev->dev, "unable to request irq\n");
> +               dev_err(dev, "unable to request irq\n");
>                 goto err1;
>         }
>
>         p->pdev = pdev;
> -       pm_runtime_enable(&pdev->dev);
> +       pm_runtime_enable(dev);
>
>         /* Platform data may override FIFO sizes */
>         p->tx_fifo_size = chipdata->tx_fifo_size;

With the above fixed:
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v3 07/10] ASoC: renesas: add MSIOF sound support

[Geert Uytterhoeven]

Hi Morimoto-san,

On Tue, 15 Apr 2025 at 03:34, Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
> Renesas MSIOF (Clock-Synchronized Serial Interface with FIFO) can work as
> both SPI and I2S. Adds MSIOF-I2S driver.
>
> MSIOF-SPI/I2S are using same DT compatible properties.
> MSIOF-I2S         uses Of-Graph for Audio-Graph-Card/Card2,
> MSIOF-SPI doesn't use  Of-Graph.
>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Thanks for the update!

The hardware driving part LGTM, so
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v3 08/10] arm64: dts: renesas: r8a779g0: tidyup MSIOF node name

[Geert Uytterhoeven]

Hi Morimoto-san,

On Tue, 15 Apr 2025 at 03:34, Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
> MSIOF will be used as both SPI or I2S. The node name is not "spi".
> Rename it to "serial-engine".
>
> Suggested-by: Krzysztof Kozlowski <krzk@kernel.org>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Thanks for your patch!

> --- a/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
> @@ -1250,7 +1250,7 @@ tpu: pwm@e6e80000 {
>                         status = "disabled";
>                 };
>
> -               msiof0: spi@e6e90000 {
> +               msiof0: serial-engine@e6e90000 {

Unfortunately this means that this instance can no longer be used in
SPI mode without changing the node name back.  E.g. with [1]:

    serial-engine@e6ea0000: Unevaluated properties are not allowed
('#address-cells', '#size-cells', 'cn5@0' were unexpected)

>                         compatible = "renesas,msiof-r8a779g0",
>                                      "renesas,rcar-gen4-msiof";
>                         reg = <0 0xe6e90000 0 0x0064>;

[1] "arm64: dts: renesas: white-hawk: cn5: Add overlay for MSIOF1"
    https://web.git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/commit/?h=topic/renesas-overlays&id=0b4f1af2c454b49b9f266900dcb578dc311b1475

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v3 07/10] ASoC: renesas: add MSIOF sound support

[ALOK TIWARI]

Hi Morimoto-san,

Thanks for your patch!
Sorry, I did not ask you earlier.


On 15-04-2025 07:04, Kuninori Morimoto wrote:
> +	/* Check MSIOF as Sound mode or SPI mode */
> +	struct device_node *port __free(device_node) = of_graph_get_next_port(dev->of_node, NULL);
> +	if (!port)
> +		return -ENODEV;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res)
> +		return -ENODEV;
> +
> +	irq = platform_get_irq(pdev, 0);
> +	if (irq <= 0)
> +		return -ENODEV;

nit: -EINVAL will be more relevant here

> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENODEV;

nit: not sure why -ENOMEM not return

> +
> +	priv->base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(priv->base))
> +		return PTR_ERR(priv->base);
> +
> +	ret = devm_request_irq(dev, irq, msiof_interrupt, 0, dev_name(dev), priv);
> +	if (ret)
> +		return ret;
> +
> +	priv->dev	= dev;
> +	priv->phy_addr	= res->start;
> +
> +	spin_lock_init(&priv->lock);
> +	platform_set_drvdata(pdev, priv);


Thanks,
Alok

Re: [PATCH v3 01/10] dt-bindings: renesas,sh-msiof: Add MSIOF I2S Sound support

[Kuninori Morimoto]

Hi Geert

Thank you for your review

> > +  # "MSIOF-SPI" specific
> > +  - if:
> > +      properties:
> > +        $nodename:
> > +          pattern: '^spi@'
> 
> This condition does not match what you wrote in the cover letter:
> the controller is used in I2S mode when a port(s) subnode is present,
> and in SPI mode when no port(s) subnode is present.
> 
> > +    then:
> > +      allOf:
> > +        - $ref: spi-controller.yaml#
> 
> Documentation/devicetree/bindings/spi/spi-controller.yaml indeed
> requires that the node-name matches "^spi(@.*|-([0-9]|[1-9][0-9]+))?$".
> The controller's node is located in the SoC-specific .dtsi, where its
> intended use case is not yet known, and its node name cannot easily be
> overridden in the board .dts that specifies the use case.  Hence the
> node name must always be "spi" (and cannot be e.g. "serial-engine").
> Let's hope there is no other use case for MSIOF that requires using
> a different node name...

Hmm... OK

So what we can do is keep spi@xxx node name, and check whether it has
Of-Graph, and select spi-controller.yaml

Thank you for your help !!

Best regards
---
Kuninori Morimoto

Re: [PATCH v3 02/10] spi: sh-msiof: use dev in sh_msiof_spi_probe()

[Kuninori Morimoto]

Hi Geert

> > sh_msiof_spi_probe() is using priv->dev everywhare,
> 
> everywhere

Grr, thank you for pointing it.

> > -       ret = devm_request_irq(&pdev->dev, i, sh_msiof_spi_irq, 0,
> > -                              dev_name(&pdev->dev), p);
> > +       ret = devm_request_irq(dev, i, sh_msiof_spi_irq, 0, dev_name(&pdev->dev), p);
> 
> Looks like you missed one instance ;-)

Because it can be 1 line now.

> With the above fixed:
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Thanks


Thank you for your help !!

Best regards
---
Kuninori Morimoto

Re: [PATCH v3 07/10] ASoC: renesas: add MSIOF sound support

[Kuninori Morimoto]

Hi ALOK

> > +	irq = platform_get_irq(pdev, 0);
> > +	if (irq <= 0)
> > +		return -ENODEV;
> 
> nit: -EINVAL will be more relevant here
> 
> > +
> > +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > +	if (!priv)
> > +		return -ENODEV;
> 
> nit: not sure why -ENOMEM not return

Thanks. Will fix in v4

Best regards
---
Kuninori Morimoto

Re: [PATCH v3 07/10] ASoC: renesas: add MSIOF sound support

[Lad, Prabhakar]

Hi,

On Wed, Apr 16, 2025 at 12:04 AM Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
>
>
> Hi ALOK
>
> > > +   irq = platform_get_irq(pdev, 0);
> > > +   if (irq <= 0)
> > > +           return -ENODEV;
> >
> > nit: -EINVAL will be more relevant here
This is not a right suggestion, please return back irq so the error
gets propagated. (Alok please read upto date documentation prior to
any suggestions, this is the third patch where I'm noticing wrong
suggestions).

Cheers,
Prabhakar

> >
> > > +
> > > +   priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > > +   if (!priv)
> > > +           return -ENODEV;
> >
> > nit: not sure why -ENOMEM not return
>
> Thanks. Will fix in v4
>
> Best regards
> ---
> Kuninori Morimoto
>

Re: [PATCH v3 01/10] dt-bindings: renesas,sh-msiof: Add MSIOF I2S Sound support

[Kuninori Morimoto]

Hi Geert (as Renesas SoC Maintainer),
   Rob   (as DT          Maintainer),
   Mark  (as SPI         Maintainer)

> > > +  # "MSIOF-SPI" specific
> > > +  - if:
> > > +      properties:
> > > +        $nodename:
> > > +          pattern: '^spi@'
> > 
> > This condition does not match what you wrote in the cover letter:
> > the controller is used in I2S mode when a port(s) subnode is present,
> > and in SPI mode when no port(s) subnode is present.
> > 
> > > +    then:
> > > +      allOf:
> > > +        - $ref: spi-controller.yaml#
> > 
> > Documentation/devicetree/bindings/spi/spi-controller.yaml indeed
> > requires that the node-name matches "^spi(@.*|-([0-9]|[1-9][0-9]+))?$".
> > The controller's node is located in the SoC-specific .dtsi, where its
> > intended use case is not yet known, and its node name cannot easily be
> > overridden in the board .dts that specifies the use case.  Hence the
> > node name must always be "spi" (and cannot be e.g. "serial-engine").
> > Let's hope there is no other use case for MSIOF that requires using
> > a different node name...

Hmm...

Now, MSIOF node has "spi@xxxx".
SoC file indicates MSIOF-SPI as default, so it has below lines

        --- SoC file ----
	msiof1: spi@xxxx {
		...
		#address-cells = <1>;
		#size-cells = <0>;
		...
	};

These are not needed for MSIOF-I2S, so removes these

        --- Board file ----
	&msiof1 {
		...
		/delete-property/#address-cells;
		/delete-property/#size-cells;
		...
	};

Now, my dt-bindings doesn't load spi-controller.yaml (as sample), but I got

	[SoC file]: Warning (spi_bus_bridge): /soc/spi@xxxx: incorrect #address-cells for SPI bus
	  also defined at [Board file]
	[SoC file]: Warning (spi_bus_bridge): /soc/spi@xxxx: incorrect #size-cells for SPI bus
	  also defined at [Board file]

MSIOF dt-bindings doesn't load spi-controller.yaml, but why I got "spi_bus_bridge"
warning ?? I wonder dt compiler (?) automatically check "spi" node ?
I have tryed some code, my expectation seems correct (In case of node name was "spi@xxx",
I got many SPI related warnings even though I didn't load spi-controller).

Best regards
---
Kuninori Morimoto

Re: [PATCH v3 01/10] dt-bindings: renesas,sh-msiof: Add MSIOF I2S Sound support

[Geert Uytterhoeven]

Hi Morimoto-san,

On Thu, 17 Apr 2025 at 01:52, Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
> > > > +  # "MSIOF-SPI" specific
> > > > +  - if:
> > > > +      properties:
> > > > +        $nodename:
> > > > +          pattern: '^spi@'
> > >
> > > This condition does not match what you wrote in the cover letter:
> > > the controller is used in I2S mode when a port(s) subnode is present,
> > > and in SPI mode when no port(s) subnode is present.
> > >
> > > > +    then:
> > > > +      allOf:
> > > > +        - $ref: spi-controller.yaml#
> > >
> > > Documentation/devicetree/bindings/spi/spi-controller.yaml indeed
> > > requires that the node-name matches "^spi(@.*|-([0-9]|[1-9][0-9]+))?$".
> > > The controller's node is located in the SoC-specific .dtsi, where its
> > > intended use case is not yet known, and its node name cannot easily be
> > > overridden in the board .dts that specifies the use case.  Hence the
> > > node name must always be "spi" (and cannot be e.g. "serial-engine").
> > > Let's hope there is no other use case for MSIOF that requires using
> > > a different node name...
>
> Hmm...
>
> Now, MSIOF node has "spi@xxxx".
> SoC file indicates MSIOF-SPI as default, so it has below lines
>
>         --- SoC file ----
>         msiof1: spi@xxxx {
>                 ...
>                 #address-cells = <1>;
>                 #size-cells = <0>;
>                 ...
>         };
>
> These are not needed for MSIOF-I2S, so removes these
>
>         --- Board file ----
>         &msiof1 {
>                 ...
>                 /delete-property/#address-cells;
>                 /delete-property/#size-cells;
>                 ...
>         };
>
> Now, my dt-bindings doesn't load spi-controller.yaml (as sample), but I got
>
>         [SoC file]: Warning (spi_bus_bridge): /soc/spi@xxxx: incorrect #address-cells for SPI bus
>           also defined at [Board file]
>         [SoC file]: Warning (spi_bus_bridge): /soc/spi@xxxx: incorrect #size-cells for SPI bus
>           also defined at [Board file]
>
> MSIOF dt-bindings doesn't load spi-controller.yaml, but why I got "spi_bus_bridge"
> warning ?? I wonder dt compiler (?) automatically check "spi" node ?
> I have tryed some code, my expectation seems correct (In case of node name was "spi@xxx",
> I got many SPI related warnings even though I didn't load spi-controller).

These come from dtc, which makes its own assumptions:

    $ git grep spi_bus_bridge
    scripts/dtc/checks.c:static void check_spi_bus_bridge(struct check
*c, struct dt_info *dti, struct node *node)
    scripts/dtc/checks.c:WARNING(spi_bus_bridge, check_spi_bus_bridge,
NULL, &addr_size_cells);
    scripts/dtc/checks.c:WARNING(spi_bus_reg, check_spi_bus_reg, NULL,
&reg_format, &spi_bus_bridge);
    scripts/dtc/checks.c:   &spi_bus_bridge,

Perhaps we do need to extend the use of role-specifying properties
like "interrupt-controller" (in Device Tree Specification v0.4 and in
dt-schema) and the few others in Documentation/devicetree/bindings:

    gpio-controller
    mctp-controller
    msi-controller
    system-power-controller

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v3 01/10] dt-bindings: renesas,sh-msiof: Add MSIOF I2S Sound support

[Kuninori Morimoto]

Hi Geert, Rob, Mark

> >         [SoC file]: Warning (spi_bus_bridge): /soc/spi@xxxx: incorrect #address-cells for SPI bus
> >           also defined at [Board file]
> >         [SoC file]: Warning (spi_bus_bridge): /soc/spi@xxxx: incorrect #size-cells for SPI bus
> >           also defined at [Board file]
> >
> > MSIOF dt-bindings doesn't load spi-controller.yaml, but why I got "spi_bus_bridge"
> > warning ?? I wonder dt compiler (?) automatically check "spi" node ?
> > I have tryed some code, my expectation seems correct (In case of node name was "spi@xxx",
> > I got many SPI related warnings even though I didn't load spi-controller).
> 
> These come from dtc, which makes its own assumptions:
> 
>     $ git grep spi_bus_bridge
>     scripts/dtc/checks.c:static void check_spi_bus_bridge(struct check
> *c, struct dt_info *dti, struct node *node)
>     scripts/dtc/checks.c:WARNING(spi_bus_bridge, check_spi_bus_bridge,
> NULL, &addr_size_cells);
>     scripts/dtc/checks.c:WARNING(spi_bus_reg, check_spi_bus_reg, NULL,
> &reg_format, &spi_bus_bridge);
>     scripts/dtc/checks.c:   &spi_bus_bridge,
> 
> Perhaps we do need to extend the use of role-specifying properties
> like "interrupt-controller" (in Device Tree Specification v0.4 and in
> dt-schema) and the few others in Documentation/devicetree/bindings:
> 
>     gpio-controller
>     mctp-controller
>     msi-controller
>     system-power-controller

Hmm... but I'm not familiar with DT. Should I do it ??
Except from SPI warning, and focus to MSIOF-I2S, my patch itself is
not so bad, right ?
I will post v4 patch-set, with comment above.

Thank you for your help !!

Best regards
---
Kuninori Morimoto

Re: [PATCH v3 01/10] dt-bindings: renesas,sh-msiof: Add MSIOF I2S Sound support

[Geert Uytterhoeven]

Hi Morimoto-san,

On Fri, 18 Apr 2025 at 00:31, Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
> > >         [SoC file]: Warning (spi_bus_bridge): /soc/spi@xxxx: incorrect #address-cells for SPI bus
> > >           also defined at [Board file]
> > >         [SoC file]: Warning (spi_bus_bridge): /soc/spi@xxxx: incorrect #size-cells for SPI bus
> > >           also defined at [Board file]
> > >
> > > MSIOF dt-bindings doesn't load spi-controller.yaml, but why I got "spi_bus_bridge"
> > > warning ?? I wonder dt compiler (?) automatically check "spi" node ?
> > > I have tryed some code, my expectation seems correct (In case of node name was "spi@xxx",
> > > I got many SPI related warnings even though I didn't load spi-controller).
> >
> > These come from dtc, which makes its own assumptions:
> >
> >     $ git grep spi_bus_bridge
> >     scripts/dtc/checks.c:static void check_spi_bus_bridge(struct check
> > *c, struct dt_info *dti, struct node *node)
> >     scripts/dtc/checks.c:WARNING(spi_bus_bridge, check_spi_bus_bridge,
> > NULL, &addr_size_cells);
> >     scripts/dtc/checks.c:WARNING(spi_bus_reg, check_spi_bus_reg, NULL,
> > &reg_format, &spi_bus_bridge);
> >     scripts/dtc/checks.c:   &spi_bus_bridge,
> >
> > Perhaps we do need to extend the use of role-specifying properties
> > like "interrupt-controller" (in Device Tree Specification v0.4 and in
> > dt-schema) and the few others in Documentation/devicetree/bindings:
> >
> >     gpio-controller
> >     mctp-controller
> >     msi-controller
> >     system-power-controller
>
> Hmm... but I'm not familiar with DT. Should I do it ??

I do not think you should do that.
This is something to be decided by the DT and SPI maintainers.

> Except from SPI warning, and focus to MSIOF-I2S, my patch itself is
> not so bad, right ?
> I will post v4 patch-set, with comment above.

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds