mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Michael Ellerman <mpe@ellerman.id.au>
To: Akshay Adiga <akshay.adiga@linux.vnet.ibm.com>,
	linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
Cc: npiggin@gmail.com, benh@kernel.crashing.org,
	ego@linux.vnet.ibm.com, huntbag@linux.vnet.ibm.com,
	Akshay Adiga <akshay.adiga@linux.vnet.ibm.com>
Subject: Re: [RFC PATCH 0/3] New device-tree format  and Opal based idle save-restore
Date: Tue, 07 Aug 2018 22:15:37 +1000	[thread overview]
Message-ID: <87zhxybceu.fsf@concordia.ellerman.id.au> (raw)
In-Reply-To: <20180802045132.12432-1-akshay.adiga@linux.vnet.ibm.com>

Akshay Adiga <akshay.adiga@linux.vnet.ibm.com> writes:

> Previously if a older kernel runs on a newer firmware, it may enable
> all available states irrespective of its capability of handling it.
> New device tree format adds a compatible flag, so that only kernel
> which has the capability to handle the version of stop state will enable
> it.
>
> Older kernel will still see stop0 and stop0_lite in older format and we
> will depricate it after some time.
>
> 1) Idea is to bump up the version string in firmware if we find a bug or
> regression in stop states. A fix will be provided in linux which would
> now know about the bumped up version of stop states, where as kernel
> without fixes would ignore the states.
>
> 2) Slowly deprecate cpuidle/cpuhotplug threshold which is hard-coded
> into cpuidle-powernv driver. Instead use compatible strings to indicate
> if idle state is suitable for cpuidle and hotplug.
>
> New idle state device tree format :
>        power-mgt {
>             ...
>          ibm,enabled-stop-levels = <0xec000000>;
>          ibm,cpu-idle-state-psscr-mask = <0x0 0x3003ff 0x0 0x3003ff>;
>          ibm,cpu-idle-state-latencies-ns = <0x3e8 0x7d0>;
>          ibm,cpu-idle-state-psscr = <0x0 0x330 0x0 0x300330>;
>          ibm,cpu-idle-state-flags = <0x100000 0x101000>;
>          ibm,cpu-idle-state-residency-ns = <0x2710 0x4e20>;
>          ibm,idle-states {
>                      stop4 {
>                          flags = <0x207000>;
>                          compatible = "ibm,state-v1",
> 				      "cpuidle",
> 				      "opal-supported";
>                          psscr-mask = <0x0 0x3003ff>;
>                          handle = <0x102>;
>                          latency-ns = <0x186a0>;
>                          residency-ns = <0x989680>;
>                          psscr = <0x0 0x300374>;
>                   };
>                     ...
>                     stop11 {
>                      ...
>                          compatible = "ibm,state-v1",
> 				      "cpuoffline",
> 				      "opal-supported";
>                          ...
>                   };
>              };
>
> Skiboot patch-set for device-tree is posted here :
> https://patchwork.ozlabs.org/project/skiboot/list/?series=58934

I don't see a device tree binding documented anywhere?

There is an existing binding defined for ARM chips, presumably it
doesn't do everything we need. But are there good reasons why we are not
using it as a base?

See: Documentation/devicetree/bindings/arm/idle-states.txt


The way you're using compatible is not really consistent with its
traditional meaning.

eg, you have multiple states with:

                compatible = "ibm,state-v1",
                            "cpuoffline",
                            "opal-supported";


This would typically mean that all those state are all "compatible" with
some semantics defined by the name "ibm,state-v1". What you're trying to
say (I think) is that each state is "version 1" of *that state*. And
only kernels that understand version 1 should use the state.

And "cpuoffline" and "opal-supported" definitely don't belong in
compatible AFAICS, they should simply be boolean properties of the node.

cheers

  parent reply	other threads:[~2018-08-07 12:15 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-02  4:51 Akshay Adiga
2018-08-02  4:51 ` [RFC PATCH 1/3] cpuidle/powernv: Add support for states with ibm,cpuidle-state-v1 Akshay Adiga
2018-08-02  4:51 ` [RFC PATCH 2/3] powernv/cpuidle: Pass pointers instead of values to stop loop Akshay Adiga
2018-08-02  4:51 ` [RFC PATCH 3/3] cpuidle/powernv: Conditionally save-restore sprs using opal Akshay Adiga
2018-08-02 14:05   ` Nicholas Piggin
2018-08-08 15:41     ` Gautham R Shenoy
2018-08-11  5:54       ` Nicholas Piggin
2018-08-07 12:15 ` Michael Ellerman [this message]
2018-08-08  6:02   ` [RFC PATCH 0/3] New device-tree format and Opal based idle save-restore Gautham R Shenoy

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=87zhxybceu.fsf@concordia.ellerman.id.au \
    --to=mpe@ellerman.id.au \
    --cc=akshay.adiga@linux.vnet.ibm.com \
    --cc=benh@kernel.crashing.org \
    --cc=ego@linux.vnet.ibm.com \
    --cc=huntbag@linux.vnet.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=npiggin@gmail.com \
    /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

all inboxes | Powered by JetHome®