[PATCH v2 0/2] ASoC: mediatek: Add support of SOF on Mediatek mt8196 SoC.

[PATCH v2 0/2] ASoC: mediatek: Add support of SOF on Mediatek mt8196 SoC.

[hailong.fan]

From: Hailong Fan <hailong.fan@mediatek.com>

Sorry for late reply.

Add support of MediaTek mt8196 SoC DSP to SOF.
This series is based on linux-next, tag: next-20250702.

---
Changes in v2:
- Rebase onto https://lore.kernel.org/all/20250320115300.137410-1-angelogioacchino.delregno@collabora.com/
- Link to v1: https://lore.kernel.org/lkml/20250320031753.13669-1-hailong.fan@mediatek.com/

This series patches dependent on:
[1]
https://lore.kernel.org/all/20250307032942.10447-1-guangjie.song@mediatek.com/
[2]
https://lore.kernel.org/all/20250307034454.12243-1-guangjie.song@mediatek.com/

Hailong Fan (2):
  dt-bindings: dsp: mediatek: add mt8196 dsp document
  ASoC: SOF: MediaTek: Add mt8196 hardware support

 .../bindings/sound/mediatek,mt8196-dsp.yaml   |  95 +++
 sound/soc/sof/mediatek/Kconfig                |   9 +
 sound/soc/sof/mediatek/Makefile               |   1 +
 sound/soc/sof/mediatek/mt8196/Makefile        |   3 +
 sound/soc/sof/mediatek/mt8196/mt8196-clk.c    |  98 +++
 sound/soc/sof/mediatek/mt8196/mt8196-clk.h    |  23 +
 sound/soc/sof/mediatek/mt8196/mt8196-loader.c |  60 ++
 sound/soc/sof/mediatek/mt8196/mt8196.c        | 556 ++++++++++++++++++
 sound/soc/sof/mediatek/mt8196/mt8196.h        | 124 ++++
 9 files changed, 969 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/mediatek,mt8196-dsp.yaml
 create mode 100644 sound/soc/sof/mediatek/mt8196/Makefile
 create mode 100644 sound/soc/sof/mediatek/mt8196/mt8196-clk.c
 create mode 100644 sound/soc/sof/mediatek/mt8196/mt8196-clk.h
 create mode 100644 sound/soc/sof/mediatek/mt8196/mt8196-loader.c
 create mode 100644 sound/soc/sof/mediatek/mt8196/mt8196.c
 create mode 100644 sound/soc/sof/mediatek/mt8196/mt8196.h

-- 
2.45.2

[PATCH v2 1/2] dt-bindings: dsp: mediatek: add mt8196 dsp document

[hailong.fan]

From: Hailong Fan <hailong.fan@mediatek.com>

This patch adds mt8196 dsp document. The dsp is used for Sound Open
Firmware driver node. It includes registers,  clocks, memory regions,
and mailbox for dsp.

Signed-off-by: Hailong Fan <hailong.fan@mediatek.com>
---
 .../bindings/sound/mediatek,mt8196-dsp.yaml   | 95 +++++++++++++++++++
 1 file changed, 95 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/mediatek,mt8196-dsp.yaml

diff --git a/Documentation/devicetree/bindings/sound/mediatek,mt8196-dsp.yaml b/Documentation/devicetree/bindings/sound/mediatek,mt8196-dsp.yaml
new file mode 100644
index 000000000000..68f594f476e8
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/mediatek,mt8196-dsp.yaml
@@ -0,0 +1,95 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/mediatek,mt8196-dsp.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek mt8196 DSP core
+
+maintainers:
+  - Hailong Fan <hailong.fan@mediatek.com>
+
+description: The MediaTek mt8196 SoC mt8196 contain a DSP core used for advanced
+  pre- and post- audio processing.
+
+properties:
+  compatible:
+    const: mediatek,mt8196-dsp
+
+  reg:
+    items:
+      - description: DSP configuration registers
+      - description: DSP SRAM
+      - description: DSP secure registers
+      - description: DSP bus registers
+
+  reg-names:
+    items:
+      - const: cfg
+      - const: sram
+      - const: sec
+      - const: bus
+
+  clocks:
+    items:
+      - description: mux for dsp clock
+      - description: 26M clock
+      - description: ADSP PLL clock
+
+  clock-names:
+    items:
+      - const: adsp_sel
+      - const: clk26m
+      - const: adsppll
+
+  power-domains:
+    maxItems: 1
+
+  mboxes:
+    items:
+      - description: mailbox for receiving audio DSP requests.
+      - description: mailbox for transmitting requests to audio DSP.
+
+  mbox-names:
+    items:
+      - const: rx
+      - const: tx
+
+  memory-region:
+    items:
+      - description: dma buffer between host and DSP.
+      - description: DSP system memory.
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - clocks
+  - clock-names
+  - power-domains
+  - mboxes
+  - mbox-names
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/mt8196-clk.h>
+    #include <dt-bindings/power/mt8196-power.h>
+    adsp@1a000000 {
+        compatible = "mediatek,mt8196-dsp";
+        reg = <0x1a000000 0x5000>,
+              <0x1a210000 0x80000>,
+              <0x1a345000 0x300>,
+              <0x1a00f000 0x1000>;
+        reg-names = "cfg", "sram", "sec", "bus";
+        clocks = <&cksys_clk CLK_CK_ADSP_SEL>,
+                 <&cksys_clk CLK_CK_TCK_26M_MX9>,
+                 <&cksys_clk CLK_CK_ADSPPLL>;
+        clock-names = "adsp_sel",
+                      "clk26m",
+                      "adsppll";
+        power-domains = <&scpsys MT8196_POWER_DOMAIN_ADSP_TOP_DORMANT>;
+        mboxes = <&adsp_mailbox0>, <&adsp_mailbox1>;
+        mbox-names = "rx", "tx";
+    };
-- 
2.45.2

[PATCH v2 2/2] ASoC: SOF: MediaTek: Add mt8196 hardware support

[hailong.fan]

From: Hailong Fan <hailong.fan@mediatek.com>

Add support of SOF on MediaTek mt8196 SoC.
MT8196 has a Cadence HiFi-5 DSP core that is responsible for running the
operating system and application programs. The IPC communication between
the AP and DSP is based on shared DRAM and mailbox interrupt.

Signed-off-by: Hailong Fan <hailong.fan@mediatek.com>
---
 sound/soc/sof/mediatek/Kconfig                |   9 +
 sound/soc/sof/mediatek/Makefile               |   1 +
 sound/soc/sof/mediatek/mt8196/Makefile        |   3 +
 sound/soc/sof/mediatek/mt8196/mt8196-clk.c    |  98 +++
 sound/soc/sof/mediatek/mt8196/mt8196-clk.h    |  23 +
 sound/soc/sof/mediatek/mt8196/mt8196-loader.c |  60 ++
 sound/soc/sof/mediatek/mt8196/mt8196.c        | 556 ++++++++++++++++++
 sound/soc/sof/mediatek/mt8196/mt8196.h        | 124 ++++
 8 files changed, 874 insertions(+)
 create mode 100644 sound/soc/sof/mediatek/mt8196/Makefile
 create mode 100644 sound/soc/sof/mediatek/mt8196/mt8196-clk.c
 create mode 100644 sound/soc/sof/mediatek/mt8196/mt8196-clk.h
 create mode 100644 sound/soc/sof/mediatek/mt8196/mt8196-loader.c
 create mode 100644 sound/soc/sof/mediatek/mt8196/mt8196.c
 create mode 100644 sound/soc/sof/mediatek/mt8196/mt8196.h

diff --git a/sound/soc/sof/mediatek/Kconfig b/sound/soc/sof/mediatek/Kconfig
index 4a2eddf6009a..36eb6f5c080b 100644
--- a/sound/soc/sof/mediatek/Kconfig
+++ b/sound/soc/sof/mediatek/Kconfig
@@ -42,4 +42,13 @@ config SND_SOC_SOF_MT8195
 	  Say Y if you have such a device.
 	  If unsure select "N".
 
+config SND_SOC_SOF_MT8196
+	tristate "SOF support for MT8196 audio DSP"
+	select SND_SOC_SOF_MTK_COMMON
+	depends on MTK_ADSP_IPC
+	help
+	  This adds support for Sound Open Firmware for MediaTek platforms
+	  using the mt8196 processors.
+	  Say Y if you have such a device.
+	  If unsure select "N".
 endif ## SND_SOC_SOF_MTK_TOPLEVEL
diff --git a/sound/soc/sof/mediatek/Makefile b/sound/soc/sof/mediatek/Makefile
index 29c5afb2f3d6..3e36d538679e 100644
--- a/sound/soc/sof/mediatek/Makefile
+++ b/sound/soc/sof/mediatek/Makefile
@@ -2,3 +2,4 @@
 obj-$(CONFIG_SND_SOC_SOF_MTK_COMMON) += mtk-adsp-common.o
 obj-$(CONFIG_SND_SOC_SOF_MT8195) += mt8195/
 obj-$(CONFIG_SND_SOC_SOF_MT8186) += mt8186/
