Re: [PATCH v3 2/2] clk: amlogic: Add A9 peripherals clock controller driver

[Jian Hu <jian.hu@amlogic.com>]

On 6/16/2026 3:51 PM, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
>
> On mar. 16 juin 2026 at 14:12, Jian Hu <jian.hu@amlogic.com> wrote:
>
>
>>>> If you think splitting it further into separate helper macros would improve
>>>> readability.
>>> One clock per macro please. Hidding 2 declaration is recipe for
>>> disaster. For ex, here the first one is static, the 2nd is not
>>
>> I'll split it into separate helper macros so that each macro expands to a
>> single clock definition.
>>
>> They are defined as follows: (Excluding struct clk_regmap)
>>
>> #define A9_VCLK_GATE(_name, _reg, _bit,  _parent)        \
>>          .data = &(struct clk_regmap_gate_data){          \
>>                  .offset = _reg,      \
>>                  .bit_idx = _bit,       \
>>          },       \
>>          .hw.init = &(struct clk_init_data) {           \
>>                  .name = #_name "_en",      \
>>                  .ops = &clk_regmap_gate_ops,           \
>>                  .parent_hws = (const struct clk_hw *[]) { _parent },    \
>>                  .num_parents = 1,      \
>>                  .flags = CLK_SET_RATE_PARENT,      \
>>          },
>>
>> #define A9_VCLK_DIV(_name, _reg, _div)       \
>>
>>      ....
>>
>> static struct clk_regmap a9_vclk_div2_en = {
>>          A9_VCLK_GATE(vclk_div2, VID_CLK_CTRL, 1, &a9_vclk.hw),
>> };
>>
>>
>> static struct clk_regmap a9_vclk_div2 = {
>>          A9_VCLK_DIV(vclk_div2, VID_CLK_CTRL, 2),
>> };
>>
>> My understanding is that you would prefer helper macros to cover only the
>> repeated initializer fields,
>> while keeping the actual clock declarations explicit.
> I do not have a definitive preference over this but I do want things to be
> consistent, at least within the driver, globaly whenever possible.
>
> Look at the other macros you have already defined in your driver and do
> the same thing, including the way you declare the variable. Apart from
> this, it seems fine.


Understood.

I'll align the new helper macros with the style already used in this driver.

>> If that's not what you had in mind, please let me know.
>>>> I can do that as well.
>>>>
> --
> Jerome

--

Jian


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic