All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] Add code to manage DSP clocks and provide dts-binding document
@ 2021-10-21  3:58 ` YC Hung
  0 siblings, 0 replies; 15+ messages in thread
From: YC Hung @ 2021-10-21  3:58 UTC (permalink / raw)
  To: broonie, tiwai, robh+dt, matthias.bgg
  Cc: yc.hung, alsa-devel, linux-mediatek, linux-arm-kernel,
	linux-kernel, devicetree, daniel.baluta, trevor.wu,
	allen-kh.cheng

From: "yc.hung" <yc.hung@mediatek.com>

This code is based on top of SOF topic/sof-dev branch and we want to have a review 
with ALSA and device Tree communities the it will be merged to SOF tree and then
merged into ALSA tree. It provides two patches, one is for mt8195 dsp clocks related.
Another is for mt8195 dsp dts binding decription.

YC Hung (2):
  ASoC: SOF: mediatek: Add mt8195 dsp clock support
  dt-bindings: dsp: mediatek: Add mt8195 DSP binding support

 .../bindings/dsp/mtk,mt8195-dsp.yaml          | 139 +++++++++++++++
 sound/soc/sof/mediatek/mt8195/Makefile        |   2 +-
 sound/soc/sof/mediatek/mt8195/mt8195-clk.c    | 164 ++++++++++++++++++
 sound/soc/sof/mediatek/mt8195/mt8195-clk.h    |  29 ++++
 sound/soc/sof/mediatek/mt8195/mt8195.c        |  23 ++-
 5 files changed, 354 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/dsp/mtk,mt8195-dsp.yaml
 create mode 100644 sound/soc/sof/mediatek/mt8195/mt8195-clk.c
 create mode 100644 sound/soc/sof/mediatek/mt8195/mt8195-clk.h

-- 
2.18.0


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

* [PATCH v2 0/2] Add code to manage DSP clocks and provide dts-binding document
@ 2021-10-21  3:58 ` YC Hung
  0 siblings, 0 replies; 15+ messages in thread
From: YC Hung @ 2021-10-21  3:58 UTC (permalink / raw)
  To: broonie, tiwai, robh+dt, matthias.bgg
  Cc: yc.hung, alsa-devel, linux-mediatek, linux-arm-kernel,
	linux-kernel, devicetree, daniel.baluta, trevor.wu,
	allen-kh.cheng

From: "yc.hung" <yc.hung@mediatek.com>

This code is based on top of SOF topic/sof-dev branch and we want to have a review 
with ALSA and device Tree communities the it will be merged to SOF tree and then
merged into ALSA tree. It provides two patches, one is for mt8195 dsp clocks related.
Another is for mt8195 dsp dts binding decription.

YC Hung (2):
  ASoC: SOF: mediatek: Add mt8195 dsp clock support
  dt-bindings: dsp: mediatek: Add mt8195 DSP binding support

 .../bindings/dsp/mtk,mt8195-dsp.yaml          | 139 +++++++++++++++
 sound/soc/sof/mediatek/mt8195/Makefile        |   2 +-
 sound/soc/sof/mediatek/mt8195/mt8195-clk.c    | 164 ++++++++++++++++++
 sound/soc/sof/mediatek/mt8195/mt8195-clk.h    |  29 ++++
 sound/soc/sof/mediatek/mt8195/mt8195.c        |  23 ++-
 5 files changed, 354 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/dsp/mtk,mt8195-dsp.yaml
 create mode 100644 sound/soc/sof/mediatek/mt8195/mt8195-clk.c
 create mode 100644 sound/soc/sof/mediatek/mt8195/mt8195-clk.h

-- 
2.18.0


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

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

* [PATCH v2 0/2] Add code to manage DSP clocks and provide dts-binding document
@ 2021-10-21  3:58 ` YC Hung
  0 siblings, 0 replies; 15+ messages in thread
From: YC Hung @ 2021-10-21  3:58 UTC (permalink / raw)
  To: broonie, tiwai, robh+dt, matthias.bgg
  Cc: devicetree, alsa-devel, allen-kh.cheng, linux-kernel,
	linux-mediatek, trevor.wu, yc.hung, daniel.baluta,
	linux-arm-kernel

From: "yc.hung" <yc.hung@mediatek.com>

This code is based on top of SOF topic/sof-dev branch and we want to have a review 
with ALSA and device Tree communities the it will be merged to SOF tree and then
merged into ALSA tree. It provides two patches, one is for mt8195 dsp clocks related.
Another is for mt8195 dsp dts binding decription.

YC Hung (2):
  ASoC: SOF: mediatek: Add mt8195 dsp clock support
  dt-bindings: dsp: mediatek: Add mt8195 DSP binding support

 .../bindings/dsp/mtk,mt8195-dsp.yaml          | 139 +++++++++++++++
 sound/soc/sof/mediatek/mt8195/Makefile        |   2 +-
 sound/soc/sof/mediatek/mt8195/mt8195-clk.c    | 164 ++++++++++++++++++
 sound/soc/sof/mediatek/mt8195/mt8195-clk.h    |  29 ++++
 sound/soc/sof/mediatek/mt8195/mt8195.c        |  23 ++-
 5 files changed, 354 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/dsp/mtk,mt8195-dsp.yaml
 create mode 100644 sound/soc/sof/mediatek/mt8195/mt8195-clk.c
 create mode 100644 sound/soc/sof/mediatek/mt8195/mt8195-clk.h

-- 
2.18.0


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

* [PATCH v2 0/2] Add code to manage DSP clocks and provide dts-binding document
@ 2021-10-21  3:58 ` YC Hung
  0 siblings, 0 replies; 15+ messages in thread
From: YC Hung @ 2021-10-21  3:58 UTC (permalink / raw)
  To: broonie, tiwai, robh+dt, matthias.bgg
  Cc: yc.hung, alsa-devel, linux-mediatek, linux-arm-kernel,
	linux-kernel, devicetree, daniel.baluta, trevor.wu,
	allen-kh.cheng

From: "yc.hung" <yc.hung@mediatek.com>

This code is based on top of SOF topic/sof-dev branch and we want to have a review 
with ALSA and device Tree communities the it will be merged to SOF tree and then
merged into ALSA tree. It provides two patches, one is for mt8195 dsp clocks related.
Another is for mt8195 dsp dts binding decription.

YC Hung (2):
  ASoC: SOF: mediatek: Add mt8195 dsp clock support
  dt-bindings: dsp: mediatek: Add mt8195 DSP binding support

 .../bindings/dsp/mtk,mt8195-dsp.yaml          | 139 +++++++++++++++
 sound/soc/sof/mediatek/mt8195/Makefile        |   2 +-
 sound/soc/sof/mediatek/mt8195/mt8195-clk.c    | 164 ++++++++++++++++++
 sound/soc/sof/mediatek/mt8195/mt8195-clk.h    |  29 ++++
 sound/soc/sof/mediatek/mt8195/mt8195.c        |  23 ++-
 5 files changed, 354 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/dsp/mtk,mt8195-dsp.yaml
 create mode 100644 sound/soc/sof/mediatek/mt8195/mt8195-clk.c
 create mode 100644 sound/soc/sof/mediatek/mt8195/mt8195-clk.h

-- 
2.18.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH v2 1/2] ASoC: SOF: mediatek: Add mt8195 dsp clock support
  2021-10-21  3:58 ` YC Hung
  (?)
  (?)
@ 2021-10-21  3:58   ` YC Hung
  -1 siblings, 0 replies; 15+ messages in thread
From: YC Hung @ 2021-10-21  3:58 UTC (permalink / raw)
  To: broonie, tiwai, robh+dt, matthias.bgg
  Cc: yc.hung, alsa-devel, linux-mediatek, linux-arm-kernel,
	linux-kernel, devicetree, daniel.baluta, trevor.wu,
	allen-kh.cheng

Add adsp clock on/off support on mt8195 platform.

Signed-off-by: YC Hung <yc.hung@mediatek.com>
---
 sound/soc/sof/mediatek/mt8195/Makefile     |   2 +-
 sound/soc/sof/mediatek/mt8195/mt8195-clk.c | 164 +++++++++++++++++++++
 sound/soc/sof/mediatek/mt8195/mt8195-clk.h |  29 ++++
 sound/soc/sof/mediatek/mt8195/mt8195.c     |  23 ++-
 4 files changed, 215 insertions(+), 3 deletions(-)
 create mode 100644 sound/soc/sof/mediatek/mt8195/mt8195-clk.c
 create mode 100644 sound/soc/sof/mediatek/mt8195/mt8195-clk.h

diff --git a/sound/soc/sof/mediatek/mt8195/Makefile b/sound/soc/sof/mediatek/mt8195/Makefile
index 60fca24c068a..650f4bce99b2 100644
--- a/sound/soc/sof/mediatek/mt8195/Makefile
+++ b/sound/soc/sof/mediatek/mt8195/Makefile
@@ -1,4 +1,4 @@
 # SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
-snd-sof-mt8195-objs := mt8195.o mt8195-loader.o
+snd-sof-mt8195-objs := mt8195.o mt8195-clk.o mt8195-loader.o
 obj-$(CONFIG_SND_SOC_SOF_MT8195) += snd-sof-mt8195.o
 
diff --git a/sound/soc/sof/mediatek/mt8195/mt8195-clk.c b/sound/soc/sof/mediatek/mt8195/mt8195-clk.c
new file mode 100644
index 000000000000..1988421f7f7b
--- /dev/null
+++ b/sound/soc/sof/mediatek/mt8195/mt8195-clk.c
@@ -0,0 +1,164 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
+//
+// Copyright(c) 2021 Mediatek Corporation. All rights reserved.
+//
+// Author: YC Hung <yc.hung@mediatek.com>
+//
+// Hardware interface for mt8195 DSP clock
+
+#include <linux/clk.h>
+#include <linux/pm_runtime.h>
+#include <linux/io.h>
+#include "mt8195.h"
+#include "mt8195-clk.h"
+
+struct clk *clk_handle[ADSP_CLK_NUM];
+
+int platform_parse_clock(struct device *dev)
+{
+	clk_handle[CLK_TOP_ADSP] = devm_clk_get(dev, "adsp_sel");
+	if (IS_ERR(clk_handle[CLK_TOP_ADSP])) {
+		dev_err(dev, "clk_get(\"adsp_sel\") failed\n");
+		return PTR_ERR(clk_handle[CLK_TOP_ADSP]);
+	}
+
+	clk_handle[CLK_TOP_CLK26M] = devm_clk_get(dev, "clk26m_ck");
+	if (IS_ERR(clk_handle[CLK_TOP_CLK26M])) {
+		dev_err(dev, "clk_get(\"clk26m_ck\") failed\n");
+		return PTR_ERR(clk_handle[CLK_TOP_CLK26M]);
+	}
+
+	clk_handle[CLK_TOP_AUDIO_LOCAL_BUS] = devm_clk_get(dev, "audio_local_bus");
+	if (IS_ERR(clk_handle[CLK_TOP_AUDIO_LOCAL_BUS])) {
+		dev_err(dev, "clk_get(\"audio_local_bus\") failed\n");
+		return PTR_ERR(clk_handle[CLK_TOP_AUDIO_LOCAL_BUS]);
+	}
+
+	clk_handle[CLK_TOP_MAINPLL_D7_D2] = devm_clk_get(dev, "mainpll_d7_d2");
+	if (IS_ERR(clk_handle[CLK_TOP_MAINPLL_D7_D2])) {
+		dev_err(dev, "clk_get(\"mainpll_d7_d2\") failed\n");
+		return PTR_ERR(clk_handle[CLK_TOP_MAINPLL_D7_D2]);
+	}
+
+	clk_handle[CLK_SCP_ADSP_AUDIODSP] = devm_clk_get(dev, "scp_adsp_audiodsp");
+	if (IS_ERR(clk_handle[CLK_SCP_ADSP_AUDIODSP])) {
+		dev_err(dev, "clk_get(\"scp_adsp_audiodsp\") failed\n");
+		return PTR_ERR(clk_handle[CLK_SCP_ADSP_AUDIODSP]);
+	}
+
+	clk_handle[CLK_TOP_AUDIO_H] = devm_clk_get(dev, "audio_h");
+	if (IS_ERR(clk_handle[CLK_TOP_AUDIO_H])) {
+		dev_err(dev, "clk_get(\"audio_h_sel\") failed\n");
+		return PTR_ERR(clk_handle[CLK_TOP_AUDIO_H]);
+	}
+
+	return 0;
+}
+
+int adsp_enable_clock(struct device *dev)
+{
+	int ret;
+
+	ret = clk_prepare_enable(clk_handle[CLK_TOP_MAINPLL_D7_D2]);
+	if (ret) {
+		dev_err(dev, "%s clk_prepare_enable(mainpll_d7_d2) fail %d\n",
+			__func__, ret);
+		return ret;
+	}
+
+	ret = clk_prepare_enable(clk_handle[CLK_TOP_ADSP]);
+	if (ret) {
+		dev_err(dev, "%s clk_prepare_enable(adsp_sel) fail %d\n",
+			__func__, ret);
+		goto disable_mainpll_d7_d2_clk;
+	}
+
+	ret = clk_prepare_enable(clk_handle[CLK_TOP_AUDIO_LOCAL_BUS]);
+	if (ret) {
+		dev_err(dev, "%s clk_prepare_enable(audio_local_bus) fail %d\n",
+			__func__, ret);
+		goto disable_dsp_sel_clk;
+	}
+
+	ret = clk_prepare_enable(clk_handle[CLK_SCP_ADSP_AUDIODSP]);
+	if (ret) {
+		dev_err(dev, "%s clk_prepare_enable(scp_adsp_audiodsp) fail %d\n",
+			__func__, ret);
+		goto disable_audio_local_bus_clk;
+	}
+
+	ret = clk_prepare_enable(clk_handle[CLK_TOP_AUDIO_H]);
+	if (ret) {
+		dev_err(dev, "%s clk_prepare_enable(audio_h) fail %d\n",
+			__func__, ret);
+		goto disable_scp_adsp_audiodsp_clk;
+	}
+
+	return 0;
+
+disable_scp_adsp_audiodsp_clk:
+	clk_disable_unprepare(clk_handle[CLK_SCP_ADSP_AUDIODSP]);
+disable_audio_local_bus_clk:
+	clk_disable_unprepare(clk_handle[CLK_TOP_AUDIO_LOCAL_BUS]);
+disable_dsp_sel_clk:
+	clk_disable_unprepare(clk_handle[CLK_TOP_ADSP]);
+disable_mainpll_d7_d2_clk:
+	clk_disable_unprepare(clk_handle[CLK_TOP_MAINPLL_D7_D2]);
+
+	return ret;
+}
+
+void adsp_disable_clock(struct device *dev)
+{
+	clk_disable_unprepare(clk_handle[CLK_TOP_AUDIO_H]);
+	clk_disable_unprepare(clk_handle[CLK_SCP_ADSP_AUDIODSP]);
+	clk_disable_unprepare(clk_handle[CLK_TOP_AUDIO_LOCAL_BUS]);
+	clk_disable_unprepare(clk_handle[CLK_TOP_ADSP]);
+	clk_disable_unprepare(clk_handle[CLK_TOP_MAINPLL_D7_D2]);
+}
+
+int adsp_default_clk_init(struct device *dev, int enable)
+{
+	int ret = 0;
+
+	dev_dbg(dev, "%s: %s\n", __func__, enable ? "on" : "off");
+
+	if (enable) {
+		ret = clk_set_parent(clk_handle[CLK_TOP_ADSP],
+				     clk_handle[CLK_TOP_CLK26M]);
+		if (ret) {
+			dev_err(dev, "failed to set dsp_sel to clk26m: %d\n", ret);
+			return ret;
+		}
+
+		ret = clk_set_parent(clk_handle[CLK_TOP_AUDIO_LOCAL_BUS],
+				     clk_handle[CLK_TOP_MAINPLL_D7_D2]);
+		if (ret) {
+			dev_err(dev, "set audio_local_bus failed %d\n", ret);
+			return ret;
+		}
+
+		ret = adsp_enable_clock(dev);
+		if (ret)
+			dev_err(dev, "failed to adsp_enable_clock: %d\n", ret);
+
+		return ret;
+	}
+
+	adsp_disable_clock(dev);
+
+	return ret;
+}
+
+int adsp_clock_on(struct device *dev)
+{
+	/* Open ADSP clock */
+	return adsp_default_clk_init(dev, 1);
+}
+
+int adsp_clock_off(struct device *dev)
+{
+	/* Close ADSP clock */
+	return adsp_default_clk_init(dev, 0);
+}
+
diff --git a/sound/soc/sof/mediatek/mt8195/mt8195-clk.h b/sound/soc/sof/mediatek/mt8195/mt8195-clk.h
new file mode 100644
index 000000000000..f985d141552a
--- /dev/null
+++ b/sound/soc/sof/mediatek/mt8195/mt8195-clk.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/*
+ * Copyright (c) 2021 MediaTek Corporation. All rights reserved.
+ *
+ *  Header file for the mt8195 DSP clock  definition
+ */
+
+#ifndef __MT8195_CLK_H
+#define __MT8195_CLK_H
+
+/*DSP clock*/
+enum ADSP_CLK_ID {
+	CLK_TOP_ADSP,
+	CLK_TOP_CLK26M,
+	CLK_TOP_AUDIO_LOCAL_BUS,
+	CLK_TOP_MAINPLL_D7_D2,
+	CLK_SCP_ADSP_AUDIODSP,
+	CLK_TOP_AUDIO_H,
+	ADSP_CLK_NUM
+};
+
+int platform_parse_clock(struct device *dev);
+int adsp_default_clk_init(struct device *dev, int enable);
+int adsp_enable_clock(struct device *dev);
+void adsp_disable_clock(struct device *dev);
+int adsp_clock_on(struct device *dev);
+int adsp_clock_off(struct device *dev);
+#endif
diff --git a/sound/soc/sof/mediatek/mt8195/mt8195.c b/sound/soc/sof/mediatek/mt8195/mt8195.c
index 99075598a35a..f323da58057b 100644
--- a/sound/soc/sof/mediatek/mt8195/mt8195.c
+++ b/sound/soc/sof/mediatek/mt8195/mt8195.c
@@ -25,6 +25,7 @@
 #include "../adsp_helper.h"
 #include "../mediatek-ops.h"
 #include "mt8195.h"