+obj-$(CONFIG_SND_SOC_SOF_MT8196) += mt8196/
diff --git a/sound/soc/sof/mediatek/mt8196/Makefile b/sound/soc/sof/mediatek/mt8196/Makefile
new file mode 100644
index 000000000000..245ff4cb387c
--- /dev/null
+++ b/sound/soc/sof/mediatek/mt8196/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
+snd-sof-mt8196-objs := mt8196.o mt8196-clk.o mt8196-loader.o
+obj-$(CONFIG_SND_SOC_SOF_MT8196) += snd-sof-mt8196.o
diff --git a/sound/soc/sof/mediatek/mt8196/mt8196-clk.c b/sound/soc/sof/mediatek/mt8196/mt8196-clk.c
new file mode 100644
index 000000000000..8a80dec145f5
--- /dev/null
+++ b/sound/soc/sof/mediatek/mt8196/mt8196-clk.c
@@ -0,0 +1,98 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
+/*
+ * Copyright(c) 2025 MediaTek Inc. All rights reserved.
+ * Author: Hailong Fan <hailong.fan@mediatek.com>
+ */
+
+// Hardware interface for mt8196 DSP clock
+
+#include <linux/clk.h>
+#include <linux/io.h>
+
+#include "../../sof-audio.h"
+#include "../../ops.h"
+#include "../adsp_helper.h"
+#include "mt8196.h"
+#include "mt8196-clk.h"
+
+static const char *adsp_clks[ADSP_CLK_MAX] = {
+	[CLK_TOP_ADSP_SEL] = "clk_top_adsp_sel",
+	[CLK_TOP_CLK26M] = "clk_top_clk26m",
+	[CLK_TOP_ADSPPLL] = "clk_top_adsppll",
+
+};
+
+int mt8196_adsp_init_clock(struct snd_sof_dev *sdev)
+{
+	struct adsp_priv *priv = sdev->pdata->hw_pdata;
+	struct device *dev = sdev->dev;
+	int i;
+
+	priv->clk = devm_kcalloc(dev, ADSP_CLK_MAX, sizeof(*priv->clk), GFP_KERNEL);
+	if (!priv->clk)
+		return -ENOMEM;
+
+	for (i = 0; i < ADSP_CLK_MAX; i++) {
+		priv->clk[i] = devm_clk_get(dev, adsp_clks[i]);
+
+		if (IS_ERR(priv->clk[i]))
+			return PTR_ERR(priv->clk[i]);
+	}
+
+	return 0;
+}
+
+static int adsp_enable_all_clock(struct snd_sof_dev *sdev)
+{
+	struct adsp_priv *priv = sdev->pdata->hw_pdata;
+	struct device *dev = sdev->dev;
+	int ret;
+
+	ret = clk_prepare_enable(priv->clk[CLK_TOP_ADSP_SEL]);
+	if (ret) {
+		dev_err(dev, "%s clk_prepare_enable(audiodsp) fail %d\n",
+			__func__, ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static void adsp_disable_all_clock(struct snd_sof_dev *sdev)
+{
+	struct adsp_priv *priv = sdev->pdata->hw_pdata;
+
+	clk_disable_unprepare(priv->clk[CLK_TOP_ADSP_SEL]);
+}
+
+int mt8196_adsp_clock_on(struct snd_sof_dev *sdev)
+{
+	struct device *dev = sdev->dev;
+	int ret;
+
+	ret = adsp_enable_all_clock(sdev);
+	if (ret) {
+		dev_err(dev, "failed to adsp_enable_clock: %d\n", ret);
+		return ret;
+	}
+
+	ret = snd_sof_dsp_update_bits(sdev, DSP_REG_BAR, ADSP_CK_EN,
+				      UART_BT_EN | DMA_AXI_EN | DMA1_EN,
+				      UART_BT_EN | DMA_AXI_EN | DMA1_EN);
+	if (ret < 0)
+		dev_err(sdev->dev, "Failed to update CK register: %d\n", ret);
+
+	ret = snd_sof_dsp_update_bits(sdev, DSP_REG_BAR, ADSP_UART_CTRL,
+				      UART_BCLK_CG | UART_RSTN,
+				      UART_BCLK_CG | UART_RSTN);
+	if (ret < 0)
+		dev_err(sdev->dev, "Failed to update UART register: %d\n", ret);
+
+	return 0;
+}
+
+void mt8196_adsp_clock_off(struct snd_sof_dev *sdev)
+{
+	snd_sof_dsp_write(sdev, DSP_REG_BAR, ADSP_UART_CTRL, 0);
+	adsp_disable_all_clock(sdev);
+}
diff --git a/sound/soc/sof/mediatek/mt8196/mt8196-clk.h b/sound/soc/sof/mediatek/mt8196/mt8196-clk.h
new file mode 100644
index 000000000000..e9f050b74a42
--- /dev/null
+++ b/sound/soc/sof/mediatek/mt8196/mt8196-clk.h
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause) */
+/*
+ * Copyright(c) 2025 MediaTek Inc. All rights reserved.
+ * Author: Hailong Fan <hailong.fan@mediatek.com>
+ */
+
+#ifndef __MT8196_CLK_H
+#define __MT8196_CLK_H
+
+struct snd_sof_dev;
+
+/* DSP clock */
+enum adsp_clk_id {
+	CLK_TOP_ADSP_SEL,
+	CLK_TOP_CLK26M,
+	CLK_TOP_ADSPPLL,
+	ADSP_CLK_MAX
+};
+
+int mt8196_adsp_init_clock(struct snd_sof_dev *sdev);
+int mt8196_adsp_clock_on(struct snd_sof_dev *sdev);
+void mt8196_adsp_clock_off(struct snd_sof_dev *sdev);
+#endif
diff --git a/sound/soc/sof/mediatek/mt8196/mt8196-loader.c b/sound/soc/sof/mediatek/mt8196/mt8196-loader.c
new file mode 100644
index 000000000000..7063221d7753
--- /dev/null
+++ b/sound/soc/sof/mediatek/mt8196/mt8196-loader.c
@@ -0,0 +1,60 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
+/*
+ * Copyright(c) 2025 MediaTek Inc. All rights reserved.
+ * Author: Hailong Fan <hailong.fan@mediatek.com>
+ */
+
+// Hardware interface for mt8196 DSP code loader
+
+#include <sound/sof.h>
+#include "mt8196.h"
+#include "../../ops.h"
+
+void mt8196_sof_hifixdsp_boot_sequence(struct snd_sof_dev *sdev, u32 boot_addr)
+{
+	/* step1. clr ADSP_ALTVEC_C0 */
+	snd_sof_dsp_write(sdev, DSP_SECREG_BAR, ADSP_ALTVEC_C0, 0);
+	snd_sof_dsp_write(sdev, DSP_SECREG_BAR, ADSP_ALTVECSEL, 0);
+
+	/* step2. set core boot address */
+	snd_sof_dsp_write(sdev, DSP_SECREG_BAR, ADSP_ALTVEC_C0, boot_addr);
+	snd_sof_dsp_write(sdev, DSP_SECREG_BAR, ADSP_ALTVECSEL, ADSP_ALTVECSEL_C0);
+
+	/* step3. set RUNSTALL to stop core */
+	snd_sof_dsp_update_bits(sdev, DSP_REG_BAR, ADSP_HIFI_RUNSTALL,
+				RUNSTALL, RUNSTALL);
+
+	/* enable mbox 0 & 1 IRQ */
+	snd_sof_dsp_update_bits(sdev, DSP_REG_BAR, ADSP_MBOX_IRQ_EN,
+				DSP_MBOX0_IRQ_EN | DSP_MBOX1_IRQ_EN,
+				DSP_MBOX0_IRQ_EN | DSP_MBOX1_IRQ_EN);
+
+	/* step4. assert core reset */
+	snd_sof_dsp_update_bits(sdev, DSP_REG_BAR, ADSP_CFGREG_SW_RSTN,
+				SW_RSTN_C0 | SW_DBG_RSTN_C0,
+				SW_RSTN_C0 | SW_DBG_RSTN_C0);
+
+	/* hardware requirement */
+	udelay(1);
+
+	/* step5. release core reset */
+	snd_sof_dsp_update_bits(sdev, DSP_REG_BAR, ADSP_CFGREG_SW_RSTN,
+				SW_RSTN_C0 | SW_DBG_RSTN_C0,
+				0);
+
+	/* step6. clear RUNSTALL to start core */
+	snd_sof_dsp_update_bits(sdev, DSP_REG_BAR, ADSP_HIFI_RUNSTALL,
+				RUNSTALL, 0);
+}
+
+void mt8196_sof_hifixdsp_shutdown(struct snd_sof_dev *sdev)
+{
+	/* set RUNSTALL to stop core */
+	snd_sof_dsp_update_bits(sdev, DSP_REG_BAR, ADSP_HIFI_RUNSTALL,
+				RUNSTALL, RUNSTALL);
+
+	/* assert core reset */
+	snd_sof_dsp_update_bits(sdev, DSP_REG_BAR, ADSP_CFGREG_SW_RSTN,
+				SW_RSTN_C0 | SW_DBG_RSTN_C0,
+				SW_RSTN_C0 | SW_DBG_RSTN_C0);
+}
diff --git a/sound/soc/sof/mediatek/mt8196/mt8196.c b/sound/soc/sof/mediatek/mt8196/mt8196.c
new file mode 100644
index 000000000000..ba6c969cbaa7
--- /dev/null
+++ b/sound/soc/sof/mediatek/mt8196/mt8196.c
@@ -0,0 +1,556 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
+/*
+ * Copyright(c) 2025 MediaTek Inc. All rights reserved.
+ * Author: Hailong Fan <hailong.fan@mediatek.com>
+ */
+
+/*
+ * Hardware interface for audio DSP on mt8196
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/firmware.h>
+#include <linux/io.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/of_platform.h>
+#include <linux/of_reserved_mem.h>
+#include <linux/module.h>
+
+#include <sound/sof.h>
+#include <sound/sof/xtensa.h>
+#include "../../ops.h"
+#include "../../sof-of-dev.h"
+#include "../adsp_helper.h"
+#include "../mtk-adsp-common.h"
+#include "mt8196.h"
+#include "mt8196-clk.h"
+
+static void sof_reg_write_bits(struct snd_sof_dev *sdev, u32 bar, u32 offset, u32 value,
+			       u8 bits, u8 len)
+{
+	u32 mask, val;
+	int ret;
+
+	mask = GENMASK(len + bits - 1, bits);
+	val = (value << bits) & mask;
+
+	ret = snd_sof_dsp_update_bits(sdev, bar, offset, mask, val);
+	if (ret < 0)
+		dev_err(sdev->dev, "Failed to update DSP register: %d\n", ret);
+}
+
+static u32 reg_read_bits(struct snd_sof_dev *sdev, u32 bar, u32 offset, u8 bits, u8 len)
+{
+	u32 target_bit_field, curr_value;
+
+	target_bit_field = GENMASK(len + bits - 1, bits);
+	curr_value = snd_sof_dsp_read(sdev, bar, offset);
+
+	return (curr_value & target_bit_field) >> bits;
+}
+
+static int mt8196_get_mailbox_offset(struct snd_sof_dev *sdev)
+{
+	return MBOX_OFFSET;
+}
+
+static int mt8196_get_window_offset(struct snd_sof_dev *sdev, u32 id)
+{
+	return MBOX_OFFSET;
+}
+
+static const struct mtk_adsp_ipc_ops dsp_ops = {
+	.handle_reply		= mtk_adsp_handle_reply,
+	.handle_request		= mtk_adsp_handle_request,
+};
+
+static int platform_parse_resource(struct platform_device *pdev, void *data)
+{
+	struct resource *mmio;
+	struct resource res;
+	struct device_node *mem_region;
+	struct device *dev = &pdev->dev;
+	struct mtk_adsp_chip_info *adsp = data;
+	int ret;
+
+	ret = of_reserved_mem_device_init(dev);
+	if (ret) {
+		dev_err(dev, "of_reserved_mem_device_init failed\n");
+		return ret;
+	}
+
+	mem_region = of_parse_phandle(dev->of_node, "memory-region", 1);
+	if (!mem_region) {
+		dev_err(dev, "no memory-region sysmem phandle\n");
+		return -ENODEV;
+	}
+
+	ret = of_address_to_resource(mem_region, 0, &res);
+	of_node_put(mem_region);
+	if (ret) {
+		dev_err(dev, "of_address_to_resource sysmem failed\n");
+		return ret;
+	}
+
+	adsp->pa_dram = (phys_addr_t)res.start;
+	if (adsp->pa_dram & DRAM_REMAP_MASK) {
+		dev_err(dev, "adsp memory(%#x) is not 4K-aligned\n",
+			(u32)adsp->pa_dram);
+		return -EINVAL;
+	}
+
+	adsp->dramsize = resource_size(&res);
+	if (adsp->dramsize < TOTAL_SIZE_SHARED_DRAM_FROM_TAIL) {
+		dev_err(dev, "adsp memory(%#x) is not enough for share\n",
+			adsp->dramsize);
+		return -EINVAL;
+	}
+
+	dev_dbg(dev, "dram pbase=%pa size=%#x\n", &adsp->pa_dram, adsp->dramsize);
+
+	mmio = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cfg");
+	if (!mmio) {
+		dev_err(dev, "no ADSP-CFG register resource\n");
+		return -ENXIO;
+	}
+
+	adsp->va_cfgreg = devm_ioremap_resource(dev, mmio);
+	if (IS_ERR(adsp->va_cfgreg))
+		return PTR_ERR(adsp->va_cfgreg);
+
+	adsp->pa_cfgreg = (phys_addr_t)mmio->start;
+	adsp->cfgregsize = resource_size(mmio);
+
+	mmio = platform_get_resource_byname(pdev, IORESOURCE_MEM, "sram");
+	if (!mmio) {
+		dev_err(dev, "no SRAM resource\n");
+		return -ENXIO;
+	}
+
+	adsp->pa_sram = (phys_addr_t)mmio->start;
+	adsp->sramsize = resource_size(mmio);
+
+	dev_dbg(dev, "sram pbase=%pa size=%#x\n", &adsp->pa_sram, adsp->sramsize);
+
+	mmio = platform_get_resource_byname(pdev, IORESOURCE_MEM, "sec");
+	if (!mmio) {
+		dev_err(dev, "no SEC register resource\n");
+		return -ENXIO;
+	}
+
+	adsp->va_secreg = devm_ioremap_resource(dev, mmio);
+	if (IS_ERR(adsp->va_secreg))
+		return PTR_ERR(adsp->va_secreg);
+
+	adsp->pa_secreg = (phys_addr_t)mmio->start;
+	adsp->secregsize = resource_size(mmio);
+
+	dev_dbg(dev, "secreg pbase=%pa size=%#x\n", &adsp->pa_secreg, adsp->secregsize);
+
+	mmio = platform_get_resource_byname(pdev, IORESOURCE_MEM, "bus");
+	if (!mmio) {
+		dev_err(dev, "no BUS register resource\n");
+		return -ENXIO;
+	}
+
+	adsp->va_busreg = devm_ioremap_resource(dev, mmio);
+	if (IS_ERR(adsp->va_busreg))
+		return PTR_ERR(adsp->va_busreg);
+
+	adsp->pa_busreg = (phys_addr_t)mmio->start;
+	adsp->busregsize = resource_size(mmio);
+
+	dev_dbg(dev, "busreg pbase=%pa, vbase=%pa, size=%#x\n", &adsp->pa_busreg,
+		&adsp->va_busreg, adsp->busregsize);
+
+	return 0;
+}
+
+static void adsp_sram_power_on(struct snd_sof_dev *sdev)
+{
+	int i;
+
+	for (i = 0; i < ADSP_SRAM_CHANNEL_NUM; i++) {
+		sof_reg_write_bits(sdev, DSP_BUSREG_BAR, ADSP_SRAM_POOL_CON, 0, i, 1);
+		while (1) {
+			if (reg_read_bits(sdev, DSP_BUSREG_BAR, ADSP_SRAM_POOL_ACK, i, 1) == 0)
+				break;
+		}
+	}
+}
+
+static void adsp_sram_power_off(struct snd_sof_dev *sdev)
+{
+	int i;
+
+	for (i = 0; i < ADSP_SRAM_CHANNEL_NUM; i++) {
+		sof_reg_write_bits(sdev, DSP_BUSREG_BAR, ADSP_SRAM_POOL_CON, 1, i, 1);
+		while (1) {
+			if (reg_read_bits(sdev, DSP_BUSREG_BAR, ADSP_SRAM_POOL_ACK, i, 1) == 1)
+				break;
+		}
+	}
+}
+
+/*  Init the basic DSP DRAM address */
+static int adsp_memory_remap_init(struct snd_sof_dev *sdev, struct mtk_adsp_chip_info *adsp)
+{
+	u32 from, to;
+
+	from = adsp_remap_region_from(DRAM_PHYS_BASE_FROM_DSP_VIEW, DSP_DRAM_SIZE);
+	to = (u32)ADSP_REMAP_REGION_TO(adsp->pa_dram);
+
+	snd_sof_dsp_write(sdev, DSP_BUSREG_BAR, AUDIO_BUS_CFG_RSV_10, from);
+	snd_sof_dsp_write(sdev, DSP_BUSREG_BAR, DSP_C0_EMI_MAP_ADDR, to);
+
+	snd_sof_dsp_update_bits(sdev, DSP_BUSREG_BAR, AUDIO_BUS_CFG_BUS_REMAP_CTRL, 1, 1);
+
+	return 0;
+}
+
+static int mt8196_run(struct snd_sof_dev *sdev)
+{
+	u32 adsp_bootup_addr;
+
+	adsp_bootup_addr = SRAM_PHYS_BASE_FROM_DSP_VIEW;
+	dev_dbg(sdev->dev, "HIFIxDSP boot from base : 0x%08X\n", adsp_bootup_addr);
+	mt8196_sof_hifixdsp_boot_sequence(sdev, adsp_bootup_addr);
+
+	return 0;
+}
+
+static int mt8196_dsp_probe(struct snd_sof_dev *sdev)
+{
+	struct platform_device *pdev = to_platform_device(sdev->dev);
+	struct adsp_priv *priv;
+	int ret;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	sdev->pdata->hw_pdata = priv;
+	priv->dev = sdev->dev;
+	priv->sdev = sdev;
+
+	priv->adsp = devm_kzalloc(&pdev->dev, sizeof(struct mtk_adsp_chip_info), GFP_KERNEL);
+	if (!priv->adsp)
+		return -ENOMEM;
+
+	ret = platform_parse_resource(pdev, priv->adsp);
+	if (ret)
+		return ret;
+
+	sdev->bar[SOF_FW_BLK_TYPE_IRAM] = devm_ioremap(sdev->dev,
+						       priv->adsp->pa_sram,
+						       priv->adsp->sramsize);
+	if (!sdev->bar[SOF_FW_BLK_TYPE_IRAM]) {
+		dev_err(sdev->dev, "failed to ioremap base %pa size %#x\n",
+			&priv->adsp->pa_sram, priv->adsp->sramsize);
+		return -ENOMEM;
+	}
+
+	priv->adsp->va_sram = sdev->bar[SOF_FW_BLK_TYPE_IRAM];
+	dev_dbg(sdev->dev, "pa_sram %pa,va: %pa, size %#x\n",
+		&priv->adsp->pa_sram, &priv->adsp->va_sram, priv->adsp->sramsize);
+
+	sdev->bar[SOF_FW_BLK_TYPE_SRAM] = devm_ioremap(sdev->dev,
+						       priv->adsp->pa_dram,
+						       priv->adsp->dramsize);
+
+	if (!sdev->bar[SOF_FW_BLK_TYPE_SRAM]) {
+		dev_err(sdev->dev, "failed to ioremap base %pa size %#x\n",
+			&priv->adsp->pa_dram, priv->adsp->dramsize);
+		return -ENOMEM;
+	}
+
+	priv->adsp->va_dram = sdev->bar[SOF_FW_BLK_TYPE_SRAM];
+
+	dev_dbg(sdev->dev, "pa_dram %pa, va: %pa, size %#x\n",
+		&priv->adsp->pa_dram, &priv->adsp->va_dram, priv->adsp->dramsize);
+
+	sdev->bar[DSP_REG_BAR] = priv->adsp->va_cfgreg;
+	sdev->bar[DSP_SECREG_BAR] = priv->adsp->va_secreg;
+	sdev->bar[DSP_BUSREG_BAR] = priv->adsp->va_busreg;
+
+	sdev->mmio_bar = SOF_FW_BLK_TYPE_SRAM;
+	sdev->mailbox_bar = SOF_FW_BLK_TYPE_SRAM;
+
+	/* set default mailbox offset for FW ready message */
+	sdev->dsp_box.offset = mt8196_get_mailbox_offset(sdev);
+
+	/* enable adsp clock before touching registers */
+	ret = mt8196_adsp_init_clock(sdev);
+	if (ret) {
+		dev_err(sdev->dev, "mt8196_adsp_init_clock failed\n");
+		return ret;
+	}
+
+	ret = mt8196_adsp_clock_on(sdev);
+	if (ret) {
+		dev_err(sdev->dev, "mt8196_adsp_clock_on fail!\n");
+		return ret;
+	}
+
+	ret = adsp_memory_remap_init(sdev, priv->adsp);
+	if (ret) {
+		dev_err(sdev->dev, "adsp_memory_remap_init fail!\n");
+		return ret;
+	}
+
+	adsp_sram_power_on(sdev);
+
+	priv->ipc_dev = platform_device_register_data(&pdev->dev, "mtk-adsp-ipc",
+						      PLATFORM_DEVID_NONE,
+						      pdev, sizeof(*pdev));
+	if (IS_ERR(priv->ipc_dev)) {
+		ret = PTR_ERR(priv->ipc_dev);
+		dev_err(sdev->dev, "failed to create mtk-adsp-ipc device\n");
+		goto err_adsp_off;
+	}
+
+	priv->dsp_ipc = dev_get_drvdata(&priv->ipc_dev->dev);
+	if (!priv->dsp_ipc) {
+		ret = -EPROBE_DEFER;
+		dev_err(sdev->dev, "failed to get drvdata\n");
+		goto exit_pdev_unregister;
+	}
+
+	mtk_adsp_ipc_set_data(priv->dsp_ipc, priv);
+	priv->dsp_ipc->ops = &dsp_ops;
+
+	return 0;
+
+exit_pdev_unregister:
+	platform_device_unregister(priv->ipc_dev);
+err_adsp_off:
+	adsp_sram_power_off(sdev);
+	mt8196_adsp_clock_off(sdev);
+
+	return ret;
+}
+
+static void mt8196_dsp_remove(struct snd_sof_dev *sdev)
+{
+	struct adsp_priv *priv = sdev->pdata->hw_pdata;
+
+	platform_device_unregister(priv->ipc_dev);
+	mt8196_sof_hifixdsp_shutdown(sdev);
+	adsp_sram_power_off(sdev);
+	mt8196_adsp_clock_off(sdev);
+}
+
+static int mt8196_dsp_shutdown(struct snd_sof_dev *sdev)
+{
+	return snd_sof_suspend(sdev->dev);
+}
+
+static int mt8196_dsp_suspend(struct snd_sof_dev *sdev, u32 target_state)
+{
+	struct adsp_priv *priv = sdev->pdata->hw_pdata;
+
+	snd_sof_dsp_update_bits(sdev, DSP_REG_BAR, ADSP_IO_CONFIG, ADSP_CLK_SEL, 0);
+	clk_set_parent(priv->clk[CLK_TOP_ADSP_SEL], priv->clk[CLK_TOP_CLK26M]);
+	mt8196_sof_hifixdsp_shutdown(sdev);
+	adsp_sram_power_off(sdev);
+	mt8196_adsp_clock_off(sdev);
+
+	return 0;
+}
+
+static int mt8196_dsp_resume(struct snd_sof_dev *sdev)
+{
+	int ret;
+	struct adsp_priv *priv = sdev->pdata->hw_pdata;
+
+	ret = mt8196_adsp_clock_on(sdev);
+	if (ret) {
+		dev_err(sdev->dev, "mt8196_adsp_clock_on fail!");
+		return ret;
+	}
+
+	clk_set_parent(priv->clk[CLK_TOP_ADSP_SEL], priv->clk[CLK_TOP_ADSPPLL]);
+	snd_sof_dsp_update_bits(sdev, DSP_REG_BAR, ADSP_IO_CONFIG, ADSP_CLK_SEL, BIT(31));
+	adsp_sram_power_on(sdev);
+
+	return 0;
+}
+
+static void mt8196_adsp_dump(struct snd_sof_dev *sdev, u32 flags)
+{
+	u32 dbg_pc, dbg_data, dbg_inst, dbg_ls0stat, dbg_status, faultinfo;
+
+	/* dump debug registers */
+	dbg_pc = snd_sof_dsp_read(sdev, DSP_REG_BAR, DSP_PDEBUGPC);
+	dbg_data = snd_sof_dsp_read(sdev, DSP_REG_BAR, DSP_PDEBUGDATA);
+	dbg_inst = snd_sof_dsp_read(sdev, DSP_REG_BAR, DSP_PDEBUGINST);
+	dbg_ls0stat = snd_sof_dsp_read(sdev, DSP_REG_BAR, DSP_PDEBUGLS0STAT);
+	dbg_status = snd_sof_dsp_read(sdev, DSP_REG_BAR, DSP_PDEBUGSTATUS);
+	faultinfo = snd_sof_dsp_read(sdev, DSP_REG_BAR, DSP_PFAULTINFO);
+
+	dev_info(sdev->dev, "adsp dump : pc %#x, data %#x, dbg_inst %#x,",
+		 dbg_pc, dbg_data, dbg_inst);
+	dev_info(sdev->dev, "ls0stat %#x, status %#x, faultinfo %#x",
+		 dbg_ls0stat, dbg_status, faultinfo);
+
+	mtk_adsp_dump(sdev, flags);
+}
+
+/*
+ * DL_24CH, DL1, UL0, UL1, UL2 are registered as SOF FE, so creating the corresponding
+ * SOF BE to complete the pipeline.
+ */
+static struct snd_soc_dai_driver mt8196_dai[] = {
+{
+	.name = "SOF_DL_24CH",
+	.playback = {
+		.channels_min = CHAN_MIN,
+		.channels_max = CHAN_MAX,
+	},
+},
+{
+	.name = "SOF_DL1",
+	.playback = {
+		.channels_min = CHAN_MIN,
+		.channels_max = CHAN_MAX,
+	},
+},
+{
+	.name = "SOF_UL0",
+	.capture = {
+		.channels_min = 1,
+		.channels_max = 2,
+	},
+},
+{
+	.name = "SOF_UL1",
+	.capture = {
+		.channels_min = CHAN_MIN,
+		.channels_max = CHAN_MAX,
+	},
+},
+{
+	.name = "SOF_UL2",
+	.capture = {
+		.channels_min = CHAN_MIN,
+		.channels_max = CHAN_MAX,
+	},
+},
+};
+
+/* mt8196 ops */
+static const struct snd_sof_dsp_ops sof_mt8196_ops = {
+	/* probe and remove */
+	.probe		= mt8196_dsp_probe,
+	.remove		= mt8196_dsp_remove,
+	.shutdown	= mt8196_dsp_shutdown,
+
+	/* DSP core boot */
+	.run		= mt8196_run,
+
+	/* Block IO */
+	.block_read	= sof_block_read,
+	.block_write	= sof_block_write,
+
+	/* Mailbox IO */
+	.mailbox_read	= sof_mailbox_read,
+	.mailbox_write	= sof_mailbox_write,
+
+	/* Register IO */
+	.write		= sof_io_write,
+	.read		= sof_io_read,
+	.write64	= sof_io_write64,
+	.read64		= sof_io_read64,
+
+	/* ipc */
+	.send_msg		= mtk_adsp_send_msg,
+	.get_mailbox_offset	= mt8196_get_mailbox_offset,
+	.get_window_offset	= mt8196_get_window_offset,
+	.ipc_msg_data		= sof_ipc_msg_data,
+	.set_stream_data_offset = sof_set_stream_data_offset,
+
+	/* misc */
+	.get_bar_index	= mtk_adsp_get_bar_index,
+
+	/* stream callbacks */
+	.pcm_open	= sof_stream_pcm_open,
+	.pcm_hw_params	= mtk_adsp_stream_pcm_hw_params,
+	.pcm_pointer	= mtk_adsp_stream_pcm_pointer,
+	.pcm_close	= sof_stream_pcm_close,
+
+	/* firmware loading */
+	.load_firmware	= snd_sof_load_firmware_memcpy,
+
+	/* Firmware ops */
+	.dsp_arch_ops = &sof_xtensa_arch_ops,
+
+	/* DAI drivers */
+	.drv		= mt8196_dai,
+	.num_drv	= ARRAY_SIZE(mt8196_dai),
+
+	/* Debug information */
+	.dbg_dump = mt8196_adsp_dump,
+	.debugfs_add_region_item = snd_sof_debugfs_add_region_item_iomem,
+
+	/* PM */
+	.suspend	= mt8196_dsp_suspend,
+	.resume		= mt8196_dsp_resume,
+
+	/* ALSA HW info flags */
+	.hw_info =	SNDRV_PCM_INFO_MMAP |
+			SNDRV_PCM_INFO_MMAP_VALID |
+			SNDRV_PCM_INFO_INTERLEAVED |
+			SNDRV_PCM_INFO_PAUSE |
+			SNDRV_PCM_INFO_NO_PERIOD_WAKEUP,
+};
+
+static struct snd_sof_of_mach sof_mt8196_machs[] = {
+	{
+		.compatible = "mediatek,mt8196",
+		.sof_tplg_filename = "sof-mt8196.tplg",
+	},
+	{}
+};
+
+static const struct sof_dev_desc sof_of_mt8196_desc = {
+	.of_machines = sof_mt8196_machs,
+	.ipc_supported_mask	= BIT(SOF_IPC_TYPE_3),
+	.ipc_default		= SOF_IPC_TYPE_3,
+	.default_fw_path = {
+		[SOF_IPC_TYPE_3] = "mediatek/sof",
+	},
+	.default_tplg_path = {
+		[SOF_IPC_TYPE_3] = "mediatek/sof-tplg",
+	},
+	.default_fw_filename = {
+		[SOF_IPC_TYPE_3] = "sof-mt8196.ri",
+	},
+	.nocodec_tplg_filename = "sof-mt8196-nocodec.tplg",
+	.ops = &sof_mt8196_ops,
+};
+
+static const struct of_device_id sof_of_mt8196_ids[] = {
+	{ .compatible = "mediatek,mt8196-dsp", .data = &sof_of_mt8196_desc},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, sof_of_mt8196_ids);
+
+/* DT driver definition */
+static struct platform_driver snd_sof_of_mt8196_driver = {
+	.probe = sof_of_probe,
+	.remove = sof_of_remove,
+	.shutdown = sof_of_shutdown,
+	.driver = {
+	.name = "sof-audio-of-mt8196",
+		.pm = pm_ptr(&sof_of_pm),
+		.of_match_table = sof_of_mt8196_ids,
+	},
+};
+module_platform_driver(snd_sof_of_mt8196_driver);
+
+MODULE_LICENSE("Dual BSD/GPL");
+MODULE_DESCRIPTION("SOF support for MT8196 platforms");
+MODULE_IMPORT_NS("SND_SOC_SOF_XTENSA");
+MODULE_IMPORT_NS("SND_SOC_SOF_MTK_COMMON");
diff --git a/sound/soc/sof/mediatek/mt8196/mt8196.h b/sound/soc/sof/mediatek/mt8196/mt8196.h
new file mode 100644
index 000000000000..5d2e14827bc2
--- /dev/null
+++ b/sound/soc/sof/mediatek/mt8196/mt8196.h
@@ -0,0 +1,124 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause) */
+/*
+ * Copyright(c) 2025 MediaTek Inc. All rights reserved.
+ * Author: Hailong Fan <hailong.fan@mediatek.com>
+ */
+
+#ifndef __MT8196_H
+#define __MT8196_H
+
+struct mtk_adsp_chip_info;
+struct snd_sof_dev;
+
+#define DSP_REG_BAR			4
+#define DSP_SECREG_BAR			5
+#define DSP_BUSREG_BAR			6
+#define CHAN_MIN			1
+#define CHAN_MAX			2
+
+/*****************************************************************************
+ *                  R E G I S T E R       TABLE
+ *****************************************************************************/
+/* dsp cfg */
+#define ADSP_CFGREG_SW_RSTN		0x0000
+#define SW_DBG_RSTN_C0			BIT(0)
+#define SW_RSTN_C0			BIT(4)
+#define ADSP_IO_CONFIG			0x000C
+#define ADSP_CLK_SEL			BIT(31)
+#define ADSP_HIFI_RUNSTALL		0x0108
+#define TRACEMEMREADY			BIT(15)
+#define RUNSTALL			BIT(12)
+#define ADSP_IRQ_MASK			0x0030
+#define ADSP_DVFSRC_REQ			0x0040
+#define ADSP_DDREN_REQ_0		0x0044
+#define ADSP_SEMAPHORE			0x0064
+#define ADSP_WDT_CON_C0			0x007C
+#define ADSP_MBOX_IRQ_EN		0x009C
+#define DSP_MBOX0_IRQ_EN		BIT(0)
+#define DSP_MBOX1_IRQ_EN		BIT(1)
+#define DSP_MBOX2_IRQ_EN		BIT(2)
+#define DSP_MBOX3_IRQ_EN		BIT(3)
+#define DSP_MBOX4_IRQ_EN		BIT(4)
+#define DSP_PDEBUGPC			0x013C
+#define DSP_PDEBUGDATA			0x0140
+#define DSP_PDEBUGINST			0x0144
+#define DSP_PDEBUGLS0STAT		0x0148
+#define DSP_PDEBUGSTATUS		0x014C
+#define DSP_PFAULTINFO			0x0150
+#define ADSP_CK_EN			0x1000
+#define DMA1_EN				BIT(12)
+#define DMA_AXI_EN			BIT(13)
+#define UART_BT_EN			BIT(14)
+#define DMA_EN				BIT(4)
+#define UART_EN				BIT(5)
+#define ADSP_UART_CTRL			0x1010
+#define UART_BCLK_CG			BIT(0)
+#define UART_RSTN			BIT(3)
+
+/* dsp sec */
+#define ADSP_PRID			0x0
+#define ADSP_ALTVEC_C0			0x04
+#define ADSP_ALTVECSEL			0x0C
+#define MT8196_ADSP_ALTVECSEL_C0	1
+
+#define ADSP_ALTVECSEL_C0		MT8196_ADSP_ALTVECSEL_C0
+
+/* dsp bus */
+#define AUDIO_BUS_CFG_RSV_10		0x30
+#define ADSP_SRAM_POOL_CON		0x190
+#define ADSP_SRAM_POOL_ACK		0x1A0
+#define ADSP_SRAM_CHANNEL_NUM		6
+#define DSP_SRAM_POOL_PD_MASK		0xF00F /* [0:3] and [12:15] */
+#define DSP_C0_EMI_MAP_ADDR		0xA00  /* ADSP Core0 To EMI Address Remap */
+#define DSP_C0_DMAEMI_MAP_ADDR		0xA08  /* DMA0 To EMI Address Remap */
+#define AUDIO_BUS_CFG_BUS_REMAP_CTRL	0x016C
+#define AUDIO_BUS_DSP2EMI_REMAP0	0x0A00
+#define AUDIO_BUS_DSP2EMI_REMAP1	0x0A04
+#define AUDIO_BUS_DMA2EMI_REMAP0	0x0A08
+#define AUDIO_BUS_DMA2EMI_REMAP1	0x0A0C
+
+/* DSP memories */
+#define MBOX_OFFSET			0x500000 /* DRAM */
+#define MBOX_SIZE			0x1000   /* consistent with which in memory.h of sof fw */
+#define DSP_DRAM_SIZE			0x900000 /* 9M */
+
+/*remap dram between AP and DSP view, 4KB aligned*/
+#define SRAM_PHYS_BASE_FROM_DSP_VIEW	0x4e100000 /* MT8196 DSP view */
+#define DRAM_PHYS_BASE_FROM_DSP_VIEW	0x90000000 /* MT8196 DSP view */
+#define DRAM_REMAP_SHIFT		12
+#define DRAM_REMAP_MASK			0xFFF
+
+/* remap */
+#define ADSP_REMAP_REGION
+#define ADSP_EMI_REMAP_SET_TOTAL	(4)
+#define EMI_REMAP_REGION		(AUDIO_BUS_CFG_RSV_10)
+#define EMI_REMAP_REPLACE		(AUDIO_BUS_DSP2EMI_REMAP0)
+#define EMI_REMAP_EN			(1U)
+#define ADSP_REMAP_REGION_TO(start)	(((start) >> 25) << 9)
+#define UPPER_16_SHIFT			16
+#define LOWER_16_MASK			0xFFFF
+#define UPPER_16_MASK			0xFFFF0000
+#define REMAP_ALIGN_SIZE		0x10000
+
+#define SIZE_SHARED_DRAM_DL		0x40000 /*Shared buffer for Downlink*/
+#define SIZE_SHARED_DRAM_UL		0x40000 /*Shared buffer for Uplink*/
+#define TOTAL_SIZE_SHARED_DRAM_FROM_TAIL	(SIZE_SHARED_DRAM_DL + SIZE_SHARED_DRAM_UL)
+
+void mt8196_sof_hifixdsp_boot_sequence(struct snd_sof_dev *sdev, u32 boot_addr);
+void mt8196_sof_hifixdsp_shutdown(struct snd_sof_dev *sdev);
+
+static inline u32 mix_begin_and_end_addr(u32 begin, u32 end)
+{
+	return ((begin >> UPPER_16_SHIFT) & LOWER_16_MASK) | (end & UPPER_16_MASK);
+}
+
+static inline u32 remap_roundup(u32 val, u32 align)
+{
+	return (val + (align - 1)) & ~(align - 1);
+}
+
+static inline u32 adsp_remap_region_from(u32 start, u32 size)
+{
+	return mix_begin_and_end_addr(start, remap_roundup(start + size, REMAP_ALIGN_SIZE));
+}
+#endif
-- 
2.45.2

Re: [PATCH v2 1/2] dt-bindings: dsp: mediatek: add mt8196 dsp document

[Krzysztof Kozlowski]

On 03/07/2025 09:56, hailong.fan wrote:
> From: Hailong Fan <hailong.fan@mediatek.com>
> 
> This patch adds mt8196 dsp document. The dsp is used for Sound Open

Please do not use "This commit/patch/change", but imperative mood. See
longer explanation here:
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95

> Firmware driver node. It includes registers,  clocks, memory regions,
> and mailbox for dsp.
> 
> Signed-off-by: Hailong Fan <hailong.fan@mediatek.com>
> ---
>  .../bindings/sound/mediatek,mt8196-dsp.yaml   | 95 +++++++++++++++++++
>  1 file changed, 95 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/sound/mediatek,mt8196-dsp.yaml
> 
> diff --git a/Documentation/devicetree/bindings/sound/mediatek,mt8196-dsp.yaml b/Documentation/devicetree/bindings/sound/mediatek,mt8196-dsp.yaml
> new file mode 100644
> index 000000000000..68f594f476e8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/mediatek,mt8196-dsp.yaml
> @@ -0,0 +1,95 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/sound/mediatek,mt8196-dsp.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek mt8196 DSP core
> +
> +maintainers:
> +  - Hailong Fan <hailong.fan@mediatek.com>
> +
> +description: The MediaTek mt8196 SoC mt8196 contain a DSP core used for advanced

Missing blank line. Look at other bindings. Also does not look wrapped
at 80 (see Linux coding style doc).


> +  pre- and post- audio processing.
> +
> +properties:
> +  compatible:
> +    const: mediatek,mt8196-dsp
> +
> +  reg:
> +    items:
> +      - description: DSP configuration registers
> +      - description: DSP SRAM
> +      - description: DSP secure registers
> +      - description: DSP bus registers
> +
> +  reg-names:
> +    items:
> +      - const: cfg
> +      - const: sram
> +      - const: sec
> +      - const: bus
> +
> +  clocks:
> +    items:
> +      - description: mux for dsp clock
> +      - description: 26M clock
> +      - description: ADSP PLL clock
> +
> +  clock-names:
> +    items:
> +      - const: adsp_sel

Isn't this called audiodsp in other bindings?

> +      - const: clk26m

Don't use frequencies. How is the pin or input called in datasheet? How
other devices call it?

> +      - const: adsppll
> +
> +  power-domains:
> +    maxItems: 1
> +
Best regards,
Krzysztof

Re: [PATCH v2 1/2] dt-bindings: dsp: mediatek: add mt8196 dsp document

[AngeloGioacchino Del Regno]

Il 03/07/25 10:08, Krzysztof Kozlowski ha scritto:
> On 03/07/2025 09:56, hailong.fan wrote:
>> From: Hailong Fan <hailong.fan@mediatek.com>
>>
>> This patch adds mt8196 dsp document. The dsp is used for Sound Open
> 
> Please do not use "This commit/patch/change", but imperative mood. See
> longer explanation here:
> https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95
> 
>> Firmware driver node. It includes registers,  clocks, memory regions,
>> and mailbox for dsp.
>>
>> Signed-off-by: Hailong Fan <hailong.fan@mediatek.com>
>> ---
>>   .../bindings/sound/mediatek,mt8196-dsp.yaml   | 95 +++++++++++++++++++
>>   1 file changed, 95 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/sound/mediatek,mt8196-dsp.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/sound/mediatek,mt8196-dsp.yaml b/Documentation/devicetree/bindings/sound/mediatek,mt8196-dsp.yaml
>> new file mode 100644
>> index 000000000000..68f594f476e8
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/sound/mediatek,mt8196-dsp.yaml

Wrong folder; this should go to dsp/mediatek,mt8196-dsp.yaml ....

...but then I don't get why MT8196 wasn't simply added to mediatek,mt8186-dsp.yaml.

>> @@ -0,0 +1,95 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/sound/mediatek,mt8196-dsp.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: MediaTek mt8196 DSP core
>> +
>> +maintainers:
>> +  - Hailong Fan <hailong.fan@mediatek.com>
>> +
>> +description: The MediaTek mt8196 SoC mt8196 contain a DSP core used for advanced
> 
> Missing blank line. Look at other bindings. Also does not look wrapped
> at 80 (see Linux coding style doc).
> 

There's also a typo, "mt8196 SoC mt8196"

> 
>> +  pre- and post- audio processing.
>> +
>> +properties:
>> +  compatible:
>> +    const: mediatek,mt8196-dsp
>> +
>> +  reg:
>> +    items:
>> +      - description: DSP configuration registers
>> +      - description: DSP SRAM
>> +      - description: DSP secure registers
>> +      - description: DSP bus registers
>> +
>> +  reg-names:
>> +    items:
>> +      - const: cfg
>> +      - const: sram
>> +      - const: sec
>> +      - const: bus
>> +
>> +  clocks:
>> +    items:
>> +      - description: mux for dsp clock
>> +      - description: 26M clock
>> +      - description: ADSP PLL clock
>> +
>> +  clock-names:
>> +    items:
>> +      - const: adsp_sel
> 
> Isn't this called audiodsp in other bindings?
> 
>> +      - const: clk26m
> 
> Don't use frequencies. How is the pin or input called in datasheet? How
> other devices call it?

In the datasheet, this is called.... CLK26M (really).

Cheers,
Angelo

> 
>> +      - const: adsppll
>> +
>> +  power-domains:
>> +    maxItems: 1
>> +
> Best regards,
> Krzysztof

Re: [PATCH v2 1/2] dt-bindings: dsp: mediatek: add mt8196 dsp document

[Rob Herring (Arm)]

On Thu, 03 Jul 2025 15:56:23 +0800, hailong.fan wrote:
> From: Hailong Fan <hailong.fan@mediatek.com>
> 
> This patch adds mt8196 dsp document. The dsp is used for Sound Open
> Firmware driver node. It includes registers,  clocks, memory regions,
> and mailbox for dsp.
> 
> Signed-off-by: Hailong Fan <hailong.fan@mediatek.com>
> ---
>  .../bindings/sound/mediatek,mt8196-dsp.yaml   | 95 +++++++++++++++++++
>  1 file changed, 95 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/sound/mediatek,mt8196-dsp.yaml
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/sound/mediatek,mt8196-dsp.example.dts:18:18: fatal error: dt-bindings/clock/mt8196-clk.h: No such file or directory
   18 |         #include <dt-bindings/clock/mt8196-clk.h>
      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[2]: *** [scripts/Makefile.dtbs:131: Documentation/devicetree/bindings/sound/mediatek,mt8196-dsp.example.dtb] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1525: dt_binding_check] Error 2
