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

[Stephen Boyd]

Quoting Marek Behún (2025-03-17 07:08:16)
> On Thu, Mar 13, 2025 at 04:33:57PM +0530, Manikanta Mylavarapu wrote:
> 
> > +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.