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

Andrew Morton
2008-02-18T12:16:12Z

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

> Hi,
>
> Le Sat, 16 Feb 2008 21:44:10 -0800,
> Andrew Morton <akpm@linux-foundation.org> 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?