Messages in this thread | | | Subject | Re: [PATCH] clk: fix clock global name usage. | From | Jerome Brunet <> | Date | Fri, 24 May 2019 17:00:08 +0200 |
| |
On Fri, 2019-05-24 at 07:33 -0700, Stephen Boyd wrote: > Do you set the index to 0 in this clk's parent_data? We purposefully set > the index to -1 in clk_core_populate_parent_map() so that the fw_name > can be NULL but the index can be something >= 0 and then we'll use that > to lookup the clk from DT. We need to support that combination. > > fw_name | index | DT lookup? > ----------+---------+------------ > NULL | >= 0 | Y > NULL | -1 | N > non-NULL | -1 | ? > non-NULL | >= 0 | Y > > Maybe we should support the ? case, because right now it will fail to do > the DT lookup when the index is -1.
Hi Stephen,
We are trying to migrate all meson clocks to the new parent structure. There is a little quirk which forces us to continue to use legacy names for a couple of clocks.
We heavily use static data which init everything to 0. Here is an example:
static struct clk_regmap g12a_aoclk_cts_rtc_oscin = { [...] .hw.init = &(struct clk_init_data){ .name = "g12a_ao_cts_rtc_oscin", .ops = &clk_regmap_mux_ops, - .parent_names = (const char *[]){ "g12a_ao_32k_by_oscin", - IN_PREFIX "ext_32k-0" }, + .parent_data = (const struct clk_parent_data []) { + { .name = "g12a_ao_32k_by_oscin" }, + { .fw_name = "ext-32k-0", }, + }, .num_parents = 2, .flags = CLK_SET_RATE_PARENT, }, };
With this, instead of taking name = "g12a_ao_32k_by_oscin" for entry #0 it takes DT names at index 0 which is not what we intended.
If I understand correctly we should put + { .name = "g12a_ao_32k_by_oscin", index = -1, },
And would be alright ?
While I understand it, it is not very obvious or nice to look at. Plus it is a bit weird that this -1 is required for .name and not .hw.
Do you think we could come up with a priority order which makes the first example work ?
Something like:
if (hw) { /* use pointer */ } else if (name) { /* use legacy global names */ } else if (fw_name) { /* use DT names */ } else if (index >= 0) /* use DT index */ } else { return -EINVAL; }
The last 2 clause could be removed if we make index an unsigned.
Cheers Jerome
> > So this patch instead?
| |