[RFC,02/22] cpufreq: ARM_DT_BL_CPUFREQ needs ARM_CPU_TOPOLOGY
diff mbox

Message ID 1367507786-505303-3-git-send-email-arnd@arndb.de
State Not Applicable, archived
Headers show

Commit Message

Arnd Bergmann May 2, 2013, 3:16 p.m. UTC
The big.LITTLE cpufreq driver uses the CPU topology API, which
needs to be reflected in Kconfig to prevent broken configurations.

warning: (ARM_DT_BL_CPUFREQ) selects ARM_BIG_LITTLE_CPUFREQ which
has unmet direct dependencies (ARCH_HAS_CPUFREQ && CPU_FREQ && ARM && ARM_CPU_TOPOLOGY)

Cc: Rafael J. Wysocki <rjw@sisk.pl>
Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: cpufreq@vger.kernel.org
Cc: linux-pm@vger.kernel.org
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/cpufreq/Kconfig.arm | 1 +
 1 file changed, 1 insertion(+)

Comments

Viresh Kumar May 3, 2013, 4:51 a.m. UTC | #1
On 2 May 2013 20:46, Arnd Bergmann <arnd@arndb.de> wrote:
> The big.LITTLE cpufreq driver uses the CPU topology API, which
> needs to be reflected in Kconfig to prevent broken configurations.
>
> warning: (ARM_DT_BL_CPUFREQ) selects ARM_BIG_LITTLE_CPUFREQ which
> has unmet direct dependencies (ARCH_HAS_CPUFREQ && CPU_FREQ && ARM && ARM_CPU_TOPOLOGY)
>
> Cc: Rafael J. Wysocki <rjw@sisk.pl>
> Cc: Viresh Kumar <viresh.kumar@linaro.org>
> Cc: cpufreq@vger.kernel.org
> Cc: linux-pm@vger.kernel.org
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/cpufreq/Kconfig.arm | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> index f3af18b..3fd6bdf 100644
> --- a/drivers/cpufreq/Kconfig.arm
> +++ b/drivers/cpufreq/Kconfig.arm
> @@ -10,6 +10,7 @@ config ARM_DT_BL_CPUFREQ
>         tristate "Generic ARM big LITTLE CPUfreq driver probed via DT"
>         select ARM_BIG_LITTLE_CPUFREQ
>         depends on OF && HAVE_CLK
> +       depends on ARM_CPU_TOPOLOGY
>         help
>           This enables the Generic CPUfreq driver for ARM big.LITTLE platform.
>           This gets frequency tables from DT.

This Kconfig thing has always been confusing to me. This is how the code looks
currently.

config ARM_BIG_LITTLE_CPUFREQ
        tristate
        depends on ARM_CPU_TOPOLOGY

config ARM_DT_BL_CPUFREQ
        tristate "Generic ARM big LITTLE CPUfreq driver probed via DT"
        select ARM_BIG_LITTLE_CPUFREQ
        depends on OF && HAVE_CLK
        help
          This enables the Generic CPUfreq driver for ARM big.LITTLE platform.
          This gets frequency tables from DT.

Because all ARM big LITTLE cpufreq stuff have dependency on
ARM_CPU_TOPOLOGY, i have added dependency at a common place. So that
we don't end up adding this in every glue layer driver. How should this be done?
--
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
Rafael Wysocki May 3, 2013, 12:15 p.m. UTC | #2
On Friday, May 03, 2013 10:21:45 AM Viresh Kumar wrote:
> On 2 May 2013 20:46, Arnd Bergmann <arnd@arndb.de> wrote:
> > The big.LITTLE cpufreq driver uses the CPU topology API, which
> > needs to be reflected in Kconfig to prevent broken configurations.
> >
> > warning: (ARM_DT_BL_CPUFREQ) selects ARM_BIG_LITTLE_CPUFREQ which
> > has unmet direct dependencies (ARCH_HAS_CPUFREQ && CPU_FREQ && ARM && ARM_CPU_TOPOLOGY)
> >
> > Cc: Rafael J. Wysocki <rjw@sisk.pl>
> > Cc: Viresh Kumar <viresh.kumar@linaro.org>
> > Cc: cpufreq@vger.kernel.org
> > Cc: linux-pm@vger.kernel.org
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> >  drivers/cpufreq/Kconfig.arm | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> > index f3af18b..3fd6bdf 100644
> > --- a/drivers/cpufreq/Kconfig.arm
> > +++ b/drivers/cpufreq/Kconfig.arm
> > @@ -10,6 +10,7 @@ config ARM_DT_BL_CPUFREQ
> >         tristate "Generic ARM big LITTLE CPUfreq driver probed via DT"
> >         select ARM_BIG_LITTLE_CPUFREQ
> >         depends on OF && HAVE_CLK
> > +       depends on ARM_CPU_TOPOLOGY
> >         help
> >           This enables the Generic CPUfreq driver for ARM big.LITTLE platform.
> >           This gets frequency tables from DT.
> 
> This Kconfig thing has always been confusing to me. This is how the code looks
> currently.
> 
> config ARM_BIG_LITTLE_CPUFREQ
>         tristate
>         depends on ARM_CPU_TOPOLOGY
> 
> config ARM_DT_BL_CPUFREQ
>         tristate "Generic ARM big LITTLE CPUfreq driver probed via DT"
>         select ARM_BIG_LITTLE_CPUFREQ
>         depends on OF && HAVE_CLK
>         help
>           This enables the Generic CPUfreq driver for ARM big.LITTLE platform.
>           This gets frequency tables from DT.
> 
> Because all ARM big LITTLE cpufreq stuff have dependency on
> ARM_CPU_TOPOLOGY, i have added dependency at a common place. So that
> we don't end up adding this in every glue layer driver. How should this be done?

