[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <200808201237.43338.bjorn.helgaas@hp.com>
Date: Wed, 20 Aug 2008 12:37:42 -0600
From: Bjorn Helgaas <bjorn.helgaas@...com>
To: Thomas Renninger <trenn@...e.de>
Cc: linux-kernel@...r.kernel.org, ak@...ux.intel.com,
len.brown@...el.com, arjan@...ux.intel.com,
linux-acpi@...r.kernel.org,
Christian Kornacker <ckornacker@...e.de>
Subject: Re: [PATCH 1/3] Introduce interface to report BIOS bugs
On Wednesday 20 August 2008 11:02:04 am Thomas Renninger wrote:
> From: Christian Kornacker <ckornacker@...e.de>
>
> This is mostly needed for ACPI systems.
> ACPI introduces an endless amount of possible BIOS
> bugs like wrong values, missing functions, etc.
> The kernel has to sanity check all of them and should
> report BIOS bugs as such to the user.
I can't quite decide whether the whole idea is over-engineering
or not. I guess my hesitation is mainly that things like this take
ongoing maintenance to keep them valuable, and that's often where
things fall apart.
> +#define FW_EMERG KERN_EMERG /* System cannot boot */
> +#define FW_ALERT KERN_ALERT /* Risk of HW or data damage,
> + e.g. overheating, dmraid */
> +#define FW_CRIT KERN_CRIT /* A major device is not functional
> + e.g. hpet, lapic, network... */
> +#define FW_ERR KERN_ERR /* A major device is not working
> + as expected, e.g. cpufreq stuck
> + to lowest freq, lowered
> + performance, increased power
> + consumption... */
> +#define FW_WARN KERN_WARNING /* A minor device does not work
> + or is not fully functional,
> + e.g. backlight brightness,
> + Hotplug capabilities of a
> + device that should be
> + hot-plugable will not work */
> +#define FW_INFO KERN_INFO /* Anything else related to BIOS
> + that is worth mentioning */
> +
> +
> +#ifdef CONFIG_REPORT_FIRMWARE_BUGS
> + #define FW_PRINT_WARN(severity, fmt, args...) printk("%s[BIOS]: " fmt "\n", \
> + severity, ##args)
> +#else
> + #define FW_PRINT_WARN(severity, fmt, args...) do { } while (0)
> +#endif
> +
> +#define FW_PRINT_CRIT(severity, fmt, args...) printk("%s[BIOS]: " fmt "\n", \
> + severity, ##args)
I think there are too many possibilities (FW_PRINT_WARN vs FW_PRINT_CRIT,
then one of FW_INFO, FW_WARN, FW_ERR, FW_CRIT, FW_ALERT, FW_EMERG).
A simpler interface with only one or two choices would give 90% of the
benefit.
My preference would be to *not* add a newline inside the interface.
Everybody knows printk needs a newline, and it's simpler if all the
printk variants follow that same rule.
The "BIOS" string is very x86-centric. I'd prefer something like
"firmware" or "FW" that's also applicable to non-x86 systems.
I'm on a real dev_printk() kick at the moment, so I'd like to see
a way to hook a message to *something*, whether it's a specific
device, an ACPI method, a table at a specific physical address, etc.
For example, this:
+ FW_PRINT_CRIT(FW_ERR, PFX "No ACPI _PSS objects for CPU"
+ " other than CPU0. Complain to your BIOS"
+ " vendor");
would be nicer if it could report the specific CPU device.
Admittedly, many of the places you touch don't currently have
an idea of a "device." But sometimes that's a deficiency in
the current Linux implementation, so I think your interface
should at least allow a device.
Maybe even something as simple as:
#define FW_BUG "[FW bug]: "
would be sufficient, with the idea that people could do this:
dev_err(&dev->dev, FW_BUG "interrupts left enabled\n");
I think the user-space value derives from having a consistent string
to grep for, so this gives you that. I'm not sure what value we get
from adding the new FW_PRINT_CRIT()/FW_PRINT_WARN() interfaces in the
kernel.
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists