Re: [PATCH v6 07/19] clk: mediatek: Add MT8188 ccusys clock support

Re: [PATCH v6 07/19] clk: mediatek: Add MT8188 ccusys clock support

[AngeloGioacchino Del Regno]

Il 09/03/23 14:54, Garmin.Chang ha scritto:
> Add MT8188 ccusys clock controller which provides clock gate
> control in Camera Computing Unit.
> 
> Signed-off-by: Garmin.Chang <Garmin.Chang@mediatek.com>
> ---
>   drivers/clk/mediatek/Makefile         |  2 +-
>   drivers/clk/mediatek/clk-mt8188-ccu.c | 48 +++++++++++++++++++++++++++
>   2 files changed, 49 insertions(+), 1 deletion(-)
>   create mode 100644 drivers/clk/mediatek/clk-mt8188-ccu.c
> 
> diff --git a/drivers/clk/mediatek/Makefile b/drivers/clk/mediatek/Makefile
> index a4189d28cecc..fb66d25e98fd 100644
> --- a/drivers/clk/mediatek/Makefile
> +++ b/drivers/clk/mediatek/Makefile
> @@ -93,7 +93,7 @@ obj-$(CONFIG_COMMON_CLK_MT8186) += clk-mt8186-mcu.o clk-mt8186-topckgen.o clk-mt
>   				   clk-mt8186-cam.o clk-mt8186-mdp.o clk-mt8186-ipe.o
>   obj-$(CONFIG_COMMON_CLK_MT8188) += clk-mt8188-apmixedsys.o clk-mt8188-topckgen.o \
>   				   clk-mt8188-peri_ao.o clk-mt8188-infra_ao.o \
> -				   clk-mt8188-cam.o
> +				   clk-mt8188-cam.o clk-mt8188-ccu.o

clk-mt8188-cam and clk-mt8188-ccu can go under a different configuration option
for modularity.

For example...

obj-$(CONFIG_COMMON_CLK_MT8188_CAM) +=  ...ccu.o, ...cam.o

Please make sure, for boot performance purposes, to order them as:

obj-$(CONFIG_.....) += driver-clk1.o driver-requiring-clk1-clocks.o

>   obj-$(CONFIG_COMMON_CLK_MT8192) += clk-mt8192.o
>   obj-$(CONFIG_COMMON_CLK_MT8192_AUDSYS) += clk-mt8192-aud.o
>   obj-$(CONFIG_COMMON_CLK_MT8192_CAMSYS) += clk-mt8192-cam.o
> diff --git a/drivers/clk/mediatek/clk-mt8188-ccu.c b/drivers/clk/mediatek/clk-mt8188-ccu.c
> new file mode 100644
> index 000000000000..b7380060f906
> --- /dev/null
> +++ b/drivers/clk/mediatek/clk-mt8188-ccu.c
> @@ -0,0 +1,48 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +//
> +// Copyright (c) 2022 MediaTek Inc.
> +// Author: Garmin Chang <garmin.chang@mediatek.com>
> +
> +#include <linux/clk-provider.h>
> +#include <linux/platform_device.h>
> +#include <dt-bindings/clock/mediatek,mt8188-clk.h>
> +
> +#include "clk-gate.h"
> +#include "clk-mtk.h"
> +
> +static const struct mtk_gate_regs ccu_cg_regs = {
> +	.set_ofs = 0x4,
> +	.clr_ofs = 0x8,
> +	.sta_ofs = 0x0,
> +};
> +
> +#define GATE_CCU(_id, _name, _parent, _shift)			\
> +	GATE_MTK(_id, _name, _parent, &ccu_cg_regs, _shift, &mtk_clk_gate_ops_setclr)
> +
> +static const struct mtk_gate ccu_clks[] = {
> +	GATE_CCU(CLK_CCU_LARB27, "ccu_larb27", "top_ccu", 0),
> +	GATE_CCU(CLK_CCU_AHB, "ccu_ahb", "top_ccu", 1),
> +	GATE_CCU(CLK_CCU_CCU0, "ccu_ccu0", "top_ccu", 2),
> +};
> +
> +static const struct mtk_clk_desc ccu_desc = {
> +	.clks = ccu_clks,
> +	.num_clks = ARRAY_SIZE(ccu_clks),
> +};
> +
> +static const struct of_device_id of_match_clk_mt8188_ccu[] = {
> +	{ .compatible = "mediatek,mt8188-ccusys", .data = &ccu_desc},

Missing space: { [...] &ccu_desc },

> +	{ /* sentinel */ }
> +};
> +
> +static struct platform_driver clk_mt8188_ccu_drv = {
> +	.probe = mtk_clk_simple_probe,
> +	.remove = mtk_clk_simple_remove,
> +	.driver = {
> +		.name = "clk-mt8188-ccu",
> +		.of_match_table = of_match_clk_mt8188_ccu,
> +	},
> +};
> +
> +builtin_platform_driver(clk_mt8188_ccu_drv);

module_platform_driver

> +MODULE_LICENSE("GPL");

Re: [PATCH v6 07/19] clk: mediatek: Add MT8188 ccusys clock support

[Garmin Chang (張家銘)]

On Tue, 2023-03-14 at 10:05 +0100, AngeloGioacchino Del Regno wrote:
> Il 09/03/23 14:54, Garmin.Chang ha scritto:
> > Add MT8188 ccusys clock controller which provides clock gate
> > control in Camera Computing Unit.
> > 
> > Signed-off-by: Garmin.Chang <Garmin.Chang@mediatek.com>
> > ---
> >   drivers/clk/mediatek/Makefile         |  2 +-
> >   drivers/clk/mediatek/clk-mt8188-ccu.c | 48
> > +++++++++++++++++++++++++++
> >   2 files changed, 49 insertions(+), 1 deletion(-)
> >   create mode 100644 drivers/clk/mediatek/clk-mt8188-ccu.c
> > 
> > diff --git a/drivers/clk/mediatek/Makefile
> > b/drivers/clk/mediatek/Makefile
> > index a4189d28cecc..fb66d25e98fd 100644
> > --- a/drivers/clk/mediatek/Makefile
> > +++ b/drivers/clk/mediatek/Makefile
> > @@ -93,7 +93,7 @@ obj-$(CONFIG_COMMON_CLK_MT8186) += clk-mt8186-
> > mcu.o clk-mt8186-topckgen.o clk-mt
> >   				   clk-mt8186-cam.o clk-mt8186-mdp.o
> > clk-mt8186-ipe.o
> >   obj-$(CONFIG_COMMON_CLK_MT8188) += clk-mt8188-apmixedsys.o clk-
> > mt8188-topckgen.o \
> >   				   clk-mt8188-peri_ao.o clk-mt8188-
> > infra_ao.o \
> > -				   clk-mt8188-cam.o
> > +				   clk-mt8188-cam.o clk-mt8188-ccu.o
> 
> clk-mt8188-cam and clk-mt8188-ccu can go under a different
> configuration option
> for modularity.
> 
> For example...
> 
> obj-$(CONFIG_COMMON_CLK_MT8188_CAM) +=  ...ccu.o, ...cam.o
> 
> Please make sure, for boot performance purposes, to order them as:
> 
> obj-$(CONFIG_.....) += driver-clk1.o driver-requiring-clk1-clocks.o
> 
Thank you for your suggestions.

Ok, I will refer to other chips to modify mt8188 about this in v7.

> >   obj-$(CONFIG_COMMON_CLK_MT8192) += clk-mt8192.o
> >   obj-$(CONFIG_COMMON_CLK_MT8192_AUDSYS) += clk-mt8192-aud.o
> >   obj-$(CONFIG_COMMON_CLK_MT8192_CAMSYS) += clk-mt8192-cam.o
> > diff --git a/drivers/clk/mediatek/clk-mt8188-ccu.c
> > b/drivers/clk/mediatek/clk-mt8188-ccu.c
> > new file mode 100644
> > index 000000000000..b7380060f906
> > --- /dev/null
> > +++ b/drivers/clk/mediatek/clk-mt8188-ccu.c
> > @@ -0,0 +1,48 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +//
> > +// Copyright (c) 2022 MediaTek Inc.
> > +// Author: Garmin Chang <garmin.chang@mediatek.com>
> > +
> > +#include <linux/clk-provider.h>
> > +#include <linux/platform_device.h>
> > +#include <dt-bindings/clock/mediatek,mt8188-clk.h>
> > +
> > +#include "clk-gate.h"
> > +#include "clk-mtk.h"
> > +
> > +static const struct mtk_gate_regs ccu_cg_regs = {
> > +	.set_ofs = 0x4,
> > +	.clr_ofs = 0x8,
> > +	.sta_ofs = 0x0,
> > +};
> > +
> > +#define GATE_CCU(_id, _name, _parent, _shift)			
> > \
> > +	GATE_MTK(_id, _name, _parent, &ccu_cg_regs, _shift,
> > &mtk_clk_gate_ops_setclr)
> > +
> > +static const struct mtk_gate ccu_clks[] = {
> > +	GATE_CCU(CLK_CCU_LARB27, "ccu_larb27", "top_ccu", 0),
> > +	GATE_CCU(CLK_CCU_AHB, "ccu_ahb", "top_ccu", 1),
> > +	GATE_CCU(CLK_CCU_CCU0, "ccu_ccu0", "top_ccu", 2),
> > +};
> > +
> > +static const struct mtk_clk_desc ccu_desc = {
> > +	.clks = ccu_clks,
> > +	.num_clks = ARRAY_SIZE(ccu_clks),
> > +};
> > +
> > +static const struct of_device_id of_match_clk_mt8188_ccu[] = {
> > +	{ .compatible = "mediatek,mt8188-ccusys", .data = &ccu_desc},
> 
> Missing space: { [...] &ccu_desc },
> 
Ok, I'll fix this in v7.

> > +	{ /* sentinel */ }
> > +};
> > +
> > +static struct platform_driver clk_mt8188_ccu_drv = {
> > +	.probe = mtk_clk_simple_probe,
> > +	.remove = mtk_clk_simple_remove,
> > +	.driver = {
> > +		.name = "clk-mt8188-ccu",
> > +		.of_match_table = of_match_clk_mt8188_ccu,
> > +	},
> > +};
> > +
> > +builtin_platform_driver(clk_mt8188_ccu_drv);
> 
> module_platform_driver
> 
> > +MODULE_LICENSE("GPL");
> 
> 

Re: [PATCH v6 05/19] clk: mediatek: Add MT8188 infrastructure clock support

[AngeloGioacchino Del Regno]

Il 09/03/23 14:54, Garmin.Chang ha scritto:
> Add MT8188 infrastructure clock controller which provides
> clock gate control for basic IP like pwm, uart, spi and so on.
> 
> Signed-off-by: Garmin.Chang <Garmin.Chang@mediatek.com>
> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
> ---
>   drivers/clk/mediatek/Makefile              |   2 +-
>   drivers/clk/mediatek/clk-mt8188-infra_ao.c | 196 +++++++++++++++++++++
>   2 files changed, 197 insertions(+), 1 deletion(-)
>   create mode 100644 drivers/clk/mediatek/clk-mt8188-infra_ao.c
> 
> diff --git a/drivers/clk/mediatek/Makefile b/drivers/clk/mediatek/Makefile
> index f38a5cea2925..172aaef29d5d 100644
> --- a/drivers/clk/mediatek/Makefile
> +++ b/drivers/clk/mediatek/Makefile
> @@ -92,7 +92,7 @@ obj-$(CONFIG_COMMON_CLK_MT8186) += clk-mt8186-mcu.o clk-mt8186-topckgen.o clk-mt
>   				   clk-mt8186-img.o clk-mt8186-vdec.o clk-mt8186-venc.o \
>   				   clk-mt8186-cam.o clk-mt8186-mdp.o clk-mt8186-ipe.o
>   obj-$(CONFIG_COMMON_CLK_MT8188) += clk-mt8188-apmixedsys.o clk-mt8188-topckgen.o \
> -				   clk-mt8188-peri_ao.o
> +				   clk-mt8188-peri_ao.o clk-mt8188-infra_ao.o
>   obj-$(CONFIG_COMMON_CLK_MT8192) += clk-mt8192.o
>   obj-$(CONFIG_COMMON_CLK_MT8192_AUDSYS) += clk-mt8192-aud.o
>   obj-$(CONFIG_COMMON_CLK_MT8192_CAMSYS) += clk-mt8192-cam.o
> diff --git a/drivers/clk/mediatek/clk-mt8188-infra_ao.c b/drivers/clk/mediatek/clk-mt8188-infra_ao.c
> new file mode 100644
> index 000000000000..edc0ba18c67f
> --- /dev/null
> +++ b/drivers/clk/mediatek/clk-mt8188-infra_ao.c
> @@ -0,0 +1,196 @@
> +// SPDX-License-Identifier: GPL-2.0-only

> +//
> +// Copyright (c) 2022 MediaTek Inc.
> +// Author: Garmin Chang <garmin.chang@mediatek.com>

Please use C-style comments (apart from the SPDX header) to be consistent with
the other clock drivers.

> +
> +#include <linux/clk-provider.h>
> +#include <linux/platform_device.h>
> +#include <dt-bindings/clock/mediatek,mt8188-clk.h>

order by name.

> +

..snip..

