Re: CLK_OF_DECLARE advice required

[Stephen Boyd]

On 06/06, Geert Uytterhoeven wrote:
> Hi Stephen,
> 
> On Mon, Jun 5, 2017 at 10:13 PM, Stephen Boyd <sboyd@xxxxxxxxxxxxxx> wrote:
> > On 06/05, Phil Elwell wrote:
> >> That sounds great, but it doesn't match my experience. Let me restate my
> >> observations with a bit more detail.
> >>
> >> In this scenario there three devices in a dependency chain:
> >>
> >>   clock -> fixed-factor->clock -> uart.
> >>
> >> The Fixed Factor Clock is declared with OF_CLK_DECLARE, while the two platform
> >> drivers use normal probe functions.
> >>
> >> 1) of_clk_init() calls encounter FFC in the list of clocks to initialise and
> >> calls parent_ready on the device node.
> >>
> >> 2) The parent clock has not been initialised, so of_clk_get returns
> >> -EPROBE_DEFER.
> >>
> >> 3) Steps 1 and 2 repeat until no progress is made, at which point the force
> >> flag is set for one last iteration. This time the parent_ready check is skipped
> >> and the code calls indirectly into _of_fixed_factor_clk_setup().
> >>
> >> 4) The FFC setup calls of_clk_get_parent_name, which returns a NULL that ends
> >> up referred to by the parent_names field of clk_init_data structure indirectly
> >> passed to clk_hw_register and clk_register.
> >
> > That's bad. Does "clock" in this scenario have a
> > clock-output-names property so we can find the name of the parent
> > of the fixed factor clock? That way we can describe the fixed
> > factor to "clock" linkage. Without that, things won't ever work.
> 
> >> Is this behaviour as intended? I can see that the NULL parent name in steps 4
> >> and 5 could be handled more gracefully, but the end result would be the same.
> >>
> >> Where and how is the "orphan" clock concept supposed to help, and what needs to
> >> be fixed in this case?
> >>
> >
> > The orphan concept helps here because of_clk_init() eventually
> > forces the registration of the fixed factor clock even though the
> > fixed factor's parent has not been registered yet. As you've
> > determined though, that isn't working properly because the fixed
> > factor code is failing to get a name for the parent. Using the
> > clock-output-names property would fix that though.
> 
> Isn't clock-output-names deprecated for clocks with a single clock
> output?
> 

Yes. I'd prefer we don't have any clock-output-names in dts.  In
this case, it's pretty much required though, until we get to a
point where we can describe parent child relationships through
another means besides strings.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project