[PATCH 2.4] dmi_ident made public

[PATCH 2.4] dmi_ident made public

[Jean Delvare]

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


Hi everyone,

Here is a simple patch for the 2.4 kernel series that makes dmi_ident
(as defined in arch/i386/kernel/dmi_scan.c) public.

The idea is that this array is actually needed by other parts of the
kernel. Known modules in this case are:
 - i8k (drivers/char/i8k.c);
 - i2c-piix4 (from the LM Sensors project [1]);
 - omke (from the OMKE project [2]).
Right now, these modules are scanning the DMI table again, on their own.
This is bad for at least two reasons:
 - waste of time;
 - code duplication.

So, this simple patch is the first step in a simplification process that
would let us remove all duplicated code. It is somehow based on a patch
Soos Peter, the author of OMKE, sent me one month ago, so I have to
credit him here.

If this patch is accepted and applied, I'll work together with Peter to
get the three above-mentioned modules simplified, as well as any other I
may have missed. Also, I'll take care of porting this patch to the 2.5
series, since it also belongs there.

All comments welcome, please CC me. And please apply if it's OK.

[1] http://www.lm-sensors.nu/
[2] http://sourceforge.net/projects/omke/

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

[-- Attachment #2: linux-2.4.21-rc1-publicdmi.diff --]
[-- Type: text/plain, Size: 2383 bytes --]

diff -ruN linux-2.4.21-rc1-orig/arch/i386/kernel/Makefile linux-2.4.21-rc1/arch/i386/kernel/Makefile
--- linux-2.4.21-rc1-orig/arch/i386/kernel/Makefile	2003-04-24 15:56:25.000000000 +0200
+++ linux-2.4.21-rc1/arch/i386/kernel/Makefile	2003-04-24 16:24:58.000000000 +0200
@@ -14,7 +14,8 @@
 
 O_TARGET := kernel.o
 
-export-objs     := mca.o mtrr.o msr.o cpuid.o microcode.o i386_ksyms.o time.o
+export-objs := mca.o mtrr.o msr.o cpuid.o microcode.o i386_ksyms.o time.o \
+               dmi_scan.o
 
 obj-y	:= process.o semaphore.o signal.o entry.o traps.o irq.o vm86.o \
 		ptrace.o i8259.o ioport.o ldt.o setup.o time.o sys_i386.o \
diff -ruN linux-2.4.21-rc1-orig/arch/i386/kernel/dmi_scan.c linux-2.4.21-rc1/arch/i386/kernel/dmi_scan.c
--- linux-2.4.21-rc1-orig/arch/i386/kernel/dmi_scan.c	2003-04-24 16:14:06.000000000 +0200
+++ linux-2.4.21-rc1/arch/i386/kernel/dmi_scan.c	2003-04-24 17:42:49.000000000 +0200
@@ -5,11 +5,13 @@
 #include <linux/init.h>
 #include <linux/apm_bios.h>
 #include <linux/slab.h>
+#include <linux/module.h>
 #include <asm/io.h>
 #include <linux/pm.h>
 #include <asm/keyboard.h>
 #include <asm/system.h>
 #include <linux/bootmem.h>
+#include <linux/dmi.h>
 
 #include "pci-i386.h"
 
@@ -131,22 +133,7 @@
 	return -1;
 }
 
-
-enum
-{
-	DMI_BIOS_VENDOR,
-	DMI_BIOS_VERSION,
-	DMI_BIOS_DATE,
-	DMI_SYS_VENDOR,
-	DMI_PRODUCT_NAME,
-	DMI_PRODUCT_VERSION,
-	DMI_BOARD_VENDOR,
-	DMI_BOARD_NAME,
-	DMI_BOARD_VERSION,
-	DMI_STRING_MAX
-};
-
-static char *dmi_ident[DMI_STRING_MAX];
+char *dmi_ident[DMI_STRING_MAX];
 
 /*
  *	Save a DMI string
@@ -829,7 +816,7 @@
 
 static void __init dmi_decode(struct dmi_header *dm)
 {
-	u8 *data = (u8 *)dm;
+//	u8 *data = (u8 *)dm;
 	
 	switch(dm->type)
 	{
@@ -877,3 +864,5 @@
 	if(err == 0)
 		dmi_check_blacklist();
 }
+
+EXPORT_SYMBOL(dmi_ident);
diff -ruN linux-2.4.21-rc1-orig/include/linux/dmi.h linux-2.4.21-rc1/include/linux/dmi.h
--- linux-2.4.21-rc1-orig/include/linux/dmi.h	1970-01-01 01:00:00.000000000 +0100
+++ linux-2.4.21-rc1/include/linux/dmi.h	2003-04-24 16:32:02.000000000 +0200
@@ -0,0 +1,20 @@
+#ifndef _LINUX_DMI_H
+#define _LINUX_DMI_H
+
+enum
+{
+	DMI_BIOS_VENDOR,
+	DMI_BIOS_VERSION,
+	DMI_BIOS_DATE,
+	DMI_SYS_VENDOR,
+	DMI_PRODUCT_NAME,
+	DMI_PRODUCT_VERSION,
+	DMI_BOARD_VENDOR,
+	DMI_BOARD_NAME,
+	DMI_BOARD_VERSION,
+	DMI_STRING_MAX
+};
+
+extern char *dmi_ident[DMI_STRING_MAX];
+
+#endif

Re: [PATCH 2.4] dmi_ident made public

[Alan Cox]

On Iau, 2003-04-24 at 17:47, Jean Delvare wrote:
> Hi everyone,
> 
> Here is a simple patch for the 2.4 kernel series that makes dmi_ident
> (as defined in arch/i386/kernel/dmi_scan.c) public.

The dmi scanner is __init, its gone after boot. The DMI tables on some
platforms may also have gone. What you actually want I suspect is a way
to register multiple DMI tables for boot time scanning to set further
flags in the dmi properties post scan.

Alan

Re: [PATCH 2.4] dmi_ident made public

[Jean Delvare]

> > Here is a simple patch for the 2.4 kernel series that makes
> > dmi_ident (as defined in arch/i386/kernel/dmi_scan.c) public.
> 
> The dmi scanner is __init, its gone after boot. The DMI tables on some
> platforms may also have gone. What you actually want I suspect is a
> way to register multiple DMI tables for boot time scanning to set
> further flags in the dmi properties post scan.

I never intended to call any function in dmi_scan from outside. I know
this stuff is __init, and what would be the point in rescanning the same
table again anyway? No, what I want is to make dmi_ident, which is the
array containing the results of the dmi scan, to be available to
everyone. Take a look at the previously sent patch, I think it's quite
straightforward.

I'm not sure I understand well what you mean with "register multible DMI
tables", so it's probably not what I want ;)

Let me know if I really missed something.

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

Re: [PATCH 2.4] dmi_ident made public

[Greg KH]

On Thu, Apr 24, 2003 at 06:47:59PM +0200, Jean Delvare wrote:
> 
> If this patch is accepted and applied, I'll work together with Peter to
> get the three above-mentioned modules simplified, as well as any other I
> may have missed. Also, I'll take care of porting this patch to the 2.5
> series, since it also belongs there.

i2c-piix4 in the 2.5 kernel tree does not need this patch, as everything
it needs to detect IBM laptops is already made public.  See the current
2.5 releases to verify this.

thanks,

greg k-h

Re: [PATCH 2.4] dmi_ident made public

[Alan Cox]

On Gwe, 2003-04-25 at 11:15, Jean Delvare wrote:
> What's more, I plan to write another module that exports the DMI data,
> as scanned at boot time, to userland (via sysfs), and such a module
> definitely requires that the DMI data is made public in dmi_scan.

I suspect the DMI module should itself do the sysfs interface, and I 
certainly think the idea of it being in sysfs has merit.

Re: [PATCH 2.4] dmi_ident made public

[Jean Delvare]

> > If this patch is accepted and applied, I'll work together with Peter
> > to get the three above-mentioned modules simplified, as well as any
> > other I may have missed. Also, I'll take care of porting this patch
> > to the 2.5 series, since it also belongs there.
> 
> i2c-piix4 in the 2.5 kernel tree does not need this patch, as
> everything it needs to detect IBM laptops is already made public.  See
> the current 2.5 releases to verify this.

Thanks for pointing this out. I took a look and could actually verify
it. This made me think again about the whole problem. I wondered wether
all other modules could be fixed that way (in which case my patch
wouldn't make sense), but it turns out that neither i8k nor omke can use
the same trick as the one you used for i2c-piix4. This is due to the
fact that i2c-piix4 only needs a flag (does the system match a given DMI
"mask"? yes/no) whereas the other modules need the actual data. Peter,
correct me if I'm wrong.

What's more, I plan to write another module that exports the DMI data,
as scanned at boot time, to userland (via sysfs), and such a module
definitely requires that the DMI data is made public in dmi_scan.

The good thing is that I think I now understand a bit better what Alan
meant yesterday by "register multiple DMI tables for boot time scanning
to set further flags in the dmi properties post scan". It must be what
you did for i2c-piix4, and isn't what I need there if my analysis is
correct.

So, I still think my patch makes sense and should be applied.

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

Re: [PATCH 2.4] dmi_ident made public

[Jean Delvare]

> > What's more, I plan to write another module that exports the DMI
> > data, as scanned at boot time, to userland (via sysfs), and such a
> > module definitely requires that the DMI data is made public in
> > dmi_scan.
> 
> I suspect the DMI module should itself do the sysfs interface, and I 
> certainly think the idea of it being in sysfs has merit.

I was not really sure about this. There are two issues that would need
to be discussed:
1* Including it into dmi_scan makes it impossible to be a module, at
least with my knowledge of the modules mechanics. Correct me if I'm
wrong.
2* There is a need for dmi scanning on IA-64 systems, where locating the
DMI table follows a different scheme from what we do for i386 systems.
Having a platform-independant module for the sysfs stuff would save us
from code duplication (which is precisely what I am trying to avoid
here).
I don't know how much of a concern these two points are, and any light
will be appreciated.

Anyway, I think this is almost a completely different problem from the
one I first posted about. There still are two modules around (i8k and
omke) that would take great benefit of the simple patch I submitted some
days ago. So, unless there is any motivated objection against it (such
as an alternative solution I wouldn't have thought about), I'd still
like to see the above mentioned patch applied, since it would give us
the possibility to remove all the duplicated code (more than 100 lines
per module).

Thanks.

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/