First announce ourself, then start working. Currently this module reports
itself when all is completed which is not most modules do. Plus some
cosmetic/whitespace cleanups.
Signed-off-by: Ben Castricum <[email protected]>
---
diff --git a/arch/x86/kernel/microcode.c b/arch/x86/kernel/microcode.c
index 69729e3..8ce70ac 100644
--- a/arch/x86/kernel/microcode.c
+++ b/arch/x86/kernel/microcode.c
@@ -5,13 +5,14 @@
* 2006 Shaohua Li <[email protected]>
*
* This driver allows to upgrade microcode on Intel processors
- * belonging to IA-32 family - PentiumPro, Pentium II,
+ * belonging to IA-32 family - PentiumPro, Pentium II,
* Pentium III, Xeon, Pentium 4, etc.
*
- * Reference: Section 8.10 of Volume III, Intel Pentium 4 Manual,
- * Order Number 245472 or free download from:
- *
- * http://developer.intel.com/design/pentium4/manuals/245472.htm
+ * Reference: Section 8.11 of Volume 3a, IA-32 Intel? Architecture
+ * Software Developer's Manual
+ * Order Number 253668 or free download from:
+ *
+ * http://developer.intel.com/design/pentium4/manuals/253668.htm
*
* For more information, go to http://www.urbanmyth.org/microcode
*
@@ -58,12 +59,12 @@
* nature of implementation.
* 1.11 22 Mar 2002 Tigran Aivazian <[email protected]>
* Fix the panic when writing zero-length microcode chunk.
- * 1.12 29 Sep 2003 Nitin Kamble <[email protected]>,
+ * 1.12 29 Sep 2003 Nitin Kamble <[email protected]>,
* Jun Nakajima <[email protected]>
* Support for the microcode updates in the new format.
* 1.13 10 Oct 2003 Tigran Aivazian <[email protected]>
* Removed ->read() method and obsoleted MICROCODE_IOCFREE ioctl
- * because we no longer hold a copy of applied microcode
+ * because we no longer hold a copy of applied microcode
* in kernel memory.
* 1.14 25 Jun 2004 Tigran Aivazian <[email protected]>
* Fix sigmatch() macro to handle old CPUs with pf == 0.
@@ -320,11 +321,11 @@ static void apply_microcode(int cpu)
return;
/* serialize access to the physical write to MSR 0x79 */
- spin_lock_irqsave(µcode_update_lock, flags);
+ spin_lock_irqsave(µcode_update_lock, flags);
/* write microcode via MSR 0x79 */
wrmsr(MSR_IA32_UCODE_WRITE,
- (unsigned long) uci->mc->bits,
+ (unsigned long) uci->mc->bits,
(unsigned long) uci->mc->bits >> 16 >> 16);
wrmsr(MSR_IA32_UCODE_REV, 0, 0);
@@ -341,7 +342,7 @@ static void apply_microcode(int cpu)
return;
}
printk(KERN_INFO "microcode: CPU%d updated from revision "
- "0x%x to 0x%x, date = %08x \n",
+ "0x%x to 0x%x, date = %08x \n",
cpu_num, uci->rev, val[1], uci->mc->hdr.date);
uci->rev = val[1];
}
@@ -534,7 +535,7 @@ static int cpu_request_microcode(int cpu
c->x86, c->x86_model, c->x86_mask);
error = request_firmware(&firmware, name, µcode_pdev->dev);
if (error) {
- pr_debug("microcode: ucode data file %s load failed\n", name);
+ pr_debug("microcode: data file %s load failed\n", name);
return error;
}
buf = firmware->data;
@@ -805,6 +806,9 @@ static int __init microcode_init (void)
{
int error;
+ printk(KERN_INFO
+ "IA-32 Microcode Update Driver: v" MICROCODE_VERSION " <[email protected]>\n");
+
error = microcode_dev_init();
if (error)
return error;
@@ -825,9 +829,6 @@ static int __init microcode_init (void)
}
register_hotcpu_notifier(&mc_cpu_notifier);
-
- printk(KERN_INFO
- "IA-32 Microcode Update Driver: v" MICROCODE_VERSION " <[email protected]>\n");
return 0;
}
On Tue, Jun 10, 2008 at 01:15:12PM +0200, Ben Castricum wrote:
> @@ -805,6 +806,9 @@ static int __init microcode_init (void)
> {
> int error;
>
> + printk(KERN_INFO
> + "IA-32 Microcode Update Driver: v" MICROCODE_VERSION " <[email protected]>\n");
> +
> error = microcode_dev_init();
> if (error)
> return error;
> @@ -825,9 +829,6 @@ static int __init microcode_init (void)
> }
>
> register_hotcpu_notifier(&mc_cpu_notifier);
> -
> - printk(KERN_INFO
> - "IA-32 Microcode Update Driver: v" MICROCODE_VERSION " <[email protected]>\n");
> return 0;
By doing this before the registration of the sysdev, we'll
now be printing this on machines that don't have the microcode
update facility. Pointless spew, for no obvious gain imo.
[Why we go through so many hoops before we check if the CPU is
capable is a mystery to me. It would make more sense to have
that be the first thing that gets checked when this inits]
Dave
--
http://www.codemonkey.org.uk
Dave Jones wrote:
> On Tue, Jun 10, 2008 at 01:15:12PM +0200, Ben Castricum wrote:
> > @@ -805,6 +806,9 @@ static int __init microcode_init (void)
> > {
> > int error;
> >
> > + printk(KERN_INFO
> > + "IA-32 Microcode Update Driver: v" MICROCODE_VERSION " <[email protected]>\n");
> > +
> > error = microcode_dev_init();
> > if (error)
> > return error;
> > @@ -825,9 +829,6 @@ static int __init microcode_init (void)
> > }
> >
> > register_hotcpu_notifier(&mc_cpu_notifier);
> > -
> > - printk(KERN_INFO
> > - "IA-32 Microcode Update Driver: v" MICROCODE_VERSION " <[email protected]>\n");
> > return 0;
>
> By doing this before the registration of the sysdev, we'll
> now be printing this on machines that don't have the microcode
> update facility. Pointless spew, for no obvious gain imo.
>
> [Why we go through so many hoops before we check if the CPU is
> capable is a mystery to me. It would make more sense to have
> that be the first thing that gets checked when this inits]
I would remove the printk:
- it is the only email on my dmesg, along two other copyright
notices (Ingo and Intel). So it seems that the dmesg clean-up
is going on.
- the code of driver is stable, and now it is updated only in
kernel trees, so IMO it is better to use kernel version on
bug report. BTW I don't think people will update the version
number of driver.
- IMO it is enough to printk the microcode loads, not the
driver load.
ciao
cate
On Thu, 12 Jun 2008, Giacomo Catenazzi wrote:
> Dave Jones wrote:
>> On Tue, Jun 10, 2008 at 01:15:12PM +0200, Ben Castricum wrote:
>> > @@ -805,6 +806,9 @@ static int __init microcode_init (void)
>> > {
>> > int error;
>> >
>> > + printk(KERN_INFO
>> > + "IA-32 Microcode Update Driver: v" MICROCODE_VERSION " <[email protected]>\n");
>> > +
>> > error = microcode_dev_init();
>> > if (error)
>> > return error;
>> > @@ -825,9 +829,6 @@ static int __init microcode_init (void)
>> > }
>> >
>> > register_hotcpu_notifier(&mc_cpu_notifier);
>> > -
>> > - printk(KERN_INFO
>> > - "IA-32 Microcode Update Driver: v" MICROCODE_VERSION " <[email protected]>\n");
>> > return 0;
>>
>> By doing this before the registration of the sysdev, we'll
>> now be printing this on machines that don't have the microcode
>> update facility. Pointless spew, for no obvious gain imo.
>>
>> [Why we go through so many hoops before we check if the CPU is
>> capable is a mystery to me. It would make more sense to have
>> that be the first thing that gets checked when this inits]
>
> I would remove the printk:
> - it is the only email on my dmesg, along two other copyright
> notices (Ingo and Intel). So it seems that the dmesg clean-up
> is going on.
> - the code of driver is stable, and now it is updated only in
> kernel trees, so IMO it is better to use kernel version on
> bug report. BTW I don't think people will update the version
> number of driver.
> - IMO it is enough to printk the microcode loads, not the
> driver load.
I would prefer Dave's suggestion, i.e. doing the printk after the
initialization.
Btw, it is not the only email, you forgot the "root@<localmachine>" in the
kernel utsname printout :)
$ dmesg | grep @
Linux version 2.6.24.4 (root@elpis) (gcc version 4.1.1 20061011 (Red Hat 4.1.1-30)) #1 SMP Thu Mar 27 17:24:19 GMT 2008
If you want so much to remove something you may remove my email address
from the printk, but please leave the basic message about driver loading.
This way we know that the driver initialized itself and loaded
successfully.
Kind regards
Tigran
* Ben Castricum <[email protected]> wrote:
> First announce ourself, then start working. Currently this module
> reports itself when all is completed which is not most modules do.
> Plus some cosmetic/whitespace cleanups.
>
> Signed-off-by: Ben Castricum <[email protected]>
applied to tip/x86/cleanups - thanks Ben,
Ingo