[PATCH] Kconfig powernow-k8 driver should depend on ACPI P-States driver

[PATCH] Kconfig powernow-k8 driver should depend on ACPI P-States driver

[Joshua Hoblitt]

Hello,

Below is a one line patch to possibly fix this bug:

http://bugs.gentoo.org/show_bug.cgi?id=178585
http://bugzilla.kernel.org/show_bug.cgi?id=8075

If the kernel is configured with:

CONFIG_X86_POWERNOW_K8=y
CONFIG_X86_ACPI_CPUFREQ=m

Which is currently an allowed configuration, the powernow-k8 driver on
an SMP system will fail with a warning like:

powernow-k8: Found 4 Dual Core AMD Opteron(tm) Processor 285 processors (version 2.00.00)
powernow-k8: MP systems not supported by PSB BIOS structure
powernow-k8: MP systems not supported by PSB BIOS structure
powernow-k8: MP systems not supported by PSB BIOS structure
powernow-k8: MP systems not supported by PSB BIOS structure

Which mimics that failure you get when powernow/cool'n'quiet is disabled
in the BIOS.  I don't know if this config combination is valid on a
uniprocessor system so this dependency may need to be enforced only if
SMP in enabled.  The other powernow-* drivers likely have the same
requirements as -k8 but I am unable to test them.

Signed-off-by: Joshua Hoblitt <jhoblitt@ifa.hawaii.edu>
--
 Kconfig |    1 +
 1 file changed, 1 insertion(+)

--- linux-2.6.21.1.orig/arch/i386/kernel/cpu/cpufreq/Kconfig    2007-04-27 11:49:26.000000000 -1000
+++ linux-2.6.21.1/arch/i386/kernel/cpu/cpufreq/Kconfig 2007-05-15 14:48:50.000000000 -1000
@@ -82,6 +82,7 @@ config X86_POWERNOW_K8
        tristate "AMD Opteron/Athlon64 PowerNow!"
        select CPU_FREQ_TABLE
        depends on EXPERIMENTAL
+       depends on X86_ACPI_CPUFREQ
        help
          This adds the CPUFreq driver for mobile AMD Opteron/Athlon64 processors.

Re: [PATCH] Kconfig powernow-k8 driver should depend on ACPI P-States driver

[Chuck Ebbert]

[Copying maintainer.]

Joshua Hoblitt wrote:
> Hello,
> 
> Below is a one line patch to possibly fix this bug:
> 
> http://bugs.gentoo.org/show_bug.cgi?id=178585
> http://bugzilla.kernel.org/show_bug.cgi?id=8075
> 
> If the kernel is configured with:
> 
> CONFIG_X86_POWERNOW_K8=y
> CONFIG_X86_ACPI_CPUFREQ=m
> 
> Which is currently an allowed configuration, the powernow-k8 driver on
> an SMP system will fail with a warning like:
> 
> powernow-k8: Found 4 Dual Core AMD Opteron(tm) Processor 285 processors (version 2.00.00)
> powernow-k8: MP systems not supported by PSB BIOS structure
> powernow-k8: MP systems not supported by PSB BIOS structure
> powernow-k8: MP systems not supported by PSB BIOS structure
> powernow-k8: MP systems not supported by PSB BIOS structure
> 
> Which mimics that failure you get when powernow/cool'n'quiet is disabled
> in the BIOS.  I don't know if this config combination is valid on a
> uniprocessor system so this dependency may need to be enforced only if
> SMP in enabled.  The other powernow-* drivers likely have the same
> requirements as -k8 but I am unable to test them.
> 
> Signed-off-by: Joshua Hoblitt <jhoblitt@ifa.hawaii.edu>
> --
>  Kconfig |    1 +
>  1 file changed, 1 insertion(+)
> 
> --- linux-2.6.21.1.orig/arch/i386/kernel/cpu/cpufreq/Kconfig    2007-04-27 11:49:26.000000000 -1000
> +++ linux-2.6.21.1/arch/i386/kernel/cpu/cpufreq/Kconfig 2007-05-15 14:48:50.000000000 -1000
> @@ -82,6 +82,7 @@ config X86_POWERNOW_K8
>         tristate "AMD Opteron/Athlon64 PowerNow!"
>         select CPU_FREQ_TABLE
>         depends on EXPERIMENTAL
> +       depends on X86_ACPI_CPUFREQ
>         help
>           This adds the CPUFreq driver for mobile AMD Opteron/Athlon64 processors.
> 
> 

