[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <cb070d12-7e6d-4b0b-9dad-af4b9d6c51bc@collabora.com>
Date: Tue, 22 Oct 2024 11:05:01 +0200
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
To: Yassine Oudjana <yassine.oudjana@...il.com>,
Michael Turquette <mturquette@...libre.com>, Stephen Boyd
<sboyd@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, Matthias Brugger <matthias.bgg@...il.com>,
Philipp Zabel <p.zabel@...gutronix.de>,
Lukas Bulwahn <lukas.bulwahn@...hat.com>,
Daniel Golle <daniel@...rotopia.org>, Sam Shih <sam.shih@...iatek.com>
Cc: Yassine Oudjana <y.oudjana@...tonmail.com>, linux-clk@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-mediatek@...ts.infradead.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 2/2] clk: mediatek: Add drivers for MT6735 syscon clock
and reset controllers
Il 21/10/24 14:16, Yassine Oudjana ha scritto:
> From: Yassine Oudjana <y.oudjana@...tonmail.com>
>
> Add drivers for IMGSYS, MFGCFG, VDECSYS and VENCSYS clocks and resets
> on MT6735.
>
> Signed-off-by: Yassine Oudjana <y.oudjana@...tonmail.com>
> ---
> MAINTAINERS | 4 ++
> drivers/clk/mediatek/Kconfig | 32 +++++++++
> drivers/clk/mediatek/Makefile | 4 ++
> drivers/clk/mediatek/clk-mt6735-imgsys.c | 57 ++++++++++++++++
> drivers/clk/mediatek/clk-mt6735-mfgcfg.c | 61 +++++++++++++++++
> drivers/clk/mediatek/clk-mt6735-vdecsys.c | 81 +++++++++++++++++++++++
> drivers/clk/mediatek/clk-mt6735-vencsys.c | 53 +++++++++++++++
> 7 files changed, 292 insertions(+)
> create mode 100644 drivers/clk/mediatek/clk-mt6735-imgsys.c
> create mode 100644 drivers/clk/mediatek/clk-mt6735-mfgcfg.c
> create mode 100644 drivers/clk/mediatek/clk-mt6735-vdecsys.c
> create mode 100644 drivers/clk/mediatek/clk-mt6735-vencsys.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 25484783f6a0b..939f9d29fc9bf 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -14533,9 +14533,13 @@ L: linux-clk@...r.kernel.org
> L: linux-mediatek@...ts.infradead.org (moderated for non-subscribers)
> S: Maintained
> F: drivers/clk/mediatek/clk-mt6735-apmixedsys.c
> +F: drivers/clk/mediatek/clk-mt6735-imgsys.c
> F: drivers/clk/mediatek/clk-mt6735-infracfg.c
> +F: drivers/clk/mediatek/clk-mt6735-mfgcfg.c
> F: drivers/clk/mediatek/clk-mt6735-pericfg.c
> F: drivers/clk/mediatek/clk-mt6735-topckgen.c
> +F: drivers/clk/mediatek/clk-mt6735-vdecsys.c
> +F: drivers/clk/mediatek/clk-mt6735-vencsys.c
> F: include/dt-bindings/clock/mediatek,mt6735-apmixedsys.h
> F: include/dt-bindings/clock/mediatek,mt6735-imgsys.h
> F: include/dt-bindings/clock/mediatek,mt6735-infracfg.h
> diff --git a/drivers/clk/mediatek/Kconfig b/drivers/clk/mediatek/Kconfig
> index 7a33f9e92d963..4dd6d2d6263fd 100644
> --- a/drivers/clk/mediatek/Kconfig
> +++ b/drivers/clk/mediatek/Kconfig
> @@ -133,6 +133,38 @@ config COMMON_CLK_MT6735
> by apmixedsys, topckgen, infracfg and pericfg on the
> MediaTek MT6735 SoC.
>
> +config COMMON_CLK_MT6735_IMGSYS
> + tristate "Clock driver for MediaTek MT6735 imgsys"
> + depends on (ARCH_MEDIATEK && COMMON_CLK_MT6735) || COMPILE_TEST
If this depends on COMMON_CLK_MT6735, it automatically also depends on
ARCH_MEDIATEK, because the former cannot be selected without satisfying
the dependency on the latter.
Also, with those being really dependant on COMMON_CLK_MT6735, it does
not make any sense to COMPILE_TEST those alone anyway...
> + select COMMON_CLK_MEDIATEK
The same goes for this select statement: it's already done when selecting
COMMON_CLK_MT6735.
Finally:
config COMMON_CLK_MT6735_IMGSYS
tristate "Clock driver for MediaTek MT6735 imgsys"
depends on COMMON_CLK_MT6735
help
blah blah blah
is just fine - and shorter too :-)
> + help
> + This enables a driver for clocks provided by imgsys
> + on the MediaTek MT6735 SoC.
> +
> +config COMMON_CLK_MT6735_MFGCFG
> + tristate "Clock driver for MediaTek MT6735 mfgcfg"
> + depends on (ARCH_MEDIATEK && COMMON_CLK_MT6735) || COMPILE_TEST
> + select COMMON_CLK_MEDIATEK
> + help
> + This enables a driver for clocks and resets provided
> + by mfgcfg on the MediaTek MT6735 SoC.
> +
> +config COMMON_CLK_MT6735_VDECSYS
> + tristate "Clock driver for MediaTek MT6735 vdecsys"
> + depends on (ARCH_MEDIATEK && COMMON_CLK_MT6735) || COMPILE_TEST
> + select COMMON_CLK_MEDIATEK
> + help
> + This enables a driver for clocks and resets provided
> + by vdecsys on the MediaTek MT6735 SoC.
> +
> +config COMMON_CLK_MT6735_VENCSYS
> + tristate "Clock driver for MediaTek MT6735 vencsys"
> + depends on (ARCH_MEDIATEK && COMMON_CLK_MT6735) || COMPILE_TEST
> + select COMMON_CLK_MEDIATEK
> + help
> + This enables a driver for clocks provided by vencsys
> + on the MediaTek MT6735 SoC.
> +
> config COMMON_CLK_MT6765
> bool "Clock driver for MediaTek MT6765"
> depends on (ARCH_MEDIATEK && ARM64) || COMPILE_TEST
..snip..
> diff --git a/drivers/clk/mediatek/clk-mt6735-vdecsys.c b/drivers/clk/mediatek/clk-mt6735-vdecsys.c
> new file mode 100644
> index 0000000000000..f59b481aaa6da
> --- /dev/null
> +++ b/drivers/clk/mediatek/clk-mt6735-vdecsys.c
> @@ -0,0 +1,81 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2022 Yassine Oudjana <y.oudjana@...tonmail.com>
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/platform_device.h>
> +
> +#include "clk-gate.h"
> +#include "clk-mtk.h"
> +
> +#include <dt-bindings/clock/mediatek,mt6735-vdecsys.h>
> +#include <dt-bindings/reset/mediatek,mt6735-vdecsys.h>
> +
..snip..
> +static u16 vdecsys_rst_bank_ofs[] = { VDEC_RESETB_CON, SMI_LARB1_RESETB_CON };
> +
> +static u16 vdecsys_rst_idx_map[] = {
> + [MT6735_VDEC_RST0_VDEC] = 0 * RST_NR_PER_BANK + 0,
> +
Please remove this extra blank line, it's not needed.
> + [MT6735_VDEC_RST1_SMI_LARB1] = 1 * RST_NR_PER_BANK + 0,
> +};
> +
> +static const struct mtk_clk_rst_desc vdecsys_resets = {
> + .version = MTK_RST_SIMPLE,
> + .rst_bank_ofs = vdecsys_rst_bank_ofs,
> + .rst_bank_nr = ARRAY_SIZE(vdecsys_rst_bank_ofs),
> + .rst_idx_map = vdecsys_rst_idx_map,
> + .rst_idx_map_nr = ARRAY_SIZE(vdecsys_rst_idx_map)
> +};
> +
> +static const struct mtk_clk_desc vdecsys_clks = {
> + .clks = vdecsys_gates,
> + .num_clks = ARRAY_SIZE(vdecsys_gates),
> +
same here.
> + .rst_desc = &vdecsys_resets
> +};
> +
The rest looks good, and I'm confident that you're getting my R-b on v2.
Cheers,
Angelo
Powered by blists - more mailing lists