linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] cpufreq: Remove architecture specific menu entries
@ 2014-11-12  4:10 Viresh Kumar
  2014-11-13  1:47 ` Rafael J. Wysocki
  0 siblings, 1 reply; 5+ messages in thread
From: Viresh Kumar @ 2014-11-12  4:10 UTC (permalink / raw)
  To: Rafael Wysocki
  Cc: linaro-kernel, linux-pm, scottwood, Yuantian.Tang, Viresh Kumar

CPUFreq driver's Kconfig entries are added in Kconfig.<arch> files and they are
all included from the main Kconfig file using a menu entry. This creates another
level of (unnecessary) hierarchy within the menuconfig entries.

The problem occurs when there are drivers usable across architectures. Either
their config entry is duplicated in all the supported architectures or is put
into the main Kconfig entry. With the later one, we have menuconfig entries for
drivers at two levels then.

Fix these issues by getting rid of another level of menuconfig entries and
populate all drivers within the main cpufreq menu.

Reported-by: Tang Yuantian <Yuantian.Tang@freescale.com>
Suggested-by: Scott Wood <scottwood@freescale.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/Kconfig | 48 +++++++++++++++++-------------------------------
 1 file changed, 17 insertions(+), 31 deletions(-)

diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig
index 3489f8f..a24d678 100644
--- a/drivers/cpufreq/Kconfig
+++ b/drivers/cpufreq/Kconfig
@@ -196,19 +196,19 @@ config CPUFREQ_DT
 
 	  If in doubt, say N.
 
-menu "x86 CPU frequency scaling drivers"
-depends on X86
+if X86
 source "drivers/cpufreq/Kconfig.x86"
-endmenu
+endif
 
-menu "ARM CPU frequency scaling drivers"
-depends on ARM || ARM64
+if ARM || ARM64
 source "drivers/cpufreq/Kconfig.arm"
-endmenu
+endif
 
-menu "AVR32 CPU frequency scaling drivers"
-depends on AVR32
+if PPC32 || PPC64
+source "drivers/cpufreq/Kconfig.powerpc"
+endif
 
+if AVR32
 config AVR32_AT32AP_CPUFREQ
 	bool "CPU frequency driver for AT32AP"
 	depends on PLATFORM_AT32AP
@@ -216,12 +216,9 @@ config AVR32_AT32AP_CPUFREQ
 	help
 	  This enables the CPU frequency driver for AT32AP processors.
 	  If in doubt, say N.
+endif
 
-endmenu
-
-menu "CPUFreq processor drivers"
-depends on IA64
-
+if IA64
 config IA64_ACPI_CPUFREQ
 	tristate "ACPI Processor P-States driver"
 	depends on ACPI_PROCESSOR
@@ -232,12 +229,9 @@ config IA64_ACPI_CPUFREQ
 	For details, take a look at <file:Documentation/cpu-freq/>.
 
 	If in doubt, say N.
+endif
 
-endmenu
-
-menu "MIPS CPUFreq processor drivers"
-depends on MIPS
-
+if MIPS
 config LOONGSON2_CPUFREQ
 	tristate "Loongson2 CPUFreq Driver"
 	help
@@ -249,16 +243,9 @@ config LOONGSON2_CPUFREQ
 	  For details, take a look at <file:Documentation/cpu-freq/>.
 
 	  If in doubt, say N.
+endif
 
-endmenu
-
-menu "PowerPC CPU frequency scaling drivers"
-depends on PPC32 || PPC64
-source "drivers/cpufreq/Kconfig.powerpc"
-endmenu
-
-menu "SPARC CPU frequency scaling drivers"
-depends on SPARC64
+if SPARC64
 config SPARC_US3_CPUFREQ
 	tristate "UltraSPARC-III CPU Frequency driver"
 	help
@@ -276,10 +263,9 @@ config SPARC_US2E_CPUFREQ
 	  For details, take a look at <file:Documentation/cpu-freq>.
 
 	  If in doubt, say N.
-endmenu
+endif
 
-menu "SH CPU Frequency scaling"
-depends on SUPERH
+if SUPERH
 config SH_CPU_FREQ
 	tristate "SuperH CPU Frequency driver"
 	help
@@ -293,7 +279,7 @@ config SH_CPU_FREQ
 	  For details, take a look at <file:Documentation/cpu-freq>.
 
 	  If unsure, say N.
-endmenu
+endif
 
 endif
 endmenu
-- 
2.0.3.693.g996b0fd


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] cpufreq: Remove architecture specific menu entries
  2014-11-13  1:47 ` Rafael J. Wysocki
@ 2014-11-13  1:43   ` Scott Wood
  2014-11-13  5:26     ` Viresh Kumar
  0 siblings, 1 reply; 5+ messages in thread
