diff mbox

[v2,3/7] cpufreq-dt: add turbo modes support

Message ID 1436456621-29839-4-git-send-email-b.zolnierkie@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Bartlomiej Zolnierkiewicz July 9, 2015, 3:43 p.m. UTC
Add turbo modes (from opp-v2 bindings) support using
existing cpufreq 'boost' mode infrastructure:
- add boost_supported field to struct cpufreq_dt_platform_data
- set dt_cpufreq_driver.boost_supported in dt_cpufreq_probe()

Cc: Tomasz Figa <tomasz.figa@gmail.com>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Javier Martinez Canillas <javier@dowhile0.org>
Cc: Thomas Abraham <thomas.ab@samsung.com>
Cc: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 drivers/cpufreq/cpufreq-dt.c | 7 ++++++-
 include/linux/cpufreq-dt.h   | 1 +
 2 files changed, 7 insertions(+), 1 deletion(-)

Comments

Krzysztof Kozlowski July 10, 2015, 8:22 a.m. UTC | #1
On 10.07.2015 00:43, Bartlomiej Zolnierkiewicz wrote:
> Add turbo modes (from opp-v2 bindings) support using
> existing cpufreq 'boost' mode infrastructure:
> - add boost_supported field to struct cpufreq_dt_platform_data
> - set dt_cpufreq_driver.boost_supported in dt_cpufreq_probe()
> 
> Cc: Tomasz Figa <tomasz.figa@gmail.com>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Javier Martinez Canillas <javier@dowhile0.org>
> Cc: Thomas Abraham <thomas.ab@samsung.com>
> Cc: Viresh Kumar <viresh.kumar@linaro.org>
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> ---
>  drivers/cpufreq/cpufreq-dt.c | 7 ++++++-
>  include/linux/cpufreq-dt.h   | 1 +
>  2 files changed, 7 insertions(+), 1 deletion(-)

Looks fine,
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Best regards,
Krzysztof
Viresh Kumar July 27, 2015, 8:37 a.m. UTC | #2
On 09-07-15, 17:43, Bartlomiej Zolnierkiewicz wrote:
> diff --git a/include/linux/cpufreq-dt.h b/include/linux/cpufreq-dt.h
> index 0414009..483ca1b 100644
> --- a/include/linux/cpufreq-dt.h
> +++ b/include/linux/cpufreq-dt.h
> @@ -17,6 +17,7 @@ struct cpufreq_dt_platform_data {
>  	 * clock.
>  	 */
>  	bool independent_clocks;
> +	bool boost_supported;
>  };

I am planning to kill this structure soon, don't add anything to it.
We should be doing this based on DT.
Bartlomiej Zolnierkiewicz July 27, 2015, 11:01 a.m. UTC | #3
Hi,

On Monday, July 27, 2015 02:07:54 PM Viresh Kumar wrote:
> On 09-07-15, 17:43, Bartlomiej Zolnierkiewicz wrote:
> > diff --git a/include/linux/cpufreq-dt.h b/include/linux/cpufreq-dt.h
> > index 0414009..483ca1b 100644
> > --- a/include/linux/cpufreq-dt.h
> > +++ b/include/linux/cpufreq-dt.h
> > @@ -17,6 +17,7 @@ struct cpufreq_dt_platform_data {
> >  	 * clock.
> >  	 */
> >  	bool independent_clocks;
> > +	bool boost_supported;
> >  };
> 
> I am planning to kill this structure soon, don't add anything to it.
> We should be doing this based on DT.

This change was in the original patch posted in April:
https://lkml.org/lkml/2015/4/10/646

your review from a month ago didn't contain this request:
https://lkml.org/lkml/2015/6/22/667

and now (after nearly 4 months) you are telling me that
I should change this because you are planning to do some
more changes in the future.

Could we please keep it as it is for now and change it
later (after independent_clocks configuration will get
ported to use device tree)?

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
Viresh Kumar July 27, 2015, 11:33 a.m. UTC | #4
On 27-07-15, 13:01, Bartlomiej Zolnierkiewicz wrote:

First of all, please don't be angry :).. We can discuss and get things
sorted out ...

> This change was in the original patch posted in April:
> https://lkml.org/lkml/2015/4/10/646

Yeah, and I already apologized for missing the request :)

> your review from a month ago didn't contain this request:
> https://lkml.org/lkml/2015/6/22/667

Your patch inserted almost 116 lines and most of the stuff was around
adding new bindings to get things working with cpufreq-dt driver.

And so I replied to the most important stuff, i.e. don't add new
bindings, we will sort it out with opp-v2.

And frankly that wasn't the time where we could have discussed how
exactly we are going to use it. Ofcourse we should get it via DT,
platform data is just not required.

So, me not NAK ing this approach was fine as it wasn't about keeping
this data in the platform data part.

