Skip to contentSkip to conversation

cpufreq: correct initialization on Intel Coppermines

PATCH In review Dominik Brodowski 3 participants 6 messages Nov 8, 2002 – Jun 15, 2003 3 messages not in the archive Driver core

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 cpufreq: correct initialization on Intel Coppermines

[2.5. PATCH] cpufreq: Intel Coppermines -- the saga continues.

The detection process for speedstep-enabled Pentium III Coppermines is considered proprietary by Intel. The attempt to detect this capability using MSRs failed. So, users need to pass the option "speedstep_coppermine=1" to the kernel (boot option or parameter) if they own a SpeedStep capable PIII Coppermine processor. Tualatins work as before.

Dominik

arch/i386/kernel/cpu/cpufreq/speedstep.c +36 -23
@@ -1,5 +1,5 @@
 /*
- *  $Id: speedstep.c,v 1.53 2002/09/29 23:43:11 db Exp $
+ *  $Id: speedstep.c,v 1.57 2002/11/05 12:01:12 db Exp $
  *
  * (C) 2001  Dave Jones, Arjan van de ven.
  * (C) 2002  Dominik Brodowski <[email protected]>
@@ -46,7 +46,8 @@
 
 /* speedstep_processor
  */
-static unsigned int                     speedstep_processor;
+static unsigned int                     speedstep_processor = 0;
+static int                              speedstep_coppermine = 0;
 
 #define SPEEDSTEP_PROCESSOR_PIII_C      0x00000001  /* Coppermine core */
 #define SPEEDSTEP_PROCESSOR_PIII_T      0x00000002  /* Tualatin core */
@@ -72,7 +73,7 @@
 #ifdef SPEEDSTEP_DEBUG
 #define dprintk(msg...) printk(msg)
 #else
-#define dprintk(msg...) do { } while(0);
+#define dprintk(msg...) do { } while(0)
 #endif
 
 
@@ -429,7 +430,7 @@
 /** 
  * speedstep_detect_processor - detect Intel SpeedStep-capable processors.
  *
- *   Returns the SPEEDSTEP_PROCESSOR_-number for the detected chipset, 
+ *   Returns the SPEEDSTEP_PROCESSOR_-number for the detected processor, 
  * or zero on failure.
  */
 static unsigned int speedstep_detect_processor (void)
@@ -474,8 +475,6 @@
 		return SPEEDSTEP_PROCESSOR_PIII_T;
 
 	case 0x08: /* Intel PIII [Coppermine] */
-		/* based on reverse-engineering information and
-		 * some guessing. HANDLE WITH CARE! */
  	        {
 			u32     msr_lo, msr_hi;
 
@@ -487,21 +486,13 @@
 			if (msr_lo != 0x0080000)
 				return 0;
 
-			/* platform ID seems to be 0x00140000 */
-			rdmsr(MSR_IA32_PLATFORM_ID, msr_lo, msr_hi);
-			dprintk(KERN_DEBUG "cpufreq: Coppermine: MSR_IA32_PLATFORM ID is 0x%x, 0x%x\n", msr_lo, msr_hi);
-			msr_hi = msr_lo & 0x001c0000;
-			if (msr_hi != 0x00140000)
-				return 0;
-
-			/* and these bits seem to be either 00_b, 01_b or
-			 * 10_b but never 11_b */
-			msr_lo &= 0x00030000;
-			if (msr_lo == 0x0030000)
-				return 0;
+			if (speedstep_coppermine)
+				return SPEEDSTEP_PROCESSOR_PIII_C;
 
-			/* let's hope this is correct... */
-			return SPEEDSTEP_PROCESSOR_PIII_C;
+			printk(KERN_INFO "cpufreq: in case this is a SpeedStep-capable Intel Pentium III Coppermine\n");
+			printk(KERN_INFO "cpufreq: processor, please pass the boot option or module parameter\n");
+			printk(KERN_INFO "cpufreq: `speedstep_coppermine=1` to the kernel. Thanks!\n");
+			return 0;
 		}
 
 	default:
@@ -622,6 +613,25 @@
 }
 
 
+#ifndef MODULE
+/**
+ * speedstep_setup  speedstep command line parameter parsing
+ *
+ * speedstep command line parameter.  Use:
+ *  speedstep_coppermine=1
+ * if the CPU in your notebook is a SpeedStep-capable Intel
+ * Pentium III Coppermine. These processors cannot be detected
+ * automatically, as Intel continues to consider the detection 
+ * alogrithm as proprietary material.
+ */
+static int __init speedstep_setup(char *str)
+{
+	speedstep_coppermine = simple_strtoul(str, &str, 0);
+	return 1;
+}
+__setup("speedstep_coppermine=", speedstep_setup);
+#endif
+
 /**
  * speedstep_init - initializes the SpeedStep CPUFreq driver
  *
@@ -637,18 +647,19 @@
 
 
 	/* detect chipset */
