Re: [PATCH] Revert "clk: fix __clk_init_parent() for single parent clocks"

From: Stephen Boyd <sboyd@kernel.org>
To: Jerome Brunet <jbrunet@baylibre.com>,
	Michael Turquette <mturquette@baylibre.com>
Cc: linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org,
	Masahiro Yamada <yamada.masahiro@socionext.com>
Subject: Re: [PATCH] Revert "clk: fix __clk_init_parent() for single parent clocks"
Date: Fri, 21 Dec 2018 14:24:55 -0800	[thread overview]
Message-ID: <154543109549.13075.4102525082189864496@swboyd.mtv.corp.google.com> (raw)
In-Reply-To: <7e6f1ce65a59f7ad7c3d0da7f8d4b41c93346c2b.camel@baylibre.com>

Quoting Jerome Brunet (2018-12-21 08:03:29)
> On Mon, 2018-12-17 at 17:54 -0800, Stephen Boyd wrote:
> > 
> > Ok. Thanks for the explanation. What should the framework do when the
> > parent is not known? Mark the clk as not an orphan but parent it to
> > NULL? That might work here.
> 
> I don't really get the difference in the framework ATM. Is this linked to the
> 'orphan probe defer logic' WIP you have been mentionning ?

Yes, we have orphan marking logic, but we haven't finished the topic and
returned PROBE_DEFER for orphaned clks. It was blocked on some Allwinner
drivers having problems but that was over a year ago so I don't see
anything blocking us now. I really want to do it soon so now is the time
to fix all of this and see what breaks.

> 
> All this below is very interresting but it looks like a very complex way to
> solve what was initially a very simple problem, with considerations that are
> still WIP a this stage.
> 
> Even with these future constraints in mind, I don't understand how the
> proposed patch changes anything to the situtation. A clock with a get_parent()
> callback (and several known parents) may still return an out of bound value
> ... CCF will still have to deal with this case gracefully, as it is currently
> doing

Ok. I think the problem you see is that the clk provider doesn't know
anything besides parent name is a string? And so you're not sure how the
provider can return NULL for "this clk won't ever appear", vs. an error
pointer for "things failed while reading"?

I think you understand I'm not trying to change clk_ops::get_parent().
I'm sidestepping the problem and other problems that we have with the
return type of that clk_op (u8) by introducing a new clk_op that can
return an error pointer for exceptional cases. Drivers would need to
migrate to this new clk op to fix the problem you're solving, and those
drivers will also need to know what their clk parents are and what the
clk_hw pointers are for them.