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

[Yingjoe Chen]

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@xxxxxxxxxxxx>
> ---
>  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@xxxxxxxxxxxx>
> +
> +#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