From: Scott Wood @ 2014-11-13  1:43 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: Viresh Kumar, linaro-kernel, linux-pm, Yuantian.Tang

On Thu, 2014-11-13 at 02:47 +0100, Rafael J. Wysocki wrote:
> On Wednesday, November 12, 2014 09:40:43 AM Viresh Kumar wrote:
> > CPUFreq driver's Kconfig entries are added in Kconfig.<arch> files and they are
> > all included from the main Kconfig file using a menu entry. This creates another
> > level of (unnecessary) hierarchy within the menuconfig entries.
> > 
> > The problem occurs when there are drivers usable across architectures. Either
> > their config entry is duplicated in all the supported architectures or is put
> > into the main Kconfig entry. With the later one, we have menuconfig entries for
> > drivers at two levels then.
> > 
> > Fix these issues by getting rid of another level of menuconfig entries and
> > populate all drivers within the main cpufreq menu.
> 
> Won't this be confusing?

Why would it be confusing?  It's already in a not-too-large "CPU
Frequency scaling" menu, the names of the individual options should be
clear enough, and they'd all be grouped at the end of the menu.

FWIW, there's already "Generic DT based cpufreq driver" under that menu.

> Can we at least have a "CPU frequency scaling drivers" menu for drivers
> the *contents* of which will depend on the architecture?

That works too -- I just don't see the confusion aspect.

-Scott



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] cpufreq: Remove architecture specific menu entries
  2014-11-12  4:10 [PATCH] cpufreq: Remove architecture specific menu entries Viresh Kumar
@ 2014-11-13  1:47 ` Rafael J. Wysocki
  2014-11-13  1:43   ` Scott Wood
  0 siblings, 1 reply; 5+ messages in thread
From: Rafael J. Wysocki @ 2014-11-13  1:47 UTC (permalink / raw)
  To: Viresh Kumar; +Cc: linaro-kernel, linux-pm, scottwood, Yuantian.Tang

On Wednesday, November 12, 2014 09:40:43 AM Viresh Kumar wrote:
> CPUFreq driver's Kconfig entries are added in Kconfig.<arch> files and they are
> all included from the main Kconfig file using a menu entry. This creates another
> level of (unnecessary) hierarchy within the menuconfig entries.
> 
> The problem occurs when there are drivers usable across architectures. Either
> their config entry is duplicated in all the supported architectures or is put
> into the main Kconfig entry. With the later one, we have menuconfig entries for
> drivers at two levels then.
> 
> Fix these issues by getting rid of another level of menuconfig entries and
> populate all drivers within the main cpufreq menu.

Won't this be confusing?

Can we at least have a "CPU frequency scaling drivers" menu for drivers
the *contents* of which will depend on the architecture?

