Thread (12 messages) 12 messages, 5 authors, 2025-03-19

Re: [PATCH v12 4/6] clk: qcom: Add NSS clock Controller driver for IPQ9574

From: Marek Behún <kabel@kernel.org>
Date: 2025-03-19 09:10:35
Also in: linux-arm-kernel, linux-arm-msm, linux-clk, linux-devicetree, lkml

On Tue, Mar 18, 2025 at 03:50:31PM -0700, Stephen Boyd wrote:
Quoting Marek Behún (2025-03-17 07:08:16)
quoted
On Thu, Mar 13, 2025 at 04:33:57PM +0530, Manikanta Mylavarapu wrote:
quoted
+static struct clk_rcg2 nss_cc_clc_clk_src = {
+     .cmd_rcgr = 0x28604,
+     .mnd_width = 0,
+     .hid_width = 5,
+     .parent_map = nss_cc_parent_map_6,
+     .freq_tbl = ftbl_nss_cc_clc_clk_src,
+     .clkr.hw.init = &(const struct clk_init_data) {
+             .name = "nss_cc_clc_clk_src",
+             .parent_data = nss_cc_parent_data_6,
+             .num_parents = ARRAY_SIZE(nss_cc_parent_data_6),
+             .ops = &clk_rcg2_ops,
+     },
+};
This structure definition gets repeated many times in this driver,
with only slight changes. (This also happens in other qualcomm clock
drivers.)

Would it be possible to refactor it into a macro, to avoid the
insane code repetition?
We have this discussion every couple years or so. The short answer is
no. The long answer is that it makes it harder to read because we don't
know what argument to the macro corresponds to the struct members.

It could probably use the CLK_HW_INIT_PARENTS_DATA macro though.

static struct clk_rcg2 nss_cc_clc_clk_src = {
     .cmd_rcgr = 0x28604,
     .mnd_width = 0,
     .hid_width = 5,
     .parent_map = nss_cc_parent_map_6,
     .freq_tbl = ftbl_nss_cc_clc_clk_src,
     .clkr.hw.init = CLK_HW_INIT_PARENTS_DATA("nss_cc_clc_clk_src",
                                              nss_cc_parent_data_6,
					      &clk_rcg2_ops, 0),
     },
};

but then we lose the const. Oh well.

The whole qcom clk driver probably needs an overhaul to just have
descriptors that populate a bunch of clks that are allocated at probe
time so that the memory footprint is smaller if you have multiple clk
drivers loaded and so that we can probe the driver again without
unloading the whole kernel module.
Okay then, in that case just ignore me :)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help