make: *** [Makefile:248: __sub-make] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250703075632.20758-2-hailong.fan@mediatek.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH v2 1/2] dt-bindings: dsp: mediatek: add mt8196 dsp document

[Krzysztof Kozlowski]

On 03/07/2025 11:05, AngeloGioacchino Del Regno wrote:
>>> +
>>> +  clocks:
>>> +    items:
>>> +      - description: mux for dsp clock
>>> +      - description: 26M clock
>>> +      - description: ADSP PLL clock
>>> +
>>> +  clock-names:
>>> +    items:
>>> +      - const: adsp_sel
>>
>> Isn't this called audiodsp in other bindings?
>>
>>> +      - const: clk26m
>>
>> Don't use frequencies. How is the pin or input called in datasheet? How
>> other devices call it?
> 
> In the datasheet, this is called.... CLK26M (really).
> 
OK, this is a valid argument, however we still try to unify the inputs
so bindings can share such pieces. It is discouraged to have similar
devices with different bindings in only one place: clk26m -> clk27m or
whatever other number.

Common is also to name the clock input based on the purpose (like bus, ref).


Best regards,
Krzysztof

Re: [PATCH v2 1/2] dt-bindings: dsp: mediatek: add mt8196 dsp document

[Hailong Fan (范海龙)]

