Re: [PATCH v4 5/5] clk: meson: t7: add t7 clock peripherals controller driver
From: Jerome Brunet <jbrunet@baylibre.com>
Date: 2025-11-04 10:15:00
Also in:
linux-amlogic, linux-clk, linux-devicetree, lkml
On Tue 04 Nov 2025 at 17:17, Jian Hu [off-list ref] wrote:
quoted
quoted
+ +static struct clk_regmap t7_dspa = { + .data = &(struct clk_regmap_mux_data){ + .offset = DSPA_CLK_CTRL0, + .mask = 0x1, + .shift = 15, + }, + .hw.init = &(struct clk_init_data){ + .name = "dspa", + .ops = &clk_regmap_mux_ops, + .parent_hws = (const struct clk_hw *[]) { + &t7_dspa_a.hw, + &t7_dspa_b.hw, + }, + .num_parents = 2, + .flags = CLK_SET_RATE_PARENT, + }, +}; + ...... + +static struct clk_regmap t7_anakin_0 = {Nitpick: for the DSP it was a/b, here it is 0/1 Could you pick one way or the other and stick to it ?ok , I will use 0/1 for DSP.
I think I prefer a/b if you don't mind. see below for why ...
quoted
quoted
+ .data = &(struct clk_regmap_gate_data){ + .offset = ANAKIN_CLK_CTRL, + .bit_idx = 8, + }, + .hw.init = &(struct clk_init_data) { + .name = "anakin_0", + .ops = &clk_regmap_gate_ops, + .parent_hws = (const struct clk_hw *[]) { &t7_anakin_0_div.hw }, + .num_parents = 1, + .flags = CLK_SET_RATE_GATE | CLK_SET_RATE_PARENT, + }, +};
[...]
quoted
quoted
+ +static struct clk_regmap t7_anakin_clk = { + .data = &(struct clk_regmap_gate_data){ + .offset = ANAKIN_CLK_CTRL, + .bit_idx = 30, + }, + .hw.init = &(struct clk_init_data) { + .name = "anakin_clk",Again, not a great name, especially considering the one above. Is this really really how the doc refers to these 2 clocks ?bit30 gate clock is after bit31 mux clock, and the gate clock is the final output clock, it is used to gate anakin clock. I will rename bit31 as anakin_pre, rename bit30 as anakin.
Ok for the last element ... but I don't like "_pre" for a mux selecting one the 2 glitch free path. It does not help understanding the tree. For such mux, when it is not the last element, I would suggest "_ab_sel" ... at least it is clear what it does so, "anakin_ab_sel" ?
quoted
quoted
+ .ops = &clk_regmap_gate_ops, + .parent_hws = (const struct clk_hw *[]) { + &t7_anakin.hw + }, + .num_parents = 1, + .flags = CLK_SET_RATE_PARENT + }, +}; +