linux-kernel - Re: [PATCH v3 4/4] clk: mediatek: Add drivers for MediaTek MT6735 main clock and reset drivers

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 27 Feb 2023 10:28:06 +0100
From:   AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...labora.com>
To:     Yassine Oudjana <yassine.oudjana@...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>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Daniel Golle <daniel@...rotopia.org>,
        Allen-KH Cheng <allen-kh.cheng@...iatek.com>,
        Tinghan Shen <tinghan.shen@...iatek.com>,
        Chen-Yu Tsai <wenst@...omium.org>,
        Edward-JW Yang <edward-jw.yang@...iatek.com>,
        Johnson Wang <johnson.wang@...iatek.com>,
        Fabien Parent <fparent@...libre.com>,
        Chun-Jie Chen <chun-jie.chen@...iatek.com>,
        Miles Chen <miles.chen@...iatek.com>,
        Bartosz Golaszewski <bgolaszewski@...libre.com>
Cc:     Yassine Oudjana <y.oudjana@...tonmail.com>,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-clk@...r.kernel.org, linux-mediatek@...ts.infradead.org,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v3 4/4] clk: mediatek: Add drivers for MediaTek MT6735
 main clock and reset drivers

Il 25/02/23 10:42, Yassine Oudjana ha scritto:
> From: Yassine Oudjana <y.oudjana@...tonmail.com>
> 
> Add drivers for MT6735 apmixedsys, topckgen, infracfg and pericfg
> clock and reset controllers. These provide the base clocks and resets
> on the platform, and should be enough to bring up all essential blocks
> including PWRAP, MSDC and peripherals (UART, I2C, SPI).
> 
> Signed-off-by: Yassine Oudjana <y.oudjana@...tonmail.com>
> ---
>   MAINTAINERS                                  |   4 +
>   drivers/clk/mediatek/Kconfig                 |   9 +
>   drivers/clk/mediatek/Makefile                |   1 +
>   drivers/clk/mediatek/clk-mt6735-apmixedsys.c | 139 ++++++
>   drivers/clk/mediatek/clk-mt6735-infracfg.c   |  78 ++++
>   drivers/clk/mediatek/clk-mt6735-pericfg.c    |  91 ++++
>   drivers/clk/mediatek/clk-mt6735-topckgen.c   | 450 +++++++++++++++++++
>   7 files changed, 772 insertions(+)
>   create mode 100644 drivers/clk/mediatek/clk-mt6735-apmixedsys.c
>   create mode 100644 drivers/clk/mediatek/clk-mt6735-infracfg.c
>   create mode 100644 drivers/clk/mediatek/clk-mt6735-pericfg.c
>   create mode 100644 drivers/clk/mediatek/clk-mt6735-topckgen.c
> 

..snip..

> diff --git a/drivers/clk/mediatek/clk-mt6735-topckgen.c b/drivers/clk/mediatek/clk-mt6735-topckgen.c
> new file mode 100644
> index 000000000000..5fa743e4b0fc
> --- /dev/null
> +++ b/drivers/clk/mediatek/clk-mt6735-topckgen.c
> @@ -0,0 +1,450 @@
> +// 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-mtk.h"
> +#include "clk-mux.h"
> +
> +#include <dt-bindings/clock/mediatek,mt6735-topckgen.h>
> +

..snip..

> +
> +int clk_mt6735_topckgen_probe(struct platform_device *pdev)

It gets *even easier* than that!

Check out this one:
https://patchwork.kernel.org/project/linux-mediatek/patch/20230222092543.19187-5-angelogioacchino.delregno@collabora.com/

...being part of:
https://patchwork.kernel.org/project/linux-mediatek/list/?series=724004

So you can use simple_probe for MT6735's topckgen too!

In this case, it would be...

static const struct mtk_clk_desc topck_desc = {
	.clks = topckgen_muxes,
	.num_clks = ARRAY_SIZE(topckgen_muxes),
	.fixed_clks = topckgen_fixed_clks,
	.num_fixed_clks = ARRAY_SIZE(topckgen_fixed_clks),
	.factor_clks = topckgen_factors,
	.num_factor_clks = ARRAY_SIZE(topckgen_factors),
	.clk_lock = &mt6735_topckgen_lock,
};

static const struct of_device_id of_match_mt6735_topckgen[] = {
	{ .compatible = "mediatek,mt6735-topckgen", .data = &topck_desc },
	{ /* sentinel */ }
};

MODULE_DEVICE_TABLE(of, of_match_mt6735_topckgen)
     ^^^^^
You're missing that on multiple clock drivers ;-)

...And you're replacing .probe(), .remove() callbacks with

static struct platform_driver clk_mt6735_topckgen = {
	.probe = mtk_clk_simple_probe,
	.remove = mtk_clk_simple_remove,

	......

Other than that, good job!

After performing these changes, please make sure to mention the dependency on
my last cleanup series on your cover letter for v4, so that maintainers will
be aware of what to do.

Your v4 smells like Reviewed-by tags all over. Keep up the great work!

Cheers,
Angelo