> +
> +static const struct mtk_clk_desc infra_ao_desc = {
> +	.clks = infra_ao_clks,
> +	.num_clks = ARRAY_SIZE(infra_ao_clks),
> +};
> +
> +static const struct of_device_id of_match_clk_mt8188_infra_ao[] = {
> +	{ .compatible = "mediatek,mt8188-infracfg-ao", .data = &infra_ao_desc },
> +	{ /* sentinel */ }
> +};

MODULE_DEVICE_TABLE is missing

> +
> +static struct platform_driver clk_mt8188_infra_ao_drv = {
> +	.probe = mtk_clk_simple_probe,
> +	.remove = mtk_clk_simple_remove,
> +	.driver = {
> +		.name = "clk-mt8188-infra_ao",
> +		.of_match_table = of_match_clk_mt8188_infra_ao,
> +	},
> +};
> +builtin_platform_driver(clk_mt8188_infra_ao_drv);

module_platform_driver()

MODULE_LICENSE


Regards,
Angelo

Re: [PATCH v6 05/19] clk: mediatek: Add MT8188 infrastructure clock support

[Garmin Chang (張家銘)]

On Tue, 2023-03-14 at 10:05 +0100, AngeloGioacchino Del Regno wrote:
> Il 09/03/23 14:54, Garmin.Chang ha scritto:
> > Add MT8188 infrastructure clock controller which provides
> > clock gate control for basic IP like pwm, uart, spi and so on.
> > 
> > Signed-off-by: Garmin.Chang <Garmin.Chang@mediatek.com>
> > Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
> > ---
> >   drivers/clk/mediatek/Makefile              |   2 +-
> >   drivers/clk/mediatek/clk-mt8188-infra_ao.c | 196
> > +++++++++++++++++++++
> >   2 files changed, 197 insertions(+), 1 deletion(-)
> >   create mode 100644 drivers/clk/mediatek/clk-mt8188-infra_ao.c
> > 
> > diff --git a/drivers/clk/mediatek/Makefile
> > b/drivers/clk/mediatek/Makefile
> > index f38a5cea2925..172aaef29d5d 100644
> > --- a/drivers/clk/mediatek/Makefile
> > +++ b/drivers/clk/mediatek/Makefile
> > @@ -92,7 +92,7 @@ obj-$(CONFIG_COMMON_CLK_MT8186) += clk-mt8186-
> > mcu.o clk-mt8186-topckgen.o clk-mt
> >   				   clk-mt8186-img.o clk-mt8186-vdec.o
> > clk-mt8186-venc.o \
> >   				   clk-mt8186-cam.o clk-mt8186-mdp.o
> > clk-mt8186-ipe.o
> >   obj-$(CONFIG_COMMON_CLK_MT8188) += clk-mt8188-apmixedsys.o clk-
> > mt8188-topckgen.o \
> > -				   clk-mt8188-peri_ao.o
> > +				   clk-mt8188-peri_ao.o clk-mt8188-
> > infra_ao.o
> >   obj-$(CONFIG_COMMON_CLK_MT8192) += clk-mt8192.o
> >   obj-$(CONFIG_COMMON_CLK_MT8192_AUDSYS) += clk-mt8192-aud.o
> >   obj-$(CONFIG_COMMON_CLK_MT8192_CAMSYS) += clk-mt8192-cam.o
> > diff --git a/drivers/clk/mediatek/clk-mt8188-infra_ao.c
> > b/drivers/clk/mediatek/clk-mt8188-infra_ao.c
> > new file mode 100644
> > index 000000000000..edc0ba18c67f
> > --- /dev/null
> > +++ b/drivers/clk/mediatek/clk-mt8188-infra_ao.c
> > @@ -0,0 +1,196 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +//
> > +// Copyright (c) 2022 MediaTek Inc.
> > +// Author: Garmin Chang <garmin.chang@mediatek.com>
> 
> Please use C-style comments (apart from the SPDX header) to be
> consistent with
> the other clock drivers.
> 

Thank you for your suggestions.

Ok, I would modify this and below in v7.
> > +
> > +#include <linux/clk-provider.h>
> > +#include <linux/platform_device.h>
> > +#include <dt-bindings/clock/mediatek,mt8188-clk.h>
> 
> order by name.
> 
Ok.
> > +
> 
> ..snip..
> 
> > +
> > +static const struct mtk_clk_desc infra_ao_desc = {
> > +	.clks = infra_ao_clks,
> > +	.num_clks = ARRAY_SIZE(infra_ao_clks),
> > +};
> > +
> > +static const struct of_device_id of_match_clk_mt8188_infra_ao[] =
> > {
> > +	{ .compatible = "mediatek,mt8188-infracfg-ao", .data =
> > &infra_ao_desc },
> > +	{ /* sentinel */ }
> > +};
> 
> MODULE_DEVICE_TABLE is missing
Ok.
> 
> > +
> > +static struct platform_driver clk_mt8188_infra_ao_drv = {
> > +	.probe = mtk_clk_simple_probe,
> > +	.remove = mtk_clk_simple_remove,
> > +	.driver = {
> > +		.name = "clk-mt8188-infra_ao",
> > +		.of_match_table = of_match_clk_mt8188_infra_ao,
> > +	},
> > +};
> > +builtin_platform_driver(clk_mt8188_infra_ao_drv);
> 
> module_platform_driver()
> 
> MODULE_LICENSE
> OK.
> 
> Regards,
> Angelo

Re: [PATCH v6 04/19] clk: mediatek: Add MT8188 peripheral clock support

[AngeloGioacchino Del Regno]

Il 09/03/23 14:54, Garmin.Chang ha scritto:
> Add MT8188 peripheral clock controller which provides clock
> gate control for ethernet/flashif/pcie/ssusb.
> 
> Signed-off-by: Garmin.Chang <Garmin.Chang@mediatek.com>
> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
> ---
>   drivers/clk/mediatek/Makefile             |  3 +-
>   drivers/clk/mediatek/clk-mt8188-peri_ao.c | 56 +++++++++++++++++++++++
>   2 files changed, 58 insertions(+), 1 deletion(-)
>   create mode 100644 drivers/clk/mediatek/clk-mt8188-peri_ao.c
> 
> diff --git a/drivers/clk/mediatek/Makefile b/drivers/clk/mediatek/Makefile
> index d845bf7308c3..f38a5cea2925 100644
> --- a/drivers/clk/mediatek/Makefile
> +++ b/drivers/clk/mediatek/Makefile
> @@ -91,7 +91,8 @@ obj-$(CONFIG_COMMON_CLK_MT8186) += clk-mt8186-mcu.o clk-mt8186-topckgen.o clk-mt
>   				   clk-mt8186-mfg.o clk-mt8186-mm.o clk-mt8186-wpe.o \
>   				   clk-mt8186-img.o clk-mt8186-vdec.o clk-mt8186-venc.o \
>   				   clk-mt8186-cam.o clk-mt8186-mdp.o clk-mt8186-ipe.o
> -obj-$(CONFIG_COMMON_CLK_MT8188) += clk-mt8188-apmixedsys.o clk-mt8188-topckgen.o
> +obj-$(CONFIG_COMMON_CLK_MT8188) += clk-mt8188-apmixedsys.o clk-mt8188-topckgen.o \
> +				   clk-mt8188-peri_ao.o
>   obj-$(CONFIG_COMMON_CLK_MT8192) += clk-mt8192.o
>   obj-$(CONFIG_COMMON_CLK_MT8192_AUDSYS) += clk-mt8192-aud.o
>   obj-$(CONFIG_COMMON_CLK_MT8192_CAMSYS) += clk-mt8192-cam.o
> diff --git a/drivers/clk/mediatek/clk-mt8188-peri_ao.c b/drivers/clk/mediatek/clk-mt8188-peri_ao.c
> new file mode 100644
> index 000000000000..683010453a10
> --- /dev/null
> +++ b/drivers/clk/mediatek/clk-mt8188-peri_ao.c
> @@ -0,0 +1,56 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +//
> +// Copyright (c) 2022 MediaTek Inc.
> +// Author: Garmin Chang <garmin.chang@mediatek.com>
> +
> +#include <linux/clk-provider.h>
> +#include <linux/platform_device.h>
> +#include <dt-bindings/clock/mediatek,mt8188-clk.h>
> +
> +#include "clk-gate.h"
> +#include "clk-mtk.h"
> +
> +static const struct mtk_gate_regs peri_ao_cg_regs = {
> +	.set_ofs = 0x10,
> +	.clr_ofs = 0x14,
> +	.sta_ofs = 0x18,
> +};
> +
> +#define GATE_PERI_AO(_id, _name, _parent, _shift)			\
> +	GATE_MTK(_id, _name, _parent, &peri_ao_cg_regs, _shift, &mtk_clk_gate_ops_setclr)
> +
> +static const struct mtk_gate peri_ao_clks[] = {
> +	GATE_PERI_AO(CLK_PERI_AO_ETHERNET, "peri_ao_ethernet", "top_axi", 0),
> +	GATE_PERI_AO(CLK_PERI_AO_ETHERNET_BUS, "peri_ao_ethernet_bus", "top_axi", 1),
> +	GATE_PERI_AO(CLK_PERI_AO_FLASHIF_BUS, "peri_ao_flashif_bus", "top_axi", 3),
> +	GATE_PERI_AO(CLK_PERI_AO_FLASHIF_26M, "peri_ao_flashif_26m", "clk26m", 4),
> +	GATE_PERI_AO(CLK_PERI_AO_FLASHIFLASHCK, "peri_ao_flashiflashck", "top_spinor", 5),
> +	GATE_PERI_AO(CLK_PERI_AO_SSUSB_2P_BUS, "peri_ao_ssusb_2p_bus", "top_usb_top_2p", 9),
> +	GATE_PERI_AO(CLK_PERI_AO_SSUSB_2P_XHCI, "peri_ao_ssusb_2p_xhci", "top_ssusb_xhci_2p", 10),
> +	GATE_PERI_AO(CLK_PERI_AO_SSUSB_3P_BUS, "peri_ao_ssusb_3p_bus", "top_usb_top_3p", 11),
> +	GATE_PERI_AO(CLK_PERI_AO_SSUSB_3P_XHCI, "peri_ao_ssusb_3p_xhci", "top_ssusb_xhci_3p", 12),
> +	GATE_PERI_AO(CLK_PERI_AO_SSUSB_BUS, "peri_ao_ssusb_bus", "top_usb_top", 13),
> +	GATE_PERI_AO(CLK_PERI_AO_SSUSB_XHCI, "peri_ao_ssusb_xhci", "top_ssusb_xhci", 14),
> +	GATE_PERI_AO(CLK_PERI_AO_ETHERNET_MAC, "peri_ao_ethernet_mac_clk", "top_snps_eth_250m", 16),
> +	GATE_PERI_AO(CLK_PERI_AO_PCIE_P0_FMEM, "peri_ao_pcie_p0_fmem", "hd_466m_fmem_ck", 24),
> +};
> +
> +static const struct mtk_clk_desc peri_ao_desc = {
> +	.clks = peri_ao_clks,
> +	.num_clks = ARRAY_SIZE(peri_ao_clks),
> +};
> +
> +static const struct of_device_id of_match_clk_mt8188_peri_ao[] = {
> +	{ .compatible = "mediatek,mt8188-pericfg-ao", .data = &peri_ao_desc },
> +	{ /* sentinel */ }
> +};

MODULE_DEVICE_TABLE is missing

> +
> +static struct platform_driver clk_mt8188_peri_ao_drv = {
> +	.probe = mtk_clk_simple_probe,
> +	.remove = mtk_clk_simple_remove,
> +	.driver = {
> +		.name = "clk-mt8188-peri_ao",
> +		.of_match_table = of_match_clk_mt8188_peri_ao,
> +	},
> +};
> +builtin_platform_driver(clk_mt8188_peri_ao_drv);

module_platform_driver()

MODULE_LICENSE

Re: [PATCH v6 04/19] clk: mediatek: Add MT8188 peripheral clock support

[Garmin Chang (張家銘)]

