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-prev] [day] [month] [year] [list]
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

Powered by Openwall GNU/*/Linux Powered by OpenVZ