+#include "mt8195-clk.h"
 
 static int platform_parse_resource(struct platform_device *pdev, void *data)
 {
@@ -231,10 +232,23 @@ static int mt8195_dsp_probe(struct snd_sof_dev *sdev)
 	if (ret)
 		return ret;
 
+	ret = platform_parse_clock(&pdev->dev);
+	if (ret) {
+		dev_err(sdev->dev, "platform_parse_clock failed\n");
+		return -EINVAL;
+	}
+
+	ret = adsp_clock_on(&pdev->dev);
+	if (ret) {
+		dev_err(sdev->dev, "adsp_clock_on fail!\n");
+		return -EINVAL;
+	}
+
 	ret = adsp_sram_power_on(sdev->dev, true);
 	if (ret) {
 		dev_err(sdev->dev, "adsp_sram_power_on fail!\n");
-		return ret;
+		ret = -EINVAL;
+		goto exit_clk_disable;
 	}
 
 	ret = adsp_memory_remap_init(&pdev->dev, priv->adsp);
@@ -282,6 +296,8 @@ static int mt8195_dsp_probe(struct snd_sof_dev *sdev)
 
 err_adsp_sram_power_off:
 	adsp_sram_power_on(&pdev->dev, false);
+exit_clk_disable:
+	adsp_clock_off(&pdev->dev);
 
 	return ret;
 }
@@ -290,7 +306,10 @@ static int mt8195_dsp_remove(struct snd_sof_dev *sdev)
 {
 	struct platform_device *pdev = container_of(sdev->dev, struct platform_device, dev);
 
-	return adsp_sram_power_on(&pdev->dev, false);
+	adsp_sram_power_on(&pdev->dev, false);
+	adsp_clock_off(&pdev->dev);
+
+	return 0;
 }
 
 /* on mt8195 there is 1 to 1 match between type and BAR idx */
-- 
2.18.0


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

* [PATCH v2 1/2] ASoC: SOF: mediatek: Add mt8195 dsp clock support
@ 2021-10-21  3:58   ` YC Hung
  0 siblings, 0 replies; 15+ messages in thread
From: YC Hung @ 2021-10-21  3:58 UTC (permalink / raw)
  To: broonie, tiwai, robh+dt, matthias.bgg
  Cc: devicetree, alsa-devel, allen-kh.cheng, linux-kernel,
	linux-mediatek, trevor.wu, yc.hung, daniel.baluta,
	linux-arm-kernel

Add adsp clock on/off support on mt8195 platform.

Signed-off-by: YC Hung <yc.hung@mediatek.com>
---
 sound/soc/sof/mediatek/mt8195/Makefile     |   2 +-
 sound/soc/sof/mediatek/mt8195/mt8195-clk.c | 164 +++++++++++++++++++++
 sound/soc/sof/mediatek/mt8195/mt8195-clk.h |  29 ++++
 sound/soc/sof/mediatek/mt8195/mt8195.c     |  23 ++-
 4 files changed, 215 insertions(+), 3 deletions(-)
 create mode 100644 sound/soc/sof/mediatek/mt8195/mt8195-clk.c
 create mode 100644 sound/soc/sof/mediatek/mt8195/mt8195-clk.h

diff --git a/sound/soc/sof/mediatek/mt8195/Makefile b/sound/soc/sof/mediatek/mt8195/Makefile
index 60fca24c068a..650f4bce99b2 100644
--- a/sound/soc/sof/mediatek/mt8195/Makefile
+++ b/sound/soc/sof/mediatek/mt8195/Makefile
@@ -1,4 +1,4 @@
 # SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
-snd-sof-mt8195-objs := mt8195.o mt8195-loader.o
+snd-sof-mt8195-objs := mt8195.o mt8195-clk.o mt8195-loader.o
 obj-$(CONFIG_SND_SOC_SOF_MT8195) += snd-sof-mt8195.o
 
diff --git a/sound/soc/sof/mediatek/mt8195/mt8195-clk.c b/sound/soc/sof/mediatek/mt8195/mt8195-clk.c
new file mode 100644
index 000000000000..1988421f7f7b
--- /dev/null
+++ b/sound/soc/sof/mediatek/mt8195/mt8195-clk.c
@@ -0,0 +1,164 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
+//
+// Copyright(c) 2021 Mediatek Corporation. All rights reserved.
+//
+// Author: YC Hung <yc.hung@mediatek.com>
+//
+// Hardware interface for mt8195 DSP clock
+
+#include <linux/clk.h>
+#include <linux/pm_runtime.h>
+#include <linux/io.h>
+#include "mt8195.h"
+#include "mt8195-clk.h"
+
+struct clk *clk_handle[ADSP_CLK_NUM];
+
+int platform_parse_clock(struct device *dev)
+{
+	clk_handle[CLK_TOP_ADSP] = devm_clk_get(dev, "adsp_sel");
+	if (IS_ERR(clk_handle[CLK_TOP_ADSP])) {
+		dev_err(dev, "clk_get(\"adsp_sel\") failed\n");
+		return PTR_ERR(clk_handle[CLK_TOP_ADSP]);
+	}
+
+	clk_handle[CLK_TOP_CLK26M] = devm_clk_get(dev, "clk26m_ck");
+	if (IS_ERR(clk_handle[CLK_TOP_CLK26M])) {
+		dev_err(dev, "clk_get(\"clk26m_ck\") failed\n");
+		return PTR_ERR(clk_handle[CLK_TOP_CLK26M]);
+	}
+
+	clk_handle[CLK_TOP_AUDIO_LOCAL_BUS] = devm_clk_get(dev, "audio_local_bus");
+	if (IS_ERR(clk_handle[CLK_TOP_AUDIO_LOCAL_BUS])) {
+		dev_err(dev, "clk_get(\"audio_local_bus\") failed\n");
+		return PTR_ERR(clk_handle[CLK_TOP_AUDIO_LOCAL_BUS]);
+	}
+
+	clk_handle[CLK_TOP_MAINPLL_D7_D2] = devm_clk_get(dev, "mainpll_d7_d2");
+	if (IS_ERR(clk_handle[CLK_TOP_MAINPLL_D7_D2])) {
+		dev_err(dev, "clk_get(\"mainpll_d7_d2\") failed\n");
+		return PTR_ERR(clk_handle[CLK_TOP_MAINPLL_D7_D2]);
+	}
+
+	clk_handle[CLK_SCP_ADSP_AUDIODSP] = devm_clk_get(dev, "scp_adsp_audiodsp");
+	if (IS_ERR(clk_handle[CLK_SCP_ADSP_AUDIODSP])) {
+		dev_err(dev, "clk_get(\"scp_adsp_audiodsp\") failed\n");
+		return PTR_ERR(clk_handle[CLK_SCP_ADSP_AUDIODSP]);
+	}
+
+	clk_handle[CLK_TOP_AUDIO_H] = devm_clk_get(dev, "audio_h");
+	if (IS_ERR(clk_handle[CLK_TOP_AUDIO_H])) {
+		dev_err(dev, "clk_get(\"audio_h_sel\") failed\n");
+		return PTR_ERR(clk_handle[CLK_TOP_AUDIO_H]);
+	}
+
+	return 0;
+}
+
+int adsp_enable_clock(struct device *dev)
+{
+	int ret;
+
+	ret = clk_prepare_enable(clk_handle[CLK_TOP_MAINPLL_D7_D2]);
+	if (ret) {
+		dev_err(dev, "%s clk_prepare_enable(mainpll_d7_d2) fail %d\n",
+			__func__, ret);
+		return ret;
+	}
+
+	ret = clk_prepare_enable(clk_handle[CLK_TOP_ADSP]);
+	if (ret) {
+		dev_err(dev, "%s clk_prepare_enable(adsp_sel) fail %d\n",
+			__func__, ret);
+		goto disable_mainpll_d7_d2_clk;
+	}
+
+	ret = clk_prepare_enable(clk_handle[CLK_TOP_AUDIO_LOCAL_BUS]);
+	if (ret) {
+		dev_err(dev, "%s clk_prepare_enable(audio_local_bus) fail %d\n",
+			__func__, ret);
+		goto disable_dsp_sel_clk;
+	}
+
+	ret = clk_prepare_enable(clk_handle[CLK_SCP_ADSP_AUDIODSP]);
+	if (ret) {
+		dev_err(dev, "%s clk_prepare_enable(scp_adsp_audiodsp) fail %d\n",
+			__func__, ret);
+		goto disable_audio_local_bus_clk;
+	}
+
+	ret = clk_prepare_enable(clk_handle[CLK_TOP_AUDIO_H]);
+	if (ret) {
+		dev_err(dev, "%s clk_prepare_enable(audio_h) fail %d\n",
+			__func__, ret);
+		goto disable_scp_adsp_audiodsp_clk;
+	}
+
+	return 0;
+
+disable_scp_adsp_audiodsp_clk:
+	clk_disable_unprepare(clk_handle[CLK_SCP_ADSP_AUDIODSP]);
+disable_audio_local_bus_clk:
+	clk_disable_unprepare(clk_handle[CLK_TOP_AUDIO_LOCAL_BUS]);
+disable_dsp_sel_clk:
+	clk_disable_unprepare(clk_handle[CLK_TOP_ADSP]);
+disable_mainpll_d7_d2_clk:
+	clk_disable_unprepare(clk_handle[CLK_TOP_MAINPLL_D7_D2]);
+
+	return ret;
+}
+
+void adsp_disable_clock(struct device *dev)
+{
+	clk_disable_unprepare(clk_handle[CLK_TOP_AUDIO_H]);
+	clk_disable_unprepare(clk_handle[CLK_SCP_ADSP_AUDIODSP]);
+	clk_disable_unprepare(clk_handle[CLK_TOP_AUDIO_LOCAL_BUS]);
+	clk_disable_unprepare(clk_handle[CLK_TOP_ADSP]);
+	clk_disable_unprepare(clk_handle[CLK_TOP_MAINPLL_D7_D2]);
+}
+
+int adsp_default_clk_init(struct device *dev, int enable)
+{
+	int ret = 0;
+
+	dev_dbg(dev, "%s: %s\n", __func__, enable ? "on" : "off");
+
+	if (enable) {
+		ret = clk_set_parent(clk_handle[CLK_TOP_ADSP],
+				     clk_handle[CLK_TOP_CLK26M]);
+		if (ret) {
+			dev_err(dev, "failed to set dsp_sel to clk26m: %d\n", ret);
+			return ret;
+		}
+
+		ret = clk_set_parent(clk_handle[CLK_TOP_AUDIO_LOCAL_BUS],
+				     clk_handle[CLK_TOP_MAINPLL_D7_D2]);
+		if (ret) {
+			dev_err(dev, "set audio_local_bus failed %d\n", ret);
+			return ret;
+		}
+
+		ret = adsp_enable_clock(dev);
+		if (ret)
+			dev_err(dev, "failed to adsp_enable_clock: %d\n", ret);
+
+		return ret;
+	}
+
+	adsp_disable_clock(dev);
+
+	return ret;
+}
+
+int adsp_clock_on(struct device *dev)
+{
+	/* Open ADSP clock */
+	return adsp_default_clk_init(dev, 1);
+}
+
+int adsp_clock_off(struct device *dev)
+{
+	/* Close ADSP clock */
+	return adsp_default_clk_init(dev, 0);
+}
+
diff --git a/sound/soc/sof/mediatek/mt8195/mt8195-clk.h b/sound/soc/sof/mediatek/mt8195/mt8195-clk.h
new file mode 100644
index 000000000000..f985d141552a
--- /dev/null
+++ b/sound/soc/sof/mediatek/mt8195/mt8195-clk.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/*
+ * Copyright (c) 2021 MediaTek Corporation. All rights reserved.
+ *
+ *  Header file for the mt8195 DSP clock  definition
+ */
+
+#ifndef __MT8195_CLK_H
+#define __MT8195_CLK_H
+
+/*DSP clock*/
+enum ADSP_CLK_ID {
+	CLK_TOP_ADSP,
+	CLK_TOP_CLK26M,
+	CLK_TOP_AUDIO_LOCAL_BUS,
+	CLK_TOP_MAINPLL_D7_D2,
+	CLK_SCP_ADSP_AUDIODSP,
+	CLK_TOP_AUDIO_H,
+	ADSP_CLK_NUM
+};
+
+int platform_parse_clock(struct device *dev);
+int adsp_default_clk_init(struct device *dev, int enable);
+int adsp_enable_clock(struct device *dev);
+void adsp_disable_clock(struct device *dev);
+int adsp_clock_on(struct device *dev);
+int adsp_clock_off(struct device *dev);
+#endif
diff --git a/sound/soc/sof/mediatek/mt8195/mt8195.c b/sound/soc/sof/mediatek/mt8195/mt8195.c
index 99075598a35a..f323da58057b 100644
--- a/sound/soc/sof/mediatek/mt8195/mt8195.c
+++ b/sound/soc/sof/mediatek/mt8195/mt8195.c
@@ -25,6 +25,7 @@
 #include "../adsp_helper.h"
 #include "../mediatek-ops.h"
 #include "mt8195.h"
+#include "mt8195-clk.h"
 
 static int platform_parse_resource(struct platform_device *pdev, void *data)
 {
@@ -231,10 +232,23 @@ static int mt8195_dsp_probe(struct snd_sof_dev *sdev)
 	if (ret)
 		return ret;
 
+	ret = platform_parse_clock(&pdev->dev);
+	if (ret) {
+		dev_err(sdev->dev, "platform_parse_clock failed\n");
+		return -EINVAL;
+	}
+
+	ret = adsp_clock_on(&pdev->dev);
+	if (ret) {
+		dev_err(sdev->dev, "adsp_clock_on fail!\n");
+		return -EINVAL;
+	}
+
 	ret = adsp_sram_power_on(sdev->dev, true);
 	if (ret) {
 		dev_err(sdev->dev, "adsp_sram_power_on fail!\n");
-		return ret;
+		ret = -EINVAL;
+		goto exit_clk_disable;
 	}
 
 	ret = adsp_memory_remap_init(&pdev->dev, priv->adsp);
@@ -282,6 +296,8 @@ static int mt8195_dsp_probe(struct snd_sof_dev *sdev)
 
 err_adsp_sram_power_off:
 	adsp_sram_power_on(&pdev->dev, false);
+exit_clk_disable:
+	adsp_clock_off(&pdev->dev);
 
 	return ret;
 }
@@ -290,7 +306,10 @@ static int mt8195_dsp_remove(struct snd_sof_dev *sdev)
 {
 	struct platform_device *pdev = container_of(sdev->dev, struct platform_device, dev);
 
-	return adsp_sram_power_on(&pdev->dev, false);
+	adsp_sram_power_on(&pdev->dev, false);
+	adsp_clock_off(&pdev->dev);
+
+	return 0;
 }
 
 /* on mt8195 there is 1 to 1 match between type and BAR idx */
-- 
2.18.0


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

* [PATCH v2 1/2] ASoC: SOF: mediatek: Add mt8195 dsp clock support
@ 2021-10-21  3:58   ` YC Hung
  0 siblings, 0 replies; 15+ messages in thread