First, there's no rule of thumb here as far as I can say.

In this particular case I think it is OK to make both ARM_DT_BL_CPUFREQ and
ARM_BIG_LITTLE_CPUFREQ depend on ARM_CPU_TOPOLOGY, because (in theory?) the
latter may be set without the former (unless you want to make ARM_DT_BL_CPUFREQ
depend on ARM_BIG_LITTLE_CPUFREQ, but then it may be kind of confusing to
users).

Thanks,
Rafael
Viresh Kumar May 3, 2013, 1:01 p.m. UTC | #3
On 3 May 2013 17:45, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> In this particular case I think it is OK to make both ARM_DT_BL_CPUFREQ and
> ARM_BIG_LITTLE_CPUFREQ depend on ARM_CPU_TOPOLOGY, because (in theory?) the
> latter may be set without the former (unless you want to make ARM_DT_BL_CPUFREQ
> depend on ARM_BIG_LITTLE_CPUFREQ, but then it may be kind of confusing to
> users).

ARM_BIG_LITTLE_CPUFREQ is the core cpufreq code for big LITTLE SoC's and every
other driver will be a glue providing ops to it. So, ARM_DT_BL_CPUFREQ
does depend
on ARM_BIG_LITTLE_CPUFREQ and that's why i added depends on
ARM_CPU_TOPOLOGY in ARM_BIG_LITTLE_CPUFREQ only and depends on
ARM_BIG_LITTLE_CPUFREQ in ARM_DT_BL_CPUFREQ.

But the problem is if ARM_DT_BL_CPUFREQ isn't selected then we still get
ARM_DT_BL_CPUFREQ enabled in menuconfig but a warning just before compilation.
Which Arnd pointed to..
--
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
Rafael Wysocki May 3, 2013, 7:27 p.m. UTC | #4
On Friday, May 03, 2013 06:31:52 PM Viresh Kumar wrote:
> On 3 May 2013 17:45, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > In this particular case I think it is OK to make both ARM_DT_BL_CPUFREQ and
> > ARM_BIG_LITTLE_CPUFREQ depend on ARM_CPU_TOPOLOGY, because (in theory?) the
> > latter may be set without the former (unless you want to make ARM_DT_BL_CPUFREQ
> > depend on ARM_BIG_LITTLE_CPUFREQ, but then it may be kind of confusing to
> > users).
> 
> ARM_BIG_LITTLE_CPUFREQ is the core cpufreq code for big LITTLE SoC's and every
> other driver will be a glue providing ops to it. So, ARM_DT_BL_CPUFREQ
> does depend
> on ARM_BIG_LITTLE_CPUFREQ and that's why i added depends on
> ARM_CPU_TOPOLOGY in ARM_BIG_LITTLE_CPUFREQ only and depends on
> ARM_BIG_LITTLE_CPUFREQ in ARM_DT_BL_CPUFREQ.

I'm seeing "select" in there, which is kind of different from "depends on".

> But the problem is if ARM_DT_BL_CPUFREQ isn't selected then we still get
> ARM_DT_BL_CPUFREQ enabled in menuconfig but a warning just before compilation.
> Which Arnd pointed to..

What do you mean by "enabled in menuconfig"?  Does it appear as an option to
select or is it actually selected?

In any case, I'm afraid adding ARM_CPU_TOPOLOGY to the "depends on" list in
ARM_DT_BL_CPUFREQ is the only way to avoid that particular warning.

Thanks,
Rafael

Patch
diff mbox

diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index f3af18b..3fd6bdf 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -10,6 +10,7 @@  config ARM_DT_BL_CPUFREQ
 	tristate "Generic ARM big LITTLE CPUfreq driver probed via DT"
 	select ARM_BIG_LITTLE_CPUFREQ
 	depends on OF && HAVE_CLK
+	depends on ARM_CPU_TOPOLOGY
 	help
 	  This enables the Generic CPUfreq driver for ARM big.LITTLE platform.
 	  This gets frequency tables from DT.