All of lore.kernel.org
 help / color / mirror / Atom feed
From: andre.przywara@arm.com (Andre Przywara)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 3/3] clk: Provide OF helper to mark clocks as CRITICAL
Date: Tue, 2 Feb 2016 13:39:58 +0000	[thread overview]
Message-ID: <56B0B1AE.2060502@arm.com> (raw)
In-Reply-To: <20160201063256.GE32462@lukather>

Hi Maxime,

On 01/02/16 06:32, Maxime Ripard wrote:
> Hi Andre,
> 
> On Wed, Jan 27, 2016 at 11:51:45PM +0000, Andr? Przywara wrote:
>> Hi,
>>
>> On 18/01/16 14:28, Lee Jones wrote:
>>> This call matches clocks which have been marked as critical in DT
>>> and sets the appropriate flag.  These flags can then be used to
>>> mark the clock core flags appropriately prior to registration.
>>
>> I like the idea of having a generic property very much. Also this solves
>> a problem I have in a very elegant way.
> 
> Not really. It has a significant set of drawbacks that we already
> detailed in the initial thread, which are mostly related to the fact
> that the clocks are to be left on is something that totally depends on
> the software support in the kernel. Some clocks should be reported as
> critical because they are simply missing a driver for it, some should
> be because the driver for it as not been compiled, some should because
> we don't have the proper clocks drivers yet for one of their
> downstream clocks.
> 
> Basically, it all boils down to this: some clocks should never ever be
> shutdown because <hardware reason>, and I believe it's the case Lee is
> in. But most of the current code that would use it might, and might
> even need at some point to shut down such a clock.

I was bascically interested in pushing the critical-clock property into
DT to solve that cumbersome clk-sunxi init scheme - which you have fixed
now in a much better way (thanks for that, btw.)
For that particular case the CPU clock really looks like being actually
critical in the hardware sense - no-one maybe except the mgmt core
should turn the one single CPU clock source off.

So I wonder if we should document this "for hardware reasons only" and
still have that property in DT?
At the weekend I coded something into the generic DT clock code to let
it parse for basically every clock node - without a particular driver
needing to ask for it.
If this sounds useful to you I can post that one.

Cheers,
Andre.
> 
> Mike's solution with the flags + handover was solving all this, I'm
> not sure why he's not pushed it forward.
> 
> Maxime
> 

WARNING: multiple messages have this Message-ID (diff)
From: Andre Przywara <andre.przywara@arm.com>
To: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: Lee Jones <lee.jones@linaro.org>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, kernel@stlinux.com,
	s.hauer@pengutronix.de, sboyd@codeaurora.org,
	geert@linux-m68k.org, mturquette@baylibre.com,
	maxime.coquelin@st.com
Subject: Re: [PATCH 3/3] clk: Provide OF helper to mark clocks as CRITICAL
Date: Tue, 2 Feb 2016 13:39:58 +0000	[thread overview]
Message-ID: <56B0B1AE.2060502@arm.com> (raw)
In-Reply-To: <20160201063256.GE32462@lukather>

Hi Maxime,

On 01/02/16 06:32, Maxime Ripard wrote:
> Hi Andre,
> 
> On Wed, Jan 27, 2016 at 11:51:45PM +0000, André Przywara wrote:
>> Hi,
>>
>> On 18/01/16 14:28, Lee Jones wrote:
>>> This call matches clocks which have been marked as critical in DT
>>> and sets the appropriate flag.  These flags can then be used to
>>> mark the clock core flags appropriately prior to registration.
>>
>> I like the idea of having a generic property very much. Also this solves
>> a problem I have in a very elegant way.
> 
> Not really. It has a significant set of drawbacks that we already
> detailed in the initial thread, which are mostly related to the fact
> that the clocks are to be left on is something that totally depends on
> the software support in the kernel. Some clocks should be reported as
> critical because they are simply missing a driver for it, some should
> be because the driver for it as not been compiled, some should because
> we don't have the proper clocks drivers yet for one of their
> downstream clocks.
> 
> Basically, it all boils down to this: some clocks should never ever be
> shutdown because <hardware reason>, and I believe it's the case Lee is
> in. But most of the current code that would use it might, and might
> even need at some point to shut down such a clock.

I was bascically interested in pushing the critical-clock property into
DT to solve that cumbersome clk-sunxi init scheme - which you have fixed
now in a much better way (thanks for that, btw.)
For that particular case the CPU clock really looks like being actually
critical in the hardware sense - no-one maybe except the mgmt core
should turn the one single CPU clock source off.

So I wonder if we should document this "for hardware reasons only" and
still have that property in DT?
At the weekend I coded something into the generic DT clock code to let
it parse for basically every clock node - without a particular driver
needing to ask for it.
If this sounds useful to you I can post that one.

Cheers,
Andre.
> 
> Mike's solution with the flags + handover was solving all this, I'm
> not sure why he's not pushed it forward.
> 
> Maxime
> 

  parent reply	other threads:[~2016-02-02 13:39 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-18 14:28 [PATCH 0/3] clk: Add support for critical clocks Lee Jones
2016-01-18 14:28 ` Lee Jones
2016-01-18 14:28 ` [PATCH 1/3] clk: Allow clocks to be marked as CRITICAL Lee Jones
2016-01-18 14:28   ` Lee Jones
2016-01-18 17:15   ` Geert Uytterhoeven
2016-01-18 17:15     ` Geert Uytterhoeven
2016-01-19  7:53     ` Lee Jones
2016-01-19  7:53       ` Lee Jones
2016-02-11  0:41   ` Michael Turquette
2016-02-11  0:41     ` Michael Turquette
2016-01-18 14:28 ` [PATCH 2/3] clk: WARN_ON about to disable a critical clock Lee Jones
2016-01-18 14:28   ` Lee Jones
2016-02-11  0:43   ` Michael Turquette
2016-02-11  0:43     ` Michael Turquette
2017-06-27 11:16     ` Dirk Behme
2017-06-27 11:16       ` Dirk Behme
2017-07-03 11:53       ` Lee Jones
2017-07-03 11:53         ` Lee Jones
2017-07-03 12:01         ` Dirk Behme
2017-07-03 12:01           ` Dirk Behme
2017-07-03 14:25           ` Lee Jones
2017-07-03 14:25             ` Lee Jones
2017-07-03 15:24             ` Dirk Behme
2017-07-03 15:24               ` Dirk Behme
2017-07-03 16:06               ` Lee Jones
2017-07-03 16:06                 ` Lee Jones
2016-01-18 14:28 ` [PATCH 3/3] clk: Provide OF helper to mark clocks as CRITICAL Lee Jones
2016-01-18 14:28   ` Lee Jones
2016-01-27 23:51   ` André Przywara
2016-01-27 23:51     ` André Przywara
2016-02-01  6:32     ` Maxime Ripard
2016-02-01  6:32       ` Maxime Ripard
2016-02-01  8:22       ` Lee Jones
2016-02-01  8:22         ` Lee Jones
2016-02-11  0:38         ` Michael Turquette
2016-02-11  0:38           ` Michael Turquette
2016-02-02 13:39       ` Andre Przywara [this message]
2016-02-02 13:39         ` Andre Przywara
2016-02-02 15:02         ` Lee Jones
2016-02-02 15:02           ` Lee Jones
2016-02-11  0:48   ` Michael Turquette
2016-02-11  0:48     ` Michael Turquette
2016-02-11  1:00 ` [PATCH 0/3] clk: Add support for critical clocks Michael Turquette
2016-02-11  1:00   ` Michael Turquette

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=56B0B1AE.2060502@arm.com \
    --to=andre.przywara@arm.com \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.