diff mbox

[v6,6/8] cpufreq:exynos:Extend Exynos cpufreq driver to support boost framework

Message ID 1374770011-22171-7-git-send-email-l.majewski@samsung.com (mailing list archive)
State Changes Requested, archived
Headers show

Commit Message

Lukasz Majewski July 25, 2013, 4:33 p.m. UTC 
The cpufreq_driver's boost_supported flag is true only when boost
support is explicitly enabled. Boost related attributes are exported only
under the same condition.

Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
Signed-off-by: Myungjoo Ham <myungjoo.ham@samsung.com>

---
Changes for v6:
- Replace exynos_driver.boost_supported = 1 to = true
- Protect boost attributes export with CONFIG_CPU_FREQ_BOOST_SW

Changes for v5:
- None

Changes for v4:
- None

Changes for v3:
- Remove low level boost code
- Move boost management code to cpufreq core code
- Use boost_supported flag to indicate if driver supports over clocking

Changes for v2:
- Removal of struct cpufreq_boost
- Removal of the CONFIG_CPU_FREQ_BOOST flag
- low_level_boost with valid address when boost is supported

 drivers/cpufreq/exynos-cpufreq.c |    6 ++++++
 1 file changed, 6 insertions(+)

Comments

Viresh Kumar July 26, 2013, 10:26 a.m. UTC | #1 
On 25 July 2013 22:03, Lukasz Majewski <l.majewski@samsung.com> wrote:

> diff --git a/drivers/cpufreq/exynos-cpufreq.c b/drivers/cpufreq/exynos-cpufreq.c
> index 9ae1871..175172d9 100644
> --- a/drivers/cpufreq/exynos-cpufreq.c
> +++ b/drivers/cpufreq/exynos-cpufreq.c
> @@ -270,6 +270,9 @@ static int exynos_cpufreq_cpu_exit(struct cpufreq_policy *policy)
>
>  static struct freq_attr *exynos_cpufreq_attr[] = {
>         &cpufreq_freq_attr_scaling_available_freqs,
> +#ifdef CONFIG_CPU_FREQ_BOOST_SW

Use ARM_EXYNOS_CPU_FREQ_BOOST_SW instead.

> +       &cpufreq_freq_attr_scaling_boost_freqs,
> +#endif
>         NULL,
>  };
>
> @@ -332,6 +335,9 @@ static int __init exynos_cpufreq_probe(struct platform_device *pdev)
>
>         locking_frequency = exynos_getspeed(0);
>
> +#ifdef CONFIG_CPU_FREQ_BOOST_SW
> +       exynos_driver.boost_supported = true;
> +#endif

So, why here and not in the definition of exynos_driver?

>         register_pm_notifier(&exynos_cpufreq_nb);
>
>         if (cpufreq_register_driver(&exynos_driver)) {
> --
> 1.7.10.4
>
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lukasz Majewski July 26, 2013, 11:26 a.m. UTC | #2 
On Fri, 26 Jul 2013 15:56:53 +0530 Viresh Kumar wrote,
> On 25 July 2013 22:03, Lukasz Majewski <l.majewski@samsung.com> wrote:
> 
> > diff --git a/drivers/cpufreq/exynos-cpufreq.c
> > b/drivers/cpufreq/exynos-cpufreq.c index 9ae1871..175172d9 100644
> > --- a/drivers/cpufreq/exynos-cpufreq.c
> > +++ b/drivers/cpufreq/exynos-cpufreq.c
> > @@ -270,6 +270,9 @@ static int exynos_cpufreq_cpu_exit(struct
> > cpufreq_policy *policy)
> >
> >  static struct freq_attr *exynos_cpufreq_attr[] = {
> >         &cpufreq_freq_attr_scaling_available_freqs,
> > +#ifdef CONFIG_CPU_FREQ_BOOST_SW
  	    ^^^^^^^^^^^^^^^^^^^^^^^^^ [*]
> 
> Use ARM_EXYNOS_CPU_FREQ_BOOST_SW instead.

For the reasons explained at [PATCH v6 5/8] I would prefer to leave [*]
here.

> 
> > +       &cpufreq_freq_attr_scaling_boost_freqs,
> > +#endif
> >         NULL,
> >  };
> >
> > @@ -332,6 +335,9 @@ static int __init exynos_cpufreq_probe(struct
> > platform_device *pdev)
> >
> >         locking_frequency = exynos_getspeed(0);
> >
> > +#ifdef CONFIG_CPU_FREQ_BOOST_SW
> > +       exynos_driver.boost_supported = true;
> > +#endif
> 
> So, why here and not in the definition of exynos_driver?

Right. I will move this to struct cpufreq_driver exynos_driver.

> 
> >         register_pm_notifier(&exynos_cpufreq_nb);
> >
> >         if (cpufreq_register_driver(&exynos_driver)) {
> > --
> > 1.7.10.4
> >
Viresh Kumar July 29, 2013, 7:01 a.m. UTC | #3 
On 26 July 2013 16:56, Lukasz Majewski <l.majewski@samsung.com> wrote:
> On Fri, 26 Jul 2013 15:56:53 +0530 Viresh Kumar wrote,
>> On 25 July 2013 22:03, Lukasz Majewski <l.majewski@samsung.com> wrote:
>>
>> > diff --git a/drivers/cpufreq/exynos-cpufreq.c
>> > b/drivers/cpufreq/exynos-cpufreq.c index 9ae1871..175172d9 100644
>> > --- a/drivers/cpufreq/exynos-cpufreq.c
>> > +++ b/drivers/cpufreq/exynos-cpufreq.c
>> > @@ -270,6 +270,9 @@ static int exynos_cpufreq_cpu_exit(struct
>> > cpufreq_policy *policy)
>> >
>> >  static struct freq_attr *exynos_cpufreq_attr[] = {
>> >         &cpufreq_freq_attr_scaling_available_freqs,
>> > +#ifdef CONFIG_CPU_FREQ_BOOST_SW
>             ^^^^^^^^^^^^^^^^^^^^^^^^^ [*]
>>
>> Use ARM_EXYNOS_CPU_FREQ_BOOST_SW instead.
>
> For the reasons explained at [PATCH v6 5/8] I would prefer to leave [*]
> here.

I don't see how that reasoning fit here.

This is exynos code and you must use exynos specific boost Kconfig
option here.. Otherwise It might be enabled without Exynos specific option,
if somebody else has selected CONFIG_CPU_FREQ_BOOST_SW in
a multi platform kernel,
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lukasz Majewski Aug. 12, 2013, 9:52 a.m. UTC | #4 
On Fri, 26 Jul 2013 15:56:53 +0530 Viresh Kumar viresh.kumar@linaro.org
wrote,
> On 25 July 2013 22:03, Lukasz Majewski <l.majewski@samsung.com> wrote:
> 
> > diff --git a/drivers/cpufreq/exynos-cpufreq.c
> > b/drivers/cpufreq/exynos-cpufreq.c index 9ae1871..175172d9 100644
> > --- a/drivers/cpufreq/exynos-cpufreq.c
> > +++ b/drivers/cpufreq/exynos-cpufreq.c
> > @@ -270,6 +270,9 @@ static int exynos_cpufreq_cpu_exit(struct
> > cpufreq_policy *policy)
> >
> >  static struct freq_attr *exynos_cpufreq_attr[] = {
> >         &cpufreq_freq_attr_scaling_available_freqs,
> > +#ifdef CONFIG_CPU_FREQ_BOOST_SW
> 
> Use ARM_EXYNOS_CPU_FREQ_BOOST_SW instead.

Ok, the ARM_EXYNOS_CPU_FREQ_BOOST_SW looks more appropriate here.

> 
> > +       &cpufreq_freq_attr_scaling_boost_freqs,
> > +#endif
> >         NULL,
> >  };
> >
> > @@ -332,6 +335,9 @@ static int __init exynos_cpufreq_probe(struct
> > platform_device *pdev)
> >
> >         locking_frequency = exynos_getspeed(0);
> >
> > +#ifdef CONFIG_CPU_FREQ_BOOST_SW
> > +       exynos_driver.boost_supported = true;
> > +#endif
> 
> So, why here and not in the definition of exynos_driver?

Ok, I will move the above code to struct cpufreq_driver exynos_driver

> 
> >         register_pm_notifier(&exynos_cpufreq_nb);
> >
> >         if (cpufreq_register_driver(&exynos_driver)) {
> > --
> > 1.7.10.4
> >
diff mbox

Patch

diff --git a/drivers/cpufreq/exynos-cpufreq.c b/drivers/cpufreq/exynos-cpufreq.c
index 9ae1871..175172d9 100644
--- a/drivers/cpufreq/exynos-cpufreq.c
+++ b/drivers/cpufreq/exynos-cpufreq.c
@@ -270,6 +270,9 @@  static int exynos_cpufreq_cpu_exit(struct cpufreq_policy *policy)
 
 static struct freq_attr *exynos_cpufreq_attr[] = {
 	&cpufreq_freq_attr_scaling_available_freqs,
+#ifdef CONFIG_CPU_FREQ_BOOST_SW
+	&cpufreq_freq_attr_scaling_boost_freqs,
+#endif
 	NULL,
 };
 
@@ -332,6 +335,9 @@  static int __init exynos_cpufreq_probe(struct platform_device *pdev)
 
 	locking_frequency = exynos_getspeed(0);
 
+#ifdef CONFIG_CPU_FREQ_BOOST_SW
+	exynos_driver.boost_supported = true;
+#endif
 	register_pm_notifier(&exynos_cpufreq_nb);
 
 	if (cpufreq_register_driver(&exynos_driver)) {