Re: [PATCH v2 1/2] clk: Warn and add workaround on misuse of .parent_data with .name only

[Stephen Boyd <sboyd@kernel.org>]

Quoting Christian Marangi (2023-01-31 08:08:28)
> By a simple mistake in a .parent_names to .parent_data conversion it was
> found that clk core assume fw_name is always provided with a parent_data
> struct for each parent and never fallback to .name to get parent name even
> if declared.

It sounds like you have clk_parent_data and the .index member is 0? Can
you show an example structure? I'm guessing it is like this:

	struct clk_parent_data pdata = { .name = "global_name" };

> 
> This is caused by clk_core_get that only checks for parent .fw_name and
> doesn't handle .name.

clk_core_get() is not supposed to operate on the .name member. It is a
firmware based lookup with clkdev as a fallback because clkdev is a
psudeo-firmware interface to assign a name to a clk when some device
pointer is used in conjunction with it.

> 
> While it's sane to request the dev to correctly do the conversion and
> add both .fw_name and .name in a parent_data struct, it's not sane to
> silently drop parents without a warning.

I suppose we can do

	WARN(parent->index >= 0 && !parent_data[i].fw_name && parent_data[i].name, ...);

or maybe better would be to make the clk registration fail if there's a
.name field and the index is non-negative and the fw_name is NULL.

Can you grep the code and see if anyone is assigning a .name without a
.fw_name or .index?

> 
> Fix this in 2 ways. Add a kernel warning when a wrong implementation is
> used and copy .name in .fw_name in parent map populate function to
> handle clk problems and malfunctions.

We shouldn't be copying .name to .fw_name. They're different things.