On Thu, 2025-07-03 at 10:08 +0200, Krzysztof Kozlowski wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On 03/07/2025 09:56, hailong.fan wrote:
> > From: Hailong Fan <hailong.fan@mediatek.com>
> > 
> > This patch adds mt8196 dsp document. The dsp is used for Sound Open
> 
> Please do not use "This commit/patch/change", but imperative mood.
> See
> longer explanation here:
> 
https://urldefense.com/v3/__https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst*L95__;Iw!!CTRNKA9wMg0ARbw!mhCB8cQ9NY4AztdErJWay3VtFNHSe89i00TgGeTd62W6m-Ios46XeDaYqM0QQ7m4SHjlNw69PvU7T9M$
> 
Okay, will be updated to:
Add device tree binding documentation for the
MediaTek MT8196 DSP. The DSP is used by the Sound Open Firmware driver
node and includes registers, clocks, memory regions, and a mailbox for
DSP communication.
 
Thanks.

> > Firmware driver node. It includes registers,  clocks, memory
> > regions,
> > and mailbox for dsp.
> > 
> > Signed-off-by: Hailong Fan <hailong.fan@mediatek.com>
> > ---
> >  .../bindings/sound/mediatek,mt8196-dsp.yaml   | 95
> > +++++++++++++++++++
> >  1 file changed, 95 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/sound/mediatek,mt8196-dsp.yaml
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/sound/mediatek,mt8196-dsp.yaml
> > b/Documentation/devicetree/bindings/sound/mediatek,mt8196-dsp.yaml
> > new file mode 100644
> > index 000000000000..68f594f476e8
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/sound/mediatek,mt8196-
> > dsp.yaml
> > @@ -0,0 +1,95 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: 
> > https://urldefense.com/v3/__http://devicetree.org/schemas/sound/mediatek,mt8196-dsp.yaml*__;Iw!!CTRNKA9wMg0ARbw!mhCB8cQ9NY4AztdErJWay3VtFNHSe89i00TgGeTd62W6m-Ios46XeDaYqM0QQ7m4SHjlNw697qH1t6A$
> > +$schema: 
> > https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.yaml*__;Iw!!CTRNKA9wMg0ARbw!mhCB8cQ9NY4AztdErJWay3VtFNHSe89i00TgGeTd62W6m-Ios46XeDaYqM0QQ7m4SHjlNw69EdDHgNY$
> > +
> > +title: MediaTek mt8196 DSP core
> > +
> > +maintainers:
> > +  - Hailong Fan <hailong.fan@mediatek.com>
> > +
> > +description: The MediaTek mt8196 SoC mt8196 contain a DSP core
> > used for advanced
> 
> Missing blank line. Look at other bindings. Also does not look
> wrapped
> at 80 (see Linux coding style doc).
> 
> 
Okay, will be update to:
description: |
The MediaTek mt8196 SoC contains
a DSP core used for advanced pre- and
  post-audio processing. This DSP
is typically used by the Sound Open Firmware
  (SOF) driver and requires
registers, clocks, memory regions, and a mailbox
  for communication.
 
Th
anks.
> > +  pre- and post- audio processing.
> > +
> > +properties:
> > +  compatible:
> > +    const: mediatek,mt8196-dsp
> > +
> > +  reg:
> > +    items:
> > +      - description: DSP configuration registers
> > +      - description: DSP SRAM
> > +      - description: DSP secure registers
> > +      - description: DSP bus registers
> > +
> > +  reg-names:
> > +    items:
> > +      - const: cfg
> > +      - const: sram
> > +      - const: sec
> > +      - const: bus
> > +
> > +  clocks:
> > +    items:
> > +      - description: mux for dsp clock
> > +      - description: 26M clock
> > +      - description: ADSP PLL clock
> > +
> > +  clock-names:
> > +    items:
> > +      - const: adsp_sel
> 
> Isn't this called audiodsp in other bindings?
> 
Yes, but essentially it is the mux selection of the ADSP clock.
So, would using "adsp_sel" make it clearer?

> > +      - const: clk26m
> 
> Don't use frequencies. How is the pin or input called in datasheet?
> How
> other devices call it?
> 
> > +      - const: adsppll
> > +
> > +  power-domains:
> > +    maxItems: 1
> > +
> 
> Best regards,
> Krzysztof