On Tue, 2023-03-14 at 10:05 +0100, AngeloGioacchino Del Regno wrote:
> Il 09/03/23 14:54, Garmin.Chang ha scritto:
> > Add MT8188 peripheral clock controller which provides clock
> > gate control for ethernet/flashif/pcie/ssusb.
> > 
> > Signed-off-by: Garmin.Chang <Garmin.Chang@mediatek.com>
> > Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
> > ---
> >   drivers/clk/mediatek/Makefile             |  3 +-
> >   drivers/clk/mediatek/clk-mt8188-peri_ao.c | 56
> > +++++++++++++++++++++++
> >   2 files changed, 58 insertions(+), 1 deletion(-)
> >   create mode 100644 drivers/clk/mediatek/clk-mt8188-peri_ao.c
> > 
> > diff --git a/drivers/clk/mediatek/Makefile
> > b/drivers/clk/mediatek/Makefile
> > index d845bf7308c3..f38a5cea2925 100644
> > --- a/drivers/clk/mediatek/Makefile
> > +++ b/drivers/clk/mediatek/Makefile
> > @@ -91,7 +91,8 @@ obj-$(CONFIG_COMMON_CLK_MT8186) += clk-mt8186-
> > mcu.o clk-mt8186-topckgen.o clk-mt
> >   				   clk-mt8186-mfg.o clk-mt8186-mm.o
> > clk-mt8186-wpe.o \
> >   				   clk-mt8186-img.o clk-mt8186-vdec.o
> > clk-mt8186-venc.o \
> >   				   clk-mt8186-cam.o clk-mt8186-mdp.o
> > clk-mt8186-ipe.o
> > -obj-$(CONFIG_COMMON_CLK_MT8188) += clk-mt8188-apmixedsys.o clk-
> > mt8188-topckgen.o
> > +obj-$(CONFIG_COMMON_CLK_MT8188) += clk-mt8188-apmixedsys.o clk-
> > mt8188-topckgen.o \
> > +				   clk-mt8188-peri_ao.o
> >   obj-$(CONFIG_COMMON_CLK_MT8192) += clk-mt8192.o
> >   obj-$(CONFIG_COMMON_CLK_MT8192_AUDSYS) += clk-mt8192-aud.o
> >   obj-$(CONFIG_COMMON_CLK_MT8192_CAMSYS) += clk-mt8192-cam.o
> > diff --git a/drivers/clk/mediatek/clk-mt8188-peri_ao.c
> > b/drivers/clk/mediatek/clk-mt8188-peri_ao.c
> > new file mode 100644
> > index 000000000000..683010453a10
> > --- /dev/null
> > +++ b/drivers/clk/mediatek/clk-mt8188-peri_ao.c
> > @@ -0,0 +1,56 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +//
> > +// Copyright (c) 2022 MediaTek Inc.
> > +// Author: Garmin Chang <garmin.chang@mediatek.com>
> > +
> > +#include <linux/clk-provider.h>
> > +#include <linux/platform_device.h>
> > +#include <dt-bindings/clock/mediatek,mt8188-clk.h>
> > +
> > +#include "clk-gate.h"
> > +#include "clk-mtk.h"
> > +
> > +static const struct mtk_gate_regs peri_ao_cg_regs = {
> > +	.set_ofs = 0x10,
> > +	.clr_ofs = 0x14,
> > +	.sta_ofs = 0x18,
> > +};
> > +
> > +#define GATE_PERI_AO(_id, _name, _parent, _shift)			
> > \
> > +	GATE_MTK(_id, _name, _parent, &peri_ao_cg_regs, _shift,
> > &mtk_clk_gate_ops_setclr)
> > +
> > +static const struct mtk_gate peri_ao_clks[] = {
> > +	GATE_PERI_AO(CLK_PERI_AO_ETHERNET, "peri_ao_ethernet",
> > "top_axi", 0),
> > +	GATE_PERI_AO(CLK_PERI_AO_ETHERNET_BUS, "peri_ao_ethernet_bus",
> > "top_axi", 1),
> > +	GATE_PERI_AO(CLK_PERI_AO_FLASHIF_BUS, "peri_ao_flashif_bus",
> > "top_axi", 3),
> > +	GATE_PERI_AO(CLK_PERI_AO_FLASHIF_26M, "peri_ao_flashif_26m",
> > "clk26m", 4),
> > +	GATE_PERI_AO(CLK_PERI_AO_FLASHIFLASHCK,
> > "peri_ao_flashiflashck", "top_spinor", 5),
> > +	GATE_PERI_AO(CLK_PERI_AO_SSUSB_2P_BUS, "peri_ao_ssusb_2p_bus",
> > "top_usb_top_2p", 9),
> > +	GATE_PERI_AO(CLK_PERI_AO_SSUSB_2P_XHCI,
> > "peri_ao_ssusb_2p_xhci", "top_ssusb_xhci_2p", 10),
> > +	GATE_PERI_AO(CLK_PERI_AO_SSUSB_3P_BUS, "peri_ao_ssusb_3p_bus",
> > "top_usb_top_3p", 11),
> > +	GATE_PERI_AO(CLK_PERI_AO_SSUSB_3P_XHCI,
> > "peri_ao_ssusb_3p_xhci", "top_ssusb_xhci_3p", 12),
> > +	GATE_PERI_AO(CLK_PERI_AO_SSUSB_BUS, "peri_ao_ssusb_bus",
> > "top_usb_top", 13),
> > +	GATE_PERI_AO(CLK_PERI_AO_SSUSB_XHCI, "peri_ao_ssusb_xhci",
> > "top_ssusb_xhci", 14),
> > +	GATE_PERI_AO(CLK_PERI_AO_ETHERNET_MAC,
> > "peri_ao_ethernet_mac_clk", "top_snps_eth_250m", 16),
> > +	GATE_PERI_AO(CLK_PERI_AO_PCIE_P0_FMEM, "peri_ao_pcie_p0_fmem",
> > "hd_466m_fmem_ck", 24),
> > +};
> > +
> > +static const struct mtk_clk_desc peri_ao_desc = {
> > +	.clks = peri_ao_clks,
> > +	.num_clks = ARRAY_SIZE(peri_ao_clks),
> > +};
> > +
> > +static const struct of_device_id of_match_clk_mt8188_peri_ao[] = {
> > +	{ .compatible = "mediatek,mt8188-pericfg-ao", .data =
> > &peri_ao_desc },
> > +	{ /* sentinel */ }
> > +};
> 
> MODULE_DEVICE_TABLE is missing
> 
Thank you for your suggestions.

Ok, I would modify this in v7.
> > +
> > +static struct platform_driver clk_mt8188_peri_ao_drv = {
> > +	.probe = mtk_clk_simple_probe,
> > +	.remove = mtk_clk_simple_remove,
> > +	.driver = {
> > +		.name = "clk-mt8188-peri_ao",
> > +		.of_match_table = of_match_clk_mt8188_peri_ao,
> > +	},
> > +};
> > +builtin_platform_driver(clk_mt8188_peri_ao_drv);
> 
> module_platform_driver()
> 
> MODULE_LICENSE

Re: [PATCH v6 02/19] clk: mediatek: Add MT8188 apmixedsys clock support

[AngeloGioacchino Del Regno]

Il 09/03/23 14:54, Garmin.Chang ha scritto:
> Add MT8188 apmixedsys clock controller which provides Plls
> generated from SoC 26m and ssusb clock gate control.
> 
> Signed-off-by: Garmin.Chang <Garmin.Chang@mediatek.com>
> ---
>   drivers/clk/mediatek/Kconfig                 |  11 ++
>   drivers/clk/mediatek/Makefile                |   1 +
>   drivers/clk/mediatek/clk-mt8188-apmixedsys.c | 154 +++++++++++++++++++
>   3 files changed, 166 insertions(+)
>   create mode 100644 drivers/clk/mediatek/clk-mt8188-apmixedsys.c
> 
> diff --git a/drivers/clk/mediatek/Kconfig b/drivers/clk/mediatek/Kconfig
> index 2d14855dd37e..d7115089c4f6 100644
> --- a/drivers/clk/mediatek/Kconfig
> +++ b/drivers/clk/mediatek/Kconfig
> @@ -604,6 +604,17 @@ config COMMON_CLK_MT8186
>   	help
>   	  This driver supports MediaTek MT8186 clocks.
>   
> +config COMMON_CLK_MT8188
> +	bool "Clock driver for MediaTek MT8188"

this can be a tristate.

> +	depends on ARM64 || COMPILE_TEST
> +	select COMMON_CLK_MEDIATEK
> +	default ARCH_MEDIATEK
> +	help
> +	  This driver supports MediaTek MT8188 basic clocks and clocks
> +	  required for various peripheral found on MediaTek. Choose
> +	  M or Y here if you want to use clocks such as peri_ao,
> +	  infra_ao, etc.
> +
>   config COMMON_CLK_MT8192
>   	bool "Clock driver for MediaTek MT8192"
>   	depends on ARM64 || COMPILE_TEST
> diff --git a/drivers/clk/mediatek/Makefile b/drivers/clk/mediatek/Makefile
> index e5d018270ed0..1f822fcf6084 100644
> --- a/drivers/clk/mediatek/Makefile
> +++ b/drivers/clk/mediatek/Makefile
> @@ -91,6 +91,7 @@ obj-$(CONFIG_COMMON_CLK_MT8186) += clk-mt8186-mcu.o clk-mt8186-topckgen.o clk-mt
>   				   clk-mt8186-mfg.o clk-mt8186-mm.o clk-mt8186-wpe.o \
>   				   clk-mt8186-img.o clk-mt8186-vdec.o clk-mt8186-venc.o \
>   				   clk-mt8186-cam.o clk-mt8186-mdp.o clk-mt8186-ipe.o
> +obj-$(CONFIG_COMMON_CLK_MT8188) += clk-mt8188-apmixedsys.o
>   obj-$(CONFIG_COMMON_CLK_MT8192) += clk-mt8192.o
>   obj-$(CONFIG_COMMON_CLK_MT8192_AUDSYS) += clk-mt8192-aud.o
>   obj-$(CONFIG_COMMON_CLK_MT8192_CAMSYS) += clk-mt8192-cam.o
> diff --git a/drivers/clk/mediatek/clk-mt8188-apmixedsys.c b/drivers/clk/mediatek/clk-mt8188-apmixedsys.c
> new file mode 100644
> index 000000000000..db64340386d9
> --- /dev/null
> +++ b/drivers/clk/mediatek/clk-mt8188-apmixedsys.c
> @@ -0,0 +1,154 @@
> +// SPDX-License-Identifier: GPL-2.0-only

// SPDX ...
/*
  * Copyright ...
  * Author ..
  */

> +//
> +// Copyright (c) 2022 MediaTek Inc.
> +// Author: Garmin Chang <garmin.chang@mediatek.com>
> +
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <dt-bindings/clock/mediatek,mt8188-clk.h>

Please order by name: dt-bindings goes first.

> +
> +#include "clk-gate.h"
> +#include "clk-mtk.h"
> +#include "clk-pll.h"
> +
> +static const struct mtk_gate_regs apmixed_cg_regs = {
> +	.set_ofs = 0x8,
> +	.clr_ofs = 0x8,
> +	.sta_ofs = 0x8,
> +};
> +
> +#define GATE_APMIXED(_id, _name, _parent, _shift)			\
> +	GATE_MTK(_id, _name, _parent, &apmixed_cg_regs, _shift, &mtk_clk_gate_ops_no_setclr_inv)
> +
> +static const struct mtk_gate apmixed_clks[] = {
> +	GATE_APMIXED(CLK_APMIXED_PLL_SSUSB26M_EN, "pll_ssusb26m_en", "clk26m", 1),
> +};
> +
> +#define MT8188_PLL_FMAX		(3800UL * MHZ)
> +#define MT8188_PLL_FMIN		(1500UL * MHZ)
> +#define MT8188_INTEGER_BITS	8
> +
> +#define PLL(_id, _name, _reg, _pwr_reg, _en_mask, _flags,		\
> +	    _rst_bar_mask, _pcwbits, _pd_reg, _pd_shift,		\
> +	    _tuner_reg, _tuner_en_reg, _tuner_en_bit,			\
> +	    _pcw_reg, _pcw_shift, _pcw_chg_reg,				\
> +	    _en_reg, _pll_en_bit) {					\
> +		.id = _id,						\
> +		.name = _name,						\
> +		.reg = _reg,						\
> +		.pwr_reg = _pwr_reg,					\
> +		.en_mask = _en_mask,					\
> +		.flags = _flags,					\
> +		.rst_bar_mask = _rst_bar_mask,				\
> +		.fmax = MT8188_PLL_FMAX,				\
> +		.fmin = MT8188_PLL_FMIN,				\
> +		.pcwbits = _pcwbits,					\
> +		.pcwibits = MT8188_INTEGER_BITS,			\
> +		.pd_reg = _pd_reg,					\
> +		.pd_shift = _pd_shift,					\
> +		.tuner_reg = _tuner_reg,				\
> +		.tuner_en_reg = _tuner_en_reg,				\
> +		.tuner_en_bit = _tuner_en_bit,				\
> +		.pcw_reg = _pcw_reg,					\
> +		.pcw_shift = _pcw_shift,				\
> +		.pcw_chg_reg = _pcw_chg_reg,				\
> +		.en_reg = _en_reg,					\
> +		.pll_en_bit = _pll_en_bit,				\
> +	}
> +
> +static const struct mtk_pll_data plls[] = {
> +	PLL(CLK_APMIXED_ETHPLL, "ethpll", 0x044C, 0x0458, 0,
> +	    0, 0, 22, 0x0450, 24, 0, 0, 0, 0x0450, 0, 0, 0, 9),
> +	PLL(CLK_APMIXED_MSDCPLL, "msdcpll", 0x0514, 0x0520, 0,
> +	    0, 0, 22, 0x0518, 24, 0, 0, 0, 0x0518, 0, 0, 0, 9),
> +	PLL(CLK_APMIXED_TVDPLL1, "tvdpll1", 0x0524, 0x0530, 0,
> +	    0, 0, 22, 0x0528, 24, 0, 0, 0, 0x0528, 0, 0, 0, 9),
> +	PLL(CLK_APMIXED_TVDPLL2, "tvdpll2", 0x0534, 0x0540, 0,
> +	    0, 0, 22, 0x0538, 24, 0, 0, 0, 0x0538, 0, 0, 0, 9),
> +	PLL(CLK_APMIXED_MMPLL, "mmpll", 0x0544, 0x0550, 0xff000000,
> +	    HAVE_RST_BAR, BIT(23), 22, 0x0548, 24, 0, 0, 0, 0x0548, 0, 0, 0, 9),
> +	PLL(CLK_APMIXED_MAINPLL, "mainpll", 0x045C, 0x0468, 0xff000000,
> +	    HAVE_RST_BAR, BIT(23), 22, 0x0460, 24, 0, 0, 0, 0x0460, 0, 0, 0, 9),
> +	PLL(CLK_APMIXED_IMGPLL, "imgpll", 0x0554, 0x0560, 0,
> +	    0, 0, 22, 0x0558, 24, 0, 0, 0, 0x0558, 0, 0, 0, 9),
> +	PLL(CLK_APMIXED_UNIVPLL, "univpll", 0x0504, 0x0510, 0xff000000,
> +	    HAVE_RST_BAR, BIT(23), 22, 0x0508, 24, 0, 0, 0, 0x0508, 0, 0, 0, 9),
> +	PLL(CLK_APMIXED_ADSPPLL, "adsppll", 0x042C, 0x0438, 0,
> +	    0, 0, 22, 0x0430, 24, 0, 0, 0, 0x0430, 0, 0, 0, 9),
> +	PLL(CLK_APMIXED_APLL1, "apll1", 0x0304, 0x0314, 0,
> +	    0, 0, 32, 0x0308, 24, 0x0034, 0x0000, 12, 0x030C, 0, 0, 0, 9),
> +	PLL(CLK_APMIXED_APLL2, "apll2", 0x0318, 0x0328, 0,
> +	    0, 0, 32, 0x031C, 24, 0x0038, 0x0000, 13, 0x0320, 0, 0, 0, 9),
> +	PLL(CLK_APMIXED_APLL3, "apll3", 0x032C, 0x033C, 0,
> +	    0, 0, 32, 0x0330, 24, 0x003C, 0x0000, 14, 0x0334, 0, 0, 0, 9),
> +	PLL(CLK_APMIXED_APLL4, "apll4", 0x0404, 0x0414, 0,
> +	    0, 0, 32, 0x0408, 24, 0x0040, 0x0000, 15, 0x040C, 0, 0, 0, 9),
> +	PLL(CLK_APMIXED_APLL5, "apll5", 0x0418, 0x0428, 0,
> +	    0, 0, 32, 0x041C, 24, 0x0044, 0x0000, 16, 0x0420, 0, 0, 0, 9),
> +	PLL(CLK_APMIXED_MFGPLL, "mfgpll", 0x0340, 0x034C, 0,
> +	    0, 0, 22, 0x0344, 24, 0, 0, 0, 0x0344, 0, 0, 0, 9),
> +};
> +
> +static const struct of_device_id of_match_clk_mt8188_apmixed[] = {
> +	{ .compatible = "mediatek,mt8188-apmixedsys", },

You don't need the comma after compatible

	{ .compatible = "mediatek,mt8188-apmixedsys" },

> +	{}

At the end, please say what the last entry is; a common way to do this is:

	{ /* sentinel */ }

...since that's a sentinel! :-)

> +};