From: YC Hung @ 2021-10-21  3:58 UTC (permalink / raw)
  To: broonie, tiwai, robh+dt, matthias.bgg
  Cc: yc.hung, alsa-devel, linux-mediatek, linux-arm-kernel,
	linux-kernel, devicetree, daniel.baluta, trevor.wu,
	allen-kh.cheng

Add adsp clock on/off support on mt8195 platform.

Signed-off-by: YC Hung <yc.hung@mediatek.com>
---
 sound/soc/sof/mediatek/mt8195/Makefile     |   2 +-
 sound/soc/sof/mediatek/mt8195/mt8195-clk.c | 164 +++++++++++++++++++++
 sound/soc/sof/mediatek/mt8195/mt8195-clk.h |  29 ++++
 sound/soc/sof/mediatek/mt8195/mt8195.c     |  23 ++-
 4 files changed, 215 insertions(+), 3 deletions(-)
 create mode 100644 sound/soc/sof/mediatek/mt8195/mt8195-clk.c
 create mode 100644 sound/soc/sof/mediatek/mt8195/mt8195-clk.h

diff --git a/sound/soc/sof/mediatek/mt8195/Makefile b/sound/soc/sof/mediatek/mt8195/Makefile
index 60fca24c068a..650f4bce99b2 100644
--- a/sound/soc/sof/mediatek/mt8195/Makefile
+++ b/sound/soc/sof/mediatek/mt8195/Makefile
@@ -1,4 +1,4 @@
 # SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
-snd-sof-mt8195-objs := mt8195.o mt8195-loader.o
+snd-sof-mt8195-objs := mt8195.o mt8195-clk.o mt8195-loader.o
 obj-$(CONFIG_SND_SOC_SOF_MT8195) += snd-sof-mt8195.o
 
diff --git a/sound/soc/sof/mediatek/mt8195/mt8195-clk.c b/sound/soc/sof/mediatek/mt8195/mt8195-clk.c
new file mode 100644
index 000000000000..1988421f7f7b
--- /dev/null
+++ b/sound/soc/sof/mediatek/mt8195/mt8195-clk.c
@@ -0,0 +1,164 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
+//
+// Copyright(c) 2021 Mediatek Corporation. All rights reserved.
+//
+// Author: YC Hung <yc.hung@mediatek.com>
+//
+// Hardware interface for mt8195 DSP clock
+
+#include <linux/clk.h>
+#include <linux/pm_runtime.h>
+#include <linux/io.h>
+#include "mt8195.h"
+#include "mt8195-clk.h"
+
+struct clk *clk_handle[ADSP_CLK_NUM];
+
+int platform_parse_clock(struct device *dev)
+{
+	clk_handle[CLK_TOP_ADSP] = devm_clk_get(dev, "adsp_sel");
+	if (IS_ERR(clk_handle[CLK_TOP_ADSP])) {
+		dev_err(dev, "clk_get(\"adsp_sel\") failed\n");
+		return PTR_ERR(clk_handle[CLK_TOP_ADSP]);
+	}
+
+	clk_handle[CLK_TOP_CLK26M] = devm_clk_get(dev, "clk26m_ck");
+	if (IS_ERR(clk_handle[CLK_TOP_CLK26M])) {
+		dev_err(dev, "clk_get(\"clk26m_ck\") failed\n");
+		return PTR_ERR(clk_handle[CLK_TOP_CLK26M]);
+	}
+
+	clk_handle[CLK_TOP_AUDIO_LOCAL_BUS] = devm_clk_get(dev, "audio_local_bus");
+	if (IS_ERR(clk_handle[CLK_TOP_AUDIO_LOCAL_BUS])) {
+		dev_err(dev, "clk_get(\"audio_local_bus\") failed\n");
+		return PTR_ERR(clk_handle[CLK_TOP_AUDIO_LOCAL_BUS]);
+	}
+
+	clk_handle[CLK_TOP_MAINPLL_D7_D2] = devm_clk_get(dev, "mainpll_d7_d2");
+	if (IS_ERR(clk_handle[CLK_TOP_MAINPLL_D7_D2])) {
+		dev_err(dev, "clk_get(\"mainpll_d7_d2\") failed\n");
+		return PTR_ERR(clk_handle[CLK_TOP_MAINPLL_D7_D2]);
+	}
+
+	clk_handle[CLK_SCP_ADSP_AUDIODSP] = devm_clk_get(dev, "scp_adsp_audiodsp");
+	if (IS_ERR(clk_handle[CLK_SCP_ADSP_AUDIODSP])) {
+		dev_err(dev, "clk_get(\"scp_adsp_audiodsp\") failed\n");
+		return PTR_ERR(clk_handle[CLK_SCP_ADSP_AUDIODSP]);
+	}
+
+	clk_handle[CLK_TOP_AUDIO_H] = devm_clk_get(dev, "audio_h");
+	if (IS_ERR(clk_handle[CLK_TOP_AUDIO_H])) {
+		dev_err(dev, "clk_get(\"audio_h_sel\") failed\n");
+		return PTR_ERR(clk_handle[CLK_TOP_AUDIO_H]);
+	}
+
+	return 0;
+}
+
+int adsp_enable_clock(struct device *dev)
+{
+	int ret;
+
+	ret = clk_prepare_enable(clk_handle[CLK_TOP_MAINPLL_D7_D2]);
+	if (ret) {
+		dev_err(dev, "%s clk_prepare_enable(mainpll_d7_d2) fail %d\n",
+			__func__, ret);
+		return ret;
+	}
+
+	ret = clk_prepare_enable(clk_handle[CLK_TOP_ADSP]);
+	if (ret) {
+		dev_err(dev, "%s clk_prepare_enable(adsp_sel) fail %d\n",
+			__func__, ret);
+		goto disable_mainpll_d7_d2_clk;
+	}
+
+	ret = clk_prepare_enable(clk_handle[CLK_TOP_AUDIO_LOCAL_BUS]);
+	if (ret) {
+		dev_err(dev, "%s clk_prepare_enable(audio_local_bus) fail %d\n",
+			__func__, ret);
+		goto disable_dsp_sel_clk;
+	}
+
+	ret = clk_prepare_enable(clk_handle[CLK_SCP_ADSP_AUDIODSP]);
+	if (ret) {
+		dev_err(dev, "%s clk_prepare_enable(scp_adsp_audiodsp) fail %d\n",
+			__func__, ret);
+		goto disable_audio_local_bus_clk;
+	}
+
+	ret = clk_prepare_enable(clk_handle[CLK_TOP_AUDIO_H]);
+	if (ret) {
+		dev_err(dev, "%s clk_prepare_enable(audio_h) fail %d\n",
+			__func__, ret);
+		goto disable_scp_adsp_audiodsp_clk;
+	}
+
+	return 0;
+
+disable_scp_adsp_audiodsp_clk:
+	clk_disable_unprepare(clk_handle[CLK_SCP_ADSP_AUDIODSP]);
+disable_audio_local_bus_clk:
+	clk_disable_unprepare(clk_handle[CLK_TOP_AUDIO_LOCAL_BUS]);
+disable_dsp_sel_clk:
+	clk_disable_unprepare(clk_handle[CLK_TOP_ADSP]);
+disable_mainpll_d7_d2_clk:
+	clk_disable_unprepare(clk_handle[CLK_TOP_MAINPLL_D7_D2]);
+
+	return ret;
+}
+
+void adsp_disable_clock(struct device *dev)
+{
+	clk_disable_unprepare(clk_handle[CLK_TOP_AUDIO_H]);
+	clk_disable_unprepare(clk_handle[CLK_SCP_ADSP_AUDIODSP]);
+	clk_disable_unprepare(clk_handle[CLK_TOP_AUDIO_LOCAL_BUS]);
+	clk_disable_unprepare(clk_handle[CLK_TOP_ADSP]);
+	clk_disable_unprepare(clk_handle[CLK_TOP_MAINPLL_D7_D2]);
+}
+
+int adsp_default_clk_init(struct device *dev, int enable)
+{
+	int ret = 0;
+
+	dev_dbg(dev, "%s: %s\n", __func__, enable ? "on" : "off");
+
+	if (enable) {
+		ret = clk_set_parent(clk_handle[CLK_TOP_ADSP],
+				     clk_handle[CLK_TOP_CLK26M]);
+		if (ret) {
+			dev_err(dev, "failed to set dsp_sel to clk26m: %d\n", ret);
+			return ret;
+		}
+
+		ret = clk_set_parent(clk_handle[CLK_TOP_AUDIO_LOCAL_BUS],
+				     clk_handle[CLK_TOP_MAINPLL_D7_D2]);
+		if (ret) {
+			dev_err(dev, "set audio_local_bus failed %d\n", ret);
+			return ret;
+		}
+
+		ret = adsp_enable_clock(dev);
+		if (ret)
+			dev_err(dev, "failed to adsp_enable_clock: %d\n", ret);
+
+		return ret;
+	}
+
+	adsp_disable_clock(dev);
+
+	return ret;
+}
+
+int adsp_clock_on(struct device *dev)
+{
+	/* Open ADSP clock */
+	return adsp_default_clk_init(dev, 1);
+}
+
+int adsp_clock_off(struct device *dev)
+{
+	/* Close ADSP clock */
+	return adsp_default_clk_init(dev, 0);
+}
+
diff --git a/sound/soc/sof/mediatek/mt8195/mt8195-clk.h b/sound/soc/sof/mediatek/mt8195/mt8195-clk.h
new file mode 100644
index 000000000000..f985d141552a
--- /dev/null
+++ b/sound/soc/sof/mediatek/mt8195/mt8195-clk.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/*
+ * Copyright (c) 2021 MediaTek Corporation. All rights reserved.
+ *
+ *  Header file for the mt8195 DSP clock  definition
+ */
+
+#ifndef __MT8195_CLK_H
+#define __MT8195_CLK_H
+
+/*DSP clock*/
+enum ADSP_CLK_ID {
+	CLK_TOP_ADSP,
+	CLK_TOP_CLK26M,
+	CLK_TOP_AUDIO_LOCAL_BUS,
+	CLK_TOP_MAINPLL_D7_D2,
+	CLK_SCP_ADSP_AUDIODSP,
+	CLK_TOP_AUDIO_H,
+	ADSP_CLK_NUM
+};
+
+int platform_parse_clock(struct device *dev);
+int adsp_default_clk_init(struct device *dev, int enable);
+int adsp_enable_clock(struct device *dev);
+void adsp_disable_clock(struct device *dev);
+int adsp_clock_on(struct device *dev);
+int adsp_clock_off(struct device *dev);
+#endif
diff --git a/sound/soc/sof/mediatek/mt8195/mt8195.c b/sound/soc/sof/mediatek/mt8195/mt8195.c
index 99075598a35a..f323da58057b 100644
--- a/sound/soc/sof/mediatek/mt8195/mt8195.c
+++ b/sound/soc/sof/mediatek/mt8195/mt8195.c
@@ -25,6 +25,7 @@
 #include "../adsp_helper.h"
 #include "../mediatek-ops.h"
 #include "mt8195.h"
+#include "mt8195-clk.h"
 
 static int platform_parse_resource(struct platform_device *pdev, void *data)
 {
@@ -231,10 +232,23 @@ static int mt8195_dsp_probe(struct snd_sof_dev *sdev)
 	if (ret)
 		return ret;
 
+	ret = platform_parse_clock(&pdev->dev);
+	if (ret) {
+		dev_err(sdev->dev, "platform_parse_clock failed\n");
+		return -EINVAL;
+	}
+
+	ret = adsp_clock_on(&pdev->dev);
+	if (ret) {
+		dev_err(sdev->dev, "adsp_clock_on fail!\n");
+		return -EINVAL;
+	}
+
 	ret = adsp_sram_power_on(sdev->dev, true);
 	if (ret) {
 		dev_err(sdev->dev, "adsp_sram_power_on fail!\n");
-		return ret;
+		ret = -EINVAL;
+		goto exit_clk_disable;
 	}
 
 	ret = adsp_memory_remap_init(&pdev->dev, priv->adsp);
@@ -282,6 +296,8 @@ static int mt8195_dsp_probe(struct snd_sof_dev *sdev)
 
 err_adsp_sram_power_off:
 	adsp_sram_power_on(&pdev->dev, false);
+exit_clk_disable:
+	adsp_clock_off(&pdev->dev);
 
 	return ret;
 }
@@ -290,7 +306,10 @@ static int mt8195_dsp_remove(struct snd_sof_dev *sdev)
 {
 	struct platform_device *pdev = container_of(sdev->dev, struct platform_device, dev);
 
-	return adsp_sram_power_on(&pdev->dev, false);
+	adsp_sram_power_on(&pdev->dev, false);
+	adsp_clock_off(&pdev->dev);
+
+	return 0;
 }
 
 /* on mt8195 there is 1 to 1 match between type and BAR idx */
-- 
2.18.0


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

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

