Re: [PATCH v2 3/4] clk: Provide an always-on clock domain framework

Quoting Rob Herring (2015-02-25 07:24:43)

On Mon, Feb 23, 2015 at 11:23 AM, Mike Turquette [off-list ref] wrote:

quoted

Quoting Lee Jones (2015-02-18 08:15:00)

quoted

Much h/w contain clocks which if turned off would prove fatal.  The
only way to recover is to restart the board(s).  This driver takes
references to clocks which are required to be always-on in order to
prevent the common clk framework from trying to turn them off during
the clk_disabled_unused() procedure.

[...]

quoted

quoted

+static int ao_clock_domain_probe(struct platform_device *pdev)
+{
+       struct device_node *np = pdev->dev.of_node;
+       int nclks, i;
+
+       nclks = of_count_phandle_with_args(np, "clocks", "#clock-cells");

Minor nitpick: please use of_clk_get_parent_count. I spent a solid 5
minutes writing that function and I need people to use it so I can get a
return on my investment.

Otherwise the patch looks good. I believe that this method is targeting
always-on clock in a production environment, which is different from the
CLK_IGNORE_UNUSED stuff which typically is helpful while bringing up new
hardware or dealing with a platform that has incomplete driver support.

There is also the usecase of keep clocks on until I load a module that
properly handles my hardware (e.g simplefb). We have a simplefb node
with clocks and the simplefb driver jumps thru some hoops to hand-off
clocks to the real driver. I don't really like it and don't want to
see more examples. And there is the case of I thought I would never
manage this clock, but kernel subsystems evolve and now I want to
manage a clock. This should not require a DT update to do so.

Regarding the latter case, this is a violation of the intent of
always-on clocks. I think a firmly worded sentence in the binding should
suffice.

Neither of these may be Lee's usecase, but I want to see them covered
by the binding.

quoted

I wonder if there is a clever way for existing clock providers
(expressed in DT) to use this without having to create a separate node
of clocks with the "always-on-clk-domain" flag. Possibly the common
clock binding could declare some always-on flag that is standardized?
Then the framework core could use this code easily. Not sure if that is
a good idea though...

I would prefer to see the always on clocks just listed within the
clock controller's node rather than creating made up nodes with clock
properties. This should be always-on until claimed IMO, but that
aspect is the OS's problem, not a DT problem.

So the common clock binding could have a new always-on list added to it,
but the clock can still be claimed and gated by drivers? I'll think on
it a bit but always-on is not the right name in that case. It is a more
general solution however (since it could still cover the sub-case of
clocks always remaining on since a driver never claims them).

Regards,
Mike

Rob