From: Lukasz Majewski <l.majewski@samsung.com>
To: Nishanth Menon <nm@ti.com>,
Viresh Kumar <viresh.kumar@linaro.org>,
Thomas Abraham <ta.omasab@gmail.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>,
Rob Herring <rob.herring@linaro.org>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
linux-samsung-soc <linux-samsung-soc@vger.kernel.org>,
"linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
Tomasz Figa <t.figa@samsung.com>,
Kukjin Kim <kgene.kim@samsung.com>,
Thomas P Abraham <thomas.ab@samsung.com>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v3 1/2] cpufreq / OPP: Allow boost frequency to be looked up from device tree
Date: Wed, 14 May 2014 08:09:30 +0200 [thread overview]
Message-ID: <20140514080930.781e0557@amdc2363> (raw)
In-Reply-To: <CAGo_u6p-uqw82ewDciDEhAt0=Nswfw33-gf8Aa+-6_k3eCcTzw@mail.gmail.com>
Hi Nishanth, Viresh
If I may add my 2 cents.
> On Tue, May 13, 2014 at 10:40 PM, Viresh Kumar
> <viresh.kumar@linaro.org> wrote:
> > On 14 May 2014 06:32, Thomas Abraham <ta.omasab@gmail.com> wrote:
> [...]
> >> +#ifdef CONFIG_CPU_FREQ_BOOST_SW
> >> + if (of_find_property(dev->of_node, "boost-frequency",
> >> &len)) {
> >
> > Does this mean another block inside the cpu node ? Like this: ?
> >
> > cpu@0 {
> > compatible = "arm,cortex-a9";
> > reg = <0>;
> > next-level-cache = <&L2>;
> > operating-points = <
> > /* kHz uV */
> > 792000 1100000
> > 396000 950000
> > 198000 850000
> > >;
> > boost-frequency = <
> > 792000
> > 198000
> > >;
I think that the above approach is more appealing [*].
> > };
> >
> > I think it we might better add another field in the opp block as
> > these OPPs are rather boost one..
>
> If we change the meaning to be:
> operating-points = <
> /* kHz uV boost? */
> 792000 1100000 1
> 396000 950000 0
>
> We are adding a modifier to the OPP definition here and does imply
> every platform which may or maynot require "boost" need to indicate
> that - basically fails at least my "least common" description for a
> generic definition. As I had indicated in other threads -> we are back
> to the discussion of "additional properties" to an OPP..
> Option 1:
> - describe modifiers separately (as in boost-frequencies) - that
> operate based on data from opp table.
> Option 2:
> - keep adding to the description of opp as time goes by - every SoC
> has it's own set of quirks that are needed for an OPP - I know that at
> TI, we have certain OPPs that can only be enabled *if* "custom
> hardware driver" is enabled, and similar stories. - yet another
> example is enable the OPP if efuse X, bit Y is set.
>
> Personally, looking at the various descriptions and bL, cpu-idle
> states dependencies on OPPs, etc.. Option 2 is a non-scalable
> approach.
I agree with Nishanth here, that point 1 (as described by Viresh at
[*]) is a more scalable approach.
>
> [...]
--
Best regards,
Lukasz Majewski
Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
WARNING: multiple messages have this Message-ID (diff)
From: l.majewski@samsung.com (Lukasz Majewski)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v3 1/2] cpufreq / OPP: Allow boost frequency to be looked up from device tree
Date: Wed, 14 May 2014 08:09:30 +0200 [thread overview]
Message-ID: <20140514080930.781e0557@amdc2363> (raw)
In-Reply-To: <CAGo_u6p-uqw82ewDciDEhAt0=Nswfw33-gf8Aa+-6_k3eCcTzw@mail.gmail.com>
Hi Nishanth, Viresh
If I may add my 2 cents.
> On Tue, May 13, 2014 at 10:40 PM, Viresh Kumar
> <viresh.kumar@linaro.org> wrote:
> > On 14 May 2014 06:32, Thomas Abraham <ta.omasab@gmail.com> wrote:
> [...]
> >> +#ifdef CONFIG_CPU_FREQ_BOOST_SW
> >> + if (of_find_property(dev->of_node, "boost-frequency",
> >> &len)) {
> >
> > Does this mean another block inside the cpu node ? Like this: ?
> >
> > cpu at 0 {
> > compatible = "arm,cortex-a9";
> > reg = <0>;
> > next-level-cache = <&L2>;
> > operating-points = <
> > /* kHz uV */
> > 792000 1100000
> > 396000 950000
> > 198000 850000
> > >;
> > boost-frequency = <
> > 792000
> > 198000
> > >;
I think that the above approach is more appealing [*].
> > };
> >
> > I think it we might better add another field in the opp block as
> > these OPPs are rather boost one..
>
> If we change the meaning to be:
> operating-points = <
> /* kHz uV boost? */
> 792000 1100000 1
> 396000 950000 0
>
> We are adding a modifier to the OPP definition here and does imply
> every platform which may or maynot require "boost" need to indicate
> that - basically fails at least my "least common" description for a
> generic definition. As I had indicated in other threads -> we are back
> to the discussion of "additional properties" to an OPP..
> Option 1:
> - describe modifiers separately (as in boost-frequencies) - that
> operate based on data from opp table.
> Option 2:
> - keep adding to the description of opp as time goes by - every SoC
> has it's own set of quirks that are needed for an OPP - I know that at
> TI, we have certain OPPs that can only be enabled *if* "custom
> hardware driver" is enabled, and similar stories. - yet another
> example is enable the OPP if efuse X, bit Y is set.
>
> Personally, looking at the various descriptions and bL, cpu-idle
> states dependencies on OPPs, etc.. Option 2 is a non-scalable
> approach.
I agree with Nishanth here, that point 1 (as described by Viresh at
[*]) is a more scalable approach.
>
> [...]
--
Best regards,
Lukasz Majewski
Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
next prev parent reply other threads:[~2014-05-14 6:09 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-05-14 1:02 [PATCH v3 0/2] cpufreq: opp: Add device tree based lookup of boost mode frequency Thomas Abraham
2014-05-14 1:02 ` Thomas Abraham
2014-05-14 1:02 ` [PATCH v3 1/2] cpufreq / OPP: Allow boost frequency to be looked up from device tree Thomas Abraham
2014-05-14 1:02 ` Thomas Abraham
2014-05-14 3:40 ` Viresh Kumar
2014-05-14 3:40 ` Viresh Kumar
2014-05-14 4:05 ` Nishanth Menon
2014-05-14 4:05 ` Nishanth Menon
2014-05-14 6:09 ` Lukasz Majewski [this message]
2014-05-14 6:09 ` Lukasz Majewski
2014-05-14 6:24 ` Viresh Kumar
2014-05-14 6:24 ` Viresh Kumar
2014-05-14 13:49 ` Thomas Abraham
2014-05-14 13:49 ` Thomas Abraham
2014-05-14 14:31 ` Nishanth Menon
2014-05-14 14:31 ` Nishanth Menon
2014-05-14 13:46 ` Thomas Abraham
2014-05-14 13:46 ` Thomas Abraham
2014-05-14 13:54 ` Viresh Kumar
2014-05-14 13:54 ` Viresh Kumar
2014-05-14 1:03 ` [PATCH v3 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency Thomas Abraham
2014-05-14 1:03 ` Thomas Abraham
2014-05-14 2:02 ` Nishanth Menon
2014-05-14 2:02 ` Nishanth Menon
2014-05-14 13:17 ` Thomas Abraham
2014-05-14 13:17 ` Thomas Abraham
2014-05-14 15:06 ` Sudeep Holla
2014-05-14 15:06 ` Sudeep Holla
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=20140514080930.781e0557@amdc2363 \
--to=l.majewski@samsung.com \
--cc=devicetree@vger.kernel.org \
--cc=kgene.kim@samsung.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-pm@vger.kernel.org \
--cc=linux-samsung-soc@vger.kernel.org \
--cc=nm@ti.com \
--cc=rjw@rjwysocki.net \
--cc=rob.herring@linaro.org \
--cc=t.figa@samsung.com \
--cc=ta.omasab@gmail.com \
--cc=thomas.ab@samsung.com \
--cc=viresh.kumar@linaro.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.