Re: 2.6.25-rc2-mm1 (x64 thermal build failure)

[Thomas Petazzoni]

Hi,

Le Sat, 16 Feb 2008 21:44:10 -0800,
Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> 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!)

Attachment: signature.asc
Description: PGP signature