linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: geert@linux-m68k.org (Geert Uytterhoeven)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 4/4] clk: dt: Introduce always-on clock domain documentation
Date: Thu, 19 Feb 2015 12:01:13 +0100	[thread overview]
Message-ID: <CAMuHMdXY5+Sm0LUsbrPraUJSNS7PsuRHaaLY--DBVt3adZ-+Cw@mail.gmail.com> (raw)
In-Reply-To: <20150219104332.GF12212@x1>

Hi Lee,

On Thu, Feb 19, 2015 at 11:43 AM, Lee Jones <lee.jones@linaro.org> wrote:
> On Thu, 19 Feb 2015, Geert Uytterhoeven wrote:
>> On Thu, Feb 19, 2015 at 11:28 AM, Lee Jones <lee.jones@linaro.org> wrote:
>> > On Thu, 19 Feb 2015, Geert Uytterhoeven wrote:
>> >> On Thu, Feb 19, 2015 at 11:11 AM, Lee Jones <lee.jones@linaro.org> wrote:
>> >> > On Thu, 19 Feb 2015, Geert Uytterhoeven wrote:
>> >> >> On Thu, Feb 19, 2015 at 10:42 AM, Lee Jones <lee.jones@linaro.org> wrote:
>> >> >> >> What kind of clocks are these? What do they control?
>> >> >> >> Memory controllers? Bus controllers?
>> >> >> >>
>> >> >> >> They must control some device(s), so there should be one or more device
>> >> >> >> nodes in DT that reference these clocks.
>> >> >> >> As soon as that information is in DT, support can be added to Linux to
>> >> >> >> make sure the "critical" clocks stay enabled, either through a real driver,
>> >> >> >> or through platform code.
>> >> >> >
>> >> >> > Some do, some don't.  For instance, we have one clock which controls
>> >> >> > SPI and I2C that must not be turned off.  We discovered this then when
>> >> >> > a suspend was attempted and the board refused to resume.  This clock
>> >> >> > also runs one of the critical interconnects that runs from the a9.  It
>> >> >> > would be wrong to remove the clk_disable() attempt from the SPI/I2C
>> >> >> > drivers because the same IP on another board might be controlled by a
>> >> >> > different clock which is able to be gated.
>> >> >> >
>> >> >> > There are also clocks which control other interconnects that are not
>> >> >> > connected to any device drivers.  If we fail to take references for
>> >> >> > them before clk_disable_unused() is called, again the board hangs.  We
>> >> >> > even lose JTAG support.
>> >> >>
>> >> >> Interconnects are buses. Can't you represent those buses in the DT
>> >> >> hierarchy, and give them clocks properties?
>> >> >
>> >> > So instead of this nice succinct, simple, cover all bases
>> >> > (interconnects was just an example, there are bound to be others),
>> >> > generic framework, you are suggesting to write drivers for devices
>> >> > which other than "don't turn my clocks off", Linux can't actually see
>> >> > or control?
>> >>
>> >> DT describes the hardware, not behavior.
>> >
>> > Okay so ...
>> >
>> > /*
>> >  * ICNs are not visible/controllable in Linux, but references to their
>> >  * clocks must be obtained and retained or the platform will become
>> >  * irrecoverably unresponsive.
>> >  */
>> > interconnects at 0 {
>> >        compatible = "always-on-clk-domain";
>>
>> st,...flexgen...
>>
>> >        clocks = <&clk_s_c0_flexgen CLK_ICN_SBC>,
>> >                 <&clk_s_c0_flexgen CLK_ICN_LMI>,
>> >                 <&clk_s_c0_flexgen CLK_ICN_CPU>,
>> >                 <&clk_s_c0_flexgen CLK_TX_ICN_DMU>;
>> > };
>>
>> And then you can have platform code that binds against st,...flexgen...,
>> and enables all referenced clocks.
>
> Flexgen isn't a device, it's a clk source.  a) writing a device driver

Sorry, I'm not familiar with ST nomenclature.
So that should become the name of the interconnect/bus.

> for a clk source seems wrong b) what if on another platform a
> different clock source supplied the clock?  Write another driver?  And
> what if the ICNs are connected to different clock sources?  More
> drivers?  c) all of these drivers will only do one thing -- pull a
> reference and keep hold of it.  You want 50 drivers (across all
> platforms) doing only that?  Or, more sanely, do you want this one
> generic framework driver doing that?
>
>> Alternatively, if you have power domains, you can add a reference to
>> the power domain, and let the power domain driver handle it.
>
> I'm not sure what a power domain driver will do?  We need a driver to
> _not_ give up references, that is all. :)

A power domain driver can do anything it wants.
That includes enabling your interconnect clocks, and keeping them enabled.

Now, if flexgen is the name of the clock source, then all devices connected
to flexgen are part of the flexgen clock domain, which can be represented
in Linux using the generic PM domain. Do all devices connected to flexgen
need to be handled similarly w.r.t. clocks? If yes, the genpd may be the place
to implement that.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

  reply	other threads:[~2015-02-19 11:01 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
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 [this message]
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=CAMuHMdXY5+Sm0LUsbrPraUJSNS7PsuRHaaLY--DBVt3adZ-+Cw@mail.gmail.com \
    --to=geert@linux-m68k.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).