Re: [PATCH v6 1/3] reset: mediatek: Move MediaTek system clock reset to reset/mediatek
From: AngeloGioacchino Del Regno
Date: Tue Oct 25 2022 - 06:36:30 EST
Il 21/10/22 12:48, Bo-Chen Chen ha scritto:
To manager MediaTek system clock reset easier, we move the driver to
drivers/reset/mediatek.
- Create reset/mediatek folder.
- Move clk/mediatek/reset.c to reset/mediatek/reset-mediatek-sysclk.c
- Because we don't want to build in unsed static variable, we use clk
KConfig to separate them. For example, when we use MT8186, we don't
want to build in the static constants for MT8195.
- Move reset data which are scattered around the mediatek drivers to
reset-mtxxxx.c.
- There are two version for mtk_reset_init because some mediatek clock
drivers (mt8135 and mt8173) are using device_node instead of device,
so we need to add two version for the init function.
Suggested-by: Stephen Boyd <sboyd@xxxxxxxxxx>
Signed-off-by: Bo-Chen Chen <rex-bc.chen@xxxxxxxxxxxx>
---
drivers/clk/mediatek/Kconfig | 1 +
drivers/clk/mediatek/Makefile | 2 +-
drivers/clk/mediatek/clk-mt2701-eth.c | 10 +-
drivers/clk/mediatek/clk-mt2701-g3d.c | 10 +-
drivers/clk/mediatek/clk-mt2701-hif.c | 10 +-
drivers/clk/mediatek/clk-mt2701.c | 22 +-
drivers/clk/mediatek/clk-mt2712.c | 22 +-
drivers/clk/mediatek/clk-mt6795-infracfg.c | 22 +-
drivers/clk/mediatek/clk-mt6795-pericfg.c | 20 +-
drivers/clk/mediatek/clk-mt7622-eth.c | 10 +-
drivers/clk/mediatek/clk-mt7622-hif.c | 12 +-
drivers/clk/mediatek/clk-mt7622.c | 22 +-
drivers/clk/mediatek/clk-mt7629-eth.c | 10 +-
drivers/clk/mediatek/clk-mt7629-hif.c | 12 +-
drivers/clk/mediatek/clk-mt8135.c | 23 +-
drivers/clk/mediatek/clk-mt8173.c | 22 +-
drivers/clk/mediatek/clk-mt8183.c | 15 +-
drivers/clk/mediatek/clk-mt8186-infra_ao.c | 23 +-
drivers/clk/mediatek/clk-mt8192.c | 27 +-
drivers/clk/mediatek/clk-mt8195-infra_ao.c | 28 +-
drivers/clk/mediatek/clk-mtk.c | 5 +-
drivers/clk/mediatek/clk-mtk.h | 5 +-
drivers/clk/mediatek/reset.c | 233 -----------
drivers/reset/Kconfig | 1 +
drivers/reset/Makefile | 1 +
drivers/reset/mediatek/Kconfig | 5 +
drivers/reset/mediatek/Makefile | 13 +
.../reset/mediatek/reset-mediatek-sysclk.c | 388 ++++++++++++++++++
drivers/reset/mediatek/reset-mt2701.c | 102 +++++
drivers/reset/mediatek/reset-mt2712.c | 42 ++
drivers/reset/mediatek/reset-mt6795.c | 61 +++
drivers/reset/mediatek/reset-mt7622.c | 91 ++++
drivers/reset/mediatek/reset-mt7629.c | 62 +++
drivers/reset/mediatek/reset-mt8135.c | 43 ++
drivers/reset/mediatek/reset-mt8173.c | 43 ++
drivers/reset/mediatek/reset-mt8183.c | 31 ++
drivers/reset/mediatek/reset-mt8186.c | 39 ++
drivers/reset/mediatek/reset-mt8192.c | 43 ++
drivers/reset/mediatek/reset-mt8195.c | 44 ++
.../linux/reset/reset-mediatek-sysclk.h | 62 +--
40 files changed, 1080 insertions(+), 557 deletions(-)
delete mode 100644 drivers/clk/mediatek/reset.c
create mode 100644 drivers/reset/mediatek/Kconfig
create mode 100644 drivers/reset/mediatek/Makefile
create mode 100644 drivers/reset/mediatek/reset-mediatek-sysclk.c
create mode 100644 drivers/reset/mediatek/reset-mt2701.c
create mode 100644 drivers/reset/mediatek/reset-mt2712.c
create mode 100644 drivers/reset/mediatek/reset-mt6795.c
create mode 100644 drivers/reset/mediatek/reset-mt7622.c
create mode 100644 drivers/reset/mediatek/reset-mt7629.c
create mode 100644 drivers/reset/mediatek/reset-mt8135.c
create mode 100644 drivers/reset/mediatek/reset-mt8173.c
create mode 100644 drivers/reset/mediatek/reset-mt8183.c
create mode 100644 drivers/reset/mediatek/reset-mt8186.c
create mode 100644 drivers/reset/mediatek/reset-mt8192.c
create mode 100644 drivers/reset/mediatek/reset-mt8195.c
rename drivers/clk/mediatek/reset.h => include/linux/reset/reset-mediatek-sysclk.h (59%)
diff --git a/drivers/clk/mediatek/Kconfig b/drivers/clk/mediatek/Kconfig
index 843cea0c7a44..e372f145eada 100644
--- a/drivers/clk/mediatek/Kconfig
+++ b/drivers/clk/mediatek/Kconfig
@@ -8,6 +8,7 @@ menu "Clock driver for MediaTek SoC"
config COMMON_CLK_MEDIATEK
tristate
select RESET_CONTROLLER
+ select RESET_MEDIATEK_SYSCLK
help
MediaTek SoCs' clock support.
..snip..
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index 3e7e5fd633a8..5cef7ccc9a7d 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -1,6 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
obj-y += core.o
obj-y += hisilicon/
+obj-y += mediatek/
I'd be more for
obj-$(CONFIG_ARCH_MEDIATEK) += mediatek/
as there's no reason to even compile these if MTK support isn't enabled at all.
obj-$(CONFIG_ARCH_STI) += sti/
obj-$(CONFIG_ARCH_TEGRA) += tegra/
obj-$(CONFIG_RESET_A10SR) += reset-a10sr.o
diff --git a/drivers/reset/mediatek/Kconfig b/drivers/reset/mediatek/Kconfig
new file mode 100644
index 000000000000..a416cb938753
--- /dev/null
+++ b/drivers/reset/mediatek/Kconfig
@@ -0,0 +1,5 @@
+# SPDX-License-Identifier: GPL-2.0-only
Similarly, we should at this point also do....
if ARCH_MEDIATEK
+config RESET_MEDIATEK_SYSCLK
+ tristate "MediaTek System Clock Reset Driver"
+ help
+ This enables the system clock reset driver for MediaTek SoCs.
endif # ARCH_MEDIATEK
diff --git a/drivers/reset/mediatek/Makefile b/drivers/reset/mediatek/Makefile
new file mode 100644
index 000000000000..83f26c2cecdd
--- /dev/null
+++ b/drivers/reset/mediatek/Makefile
@@ -0,0 +1,13 @@
+# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_RESET_MEDIATEK_SYSCLK) += reset-mediatek-sysclk.o
+obj-$(CONFIG_COMMON_CLK_MT2701) += reset-mt2701.o
+obj-$(CONFIG_COMMON_CLK_MT2712) += reset-mt2712.o
+obj-$(CONFIG_COMMON_CLK_MT6795) += reset-mt6795.o
+obj-$(CONFIG_COMMON_CLK_MT7622) += reset-mt7622.o
+obj-$(CONFIG_COMMON_CLK_MT7629) += reset-mt7629.o
+obj-$(CONFIG_COMMON_CLK_MT8135) += reset-mt8135.o
+obj-$(CONFIG_COMMON_CLK_MT8173) += reset-mt8173.o
+obj-$(CONFIG_COMMON_CLK_MT8183) += reset-mt8183.o
+obj-$(CONFIG_COMMON_CLK_MT8186) += reset-mt8186.o
+obj-$(CONFIG_COMMON_CLK_MT8192) += reset-mt8192.o
+obj-$(CONFIG_COMMON_CLK_MT8195) += reset-mt8195.o
diff --git a/drivers/reset/mediatek/reset-mediatek-sysclk.c b/drivers/reset/mediatek/reset-mediatek-sysclk.c
new file mode 100644
index 000000000000..9cf115e66a4d
--- /dev/null
+++ b/drivers/reset/mediatek/reset-mediatek-sysclk.c
..snip..
+
+static struct mtk_rst_id mtk_sysclk_reset_ids[] = {
..snip..
+ {
+ .name = "mt2712-peri-rst",
+ .driver_data = MTK_RST_ID_MT2712_PERI,
+ },
+ {
+ .name = "mt6795-ifa",
Keep the names consistent please... "mt6795-infra-rst"
+ .driver_data = MTK_RST_ID_MT6795_INFRA,
+ },
+ {
+ .name = "mt6795-peri",
mt6795-peri-rst
+ .driver_data = MTK_RST_ID_MT6795_PERI,
+ },
+ {
+ .name = "mt7622-eth-rst",
+ .driver_data = MTK_RST_ID_MT7622_ETH,
+ },
+ {
+ .name = "mt7622-usb-rst",
+ .driver_data = MTK_RST_ID_MT7622_SSUSBSYS,
+ },
+ {
+ .name = "mt7622-pcie-rst",
+ .driver_data = MTK_RST_ID_MT7622_PCIESYS,
+ },
+ {
+ .name = "mt7622-infrasys-rst",
+ .driver_data = MTK_RST_ID_MT7622_INFRASYS,
+ },
+ {
+ .name = "mt7622-pericfg-rst",
+ .driver_data = MTK_RST_ID_MT7622_PERICFG,
+ },
+ {
+ .name = "mt7629-ethsys-rst",
+ .driver_data = MTK_RST_ID_MT7629_ETHSYS,
+ },
+ {
+ .name = "mt7629-usb-rst",
+ .driver_data = MTK_RST_ID_MT7629_SSUSBSYS,
+ },
+ {
+ .name = "mt7629-pcie-rst",
+ .driver_data = MTK_RST_ID_MT7629_PCIESYS,
+ },
+ {
+ .name = "clk_mt8135.mt8135-infrasys-rst",
Why do we have this "clk_mt8135." prefix here (and also mt8173), when all of
the others don't have any prefix?
That's not consistent and shall be changed, unless there's a valid reason not to.
+ .driver_data = MTK_RST_ID_MT8135_INFRASYS,
+ },
+ {
+ .name = "clk_mt8135.mt8135-pericfg-rst",
+ .driver_data = MTK_RST_ID_MT8135_PERICFG,
+ },
+ {
+ .name = "clk_mt8173.mt8173-infracfg-rst",
+ .driver_data = MTK_RST_ID_MT8173_INFRACFG,
+ },
+ {
+ .name = "clk_mt8173.mt8173-pericfg-rst",
+ .driver_data = MTK_RST_ID_MT8173_PERICFG,
+ },
+ {
+ .name = "mt8183-infra-rst",
+ .driver_data = MTK_RST_ID_MT8183_INFRA,
+ },
+ {
+ .name = "mt8186-infra-ao-rst",
+ .driver_data = MTK_RST_ID_MT8186_INFRA_AO,
+ },
+ {
+ .name = "mt8192-infra-rst",
+ .driver_data = MTK_RST_ID_MT8192_INFRA,
+ },
+ {
+ .name = "mt8195-infra-ao-rst",
+ .driver_data = MTK_RST_ID_MT8195_INFRA_AO,
+ },
+ {
+ },
+};
+
..snip..
+
+int mtk_rst_register_clk_rst_data(u32 index, struct mtk_clk_rst_data *data)
+{
+ if (index >= MTK_RST_ID_MAX)
+ return -EINVAL;
+
+ p_clk_rst_data[index] = data;
+
+ pr_info("%s, register mediatek sysclock reset(%d).\n", __func__, index);
Is this really informative?
There's sysfs telling you infos about drivers that has been (or hasn't been yet)
registered... so, please change that to a pr_debug() instead.
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(mtk_rst_register_clk_rst_data);
+
Apart from that, looks good.
Regards,
Angelo