devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Lee Jones <lee.jones@linaro.org>
To: Maxime Coquelin <maxime.coquelin@st.com>
Cc: Jassi Brar <jassisinghbrar@gmail.com>,
	Devicetree List <devicetree@vger.kernel.org>,
	Mike Turquette <mturquette@linaro.org>,
	kernel@stlinux.com, Stephen Boyd <sboyd@codeaurora.org>,
	lkml <linux-kernel@vger.kernel.org>,
	Jassi Brar <jaswinder.singh@linaro.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [STLinux Kernel] [PATCH 3/4] clk: Provide always-on clock support
Date: Mon, 2 Mar 2015 10:32:28 +0000	[thread overview]
Message-ID: <20150302103228.GD32347@x1> (raw)
In-Reply-To: <54F43A98.509@st.com>

On Mon, 02 Mar 2015, Maxime Coquelin wrote:
> On 03/02/2015 11:18 AM, Lee Jones wrote:
> >On Mon, 02 Mar 2015, Jassi Brar wrote:
> >
> >>On Mon, Mar 2, 2015 at 2:06 PM, Lee Jones <lee.jones@linaro.org> wrote:
> >>>On Sat, 28 Feb 2015, Jassi Brar wrote:
> >>>
> >>>>On 28 February 2015 at 02:44, Lee Jones <lee.jones@linaro.org> wrote:
> >>>>>Lots of platforms contain clocks which if turned off would prove fatal.
> >>>>>The only way to recover from these catastrophic failures is to restart
> >>>>>the board(s).  Now, when a clock is registered with the framework it is
> >>>>>compared against a list of provided always-on clock names which must be
> >>>>>kept ungated.  If it matches, we enable the existing CLK_IGNORE_UNUSED
> >>>>>flag, which will prevent the common clk framework from attempting to
> >>>>>gate it during the clk_disable_unused() procedure.
> >>>>>
> >>>>If a clock is critical on a certain board, it could be got+enabled
> >>>>during early boot so there is always a user.
> >>>I tried this.  There was push-back from the DT maintainers.
> >>>
> >>>   http://lists.infradead.org/pipermail/linux-arm-kernel/2015-February/324417.html
> >>>
> >>Thanks, I wasn't aware of the history.
> >>
> >>>>To be able to do that from DT, maybe add a new, say, CLK_ALWAYS_ON
> >>>>flag could be made to initialize the clock with one phantom user
> >>>>already. Or just reuse the CLK_IGNORE_UNUSED?
> >>>How is that different to what this set is doing?
> >>>
> >>The phantom user - that's there but none can see it.
> >>
> >>How about?
> >>
> >>+       of_property_for_each_string(np, "clock-always-on", prop, clkname) {
> >>+               clk = __clk_lookup(clkname);
> >>+               if (!clk)
> >>+                       continue;
> >>+
> >>+               clk->core->enable_count = 1;
> >>+               clk->core->prepare_count = 1;
> >>+       }
> >This is only fractionally different from the current implementation.
> >
> >I believe the current way it slightly nicer, as we don't have to fake
> >the user count.  This solution is saying "one of the drivers is still
> >consuming this clock", instead, in the original implementation we're
> >saying "we know there are no consumers of this clock, but keep it on
> >anyway due to [insert reason here]".
> >
> So maybe introducing a new "CLK_DISABLE_NEVER" flag will be more
> explicit than hacking around "CLK_IGNORE_UNUSED" one?

I am okay with that if Mike is; however, personally I think
CLK_IGNORE_UNUSED is pretty descriptive.  If clocks are unused
(reference count reaches zero) the framework will disable the clock --
fair enough.  So in order to prevent that from happening in cases
where these clocks are used by entities other than device drivers
(perhaps these devices are even invisible to Linux) we have to tell
the framework to "ignore the fact that this clock _appears_ to be
unused".  Hence CLK_IGNORE_UNUSED.

I'm struggling to think of a solid reason for inventing new flags when
we have one that is perfectly suitable/descriptive.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

  reply	other threads:[~2015-03-02 10:32 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-27 21:14 [PATCH 0/4] clk: Provide support for always-on clocks Lee Jones
2015-02-27 21:14 ` [PATCH 1/4] ARM: sti: stih407-family: Supply defines for CLOCKGEN A0 Lee Jones
2015-02-27 21:14 ` [PATCH 2/4] ARM: sti: stih410-clocks: Identify critical clocks as always-on Lee Jones
     [not found]   ` <1425071674-16995-3-git-send-email-lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2015-04-02  8:00     ` [STLinux Kernel] " Peter Griffin
2015-02-27 21:14 ` [PATCH 3/4] clk: Provide always-on clock support Lee Jones
2015-02-27 21:51   ` Lee Jones
     [not found]   ` <1425071674-16995-4-git-send-email-lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2015-02-28  7:52     ` [STLinux Kernel] " Maxime Coquelin
     [not found]       ` <54F173B7.9030305-qxv4g6HH51o@public.gmane.org>
2015-03-02  8:16         ` Lee Jones
2015-04-01  1:13           ` Michael Turquette
2015-02-28  9:21     ` Jassi Brar
     [not found]       ` <CAJe_Zhf=GBb8AfnTLaGxGcZkVSm9TtdnTsmpH_MBmbMT8ViMDg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-03-02  8:36         ` Lee Jones
2015-03-02 10:08           ` Jassi Brar
2015-03-02 10:18             ` Lee Jones
2015-03-02 10:25               ` [STLinux Kernel] " Maxime Coquelin
2015-03-02 10:32                 ` Lee Jones [this message]
2015-03-02 10:28               ` Jassi Brar
2015-03-02 10:40                 ` Lee Jones
     [not found]                 ` <CAJe_Zhc-94afeWegEeOoL3X13y35MDJvqME1x7XMy3vVVXYedg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-04-01  1:42                   ` Michael Turquette
2015-04-02  4:39                     ` Jassi Brar
2015-04-02  7:10                       ` Lee Jones
2015-02-27 21:14 ` [PATCH 4/4] clk: dt: Introduce binding for " Lee Jones
     [not found] ` <1425071674-16995-1-git-send-email-lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2015-04-02  8:12   ` [STLinux Kernel] [PATCH 0/4] clk: Provide support for always-on clocks Peter Griffin
2015-04-02  9:45     ` Gabriel Fernandez
2015-04-02 10:53       ` Lee Jones
2015-04-02 10:58       ` Lee Jones
2015-04-02 10:52     ` 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=20150302103228.GD32347@x1 \
    --to=lee.jones@linaro.org \
    --cc=devicetree@vger.kernel.org \
    --cc=jassisinghbrar@gmail.com \
    --cc=jaswinder.singh@linaro.org \
    --cc=kernel@stlinux.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maxime.coquelin@st.com \
    --cc=mturquette@linaro.org \
    --cc=sboyd@codeaurora.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).