> and now (after nearly 4 months) you are telling me that

I will say a month, as we discarded most of that patch recently :)

> I should change this because you are planning to do some
> more changes in the future.

Its not about me doing some changes. But the whole point of doing the
opp-v2 thing was to get rid of such platform data things..

Just that your work is competing with opp-v2 code :)

> Could we please keep it as it is for now and change it
> later (after independent_clocks configuration will get
> ported to use device tree)?

I thought we can get your work to a better shape, with all credit to
you. But if you have some dependency on this for 4.3, then I don't
mind killing this structure after you have polluted it a bit more :)
Bartlomiej Zolnierkiewicz July 27, 2015, 11:58 a.m. UTC | #5
On Monday, July 27, 2015 05:03:40 PM Viresh Kumar wrote:
> On 27-07-15, 13:01, Bartlomiej Zolnierkiewicz wrote:
> 
> First of all, please don't be angry :).. We can discuss and get things
> sorted out ...

OK :)

> > This change was in the original patch posted in April:
> > https://lkml.org/lkml/2015/4/10/646
> 
> Yeah, and I already apologized for missing the request :)
> 
> > your review from a month ago didn't contain this request:
> > https://lkml.org/lkml/2015/6/22/667
> 
> Your patch inserted almost 116 lines and most of the stuff was around
> adding new bindings to get things working with cpufreq-dt driver.
> 
> And so I replied to the most important stuff, i.e. don't add new
> bindings, we will sort it out with opp-v2.
> 
> And frankly that wasn't the time where we could have discussed how
> exactly we are going to use it. Ofcourse we should get it via DT,
> platform data is just not required.
> 
> So, me not NAK ing this approach was fine as it wasn't about keeping
> this data in the platform data part.
> 
> > and now (after nearly 4 months) you are telling me that
> 
> I will say a month, as we discarded most of that patch recently :)
> 
> > I should change this because you are planning to do some
> > more changes in the future.
> 
> Its not about me doing some changes. But the whole point of doing the
> opp-v2 thing was to get rid of such platform data things..
> 
> Just that your work is competing with opp-v2 code :)
> 
> > Could we please keep it as it is for now and change it
> > later (after independent_clocks configuration will get
> > ported to use device tree)?
> 
> I thought we can get your work to a better shape, with all credit to
> you. But if you have some dependency on this for 4.3, then I don't
> mind killing this structure after you have polluted it a bit more :)

Thank you.  This is exactly the case here (I would like to get
Exynos4x12 conversion to use cpufreq-dt + exynos-cpufreq removal
in v4.3 if possible and adding new DT bindings will most likely
slow down the process considerably).

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
Viresh Kumar July 27, 2015, 12:01 p.m. UTC | #6
On 27-07-15, 13:58, Bartlomiej Zolnierkiewicz wrote:
> Thank you.  This is exactly the case here (I would like to get
> Exynos4x12 conversion to use cpufreq-dt + exynos-cpufreq removal
> in v4.3 if possible and adding new DT bindings will most likely
> slow down the process considerably).

Heh, I never asked you to add new DT bindings, I said we can solve it
with DT. We already have turbo properties in DT.
diff mbox

Patch

diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c
index 60d98fb..9024205 100644
--- a/drivers/cpufreq/cpufreq-dt.c
+++ b/drivers/cpufreq/cpufreq-dt.c
@@ -408,6 +408,7 @@  static struct cpufreq_driver dt_cpufreq_driver = {
 
 static int dt_cpufreq_probe(struct platform_device *pdev)
 {
+	struct cpufreq_dt_platform_data *pd;
 	struct device *cpu_dev;
 	struct regulator *cpu_reg;
 	struct clk *cpu_clk;
@@ -428,7 +429,11 @@  static int dt_cpufreq_probe(struct platform_device *pdev)
 	if (!IS_ERR(cpu_reg))
 		regulator_put(cpu_reg);
 
-	dt_cpufreq_driver.driver_data = dev_get_platdata(&pdev->dev);
+	pd = dev_get_platdata(&pdev->dev);
+	dt_cpufreq_driver.driver_data = pd;
+
+	if (pd)
+		dt_cpufreq_driver.boost_supported = pd->boost_supported;
 
 	ret = cpufreq_register_driver(&dt_cpufreq_driver);
 	if (ret)
diff --git a/include/linux/cpufreq-dt.h b/include/linux/cpufreq-dt.h
index 0414009..483ca1b 100644
--- a/include/linux/cpufreq-dt.h
+++ b/include/linux/cpufreq-dt.h
@@ -17,6 +17,7 @@  struct cpufreq_dt_platform_data {
 	 * clock.
 	 */
 	bool independent_clocks;
+	bool boost_supported;
 };
 
 #endif /* __CPUFREQ_DT_H__ */