[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <04acf661-06b5-22b5-ca5d-50078495f7e8@collabora.com>
Date: Mon, 27 Feb 2023 13:42:20 +0100
From: AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com>
To: Yassine Oudjana <yassine.oudjana@...il.com>
Cc: 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>,
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 27/02/23 11:39, Yassine Oudjana ha scritto:
>
> On Mon, Feb 27 2023 at 10:28:06 AM +01:00:00, AngeloGioacchino Del Regno
> <angelogioacchino.delregno@...labora.com> wrote:
>> 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!
>
> Isn't this basically what I did in v2[1][2]? What changed now?
>
*Basically*, yes. *Practically*, no.
To answer all your questions about that, please read my part 1 series that already
landed. The part 2 adds the factor clocks to the mix and performs a full migration
to platform_driver and modularity to 99% of MediaTek clock drivers.
>>
>> 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
>
> [1]
> https://lore.kernel.org/linux-mediatek/20220519142211.458336-5-y.oudjana@protonmail.com/
> [2]
> https://patchwork.kernel.org/project/linux-clk/patch/20220519134728.456643-7-y.oudjana@protonmail.com/
>
>
>
Powered by blists - more mailing lists