Re: [PATCH v2 1/2] dt-bindings: dsp: mediatek: add mt8196 dsp document

[Hailong Fan (范海龙)]

On Thu, 2025-07-03 at 12:24 +0200, Krzysztof Kozlowski wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On 03/07/2025 11:05, AngeloGioacchino Del Regno wrote:
> > > > +
> > > > +  clocks:
> > > > +    items:
> > > > +      - description: mux for dsp clock
> > > > +      - description: 26M clock
> > > > +      - description: ADSP PLL clock
> > > > +
> > > > +  clock-names:
> > > > +    items:
> > > > +      - const: adsp_sel
> > > 
> > > Isn't this called audiodsp in other bindings?
> > > 
> > > > +      - const: clk26m
> > > 
> > > Don't use frequencies. How is the pin or input called in
> > > datasheet? How
> > > other devices call it?
> > 
> > In the datasheet, this is called.... CLK26M (really).
> > 
> 
> OK, this is a valid argument, however we still try to unify the
> inputs
> so bindings can share such pieces. It is discouraged to have similar
> devices with different bindings in only one place: clk26m -> clk27m
> or
> whatever other number.
> 
> Common is also to name the clock input based on the purpose (like
> bus, ref).
> 
> 
> Best regards,
> Krzysztof

On the MediaTek platform, the main SoC clock sources are typically 32K,
13M, and 26M. This is why the terms clk32k, clk13m, and clk26m are
used.
Specifically, clk26m refers to the system’s 26 MHz clock source.
The clkxxx naming convention is intended to indicate that these are SoC
clock sources, while also distinguishing between different clock
frequencies.
 
