On Mon, Sep 29, 2014 at 11:10:53AM +0200, Geert Uytterhoeven wrote:

Hi Thierry,

On Mon, Sep 29, 2014 at 10:54 AM, Thierry Reding
[off-list ref] wrote:

quoted

On Mon, Sep 29, 2014 at 10:27:41AM +0200, Geert Uytterhoeven wrote:

quoted

(CC linux-pm, as PM is the real reason behind disabling unused clocks)
(CC gregkh and lkml, for driver core)

On Mon, Sep 29, 2014 at 10:06 AM, Thierry Reding
[off-list ref] wrote:

quoted

If we start extending the binding with board-level details we end up
duplicating the device tree node for the proper video device. Also note
that it won't stop at clocks. Other setups will require regulators to be
listed in this device tree node as well so that they don't get disabled
at late_initcall. And the regulator bindings don't provide a method to
list an arbitrary number of clocks in a single property in the way that
the clocks property works.

Then (optional) regulator support needs to be added.

Can you elaborate?

I'm not so familiar with regulators, but I guess it's similar to clocks?

The bindings are different. Essentially what you use is a *-supply
property per regulator. There is no way to specify more than one
regulator in a single property.

So if you want to keep that generic you have to do crazy things like:

	simplefb {
		enable-0-supply = <&reg1>;
		enable-1-supply = <&reg2>;
		...
	};

I suppose a more generic supplies property could be created to support
this use-case, but I think this kind of proves my point. The only way
that the original proposal is going to work for other resources is if
they follow the same kind of binding. I don't think it makes sense to
introduce such a prerequisite merely because it would make life easy
for some exotic driver with a very specific application.

quoted

And then all of a sudden something that was supposed to be simple and
generic needs to know the specifics of some hardware device.

And suddenly we wish we could write a real driver and put the stuff in
the DTS, not DTB...

Oh, there's no doubt a real driver would be preferrable. Note that
simplefb is only meant to be a shim to pass a framebuffer from firmware
to kernel. In some cases it can be used with longer lifetime, like for
example if you really want to have graphical output but the real driver
isn't there yet.

Being a shim driver is precisely the reason why I think the binding
shouldn't be extended to cover all possible types of resources. That
should all go into the binding for the real device.

quoted

quoted

quoted

The only reasonable thing for simplefb to do is not deal with any kind
of resource at all (except perhaps area that contains the framebuffer
memory).

So how about instead of requiring resources to be explicitly claimed we
introduce something like the below patch? The intention being to give
"firmware device" drivers a way of signalling to the clock framework
that they need rely on clocks set up by firmware and when they no longer
need them. This implements essentially what Mark (CC'ing again on this
subthread) suggested earlier in this thread. Basically, it will allow
drivers to determine the time when unused clocks are really unused. It
will of course only work when used correctly by drivers. For the case of
simplefb I'd expect its .probe() implementation to call the new
clk_ignore_unused() function and once it has handed over control of the
display hardware to the real driver it can call clk_unignore_unused() to
signal that all unused clocks that it cares about have now been claimed.
This is "reference counted" and can therefore be used by more than a
single driver if necessary. Similar functionality could be added for
other resource subsystems as needed.

This still won't work for modules, right? Or am I missing something?
With modules you will never know in advance what will be used and what
won't be used, so you need to keep all clocks, regulators, PM domains, ...
up and running?

No. The way this works is that your firmware shim driver, simplefb in
this case, will call clk_ignore_unused() to tell the clock framework
that it uses clocks set up by the firmware, and therefore requests that
no clocks should be considered unused (for now). Later on when the
proper driver has successfully taken over from the shim driver, the shim
driver can unregister itself and call clk_unignore_unused(), which will
drop its "reference" on the unused clocks. When all references have been
dropped the clock framework will then disable all remaining unused
clocks.

So the shim must be built-in, not modular.

Correct. Making it a module isn't very useful in my opinion. You'd loose
all the advantages.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140929/08c17aac/attachment.sig>