Re: [PATCH v3 2/2] clk: amlogic: Add A9 peripherals clock controller driver
Jian Hu <[email protected]>
| Newsgroups | org.infradead.lists.linux-amlogic,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-clk,org.kernel.vger.linux-devicetree,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
On 6/16/2026 3:51 PM, Jerome Brunet wrote: > [ EXTERNAL EMAIL ] > > On mar. 16 juin 2026 at 14:12, Jian Hu <[email protected]> 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 [email protected] http://lists.infradead.org/mailman/listinfo/linux-amlogic