Re: [PATCH v11 3/5] soc: mediatek: Move mt8173 MMSYS to platform driver

Hi Stephen,

On 5/3/20 22:01, Stephen Boyd wrote:

Quoting Enric Balletbo i Serra (2020-03-02 03:01:26)

quoted

From: Matthias Brugger <mbrugger@suse.com>

There is no strong reason for this to use CLK_OF_DECLARE instead of
being a platform driver.

Cool.

quoted

Plus, this driver provides clocks but also
a shared register space for the mediatek-drm and the mediatek-mdp
driver. So move to drivers/soc/mediatek as a platform driver.

Signed-off-by: Matthias Brugger <mbrugger@suse.com>
Signed-off-by: Enric Balletbo i Serra <redacted>
Reviewed-by: CK Hu <redacted>
---

Changes in v11: None
Changes in v10:
- Renamed to be generic mtk-mmsys
- Add driver data support to be able to support diferent SoCs

Changes in v9:
- Move mmsys to drivers/soc/mediatek (CK)

Changes in v8:
- Be a builtin_platform_driver like other mediatek mmsys drivers.

Changes in v7:
- Free clk_data->clks as well
- Get rid of private data structure

 drivers/clk/mediatek/clk-mt8173.c | 104 --------------------
 drivers/soc/mediatek/Kconfig      |   7 ++
 drivers/soc/mediatek/Makefile     |   1 +
 drivers/soc/mediatek/mtk-mmsys.c  | 154 ++++++++++++++++++++++++++++++

Can you generate with -M so that we can see what has actually changed?

Sure, sorry about that.

quoted

 4 files changed, 162 insertions(+), 104 deletions(-)
 create mode 100644 drivers/soc/mediatek/mtk-mmsys.c

diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
index 2114b563478c..7a156944d50e 100644
--- a/drivers/soc/mediatek/Kconfig
+++ b/drivers/soc/mediatek/Kconfig

@@ -44,4 +44,11 @@ config MTK_SCPSYS
          Say yes here to add support for the MediaTek SCPSYS power domain
          driver.
 
+config MTK_MMSYS
+       bool "MediaTek MMSYS Support"
+       depends on COMMON_CLK_MT8173

Does it need some default so that defconfig updates don't break things?

Right.

quoted

+       help
+         Say yes here to add support for the MediaTek Multimedia
+         Subsystem (MMSYS).
+
 endmenu

diff --git a/drivers/soc/mediatek/Makefile b/drivers/soc/mediatek/Makefile
index b01733074ad6..01f9f873634a 100644
--- a/drivers/soc/mediatek/Makefile
+++ b/drivers/soc/mediatek/Makefile

@@ -3,3 +3,4 @@ obj-$(CONFIG_MTK_CMDQ) += mtk-cmdq-helper.o
 obj-$(CONFIG_MTK_INFRACFG) += mtk-infracfg.o
 obj-$(CONFIG_MTK_PMIC_WRAP) += mtk-pmic-wrap.o
 obj-$(CONFIG_MTK_SCPSYS) += mtk-scpsys.o
+obj-$(CONFIG_MTK_MMSYS) += mtk-mmsys.o

diff --git a/drivers/soc/mediatek/mtk-mmsys.c b/drivers/soc/mediatek/mtk-mmsys.c
new file mode 100644
index 000000000000..473cdf732fb5
--- /dev/null
+++ b/drivers/soc/mediatek/mtk-mmsys.c

@@ -0,0 +1,154 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2014 MediaTek Inc.
+ * Author: James Liao <jamesjj.liao@mediatek.com>
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+
+#include "../../clk/mediatek/clk-gate.h"
+#include "../../clk/mediatek/clk-mtk.h"

Why not use include/linux/clk/?

I can move these files to include, this will impact a lot more of drivers but,
yes, I think is the right way.

But I also don't understand why the clk driver is moved outside of
drivers/clk/ into drivers/soc/. Commit text saying that it has shared
registers doesn't mean it can't still keep the clk driver part in the
drivers/clk/ area.

Actually moving this to the soc directory has been requested by CK (mediatek) as
a change in v8. You can see the discussion in [1]

Thanks,
 Enric

[1] https://patchwork.kernel.org/cover/11394709/

quoted

+
+#include <dt-bindings/clock/mt8173-clk.h>
+
+static const struct mtk_gate_regs mm0_cg_regs = {
+       .set_ofs = 0x0104,
+       .clr_ofs = 0x0108,
+       .sta_ofs = 0x0100,
+};
+
+static const struct mtk_gate_regs mm1_cg_regs = {
+       .set_ofs = 0x0114,
+       .clr_ofs = 0x0118,
+       .sta_ofs = 0x0110,
+};