You miss just one little step for module auto-loading here.

MODULE_DEVICE_TABLE(of, of_match_clk_mt8188_apmixed);

> +
> +static int clk_mt8188_apmixed_probe(struct platform_device *pdev)
> +{
> +	struct clk_hw_onecell_data *clk_data;
> +	struct device_node *node = pdev->dev.of_node;
> +	int r;
> +
> +	clk_data = mtk_alloc_clk_data(CLK_APMIXED_NR_CLK);
> +	if (!clk_data)
> +		return -ENOMEM;
> +
> +	r = mtk_clk_register_plls(node, plls, ARRAY_SIZE(plls), clk_data);
> +	if (r)
> +		goto free_apmixed_data;
> +
> +	r = mtk_clk_register_gates(&pdev->dev, node, apmixed_clks,
> +				   ARRAY_SIZE(apmixed_clks), clk_data);
> +	if (r)
> +		goto unregister_plls;
> +
> +	r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_data);
> +	if (r)
> +		goto unregister_gates;
> +
> +	platform_set_drvdata(pdev, clk_data);
> +
> +	return r;

Reaching this point means that no error occurred, so here...

	return 0;

> +
> +unregister_gates:
> +	mtk_clk_unregister_gates(apmixed_clks, ARRAY_SIZE(apmixed_clks), clk_data);
> +unregister_plls:
> +	mtk_clk_unregister_plls(plls, ARRAY_SIZE(plls), clk_data);
> +free_apmixed_data:
> +	mtk_free_clk_data(clk_data);
> +	return r;
> +}
> +
> +static int clk_mt8188_apmixed_remove(struct platform_device *pdev)
> +{
> +	struct device_node *node = pdev->dev.of_node;
> +	struct clk_hw_onecell_data *clk_data = platform_get_drvdata(pdev);
> +
> +	of_clk_del_provider(node);
> +	mtk_clk_unregister_gates(apmixed_clks, ARRAY_SIZE(apmixed_clks), clk_data);
> +	mtk_clk_unregister_plls(plls, ARRAY_SIZE(plls), clk_data);
> +	mtk_free_clk_data(clk_data);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver clk_mt8188_apmixed_drv = {
> +	.probe = clk_mt8188_apmixed_probe,
> +	.remove = clk_mt8188_apmixed_remove,
> +	.driver = {
> +		.name = "clk-mt8188-apmixed",
> +		.of_match_table = of_match_clk_mt8188_apmixed,
> +	},
> +};
> +builtin_platform_driver(clk_mt8188_apmixed_drv);

You can change builtin_platform_driver() to module_platform_driver() here to
achieve modularity.

Regards,
Angelo

Re: [PATCH v6 02/19] clk: mediatek: Add MT8188 apmixedsys clock support

[Garmin Chang (張家銘)]

On Tue, 2023-03-14 at 10:05 +0100, AngeloGioacchino Del Regno wrote:
> Il 09/03/23 14:54, Garmin.Chang ha scritto:
> > Add MT8188 apmixedsys clock controller which provides Plls
> > generated from SoC 26m and ssusb clock gate control.
> > 
> > Signed-off-by: Garmin.Chang <Garmin.Chang@mediatek.com>
> > ---
> >   drivers/clk/mediatek/Kconfig                 |  11 ++
> >   drivers/clk/mediatek/Makefile                |   1 +
> >   drivers/clk/mediatek/clk-mt8188-apmixedsys.c | 154
> > +++++++++++++++++++
> >   3 files changed, 166 insertions(+)
> >   create mode 100644 drivers/clk/mediatek/clk-mt8188-apmixedsys.c
> > 
> > diff --git a/drivers/clk/mediatek/Kconfig
> > b/drivers/clk/mediatek/Kconfig
> > index 2d14855dd37e..d7115089c4f6 100644
> > --- a/drivers/clk/mediatek/Kconfig
> > +++ b/drivers/clk/mediatek/Kconfig
> > @@ -604,6 +604,17 @@ config COMMON_CLK_MT8186
> >   	help
> >   	  This driver supports MediaTek MT8186 clocks.
> >   
> > +config COMMON_CLK_MT8188
> > +	bool "Clock driver for MediaTek MT8188"
> 
> this can be a tristate.
> 
> > +	depends on ARM64 || COMPILE_TEST
> > +	select COMMON_CLK_MEDIATEK
> > +	default ARCH_MEDIATEK
> > +	help
> > +	  This driver supports MediaTek MT8188 basic clocks and clocks
> > +	  required for various peripheral found on MediaTek. Choose
> > +	  M or Y here if you want to use clocks such as peri_ao,
> > +	  infra_ao, etc.
> > +
> >   config COMMON_CLK_MT8192
> >   	bool "Clock driver for MediaTek MT8192"
> >   	depends on ARM64 || COMPILE_TEST
> > diff --git a/drivers/clk/mediatek/Makefile
> > b/drivers/clk/mediatek/Makefile
> > index e5d018270ed0..1f822fcf6084 100644
> > --- a/drivers/clk/mediatek/Makefile
> > +++ b/drivers/clk/mediatek/Makefile
> > @@ -91,6 +91,7 @@ obj-$(CONFIG_COMMON_CLK_MT8186) += clk-mt8186-
> > mcu.o clk-mt8186-topckgen.o clk-mt
> >   				   clk-mt8186-mfg.o clk-mt8186-mm.o
> > clk-mt8186-wpe.o \
> >   				   clk-mt8186-img.o clk-mt8186-vdec.o
> > clk-mt8186-venc.o \
> >   				   clk-mt8186-cam.o clk-mt8186-mdp.o
> > clk-mt8186-ipe.o
> > +obj-$(CONFIG_COMMON_CLK_MT8188) += clk-mt8188-apmixedsys.o
> >   obj-$(CONFIG_COMMON_CLK_MT8192) += clk-mt8192.o
> >   obj-$(CONFIG_COMMON_CLK_MT8192_AUDSYS) += clk-mt8192-aud.o
> >   obj-$(CONFIG_COMMON_CLK_MT8192_CAMSYS) += clk-mt8192-cam.o
> > diff --git a/drivers/clk/mediatek/clk-mt8188-apmixedsys.c
> > b/drivers/clk/mediatek/clk-mt8188-apmixedsys.c
> > new file mode 100644
> > index 000000000000..db64340386d9
> > --- /dev/null
> > +++ b/drivers/clk/mediatek/clk-mt8188-apmixedsys.c
> > @@ -0,0 +1,154 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> 
> // SPDX ...
> /*
>   * Copyright ...
>   * Author ..
>   */
> 
> > +//
> > +// Copyright (c) 2022 MediaTek Inc.
> > +// Author: Garmin Chang <garmin.chang@mediatek.com>
> > +
> > +#include <linux/of_device.h>
> > +#include <linux/platform_device.h>
> > +#include <dt-bindings/clock/mediatek,mt8188-clk.h>
> 
> Please order by name: dt-bindings goes first.
> 
Thank you for your suggestions.

Ok, I would modify this and below in v7.
> > +
> > +#include "clk-gate.h"
> > +#include "clk-mtk.h"
> > +#include "clk-pll.h"
> > +
> > +static const struct mtk_gate_regs apmixed_cg_regs = {
> > +	.set_ofs = 0x8,
> > +	.clr_ofs = 0x8,
> > +	.sta_ofs = 0x8,
> > +};
> > +
> > +#define GATE_APMIXED(_id, _name, _parent, _shift)			
> > \
> > +	GATE_MTK(_id, _name, _parent, &apmixed_cg_regs, _shift,
> > &mtk_clk_gate_ops_no_setclr_inv)
> > +
> > +static const struct mtk_gate apmixed_clks[] = {
> > +	GATE_APMIXED(CLK_APMIXED_PLL_SSUSB26M_EN, "pll_ssusb26m_en",
> > "clk26m", 1),
> > +};
> > +
> > +#define MT8188_PLL_FMAX		(3800UL * MHZ)
> > +#define MT8188_PLL_FMIN		(1500UL * MHZ)
> > +#define MT8188_INTEGER_BITS	8
> > +
> > +#define PLL(_id, _name, _reg, _pwr_reg, _en_mask, _flags,		
> > \
> > +	    _rst_bar_mask, _pcwbits, _pd_reg, _pd_shift,		\
> > +	    _tuner_reg, _tuner_en_reg, _tuner_en_bit,			
> > \
> > +	    _pcw_reg, _pcw_shift, _pcw_chg_reg,				
> > \
> > +	    _en_reg, _pll_en_bit) {					\
> > +		.id = _id,						\
> > +		.name = _name,						
> > \
> > +		.reg = _reg,						
> > \
> > +		.pwr_reg = _pwr_reg,					
> > \
> > +		.en_mask = _en_mask,					
> > \
> > +		.flags = _flags,					\
> > +		.rst_bar_mask = _rst_bar_mask,				
> > \
> > +		.fmax = MT8188_PLL_FMAX,				\
> > +		.fmin = MT8188_PLL_FMIN,				\
> > +		.pcwbits = _pcwbits,					
> > \
> > +		.pcwibits = MT8188_INTEGER_BITS,			\
> > +		.pd_reg = _pd_reg,					\
> > +		.pd_shift = _pd_shift,					
> > \
> > +		.tuner_reg = _tuner_reg,				\
> > +		.tuner_en_reg = _tuner_en_reg,				
> > \
> > +		.tuner_en_bit = _tuner_en_bit,				
> > \
> > +		.pcw_reg = _pcw_reg,					
> > \
> > +		.pcw_shift = _pcw_shift,				\
> > +		.pcw_chg_reg = _pcw_chg_reg,				
> > \
> > +		.en_reg = _en_reg,					\
> > +		.pll_en_bit = _pll_en_bit,				\
> > +	}
> > +
> > +static const struct mtk_pll_data plls[] = {
> > +	PLL(CLK_APMIXED_ETHPLL, "ethpll", 0x044C, 0x0458, 0,
> > +	    0, 0, 22, 0x0450, 24, 0, 0, 0, 0x0450, 0, 0, 0, 9),
> > +	PLL(CLK_APMIXED_MSDCPLL, "msdcpll", 0x0514, 0x0520, 0,
> > +	    0, 0, 22, 0x0518, 24, 0, 0, 0, 0x0518, 0, 0, 0, 9),
> > +	PLL(CLK_APMIXED_TVDPLL1, "tvdpll1", 0x0524, 0x0530, 0,
> > +	    0, 0, 22, 0x0528, 24, 0, 0, 0, 0x0528, 0, 0, 0, 9),
> > +	PLL(CLK_APMIXED_TVDPLL2, "tvdpll2", 0x0534, 0x0540, 0,
> > +	    0, 0, 22, 0x0538, 24, 0, 0, 0, 0x0538, 0, 0, 0, 9),
> > +	PLL(CLK_APMIXED_MMPLL, "mmpll", 0x0544, 0x0550, 0xff000000,
> > +	    HAVE_RST_BAR, BIT(23), 22, 0x0548, 24, 0, 0, 0, 0x0548, 0,
> > 0, 0, 9),
> > +	PLL(CLK_APMIXED_MAINPLL, "mainpll", 0x045C, 0x0468, 0xff000000,
> > +	    HAVE_RST_BAR, BIT(23), 22, 0x0460, 24, 0, 0, 0, 0x0460, 0,
> > 0, 0, 9),
> > +	PLL(CLK_APMIXED_IMGPLL, "imgpll", 0x0554, 0x0560, 0,
> > +	    0, 0, 22, 0x0558, 24, 0, 0, 0, 0x0558, 0, 0, 0, 9),
> > +	PLL(CLK_APMIXED_UNIVPLL, "univpll", 0x0504, 0x0510, 0xff000000,
> > +	    HAVE_RST_BAR, BIT(23), 22, 0x0508, 24, 0, 0, 0, 0x0508, 0,
> > 0, 0, 9),
> > +	PLL(CLK_APMIXED_ADSPPLL, "adsppll", 0x042C, 0x0438, 0,
> > +	    0, 0, 22, 0x0430, 24, 0, 0, 0, 0x0430, 0, 0, 0, 9),
> > +	PLL(CLK_APMIXED_APLL1, "apll1", 0x0304, 0x0314, 0,
> > +	    0, 0, 32, 0x0308, 24, 0x0034, 0x0000, 12, 0x030C, 0, 0, 0,
> > 9),
> > +	PLL(CLK_APMIXED_APLL2, "apll2", 0x0318, 0x0328, 0,
> > +	    0, 0, 32, 0x031C, 24, 0x0038, 0x0000, 13, 0x0320, 0, 0, 0,
> > 9),
> > +	PLL(CLK_APMIXED_APLL3, "apll3", 0x032C, 0x033C, 0,
> > +	    0, 0, 32, 0x0330, 24, 0x003C, 0x0000, 14, 0x0334, 0, 0, 0,
> > 9),
> > +	PLL(CLK_APMIXED_APLL4, "apll4", 0x0404, 0x0414, 0,
> > +	    0, 0, 32, 0x0408, 24, 0x0040, 0x0000, 15, 0x040C, 0, 0, 0,
> > 9),
> > +	PLL(CLK_APMIXED_APLL5, "apll5", 0x0418, 0x0428, 0,
> > +	    0, 0, 32, 0x041C, 24, 0x0044, 0x0000, 16, 0x0420, 0, 0, 0,
> > 9),
> > +	PLL(CLK_APMIXED_MFGPLL, "mfgpll", 0x0340, 0x034C, 0,
> > +	    0, 0, 22, 0x0344, 24, 0, 0, 0, 0x0344, 0, 0, 0, 9),
> > +};
> > +
> > +static const struct of_device_id of_match_clk_mt8188_apmixed[] = {
> > +	{ .compatible = "mediatek,mt8188-apmixedsys", },
> 
> You don't need the comma after compatible
> 
> 	{ .compatible = "mediatek,mt8188-apmixedsys" },
> 
> > +	{}
> 
> At the end, please say what the last entry is; a common way to do
> this is:
> 
> 	{ /* sentinel */ }
> 
> ...since that's a sentinel! :-)
> Ok.
> > +};
> 
> You miss just one little step for module auto-loading here.
> 
> MODULE_DEVICE_TABLE(of, of_match_clk_mt8188_apmixed);
  Ok.