-	speedstep_chipset = speedstep_detect_chipset();
+	speedstep_chipset = speedstep_detect_chipset(); 
 
 	/* detect chipset */
 	if (speedstep_chipset)
 		speedstep_processor = speedstep_detect_processor();
 
 	if ((!speedstep_chipset) || (!speedstep_processor)) {
-		dprintk(KERN_INFO "cpufreq: Intel(R) SpeedStep(TM) for this %s not (yet) available.\n", speedstep_processor ? "chipset" : "processor");
+		printk(KERN_INFO "a 0x%x b 0x%x\n", speedstep_processor, speedstep_chipset);
+		printk(KERN_INFO "cpufreq: Intel(R) SpeedStep(TM) for this %s not (yet) available.\n", speedstep_chipset ? "processor" : "chipset");
 		return -ENODEV;
 	}
 
-	dprintk(KERN_INFO "cpufreq: Intel(R) SpeedStep(TM) support $Revision: 1.53 $\n");
+	dprintk(KERN_INFO "cpufreq: Intel(R) SpeedStep(TM) support $Revision: 1.57 $\n");
 	dprintk(KERN_DEBUG "cpufreq: chipset 0x%x - processor 0x%x\n", 
 	       speedstep_chipset, speedstep_processor);
 
@@ -726,3 +737,5 @@
 MODULE_LICENSE ("GPL");
 module_init(speedstep_init);
 module_exit(speedstep_exit);
+
+MODULE_PARM (speedstep_coppermine, "i");
#1 lore raw
Quoted 2 lines · Dominik Brodowski

The detection process for speedstep-enabled Pentium III Coppermines is considered proprietary by Intel.

They seem to have changed their mind:

http://www.intel.com/support/processors/sb/cs-003779-prd24.htm

Which looks a bit like what was implemented at first. Here is a patch. I kept the setup parameter, but it might be removed now?

arch/i386/kernel/cpu/cpufreq/speedstep.c +9 -0
@@ -503,6 +503,15 @@
 			if (speedstep_coppermine)
 				return SPEEDSTEP_PROCESSOR_PIII_C;
 
+			/* if the processor is a mobile version,
+			 * platform ID has bit 50 set
+			 * it has SpeedStep technology if either
+			 * bit 56 or 57 is set */
+			rdmsr(MSR_IA32_PLATFORM_ID, msr_lo, msr_hi);
+			dprintk(KERN_DEBUG "cpufreq: Coppermine: MSR_IA32_PLATFORM ID is 0x%x, 0x%x\n", msr_lo, msr_hi);
+			if ((msr_hi & (1<<18)) && (msr_hi & (3<<24)))
+				return SPEEDSTEP_PROCESSOR_PIII_C;
+
 			printk(KERN_INFO "cpufreq: in case this is a SpeedStep-capable Intel Pentium III Coppermine\n");
 			printk(KERN_INFO "cpufreq: processor, please pass the boot option or module parameter\n");
 			printk(KERN_INFO "cpufreq: `speedstep_coppermine=1` to the kernel. Thanks!\n");

A message that is not in the archive (sent off-list or not yet received): [email protected]

A message that is not in the archive (sent off-list or not yet received): [email protected]

A message that is not in the archive (sent off-list or not yet received): [email protected]

#5 lore raw

PATCH 1/1 bug if cpufreq driver initialization fails

Le dim 15 jun 2003 11:50:44 GMT, Dominik Brodowski a tapoté sur son clavier :

Quoted 6 lines
Quoted 2 lines

However, my 440BX chipset is still not supported, and I couldn't find anything in Intel's documentation...

Do you know of Bruno Ducrot's work?

http://www.poupinou.org/cpufreq/

I read the list archive yesterday, and tried this work. I got the results
http://youpibouh.thefreecat.org/speedstep/ac
http://youpibouh.thefreecat.org/speedstep/battery
and hence tried my gpo_hilo=11, and it works... Sometimes.

I seems like the hardware si sometimes slow to get to the low speed. My PIII is a 800Mhz/650Mhz, but initialization sometimes shows 800 and 800, as if the speed measure was done before the hardware actually achieved the slow down.

As a result, speedstep_cpu_init() fails, hence cpufreq_add_dev() fails, which is ignored by sysdev_driver_register(). In the end, the module is kept loaded (since speedstep_init() successes, since cpufreq_register_driver successes()), and I have to unload it by hand, which entails kobject badness warning of course, since cpufreq then tries to remove entries which were actually never added to /sys!

I hence modified drivers/base/sys.c to have sysdev_driver_register() fail as well, and then I also had to modify kernel/cpufreq.c, because this failure did not imply a setting cpufreq_driver to NULL (preventing me from reinsmoding speedstep-ich: EBUSY)

I'd also suggest that the speedstep drivers printks something if everything went ok (including the cpufreq_frequency_table_cpuinfo() call), the low & high speed for instance, just to be sure everything went ok

Regards,
Samuel Thibault

