Re: [PATCH 1/3] Introduce interface to report BIOS bugs

Andi Kleen
2008-08-20T17:37:44Z

Thomas Renninger wrote:

I approve of the basic idea. Some nits in the implementation.

First shouted macros look ugly. Can you use something lower case
similar to dev_printk()?

> +
> +#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... */

These should probably have another string after the KERN_* severities appended
because KERN_* doesn't make it into the syslog log files or serial
console logs and you would lose the severity then.

> +config REPORT_FIRMWARE_BUGS
> + bool "Report Firmware Bugs"
> + default y
> + help
> + This option will make the kernel print out all firmware bug messages
> + it finds. This especially is very useful on ACPI systems where
> + potentially a lot firmware bugs can happen and should be reported.
> +
> + Always say yes here unless memory really matters.

I don't think this should be a CONFIG. If someone really wants to save
that much memory they can disable printk.

-Andi