Hi,

On 11/18/2014 12:21 PM, Thierry Reding wrote:

On Tue, Nov 18, 2014 at 12:01:08PM +0100, Hans de Goede wrote:

quoted

Hi,

On 11/18/2014 11:19 AM, Thierry Reding wrote:

quoted

On Thu, Nov 13, 2014 at 09:58:41AM +0100, Geert Uytterhoeven wrote:

quoted

Hi Maxime,

On Thu, Nov 13, 2014 at 9:52 AM, Maxime Ripard
[off-list ref] wrote:

quoted

quoted

-module_init(simplefb_init);
+/*
+ * While this can be a module, if builtin it's most likely the console
+ * So let's leave module_exit but move module_init to an earlier place
+ */

Not really related to this patch itself, but do we want to support
simplefb as a module? It seems like it's going to be most of the time
broken.

If it depends on clocks, it won't work as a module, as CCF will have disabled
all unused clocks at that point.

If it does depend on anything beyond clocks it won't work at all. Clocks
are special because they get set up very early at boot time. If it turns
out that a simplefb ever needs a regulator to remain on, and that's even
quite likely to happen eventually, it's going to fail miserably, because
those regulators will typically be provided by a PMIC on an I2C bus. The
regulator won't be registered until very late into the boot process and
a regulator_get() call will almost certainly cause the simplefb driver
to defer probing.

Right, this has been discussed already and the plan is to have simplefb
continue its probe function and return success from it if it encounters
any -eprobe_defer errors, while tracking which resources it misses.

And then have a late_initcall which will claim any resources which failed
with -eprobe beforehand.

How do you ensure that the late_initcall gets run before any of the
other late_initcalls that disable the resources?

Also my recollection is
that deferred probing will first be triggered the first time from a
late_initcall, so chances aren't very high that all resources have shown
up by that time.

So I just looked up the relevant code, and your right, this means that
the whole model of "disable unused resources once probing is done" which
we use for e.g. clocks, is already somewhat broken since there is
no guarantee probing is really done when the cleanup code runs.

quoted

quoted

Now deferring probing is a real showstopper for simplefb, because not
only does it make the framebuffer useless as early boot console, once
probing is attempted again the clocks that it would have needed to
acquire to keep going will already have been switched off, too.

That is not true, even with the current implementation, if all necessary
drivers are built in, then simplefb will come up later, but it will still
come up before the late_initcall which disables the clocks.

Yes, in the current implementation because clocks typically are
registered very early and thus you don't hit the deferred probe. The
same is not true for other types of resources where it's actually quite
common to hit deferred probing (regulators is a very notorious one).

It doesn't matter whether a driver is built-in or not, once you hit
deferred probing you lose.

quoted

Once we do the split probing described above (which is something which
we plan to do when it becomes necessary), then simplefb will still come
up early.

It will come up early but won't have acquired all the resources that it
needs, so unless you somehow manage to order late_initcalls in exactly
the way that you need them, the frameworks will still turn off what you
haven't managed to claim yet.

If it is a resource which only shows up as a result of deferred probing,
then it may very well not have been probed & registered yet, when the
framework cleanup functions runs, and thus will not get turned off...

So yes you're right that deferred probing may cause issues, but it seems
that this is not something simplefb specific, but rather a generic problem
with deferred-probing vs subsys cleanup functions.

My view on this is simple, lets worry about this when we actually have
a board which hits these issues, and then we'll see from there.

Regards,

Hans