Therefore, using other terms could potentially cause confusion.
 
For example, the ADSP clock sources are ADSPPLL(800MHz) and
clk26m(26MHz).
On other platforms, the definition of clk26m in the Device Tree Source
(DTS) is as follows:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/mediatek/mt8188.dtsi#n328
The mt8196 platform uses the same design.
 
Best Regards,
Hailong.Fan

Re: [PATCH v2 1/2] dt-bindings: dsp: mediatek: add mt8196 dsp document

[Hailong Fan (范海龙)]

On Thu, 2025-07-03 at 11:05 +0200, AngeloGioacchino Del Regno wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> Il 03/07/25 10:08, Krzysztof Kozlowski ha scritto:
> > On 03/07/2025 09:56, hailong.fan wrote:
> > > From: Hailong Fan <hailong.fan@mediatek.com>
> > > 
> > > This patch adds mt8196 dsp document. The dsp is used for Sound
> > > Open
> > 
> > Please do not use "This commit/patch/change", but imperative mood.
> > See
> > longer explanation here:
> > 
https://urldefense.com/v3/__https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst*L95__;Iw!!CTRNKA9wMg0ARbw!g39q48fUCsDJ5PETUtQ0PK0Bkwf0Ce1F_ZzJiEiWMzuybWQ0dSPzblvLlJ-4saWTSoJWuS5imsXbpdgLBvUy4wxw9C5w7B4m$
> > 
> > > Firmware driver node. It includes registers,  clocks, memory
> > > regions,
> > > and mailbox for dsp.
> > > 
> > > Signed-off-by: Hailong Fan <hailong.fan@mediatek.com>
> > > ---
> > >   .../bindings/sound/mediatek,mt8196-dsp.yaml   | 95
> > > +++++++++++++++++++
> > >   1 file changed, 95 insertions(+)
> > >   create mode 100644
> > > Documentation/devicetree/bindings/sound/mediatek,mt8196-dsp.yaml
> > > 
> > > diff --git
> > > a/Documentation/devicetree/bindings/sound/mediatek,mt8196-
> > > dsp.yaml
> > > b/Documentation/devicetree/bindings/sound/mediatek,mt8196-
> > > dsp.yaml
> > > new file mode 100644
> > > index 000000000000..68f594f476e8
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/sound/mediatek,mt8196-
> > > dsp.yaml
> 
> Wrong folder; this should go to dsp/mediatek,mt8196-dsp.yaml ....
> 
> ...but then I don't get why MT8196 wasn't simply added to
> mediatek,mt8186-dsp.yaml.
> 
Yes, the first version was written this way, but the maintainer
suggested the following, so I'm not sure which approach is more
appropriate. Could you please provide a final recommendation?
 
v1 link: 
https://lore.kernel.org/all/a72988212d0d95bfe76eb9c53bbdb8c57c989377.camel@mediatek.com/
 
comment:
>
> > ---
> >  .../bindings/dsp/mediatek,mt8196-dsp.yaml     | 96
> > +++++++++++++++++++
>
> Don't grow dsp directory. Either this is remoteproc or some sound
> component, so place it accordingly.
This is a reference to the approach used in a previous MediaTek
project:
 

https://web.git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/dsp
Do we need to move all the files under the DSP directory to the sound
directory?

> > > @@ -0,0 +1,95 @@
> > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: 
> > > https://urldefense.com/v3/__http://devicetree.org/schemas/sound/mediatek,mt8196-dsp.yaml*__;Iw!!CTRNKA9wMg0ARbw!g39q48fUCsDJ5PETUtQ0PK0Bkwf0Ce1F_ZzJiEiWMzuybWQ0dSPzblvLlJ-4saWTSoJWuS5imsXbpdgLBvUy4wxw9GN4ryBQ$
> > > +$schema: 
> > > https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.yaml*__;Iw!!CTRNKA9wMg0ARbw!g39q48fUCsDJ5PETUtQ0PK0Bkwf0Ce1F_ZzJiEiWMzuybWQ0dSPzblvLlJ-4saWTSoJWuS5imsXbpdgLBvUy4wxw9NKbMuhN$
> > > +
> > > +title: MediaTek mt8196 DSP core
> > > +
> > > +maintainers:
> > > +  - Hailong Fan <hailong.fan@mediatek.com>
> > > +
> > > +description: The MediaTek mt8196 SoC mt8196 contain a DSP core
> > > used for advanced
> > 
> > Missing blank line. Look at other bindings. Also does not look
> > wrapped
> > at 80 (see Linux coding style doc).
> > 
> 
> There's also a typo, "mt8196 SoC mt8196"
> 
Will update in next version, thanks.

> > 
> > > +  pre- and post- audio processing.
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: mediatek,mt8196-dsp
> > > +
> > > +  reg:
> > > +    items:
> > > +      - description: DSP configuration registers
> > > +      - description: DSP SRAM
> > > +      - description: DSP secure registers
> > > +      - description: DSP bus registers
> > > +
> > > +  reg-names:
> > > +    items:
> > > +      - const: cfg
> > > +      - const: sram
> > > +      - const: sec
> > > +      - const: bus
> > > +
> > > +  clocks:
> > > +    items:
> > > +      - description: mux for dsp clock
> > > +      - description: 26M clock
> > > +      - description: ADSP PLL clock
> > > +
> > > +  clock-names:
> > > +    items:
> > > +      - const: adsp_sel
> > 
> > Isn't this called audiodsp in other bindings?
> > 
> > > +      - const: clk26m
> > 
> > Don't use frequencies. How is the pin or input called in datasheet?
> > How
> > other devices call it?
> 
> In the datasheet, this is called.... CLK26M (really).
> 
> Cheers,
> Angelo
> 
> > 
> > > +      - const: adsppll
> > > +
> > > +  power-domains:
> > > +    maxItems: 1
> > > +
> > 
> > Best regards,
> > Krzysztof
> 
> 

Re: [PATCH v2 1/2] dt-bindings: dsp: mediatek: add mt8196 dsp document

[Krzysztof Kozlowski]

On 09/07/2025 04:52, Hailong Fan (范海龙) wrote:
>>> +  clocks:
>>> +    items:
>>> +      - description: mux for dsp clock
>>> +      - description: 26M clock
>>> +      - description: ADSP PLL clock
>>> +
>>> +  clock-names:
>>> +    items:
>>> +      - const: adsp_sel
>>
>> Isn't this called audiodsp in other bindings?
>>
> Yes, but essentially it is the mux selection of the ADSP clock.
> So, would using "adsp_sel" make it clearer?
> 
So every new binding will come with new name? Stick to the same name if
this is the same clock.

Best regards,
Krzysztof

Re: [PATCH v2 1/2] dt-bindings: dsp: mediatek: add mt8196 dsp document

[Krzysztof Kozlowski]

On 09/07/2025 08:05, Hailong Fan (范海龙) wrote:
>>
>> OK, this is a valid argument, however we still try to unify the
>> inputs
>> so bindings can share such pieces. It is discouraged to have similar
>> devices with different bindings in only one place: clk26m -> clk27m
>> or
>> whatever other number.
>>
>> Common is also to name the clock input based on the purpose (like
>> bus, ref).
>>
>>
>> Best regards,
>> Krzysztof
> 
> On the MediaTek platform, the main SoC clock sources are typically 32K,
> 13M, and 26M. This is why the terms clk32k, clk13m, and clk26m are
> used.
> Specifically, clk26m refers to the system’s 26 MHz clock source.
> The clkxxx naming convention is intended to indicate that these are SoC

But it should nnot.

> clock sources, while also distinguishing between different clock
> frequencies.
>  
> Therefore, using other terms could potentially cause confusion.

No, you don't understand. This device receives some main PLL or ref
clock. That's the only important information in the name, not its
frequency..

>  
> For example, the ADSP clock sources are ADSPPLL(800MHz) and
> clk26m(26MHz).
> On other platforms, the definition of clk26m in the Device Tree Source
> (DTS) is as follows:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/mediatek/mt8188.dtsi#n328
> The mt8196 platform uses the same design.

So you add poor code, because previously Mediatek added more poor code?
What sort of argument is that?


Best regards,
Krzysztof

Re: [PATCH v2 1/2] dt-bindings: dsp: mediatek: add mt8196 dsp document

[Krzysztof Kozlowski]

On 09/07/2025 08:34, Hailong Fan (范海龙) wrote:
> On Thu, 2025-07-03 at 11:05 +0200, AngeloGioacchino Del Regno wrote:
>> External email : Please do not click links or open attachments until
>> you have verified the sender or the content.
>>
>>
>> Il 03/07/25 10:08, Krzysztof Kozlowski ha scritto:
>>> On 03/07/2025 09:56, hailong.fan wrote:
>>>> From: Hailong Fan <hailong.fan@mediatek.com>
>>>>
>>>> This patch adds mt8196 dsp document. The dsp is used for Sound
>>>> Open
>>>
>>> Please do not use "This commit/patch/change", but imperative mood.
>>> See
>>> longer explanation here:
>>>
> https://urldefense.com/v3/__https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst*L95__;Iw!!CTRNKA9wMg0ARbw!g39q48fUCsDJ5PETUtQ0PK0Bkwf0Ce1F_ZzJiEiWMzuybWQ0dSPzblvLlJ-4saWTSoJWuS5imsXbpdgLBvUy4wxw9C5w7B4m$
>>>