> 
> > +
> > +static int clk_mt8188_apmixed_probe(struct platform_device *pdev)
> > +{
> > +	struct clk_hw_onecell_data *clk_data;
> > +	struct device_node *node = pdev->dev.of_node;
> > +	int r;
> > +
> > +	clk_data = mtk_alloc_clk_data(CLK_APMIXED_NR_CLK);
> > +	if (!clk_data)
> > +		return -ENOMEM;
> > +
> > +	r = mtk_clk_register_plls(node, plls, ARRAY_SIZE(plls),
> > clk_data);
> > +	if (r)
> > +		goto free_apmixed_data;
> > +
> > +	r = mtk_clk_register_gates(&pdev->dev, node, apmixed_clks,
> > +				   ARRAY_SIZE(apmixed_clks), clk_data);
> > +	if (r)
> > +		goto unregister_plls;
> > +
> > +	r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get,
> > clk_data);
> > +	if (r)
> > +		goto unregister_gates;
> > +
> > +	platform_set_drvdata(pdev, clk_data);
> > +
> > +	return r;
> 
> Reaching this point means that no error occurred, so here...
> 
> 	return 0;
> 
OK.
> > +
> > +unregister_gates:
> > +	mtk_clk_unregister_gates(apmixed_clks,
> > ARRAY_SIZE(apmixed_clks), clk_data);
> > +unregister_plls:
> > +	mtk_clk_unregister_plls(plls, ARRAY_SIZE(plls), clk_data);
> > +free_apmixed_data:
> > +	mtk_free_clk_data(clk_data);
> > +	return r;
> > +}
> > +
> > +static int clk_mt8188_apmixed_remove(struct platform_device *pdev)
> > +{
> > +	struct device_node *node = pdev->dev.of_node;
> > +	struct clk_hw_onecell_data *clk_data =
> > platform_get_drvdata(pdev);
> > +
> > +	of_clk_del_provider(node);
> > +	mtk_clk_unregister_gates(apmixed_clks,
> > ARRAY_SIZE(apmixed_clks), clk_data);
> > +	mtk_clk_unregister_plls(plls, ARRAY_SIZE(plls), clk_data);
> > +	mtk_free_clk_data(clk_data);
> > +
> > +	return 0;
> > +}
> > +
> > +static struct platform_driver clk_mt8188_apmixed_drv = {
> > +	.probe = clk_mt8188_apmixed_probe,
> > +	.remove = clk_mt8188_apmixed_remove,
> > +	.driver = {
> > +		.name = "clk-mt8188-apmixed",
> > +		.of_match_table = of_match_clk_mt8188_apmixed,
> > +	},
> > +};
> > +builtin_platform_driver(clk_mt8188_apmixed_drv);
> 
> You can change builtin_platform_driver() to module_platform_driver()
> here to
> achieve modularity.
OK.
> 
> Regards,
> Angelo
> 

Re: [PATCH v6 03/19] clk: mediatek: Add MT8188 topckgen clock support

[AngeloGioacchino Del Regno]

Il 09/03/23 14:54, Garmin.Chang ha scritto:
> Add MT8188 topckgen clock controller which provides muxes, dividers
> to handle variety clock selection in other IP blocks.
> 
> Signed-off-by: Garmin.Chang <Garmin.Chang@mediatek.com>
> ---
>   drivers/clk/mediatek/Makefile              |    2 +-
>   drivers/clk/mediatek/clk-mt8188-topckgen.c | 1347 ++++++++++++++++++++
>   2 files changed, 1348 insertions(+), 1 deletion(-)
>   create mode 100644 drivers/clk/mediatek/clk-mt8188-topckgen.c
> 
> diff --git a/drivers/clk/mediatek/Makefile b/drivers/clk/mediatek/Makefile
> index 1f822fcf6084..d845bf7308c3 100644
> --- a/drivers/clk/mediatek/Makefile
> +++ b/drivers/clk/mediatek/Makefile
> @@ -91,7 +91,7 @@ obj-$(CONFIG_COMMON_CLK_MT8186) += clk-mt8186-mcu.o clk-mt8186-topckgen.o clk-mt
>   				   clk-mt8186-mfg.o clk-mt8186-mm.o clk-mt8186-wpe.o \
>   				   clk-mt8186-img.o clk-mt8186-vdec.o clk-mt8186-venc.o \
>   				   clk-mt8186-cam.o clk-mt8186-mdp.o clk-mt8186-ipe.o
> -obj-$(CONFIG_COMMON_CLK_MT8188) += clk-mt8188-apmixedsys.o
> +obj-$(CONFIG_COMMON_CLK_MT8188) += clk-mt8188-apmixedsys.o clk-mt8188-topckgen.o
>   obj-$(CONFIG_COMMON_CLK_MT8192) += clk-mt8192.o
>   obj-$(CONFIG_COMMON_CLK_MT8192_AUDSYS) += clk-mt8192-aud.o
>   obj-$(CONFIG_COMMON_CLK_MT8192_CAMSYS) += clk-mt8192-cam.o
> diff --git a/drivers/clk/mediatek/clk-mt8188-topckgen.c b/drivers/clk/mediatek/clk-mt8188-topckgen.c
> new file mode 100644
> index 000000000000..b3f9577de081
> --- /dev/null
> +++ b/drivers/clk/mediatek/clk-mt8188-topckgen.c
> @@ -0,0 +1,1347 @@

..snip..

> +
> +static const struct of_device_id of_match_clk_mt8188_topck[] = {
> +	{ .compatible = "mediatek,mt8188-topckgen", },

	{ .compatible = "mediatek,mt8188-topckgen" },
	{ /* sentinel */ }

> +	{}
> +};
> +
> +/* Register mux notifier for MFG mux */
> +static int clk_mt8188_reg_mfg_mux_notifier(struct device *dev, struct clk *clk)
> +{
> +	struct mtk_mux_nb *mfg_mux_nb;
> +
> +	mfg_mux_nb = devm_kzalloc(dev, sizeof(*mfg_mux_nb), GFP_KERNEL);
> +	if (!mfg_mux_nb)
> +		return -ENOMEM;
> +
> +	mfg_mux_nb->ops = &clk_mux_ops;
> +	mfg_mux_nb->bypass_index = 0; /* Bypass to TOP_MFG_CORE_TMP */
> +
> +	return devm_mtk_clk_mux_notifier_register(dev, clk, mfg_mux_nb);
> +}
> +
> +static int clk_mt8188_topck_probe(struct platform_device *pdev)
> +{
> +	struct clk_hw_onecell_data *top_clk_data;
> +	struct device_node *node = pdev->dev.of_node;
> +	struct clk_hw *hw;
> +	int r;
> +	void __iomem *base;
> +
> +	top_clk_data = mtk_alloc_clk_data(CLK_TOP_NR_CLK);
> +	if (!top_clk_data)
> +		return -ENOMEM;
> +
> +	base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(base)) {
> +		r = PTR_ERR(base);
> +		goto free_top_data;
> +	}
> +
> +	r = mtk_clk_register_fixed_clks(top_fixed_clks, ARRAY_SIZE(top_fixed_clks),
> +					top_clk_data);
> +	if (r)
> +		goto free_top_data;
> +
> +	r = mtk_clk_register_factors(top_divs, ARRAY_SIZE(top_divs), top_clk_data);
> +	if (r)
> +		goto unregister_fixed_clks;
> +
> +	r = mtk_clk_register_muxes(&pdev->dev, top_mtk_muxes,
> +				   ARRAY_SIZE(top_mtk_muxes), node,
> +				   &mt8188_clk_lock, top_clk_data);
> +	if (r)
> +		goto unregister_factors;
> +
> +	hw = devm_clk_hw_register_mux(&pdev->dev, "mfg_ck_fast_ref", mfg_fast_ref_parents,
> +				      ARRAY_SIZE(mfg_fast_ref_parents), CLK_SET_RATE_PARENT,
> +				      (base + 0x250), 8, 1, 0, &mt8188_clk_lock);

If you make this a mtk mux and put it in top_mtk_muxes, you can migrate topckgen to
the simple_probe() mechanism, greatly reducing the size of this file.

