| 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
| ||
|
Message-ID: <1j8qlrd512.fsf@starbuckisacylon.baylibre.com>
Date: Mon, 16 Jun 2025 18:28:57 +0200
From: Jerome Brunet <jbrunet@...libre.com>
To: Jian Hu <jian.hu@...ogic.com>
Cc: Xianwei Zhao <xianwei.zhao@...ogic.com>, Chuan Liu
<chuan.liu@...ogic.com>, Neil Armstrong <neil.armstrong@...aro.org>,
Kevin Hilman <khilman@...libre.com>, Stephen Boyd <sboyd@...nel.org>,
Michael Turquette <mturquette@...libre.com>, Dmitry Rokosov
<ddrokosov@...rdevices.ru>, robh+dt <robh+dt@...nel.org>, Rob Herring
<robh@...nel.org>, devicetree <devicetree@...r.kernel.org>, linux-clk
<linux-clk@...r.kernel.org>, linux-amlogic
<linux-amlogic@...ts.infradead.org>, linux-kernel
<linux-kernel@...r.kernel.org>, linux-arm-kernel
<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v3 5/6] clk: meson: add MESON_PCLK_V2 for sys gate clock
On Fri 13 Jun 2025 at 13:35, Jian Hu <jian.hu@...ogic.com> wrote:
> On 2025/5/14 15:39, Jerome Brunet wrote:
>
> [ EXTERNAL EMAIL ]
Please fix your mailer, can't read that reply
>
> On Fri 09 May 2025 at 07:48, Jian Hu <jian.hu@...ogic.com> wrote:
>
> A new MESON_PCLK_V2 macro is introduced for the sys gate clock. Its parent
> is an SCMI clock. It belongs to another clock controller, and the parent
> configuration is different from that of MESON_PCLK. This avoids new macro
> definition in the peripheral clock driver.
>
> Signed-off-by: Jian Hu <jian.hu@...ogic.com>
> ---
> drivers/clk/meson/clk-regmap.h | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/drivers/clk/meson/clk-regmap.h b/drivers/clk/meson/clk-regmap.h
> index e365312da54e..61b8fc2d875f 100644
> --- a/drivers/clk/meson/clk-regmap.h
> +++ b/drivers/clk/meson/clk-regmap.h
>
>
> This file is not meant for amlogic specific stuff. I know some found
> their way in regardless but that's being fixed
>
> I found macro MESON_PCLK is used in G12A/MESON8B/GXBB/S4.
>
> you mean the macro will be fixed? and use something else instead.
>
>
> @@ -134,4 +134,28 @@ struct clk_regmap _name = { \
>
> #define MESON_PCLK_RO(_name, _reg, _bit, _pname) \
> __MESON_PCLK(_name, _reg, _bit, &clk_regmap_gate_ro_ops, _pname)
> +
> +#define __MESON_PCLK_V2(_name, _reg, _bit, _ops, _pname) \
> +struct clk_regmap _name = { \
> + .data = &(struct clk_regmap_gate_data){ \
> + .offset = (_reg), \
> + .bit_idx = (_bit), \
> + }, \
> + .hw.init = &(struct clk_init_data) { \
> + .name = #_name, \
> + .ops = _ops, \
> + .parent_data = &(const struct clk_parent_data) { \
> + .fw_name = #_pname, \
> + }, \
> + .num_parents = 1, \
> + .flags = (CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED), \
> + }, \
> +}
>
>
> The proliferation of those macros has been going on for far too long,
> add using CLK_IGNORE_UNUSED inside is certainly a mistake I won't
> repeat.
>
> This will be part of more general clean-up that currently depends on a
> this [1] patch to go further. You'll have to be patient.
>
> [1]: https://lore.kernel.org/r/20250417-clk-hw-get-helpers-v1-0-7743e509612a@baylibre.com
>
> OK, CLK_IGNORE_UNUSED is added for all gate clocks. it is better to remove it. and wait for your patch merged.
>
> And quote your comments in v2. the T7_CLK_GATE is newly added in T7 Peripheral driver in v2.
>
> If I understand you correctly. A common macro should be added for it and remove T7_CLK_GATE.
>
> So I add a common macro in clk-regmap.h, and it can be used for different SoCs.
>
> MESON_PCLK and MESON_PCLK_V2 difference is a clocks's parent defination.
>
> the sys gate clock parent is in another clock controller. So parent_data member is need for it.
>
> and parent_hw member is for previous SoCs which clock and parent clock are in one clock controller.
>
> How should we deal with it.
>
> 1.Add the macro in T7 Perpheral driver.
>
> 2.Or add a common macro for parent_data, and where the macro should be placed?
>
> ---
>
>> +#define T7_CLK_GATE(_name, _reg, _bit, _fw_name, _flags) \
>
> See, redefining the peripheral once again ... something all the SoCs
> uses with minor variation.
>
>> +struct clk_regmap _name = { \
>> + .data = &(struct clk_regmap_gate_data){ \
>> + .offset = (_reg), \
>> + .bit_idx = (_bit), \
>> + }, \
>> + .hw.init = &(struct clk_init_data) { \
>> + .name = #_name, \
>
> There is an exception in the naming convention for peripheral clocks.
>
> The name is soc id prefixed in most SoC. It is these pointless minor
> diff that makes factorisation difficult.
>
>> + .ops = &clk_regmap_gate_ops, \
>> + .parent_data = &(const struct clk_parent_data) { \
>> + .fw_name = #_fw_name, \
>> + }, \
>> + .num_parents = 1, \
>> + .flags = (_flags), \
>> + }, \
>
> ---
>
>
> +
> +#define MESON_PCLK_V2(_name, _reg, _bit, _pname) \
> + __MESON_PCLK_V2(_name, _reg, _bit, &clk_regmap_gate_ops, _pname)
> +
> +#define MESON_PCLK_RO_V2(_name, _reg, _bit, _pname) \
> + __MESON_PCLK_V2(_name, _reg, _bit, &clk_regmap_gate_ro_ops, _pname)
> +
> #endif /* __CLK_REGMAP_H */
--
Jerome
Powered by blists - more mailing lists