* [PATCH v2 1/2] ASoC: SOF: mediatek: Add mt8195 dsp clock support
@ 2021-10-21  3:58   ` YC Hung
  0 siblings, 0 replies; 15+ messages in thread
From: YC Hung @ 2021-10-21  3:58 UTC (permalink / raw)
  To: broonie, tiwai, robh+dt, matthias.bgg
  Cc: yc.hung, alsa-devel, linux-mediatek, linux-arm-kernel,
	linux-kernel, devicetree, daniel.baluta, trevor.wu,
	allen-kh.cheng

Add adsp clock on/off support on mt8195 platform.

Signed-off-by: YC Hung <yc.hung@mediatek.com>
---
 sound/soc/sof/mediatek/mt8195/Makefile     |   2 +-
 sound/soc/sof/mediatek/mt8195/mt8195-clk.c | 164 +++++++++++++++++++++
 sound/soc/sof/mediatek/mt8195/mt8195-clk.h |  29 ++++
 sound/soc/sof/mediatek/mt8195/mt8195.c     |  23 ++-
 4 files changed, 215 insertions(+), 3 deletions(-)
 create mode 100644 sound/soc/sof/mediatek/mt8195/mt8195-clk.c
 create mode 100644 sound/soc/sof/mediatek/mt8195/mt8195-clk.h

diff --git a/sound/soc/sof/mediatek/mt8195/Makefile b/sound/soc/sof/mediatek/mt8195/Makefile
index 60fca24c068a..650f4bce99b2 100644
--- a/sound/soc/sof/mediatek/mt8195/Makefile
+++ b/sound/soc/sof/mediatek/mt8195/Makefile
@@ -1,4 +1,4 @@
 # SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
-snd-sof-mt8195-objs := mt8195.o mt8195-loader.o
+snd-sof-mt8195-objs := mt8195.o mt8195-clk.o mt8195-loader.o
 obj-$(CONFIG_SND_SOC_SOF_MT8195) += snd-sof-mt8195.o
 
diff --git a/sound/soc/sof/mediatek/mt8195/mt8195-clk.c b/sound/soc/sof/mediatek/mt8195/mt8195-clk.c
new file mode 100644
index 000000000000..1988421f7f7b
--- /dev/null
+++ b/sound/soc/sof/mediatek/mt8195/mt8195-clk.c
@@ -0,0 +1,164 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
+//
+// Copyright(c) 2021 Mediatek Corporation. All rights reserved.
+//
+// Author: YC Hung <yc.hung@mediatek.com>
+//
+// Hardware interface for mt8195 DSP clock
+
+#include <linux/clk.h>
+#include <linux/pm_runtime.h>
+#include <linux/io.h>
+#include "mt8195.h"
+#include "mt8195-clk.h"
+
+struct clk *clk_handle[ADSP_CLK_NUM];
+
+int platform_parse_clock(struct device *dev)
+{
+	clk_handle[CLK_TOP_ADSP] = devm_clk_get(dev, "adsp_sel");
+	if (IS_ERR(clk_handle[CLK_TOP_ADSP])) {
+		dev_err(dev, "clk_get(\"adsp_sel\") failed\n");
+		return PTR_ERR(clk_handle[CLK_TOP_ADSP]);
+	}
+
+	clk_handle[CLK_TOP_CLK26M] = devm_clk_get(dev, "clk26m_ck");
+	if (IS_ERR(clk_handle[CLK_TOP_CLK26M])) {
+		dev_err(dev, "clk_get(\"clk26m_ck\") failed\n");
+		return PTR_ERR(clk_handle[CLK_TOP_CLK26M]);
+	}
+
+	clk_handle[CLK_TOP_AUDIO_LOCAL_BUS] = devm_clk_get(dev, "audio_local_bus");
+	if (IS_ERR(clk_handle[CLK_TOP_AUDIO_LOCAL_BUS])) {
+		dev_err(dev, "clk_get(\"audio_local_bus\") failed\n");
+		return PTR_ERR(clk_handle[CLK_TOP_AUDIO_LOCAL_BUS]);
+	}
+
+	clk_handle[CLK_TOP_MAINPLL_D7_D2] = devm_clk_get(dev, "mainpll_d7_d2");
+	if (IS_ERR(clk_handle[CLK_TOP_MAINPLL_D7_D2])) {
+		dev_err(dev, "clk_get(\"mainpll_d7_d2\") failed\n");
+		return PTR_ERR(clk_handle[CLK_TOP_MAINPLL_D7_D2]);
+	}
+
+	clk_handle[CLK_SCP_ADSP_AUDIODSP] = devm_clk_get(dev, "scp_adsp_audiodsp");
+	if (IS_ERR(clk_handle[CLK_SCP_ADSP_AUDIODSP])) {
+		dev_err(dev, "clk_get(\"scp_adsp_audiodsp\") failed\n");
+		return PTR_ERR(clk_handle[CLK_SCP_ADSP_AUDIODSP]);
+	}
+
+	clk_handle[CLK_TOP_AUDIO_H] = devm_clk_get(dev, "audio_h");
+	if (IS_ERR(clk_handle[CLK_TOP_AUDIO_H])) {
+		dev_err(dev, "clk_get(\"audio_h_sel\") failed\n");
+		return PTR_ERR(clk_handle[CLK_TOP_AUDIO_H]);
+	}
+
+	return 0;
+}
+
+int adsp_enable_clock(struct device *dev)
+{
+	int ret;
+
+	ret = clk_prepare_enable(clk_handle[CLK_TOP_MAINPLL_D7_D2]);
+	if (ret) {
+		dev_err(dev, "%s clk_prepare_enable(mainpll_d7_d2) fail %d\n",
+			__func__, ret);
+		return ret;
+	}
+
+	ret = clk_prepare_enable(clk_handle[CLK_TOP_ADSP]);
+	if (ret) {
+		dev_err(dev, "%s clk_prepare_enable(adsp_sel) fail %d\n",
+			__func__, ret);
+		goto disable_mainpll_d7_d2_clk;
+	}
+
+	ret = clk_prepare_enable(clk_handle[CLK_TOP_AUDIO_LOCAL_BUS]);
+	if (ret) {
+		dev_err(dev, "%s clk_prepare_enable(audio_local_bus) fail %d\n",
+			__func__, ret);
+		goto disable_dsp_sel_clk;
+	}
+
+	ret = clk_prepare_enable(clk_handle[CLK_SCP_ADSP_AUDIODSP]);
+	if (ret) {
+		dev_err(dev, "%s clk_prepare_enable(scp_adsp_audiodsp) fail %d\n",
+			__func__, ret);
+		goto disable_audio_local_bus_clk;
+	}
+
+	ret = clk_prepare_enable(clk_handle[CLK_TOP_AUDIO_H]);
+	if (ret) {
+		dev_err(dev, "%s clk_prepare_enable(audio_h) fail %d\n",
+			__func__, ret);
+		goto disable_scp_adsp_audiodsp_clk;
+	}
+
+	return 0;
+
+disable_scp_adsp_audiodsp_clk:
+	clk_disable_unprepare(clk_handle[CLK_SCP_ADSP_AUDIODSP]);
+disable_audio_local_bus_clk:
+	clk_disable_unprepare(clk_handle[CLK_TOP_AUDIO_LOCAL_BUS]);
+disable_dsp_sel_clk:
+	clk_disable_unprepare(clk_handle[CLK_TOP_ADSP]);
+disable_mainpll_d7_d2_clk:
+	clk_disable_unprepare(clk_handle[CLK_TOP_MAINPLL_D7_D2]);
+
+	return ret;
+}
+
+void adsp_disable_clock(struct device *dev)
+{
+	clk_disable_unprepare(clk_handle[CLK_TOP_AUDIO_H]);
+	clk_disable_unprepare(clk_handle[CLK_SCP_ADSP_AUDIODSP]);
+	clk_disable_unprepare(clk_handle[CLK_TOP_AUDIO_LOCAL_BUS]);
+	clk_disable_unprepare(clk_handle[CLK_TOP_ADSP]);
+	clk_disable_unprepare(clk_handle[CLK_TOP_MAINPLL_D7_D2]);
+}
+
+int adsp_default_clk_init(struct device *dev, int enable)
+{
+	int ret = 0;
+
+	dev_dbg(dev, "%s: %s\n", __func__, enable ? "on" : "off");
+
+	if (enable) {
+		ret = clk_set_parent(clk_handle[CLK_TOP_ADSP],
+				     clk_handle[CLK_TOP_CLK26M]);
+		if (ret) {
+			dev_err(dev, "failed to set dsp_sel to clk26m: %d\n", ret);
+			return ret;
+		}
+
+		ret = clk_set_parent(clk_handle[CLK_TOP_AUDIO_LOCAL_BUS],
+				     clk_handle[CLK_TOP_MAINPLL_D7_D2]);
+		if (ret) {
+			dev_err(dev, "set audio_local_bus failed %d\n", ret);
+			return ret;
+		}
+
+		ret = adsp_enable_clock(dev);
+		if (ret)
+			dev_err(dev, "failed to adsp_enable_clock: %d\n", ret);
+
+		return ret;
+	}
+
+	adsp_disable_clock(dev);
+
+	return ret;
+}
+
+int adsp_clock_on(struct device *dev)
+{
+	/* Open ADSP clock */
+	return adsp_default_clk_init(dev, 1);
+}
+
+int adsp_clock_off(struct device *dev)
+{
+	/* Close ADSP clock */
+	return adsp_default_clk_init(dev, 0);
+}
+
diff --git a/sound/soc/sof/mediatek/mt8195/mt8195-clk.h b/sound/soc/sof/mediatek/mt8195/mt8195-clk.h
new file mode 100644
index 000000000000..f985d141552a
--- /dev/null
+++ b/sound/soc/sof/mediatek/mt8195/mt8195-clk.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/*
+ * Copyright (c) 2021 MediaTek Corporation. All rights reserved.
+ *
+ *  Header file for the mt8195 DSP clock  definition
+ */
+
+#ifndef __MT8195_CLK_H
+#define __MT8195_CLK_H
+
+/*DSP clock*/
+enum ADSP_CLK_ID {
+	CLK_TOP_ADSP,
+	CLK_TOP_CLK26M,
+	CLK_TOP_AUDIO_LOCAL_BUS,
+	CLK_TOP_MAINPLL_D7_D2,
+	CLK_SCP_ADSP_AUDIODSP,
+	CLK_TOP_AUDIO_H,
+	ADSP_CLK_NUM
+};
+
+int platform_parse_clock(struct device *dev);
+int adsp_default_clk_init(struct device *dev, int enable);
+int adsp_enable_clock(struct device *dev);
+void adsp_disable_clock(struct device *dev);
+int adsp_clock_on(struct device *dev);
+int adsp_clock_off(struct device *dev);
+#endif
diff --git a/sound/soc/sof/mediatek/mt8195/mt8195.c b/sound/soc/sof/mediatek/mt8195/mt8195.c
index 99075598a35a..f323da58057b 100644
--- a/sound/soc/sof/mediatek/mt8195/mt8195.c
+++ b/sound/soc/sof/mediatek/mt8195/mt8195.c
@@ -25,6 +25,7 @@
 #include "../adsp_helper.h"
 #include "../mediatek-ops.h"
 #include "mt8195.h"
+#include "mt8195-clk.h"
 
 static int platform_parse_resource(struct platform_device *pdev, void *data)
 {
@@ -231,10 +232,23 @@ static int mt8195_dsp_probe(struct snd_sof_dev *sdev)
 	if (ret)
 		return ret;
 
+	ret = platform_parse_clock(&pdev->dev);
+	if (ret) {
+		dev_err(sdev->dev, "platform_parse_clock failed\n");
+		return -EINVAL;
+	}
+
+	ret = adsp_clock_on(&pdev->dev);
+	if (ret) {
+		dev_err(sdev->dev, "adsp_clock_on fail!\n");
+		return -EINVAL;
+	}
+
 	ret = adsp_sram_power_on(sdev->dev, true);
 	if (ret) {
 		dev_err(sdev->dev, "adsp_sram_power_on fail!\n");
-		return ret;
+		ret = -EINVAL;
+		goto exit_clk_disable;
 	}
 
 	ret = adsp_memory_remap_init(&pdev->dev, priv->adsp);
@@ -282,6 +296,8 @@ static int mt8195_dsp_probe(struct snd_sof_dev *sdev)
 
 err_adsp_sram_power_off:
 	adsp_sram_power_on(&pdev->dev, false);
+exit_clk_disable:
+	adsp_clock_off(&pdev->dev);
 
 	return ret;
 }
@@ -290,7 +306,10 @@ static int mt8195_dsp_remove(struct snd_sof_dev *sdev)
 {
 	struct platform_device *pdev = container_of(sdev->dev, struct platform_device, dev);
 
-	return adsp_sram_power_on(&pdev->dev, false);
+	adsp_sram_power_on(&pdev->dev, false);
+	adsp_clock_off(&pdev->dev);
+
+	return 0;
 }
 
 /* on mt8195 there is 1 to 1 match between type and BAR idx */
-- 
2.18.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH v2 2/2] dt-bindings: dsp: mediatek: Add mt8195 DSP binding support
  2021-10-21  3:58 ` YC Hung
  (?)
  (?)
@ 2021-10-21  3:58   ` YC Hung
  -1 siblings, 0 replies; 15+ messages in thread
From: YC Hung @ 2021-10-21  3:58 UTC (permalink / raw)
  To: broonie, tiwai, robh+dt, matthias.bgg
  Cc: yc.hung, alsa-devel, linux-mediatek, linux-arm-kernel,
	linux-kernel, devicetree, daniel.baluta, trevor.wu,
	allen-kh.cheng

This describes the mt8195 DSP device tree node.

Signed-off-by: YC Hung <yc.hung@mediatek.com>
---
 .../bindings/dsp/mtk,mt8195-dsp.yaml          | 139 ++++++++++++++++++
 1 file changed, 139 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/dsp/mtk,mt8195-dsp.yaml

diff --git a/Documentation/devicetree/bindings/dsp/mtk,mt8195-dsp.yaml b/Documentation/devicetree/bindings/dsp/mtk,mt8195-dsp.yaml
new file mode 100644
index 000000000000..f113f71ca094
--- /dev/null
+++ b/Documentation/devicetree/bindings/dsp/mtk,mt8195-dsp.yaml
@@ -0,0 +1,139 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/dsp/mtk,mt8195-dsp.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Mediatek mt8195 DSP core
+
+maintainers:
+  - YC Hung <yc.hung@mediatek.com>
+
+description: |
+  Some boards from mt8195 contain a DSP core used for
+  advanced pre- and post- audio processing.
+properties:
+  compatible:
+    const: mediatek,mt8195-dsp
+
+  reg:
+    maxItems: 2
+
+  reg-names:
+    maxItems: 2
+
+  interrupts:
+    maxItems: 1
+
+  interrupt-names:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: mux for audio dsp clock
+      - description: 26M clock
+      - description: mux for audio dsp local bus
+      - description: default audio dsp local bus clock source
+      - description: clock gate for audio dsp clock
+      - description: mux for audio dsp access external bus
+
+  clock-names:
+    items:
+      - const: adsp_sel
+      - const: clk26m_ck
+      - const: audio_local_bus
+      - const: mainpll_d7_d2
+      - const: scp_adsp_audiodsp
+      - const: audio_h
+
+  power-domains:
+    maxItems: 1
+
+  mboxes:
+    maxItems: 2
+
+  mbox-names:
+    description:
+      Specifies the mailboxes used to communicate with audio DSP
+    items:
+      - const: mbox0
+      - const: mbox1
+
+  memory-region:
+    description:
+      phandle to a node describing reserved memory (System RAM memory)
+      used by DSP (see bindings/reserved-memory/reserved-memory.txt)
+    maxItems: 2
+
+  sound:
+    description:
+      Sound subnode includes ASoC platform, DPTx codec node, and
+      HDMI codec node.
+
+    type: object
+
+    properties:
+      mediatek,platform:
+        $ref: "/schemas/types.yaml#/definitions/phandle"
+        description: The phandle of MT8195 ASoC platform.
+
+      mediatek,dptx-codec:
+        $ref: "/schemas/types.yaml#/definitions/phandle"
+        description: The phandle of MT8195 Display Port Tx codec node.
+
+      mediatek,hdmi-codec:
+        $ref: "/schemas/types.yaml#/definitions/phandle"
+        description: The phandle of MT8195 HDMI codec node.
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - interrupts
+  - interrupt-names
+  - clocks
+  - clock-names
+  - memory-region
+  - power-domains
+  - mbox-names
+  - mboxes
+  - sound
+
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    adsp: adsp@10803000 {
+       compatible =  "mediatek,mt8195-dsp";
+       reg = <0x10803000  0x1000>,
+             <0x10840000  0x40000>;
+       reg-names = "cfg", "sram";
+       interrupts = <GIC_SPI 694 IRQ_TYPE_LEVEL_HIGH 0>;
+       interrupt-names = "wdt";
+       clocks = <&topckgen 10>, //CLK_TOP_ADSP
+                <&clk26m>,
+                <&topckgen 107>, //CLK_TOP_AUDIO_LOCAL_BUS
+                <&topckgen 136>, //CLK_TOP_MAINPLL_D7_D2
+                <&scp_adsp 0>, //CLK_SCP_ADSP_AUDIODSP
+                <&topckgen 34>; //CLK_TOP_AUDIO_H
+       clock-names = "adsp_sel",
+                     "clk26m_ck",
+                     "audio_local_bus",
+                     "mainpll_d7_d2",
+                     "scp_adsp_audiodsp",
+                     "audio_h";
+       memory-region = <&adsp_dma_mem_reserved>,
+                       <&adsp_mem_reserved>;
+       power-domains = <&spm 6>; //MT8195_POWER_DOMAIN_ADSP
+       mbox-names = "mbox0", "mbox1";
+       mboxes = <&adsp_mailbox 0>, <&adsp_mailbox 1>;
+       status = "disabled";
+       sound {
+              mediatek,dptx-codec = <&dp_tx>;
+              mediatek,hdmi-codec = <&hdmi0>;
+              mediatek,platform = <&afe>;
+             };
+       };
-- 
2.18.0


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

* [PATCH v2 2/2] dt-bindings: dsp: mediatek: Add mt8195 DSP binding support
@ 2021-10-21  3:58   ` YC Hung
  0 siblings, 0 replies; 15+ messages in thread
From: YC Hung @ 2021-10-21  3:58 UTC (permalink / raw)
  To: broonie, tiwai, robh+dt, matthias.bgg
  Cc: devicetree, alsa-devel, allen-kh.cheng, linux-kernel,
	linux-mediatek, trevor.wu, yc.hung, daniel.baluta,
	linux-arm-kernel

This describes the mt8195 DSP device tree node.

Signed-off-by: YC Hung <yc.hung@mediatek.com>
---
 .../bindings/dsp/mtk,mt8195-dsp.yaml          | 139 ++++++++++++++++++
 1 file changed, 139 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/dsp/mtk,mt8195-dsp.yaml

diff --git a/Documentation/devicetree/bindings/dsp/mtk,mt8195-dsp.yaml b/Documentation/devicetree/bindings/dsp/mtk,mt8195-dsp.yaml
new file mode 100644
index 000000000000..f113f71ca094
--- /dev/null
+++ b/Documentation/devicetree/bindings/dsp/mtk,mt8195-dsp.yaml
@@ -0,0 +1,139 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/dsp/mtk,mt8195-dsp.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Mediatek mt8195 DSP core
+
+maintainers:
+  - YC Hung <yc.hung@mediatek.com>
+
+description: |
+  Some boards from mt8195 contain a DSP core used for
+  advanced pre- and post- audio processing.
+properties:
+  compatible:
+    const: mediatek,mt8195-dsp
+
+  reg:
+    maxItems: 2
+
+  reg-names:
+    maxItems: 2
+
+  interrupts:
+    maxItems: 1
+
+  interrupt-names:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: mux for audio dsp clock
+      - description: 26M clock
+      - description: mux for audio dsp local bus
+      - description: default audio dsp local bus clock source
+      - description: clock gate for audio dsp clock
+      - description: mux for audio dsp access external bus
+
+  clock-names:
+    items:
+      - const: adsp_sel
+      - const: clk26m_ck
+      - const: audio_local_bus
+      - const: mainpll_d7_d2
+      - const: scp_adsp_audiodsp
+      - const: audio_h
+
+  power-domains:
+    maxItems: 1
+
+  mboxes:
+    maxItems: 2
+
+  mbox-names:
+    description:
+      Specifies the mailboxes used to communicate with audio DSP
+    items:
+      - const: mbox0
+      - const: mbox1
+
+  memory-region:
+    description:
+      phandle to a node describing reserved memory (System RAM memory)
+      used by DSP (see bindings/reserved-memory/reserved-memory.txt)
+    maxItems: 2
+
+  sound:
+    description:
+      Sound subnode includes ASoC platform, DPTx codec node, and
+      HDMI codec node.
+
+    type: object
+
+    properties:
+      mediatek,platform:
+        $ref: "/schemas/types.yaml#/definitions/phandle"
+        description: The phandle of MT8195 ASoC platform.
+
+      mediatek,dptx-codec:
+        $ref: "/schemas/types.yaml#/definitions/phandle"
+        description: The phandle of MT8195 Display Port Tx codec node.
+
+      mediatek,hdmi-codec:
+        $ref: "/schemas/types.yaml#/definitions/phandle"
+        description: The phandle of MT8195 HDMI codec node.
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - interrupts
+  - interrupt-names
+  - clocks
+  - clock-names
+  - memory-region
+  - power-domains
+  - mbox-names
+  - mboxes
+  - sound
+
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    adsp: adsp@10803000 {
+       compatible =  "mediatek,mt8195-dsp";
+       reg = <0x10803000  0x1000>,
+             <0x10840000  0x40000>;
+       reg-names = "cfg", "sram";
+       interrupts = <GIC_SPI 694 IRQ_TYPE_LEVEL_HIGH 0>;
+       interrupt-names = "wdt";
+       clocks = <&topckgen 10>, //CLK_TOP_ADSP
+                <&clk26m>,
+                <&topckgen 107>, //CLK_TOP_AUDIO_LOCAL_BUS
+                <&topckgen 136>, //CLK_TOP_MAINPLL_D7_D2
+                <&scp_adsp 0>, //CLK_SCP_ADSP_AUDIODSP
+                <&topckgen 34>; //CLK_TOP_AUDIO_H
+       clock-names = "adsp_sel",
+                     "clk26m_ck",
+                     "audio_local_bus",
+                     "mainpll_d7_d2",
+                     "scp_adsp_audiodsp",
+                     "audio_h";
+       memory-region = <&adsp_dma_mem_reserved>,
+                       <&adsp_mem_reserved>;
+       power-domains = <&spm 6>; //MT8195_POWER_DOMAIN_ADSP
+       mbox-names = "mbox0", "mbox1";
+       mboxes = <&adsp_mailbox 0>, <&adsp_mailbox 1>;
+       status = "disabled";
+       sound {
+              mediatek,dptx-codec = <&dp_tx>;
+              mediatek,hdmi-codec = <&hdmi0>;
+              mediatek,platform = <&afe>;
+             };
+       };
-- 
2.18.0


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

* [PATCH v2 2/2] dt-bindings: dsp: mediatek: Add mt8195 DSP binding support
@ 2021-10-21  3:58   ` YC Hung
  0 siblings, 0 replies; 15+ messages in thread
From: YC Hung @ 2021-10-21  3:58 UTC (permalink / raw)
  To: broonie, tiwai, robh+dt, matthias.bgg
  Cc: yc.hung, alsa-devel, linux-mediatek, linux-arm-kernel,
	linux-kernel, devicetree, daniel.baluta, trevor.wu,
	allen-kh.cheng

This describes the mt8195 DSP device tree node.

Signed-off-by: YC Hung <yc.hung@mediatek.com>
---
 .../bindings/dsp/mtk,mt8195-dsp.yaml          | 139 ++++++++++++++++++
 1 file changed, 139 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/dsp/mtk,mt8195-dsp.yaml

diff --git a/Documentation/devicetree/bindings/dsp/mtk,mt8195-dsp.yaml b/Documentation/devicetree/bindings/dsp/mtk,mt8195-dsp.yaml
new file mode 100644
index 000000000000..f113f71ca094
--- /dev/null
+++ b/Documentation/devicetree/bindings/dsp/mtk,mt8195-dsp.yaml
@@ -0,0 +1,139 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/dsp/mtk,mt8195-dsp.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Mediatek mt8195 DSP core
+
+maintainers:
+  - YC Hung <yc.hung@mediatek.com>
+
+description: |
+  Some boards from mt8195 contain a DSP core used for
+  advanced pre- and post- audio processing.
+properties:
+  compatible:
+    const: mediatek,mt8195-dsp
+
+  reg:
+    maxItems: 2
+
+  reg-names:
+    maxItems: 2
+
+  interrupts:
+    maxItems: 1
+
+  interrupt-names:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: mux for audio dsp clock
+      - description: 26M clock
+      - description: mux for audio dsp local bus
+      - description: default audio dsp local bus clock source
+      - description: clock gate for audio dsp clock
+      - description: mux for audio dsp access external bus
+
+  clock-names:
+    items:
+      - const: adsp_sel
+      - const: clk26m_ck
+      - const: audio_local_bus
+      - const: mainpll_d7_d2
+      - const: scp_adsp_audiodsp
+      - const: audio_h
+
+  power-domains:
+    maxItems: 1
+
+  mboxes:
+    maxItems: 2
+
+  mbox-names:
+    description:
+      Specifies the mailboxes used to communicate with audio DSP
+    items:
+      - const: mbox0
+      - const: mbox1
+
+  memory-region:
+    description:
+      phandle to a node describing reserved memory (System RAM memory)
+      used by DSP (see bindings/reserved-memory/reserved-memory.txt)
+    maxItems: 2
+
+  sound:
+    description:
+      Sound subnode includes ASoC platform, DPTx codec node, and
+      HDMI codec node.
+
+    type: object
+
+    properties:
+      mediatek,platform:
+        $ref: "/schemas/types.yaml#/definitions/phandle"
+        description: The phandle of MT8195 ASoC platform.
+
+      mediatek,dptx-codec:
+        $ref: "/schemas/types.yaml#/definitions/phandle"
+        description: The phandle of MT8195 Display Port Tx codec node.
+
+      mediatek,hdmi-codec:
+        $ref: "/schemas/types.yaml#/definitions/phandle"
+        description: The phandle of MT8195 HDMI codec node.
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - interrupts
+  - interrupt-names
+  - clocks
+  - clock-names
+  - memory-region
+  - power-domains
+  - mbox-names
+  - mboxes
+  - sound
+
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    adsp: adsp@10803000 {
+       compatible =  "mediatek,mt8195-dsp";
+       reg = <0x10803000  0x1000>,
+             <0x10840000  0x40000>;
+       reg-names = "cfg", "sram";
+       interrupts = <GIC_SPI 694 IRQ_TYPE_LEVEL_HIGH 0>;
+       interrupt-names = "wdt";
+       clocks = <&topckgen 10>, //CLK_TOP_ADSP
+                <&clk26m>,
+                <&topckgen 107>, //CLK_TOP_AUDIO_LOCAL_BUS
+                <&topckgen 136>, //CLK_TOP_MAINPLL_D7_D2
+                <&scp_adsp 0>, //CLK_SCP_ADSP_AUDIODSP
+                <&topckgen 34>; //CLK_TOP_AUDIO_H
+       clock-names = "adsp_sel",
+                     "clk26m_ck",
+                     "audio_local_bus",
+                     "mainpll_d7_d2",
+                     "scp_adsp_audiodsp",
+                     "audio_h";
+       memory-region = <&adsp_dma_mem_reserved>,
+                       <&adsp_mem_reserved>;
+       power-domains = <&spm 6>; //MT8195_POWER_DOMAIN_ADSP
+       mbox-names = "mbox0", "mbox1";
+       mboxes = <&adsp_mailbox 0>, <&adsp_mailbox 1>;
+       status = "disabled";
+       sound {
+              mediatek,dptx-codec = <&dp_tx>;
+              mediatek,hdmi-codec = <&hdmi0>;
+              mediatek,platform = <&afe>;
+             };
+       };
-- 
2.18.0


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

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

* [PATCH v2 2/2] dt-bindings: dsp: mediatek: Add mt8195 DSP binding support
@ 2021-10-21  3:58   ` YC Hung
  0 siblings, 0 replies; 15+ messages in thread
From: YC Hung @ 2021-10-21  3:58 UTC (permalink / raw)
  To: broonie, tiwai, robh+dt, matthias.bgg
  Cc: yc.hung, alsa-devel, linux-mediatek, linux-arm-kernel,
	linux-kernel, devicetree, daniel.baluta, trevor.wu,
	allen-kh.cheng

This describes the mt8195 DSP device tree node.

Signed-off-by: YC Hung <yc.hung@mediatek.com>
---
 .../bindings/dsp/mtk,mt8195-dsp.yaml          | 139 ++++++++++++++++++
 1 file changed, 139 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/dsp/mtk,mt8195-dsp.yaml

diff --git a/Documentation/devicetree/bindings/dsp/mtk,mt8195-dsp.yaml b/Documentation/devicetree/bindings/dsp/mtk,mt8195-dsp.yaml
new file mode 100644
index 000000000000..f113f71ca094
--- /dev/null
+++ b/Documentation/devicetree/bindings/dsp/mtk,mt8195-dsp.yaml
@@ -0,0 +1,139 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/dsp/mtk,mt8195-dsp.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Mediatek mt8195 DSP core
+
+maintainers:
+  - YC Hung <yc.hung@mediatek.com>
+
+description: |
+  Some boards from mt8195 contain a DSP core used for
+  advanced pre- and post- audio processing.
+properties:
+  compatible:
+    const: mediatek,mt8195-dsp
+
+  reg:
+    maxItems: 2
+
+  reg-names:
+    maxItems: 2
+
+  interrupts:
+    maxItems: 1
+
+  interrupt-names:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: mux for audio dsp clock
+      - description: 26M clock
+      - description: mux for audio dsp local bus
+      - description: default audio dsp local bus clock source
+      - description: clock gate for audio dsp clock
+      - description: mux for audio dsp access external bus
+
+  clock-names:
+    items:
+      - const: adsp_sel
+      - const: clk26m_ck
+      - const: audio_local_bus
+      - const: mainpll_d7_d2
+      - const: scp_adsp_audiodsp
+      - const: audio_h
+
+  power-domains:
+    maxItems: 1
+
+  mboxes:
+    maxItems: 2
+
+  mbox-names:
+    description:
+      Specifies the mailboxes used to communicate with audio DSP
+    items:
+      - const: mbox0
+      - const: mbox1
+
+  memory-region:
+    description:
+      phandle to a node describing reserved memory (System RAM memory)
+      used by DSP (see bindings/reserved-memory/reserved-memory.txt)
+    maxItems: 2
+
+  sound:
+    description:
+      Sound subnode includes ASoC platform, DPTx codec node, and
+      HDMI codec node.
+
+    type: object
+
+    properties:
+      mediatek,platform:
+        $ref: "/schemas/types.yaml#/definitions/phandle"
+        description: The phandle of MT8195 ASoC platform.
+
+      mediatek,dptx-codec:
+        $ref: "/schemas/types.yaml#/definitions/phandle"
+        description: The phandle of MT8195 Display Port Tx codec node.
+
+      mediatek,hdmi-codec:
+        $ref: "/schemas/types.yaml#/definitions/phandle"
+        description: The phandle of MT8195 HDMI codec node.
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - interrupts
+  - interrupt-names
+  - clocks
+  - clock-names
+  - memory-region
+  - power-domains
+  - mbox-names
+  - mboxes
+  - sound
+
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    adsp: adsp@10803000 {
+       compatible =  "mediatek,mt8195-dsp";
+       reg = <0x10803000  0x1000>,
+             <0x10840000  0x40000>;
+       reg-names = "cfg", "sram";
+       interrupts = <GIC_SPI 694 IRQ_TYPE_LEVEL_HIGH 0>;
+       interrupt-names = "wdt";
+       clocks = <&topckgen 10>, //CLK_TOP_ADSP
+                <&clk26m>,
+                <&topckgen 107>, //CLK_TOP_AUDIO_LOCAL_BUS
+                <&topckgen 136>, //CLK_TOP_MAINPLL_D7_D2
+                <&scp_adsp 0>, //CLK_SCP_ADSP_AUDIODSP
+                <&topckgen 34>; //CLK_TOP_AUDIO_H
+       clock-names = "adsp_sel",
+                     "clk26m_ck",
+                     "audio_local_bus",
+                     "mainpll_d7_d2",
+                     "scp_adsp_audiodsp",
+                     "audio_h";
+       memory-region = <&adsp_dma_mem_reserved>,
+                       <&adsp_mem_reserved>;
+       power-domains = <&spm 6>; //MT8195_POWER_DOMAIN_ADSP
+       mbox-names = "mbox0", "mbox1";
+       mboxes = <&adsp_mailbox 0>, <&adsp_mailbox 1>;
+       status = "disabled";
+       sound {
+              mediatek,dptx-codec = <&dp_tx>;
+              mediatek,hdmi-codec = <&hdmi0>;
+              mediatek,platform = <&afe>;
+             };
+       };
-- 
2.18.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v2 1/2] ASoC: SOF: mediatek: Add mt8195 dsp clock support
  2021-10-21  3:58   ` YC Hung
  (?)
@ 2021-10-21 11:28     ` AngeloGioacchino Del Regno
  -1 siblings, 0 replies; 15+ messages in thread
