Re: 2.6.25-rc2-mm1 (x64 thermal build failure)
On Wed, 20 Feb 2008 08:21:33 +0100 Thomas Petazzoni [EMAIL PROTECTED] wrote: Le Tue, 19 Feb 2008 15:21:29 -0800, Andrew Morton [EMAIL PROTECTED] a __crit : ug, sorry, if I'd realised it was like this I'd have said don't bother. Apart from the obvious problem, this means that people will keep breaking CONFIG_DMI=n all the time, because they will forget the ifdefs, and the number of people who test with CONFIG_DMI=n will be small. Yes, #ifdef CONFIG_DMI is not very comfortable. That why I proposed things such as DECLARE_DMI_FIXUP_TABLE(), because it would force people to use these macros, which would then be working correctly depending on DMI=y/n. However, there's still the issue of driver_data that I mentionned in my earlier post. What should I do ? Option 1 ? Option 2 ? Give up with the patch ? Thanks for your comments, Option 1 would be best, I think: 1) Remove the #ifdef CONFIG_DMI around DMI fixup tables and callbacks definition, so that everything exists and gcc is happy. gcc is able to optimize out the DMI fixup table (it is not present in the binary when compiling with DMI=n), but gcc doesn't seem to be able to optimize out the DMI fixup callbacks (they are still present in the binary). So this would leave some unused code in the binary, which is not completely satisfying. gcc _should_ be able to remove the callbacks as long as they are static and have no references. If even the latest gcc versions are still incluing the unreferenced, static function in the final vmlinux then let's get gcc fixed? - To unsubscribe from this list: send the line unsubscribe linux-acpi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 2.6.25-rc2-mm1 (x64 thermal build failure)
On Tue, 19 Feb 2008 16:55:02 +0100 Thomas Petazzoni wrote: Le Mon, 18 Feb 2008 04:13:40 -0800, Andrew Morton [EMAIL PROTECTED] a écrit : Option 3 wold be to add more #ifdef CONFIG_DMI lines around the place. How ugly would that get? Like the attached patch. #ifdef CONFIG_DMI everywhere :-( Does this patch apply to -mm? Seem like No. After converting it from mime(?) to ASCII and fixing one #if (change and to ) fixing patch rejects, it does build cleanly. --- ~Randy - To unsubscribe from this list: send the line unsubscribe linux-acpi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 2.6.25-rc2-mm1 (x64 thermal build failure)
Le Mon, 18 Feb 2008 04:13:40 -0800,
Andrew Morton [EMAIL PROTECTED] a écrit :
Option 3 wold be to add more #ifdef CONFIG_DMI lines around the
place. How ugly would that get?
Like the attached patch. #ifdef CONFIG_DMI everywhere :-(
Sincerly,
Thomas
---
Turn CONFIG_DMI into a selectable option if EMBEDDED is defined, in
order to be able to remove the DMI table scanning code if it's not
needed, and then reduce the kernel code size.
The DMI code users are modified, so that they either depend on
CONFIG_DMI (for the drivers who really need DMI to work) or their
DMI-related code is enclosed in #ifdef CONFIG_DMI.
With CONFIG_DMI (i.e before) :
textdata bss dec hex filename
1076076 128656 98304 1303036 13e1fc vmlinux
Without CONFIG_DMI (i.e after) :
textdata bss dec hex filename
1068092 126308 98304 1292704 13b9a0 vmlinux
Result:
textdata bss dec hex filename
-7984 -2348 0 -10332 -285c vmlinux
The new option appears in Processor type and features, only when
CONFIG_EMBEDDED is defined.
This patch is part of the Linux Tiny project, and is based on previous
work done by Matt Mackall [EMAIL PROTECTED].
Signed-off-by: Thomas Petazzoni [EMAIL PROTECTED]
---
arch/x86/Kconfig | 13 ++---
arch/x86/kernel/acpi/boot.c|4 ++--
arch/x86/kernel/acpi/sleep_32.c|2 ++
arch/x86/kernel/apm_32.c |2 ++
arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c |4 ++--
arch/x86/kernel/cpu/cpufreq/powernow-k7.c |3 ++-
arch/x86/kernel/io_delay.c |2 ++
arch/x86/kernel/reboot.c |2 ++
arch/x86/kernel/tsc_32.c |2 ++
arch/x86/mach-generic/bigsmp.c |3 ++-
arch/x86/pci/acpi.c|2 ++
arch/x86/pci/common.c |2 ++
arch/x86/pci/fixup.c |5 -
arch/x86/pci/irq.c |2 ++
drivers/acpi/sleep/main.c |2 ++
drivers/ata/ahci.c |2 ++
drivers/ata/ata_piix.c |4
drivers/ata/pata_cs5530.c |2 ++
drivers/ata/pata_via.c |3 ++-
drivers/char/Kconfig |2 +-
drivers/hwmon/Kconfig |2 ++
drivers/hwmon/abituguru.c |2 --
drivers/i2c/busses/i2c-piix4.c |2 ++
drivers/ide/ide-acpi.c |3 +++
drivers/ide/pci/alim15x3.c |3 ++-
drivers/ide/pci/via82cxxx.c|3 ++-
drivers/input/keyboard/atkbd.c |4
drivers/input/misc/wistron_btns.c |2 ++
drivers/input/mouse/lifebook.c |5 +++--
drivers/input/mouse/synaptics.c|2 +-
drivers/leds/leds-clevo-mail.c |2 ++
drivers/misc/Kconfig |1 +
drivers/misc/acer-wmi.c| 14 --
drivers/misc/sony-laptop.c |4
drivers/net/via-rhine.c|2 ++
drivers/pnp/pnpbios/core.c |2 ++
drivers/pnp/quirks.c |2 ++
drivers/video/Kconfig |2 +-
include/linux/dmi.h|3 ++-
39 files changed, 96 insertions(+), 27 deletions(-)
Index: linux/arch/x86/Kconfig
===
--- linux.orig/arch/x86/Kconfig
+++ linux/arch/x86/Kconfig
@@ -90,9 +90,6 @@
config ARCH_MAY_HAVE_PC_FDC
def_bool y
-config DMI
- def_bool y
-
config RWSEM_GENERIC_SPINLOCK
def_bool !X86_XADD
@@ -433,6 +430,15 @@
# Mark as embedded because too many people got it wrong.
# The code disables itself when not needed.
+config DMI
+ default y
+ bool Enable DMI scanning if EMBEDDED
+ help
+ Enabled scanning of DMI to identify machine quirks. Say Y
+ here unless you have verified that your setup is not
+ affected by entries in the DMI blacklist. Required by PNP
+ BIOS code.
+
config GART_IOMMU
bool GART IOMMU support if EMBEDDED
default y
@@ -645,6 +651,7 @@
config I8K
tristate Dell laptop support
+ depends on DMI
---help---
This adds a driver to safely access the System Management Mode
of the CPU on the Dell Inspiron 8000. The System Management Mode
Index: linux/arch/x86/kernel/acpi/boot.c
===
--- linux.orig/arch/x86/kernel/acpi/boot.c
+++ linux/arch/x86/kernel/acpi/boot.c
@@ -900,7 +900,7 @@
return;
}
-#ifdef __i386__
+#if defined(__i386__) defined(CONFIG_DMI)
static int __init disable_acpi_irq(const struct dmi_system_id *d)
{
@@ -1121,7 +1121,7 @@
{}
};
-#endif /*
Re: 2.6.25-rc2-mm1 (x64 thermal build failure)
Thomas Petazzoni wrote: Le Tue, 19 Feb 2008 09:41:47 -0800, Randy Dunlap [EMAIL PROTECTED] a écrit : Does this patch apply to -mm? Seem like No. No, it was generated against 2.6.25-rc2. After converting it from mime(?) to ASCII Probably due to my PGP-MIME signature. Will try to remember that I should disable it next time. and fixing one #if (change and to ) Oops. Fixed on my side too. fixing patch rejects, it does build cleanly. The rejects are probably due to the patch being applied to -mm. It applies fine on -rc here. Any opinion about whether the patch is clean ? Worth it ? It seems reasonable to me as long as the option depends on EMBEDDED, as it does. -- ~Randy - To unsubscribe from this list: send the line unsubscribe linux-acpi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 2.6.25-rc2-mm1 (x64 thermal build failure)
Le Tue, 19 Feb 2008 09:41:47 -0800, Randy Dunlap [EMAIL PROTECTED] a écrit : Does this patch apply to -mm? Seem like No. No, it was generated against 2.6.25-rc2. After converting it from mime(?) to ASCII Probably due to my PGP-MIME signature. Will try to remember that I should disable it next time. and fixing one #if (change and to ) Oops. Fixed on my side too. fixing patch rejects, it does build cleanly. The rejects are probably due to the patch being applied to -mm. It applies fine on -rc here. Any opinion about whether the patch is clean ? Worth it ? Thanks for testing the patch, Thomas -- Thomas Petazzoni, Free Electrons Free Embedded Linux Training Materials on http://free-electrons.com/training (More than 1500 pages!) - To unsubscribe from this list: send the line unsubscribe linux-acpi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 2.6.25-rc2-mm1 (x64 thermal build failure)
On Tue, 19 Feb 2008 16:55:02 +0100 Thomas Petazzoni [EMAIL PROTECTED] wrote: Le Mon, 18 Feb 2008 04:13:40 -0800, Andrew Morton [EMAIL PROTECTED] a __crit : Option 3 wold be to add more #ifdef CONFIG_DMI lines around the place. How ugly would that get? Like the attached patch. #ifdef CONFIG_DMI everywhere :-( ug, sorry, if I'd realised it was like this I'd have said don't bother. Apart from the obvious problem, this means that people will keep breaking CONFIG_DMI=n all the time, because they will forget the ifdefs, and the number of people who test with CONFIG_DMI=n will be small. - To unsubscribe from this list: send the line unsubscribe linux-acpi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 2.6.25-rc2-mm1 (x64 thermal build failure)
Le Tue, 19 Feb 2008 15:21:29 -0800, Andrew Morton [EMAIL PROTECTED] a écrit : ug, sorry, if I'd realised it was like this I'd have said don't bother. Apart from the obvious problem, this means that people will keep breaking CONFIG_DMI=n all the time, because they will forget the ifdefs, and the number of people who test with CONFIG_DMI=n will be small. Yes, #ifdef CONFIG_DMI is not very comfortable. That why I proposed things such as DECLARE_DMI_FIXUP_TABLE(), because it would force people to use these macros, which would then be working correctly depending on DMI=y/n. However, there's still the issue of driver_data that I mentionned in my earlier post. What should I do ? Option 1 ? Option 2 ? Give up with the patch ? Thanks for your comments, Thomas -- Thomas Petazzoni, Free Electrons Free Embedded Linux Training Materials on http://free-electrons.com/training (More than 1500 pages!) signature.asc Description: PGP signature
Re: 2.6.25-rc2-mm1 (x64 thermal build failure)
Hi,
Le Sat, 16 Feb 2008 21:44:10 -0800,
Andrew Morton [EMAIL PROTECTED] a écrit :
Bustage in x86-configurable-dmi-scanning-code.patch. Previously,
DMI=y was just hardwired. Now, it becomes selectable and stuff
breaks.
I guess the DMI=n version of dmi_check_system() could become a macro
so we don't emit a reference to its argument, but that might generate
unused-variable warnings elsewhere.
Thanks for your report. The issue is that some DMI fixup tables and
callbacks are defined inside #ifdef CONFIG_DMI, some others are not. We
need to normalize that to fix the build issue in all situations.
I've thought about it, and I see two options, but I can't decide which
one is the best, so I request your opinion on that.
1) Remove the #ifdef CONFIG_DMI around DMI fixup tables and callbacks
definition, so that everything exists and gcc is happy. gcc is able
to optimize out the DMI fixup table (it is not present in the binary
when compiling with DMI=n), but gcc doesn't seem to be able to
optimize out the DMI fixup callbacks (they are still present in the
binary). So this would leave some unused code in the binary, which
is not completely satisfying.
2) Define macros such as DECLARE_DMI_FIXUP_TABLE and
DECLARE_DMI_FIXUP_CALLBACK, which could then be used like this:
DECLARE_DMI_FIXUP_CALLBACK(set_bios_reboot, __init, d, {
if (reboot_type != BOOT_BIOS) {
reboot_type = BOOT_BIOS;
printk(KERN_INFO %s series board detected. Selecting
BIOS-method for reboots.\n, d-ident);
}
return 0;
});
DECLARE_DMI_FIXUP_TABLE(reboot_dmi_table, __initdata, {
{ /* Handle problems with rebooting on Dell E520's */
.callback = set_bios_reboot,
.ident = Dell E520,
.matches = {
DMI_MATCH(DMI_SYS_VENDOR, Dell Inc.),
DMI_MATCH(DMI_PRODUCT_NAME, Dell DM061),
},
}
});
And use them everywhere, so that DMI fixup tables and callbacks
are properly compiled out when DMI=n. Here are the macro definition:
#ifdef CONFIG_DMI
#define DECLARE_DMI_FIXUP_TABLE(name, opts, contents...) \
static struct dmi_system_id opts name [] = contents
#define DECLARE_DMI_FIXUP_CALLBACK(name, opts, id, contents...) \
static int opts name(const struct dmi_system_id *id) contents
#else
#define DECLARE_DMI_FIXUP_TABLE(name, opts, contents...)
#define DECLARE_DMI_FIXUP_CALLBACK(name, opts, contents...)
#endif
The issue I have with this option is that there are sometimes
driver_data associated to DMI callbacks (see
drivers/input/misc/wistron_btns.c for example) and I don't exactly
see how to create a similar DECLARE_DMI_FIXUP_CALLBACK_DATA macro.
Thanks for your insights,
Thomas
--
Thomas Petazzoni, Free Electrons
Free Embedded Linux Training Materials
on http://free-electrons.com/training
(More than 1500 pages!)
signature.asc
Description: PGP signature
Re: 2.6.25-rc2-mm1 (x64 thermal build failure)
On Mon, 18 Feb 2008 11:15:36 +0100 Thomas Petazzoni [EMAIL PROTECTED] wrote:
Hi,
Le Sat, 16 Feb 2008 21:44:10 -0800,
Andrew Morton [EMAIL PROTECTED] a __crit :
Bustage in x86-configurable-dmi-scanning-code.patch. Previously,
DMI=y was just hardwired. Now, it becomes selectable and stuff
breaks.
I guess the DMI=n version of dmi_check_system() could become a macro
so we don't emit a reference to its argument, but that might generate
unused-variable warnings elsewhere.
Thanks for your report. The issue is that some DMI fixup tables and
callbacks are defined inside #ifdef CONFIG_DMI, some others are not. We
need to normalize that to fix the build issue in all situations.
I've thought about it, and I see two options, but I can't decide which
one is the best, so I request your opinion on that.
1) Remove the #ifdef CONFIG_DMI around DMI fixup tables and callbacks
definition, so that everything exists and gcc is happy. gcc is able
to optimize out the DMI fixup table (it is not present in the binary
when compiling with DMI=n), but gcc doesn't seem to be able to
optimize out the DMI fixup callbacks (they are still present in the
binary). So this would leave some unused code in the binary, which
is not completely satisfying.
2) Define macros such as DECLARE_DMI_FIXUP_TABLE and
DECLARE_DMI_FIXUP_CALLBACK, which could then be used like this:
DECLARE_DMI_FIXUP_CALLBACK(set_bios_reboot, __init, d, {
if (reboot_type != BOOT_BIOS) {
reboot_type = BOOT_BIOS;
printk(KERN_INFO %s series board detected. Selecting
BIOS-method for reboots.\n, d-ident);
}
return 0;
});
DECLARE_DMI_FIXUP_TABLE(reboot_dmi_table, __initdata, {
{ /* Handle problems with rebooting on Dell E520's */
.callback = set_bios_reboot,
.ident = Dell E520,
.matches = {
DMI_MATCH(DMI_SYS_VENDOR, Dell Inc.),
DMI_MATCH(DMI_PRODUCT_NAME, Dell DM061),
},
}
});
And use them everywhere, so that DMI fixup tables and callbacks
are properly compiled out when DMI=n. Here are the macro definition:
#ifdef CONFIG_DMI
#define DECLARE_DMI_FIXUP_TABLE(name, opts, contents...) \
static struct dmi_system_id opts name [] = contents
#define DECLARE_DMI_FIXUP_CALLBACK(name, opts, id, contents...) \
static int opts name(const struct dmi_system_id *id) contents
#else
#define DECLARE_DMI_FIXUP_TABLE(name, opts, contents...)
#define DECLARE_DMI_FIXUP_CALLBACK(name, opts, contents...)
#endif
The issue I have with this option is that there are sometimes
driver_data associated to DMI callbacks (see
drivers/input/misc/wistron_btns.c for example) and I don't exactly
see how to create a similar DECLARE_DMI_FIXUP_CALLBACK_DATA macro.
Option 3 wold be to add more #ifdef CONFIG_DMI lines around the place. How
ugly would that get?
-
To unsubscribe from this list: send the line unsubscribe linux-acpi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 2.6.25-rc2-mm1 (x64 thermal build failure)
On Sat, 16 Feb 2008 00:25:22 -0800 Andrew Morton wrote: ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.25-rc2/2.6.25-rc2-mm1/ ACPI is enabled, but DMI=n. linux-2.6.25-rc2-mm1/drivers/acpi/thermal.c: In function 'acpi_thermal_init': linux-2.6.25-rc2-mm1/drivers/acpi/thermal.c:1792: error: 'thermal_dmi_table' undeclared (first use in this function) linux-2.6.25-rc2-mm1/drivers/acpi/thermal.c:1792: error: (Each undeclared identifier is reported only once linux-2.6.25-rc2-mm1/drivers/acpi/thermal.c:1792: error: for each function it appears in.) make[3]: *** [drivers/acpi/thermal.o] Error 1 --- ~Randy - To unsubscribe from this list: send the line unsubscribe linux-acpi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 2.6.25-rc2-mm1 (x64 thermal build failure)
On Sat, 16 Feb 2008 21:16:03 -0800 Randy Dunlap [EMAIL PROTECTED] wrote: On Sat, 16 Feb 2008 00:25:22 -0800 Andrew Morton wrote: ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.25-rc2/2.6.25-rc2-mm1/ ACPI is enabled, but DMI=n. linux-2.6.25-rc2-mm1/drivers/acpi/thermal.c: In function 'acpi_thermal_init': linux-2.6.25-rc2-mm1/drivers/acpi/thermal.c:1792: error: 'thermal_dmi_table' undeclared (first use in this function) linux-2.6.25-rc2-mm1/drivers/acpi/thermal.c:1792: error: (Each undeclared identifier is reported only once linux-2.6.25-rc2-mm1/drivers/acpi/thermal.c:1792: error: for each function it appears in.) make[3]: *** [drivers/acpi/thermal.o] Error 1 Bustage in x86-configurable-dmi-scanning-code.patch. Previously, DMI=y was just hardwired. Now, it becomes selectable and stuff breaks. I guess the DMI=n version of dmi_check_system() could become a macro so we don't emit a reference to its argument, but that might generate unused-variable warnings elsewhere. - To unsubscribe from this list: send the line unsubscribe linux-acpi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
