linux-kernel - Re: [PATCH v2 08/19] clk: mediatek: Add MT8188 imgsys clock support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <cb050482-cf6e-829e-7fd4-cfcde72c545b@collabora.com>
Date:   Thu, 27 Oct 2022 10:25:40 +0200
From:   AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...labora.com>
To:     "Garmin.Chang" <Garmin.Chang@...iatek.com>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>,
        Richard Cochran <richardcochran@...il.com>
Cc:     Project_Global_Chrome_Upstream_Group@...iatek.com,
        devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org, linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH v2 08/19] clk: mediatek: Add MT8188 imgsys clock support

Il 24/10/22 11:42, 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@...iatek.com>
> ---
>   drivers/clk/mediatek/Makefile         |   2 +-
>   drivers/clk/mediatek/clk-mt8188-img.c | 124 ++++++++++++++++++++++++++
>   2 files changed, 125 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 bd0a2aa5b6fa..242b49bafa9e 100644
> --- a/drivers/clk/mediatek/Makefile
> +++ b/drivers/clk/mediatek/Makefile
> @@ -84,7 +84,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
>   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-img.c b/drivers/clk/mediatek/clk-mt8188-img.c
> new file mode 100644
> index 000000000000..00f3bbf4d502
> --- /dev/null
> +++ b/drivers/clk/mediatek/clk-mt8188-img.c
> @@ -0,0 +1,124 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +//
> +// Copyright (c) 2022 MediaTek Inc.
> +// Author: Garmin Chang <garmin.chang@...iatek.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 imgsys_cg_regs = {
> +	.set_ofs = 0x4,
> +	.clr_ofs = 0x8,
> +	.sta_ofs = 0x0,
> +};
> +
> +#define GATE_IMGSYS(_id, _name, _parent, _shift)			\
> +	GATE_MTK(_id, _name, _parent, &imgsys_cg_regs, _shift, &mtk_clk_gate_ops_setclr)
> +
> +static const struct mtk_gate imgsys_main_clks[] = {
> +	GATE_IMGSYS(CLK_IMGSYS_MAIN_LARB9, "imgsys_main_larb9", "top_img", 0),
> +	GATE_IMGSYS(CLK_IMGSYS_MAIN_TRAW0, "imgsys_main_traw0", "top_img", 1),
> +	GATE_IMGSYS(CLK_IMGSYS_MAIN_TRAW1, "imgsys_main_traw1", "top_img", 2),
> +	GATE_IMGSYS(CLK_IMGSYS_MAIN_VCORE_GALS, "imgsys_main_vcore_gals", "top_img", 3),
> +	GATE_IMGSYS(CLK_IMGSYS_MAIN_DIP0, "imgsys_main_dip0", "top_img", 8),
> +	GATE_IMGSYS(CLK_IMGSYS_MAIN_WPE0, "imgsys_main_wpe0", "top_img", 9),
> +	GATE_IMGSYS(CLK_IMGSYS_MAIN_IPE, "imgsys_main_ipe", "top_img", 10),
> +	GATE_IMGSYS(CLK_IMGSYS_MAIN_WPE1, "imgsys_main_wpe1", "top_img", 12),
> +	GATE_IMGSYS(CLK_IMGSYS_MAIN_WPE2, "imgsys_main_wpe2", "top_img", 13),
> +	GATE_IMGSYS(CLK_IMGSYS_MAIN_GALS, "imgsys_main_gals", "top_img", 31),
> +};
> +
> +static const struct mtk_gate imgsys_wpe1_clks[] = {
> +	GATE_IMGSYS(CLK_IMGSYS_WPE1_LARB11, "imgsys_wpe1_larb11", "top_img", 0),
> +	GATE_IMGSYS(CLK_IMGSYS_WPE1, "imgsys_wpe1", "top_img", 1),
> +};
> +
> +static const struct mtk_gate imgsys_wpe2_clks[] = {
> +	GATE_IMGSYS(CLK_IMGSYS_WPE2_LARB11, "imgsys_wpe2_larb11", "top_img", 0),
> +	GATE_IMGSYS(CLK_IMGSYS_WPE2, "imgsys_wpe2", "top_img", 1),
> +};
> +
> +static const struct mtk_gate imgsys_wpe3_clks[] = {
> +	GATE_IMGSYS(CLK_IMGSYS_WPE3_LARB11, "imgsys_wpe3_larb11", "top_img", 0),
> +	GATE_IMGSYS(CLK_IMGSYS_WPE3, "imgsys_wpe3", "top_img", 1),
> +};
> +
> +static const struct mtk_gate imgsys1_dip_top_clks[] = {
> +	GATE_IMGSYS(CLK_IMGSYS1_DIP_TOP_LARB10, "imgsys1_dip_larb10", "top_img", 0),
> +	GATE_IMGSYS(CLK_IMGSYS1_DIP_TOP_DIP_TOP, "imgsys1_dip_dip_top", "top_img", 1),
> +};
> +
> +static const struct mtk_gate imgsys1_dip_nr_clks[] = {
> +	GATE_IMGSYS(CLK_IMGSYS1_DIP_NR_LARB15, "imgsys1_dip_nr_larb15", "top_img", 0),
> +	GATE_IMGSYS(CLK_IMGSYS1_DIP_NR_DIP_NR, "imgsys1_dip_nr_dip_nr", "top_img", 1),
> +};
> +
> +static const struct mtk_clk_desc imgsys_main_desc = {
> +	.clks = imgsys_main_clks,
> +	.num_clks = ARRAY_SIZE(imgsys_main_clks),
> +};
> +
> +static const struct mtk_clk_desc imgsys_wpe1_desc = {
> +	.clks = imgsys_wpe1_clks,
> +	.num_clks = ARRAY_SIZE(imgsys_wpe1_clks),
> +};
> +
> +static const struct mtk_clk_desc imgsys_wpe2_desc = {
> +	.clks = imgsys_wpe2_clks,
> +	.num_clks = ARRAY_SIZE(imgsys_wpe2_clks),
> +};
> +
> +static const struct mtk_clk_desc imgsys_wpe3_desc = {
> +	.clks = imgsys_wpe3_clks,
> +	.num_clks = ARRAY_SIZE(imgsys_wpe3_clks),
> +};
> +
> +static const struct mtk_clk_desc imgsys1_dip_top_desc = {
> +	.clks = imgsys1_dip_top_clks,
> +	.num_clks = ARRAY_SIZE(imgsys1_dip_top_clks),
> +};
> +
> +static const struct mtk_clk_desc imgsys1_dip_nr_desc = {
> +	.clks = imgsys1_dip_nr_clks,
> +	.num_clks = ARRAY_SIZE(imgsys1_dip_nr_clks),
> +};
> +
> +static const struct of_device_id of_match_clk_mt8188_imgsys_main[] = {
> +	{
> +		.compatible = "mediatek,mt8188-imgsys",
> +		.data = &imgsys_main_desc,
> +	}, {
> +		.compatible = "mediatek,mt8188-imgsys_wpe1",

I know that this was done in other clock drivers as well, but can we please
stop using underscores in devicetree compatibles?
That makes them look more consistent with the rest of the DT.

"mediatek,mt8188-imgsys-wpe1", as an example, would look a bit better.

P.S.: Please do the same on all of the other drivers that you are introducing
       with this series.

Thanks,
Angelo