From: AngeloGioacchino Del Regno @ 2021-10-21 11:28 UTC (permalink / raw)
  To: YC Hung, broonie, tiwai, robh+dt, matthias.bgg
  Cc: alsa-devel, linux-mediatek, linux-arm-kernel, linux-kernel,
	devicetree, daniel.baluta, trevor.wu, allen-kh.cheng

Il 21/10/21 05:58, YC Hung ha scritto:
> Add adsp clock on/off support on mt8195 platform.
> 
> Signed-off-by: YC Hung <yc.hung@mediatek.com>
> ---
>   sound/soc/sof/mediatek/mt8195/Makefile     |   2 +-
>   sound/soc/sof/mediatek/mt8195/mt8195-clk.c | 164 +++++++++++++++++++++
>   sound/soc/sof/mediatek/mt8195/mt8195-clk.h |  29 ++++
>   sound/soc/sof/mediatek/mt8195/mt8195.c     |  23 ++-
>   4 files changed, 215 insertions(+), 3 deletions(-)
>   create mode 100644 sound/soc/sof/mediatek/mt8195/mt8195-clk.c
>   create mode 100644 sound/soc/sof/mediatek/mt8195/mt8195-clk.h
> 

Hello,
Thanks for the patch! However, there's something to improve:

> diff --git a/sound/soc/sof/mediatek/mt8195/Makefile b/sound/soc/sof/mediatek/mt8195/Makefile
> index 60fca24c068a..650f4bce99b2 100644
> --- a/sound/soc/sof/mediatek/mt8195/Makefile
> +++ b/sound/soc/sof/mediatek/mt8195/Makefile
> @@ -1,4 +1,4 @@
>   # SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
> -snd-sof-mt8195-objs := mt8195.o mt8195-loader.o
> +snd-sof-mt8195-objs := mt8195.o mt8195-clk.o mt8195-loader.o
>   obj-$(CONFIG_SND_SOC_SOF_MT8195) += snd-sof-mt8195.o
>   
> diff --git a/sound/soc/sof/mediatek/mt8195/mt8195-clk.c b/sound/soc/sof/mediatek/mt8195/mt8195-clk.c
> new file mode 100644
> index 000000000000..1988421f7f7b
> --- /dev/null
> +++ b/sound/soc/sof/mediatek/mt8195/mt8195-clk.c
> @@ -0,0 +1,164 @@
> +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
> +//
> +// Copyright(c) 2021 Mediatek Corporation. All rights reserved.
> +//
> +// Author: YC Hung <yc.hung@mediatek.com>
> +//
> +// Hardware interface for mt8195 DSP clock
> +
> +#include <linux/clk.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/io.h>
> +#include "mt8195.h"
> +#include "mt8195-clk.h"
> +
> +struct clk *clk_handle[ADSP_CLK_NUM];

I think that this one can be moved to `struct adsp_priv` (or elsewhere, if more
appropriate) as to not use global variables/handles.

> +
> +int platform_parse_clock(struct device *dev)
> +{
> +	clk_handle[CLK_TOP_ADSP] = devm_clk_get(dev, "adsp_sel");
> +	if (IS_ERR(clk_handle[CLK_TOP_ADSP])) {
> +		dev_err(dev, "clk_get(\"adsp_sel\") failed\n");
> +		return PTR_ERR(clk_handle[CLK_TOP_ADSP]);
> +	}
> +
> +	clk_handle[CLK_TOP_CLK26M] = devm_clk_get(dev, "clk26m_ck");
> +	if (IS_ERR(clk_handle[CLK_TOP_CLK26M])) {
> +		dev_err(dev, "clk_get(\"clk26m_ck\") failed\n");
> +		return PTR_ERR(clk_handle[CLK_TOP_CLK26M]);
> +	}
> +
> +	clk_handle[CLK_TOP_AUDIO_LOCAL_BUS] = devm_clk_get(dev, "audio_local_bus");
> +	if (IS_ERR(clk_handle[CLK_TOP_AUDIO_LOCAL_BUS])) {
> +		dev_err(dev, "clk_get(\"audio_local_bus\") failed\n");
> +		return PTR_ERR(clk_handle[CLK_TOP_AUDIO_LOCAL_BUS]);
> +	}
> +
> +	clk_handle[CLK_TOP_MAINPLL_D7_D2] = devm_clk_get(dev, "mainpll_d7_d2");
> +	if (IS_ERR(clk_handle[CLK_TOP_MAINPLL_D7_D2])) {
> +		dev_err(dev, "clk_get(\"mainpll_d7_d2\") failed\n");
> +		return PTR_ERR(clk_handle[CLK_TOP_MAINPLL_D7_D2]);
> +	}
> +
> +	clk_handle[CLK_SCP_ADSP_AUDIODSP] = devm_clk_get(dev, "scp_adsp_audiodsp");
> +	if (IS_ERR(clk_handle[CLK_SCP_ADSP_AUDIODSP])) {
> +		dev_err(dev, "clk_get(\"scp_adsp_audiodsp\") failed\n");
> +		return PTR_ERR(clk_handle[CLK_SCP_ADSP_AUDIODSP]);
> +	}
> +
> +	clk_handle[CLK_TOP_AUDIO_H] = devm_clk_get(dev, "audio_h");
> +	if (IS_ERR(clk_handle[CLK_TOP_AUDIO_H])) {
> +		dev_err(dev, "clk_get(\"audio_h_sel\") failed\n");
> +		return PTR_ERR(clk_handle[CLK_TOP_AUDIO_H]);
> +	}
> +
> +	return 0;
> +}
> +
> +int adsp_enable_clock(struct device *dev)

You are using this function only in this file, please make it static.

> +{
> +	int ret;
> +
> +	ret = clk_prepare_enable(clk_handle[CLK_TOP_MAINPLL_D7_D2]);
> +	if (ret) {
> +		dev_err(dev, "%s clk_prepare_enable(mainpll_d7_d2) fail %d\n",
> +			__func__, ret);
> +		return ret;
> +	}
> +
> +	ret = clk_prepare_enable(clk_handle[CLK_TOP_ADSP]);
> +	if (ret) {
> +		dev_err(dev, "%s clk_prepare_enable(adsp_sel) fail %d\n",
> +			__func__, ret);
> +		goto disable_mainpll_d7_d2_clk;
> +	}
> +
> +	ret = clk_prepare_enable(clk_handle[CLK_TOP_AUDIO_LOCAL_BUS]);
> +	if (ret) {
> +		dev_err(dev, "%s clk_prepare_enable(audio_local_bus) fail %d\n",
> +			__func__, ret);
> +		goto disable_dsp_sel_clk;
> +	}
> +
> +	ret = clk_prepare_enable(clk_handle[CLK_SCP_ADSP_AUDIODSP]);
> +	if (ret) {
> +		dev_err(dev, "%s clk_prepare_enable(scp_adsp_audiodsp) fail %d\n",
> +			__func__, ret);
> +		goto disable_audio_local_bus_clk;
> +	}
> +
> +	ret = clk_prepare_enable(clk_handle[CLK_TOP_AUDIO_H]);
> +	if (ret) {
> +		dev_err(dev, "%s clk_prepare_enable(audio_h) fail %d\n",
> +			__func__, ret);
> +		goto disable_scp_adsp_audiodsp_clk;
> +	}
> +
> +	return 0;
> +
> +disable_scp_adsp_audiodsp_clk:
> +	clk_disable_unprepare(clk_handle[CLK_SCP_ADSP_AUDIODSP]);
> +disable_audio_local_bus_clk:
> +	clk_disable_unprepare(clk_handle[CLK_TOP_AUDIO_LOCAL_BUS]);
> +disable_dsp_sel_clk:
> +	clk_disable_unprepare(clk_handle[CLK_TOP_ADSP]);
> +disable_mainpll_d7_d2_clk:
> +	clk_disable_unprepare(clk_handle[CLK_TOP_MAINPLL_D7_D2]);
> +
> +	return ret;
> +}
> +
> +void adsp_disable_clock(struct device *dev)

Same here...

> +{
> +	clk_disable_unprepare(clk_handle[CLK_TOP_AUDIO_H]);
> +	clk_disable_unprepare(clk_handle[CLK_SCP_ADSP_AUDIODSP]);
> +	clk_disable_unprepare(clk_handle[CLK_TOP_AUDIO_LOCAL_BUS]);
> +	clk_disable_unprepare(clk_handle[CLK_TOP_ADSP]);
> +	clk_disable_unprepare(clk_handle[CLK_TOP_MAINPLL_D7_D2]);
> +}
> +
> +int adsp_default_clk_init(struct device *dev, int enable)

...and same here.

Also, your `enable` parameter can logically (currently) only have two meanings:
enable, or disable; I think that it would be better to use a bool instead.

> +{
> +	int ret = 0;
> +
> +	dev_dbg(dev, "%s: %s\n", __func__, enable ? "on" : "off");
> +
> +	if (enable) {
> +		ret = clk_set_parent(clk_handle[CLK_TOP_ADSP],
> +				     clk_handle[CLK_TOP_CLK26M]);
> +		if (ret) {
> +			dev_err(dev, "failed to set dsp_sel to clk26m: %d\n", ret);
> +			return ret;
> +		}
> +
> +		ret = clk_set_parent(clk_handle[CLK_TOP_AUDIO_LOCAL_BUS],
> +				     clk_handle[CLK_TOP_MAINPLL_D7_D2]);
> +		if (ret) {
> +			dev_err(dev, "set audio_local_bus failed %d\n", ret);
> +			return ret;
> +		}
> +

I think it'd be a good idea to...

> +		ret = adsp_enable_clock(dev);
> +		if (ret)

		if (ret) {

> +			dev_err(dev, "failed to adsp_enable_clock: %d\n", ret);

			return ret;
		}

> +
> +		return ret;

...and remove this return...

> +	}
> +

	} else {

> +	adsp_disable_clock(dev);

	}

> +
> +	return ret;

...and then, since you are covering all of the error cases before the end of
this function, here you should just

	return 0;

> +}
> +
> +int adsp_clock_on(struct device *dev)
> +{
> +	/* Open ADSP clock */
> +	return adsp_default_clk_init(dev, 1);
> +}
> +
> +int adsp_clock_off(struct device *dev)
> +{
> +	/* Close ADSP clock */
> +	return adsp_default_clk_init(dev, 0);
> +}
> +
> diff --git a/sound/soc/sof/mediatek/mt8195/mt8195-clk.h b/sound/soc/sof/mediatek/mt8195/mt8195-clk.h
> new file mode 100644
> index 000000000000..f985d141552a
> --- /dev/null
> +++ b/sound/soc/sof/mediatek/mt8195/mt8195-clk.h
> @@ -0,0 +1,29 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +/*
> + * Copyright (c) 2021 MediaTek Corporation. All rights reserved.
> + *
> + *  Header file for the mt8195 DSP clock  definition
> + */
> +
> +#ifndef __MT8195_CLK_H
> +#define __MT8195_CLK_H
> +
> +/*DSP clock*/

/* DSP clock id */