> +	if (IS_ERR(hw)) {
> +		r = PTR_ERR(hw);
> +		goto unregister_muxes;
> +	}
> +	top_clk_data->hws[CLK_TOP_MFG_CK_FAST_REF] = hw;
> +
> +	r = clk_mt8188_reg_mfg_mux_notifier(&pdev->dev,
> +					    top_clk_data->hws[CLK_TOP_MFG_CK_FAST_REF]->clk);
> +	if (r)
> +		goto unregister_muxes;
> +
> +	r = mtk_clk_register_composites(&pdev->dev, top_adj_divs,
> +					ARRAY_SIZE(top_adj_divs), base,
> +					&mt8188_clk_lock, top_clk_data);
> +	if (r)
> +		goto unregister_muxes;
> +
> +	r = mtk_clk_register_gates(&pdev->dev, node, top_clks,
> +				   ARRAY_SIZE(top_clks), top_clk_data);
> +	if (r)
> +		goto unregister_composite_divs;
> +
> +	r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, top_clk_data);
> +	if (r)
> +		goto unregister_gates;
> +
> +	platform_set_drvdata(pdev, top_clk_data);
> +
> +	return r;
> +
> +unregister_gates:
> +	mtk_clk_unregister_gates(top_clks, ARRAY_SIZE(top_clks), top_clk_data);
> +unregister_composite_divs:
> +	mtk_clk_unregister_composites(top_adj_divs, ARRAY_SIZE(top_adj_divs), top_clk_data);
> +unregister_muxes:
> +	mtk_clk_unregister_muxes(top_mtk_muxes, ARRAY_SIZE(top_mtk_muxes), top_clk_data);
> +unregister_factors:
> +	mtk_clk_unregister_factors(top_divs, ARRAY_SIZE(top_divs), top_clk_data);
> +unregister_fixed_clks:
> +	mtk_clk_unregister_fixed_clks(top_fixed_clks, ARRAY_SIZE(top_fixed_clks), top_clk_data);
> +free_top_data:
> +	mtk_free_clk_data(top_clk_data);
> +	return r;
> +}
> +
> +static int clk_mt8188_topck_remove(struct platform_device *pdev)
> +{
> +	struct clk_hw_onecell_data *top_clk_data = platform_get_drvdata(pdev);
> +	struct device_node *node = pdev->dev.of_node;
> +
> +	of_clk_del_provider(node);
> +	mtk_clk_unregister_gates(top_clks, ARRAY_SIZE(top_clks), top_clk_data);
> +	mtk_clk_unregister_composites(top_adj_divs, ARRAY_SIZE(top_adj_divs), top_clk_data);
> +	mtk_clk_unregister_muxes(top_mtk_muxes, ARRAY_SIZE(top_mtk_muxes), top_clk_data);
> +	mtk_clk_unregister_factors(top_divs, ARRAY_SIZE(top_divs), top_clk_data);
> +	mtk_clk_unregister_fixed_clks(top_fixed_clks, ARRAY_SIZE(top_fixed_clks), top_clk_data);
> +	mtk_free_clk_data(top_clk_data);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver clk_mt8188_topck_drv = {
> +	.probe = clk_mt8188_topck_probe,
> +	.remove = clk_mt8188_topck_remove,
> +	.driver = {
> +		.name = "clk-mt8188-topck",
> +		.of_match_table = of_match_clk_mt8188_topck,
> +	},
> +};
> +builtin_platform_driver(clk_mt8188_topck_drv);

module_platform_driver....
MODULE_LICENSE....

Re: [PATCH v6 03/19] clk: mediatek: Add MT8188 topckgen clock support

[Garmin Chang (張家銘)]

On Tue, 2023-03-14 at 10:10 +0100, AngeloGioacchino Del Regno wrote:
> Il 09/03/23 14:54, Garmin.Chang ha scritto:
> > Add MT8188 topckgen clock controller which provides muxes, dividers
> > to handle variety clock selection in other IP blocks.
> > 
> > Signed-off-by: Garmin.Chang <Garmin.Chang@mediatek.com>
> > ---
> >   drivers/clk/mediatek/Makefile              |    2 +-
> >   drivers/clk/mediatek/clk-mt8188-topckgen.c | 1347
> > ++++++++++++++++++++
> >   2 files changed, 1348 insertions(+), 1 deletion(-)
> >   create mode 100644 drivers/clk/mediatek/clk-mt8188-topckgen.c
> > 
> > diff --git a/drivers/clk/mediatek/Makefile
> > b/drivers/clk/mediatek/Makefile
> > index 1f822fcf6084..d845bf7308c3 100644
> > --- a/drivers/clk/mediatek/Makefile
> > +++ b/drivers/clk/mediatek/Makefile
> > @@ -91,7 +91,7 @@ obj-$(CONFIG_COMMON_CLK_MT8186) += clk-mt8186-
> > mcu.o clk-mt8186-topckgen.o clk-mt
> >   				   clk-mt8186-mfg.o clk-mt8186-mm.o
> > clk-mt8186-wpe.o \
> >   				   clk-mt8186-img.o clk-mt8186-vdec.o
> > clk-mt8186-venc.o \
> >   				   clk-mt8186-cam.o clk-mt8186-mdp.o
> > clk-mt8186-ipe.o
> > -obj-$(CONFIG_COMMON_CLK_MT8188) += clk-mt8188-apmixedsys.o
> > +obj-$(CONFIG_COMMON_CLK_MT8188) += clk-mt8188-apmixedsys.o clk-
> > mt8188-topckgen.o
> >   obj-$(CONFIG_COMMON_CLK_MT8192) += clk-mt8192.o
> >   obj-$(CONFIG_COMMON_CLK_MT8192_AUDSYS) += clk-mt8192-aud.o
> >   obj-$(CONFIG_COMMON_CLK_MT8192_CAMSYS) += clk-mt8192-cam.o
> > diff --git a/drivers/clk/mediatek/clk-mt8188-topckgen.c
> > b/drivers/clk/mediatek/clk-mt8188-topckgen.c
> > new file mode 100644
> > index 000000000000..b3f9577de081
> > --- /dev/null
> > +++ b/drivers/clk/mediatek/clk-mt8188-topckgen.c
> > @@ -0,0 +1,1347 @@
> 
> ..snip..
> 
> > +
> > +static const struct of_device_id of_match_clk_mt8188_topck[] = {
> > +	{ .compatible = "mediatek,mt8188-topckgen", },
> 
> 	{ .compatible = "mediatek,mt8188-topckgen" },
> 	{ /* sentinel */ }
> 
Thank you for your suggestions.

Ok, I would remove comma in v7.
> > +	{}
> > +};
> > +
> > +/* Register mux notifier for MFG mux */
> > +static int clk_mt8188_reg_mfg_mux_notifier(struct device *dev,
> > struct clk *clk)
> > +{
> > +	struct mtk_mux_nb *mfg_mux_nb;
> > +
> > +	mfg_mux_nb = devm_kzalloc(dev, sizeof(*mfg_mux_nb),
> > GFP_KERNEL);
> > +	if (!mfg_mux_nb)
> > +		return -ENOMEM;
> > +
> > +	mfg_mux_nb->ops = &clk_mux_ops;
> > +	mfg_mux_nb->bypass_index = 0; /* Bypass to TOP_MFG_CORE_TMP */
> > +
> > +	return devm_mtk_clk_mux_notifier_register(dev, clk,
> > mfg_mux_nb);
> > +}
> > +
> > +static int clk_mt8188_topck_probe(struct platform_device *pdev)
> > +{
> > +	struct clk_hw_onecell_data *top_clk_data;
> > +	struct device_node *node = pdev->dev.of_node;
> > +	struct clk_hw *hw;
> > +	int r;
> > +	void __iomem *base;
> > +
> > +	top_clk_data = mtk_alloc_clk_data(CLK_TOP_NR_CLK);
> > +	if (!top_clk_data)
> > +		return -ENOMEM;
> > +
> > +	base = devm_platform_ioremap_resource(pdev, 0);
> > +	if (IS_ERR(base)) {
> > +		r = PTR_ERR(base);
> > +		goto free_top_data;
> > +	}
> > +
> > +	r = mtk_clk_register_fixed_clks(top_fixed_clks,
> > ARRAY_SIZE(top_fixed_clks),
> > +					top_clk_data);
> > +	if (r)
> > +		goto free_top_data;
> > +
> > +	r = mtk_clk_register_factors(top_divs, ARRAY_SIZE(top_divs),
> > top_clk_data);
> > +	if (r)
> > +		goto unregister_fixed_clks;
> > +
> > +	r = mtk_clk_register_muxes(&pdev->dev, top_mtk_muxes,
> > +				   ARRAY_SIZE(top_mtk_muxes), node,
> > +				   &mt8188_clk_lock, top_clk_data);
> > +	if (r)
> > +		goto unregister_factors;
> > +
> > +	hw = devm_clk_hw_register_mux(&pdev->dev, "mfg_ck_fast_ref",
> > mfg_fast_ref_parents,
> > +				      ARRAY_SIZE(mfg_fast_ref_parents),
> > CLK_SET_RATE_PARENT,
> > +				      (base + 0x250), 8, 1, 0,
> > &mt8188_clk_lock);
> 
> If you make this a mtk mux and put it in top_mtk_muxes, you can
> migrate topckgen to
> the simple_probe() mechanism, greatly reducing the size of this file.
> 
After checking, the mt8195 chip uses top_mtk_muxes and does not use
simple_probe() mechanism.

mt8188 and mt8195 also use top_mtk_muxes in the same way, so
mt8188 will not be modified this time.

> > +	if (IS_ERR(hw)) {
> > +		r = PTR_ERR(hw);
> > +		goto unregister_muxes;
> > +	}
> > +	top_clk_data->hws[CLK_TOP_MFG_CK_FAST_REF] = hw;
> > +
> > +	r = clk_mt8188_reg_mfg_mux_notifier(&pdev->dev,
> > +					    top_clk_data-
> > >hws[CLK_TOP_MFG_CK_FAST_REF]->clk);
> > +	if (r)
> > +		goto unregister_muxes;
> > +
> > +	r = mtk_clk_register_composites(&pdev->dev, top_adj_divs,
> > +					ARRAY_SIZE(top_adj_divs), base,
> > +					&mt8188_clk_lock,
> > top_clk_data);
> > +	if (r)
> > +		goto unregister_muxes;
> > +
> > +	r = mtk_clk_register_gates(&pdev->dev, node, top_clks,
> > +				   ARRAY_SIZE(top_clks), top_clk_data);
> > +	if (r)
> > +		goto unregister_composite_divs;
> > +
> > +	r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get,
> > top_clk_data);
> > +	if (r)
> > +		goto unregister_gates;
> > +
> > +	platform_set_drvdata(pdev, top_clk_data);
> > +
> > +	return r;
> > +
> > +unregister_gates:
> > +	mtk_clk_unregister_gates(top_clks, ARRAY_SIZE(top_clks),
> > top_clk_data);
> > +unregister_composite_divs:
> > +	mtk_clk_unregister_composites(top_adj_divs,
> > ARRAY_SIZE(top_adj_divs), top_clk_data);
> > +unregister_muxes:
> > +	mtk_clk_unregister_muxes(top_mtk_muxes,
> > ARRAY_SIZE(top_mtk_muxes), top_clk_data);
> > +unregister_factors:
> > +	mtk_clk_unregister_factors(top_divs, ARRAY_SIZE(top_divs),
> > top_clk_data);
> > +unregister_fixed_clks:
> > +	mtk_clk_unregister_fixed_clks(top_fixed_clks,
> > ARRAY_SIZE(top_fixed_clks), top_clk_data);
> > +free_top_data:
> > +	mtk_free_clk_data(top_clk_data);
> > +	return r;
> > +}
> > +
> > +static int clk_mt8188_topck_remove(struct platform_device *pdev)
> > +{
> > +	struct clk_hw_onecell_data *top_clk_data =
> > platform_get_drvdata(pdev);
> > +	struct device_node *node = pdev->dev.of_node;
> > +
> > +	of_clk_del_provider(node);
> > +	mtk_clk_unregister_gates(top_clks, ARRAY_SIZE(top_clks),
> > top_clk_data);
> > +	mtk_clk_unregister_composites(top_adj_divs,
> > ARRAY_SIZE(top_adj_divs), top_clk_data);
> > +	mtk_clk_unregister_muxes(top_mtk_muxes,
> > ARRAY_SIZE(top_mtk_muxes), top_clk_data);
> > +	mtk_clk_unregister_factors(top_divs, ARRAY_SIZE(top_divs),
> > top_clk_data);
> > +	mtk_clk_unregister_fixed_clks(top_fixed_clks,
> > ARRAY_SIZE(top_fixed_clks), top_clk_data);
> > +	mtk_free_clk_data(top_clk_data);
> > +
> > +	return 0;
> > +}
> > +
> > +static struct platform_driver clk_mt8188_topck_drv = {
> > +	.probe = clk_mt8188_topck_probe,
> > +	.remove = clk_mt8188_topck_remove,
> > +	.driver = {
> > +		.name = "clk-mt8188-topck",
> > +		.of_match_table = of_match_clk_mt8188_topck,
> > +	},
> > +};
> > +builtin_platform_driver(clk_mt8188_topck_drv);
> 
> module_platform_driver....
> MODULE_LICENSE....

Re: [PATCH v6 08/19] clk: mediatek: Add MT8188 imgsys clock support

[AngeloGioacchino Del Regno]

Il 09/03/23 14:54, Garmin.Chang ha scritto:
> Add MT8188 imgsys clock controllers which provide clock gate
> control for image IP blocks.
> 
> Signed-off-by: Garmin.Chang <Garmin.Chang@mediatek.com>
> ---
>   drivers/clk/mediatek/Makefile         |   2 +-
>   drivers/clk/mediatek/clk-mt8188-img.c | 110 ++++++++++++++++++++++++++
>   2 files changed, 111 insertions(+), 1 deletion(-)
>   create mode 100644 drivers/clk/mediatek/clk-mt8188-img.c
> 
> diff --git a/drivers/clk/mediatek/Makefile b/drivers/clk/mediatek/Makefile
> index fb66d25e98fd..935805632018 100644
> --- a/drivers/clk/mediatek/Makefile
> +++ b/drivers/clk/mediatek/Makefile
> @@ -93,7 +93,7 @@ obj-$(CONFIG_COMMON_CLK_MT8186) += clk-mt8186-mcu.o clk-mt8186-topckgen.o clk-mt
>   				   clk-mt8186-cam.o clk-mt8186-mdp.o clk-mt8186-ipe.o
>   obj-$(CONFIG_COMMON_CLK_MT8188) += clk-mt8188-apmixedsys.o clk-mt8188-topckgen.o \
>   				   clk-mt8188-peri_ao.o clk-mt8188-infra_ao.o \
> -				   clk-mt8188-cam.o clk-mt8188-ccu.o
> +				   clk-mt8188-cam.o clk-mt8188-ccu.o clk-mt8188-img.o

imgsys can go under a different config option.

I won't send any more comments on the other 10 clocks for the same reason, so
please split the clocks as needed. Check the others like MT8186, MT8192, MT8195.

Re: [PATCH v6 08/19] clk: mediatek: Add MT8188 imgsys clock support

[Garmin Chang (張家銘)]

