Re: [PATCH v4 19/34] clk: mediatek: Add MT8192 imp i2c wrapper c clock support

Yingjoe Chen
2020-10-24T16:22:48Z

On Thu, 2020-10-22 at 20:37 +0800, Weiyi Lu wrote:
> Add MT8192 imp i2c wrapper c clock provider
>
> Signed-off-by: Weiyi Lu <weiyi.lu@mediatek.com>
> ---
> drivers/clk/mediatek/Kconfig | 6 +++
> drivers/clk/mediatek/Makefile | 1 +
> drivers/clk/mediatek/clk-mt8192-imp_iic_wrap_c.c | 62 ++++++++++++++++++++++++
> 3 files changed, 69 insertions(+)
> create mode 100644 drivers/clk/mediatek/clk-mt8192-imp_iic_wrap_c.c
>
> diff --git a/drivers/clk/mediatek/Kconfig b/drivers/clk/mediatek/Kconfig
> index 99b0168..a0eb76d 100644
> --- a/drivers/clk/mediatek/Kconfig
> +++ b/drivers/clk/mediatek/Kconfig
> @@ -491,6 +491,12 @@ config COMMON_CLK_MT8192_IMGSYS2
> help
> This driver supports MediaTek MT8192 imgsys2 clocks.
>
> +config COMMON_CLK_MT8192_IMP_IIC_WRAP_C
> + bool "Clock driver for MediaTek MT8192 imp_iic_wrap_c"
> + depends on COMMON_CLK_MT8192
> + help
> + This driver supports MediaTek MT8192 imp_iic_wrap_c clocks.
> +
> config COMMON_CLK_MT8516
> bool "Clock driver for MediaTek MT8516"
> depends on ARCH_MEDIATEK || COMPILE_TEST
> diff --git a/drivers/clk/mediatek/Makefile b/drivers/clk/mediatek/Makefile
> index 012a01a..8aac821 100644
> --- a/drivers/clk/mediatek/Makefile
> +++ b/drivers/clk/mediatek/Makefile
> @@ -69,5 +69,6 @@ obj-$(CONFIG_COMMON_CLK_MT8192_CAMSYS_RAWB) += clk-mt8192-cam_rawb.o
> obj-$(CONFIG_COMMON_CLK_MT8192_CAMSYS_RAWC) += clk-mt8192-cam_rawc.o
> obj-$(CONFIG_COMMON_CLK_MT8192_IMGSYS) += clk-mt8192-img.o
> obj-$(CONFIG_COMMON_CLK_MT8192_IMGSYS2) += clk-mt8192-img2.o
> +obj-$(CONFIG_COMMON_CLK_MT8192_IMP_IIC_WRAP_C) += clk-mt8192-imp_iic_wrap_c.o
> obj-$(CONFIG_COMMON_CLK_MT8516) += clk-mt8516.o
> obj-$(CONFIG_COMMON_CLK_MT8516_AUDSYS) += clk-mt8516-aud.o
> diff --git a/drivers/clk/mediatek/clk-mt8192-imp_iic_wrap_c.c b/drivers/clk/mediatek/clk-mt8192-imp_iic_wrap_c.c
> new file mode 100644
> index 0000000..e7a0033
> --- /dev/null
> +++ b/drivers/clk/mediatek/clk-mt8192-imp_iic_wrap_c.c
> @@ -0,0 +1,62 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +//
> +// Copyright (c) 2020 MediaTek Inc.
> +// Author: Weiyi Lu <weiyi.lu@mediatek.com>
> +
> +#include <linux/clk-provider.h>
> +#include <linux/platform_device.h>
> +
> +#include "clk-mtk.h"
> +#include "clk-gate.h"
> +
> +#include <dt-bindings/clock/mt8192-clk.h>
> +
> +static const struct mtk_gate_regs imp_iic_wrap_c_cg_regs = {
> + .set_ofs = 0xe08,
> + .clr_ofs = 0xe04,
> + .sta_ofs = 0xe00,
> +};
> +
> +#define GATE_IMP_IIC_WRAP_C(_id, _name, _parent, _shift) \
> + GATE_MTK_FLAGS(_id, _name, _parent, &imp_iic_wrap_c_cg_regs, _shift, \
> + &mtk_clk_gate_ops_setclr, CLK_OPS_PARENT_ENABLE)
> +
> +static const struct mtk_gate imp_iic_wrap_c_clks[] = {
> + GATE_IMP_IIC_WRAP_C(CLK_IMP_IIC_WRAP_C_I2C10, "imp_iic_wrap_c_i2c10", "infra_i2c0", 0),
> + GATE_IMP_IIC_WRAP_C(CLK_IMP_IIC_WRAP_C_I2C11, "imp_iic_wrap_c_i2c11", "infra_i2c0", 1),
> + GATE_IMP_IIC_WRAP_C(CLK_IMP_IIC_WRAP_C_I2C12, "imp_iic_wrap_c_i2c12", "infra_i2c0", 2),
> + GATE_IMP_IIC_WRAP_C(CLK_IMP_IIC_WRAP_C_I2C13, "imp_iic_wrap_c_i2c13", "infra_i2c0", 3),
> +};
> +
> +static int clk_mt8192_imp_iic_wrap_c_probe(struct platform_device *pdev)
> +{
> + struct clk_onecell_data *clk_data;
> + struct device_node *node = pdev->dev.of_node;
> + int r;
> +
> + clk_data = mtk_alloc_clk_data(CLK_IMP_IIC_WRAP_C_NR_CLK);
> + if (!clk_data)
> + return -ENOMEM;
> +
> + r = mtk_clk_register_gates(node, imp_iic_wrap_c_clks, ARRAY_SIZE(imp_iic_wrap_c_clks),
> + clk_data);
> + if (r)
> + return r;
> +
> + return of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
> +}
> +
> +static const struct of_device_id of_match_clk_mt8192_imp_iic_wrap_c[] = {
> + { .compatible = "mediatek,mt8192-imp_iic_wrap_c", },
> + {}
> +};

It seems these mt8192-imp_iic_wrap_* drivers are very similar.
I think it make more sense to use 1 single driver to provide them, ie:

+static const struct of_device_id of_match_clk_mt8192_imp_iic_wrap[] = {
+ { .compatible = "mediatek,mt8192-imp_iic_wrap_c", imp_iic_wrap_c_clks},
+ { .compatible = "mediatek,mt8192-imp_iic_wrap_e", imp_iic_wrap_e_clks},
+ { .compatible = "mediatek,mt8192-imp_iic_wrap_n", imp_iic_wrap_n_clks},
....
+ {}
+};

Maybe other clk drivers can be merged to the same driverl.

Joe.C