> +enum ADSP_CLK_ID {

lowercase please: enum adsp_clk_id

> +	CLK_TOP_ADSP,
> +	CLK_TOP_CLK26M,
> +	CLK_TOP_AUDIO_LOCAL_BUS,
> +	CLK_TOP_MAINPLL_D7_D2,
> +	CLK_SCP_ADSP_AUDIODSP,
> +	CLK_TOP_AUDIO_H,
> +	ADSP_CLK_NUM

What about ADSP_CLK_MAX instead?
Personal preference here, nothing worrying.

> +};
> +
> +int platform_parse_clock(struct device *dev);
> +int adsp_default_clk_init(struct device *dev, int enable);
> +int adsp_enable_clock(struct device *dev);
> +void adsp_disable_clock(struct device *dev);
> +int adsp_clock_on(struct device *dev);
> +int adsp_clock_off(struct device *dev);
> +#endif
> diff --git a/sound/soc/sof/mediatek/mt8195/mt8195.c b/sound/soc/sof/mediatek/mt8195/mt8195.c
> index 99075598a35a..f323da58057b 100644
> --- a/sound/soc/sof/mediatek/mt8195/mt8195.c
> +++ b/sound/soc/sof/mediatek/mt8195/mt8195.c
> @@ -25,6 +25,7 @@
>   #include "../adsp_helper.h"
>   #include "../mediatek-ops.h"
>   #include "mt8195.h"
> +#include "mt8195-clk.h"
>   
>   static int platform_parse_resource(struct platform_device *pdev, void *data)
>   {
> @@ -231,10 +232,23 @@ static int mt8195_dsp_probe(struct snd_sof_dev *sdev)
>   	if (ret)
>   		return ret;
>   
> +	ret = platform_parse_clock(&pdev->dev);
> +	if (ret) {
> +		dev_err(sdev->dev, "platform_parse_clock failed\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = adsp_clock_on(&pdev->dev);
> +	if (ret) {
> +		dev_err(sdev->dev, "adsp_clock_on fail!\n");
> +		return -EINVAL;
> +	}
> +
>   	ret = adsp_sram_power_on(sdev->dev, true);
>   	if (ret) {
>   		dev_err(sdev->dev, "adsp_sram_power_on fail!\n");
> -		return ret;
> +		ret = -EINVAL;
> +		goto exit_clk_disable;
Why are you overriding adsp_sram_power_on()'s return value?
As of now, this function is supposed to return -ENOMEM or 0.. and you
shouldn't override the return value with -EINVAL here.

>   	}
>   
>   	ret = adsp_memory_remap_init(&pdev->dev, priv->adsp);
> @@ -282,6 +296,8 @@ static int mt8195_dsp_probe(struct snd_sof_dev *sdev)
>   
>   err_adsp_sram_power_off:
>   	adsp_sram_power_on(&pdev->dev, false);
> +exit_clk_disable:
> +	adsp_clock_off(&pdev->dev);
>   
>   	return ret;
>   }
> @@ -290,7 +306,10 @@ static int mt8195_dsp_remove(struct snd_sof_dev *sdev)
>   {
>   	struct platform_device *pdev = container_of(sdev->dev, struct platform_device, dev);
>   
> -	return adsp_sram_power_on(&pdev->dev, false);
> +	adsp_sram_power_on(&pdev->dev, false);
> +	adsp_clock_off(&pdev->dev);
> +
> +	return 0;
>   }
>   
>   /* on mt8195 there is 1 to 1 match between type and BAR idx */
> 

Regards,
- Angelo

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

* Re: [PATCH v2 1/2] ASoC: SOF: mediatek: Add mt8195 dsp clock support
@ 2021-10-21 11:28     ` AngeloGioacchino Del Regno
  0 siblings, 0 replies; 15+ messages in thread
From: AngeloGioacchino Del Regno @ 2021-10-21 11:28 UTC (permalink / raw)
  To: YC Hung, broonie, tiwai, robh+dt, matthias.bgg
  Cc: alsa-devel, linux-mediatek, linux-arm-kernel, linux-kernel,
	devicetree, daniel.baluta, trevor.wu, allen-kh.cheng

Il 21/10/21 05:58, YC Hung ha scritto:
> Add adsp clock on/off support on mt8195 platform.
> 
> Signed-off-by: YC Hung <yc.hung@mediatek.com>
> ---
>   sound/soc/sof/mediatek/mt8195/Makefile     |   2 +-
>   sound/soc/sof/mediatek/mt8195/mt8195-clk.c | 164 +++++++++++++++++++++
>   sound/soc/sof/mediatek/mt8195/mt8195-clk.h |  29 ++++
>   sound/soc/sof/mediatek/mt8195/mt8195.c     |  23 ++-
>   4 files changed, 215 insertions(+), 3 deletions(-)
>   create mode 100644 sound/soc/sof/mediatek/mt8195/mt8195-clk.c
>   create mode 100644 sound/soc/sof/mediatek/mt8195/mt8195-clk.h
> 

Hello,
Thanks for the patch! However, there's something to improve:

> diff --git a/sound/soc/sof/mediatek/mt8195/Makefile b/sound/soc/sof/mediatek/mt8195/Makefile
> index 60fca24c068a..650f4bce99b2 100644
> --- a/sound/soc/sof/mediatek/mt8195/Makefile
> +++ b/sound/soc/sof/mediatek/mt8195/Makefile
> @@ -1,4 +1,4 @@
>   # SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
> -snd-sof-mt8195-objs := mt8195.o mt8195-loader.o
> +snd-sof-mt8195-objs := mt8195.o mt8195-clk.o mt8195-loader.o
>   obj-$(CONFIG_SND_SOC_SOF_MT8195) += snd-sof-mt8195.o
>   
> diff --git a/sound/soc/sof/mediatek/mt8195/mt8195-clk.c b/sound/soc/sof/mediatek/mt8195/mt8195-clk.c
> new file mode 100644
> index 000000000000..1988421f7f7b
> --- /dev/null
> +++ b/sound/soc/sof/mediatek/mt8195/mt8195-clk.c
> @@ -0,0 +1,164 @@
> +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
> +//
> +// Copyright(c) 2021 Mediatek Corporation. All rights reserved.
> +//
> +// Author: YC Hung <yc.hung@mediatek.com>
> +//
> +// Hardware interface for mt8195 DSP clock
> +
> +#include <linux/clk.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/io.h>
> +#include "mt8195.h"
> +#include "mt8195-clk.h"
> +
> +struct clk *clk_handle[ADSP_CLK_NUM];

I think that this one can be moved to `struct adsp_priv` (or elsewhere, if more
appropriate) as to not use global variables/handles.

> +
> +int platform_parse_clock(struct device *dev)
> +{
> +	clk_handle[CLK_TOP_ADSP] = devm_clk_get(dev, "adsp_sel");
> +	if (IS_ERR(clk_handle[CLK_TOP_ADSP])) {
> +		dev_err(dev, "clk_get(\"adsp_sel\") failed\n");
> +		return PTR_ERR(clk_handle[CLK_TOP_ADSP]);
> +	}
> +
> +	clk_handle[CLK_TOP_CLK26M] = devm_clk_get(dev, "clk26m_ck");
> +	if (IS_ERR(clk_handle[CLK_TOP_CLK26M])) {
> +		dev_err(dev, "clk_get(\"clk26m_ck\") failed\n");
> +		return PTR_ERR(clk_handle[CLK_TOP_CLK26M]);
> +	}
> +
> +	clk_handle[CLK_TOP_AUDIO_LOCAL_BUS] = devm_clk_get(dev, "audio_local_bus");
> +	if (IS_ERR(clk_handle[CLK_TOP_AUDIO_LOCAL_BUS])) {
> +		dev_err(dev, "clk_get(\"audio_local_bus\") failed\n");
> +		return PTR_ERR(clk_handle[CLK_TOP_AUDIO_LOCAL_BUS]);
> +	}
> +
> +	clk_handle[CLK_TOP_MAINPLL_D7_D2] = devm_clk_get(dev, "mainpll_d7_d2");
> +	if (IS_ERR(clk_handle[CLK_TOP_MAINPLL_D7_D2])) {
> +		dev_err(dev, "clk_get(\"mainpll_d7_d2\") failed\n");
> +		return PTR_ERR(clk_handle[CLK_TOP_MAINPLL_D7_D2]);
> +	}
> +
> +	clk_handle[CLK_SCP_ADSP_AUDIODSP] = devm_clk_get(dev, "scp_adsp_audiodsp");
> +	if (IS_ERR(clk_handle[CLK_SCP_ADSP_AUDIODSP])) {
> +		dev_err(dev, "clk_get(\"scp_adsp_audiodsp\") failed\n");
> +		return PTR_ERR(clk_handle[CLK_SCP_ADSP_AUDIODSP]);
> +	}
> +
> +	clk_handle[CLK_TOP_AUDIO_H] = devm_clk_get(dev, "audio_h");
> +	if (IS_ERR(clk_handle[CLK_TOP_AUDIO_H])) {
> +		dev_err(dev, "clk_get(\"audio_h_sel\") failed\n");
> +		return PTR_ERR(clk_handle[CLK_TOP_AUDIO_H]);
> +	}
> +
> +	return 0;
> +}
> +
> +int adsp_enable_clock(struct device *dev)

You are using this function only in this file, please make it static.

> +{
> +	int ret;
> +
> +	ret = clk_prepare_enable(clk_handle[CLK_TOP_MAINPLL_D7_D2]);
> +	if (ret) {
> +		dev_err(dev, "%s clk_prepare_enable(mainpll_d7_d2) fail %d\n",
> +			__func__, ret);
> +		return ret;
> +	}
> +
> +	ret = clk_prepare_enable(clk_handle[CLK_TOP_ADSP]);
> +	if (ret) {
> +		dev_err(dev, "%s clk_prepare_enable(adsp_sel) fail %d\n",
> +			__func__, ret);
> +		goto disable_mainpll_d7_d2_clk;
> +	}
> +
> +	ret = clk_prepare_enable(clk_handle[CLK_TOP_AUDIO_LOCAL_BUS]);
> +	if (ret) {
> +		dev_err(dev, "%s clk_prepare_enable(audio_local_bus) fail %d\n",
> +			__func__, ret);
> +		goto disable_dsp_sel_clk;
> +	}
> +
> +	ret = clk_prepare_enable(clk_handle[CLK_SCP_ADSP_AUDIODSP]);
> +	if (ret) {
> +		dev_err(dev, "%s clk_prepare_enable(scp_adsp_audiodsp) fail %d\n",
> +			__func__, ret);
> +		goto disable_audio_local_bus_clk;
> +	}
> +
> +	ret = clk_prepare_enable(clk_handle[CLK_TOP_AUDIO_H]);
> +	if (ret) {
> +		dev_err(dev, "%s clk_prepare_enable(audio_h) fail %d\n",
> +			__func__, ret);
> +		goto disable_scp_adsp_audiodsp_clk;
> +	}
> +
> +	return 0;
> +
> +disable_scp_adsp_audiodsp_clk:
> +	clk_disable_unprepare(clk_handle[CLK_SCP_ADSP_AUDIODSP]);
> +disable_audio_local_bus_clk:
> +	clk_disable_unprepare(clk_handle[CLK_TOP_AUDIO_LOCAL_BUS]);
> +disable_dsp_sel_clk:
> +	clk_disable_unprepare(clk_handle[CLK_TOP_ADSP]);
> +disable_mainpll_d7_d2_clk:
> +	clk_disable_unprepare(clk_handle[CLK_TOP_MAINPLL_D7_D2]);
> +
> +	return ret;
> +}
> +
> +void adsp_disable_clock(struct device *dev)

Same here...

> +{
> +	clk_disable_unprepare(clk_handle[CLK_TOP_AUDIO_H]);
> +	clk_disable_unprepare(clk_handle[CLK_SCP_ADSP_AUDIODSP]);
> +	clk_disable_unprepare(clk_handle[CLK_TOP_AUDIO_LOCAL_BUS]);
> +	clk_disable_unprepare(clk_handle[CLK_TOP_ADSP]);
> +	clk_disable_unprepare(clk_handle[CLK_TOP_MAINPLL_D7_D2]);
> +}
> +
> +int adsp_default_clk_init(struct device *dev, int enable)

...and same here.

Also, your `enable` parameter can logically (currently) only have two meanings:
enable, or disable; I think that it would be better to use a bool instead.

> +{
> +	int ret = 0;
> +
> +	dev_dbg(dev, "%s: %s\n", __func__, enable ? "on" : "off");
> +
> +	if (enable) {
> +		ret = clk_set_parent(clk_handle[CLK_TOP_ADSP],
> +				     clk_handle[CLK_TOP_CLK26M]);
> +		if (ret) {
> +			dev_err(dev, "failed to set dsp_sel to clk26m: %d\n", ret);
> +			return ret;
> +		}
> +
> +		ret = clk_set_parent(clk_handle[CLK_TOP_AUDIO_LOCAL_BUS],
> +				     clk_handle[CLK_TOP_MAINPLL_D7_D2]);
> +		if (ret) {
> +			dev_err(dev, "set audio_local_bus failed %d\n", ret);
> +			return ret;
> +		}
> +

I think it'd be a good idea to...

> +		ret = adsp_enable_clock(dev);
> +		if (ret)

		if (ret) {

> +			dev_err(dev, "failed to adsp_enable_clock: %d\n", ret);

			return ret;
		}

> +
> +		return ret;

...and remove this return...

> +	}
> +

	} else {

> +	adsp_disable_clock(dev);

	}

> +
> +	return ret;

...and then, since you are covering all of the error cases before the end of
this function, here you should just

	return 0;

> +}
> +
> +int adsp_clock_on(struct device *dev)
> +{
> +	/* Open ADSP clock */
> +	return adsp_default_clk_init(dev, 1);
> +}
> +
> +int adsp_clock_off(struct device *dev)
> +{
> +	/* Close ADSP clock */
> +	return adsp_default_clk_init(dev, 0);
> +}
> +
> diff --git a/sound/soc/sof/mediatek/mt8195/mt8195-clk.h b/sound/soc/sof/mediatek/mt8195/mt8195-clk.h
> new file mode 100644
> index 000000000000..f985d141552a
> --- /dev/null
> +++ b/sound/soc/sof/mediatek/mt8195/mt8195-clk.h
> @@ -0,0 +1,29 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +/*
> + * Copyright (c) 2021 MediaTek Corporation. All rights reserved.
> + *
> + *  Header file for the mt8195 DSP clock  definition
> + */
> +
> +#ifndef __MT8195_CLK_H
> +#define __MT8195_CLK_H
> +
> +/*DSP clock*/

/* DSP clock id */

> +enum ADSP_CLK_ID {

lowercase please: enum adsp_clk_id

> +	CLK_TOP_ADSP,
> +	CLK_TOP_CLK26M,
> +	CLK_TOP_AUDIO_LOCAL_BUS,
> +	CLK_TOP_MAINPLL_D7_D2,
> +	CLK_SCP_ADSP_AUDIODSP,
> +	CLK_TOP_AUDIO_H,
> +	ADSP_CLK_NUM

What about ADSP_CLK_MAX instead?
Personal preference here, nothing worrying.

> +};
> +
> +int platform_parse_clock(struct device *dev);
> +int adsp_default_clk_init(struct device *dev, int enable);
> +int adsp_enable_clock(struct device *dev);
> +void adsp_disable_clock(struct device *dev);
> +int adsp_clock_on(struct device *dev);
> +int adsp_clock_off(struct device *dev);
> +#endif
> diff --git a/sound/soc/sof/mediatek/mt8195/mt8195.c b/sound/soc/sof/mediatek/mt8195/mt8195.c
> index 99075598a35a..f323da58057b 100644
> --- a/sound/soc/sof/mediatek/mt8195/mt8195.c
> +++ b/sound/soc/sof/mediatek/mt8195/mt8195.c
> @@ -25,6 +25,7 @@
>   #include "../adsp_helper.h"
>   #include "../mediatek-ops.h"
>   #include "mt8195.h"
> +#include "mt8195-clk.h"
>   
>   static int platform_parse_resource(struct platform_device *pdev, void *data)
>   {
> @@ -231,10 +232,23 @@ static int mt8195_dsp_probe(struct snd_sof_dev *sdev)
>   	if (ret)
>   		return ret;
>   
> +	ret = platform_parse_clock(&pdev->dev);
> +	if (ret) {
> +		dev_err(sdev->dev, "platform_parse_clock failed\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = adsp_clock_on(&pdev->dev);
> +	if (ret) {
> +		dev_err(sdev->dev, "adsp_clock_on fail!\n");
> +		return -EINVAL;
> +	}
> +
>   	ret = adsp_sram_power_on(sdev->dev, true);
>   	if (ret) {
>   		dev_err(sdev->dev, "adsp_sram_power_on fail!\n");
> -		return ret;
> +		ret = -EINVAL;
> +		goto exit_clk_disable;
Why are you overriding adsp_sram_power_on()'s return value?
As of now, this function is supposed to return -ENOMEM or 0.. and you
shouldn't override the return value with -EINVAL here.

>   	}
>   
>   	ret = adsp_memory_remap_init(&pdev->dev, priv->adsp);
> @@ -282,6 +296,8 @@ static int mt8195_dsp_probe(struct snd_sof_dev *sdev)
>   
>   err_adsp_sram_power_off:
>   	adsp_sram_power_on(&pdev->dev, false);
> +exit_clk_disable:
> +	adsp_clock_off(&pdev->dev);
>   
>   	return ret;
>   }
> @@ -290,7 +306,10 @@ static int mt8195_dsp_remove(struct snd_sof_dev *sdev)
>   {
>   	struct platform_device *pdev = container_of(sdev->dev, struct platform_device, dev);
>   
> -	return adsp_sram_power_on(&pdev->dev, false);
> +	adsp_sram_power_on(&pdev->dev, false);
> +	adsp_clock_off(&pdev->dev);
> +
> +	return 0;
>   }
>   
>   /* on mt8195 there is 1 to 1 match between type and BAR idx */
> 

Regards,
- Angelo

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

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

* Re: [PATCH v2 1/2] ASoC: SOF: mediatek: Add mt8195 dsp clock support
@ 2021-10-21 11:28     ` AngeloGioacchino Del Regno
  0 siblings, 0 replies; 15+ messages in thread
From: AngeloGioacchino Del Regno @ 2021-10-21 11:28 UTC (permalink / raw)
  To: YC Hung, broonie, tiwai, robh+dt, matthias.bgg
  Cc: alsa-devel, linux-mediatek, linux-arm-kernel, linux-kernel,
	devicetree, daniel.baluta, trevor.wu, allen-kh.cheng

Il 21/10/21 05:58, YC Hung ha scritto:
> Add adsp clock on/off support on mt8195 platform.
> 
> Signed-off-by: YC Hung <yc.hung@mediatek.com>
> ---
>   sound/soc/sof/mediatek/mt8195/Makefile     |   2 +-
>   sound/soc/sof/mediatek/mt8195/mt8195-clk.c | 164 +++++++++++++++++++++
>   sound/soc/sof/mediatek/mt8195/mt8195-clk.h |  29 ++++
>   sound/soc/sof/mediatek/mt8195/mt8195.c     |  23 ++-
>   4 files changed, 215 insertions(+), 3 deletions(-)
>   create mode 100644 sound/soc/sof/mediatek/mt8195/mt8195-clk.c
>   create mode 100644 sound/soc/sof/mediatek/mt8195/mt8195-clk.h
> 

Hello,
Thanks for the patch! However, there's something to improve:

> diff --git a/sound/soc/sof/mediatek/mt8195/Makefile b/sound/soc/sof/mediatek/mt8195/Makefile
> index 60fca24c068a..650f4bce99b2 100644
> --- a/sound/soc/sof/mediatek/mt8195/Makefile
> +++ b/sound/soc/sof/mediatek/mt8195/Makefile
> @@ -1,4 +1,4 @@
>   # SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
> -snd-sof-mt8195-objs := mt8195.o mt8195-loader.o
> +snd-sof-mt8195-objs := mt8195.o mt8195-clk.o mt8195-loader.o
>   obj-$(CONFIG_SND_SOC_SOF_MT8195) += snd-sof-mt8195.o
>   
> diff --git a/sound/soc/sof/mediatek/mt8195/mt8195-clk.c b/sound/soc/sof/mediatek/mt8195/mt8195-clk.c
> new file mode 100644
> index 000000000000..1988421f7f7b
> --- /dev/null
> +++ b/sound/soc/sof/mediatek/mt8195/mt8195-clk.c
> @@ -0,0 +1,164 @@
> +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
> +//
> +// Copyright(c) 2021 Mediatek Corporation. All rights reserved.
> +//
> +// Author: YC Hung <yc.hung@mediatek.com>
> +//
> +// Hardware interface for mt8195 DSP clock
> +
> +#include <linux/clk.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/io.h>
> +#include "mt8195.h"
> +#include "mt8195-clk.h"
> +
> +struct clk *clk_handle[ADSP_CLK_NUM];

I think that this one can be moved to `struct adsp_priv` (or elsewhere, if more
appropriate) as to not use global variables/handles.

> +
> +int platform_parse_clock(struct device *dev)
> +{
> +	clk_handle[CLK_TOP_ADSP] = devm_clk_get(dev, "adsp_sel");
> +	if (IS_ERR(clk_handle[CLK_TOP_ADSP])) {
> +		dev_err(dev, "clk_get(\"adsp_sel\") failed\n");
> +		return PTR_ERR(clk_handle[CLK_TOP_ADSP]);
> +	}
> +
> +	clk_handle[CLK_TOP_CLK26M] = devm_clk_get(dev, "clk26m_ck");
> +	if (IS_ERR(clk_handle[CLK_TOP_CLK26M])) {
> +		dev_err(dev, "clk_get(\"clk26m_ck\") failed\n");
> +		return PTR_ERR(clk_handle[CLK_TOP_CLK26M]);
> +	}
> +
> +	clk_handle[CLK_TOP_AUDIO_LOCAL_BUS] = devm_clk_get(dev, "audio_local_bus");
> +	if (IS_ERR(clk_handle[CLK_TOP_AUDIO_LOCAL_BUS])) {
> +		dev_err(dev, "clk_get(\"audio_local_bus\") failed\n");
> +		return PTR_ERR(clk_handle[CLK_TOP_AUDIO_LOCAL_BUS]);
> +	}
> +
> +	clk_handle[CLK_TOP_MAINPLL_D7_D2] = devm_clk_get(dev, "mainpll_d7_d2");
> +	if (IS_ERR(clk_handle[CLK_TOP_MAINPLL_D7_D2])) {
> +		dev_err(dev, "clk_get(\"mainpll_d7_d2\") failed\n");
> +		return PTR_ERR(clk_handle[CLK_TOP_MAINPLL_D7_D2]);
> +	}
> +
> +	clk_handle[CLK_SCP_ADSP_AUDIODSP] = devm_clk_get(dev, "scp_adsp_audiodsp");
> +	if (IS_ERR(clk_handle[CLK_SCP_ADSP_AUDIODSP])) {
> +		dev_err(dev, "clk_get(\"scp_adsp_audiodsp\") failed\n");
> +		return PTR_ERR(clk_handle[CLK_SCP_ADSP_AUDIODSP]);
> +	}
> +
> +	clk_handle[CLK_TOP_AUDIO_H] = devm_clk_get(dev, "audio_h");
> +	if (IS_ERR(clk_handle[CLK_TOP_AUDIO_H])) {
> +		dev_err(dev, "clk_get(\"audio_h_sel\") failed\n");
> +		return PTR_ERR(clk_handle[CLK_TOP_AUDIO_H]);
> +	}
> +
> +	return 0;
> +}
> +
> +int adsp_enable_clock(struct device *dev)

You are using this function only in this file, please make it static.

> +{
> +	int ret;
> +
> +	ret = clk_prepare_enable(clk_handle[CLK_TOP_MAINPLL_D7_D2]);
> +	if (ret) {
> +		dev_err(dev, "%s clk_prepare_enable(mainpll_d7_d2) fail %d\n",
> +			__func__, ret);
> +		return ret;
> +	}
> +
> +	ret = clk_prepare_enable(clk_handle[CLK_TOP_ADSP]);
> +	if (ret) {
> +		dev_err(dev, "%s clk_prepare_enable(adsp_sel) fail %d\n",
> +			__func__, ret);
> +		goto disable_mainpll_d7_d2_clk;
> +	}
> +
> +	ret = clk_prepare_enable(clk_handle[CLK_TOP_AUDIO_LOCAL_BUS]);
> +	if (ret) {
> +		dev_err(dev, "%s clk_prepare_enable(audio_local_bus) fail %d\n",
> +			__func__, ret);
> +		goto disable_dsp_sel_clk;
> +	}
> +
> +	ret = clk_prepare_enable(clk_handle[CLK_SCP_ADSP_AUDIODSP]);
> +	if (ret) {
> +		dev_err(dev, "%s clk_prepare_enable(scp_adsp_audiodsp) fail %d\n",
> +			__func__, ret);
> +		goto disable_audio_local_bus_clk;
> +	}
> +
> +	ret = clk_prepare_enable(clk_handle[CLK_TOP_AUDIO_H]);
> +	if (ret) {
> +		dev_err(dev, "%s clk_prepare_enable(audio_h) fail %d\n",
> +			__func__, ret);
> +		goto disable_scp_adsp_audiodsp_clk;
> +	}
> +
> +	return 0;
> +
> +disable_scp_adsp_audiodsp_clk:
> +	clk_disable_unprepare(clk_handle[CLK_SCP_ADSP_AUDIODSP]);
> +disable_audio_local_bus_clk:
> +	clk_disable_unprepare(clk_handle[CLK_TOP_AUDIO_LOCAL_BUS]);
> +disable_dsp_sel_clk:
> +	clk_disable_unprepare(clk_handle[CLK_TOP_ADSP]);
> +disable_mainpll_d7_d2_clk:
> +	clk_disable_unprepare(clk_handle[CLK_TOP_MAINPLL_D7_D2]);
> +
> +	return ret;
> +}
> +
> +void adsp_disable_clock(struct device *dev)

Same here...

> +{
> +	clk_disable_unprepare(clk_handle[CLK_TOP_AUDIO_H]);
> +	clk_disable_unprepare(clk_handle[CLK_SCP_ADSP_AUDIODSP]);
> +	clk_disable_unprepare(clk_handle[CLK_TOP_AUDIO_LOCAL_BUS]);
> +	clk_disable_unprepare(clk_handle[CLK_TOP_ADSP]);
> +	clk_disable_unprepare(clk_handle[CLK_TOP_MAINPLL_D7_D2]);
> +}
> +
> +int adsp_default_clk_init(struct device *dev, int enable)

...and same here.

Also, your `enable` parameter can logically (currently) only have two meanings:
enable, or disable; I think that it would be better to use a bool instead.

> +{
> +	int ret = 0;
> +
> +	dev_dbg(dev, "%s: %s\n", __func__, enable ? "on" : "off");
> +
> +	if (enable) {
> +		ret = clk_set_parent(clk_handle[CLK_TOP_ADSP],
> +				     clk_handle[CLK_TOP_CLK26M]);
> +		if (ret) {
> +			dev_err(dev, "failed to set dsp_sel to clk26m: %d\n", ret);
> +			return ret;
> +		}
> +
> +		ret = clk_set_parent(clk_handle[CLK_TOP_AUDIO_LOCAL_BUS],
> +				     clk_handle[CLK_TOP_MAINPLL_D7_D2]);
> +		if (ret) {
> +			dev_err(dev, "set audio_local_bus failed %d\n", ret);
> +			return ret;
> +		}
> +

I think it'd be a good idea to...

> +		ret = adsp_enable_clock(dev);
> +		if (ret)

		if (ret) {

> +			dev_err(dev, "failed to adsp_enable_clock: %d\n", ret);

			return ret;
		}

> +
> +		return ret;

...and remove this return...

> +	}
> +

	} else {

> +	adsp_disable_clock(dev);

	}

> +
> +	return ret;

...and then, since you are covering all of the error cases before the end of
this function, here you should just

	return 0;

> +}
> +
> +int adsp_clock_on(struct device *dev)
> +{
> +	/* Open ADSP clock */
> +	return adsp_default_clk_init(dev, 1);
> +}
> +
> +int adsp_clock_off(struct device *dev)
> +{
> +	/* Close ADSP clock */
> +	return adsp_default_clk_init(dev, 0);
> +}
> +
> diff --git a/sound/soc/sof/mediatek/mt8195/mt8195-clk.h b/sound/soc/sof/mediatek/mt8195/mt8195-clk.h
> new file mode 100644
> index 000000000000..f985d141552a
> --- /dev/null
> +++ b/sound/soc/sof/mediatek/mt8195/mt8195-clk.h
> @@ -0,0 +1,29 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +/*
> + * Copyright (c) 2021 MediaTek Corporation. All rights reserved.
> + *
> + *  Header file for the mt8195 DSP clock  definition
> + */
> +
> +#ifndef __MT8195_CLK_H
> +#define __MT8195_CLK_H
> +
> +/*DSP clock*/

/* DSP clock id */

> +enum ADSP_CLK_ID {

lowercase please: enum adsp_clk_id

> +	CLK_TOP_ADSP,
> +	CLK_TOP_CLK26M,
> +	CLK_TOP_AUDIO_LOCAL_BUS,
> +	CLK_TOP_MAINPLL_D7_D2,
> +	CLK_SCP_ADSP_AUDIODSP,
> +	CLK_TOP_AUDIO_H,
> +	ADSP_CLK_NUM

What about ADSP_CLK_MAX instead?
Personal preference here, nothing worrying.

> +};
> +
> +int platform_parse_clock(struct device *dev);
> +int adsp_default_clk_init(struct device *dev, int enable);
> +int adsp_enable_clock(struct device *dev);
> +void adsp_disable_clock(struct device *dev);
> +int adsp_clock_on(struct device *dev);
> +int adsp_clock_off(struct device *dev);
> +#endif
> diff --git a/sound/soc/sof/mediatek/mt8195/mt8195.c b/sound/soc/sof/mediatek/mt8195/mt8195.c
> index 99075598a35a..f323da58057b 100644
> --- a/sound/soc/sof/mediatek/mt8195/mt8195.c
> +++ b/sound/soc/sof/mediatek/mt8195/mt8195.c
> @@ -25,6 +25,7 @@
>   #include "../adsp_helper.h"
>   #include "../mediatek-ops.h"
>   #include "mt8195.h"
> +#include "mt8195-clk.h"
>   
>   static int platform_parse_resource(struct platform_device *pdev, void *data)
>   {
> @@ -231,10 +232,23 @@ static int mt8195_dsp_probe(struct snd_sof_dev *sdev)
>   	if (ret)
>   		return ret;
>   
> +	ret = platform_parse_clock(&pdev->dev);
> +	if (ret) {
> +		dev_err(sdev->dev, "platform_parse_clock failed\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = adsp_clock_on(&pdev->dev);
> +	if (ret) {
> +		dev_err(sdev->dev, "adsp_clock_on fail!\n");
> +		return -EINVAL;
> +	}
> +
>   	ret = adsp_sram_power_on(sdev->dev, true);
>   	if (ret) {
>   		dev_err(sdev->dev, "adsp_sram_power_on fail!\n");
> -		return ret;
> +		ret = -EINVAL;
> +		goto exit_clk_disable;
Why are you overriding adsp_sram_power_on()'s return value?
As of now, this function is supposed to return -ENOMEM or 0.. and you
shouldn't override the return value with -EINVAL here.

>   	}
>   
>   	ret = adsp_memory_remap_init(&pdev->dev, priv->adsp);
> @@ -282,6 +296,8 @@ static int mt8195_dsp_probe(struct snd_sof_dev *sdev)
>   
>   err_adsp_sram_power_off:
>   	adsp_sram_power_on(&pdev->dev, false);
> +exit_clk_disable:
> +	adsp_clock_off(&pdev->dev);
>   
>   	return ret;
>   }
> @@ -290,7 +306,10 @@ static int mt8195_dsp_remove(struct snd_sof_dev *sdev)
>   {
>   	struct platform_device *pdev = container_of(sdev->dev, struct platform_device, dev);
>   
> -	return adsp_sram_power_on(&pdev->dev, false);
> +	adsp_sram_power_on(&pdev->dev, false);
> +	adsp_clock_off(&pdev->dev);
> +
> +	return 0;
>   }
>   
>   /* on mt8195 there is 1 to 1 match between type and BAR idx */
> 

Regards,
- Angelo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

end of thread, other threads:[~2021-10-21 11:29 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-21  3:58 [PATCH v2 0/2] Add code to manage DSP clocks and provide dts-binding document YC Hung
2021-10-21  3:58 ` YC Hung
2021-10-21  3:58 ` YC Hung
2021-10-21  3:58 ` YC Hung
2021-10-21  3:58 ` [PATCH v2 1/2] ASoC: SOF: mediatek: Add mt8195 dsp clock support YC Hung
2021-10-21  3:58   ` YC Hung
2021-10-21  3:58   ` YC Hung
2021-10-21  3:58   ` YC Hung
2021-10-21 11:28   ` AngeloGioacchino Del Regno
2021-10-21 11:28     ` AngeloGioacchino Del Regno
2021-10-21 11:28     ` AngeloGioacchino Del Regno
2021-10-21  3:58 ` [PATCH v2 2/2] dt-bindings: dsp: mediatek: Add mt8195 DSP binding support YC Hung
2021-10-21  3:58   ` YC Hung
2021-10-21  3:58   ` YC Hung
2021-10-21  3:58   ` YC Hung

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.