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

[Andrew Morton]

On Mon, 18 Feb 2008 11:15:36 +0100 Thomas Petazzoni <thomas.petazzoni@xxxxxxxxxxxxxxxxxx> wrote:

> 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.
> 

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-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/