Re: [PATCH] Kconfig powernow-k8 driver should depend on ACPI P-States driver

[Dave Jones]

<dropping trivial@ from cc: as this definitly isn't>

On Tue, May 15, 2007 at 03:17:11PM -1000, Joshua Hoblitt wrote:
 
 > --- linux-2.6.21.1.orig/arch/i386/kernel/cpu/cpufreq/Kconfig    2007-04-27 11:49:26.000000000 -1000
 > +++ linux-2.6.21.1/arch/i386/kernel/cpu/cpufreq/Kconfig 2007-05-15 14:48:50.000000000 -1000
 > @@ -82,6 +82,7 @@ config X86_POWERNOW_K8
 >         tristate "AMD Opteron/Athlon64 PowerNow!"
 >         select CPU_FREQ_TABLE
 >         depends on EXPERIMENTAL
 > +       depends on X86_ACPI_CPUFREQ
 >         help
 >           This adds the CPUFreq driver for mobile AMD Opteron/Athlon64 processors.

This is completely the wrong thing to do.
There's no dependancy between these two modules, as they are two independant
cpufreq drivers.  I think you may be confused with X86_POWERNOW_K8_ACPI
(Which should get set if you have ACPI_PROCESSOR enabled)

	Dave

-- 
http://www.codemonkey.org.uk

Re: [PATCH] Kconfig powernow-k8 driver should depend on ACPI P-States driver

[Prakash Punnoor]