On Tue, 2023-03-14 at 10:10 +0100, AngeloGioacchino Del Regno wrote:
> Il 09/03/23 14:54, Garmin.Chang ha scritto:
> > Add MT8188 imgsys clock controllers which provide clock gate
> > control for image IP blocks.
> > 
> > Signed-off-by: Garmin.Chang <Garmin.Chang@mediatek.com>
> > ---
> >   drivers/clk/mediatek/Makefile         |   2 +-
> >   drivers/clk/mediatek/clk-mt8188-img.c | 110
> > ++++++++++++++++++++++++++
> >   2 files changed, 111 insertions(+), 1 deletion(-)
> >   create mode 100644 drivers/clk/mediatek/clk-mt8188-img.c
> > 
> > diff --git a/drivers/clk/mediatek/Makefile
> > b/drivers/clk/mediatek/Makefile
> > index fb66d25e98fd..935805632018 100644
> > --- a/drivers/clk/mediatek/Makefile
> > +++ b/drivers/clk/mediatek/Makefile
> > @@ -93,7 +93,7 @@ obj-$(CONFIG_COMMON_CLK_MT8186) += clk-mt8186-
> > mcu.o clk-mt8186-topckgen.o clk-mt
> >   				   clk-mt8186-cam.o clk-mt8186-mdp.o
> > clk-mt8186-ipe.o
> >   obj-$(CONFIG_COMMON_CLK_MT8188) += clk-mt8188-apmixedsys.o clk-
> > mt8188-topckgen.o \
> >   				   clk-mt8188-peri_ao.o clk-mt8188-
> > infra_ao.o \
> > -				   clk-mt8188-cam.o clk-mt8188-ccu.o
> > +				   clk-mt8188-cam.o clk-mt8188-ccu.o
> > clk-mt8188-img.o
> 
> imgsys can go under a different config option.
> 
> I won't send any more comments on the other 10 clocks for the same
> reason, so
> please split the clocks as needed. Check the others like MT8186,
> MT8192, MT8195.
> 
Thank you for your suggestions.

Ok, I will modify all mt8188 clocks about this in v7.


Thanks,
Best Regards,
Garmin

Re: [PATCH v6 12/19] clk: mediatek: Add MT8188 vdosys0 clock support

[AngeloGioacchino Del Regno]

Il 09/03/23 14:54, Garmin.Chang ha scritto:
> Add MT8188 vdosys0 clock controller which provides clock gate
> control in video system. This is integrated with mtk-mmsys
> driver which will populate device by platform_device_register_data
> to start vdosys clock driver.
> 
> Signed-off-by: Garmin.Chang <Garmin.Chang@mediatek.com>
> ---
>   drivers/clk/mediatek/Makefile          |   3 +-
>   drivers/clk/mediatek/clk-mt8188-vdo0.c | 135 +++++++++++++++++++++++++
>   2 files changed, 137 insertions(+), 1 deletion(-)
>   create mode 100644 drivers/clk/mediatek/clk-mt8188-vdo0.c
> 
> diff --git a/drivers/clk/mediatek/Makefile b/drivers/clk/mediatek/Makefile
> index bf8e50b54bb4..fca66c37cecc 100644
> --- a/drivers/clk/mediatek/Makefile
> +++ b/drivers/clk/mediatek/Makefile
> @@ -94,7 +94,8 @@ obj-$(CONFIG_COMMON_CLK_MT8186) += clk-mt8186-mcu.o clk-mt8186-topckgen.o clk-mt
>   obj-$(CONFIG_COMMON_CLK_MT8188) += clk-mt8188-apmixedsys.o clk-mt8188-topckgen.o \
>   				   clk-mt8188-peri_ao.o clk-mt8188-infra_ao.o \
>   				   clk-mt8188-cam.o clk-mt8188-ccu.o clk-mt8188-img.o \
> -				   clk-mt8188-ipe.o clk-mt8188-mfg.o clk-mt8188-vdec.o
> +				   clk-mt8188-ipe.o clk-mt8188-mfg.o clk-mt8188-vdec.o \
> +				   clk-mt8188-vdo0.o
>   obj-$(CONFIG_COMMON_CLK_MT8192) += clk-mt8192.o
>   obj-$(CONFIG_COMMON_CLK_MT8192_AUDSYS) += clk-mt8192-aud.o
>   obj-$(CONFIG_COMMON_CLK_MT8192_CAMSYS) += clk-mt8192-cam.o
> diff --git a/drivers/clk/mediatek/clk-mt8188-vdo0.c b/drivers/clk/mediatek/clk-mt8188-vdo0.c
> new file mode 100644
> index 000000000000..f649f603aab7
> --- /dev/null
> +++ b/drivers/clk/mediatek/clk-mt8188-vdo0.c
> @@ -0,0 +1,135 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +//
> +// Copyright (c) 2022 MediaTek Inc.
> +// Author: Garmin Chang <garmin.chang@mediatek.com>
> +
> +#include <linux/clk-provider.h>
> +#include <linux/platform_device.h>
> +#include <dt-bindings/clock/mediatek,mt8188-clk.h>
> +
> +#include "clk-gate.h"
> +#include "clk-mtk.h"
> +
> +static const struct mtk_gate_regs vdo0_0_cg_regs = {
> +	.set_ofs = 0x104,
> +	.clr_ofs = 0x108,
> +	.sta_ofs = 0x100,
> +};
> +
> +static const struct mtk_gate_regs vdo0_1_cg_regs = {
> +	.set_ofs = 0x114,
> +	.clr_ofs = 0x118,
> +	.sta_ofs = 0x110,
> +};
> +
> +static const struct mtk_gate_regs vdo0_2_cg_regs = {
> +	.set_ofs = 0x124,
> +	.clr_ofs = 0x128,
> +	.sta_ofs = 0x120,
> +};
> +
> +#define GATE_VDO0_0(_id, _name, _parent, _shift)			\
> +	GATE_MTK(_id, _name, _parent, &vdo0_0_cg_regs, _shift, &mtk_clk_gate_ops_setclr)
> +
> +#define GATE_VDO0_1(_id, _name, _parent, _shift)			\
> +	GATE_MTK(_id, _name, _parent, &vdo0_1_cg_regs, _shift, &mtk_clk_gate_ops_setclr)
> +
> +#define GATE_VDO0_2(_id, _name, _parent, _shift)			\
> +	GATE_MTK(_id, _name, _parent, &vdo0_2_cg_regs, _shift, &mtk_clk_gate_ops_setclr)
> +
> +#define GATE_VDO0_2_FLAGS(_id, _name, _parent, _shift, _flags)		\
> +	GATE_MTK_FLAGS(_id, _name, _parent, &vdo0_2_cg_regs, _shift,	\
> +	&mtk_clk_gate_ops_setclr, _flags)
> +
> +static const struct mtk_gate vdo0_clks[] = {
> +	/* VDO0_0 */
> +	GATE_VDO0_0(CLK_VDO0_DISP_OVL0, "vdo0_disp_ovl0", "top_vpp", 0),
> +	GATE_VDO0_0(CLK_VDO0_FAKE_ENG0, "vdo0_fake_eng0", "top_vpp", 2),
> +	GATE_VDO0_0(CLK_VDO0_DISP_CCORR0, "vdo0_disp_ccorr0", "top_vpp", 4),
> +	GATE_VDO0_0(CLK_VDO0_DISP_MUTEX0, "vdo0_disp_mutex0", "top_vpp", 6),
> +	GATE_VDO0_0(CLK_VDO0_DISP_GAMMA0, "vdo0_disp_gamma0", "top_vpp", 8),
> +	GATE_VDO0_0(CLK_VDO0_DISP_DITHER0, "vdo0_disp_dither0", "top_vpp", 10),
> +	GATE_VDO0_0(CLK_VDO0_DISP_WDMA0, "vdo0_disp_wdma0", "top_vpp", 17),
> +	GATE_VDO0_0(CLK_VDO0_DISP_RDMA0, "vdo0_disp_rdma0", "top_vpp", 19),
> +	GATE_VDO0_0(CLK_VDO0_DSI0, "vdo0_dsi0", "top_vpp", 21),
> +	GATE_VDO0_0(CLK_VDO0_DSI1, "vdo0_dsi1", "top_vpp", 22),
> +	GATE_VDO0_0(CLK_VDO0_DSC_WRAP0, "vdo0_dsc_wrap0", "top_vpp", 23),
> +	GATE_VDO0_0(CLK_VDO0_VPP_MERGE0, "vdo0_vpp_merge0", "top_vpp", 24),
> +	GATE_VDO0_0(CLK_VDO0_DP_INTF0, "vdo0_dp_intf0", "top_vpp", 25),
> +	GATE_VDO0_0(CLK_VDO0_DISP_AAL0, "vdo0_disp_aal0", "top_vpp", 26),
> +	GATE_VDO0_0(CLK_VDO0_INLINEROT0, "vdo0_inlinerot0", "top_vpp", 27),
> +	GATE_VDO0_0(CLK_VDO0_APB_BUS, "vdo0_apb_bus", "top_vpp", 28),
> +	GATE_VDO0_0(CLK_VDO0_DISP_COLOR0, "vdo0_disp_color0", "top_vpp", 29),
> +	GATE_VDO0_0(CLK_VDO0_MDP_WROT0, "vdo0_mdp_wrot0", "top_vpp", 30),
> +	GATE_VDO0_0(CLK_VDO0_DISP_RSZ0, "vdo0_disp_rsz0", "top_vpp", 31),
> +	/* VDO0_1 */
> +	GATE_VDO0_1(CLK_VDO0_DISP_POSTMASK0, "vdo0_disp_postmask0", "top_vpp", 0),
> +	GATE_VDO0_1(CLK_VDO0_FAKE_ENG1, "vdo0_fake_eng1", "top_vpp", 1),
> +	GATE_VDO0_1(CLK_VDO0_DL_ASYNC2, "vdo0_dl_async2", "top_vpp", 5),
> +	GATE_VDO0_1(CLK_VDO0_DL_RELAY3, "vdo0_dl_relay3", "top_vpp", 6),
> +	GATE_VDO0_1(CLK_VDO0_DL_RELAY4, "vdo0_dl_relay4", "top_vpp", 7),
> +	GATE_VDO0_1(CLK_VDO0_SMI_GALS, "vdo0_smi_gals", "top_vpp", 10),
> +	GATE_VDO0_1(CLK_VDO0_SMI_COMMON, "vdo0_smi_common", "top_vpp", 11),
> +	GATE_VDO0_1(CLK_VDO0_SMI_EMI, "vdo0_smi_emi", "top_vpp", 12),
> +	GATE_VDO0_1(CLK_VDO0_SMI_IOMMU, "vdo0_smi_iommu", "top_vpp", 13),
> +	GATE_VDO0_1(CLK_VDO0_SMI_LARB, "vdo0_smi_larb", "top_vpp", 14),
> +	GATE_VDO0_1(CLK_VDO0_SMI_RSI, "vdo0_smi_rsi", "top_vpp", 15),
> +	/* VDO0_2 */
> +	GATE_VDO0_2(CLK_VDO0_DSI0_DSI, "vdo0_dsi0_dsi", "top_dsi_occ", 0),
> +	GATE_VDO0_2(CLK_VDO0_DSI1_DSI, "vdo0_dsi1_dsi", "top_dsi_occ", 8),
> +	GATE_VDO0_2_FLAGS(CLK_VDO0_DP_INTF0_DP_INTF, "vdo0_dp_intf0_dp_intf",
> +		"top_edp", 16, CLK_SET_RATE_PARENT),
> +};
> +
> +static int clk_mt8188_vdo0_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device_node *node = dev->parent->of_node;
> +	struct clk_hw_onecell_data *clk_data;
> +	int r;
> +
> +	clk_data = mtk_alloc_clk_data(CLK_VDO0_NR_CLK);
> +	if (!clk_data)
> +		return -ENOMEM;
> +
> +	r = mtk_clk_register_gates(&pdev->dev, node, vdo0_clks,
> +				   ARRAY_SIZE(vdo0_clks), clk_data);
> +	if (r)
> +		goto free_vdo0_data;
> +
> +	r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_data);
> +	if (r)
> +		goto unregister_gates;
> +
> +	platform_set_drvdata(pdev, clk_data);
> +
> +	return r;
> +
> +unregister_gates:
> +	mtk_clk_unregister_gates(vdo0_clks, ARRAY_SIZE(vdo0_clks), clk_data);
> +free_vdo0_data:
> +	mtk_free_clk_data(clk_data);
> +	return r;
> +}
> +
> +static int clk_mt8188_vdo0_remove(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device_node *node = dev->parent->of_node;
> +	struct clk_hw_onecell_data *clk_data = platform_get_drvdata(pdev);
> +
> +	of_clk_del_provider(node);
> +	mtk_clk_unregister_gates(vdo0_clks, ARRAY_SIZE(vdo0_clks), clk_data);
> +	mtk_free_clk_data(clk_data);
> +
> +	return 0;
> +}
> +

static const struct mtk_clk_desc vdo0_desc = {
	.clks ...
	.num_clks ....
};

static const struct platform_device_id clk_mt8188_vdo0_id_table[] = {
	{ .name = "clk-mt8188-vdo0", .driver_data = (kernel_ulong_T)&vdo0_desc },
	{ /* sentinel */ }
};

> +static struct platform_driver clk_mt8188_vdo0_drv = {
> +	.probe = clk_mt8188_vdo0_probe,
> +	.remove = clk_mt8188_vdo0_remove,

	.probe = mtk_clk_pdev_probe,
	.remove = mtk_clk_pdev_remove,

> +	.driver = {
> +		.name = "clk-mt8188-vdo0",
> +	},

	.id_table = clk_mt8188_vdo0_id_table,

> +};
> +builtin_platform_driver(clk_mt8188_vdo0_drv);