Fix your email client not to produce such junk like above ^.

>>>> Firmware driver node. It includes registers,  clocks, memory
>>>> regions,
>>>> and mailbox for dsp.
>>>>
>>>> Signed-off-by: Hailong Fan <hailong.fan@mediatek.com>
>>>> ---
>>>>   .../bindings/sound/mediatek,mt8196-dsp.yaml   | 95
>>>> +++++++++++++++++++
>>>>   1 file changed, 95 insertions(+)
>>>>   create mode 100644
>>>> Documentation/devicetree/bindings/sound/mediatek,mt8196-dsp.yaml
>>>>
>>>> diff --git
>>>> a/Documentation/devicetree/bindings/sound/mediatek,mt8196-
>>>> dsp.yaml
>>>> b/Documentation/devicetree/bindings/sound/mediatek,mt8196-
>>>> dsp.yaml
>>>> new file mode 100644
>>>> index 000000000000..68f594f476e8
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/sound/mediatek,mt8196-
>>>> dsp.yaml
>>
>> Wrong folder; this should go to dsp/mediatek,mt8196-dsp.yaml ....
>>
>> ...but then I don't get why MT8196 wasn't simply added to
>> mediatek,mt8186-dsp.yaml.
>>
> Yes, the first version was written this way, but the maintainer
> suggested the following, so I'm not sure which approach is more
> appropriate. Could you please provide a final recommendation?
>  
> v1 link: 
> https://lore.kernel.org/all/a72988212d0d95bfe76eb9c53bbdb8c57c989377.camel@mediatek.com/

You linked to your reply. What does it prove? You are the maintainer and
you gave this recommendation for your own patch?

>  
> comment:
>>
>>> ---
>>>  .../bindings/dsp/mediatek,mt8196-dsp.yaml     | 96
>>> +++++++++++++++++++
>>
>> Don't grow dsp directory. Either this is remoteproc or some sound
>> component, so place it accordingly.
> This is a reference to the approach used in a previous MediaTek
> project:
>  
> 
> https://web.git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/dsp
> Do we need to move all the files under the DSP directory to the sound
> directory?

No.


Best regards,
Krzysztof

Re: [PATCH v2 1/2] dt-bindings: dsp: mediatek: add mt8196 dsp document

[Hailong Fan (范海龙)]

On Wed, 2025-07-09 at 08:57 +0200, Krzysztof Kozlowski wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On 09/07/2025 08:05, Hailong Fan (范海龙) wrote:
> > > 
> > > OK, this is a valid argument, however we still try to unify the
> > > inputs
> > > so bindings can share such pieces. It is discouraged to have
> > > similar
> > > devices with different bindings in only one place: clk26m ->
> > > clk27m
> > > or
> > > whatever other number.
> > > 
> > > Common is also to name the clock input based on the purpose (like
> > > bus, ref).
> > > 
> > > 
> > > Best regards,
> > > Krzysztof
> > 
> > On the MediaTek platform, the main SoC clock sources are typically
> > 32K,
> > 13M, and 26M. This is why the terms clk32k, clk13m, and clk26m are
> > used.
> > Specifically, clk26m refers to the system’s 26 MHz clock source.
> > The clkxxx naming convention is intended to indicate that these are
> > SoC
> 
> But it should nnot.
> 
> > clock sources, while also distinguishing between different clock
> > frequencies.
> > 
> > Therefore, using other terms could potentially cause confusion.
> 
> No, you don't understand. This device receives some main PLL or ref
> clock. That's the only important information in the name, not its
> frequency..
> 
> > 
> > For example, the ADSP clock sources are ADSPPLL(800MHz) and
> > clk26m(26MHz).
> > On other platforms, the definition of clk26m in the Device Tree
> > Source
> > (DTS) is as follows:
> > 
> > 
https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/mediatek/mt8188.dtsi*n328__;Iw!!CTRNKA9wMg0ARbw!ibScPB89tyVolnUhBscm6gkEwMebDiD8FozTY1H_-cMZ5_klBxmJxgWlOTOhboITEls4ef4w-Tce2qs$
> > The mt8196 platform uses the same design.
> 
> So you add poor code, because previously Mediatek added more poor
> code?
> What sort of argument is that?
Thank you for your suggestion. We are planning to update it to sys_clk
in the next version. What do you think?
> 
> 
> Best regards,
> Krzysztof

Re: [PATCH v2 1/2] dt-bindings: dsp: mediatek: add mt8196 dsp document

[Hailong Fan (范海龙)]

On Wed, 2025-07-09 at 08:59 +0200, Krzysztof Kozlowski wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On 09/07/2025 08:34, Hailong Fan (范海龙) wrote:
> > On Thu, 2025-07-03 at 11:05 +0200, AngeloGioacchino Del Regno
> > wrote:
> > > External email : Please do not click links or open attachments
> > > until
> > > you have verified the sender or the content.
> > > 
> > > 
> > > Il 03/07/25 10:08, Krzysztof Kozlowski ha scritto:
> > > > On 03/07/2025 09:56, hailong.fan wrote:
> > > > > From: Hailong Fan <hailong.fan@mediatek.com>
> > > > > 
> > > > > This patch adds mt8196 dsp document. The dsp is used for
> > > > > Sound
> > > > > Open
> > > > 
> > > > Please do not use "This commit/patch/change", but imperative
> > > > mood.
> > > > See
> > > > longer explanation here:
> > > > 
> > 
> > 
https://urldefense.com/v3/__https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst*L95__;Iw!!CTRNKA9wMg0ARbw!g39q48fUCsDJ5PETUtQ0PK0Bkwf0Ce1F_ZzJiEiWMzuybWQ0dSPzblvLlJ-4saWTSoJWuS5imsXbpdgLBvUy4wxw9C5w7B4m$
> > > > 
> 
> Fix your email client not to produce such junk like above ^.
OK,thx.
> 
> > > > > Firmware driver node. It includes registers,  clocks, memory
> > > > > regions,
> > > > > and mailbox for dsp.
> > > > > 
> > > > > Signed-off-by: Hailong Fan <hailong.fan@mediatek.com>
> > > > > ---
> > > > >   .../bindings/sound/mediatek,mt8196-dsp.yaml   | 95
> > > > > +++++++++++++++++++
> > > > >   1 file changed, 95 insertions(+)
> > > > >   create mode 100644
> > > > > Documentation/devicetree/bindings/sound/mediatek,mt8196-
> > > > > dsp.yaml
> > > > > 
> > > > > diff --git
> > > > > a/Documentation/devicetree/bindings/sound/mediatek,mt8196-
> > > > > dsp.yaml
> > > > > b/Documentation/devicetree/bindings/sound/mediatek,mt8196-
> > > > > dsp.yaml
> > > > > new file mode 100644
> > > > > index 000000000000..68f594f476e8
> > > > > --- /dev/null
> > > > > +++
> > > > > b/Documentation/devicetree/bindings/sound/mediatek,mt8196-
> > > > > dsp.yaml
> > > 
> > > Wrong folder; this should go to dsp/mediatek,mt8196-dsp.yaml ....
> > > 
> > > ...but then I don't get why MT8196 wasn't simply added to
> > > mediatek,mt8186-dsp.yaml.
> > > 
> > 
> > Yes, the first version was written this way, but the maintainer
> > suggested the following, so I'm not sure which approach is more
> > appropriate. Could you please provide a final recommendation?
> > 
> > v1 link:
> > 
https://lore.kernel.org/all/a72988212d0d95bfe76eb9c53bbdb8c57c989377.camel@mediatek.com/
> 
> You linked to your reply. What does it prove? You are the maintainer
> and
> you gave this recommendation for your own patch?
> 
> > 
> > comment:
> > > 
> > > > ---
> > > >  .../bindings/dsp/mediatek,mt8196-dsp.yaml     | 96
> > > > +++++++++++++++++++
> > > 
> > > Don't grow dsp directory. Either this is remoteproc or some sound
> > > component, so place it accordingly.
> > 
> > This is a reference to the approach used in a previous MediaTek
> > project:
> > 
> > 
> > 
https://urldefense.com/v3/__https://web.git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/dsp__;!!CTRNKA9wMg0ARbw!nYISmR0CRlwCkvCsX__KkrKPFCiGy1jjwq_rZjMMXmQ6RhzlUeLgBmUpxvAslqJZTvM7J-QDvq_hpD4$
> > Do we need to move all the files under the DSP directory to the
> > sound
> > directory?
> 
> No.
Got it, we will place mediatek,mt8196-dsp.yaml in the
Documentation/devicetree/bindings/dsp directory. Thx.
> 
> 
> Best regards,
> Krzysztof

Re: [PATCH v2 1/2] dt-bindings: dsp: mediatek: add mt8196 dsp document

[Hailong Fan (范海龙)]

On Wed, 2025-07-09 at 08:55 +0200, Krzysztof Kozlowski wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On 09/07/2025 04:52, Hailong Fan (范海龙) wrote:
> > > > +  clocks:
> > > > +    items:
> > > > +      - description: mux for dsp clock
> > > > +      - description: 26M clock
> > > > +      - description: ADSP PLL clock
> > > > +
> > > > +  clock-names:
> > > > +    items:
> > > > +      - const: adsp_sel
> > > 
> > > Isn't this called audiodsp in other bindings?
> > > 
> > 
> > Yes, but essentially it is the mux selection of the ADSP clock.
> > So, would using "adsp_sel" make it clearer?
> > 
> 
> So every new binding will come with new name? Stick to the same name
> if
> this is the same clock.
> 
> Best regards,
> Krzysztof
Got it, we will keep the name consistent, thanks a lot.