[-- Attachment #1: Type: text/plain, Size: 820 bytes --]

Am Mittwoch 16 Mai 2007 schrieb Joshua Hoblitt:
> Hello,
>
> Below is a one line patch to possibly fix this bug:
>
> http://bugs.gentoo.org/show_bug.cgi?id=178585
> http://bugzilla.kernel.org/show_bug.cgi?id=8075
>
> If the kernel is configured with:
>
> CONFIG_X86_POWERNOW_K8=y
> CONFIG_X86_ACPI_CPUFREQ=m
>
> Which is currently an allowed configuration, the powernow-k8 driver on
> an SMP system will fail with a warning like:

NAK. I have an Athlon X2 and it works for me w/o the P states driver compiled 
in.

YOu probably should ask the vendor to fix the bios or simply compile in the p 
states driver additionally w/o forcing the user to do it.

Maybe you want to give a hint in the p states driver help text?
-- 
(°=                 =°)
//\ Prakash Punnoor /\\
V_/                 \_V

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Kconfig powernow-k8 driver should depend on ACPI P-States driver

[Duane Griffin]

On 16/05/07, Prakash Punnoor <prakash@punnoor.de> wrote:
> Maybe you want to give a hint in the p states driver help text?

I think a hint is the right thing to do, but in the PowerNow! driver
rather than the p states one. How about adding something like this to
the X86_POWERNOW_K8 (and X86_POWERNOW_K7?) help text:

"ACPI support is required for non-UP systems and requires ACPI_PROCESSOR
 to be selected. If ACPI_PROCESSOR is compiled as a module then this
 option must be too in order for ACPI support to be available."

Cheers,
Duane.

-- 
"I never could learn to drink that blood and call it wine" - Bob Dylan

Re: [PATCH] Kconfig powernow-k8 driver should depend on ACPI P-States driver

[Daniel Drake]

Duane Griffin wrote:
> On 16/05/07, Prakash Punnoor <prakash@punnoor.de> wrote:
>> Maybe you want to give a hint in the p states driver help text?
> 
> I think a hint is the right thing to do, but in the PowerNow! driver
> rather than the p states one. How about adding something like this to
> the X86_POWERNOW_K8 (and X86_POWERNOW_K7?) help text:

Please hold off this patch until we've gained a more complete 
understanding of the issues and workarounds (which will probably lead to 
us writing a better patch anyway)

Daniel

Re: [PATCH] Kconfig powernow-k8 driver should depend on ACPI P-States driver

[Dave Jones]

On Wed, May 16, 2007 at 08:53:13PM +0100, Duane Griffin wrote:
 > On 16/05/07, Prakash Punnoor <prakash@punnoor.de> wrote:
 > > Maybe you want to give a hint in the p states driver help text?
 > 
 > I think a hint is the right thing to do, but in the PowerNow! driver
 > rather than the p states one. How about adding something like this to
 > the X86_POWERNOW_K8 (and X86_POWERNOW_K7?) help text:

The mobile K7s which had powernow support weren't SMP capable, so they're
irrelevant.

 > "ACPI support is required for non-UP systems and requires ACPI_PROCESSOR
 >  to be selected. If ACPI_PROCESSOR is compiled as a module then this
 >  option must be too in order for ACPI support to be available."

X86_POWERNOW_K8_ACPI is already 'default y'. I think the problem lies in
that people aren't enabling its dependancy, ACPI_PROCESSOR.

We want something along the lines of..

config X86_POWERNOW_K8_ACPI
	bool
	if SMP & X86_POWERNOW_K8_ACPI
	  select ACPI_PROCESSOR

kconfig language quirks aside..


	Dave

-- 
http://www.codemonkey.org.uk

Re: [PATCH] Kconfig powernow-k8 driver should depend on ACPI P-States driver

[Joshua Hoblitt]

[-- Attachment #1: Type: text/plain, Size: 926 bytes --]

On Wed, May 16, 2007 at 04:04:40PM -0400, Daniel Drake wrote:
> Duane Griffin wrote:
> >On 16/05/07, Prakash Punnoor <prakash@punnoor.de> wrote:
> >>Maybe you want to give a hint in the p states driver help text?
> >
> >I think a hint is the right thing to do, but in the PowerNow! driver
> >rather than the p states one. How about adding something like this to
> >the X86_POWERNOW_K8 (and X86_POWERNOW_K7?) help text:
> 
> Please hold off this patch until we've gained a more complete 
> understanding of the issues and workarounds (which will probably lead to 
> us writing a better patch anyway)

I agree - the issue is more complicated that I had thought.  I received
an off list message saying that this dependency isn't required on a
single-socket Athlon X2 system.  So it sounds like isn't an SMP setup
issue as much as it is a multi-socket or perhaps just an issue with my
vendor (Tyan).

-J

--

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Kconfig powernow-k8 driver should depend on ACPI P-States driver

[Ed Sweetman]

Joshua Hoblitt wrote:
> On Wed, May 16, 2007 at 04:04:40PM -0400, Daniel Drake wrote:
>   
>> Duane Griffin wrote:
>>     
>>> On 16/05/07, Prakash Punnoor <prakash@punnoor.de> wrote:
>>>       
>>>> Maybe you want to give a hint in the p states driver help text?
>>>>         
>>> I think a hint is the right thing to do, but in the PowerNow! driver
>>> rather than the p states one. How about adding something like this to
>>> the X86_POWERNOW_K8 (and X86_POWERNOW_K7?) help text:
>>>       
>> Please hold off this patch until we've gained a more complete 
>> understanding of the issues and workarounds (which will probably lead to 
>> us writing a better patch anyway)
>>     
>
> I agree - the issue is more complicated that I had thought.  I received
> an off list message saying that this dependency isn't required on a
> single-socket Athlon X2 system.  So it sounds like isn't an SMP setup
> issue as much as it is a multi-socket or perhaps just an issue with my
> vendor (Tyan).
>
> -J
>
> --
>   
Like i mentioned off list, the problem here is that cpu freq modules 
dont depend (Kconfig) on CONFIG_ACPI_PROCESSOR, yet they do.  Even the 
help text of CONFIG_ACPI_PROCESSOR says this.  I'm unsure as to why the 
drivers that do, dont make the dependency known to Kconfig, since 
obviously the writer of acpi_processor knew the dependency existed.

Simple fix would be to just include the depends line for 
CONFIG_ACPI_PROCESSOR for the powernow driver, and any others people 
wonna test.   I suspect most people just compile in the acpi_processor 
driver, never seeing this problem occur.

Re: [PATCH] Kconfig powernow-k8 driver should depend on ACPI P-States driver

[Daniel Drake]

Ed Sweetman wrote:
> Like i mentioned off list, the problem here is that cpu freq modules 
> dont depend (Kconfig) on CONFIG_ACPI_PROCESSOR, yet they do.

Not really. Firstly, some of the cpufreq modules *do* depend on 
CONFIG_ACPI_PROCESSOR. Secondly, the ones that don't have an existing 
dependency do not actually depend on ACPI_PROCESSOR in some/most 
configurations.

I'll send in a patch to fix the real problem soon.

Daniel

Re: [PATCH] Kconfig powernow-k8 driver should depend on ACPI P-States driver

[Ed Sweetman]

Daniel Drake wrote:
> Ed Sweetman wrote:
>> Like i mentioned off list, the problem here is that cpu freq modules 
>> dont depend (Kconfig) on CONFIG_ACPI_PROCESSOR, yet they do.
>
> Not really. Firstly, some of the cpufreq modules *do* depend on 
> CONFIG_ACPI_PROCESSOR. Secondly, the ones that don't have an existing 
> dependency do not actually depend on ACPI_PROCESSOR in some/most 
> configurations.
>
> I'll send in a patch to fix the real problem soon.
>
> Daniel

The way i patched it was to just include a "select ACPI_PROCESSOR" in
the X86_POWERNOW_K8 Kconfig entry.

Since the "driver" that the user sees in the Kconfig is X86_POWERNOW_K8
is actually not a driver at all, our actual driver behaves differently
since the "pseudo" driver only depends is CPUFREQ.  This is misleading,
as the actual driver beneath it, depends on ACPI_PROCESSOR too.  Now the
driver beneath it responds as you'd think.  It disables itself when it's
depends lines are invalid.  Which is good. But the menu entry that we
see is for X86_POWERNOW_K8, and that isn't disabled or anything when
those depends lines of the driver it actually represents fails.


This is easily fixed with the select line in the "pseudo" driver
...which i find a little more appropriate than a depends line.


As for actual module dependency issues, i haven't bothered looking into
that.   As far as the Kconfig shows it shouldn't be allowed to have
ACPI_PROCESSOR as a module at all. So maybe that's intended.

Re: [PATCH] Kconfig powernow-k8 driver should depend on ACPI P-States driver

[Joshua Hoblitt]

[-- Attachment #1: Type: text/plain, Size: 2298 bytes --]

I think it's pretty clear that Dave and Daniel were both correct and
that ACPI_PROCESSOR is the correct dependency for multi-socket systems.
However, it's worth noting that this dependency seems to be unrelated to
SMP support.  Ed Sweetman has reported that his single-socket but
multi-core system doesn't require ACPI_PROCESSOR for powernow support.
I just tried booting 2.6.21 w/o SMP support and w/o ACPI_PROCESSOR on
one of my multi-socket/multi-core systems.  Sure enough, powernow 
won't work without ACPI_PROCESSOR:

powernow-k8: Found 1 Dual-Core AMD Opteron(tm) Processor 2220 processors (version 2.00.00)
powernow-k8: BIOS error - no PSB or ACPI _PSS objects

I suppose this means that the BIOS does something different to enable
SMP on a multi-core single socket and multi-socket systems.  Anyways, I
believe the question that needs to be answered is: is it reasonable for
X86_POWERNOW_K8 to select ACPI_PROCESSOR if SMP is set?  I'm not sure we
can do anything more intelligent unless Kconfig had more knowledge of
how the hardware other than just SMP/!SMP.

-J

--
On Wed, May 16, 2007 at 04:48:07PM -0400, Dave Jones wrote:
> On Wed, May 16, 2007 at 08:53:13PM +0100, Duane Griffin wrote:
>  > On 16/05/07, Prakash Punnoor <prakash@punnoor.de> wrote:
>  > > Maybe you want to give a hint in the p states driver help text?
>  > 
>  > I think a hint is the right thing to do, but in the PowerNow! driver
>  > rather than the p states one. How about adding something like this to
>  > the X86_POWERNOW_K8 (and X86_POWERNOW_K7?) help text:
> 
> The mobile K7s which had powernow support weren't SMP capable, so they're
> irrelevant.
> 
>  > "ACPI support is required for non-UP systems and requires ACPI_PROCESSOR
>  >  to be selected. If ACPI_PROCESSOR is compiled as a module then this
>  >  option must be too in order for ACPI support to be available."
> 
> X86_POWERNOW_K8_ACPI is already 'default y'. I think the problem lies in
> that people aren't enabling its dependancy, ACPI_PROCESSOR.
> 
> We want something along the lines of..
> 
> config X86_POWERNOW_K8_ACPI
> 	bool
> 	if SMP & X86_POWERNOW_K8_ACPI
> 	  select ACPI_PROCESSOR
> 
> kconfig language quirks aside..
> 
> 
> 	Dave
> 
> -- 
> http://www.codemonkey.org.uk

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Kconfig powernow-k8 driver should depend on ACPI P-States driver

[Ed Sweetman]

Joshua Hoblitt wrote:
> I think it's pretty clear that Dave and Daniel were both correct and
> that ACPI_PROCESSOR is the correct dependency for multi-socket systems.
> However, it's worth noting that this dependency seems to be unrelated to
> SMP support.  Ed Sweetman has reported that his single-socket but
> multi-core system doesn't require ACPI_PROCESSOR for powernow support.
> I just tried booting 2.6.21 w/o SMP support and w/o ACPI_PROCESSOR on
> one of my multi-socket/multi-core systems.  Sure enough, powernow 
> won't work without ACPI_PROCESSOR:
>
>   

I said before and originally, in response to the original post by you, 
that i didn't need acpi p-states driver, which is what this thread was 
suggesting.  We later determined that the thread title was misleading, 
this thread should be titled   "Kconfig powernow-k8 driver should depend 
on ACPI_PROCESSOR driver"

I've always compiled acpi_processor in the kernel, i use acpi, i have a 
processor, it seemed to fit.
> powernow-k8: Found 1 Dual-Core AMD Opteron(tm) Processor 2220 processors (version 2.00.00)
> powernow-k8: BIOS error - no PSB or ACPI _PSS objects
>
> I suppose this means that the BIOS does something different to enable
> SMP on a multi-core single socket and multi-socket systems.  Anyways, I
> believe the question that needs to be answered is: is it reasonable for
> X86_POWERNOW_K8 to select ACPI_PROCESSOR if SMP is set?  I'm not sure we
> can do anything more intelligent unless Kconfig had more knowledge of
> how the hardware other than just SMP/!SMP.
>
> -J
>
>   

My patch does the most intelligent thing thus far, it lets the user 
decide.  If the user is smart enough to enable cpufreq's powernow-k8 
driver, then the user is smart enough to decide if he wants/needs to use 
the acpi support of that driver.    I'm just exposing the driver that is 
causing all of this confusion to the user, since this is the driver that 
gets enabled/disabled behind the scenes depending on acpi_processor 
being selected.  Now it's not automatically handled, the user can enable 
acpi support or disable it (in the powernow driver), even if 
acpi_processor is compiled in.

you should be able to use the acpi support of Powernow-k8 even in UP 
situations, so only enabling it for SMP is wrong.  Enabling it whenever 
you have acpi compiled in and acpi_processor compiled in is wrong, 
because the user may be using a UP system and not want to lookup the 
acpi tables for powernow for some ungodly reason.   Just allowing the 
user to handle it works best.  See my previous post for the patch. 

> --
> On Wed, May 16, 2007 at 04:48:07PM -0400, Dave Jones wrote:
>   
>> On Wed, May 16, 2007 at 08:53:13PM +0100, Duane Griffin wrote:
>>  > On 16/05/07, Prakash Punnoor <prakash@punnoor.de> wrote:
>>  > > Maybe you want to give a hint in the p states driver help text?
>>  > 
>>  > I think a hint is the right thing to do, but in the PowerNow! driver
>>  > rather than the p states one. How about adding something like this to
>>  > the X86_POWERNOW_K8 (and X86_POWERNOW_K7?) help text:
>>
>> The mobile K7s which had powernow support weren't SMP capable, so they're
>> irrelevant.
>>
>>  > "ACPI support is required for non-UP systems and requires ACPI_PROCESSOR
>>  >  to be selected. If ACPI_PROCESSOR is compiled as a module then this
>>  >  option must be too in order for ACPI support to be available."
>>
>> X86_POWERNOW_K8_ACPI is already 'default y'. I think the problem lies in
>> that people aren't enabling its dependancy, ACPI_PROCESSOR.
>>
>> We want something along the lines of..
>>
>> config X86_POWERNOW_K8_ACPI
>> 	bool
>> 	if SMP & X86_POWERNOW_K8_ACPI
>> 	  select ACPI_PROCESSOR
>>
>> kconfig language quirks aside..
>>
>>
>> 	Dave
>>
>> -- 
>> http://www.codemonkey.org.uk
>>     

Re: [PATCH] Kconfig powernow-k8 driver should depend on ACPI P-States driver

[Ed Sweetman]

the previous post i keep referring you to has a patch that was mangled 
...here is the non-mangled version

--- ./linux-backup/arch/x86_64/kernel/cpufreq/Kconfig   2007-02-04 
13:44:54.000000000 -0500
+++ ./linux-2.6.21-rc5-mm2/arch/x86_64/kernel/cpufreq/Kconfig 
2007-05-17 18:13:07.000000000 -0400
@@ -10,20 +10,27 @@

  comment "CPUFreq processor drivers"

-config X86_POWERNOW_K8
+config  X86_POWERNOW_K8
         tristate "AMD Opteron/Athlon64 PowerNow!"
         select CPU_FREQ_TABLE
         help
           This adds the CPUFreq driver for mobile AMD Opteron/Athlon64 
processors.
+         An acpi interface is available if acpi support has been selected.
+         This is required for multi-socket and other systems but not 
necessarily required for UP single socket systems.

           For details, take a look at <file:Documentation/cpu-freq/>.

           If in doubt, say N.

  config X86_POWERNOW_K8_ACPI
-       bool
-       depends on X86_POWERNOW_K8 && ACPI_PROCESSOR
-       depends on !(X86_POWERNOW_K8 = y && ACPI_PROCESSOR = m)
+       bool "ACPI Support"
+       select ACPI_PROCESSOR
+       depends on X86_POWERNOW_K8
+       help
+           This provides access to the acpi tables for full p-state 
functionality. This driver is also required
+           for cpufreq to work with multi-socket and other smp systems.
+
+           It is safe to say Y here.
         default y

  config X86_SPEEDSTEP_CENTRINO

Re: [PATCH] Kconfig powernow-k8 driver should depend on ACPI P-States driver

[Dave Jones]

On Fri, May 18, 2007 at 12:09:38AM -0400, Ed Sweetman wrote:
 > the previous post i keep referring you to has a patch that was mangled 
 > ...here is the non-mangled version
 > 
 > --- ./linux-backup/arch/x86_64/kernel/cpufreq/Kconfig   2007-02-04 
 > 13:44:54.000000000 -0500
 > +++ ./linux-2.6.21-rc5-mm2/arch/x86_64/kernel/cpufreq/Kconfig 
 > 2007-05-17 18:13:07.000000000 -0400
 > @@ -10,20 +10,27 @@
 > 
 >   comment "CPUFreq processor drivers"
 > 
 > -config X86_POWERNOW_K8
 > +config  X86_POWERNOW_K8
 >          tristate "AMD Opteron/Athlon64 PowerNow!"

still has unnecessary whitespace changes

 >          select CPU_FREQ_TABLE
 >          help
 >            This adds the CPUFreq driver for mobile AMD Opteron/Athlon64 
 > processors.
 > +         An acpi interface is available if acpi support has been selected.
 > +         This is required for multi-socket and other systems but not 
 > necessarily required for UP single socket systems.

and still wordwrapped.
(also capitalise ACPI)

	Dave

-- 
http://www.codemonkey.org.uk

Re: [PATCH] Kconfig powernow-k8 driver should depend on ACPI P-States driver

[Joshua Hoblitt]

[-- Attachment #1: Type: text/plain, Size: 2897 bytes --]

On Fri, May 18, 2007 at 12:01:08PM -0400, Dave Jones wrote:
> On Fri, May 18, 2007 at 12:09:38AM -0400, Ed Sweetman wrote:
[snip]
> still has unnecessary whitespace changes
[snip]
> and still wordwrapped.
> (also capitalise ACPI)

I haven't seen any more e-mail traffic on this topic so I'm assuming
that the ball has been dropped.  Please excuse me if an acceptable patch
has been submitted that I wasn't CC'd on. 

Here is cleaned up version of Ed's patch that I believe addresses Dave's
stylistic concerns applies the relevant changes to both x86 & x86_64.

Signed-off-by: Joshua Hoblitt <jhoblitt@ifa.hawaii.edu>
--
 i386/kernel/cpu/cpufreq/Kconfig |   13 ++++++++++---
 x86_64/kernel/cpufreq/Kconfig   |   13 ++++++++++---
 2 files changed, 20 insertions(+), 6 deletions(-)

diff -Nurp linux-2.6.22-rc1-mm1.orig/arch/i386/kernel/cpu/cpufreq/Kconfig linux-2.6.22-rc1-mm1/arch/i386/kernel/cpu/cpufreq/Kconfig
--- linux-2.6.22-rc1-mm1.orig/arch/i386/kernel/cpu/cpufreq/Kconfig	2007-04-27 11:49:26.000000000 -1000
+++ linux-2.6.22-rc1-mm1/arch/i386/kernel/cpu/cpufreq/Kconfig	2007-05-21 16:20:47.000000000 -1000
@@ -90,10 +90,17 @@ config X86_POWERNOW_K8
 	  If in doubt, say N.
 
 config X86_POWERNOW_K8_ACPI
-	bool
-	depends on X86_POWERNOW_K8 && ACPI_PROCESSOR
-	depends on !(X86_POWERNOW_K8 = y && ACPI_PROCESSOR = m)
+	bool "ACPI Support"
+	select ACPI_PROCESSOR
+	depends on X86_POWERNOW_K8
 	default y
+	help
+	  This provides access to the K8s Processor Performance States via ACPI.
+	  This driver is probably required for CPUFreq to work with multi-socket and
+	  SMP systems.  It is not required on at least some single-socket yet
+	  multi-core systems, even if SMP is enabled.
+
+	  It is safe to say Y here.
 
 config X86_GX_SUSPMOD
 	tristate "Cyrix MediaGX/NatSemi Geode Suspend Modulation"
diff -Nurp linux-2.6.22-rc1-mm1.orig/arch/x86_64/kernel/cpufreq/Kconfig linux-2.6.22-rc1-mm1/arch/x86_64/kernel/cpufreq/Kconfig
--- linux-2.6.22-rc1-mm1.orig/arch/x86_64/kernel/cpufreq/Kconfig	2007-05-21 16:11:16.000000000 -1000
+++ linux-2.6.22-rc1-mm1/arch/x86_64/kernel/cpufreq/Kconfig	2007-05-21 16:29:11.000000000 -1000
@@ -24,10 +24,17 @@ config X86_POWERNOW_K8
 	  If in doubt, say N.
 
 config X86_POWERNOW_K8_ACPI
-	bool
-	depends on X86_POWERNOW_K8 && ACPI_PROCESSOR
-	depends on !(X86_POWERNOW_K8 = y && ACPI_PROCESSOR = m)
+	bool "ACPI Support"
+	select ACPI_PROCESSOR
+	depends on X86_POWERNOW_K8
 	default y
+	help
+	  This provides access to the K8s Processor Performance States via ACPI.
+	  This driver is probably required for CPUFreq to work with multi-socket and
+	  SMP systems.  It is not required on at least some single-socket yet
+	  multi-core systems, even if SMP is enabled.
+	
+	  It is safe to say Y here.
 
 config X86_SPEEDSTEP_CENTRINO
 	tristate "Intel Enhanced SpeedStep (deprecated)"

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Kconfig powernow-k8 driver should depend on ACPI P-States driver

[Dave Jones]

On Mon, May 21, 2007 at 04:47:43PM -1000, Joshua Hoblitt wrote:

apologies for the delay in getting back to this.

 > Here is cleaned up version of Ed's patch that I believe addresses Dave's
 > stylistic concerns applies the relevant changes to both x86 & x86_64.

The x86-64 version is a symlink to the i386 file, so is unnecessary.
(That trap catches people out regularly).

 > Signed-off-by: Joshua Hoblitt <jhoblitt@ifa.hawaii.edu>

The patch content looks ok to me, Daniel, ack?


 > diff -Nurp linux-2.6.22-rc1-mm1.orig/arch/i386/kernel/cpu/cpufreq/Kconfig l=
 > inux-2.6.22-rc1-mm1/arch/i386/kernel/cpu/cpufreq/Kconfig
 > --- linux-2.6.22-rc1-mm1.orig/arch/i386/kernel/cpu/cpufreq/Kconfig  2007-04-=
 > 27 11:49:26.000000000 -1000
 > +++ linux-2.6.22-rc1-mm1/arch/i386/kernel/cpu/cpufreq/Kconfig   2007-05-21 16=
 > :20:47.000000000 -1000
 > @@ -90,10 +90,17 @@ config X86_POWERNOW_K8
 >       If in doubt, say N.
 > =20
 >  config X86_POWERNOW_K8_ACPI
 > -   bool
 > -   depends on X86_POWERNOW_K8 && ACPI_PROCESSOR
 > -   depends on !(X86_POWERNOW_K8 =3D y && ACPI_PROCESSOR =3D m)
 > +   bool "ACPI Support"

MIME damage prevents this from applying.

	Dave

-- 
http://www.codemonkey.org.uk

Re: [PATCH] Kconfig powernow-k8 driver should depend on ACPI P-States driver

[Daniel Drake]

Dave Jones wrote:
> The patch content looks ok to me, Daniel, ack?

Works for me, thanks.

Daniel