drivers/base/sys.c +7 -3
@@ -116,6 +116,7 @@
 int sysdev_driver_register(struct sysdev_class * cls, 
 			   struct sysdev_driver * drv)
 {
+	int ret = 0;
 	down_write(&system_subsys.rwsem);
 	if (cls && kset_get(&cls->kset)) {
 		list_add_tail(&drv->entry,&cls->drivers);
@@ -123,13 +124,16 @@
 		/* If devices of this class already exist, tell the driver */
 		if (drv->add) {
 			struct sys_device *dev;
-			list_for_each_entry(dev, &cls->kset.list, kobj.entry)
-				drv->add(dev);
+			list_for_each_entry(dev, &cls->kset.list, kobj.entry) {
+				ret = drv->add(dev);
+				if (ret) goto out;
+			}
 		}
 	} else
 		list_add_tail(&drv->entry,&global_drivers);
+out:
 	up_write(&system_subsys.rwsem);
-	return 0;
+	return ret;
 }
 
 
kernel/cpufreq.c +9 -3
@@ -811,6 +811,8 @@
  */
 int cpufreq_register_driver(struct cpufreq_driver *driver_data)
 {
+	int ret;
+
 	if (!driver_data || !driver_data->verify || !driver_data->init ||
 	    ((!driver_data->setpolicy) && (!driver_data->target)))
 		return -EINVAL;
@@ -825,13 +827,17 @@
 
 	cpufreq_driver->policy = kmalloc(NR_CPUS * sizeof(struct cpufreq_policy), GFP_KERNEL);
 	if (!cpufreq_driver->policy) {
-		cpufreq_driver = NULL;
-		return -ENOMEM;
+		ret = -ENOMEM;
+		goto out;
 	}
 
 	memset(cpufreq_driver->policy, 0, NR_CPUS * sizeof(struct cpufreq_policy));
 
-	return sysdev_driver_register(&cpu_sysdev_class,&cpufreq_sysdev_driver);
+	ret = sysdev_driver_register(&cpu_sysdev_class,&cpufreq_sysdev_driver);
+out:
+	if (ret)
+		cpufreq_driver = NULL;
+	return ret;
 }
 EXPORT_SYMBOL_GPL(cpufreq_register_driver);
 
#6 lore raw

Re: [2.5 PATCH] bug if cpufreq driver initialization fails

Quoted 4 lines · Samuel Thibault

I hence modified drivers/base/sys.c to have sysdev_driver_register() fail as well, and then I also had to modify kernel/cpufreq.c, because this failure did not imply a setting cpufreq_driver to NULL (preventing me from reinsmoding speedstep-ich: EBUSY)

Unfortunately, you created a by by doing so. Eg:

- you have 3 devices on kset.list.
- you successfully register 2 of them with a driver.
- you fail one.
- sysdev_driver_register returns failure.
- module is unloaded while other parts of the kernel have references into the driver.
- the kernel oopses.

Quoted 4 lines

I'd also suggest that the speedstep drivers printks something if everything went ok (including the cpufreq_frequency_table_cpuinfo() call), the low & high speed for instance, just to be sure everything went ok

IMO its better to printk something on failure.

Signature · 2 lines
Russell King ([email protected])                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html
#7 lore raw

Re: [2.5 PATCH] bug if cpufreq driver initialization fails

Le dim 15 jun 2003 19:16:50 GMT, Russell King a tapoté sur son clavier :

Quoted 8 lines · Samuel Thibault
Quoted 4 lines

I hence modified drivers/base/sys.c to have sysdev_driver_register() fail as well, and then I also had to modify kernel/cpufreq.c, because this failure did not imply a setting cpufreq_driver to NULL (preventing me from reinsmoding speedstep-ich: EBUSY)

Unfortunately, you created a by by doing so. Eg:

Quoted 7 lines

- you have 3 devices on kset.list.
- you successfully register 2 of them with a driver.
- you fail one.
- sysdev_driver_register returns failure.
- module is unloaded while other parts of the kernel have references into the driver.
- the kernel oopses.

Ok, that's what I was wondering, might an exit loop removing the already added driver be done?

(the second half of the patch might still be applied as soon as now)

Regards,
Samuel Thibault

#8 lore raw

Re: [2.5 PATCH] bug if cpufreq driver initialization fails

Quoted 16 lines · Samuel Thibault

Le dim 15 jun 2003 19:16:50 GMT, Russell King a tapoté sur son clavier :

Quoted 8 lines · Samuel Thibault
Quoted 4 lines

I hence modified drivers/base/sys.c to have sysdev_driver_register() fail as well, and then I also had to modify kernel/cpufreq.c, because this failure did not imply a setting cpufreq_driver to NULL (preventing me from reinsmoding speedstep-ich: EBUSY)

Unfortunately, you created a by by doing so. Eg:

Quoted 7 lines

- you have 3 devices on kset.list.
- you successfully register 2 of them with a driver.
- you fail one.
- sysdev_driver_register returns failure.
- module is unloaded while other parts of the kernel have references into the driver.
- the kernel oopses.

Quoted 4 lines

Ok, that's what I was wondering, might an exit loop removing the already added driver be done?

(the second half of the patch might still be applied as soon as now)

Also, I think people need to consider whether "all devices failed" or "any device failed" counts as overall failure.

Signature · 2 lines
Russell King ([email protected])                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html