module_platform_driver and MODULE_LICENSE.

Same for VDO1, VPP0, VPP1.

Regards,
Angelo

Re: [PATCH v6 12/19] clk: mediatek: Add MT8188 vdosys0 clock support

[Garmin Chang (張家銘)]

On Tue, 2023-03-14 at 10:13 +0100, AngeloGioacchino Del Regno wrote:
> Il 09/03/23 14:54, Garmin.Chang ha scritto:
> > Add MT8188 vdosys0 clock controller which provides clock gate
> > control in video system. This is integrated with mtk-mmsys
> > driver which will populate device by platform_device_register_data
> > to start vdosys clock driver.
> > 
> > Signed-off-by: Garmin.Chang <Garmin.Chang@mediatek.com>
> > ---
> >   drivers/clk/mediatek/Makefile          |   3 +-
> >   drivers/clk/mediatek/clk-mt8188-vdo0.c | 135
> > +++++++++++++++++++++++++
> >   2 files changed, 137 insertions(+), 1 deletion(-)
> >   create mode 100644 drivers/clk/mediatek/clk-mt8188-vdo0.c
> > 
> > diff --git a/drivers/clk/mediatek/Makefile
> > b/drivers/clk/mediatek/Makefile
> > index bf8e50b54bb4..fca66c37cecc 100644
> > --- a/drivers/clk/mediatek/Makefile
> > +++ b/drivers/clk/mediatek/Makefile
> > @@ -94,7 +94,8 @@ obj-$(CONFIG_COMMON_CLK_MT8186) += clk-mt8186-
> > mcu.o clk-mt8186-topckgen.o clk-mt
> >   obj-$(CONFIG_COMMON_CLK_MT8188) += clk-mt8188-apmixedsys.o clk-
> > mt8188-topckgen.o \
> >   				   clk-mt8188-peri_ao.o clk-mt8188-
> > infra_ao.o \
> >   				   clk-mt8188-cam.o clk-mt8188-ccu.o
> > clk-mt8188-img.o \
> > -				   clk-mt8188-ipe.o clk-mt8188-mfg.o
> > clk-mt8188-vdec.o
> > +				   clk-mt8188-ipe.o clk-mt8188-mfg.o
> > clk-mt8188-vdec.o \
> > +				   clk-mt8188-vdo0.o
> >   obj-$(CONFIG_COMMON_CLK_MT8192) += clk-mt8192.o
> >   obj-$(CONFIG_COMMON_CLK_MT8192_AUDSYS) += clk-mt8192-aud.o
> >   obj-$(CONFIG_COMMON_CLK_MT8192_CAMSYS) += clk-mt8192-cam.o
> > diff --git a/drivers/clk/mediatek/clk-mt8188-vdo0.c
> > b/drivers/clk/mediatek/clk-mt8188-vdo0.c
> > new file mode 100644
> > index 000000000000..f649f603aab7
> > --- /dev/null
> > +++ b/drivers/clk/mediatek/clk-mt8188-vdo0.c
> > @@ -0,0 +1,135 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +//
> > +// Copyright (c) 2022 MediaTek Inc.
> > +// Author: Garmin Chang <garmin.chang@mediatek.com>
> > +
> > +#include <linux/clk-provider.h>
> > +#include <linux/platform_device.h>
> > +#include <dt-bindings/clock/mediatek,mt8188-clk.h>
> > +
> > +#include "clk-gate.h"
> > +#include "clk-mtk.h"
> > +
> > +static const struct mtk_gate_regs vdo0_0_cg_regs = {
> > +	.set_ofs = 0x104,
> > +	.clr_ofs = 0x108,
> > +	.sta_ofs = 0x100,
> > +};
> > +
> > +static const struct mtk_gate_regs vdo0_1_cg_regs = {
> > +	.set_ofs = 0x114,
> > +	.clr_ofs = 0x118,
> > +	.sta_ofs = 0x110,
> > +};
> > +
> > +static const struct mtk_gate_regs vdo0_2_cg_regs = {
> > +	.set_ofs = 0x124,
> > +	.clr_ofs = 0x128,
> > +	.sta_ofs = 0x120,
> > +};
> > +
> > +#define GATE_VDO0_0(_id, _name, _parent, _shift)			
> > \
> > +	GATE_MTK(_id, _name, _parent, &vdo0_0_cg_regs, _shift,
> > &mtk_clk_gate_ops_setclr)
> > +
> > +#define GATE_VDO0_1(_id, _name, _parent, _shift)			
> > \
> > +	GATE_MTK(_id, _name, _parent, &vdo0_1_cg_regs, _shift,
> > &mtk_clk_gate_ops_setclr)
> > +
> > +#define GATE_VDO0_2(_id, _name, _parent, _shift)			
> > \
> > +	GATE_MTK(_id, _name, _parent, &vdo0_2_cg_regs, _shift,
> > &mtk_clk_gate_ops_setclr)
> > +
> > +#define GATE_VDO0_2_FLAGS(_id, _name, _parent, _shift, _flags)	
> > 	\
> > +	GATE_MTK_FLAGS(_id, _name, _parent, &vdo0_2_cg_regs, _shift,	
> > \
> > +	&mtk_clk_gate_ops_setclr, _flags)
> > +
> > +static const struct mtk_gate vdo0_clks[] = {
> > +	/* VDO0_0 */
> > +	GATE_VDO0_0(CLK_VDO0_DISP_OVL0, "vdo0_disp_ovl0", "top_vpp",
> > 0),
> > +	GATE_VDO0_0(CLK_VDO0_FAKE_ENG0, "vdo0_fake_eng0", "top_vpp",
> > 2),
> > +	GATE_VDO0_0(CLK_VDO0_DISP_CCORR0, "vdo0_disp_ccorr0",
> > "top_vpp", 4),
> > +	GATE_VDO0_0(CLK_VDO0_DISP_MUTEX0, "vdo0_disp_mutex0",
> > "top_vpp", 6),
> > +	GATE_VDO0_0(CLK_VDO0_DISP_GAMMA0, "vdo0_disp_gamma0",
> > "top_vpp", 8),
> > +	GATE_VDO0_0(CLK_VDO0_DISP_DITHER0, "vdo0_disp_dither0",
> > "top_vpp", 10),
> > +	GATE_VDO0_0(CLK_VDO0_DISP_WDMA0, "vdo0_disp_wdma0", "top_vpp",
> > 17),
> > +	GATE_VDO0_0(CLK_VDO0_DISP_RDMA0, "vdo0_disp_rdma0", "top_vpp",
> > 19),
> > +	GATE_VDO0_0(CLK_VDO0_DSI0, "vdo0_dsi0", "top_vpp", 21),
> > +	GATE_VDO0_0(CLK_VDO0_DSI1, "vdo0_dsi1", "top_vpp", 22),
> > +	GATE_VDO0_0(CLK_VDO0_DSC_WRAP0, "vdo0_dsc_wrap0", "top_vpp",
> > 23),
> > +	GATE_VDO0_0(CLK_VDO0_VPP_MERGE0, "vdo0_vpp_merge0", "top_vpp",
> > 24),
> > +	GATE_VDO0_0(CLK_VDO0_DP_INTF0, "vdo0_dp_intf0", "top_vpp", 25),
> > +	GATE_VDO0_0(CLK_VDO0_DISP_AAL0, "vdo0_disp_aal0", "top_vpp",
> > 26),
> > +	GATE_VDO0_0(CLK_VDO0_INLINEROT0, "vdo0_inlinerot0", "top_vpp",
> > 27),
> > +	GATE_VDO0_0(CLK_VDO0_APB_BUS, "vdo0_apb_bus", "top_vpp", 28),
> > +	GATE_VDO0_0(CLK_VDO0_DISP_COLOR0, "vdo0_disp_color0",
> > "top_vpp", 29),
> > +	GATE_VDO0_0(CLK_VDO0_MDP_WROT0, "vdo0_mdp_wrot0", "top_vpp",
> > 30),
> > +	GATE_VDO0_0(CLK_VDO0_DISP_RSZ0, "vdo0_disp_rsz0", "top_vpp",
> > 31),
> > +	/* VDO0_1 */
> > +	GATE_VDO0_1(CLK_VDO0_DISP_POSTMASK0, "vdo0_disp_postmask0",
> > "top_vpp", 0),
> > +	GATE_VDO0_1(CLK_VDO0_FAKE_ENG1, "vdo0_fake_eng1", "top_vpp",
> > 1),
> > +	GATE_VDO0_1(CLK_VDO0_DL_ASYNC2, "vdo0_dl_async2", "top_vpp",
> > 5),
> > +	GATE_VDO0_1(CLK_VDO0_DL_RELAY3, "vdo0_dl_relay3", "top_vpp",
> > 6),
> > +	GATE_VDO0_1(CLK_VDO0_DL_RELAY4, "vdo0_dl_relay4", "top_vpp",
> > 7),
> > +	GATE_VDO0_1(CLK_VDO0_SMI_GALS, "vdo0_smi_gals", "top_vpp", 10),
> > +	GATE_VDO0_1(CLK_VDO0_SMI_COMMON, "vdo0_smi_common", "top_vpp",
> > 11),
> > +	GATE_VDO0_1(CLK_VDO0_SMI_EMI, "vdo0_smi_emi", "top_vpp", 12),
> > +	GATE_VDO0_1(CLK_VDO0_SMI_IOMMU, "vdo0_smi_iommu", "top_vpp",
> > 13),
> > +	GATE_VDO0_1(CLK_VDO0_SMI_LARB, "vdo0_smi_larb", "top_vpp", 14),
> > +	GATE_VDO0_1(CLK_VDO0_SMI_RSI, "vdo0_smi_rsi", "top_vpp", 15),
> > +	/* VDO0_2 */
> > +	GATE_VDO0_2(CLK_VDO0_DSI0_DSI, "vdo0_dsi0_dsi", "top_dsi_occ",
> > 0),
> > +	GATE_VDO0_2(CLK_VDO0_DSI1_DSI, "vdo0_dsi1_dsi", "top_dsi_occ",
> > 8),
> > +	GATE_VDO0_2_FLAGS(CLK_VDO0_DP_INTF0_DP_INTF,
> > "vdo0_dp_intf0_dp_intf",
> > +		"top_edp", 16, CLK_SET_RATE_PARENT),
> > +};
> > +
> > +static int clk_mt8188_vdo0_probe(struct platform_device *pdev)
> > +{
> > +	struct device *dev = &pdev->dev;
> > +	struct device_node *node = dev->parent->of_node;
> > +	struct clk_hw_onecell_data *clk_data;
> > +	int r;
> > +
> > +	clk_data = mtk_alloc_clk_data(CLK_VDO0_NR_CLK);
> > +	if (!clk_data)
> > +		return -ENOMEM;
> > +
> > +	r = mtk_clk_register_gates(&pdev->dev, node, vdo0_clks,
> > +				   ARRAY_SIZE(vdo0_clks), clk_data);
> > +	if (r)
> > +		goto free_vdo0_data;
> > +
> > +	r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get,
> > clk_data);
> > +	if (r)
> > +		goto unregister_gates;
> > +
> > +	platform_set_drvdata(pdev, clk_data);
> > +
> > +	return r;
> > +
> > +unregister_gates:
> > +	mtk_clk_unregister_gates(vdo0_clks, ARRAY_SIZE(vdo0_clks),
> > clk_data);
> > +free_vdo0_data:
> > +	mtk_free_clk_data(clk_data);
> > +	return r;
> > +}
> > +
> > +static int clk_mt8188_vdo0_remove(struct platform_device *pdev)
> > +{
> > +	struct device *dev = &pdev->dev;
> > +	struct device_node *node = dev->parent->of_node;
> > +	struct clk_hw_onecell_data *clk_data =
> > platform_get_drvdata(pdev);
> > +
> > +	of_clk_del_provider(node);
> > +	mtk_clk_unregister_gates(vdo0_clks, ARRAY_SIZE(vdo0_clks),
> > clk_data);
> > +	mtk_free_clk_data(clk_data);
> > +
> > +	return 0;
> > +}
> > +
> 
> static const struct mtk_clk_desc vdo0_desc = {
> 	.clks ...
> 	.num_clks ....
> };
> 
> static const struct platform_device_id clk_mt8188_vdo0_id_table[] = {
> 	{ .name = "clk-mt8188-vdo0", .driver_data =
> (kernel_ulong_T)&vdo0_desc },
> 	{ /* sentinel */ }
> };
> 
> > +static struct platform_driver clk_mt8188_vdo0_drv = {
> > +	.probe = clk_mt8188_vdo0_probe,
> > +	.remove = clk_mt8188_vdo0_remove,
> 
> 	.probe = mtk_clk_pdev_probe,
> 	.remove = mtk_clk_pdev_remove,
> 
> > +	.driver = {
> > +		.name = "clk-mt8188-vdo0",
> > +	},
> 
> 	.id_table = clk_mt8188_vdo0_id_table,
> 
> > +};
> > +builtin_platform_driver(clk_mt8188_vdo0_drv);
> 
> module_platform_driver and MODULE_LICENSE.
> 
> Same for VDO1, VPP0, VPP1.
> 
> Regards,
> Angelo

Thank you for your suggestions.

OK, I use mtk_clk_pdev_probe for VDO0, VDO1, VPP0, VPP1 in v7.

Thanks,
Best Regards,
Garmin