> Reported-by: Tang Yuantian <Yuantian.Tang@freescale.com>
> Suggested-by: Scott Wood <scottwood@freescale.com>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/cpufreq/Kconfig | 48 +++++++++++++++++-------------------------------
>  1 file changed, 17 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig
> index 3489f8f..a24d678 100644
> --- a/drivers/cpufreq/Kconfig
> +++ b/drivers/cpufreq/Kconfig
> @@ -196,19 +196,19 @@ config CPUFREQ_DT
>  
>  	  If in doubt, say N.
>  
> -menu "x86 CPU frequency scaling drivers"
> -depends on X86
> +if X86
>  source "drivers/cpufreq/Kconfig.x86"
> -endmenu
> +endif
>  
> -menu "ARM CPU frequency scaling drivers"
> -depends on ARM || ARM64
> +if ARM || ARM64
>  source "drivers/cpufreq/Kconfig.arm"
> -endmenu
> +endif
>  
> -menu "AVR32 CPU frequency scaling drivers"
> -depends on AVR32
> +if PPC32 || PPC64
> +source "drivers/cpufreq/Kconfig.powerpc"
> +endif
>  
> +if AVR32
>  config AVR32_AT32AP_CPUFREQ
>  	bool "CPU frequency driver for AT32AP"
>  	depends on PLATFORM_AT32AP
> @@ -216,12 +216,9 @@ config AVR32_AT32AP_CPUFREQ
>  	help
>  	  This enables the CPU frequency driver for AT32AP processors.
>  	  If in doubt, say N.
> +endif
>  
> -endmenu
> -
> -menu "CPUFreq processor drivers"
> -depends on IA64
> -
> +if IA64
>  config IA64_ACPI_CPUFREQ
>  	tristate "ACPI Processor P-States driver"
>  	depends on ACPI_PROCESSOR
> @@ -232,12 +229,9 @@ config IA64_ACPI_CPUFREQ
>  	For details, take a look at <file:Documentation/cpu-freq/>.
>  
>  	If in doubt, say N.
> +endif
>  
> -endmenu
> -
> -menu "MIPS CPUFreq processor drivers"
> -depends on MIPS
> -
> +if MIPS
>  config LOONGSON2_CPUFREQ
>  	tristate "Loongson2 CPUFreq Driver"
>  	help
> @@ -249,16 +243,9 @@ config LOONGSON2_CPUFREQ
>  	  For details, take a look at <file:Documentation/cpu-freq/>.
>  
>  	  If in doubt, say N.
> +endif
>  
> -endmenu
> -
> -menu "PowerPC CPU frequency scaling drivers"
> -depends on PPC32 || PPC64
> -source "drivers/cpufreq/Kconfig.powerpc"
> -endmenu
> -
> -menu "SPARC CPU frequency scaling drivers"
> -depends on SPARC64
> +if SPARC64
>  config SPARC_US3_CPUFREQ
>  	tristate "UltraSPARC-III CPU Frequency driver"
>  	help
> @@ -276,10 +263,9 @@ config SPARC_US2E_CPUFREQ
>  	  For details, take a look at <file:Documentation/cpu-freq>.
>  
>  	  If in doubt, say N.
> -endmenu
> +endif
>  
> -menu "SH CPU Frequency scaling"
> -depends on SUPERH
> +if SUPERH
>  config SH_CPU_FREQ
>  	tristate "SuperH CPU Frequency driver"
>  	help
> @@ -293,7 +279,7 @@ config SH_CPU_FREQ
>  	  For details, take a look at <file:Documentation/cpu-freq>.
>  
>  	  If unsure, say N.
> -endmenu
> +endif
>  
>  endif
>  endmenu
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] cpufreq: Remove architecture specific menu entries
  2014-11-13  1:43   ` Scott Wood
@ 2014-11-13  5:26     ` Viresh Kumar
  2014-11-13 22:17       ` Rafael J. Wysocki
  0 siblings, 1 reply; 5+ messages in thread
From: Viresh Kumar @ 2014-11-13  5:26 UTC (permalink / raw)
  To: Scott Wood
  Cc: Rafael J. Wysocki, Lists linaro-kernel, linux-pm@vger.kernel.org,
	Yuantian Tang

On 13 November 2014 07:13, Scott Wood <scottwood@freescale.com> wrote:
> On Thu, 2014-11-13 at 02:47 +0100, Rafael J. Wysocki wrote:
>> On Wednesday, November 12, 2014 09:40:43 AM Viresh Kumar wrote:
>> > CPUFreq driver's Kconfig entries are added in Kconfig.<arch> files and they are
>> > all included from the main Kconfig file using a menu entry. This creates another
>> > level of (unnecessary) hierarchy within the menuconfig entries.
>> >
>> > The problem occurs when there are drivers usable across architectures. Either
>> > their config entry is duplicated in all the supported architectures or is put
>> > into the main Kconfig entry. With the later one, we have menuconfig entries for
>> > drivers at two levels then.
>> >
>> > Fix these issues by getting rid of another level of menuconfig entries and
>> > populate all drivers within the main cpufreq menu.
>>
>> Won't this be confusing?
>
> Why would it be confusing?  It's already in a not-too-large "CPU
> Frequency scaling" menu, the names of the individual options should be
> clear enough, and they'd all be grouped at the end of the menu.
>
> FWIW, there's already "Generic DT based cpufreq driver" under that menu.
>
>> Can we at least have a "CPU frequency scaling drivers" menu for drivers
>> the *contents* of which will depend on the architecture?
>
> That works too -- I just don't see the confusion aspect.

After thinking a bit about the concerns raised by both of you, I decided to look
at what others are doing about this. And this is the best I came around:

diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig
index a24d678..230a912 100644
--- a/drivers/cpufreq/Kconfig
+++ b/drivers/cpufreq/Kconfig
@@ -183,6 +183,8 @@ config CPU_FREQ_GOV_CONSERVATIVE

          If in doubt, say N.

+comment "CPU frequency scaling drivers"
+
 config CPUFREQ_DT
        tristate "Generic DT based cpufreq driver"
        depends on HAVE_CLK && OF


And this is how it looks in menuconfig now:

        [*] CPU Frequency scaling
        <*>   CPU frequency translation statistics (NEW)
        [ ]     CPU frequency translation statistics details (NEW)
              Default CPUFreq governor (performance)  --->
        -*-   'performance' governor
        < >   'powersave' governor (NEW)
        < >   'userspace' governor for userspace frequency scaling
(NEW)
        < >   'ondemand' cpufreq policy governor (NEW)
        < >   'conservative' cpufreq governor (NEW)
              *** CPU frequency scaling drivers ***
        < >   Generic DT based cpufreq driver (NEW)
        < >   Generic ARM big LITTLE CPUfreq driver (NEW)
        [*]   SAMSUNG EXYNOS4210 (NEW)
        [*]   SAMSUNG EXYNOS4x12 (NEW)
        [*]   SAMSUNG EXYNOS5250 (NEW)
        [*]   SAMSUNG EXYNOS5440 (NEW)
        [ ]   EXYNOS Frequency Overclocking - Software (NEW)


Good enough ?

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] cpufreq: Remove architecture specific menu entries
  2014-11-13  5:26     ` Viresh Kumar
