linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: mturquette@linaro.org (Mike Turquette)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 3/4] clk: Provide an always-on clock domain framework
Date: Wed, 25 Feb 2015 10:23:25 -0800	[thread overview]
Message-ID: <20150225182325.421.43945@quantum> (raw)
In-Reply-To: <CAL_Jsq+N4D8fUCwrYwxLaRpbhsgi5eZy5cLpsSmp8VoTQZUqow@mail.gmail.com>

Quoting Rob Herring (2015-02-25 07:24:43)
> On Mon, Feb 23, 2015 at 11:23 AM, Mike Turquette <mturquette@linaro.org> wrote:
> > Quoting Lee Jones (2015-02-18 08:15:00)
> >> 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.
> 
> [...]
> 
> >> +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.
> 
> > 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

  parent reply	other threads:[~2015-02-25 18:23 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-18 16:14 No subject Lee Jones
2015-02-18 16:14 ` [PATCH v2 1/4] ARM: sti: stih407-family: Supply defines for CLOCKGEN A0 Lee Jones
2015-02-18 16:14 ` [PATCH v2 2/4] ARM: sti: stih407-family: Provide Clock Domain information Lee Jones
2015-02-18 16:15 ` [PATCH v2 3/4] clk: Provide an always-on clock domain framework Lee Jones
2015-02-23 10:34   ` [STLinux Kernel] " Peter Griffin
2015-02-23 17:23   ` Mike Turquette
2015-02-24 11:04     ` Lee Jones
2015-02-25 15:24     ` Rob Herring
2015-02-25 15:48       ` Lee Jones
2015-02-25 18:26         ` Mike Turquette
2015-02-25 18:23       ` Mike Turquette [this message]
2015-02-18 16:15 ` [PATCH v2 4/4] clk: dt: Introduce always-on clock domain documentation Lee Jones
2015-02-18 16:54   ` Rob Herring
2015-02-18 17:12     ` Lee Jones
2015-02-18 18:50       ` Rob Herring
2015-02-18 21:54         ` Lee Jones
2015-02-18 23:45           ` Rob Herring
2015-02-19 10:05             ` Lee Jones
2015-02-19  9:27           ` Geert Uytterhoeven
2015-02-19  9:42             ` Lee Jones
2015-02-19  9:55               ` Geert Uytterhoeven
2015-02-19 10:11                 ` Lee Jones
2015-02-19 10:18                   ` Geert Uytterhoeven
2015-02-19 10:28                     ` Lee Jones
2015-02-19 10:35                       ` Geert Uytterhoeven
2015-02-19 10:43                         ` Lee Jones
2015-02-19 11:01                           ` Geert Uytterhoeven
2015-02-19 11:13                             ` Lee Jones

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150225182325.421.43945@quantum \
    --to=mturquette@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).