lkml.org 
[lkml]   [2019]   [May]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] clk: fix clock global name usage.
From
Date
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?


\
 
 \ /
  Last update: 2019-05-24 17:00    [W:0.082 / U:11.928 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site