@ 2014-11-13 22:17       ` Rafael J. Wysocki
  0 siblings, 0 replies; 5+ messages in thread
From: Rafael J. Wysocki @ 2014-11-13 22:17 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Scott Wood, Lists linaro-kernel, linux-pm@vger.kernel.org,
	Yuantian Tang

On Thursday, November 13, 2014 10:56:53 AM Viresh Kumar wrote:
> On 13 November 2014 07:13, Scott Wood <scottwood@freescale.com> wrote:
> > On Thu, 2014-11-13 at 02:47 +0100, Rafael J. Wysocki wrote:
> >> On Wednesday, November 12, 2014 09:40:43 AM Viresh Kumar wrote:
> >> > CPUFreq driver's Kconfig entries are added in Kconfig.<arch> files and they are
> >> > all included from the main Kconfig file using a menu entry. This creates another
> >> > level of (unnecessary) hierarchy within the menuconfig entries.
> >> >
> >> > The problem occurs when there are drivers usable across architectures. Either
> >> > their config entry is duplicated in all the supported architectures or is put
> >> > into the main Kconfig entry. With the later one, we have menuconfig entries for
> >> > drivers at two levels then.
> >> >
> >> > Fix these issues by getting rid of another level of menuconfig entries and
> >> > populate all drivers within the main cpufreq menu.
> >>
> >> Won't this be confusing?
> >
> > Why would it be confusing?  It's already in a not-too-large "CPU
> > Frequency scaling" menu, the names of the individual options should be
> > clear enough, and they'd all be grouped at the end of the menu.
> >
> > FWIW, there's already "Generic DT based cpufreq driver" under that menu.
> >
> >> Can we at least have a "CPU frequency scaling drivers" menu for drivers
> >> the *contents* of which will depend on the architecture?
> >
> > That works too -- I just don't see the confusion aspect.
> 
> After thinking a bit about the concerns raised by both of you, I decided to look
> at what others are doing about this. And this is the best I came around:
> 
> diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig
> index a24d678..230a912 100644
> --- a/drivers/cpufreq/Kconfig
> +++ b/drivers/cpufreq/Kconfig
> @@ -183,6 +183,8 @@ config CPU_FREQ_GOV_CONSERVATIVE
> 
>           If in doubt, say N.
> 
> +comment "CPU frequency scaling drivers"
> +
>  config CPUFREQ_DT
>         tristate "Generic DT based cpufreq driver"
>         depends on HAVE_CLK && OF
> 
> 
> And this is how it looks in menuconfig now:
> 
>         [*] CPU Frequency scaling
>         <*>   CPU frequency translation statistics (NEW)
>         [ ]     CPU frequency translation statistics details (NEW)
>               Default CPUFreq governor (performance)  --->
>         -*-   'performance' governor
>         < >   'powersave' governor (NEW)
>         < >   'userspace' governor for userspace frequency scaling
> (NEW)
>         < >   'ondemand' cpufreq policy governor (NEW)
>         < >   'conservative' cpufreq governor (NEW)
>               *** CPU frequency scaling drivers ***
>         < >   Generic DT based cpufreq driver (NEW)
>         < >   Generic ARM big LITTLE CPUfreq driver (NEW)
>         [*]   SAMSUNG EXYNOS4210 (NEW)
>         [*]   SAMSUNG EXYNOS4x12 (NEW)
>         [*]   SAMSUNG EXYNOS5250 (NEW)
>         [*]   SAMSUNG EXYNOS5440 (NEW)
>         [ ]   EXYNOS Frequency Overclocking - Software (NEW)
> 
> 
> Good enough ?

Yes, that works too.

That "EXYNOS Frequency Overclocking" item looks odd, but that's a different
matter ...


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2014-11-13 21:56 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-12  4:10 [PATCH] cpufreq: Remove architecture specific menu entries Viresh Kumar
2014-11-13  1:47 ` Rafael J. Wysocki
2014-11-13  1:43   ` Scott Wood
2014-11-13  5:26     ` Viresh Kumar
2014-11-13 22:17       ` Rafael J. Wysocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).