On Tue, 2014-03-11 at 17:02 -0700, Ruchi Kandoi wrote:
> This API would be called from the platform specific code,
[]
> This is already in use on some Android devices. We are trying to make
> this a generic API which could be called by other platforms as well,
> standardizing the format in which the info is
> logged in dmesg and the format of the info exported to userspace for
> collecting power management statistics.
logging comments:
> >> diff --git a/kernel/power/wakeup_reason.c b/kernel/power/wakeup_reason.c
[]
> >> @@ -0,0 +1,140 @@
Please add the standard
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
before any #include
to prefix this with either "power: " or "wakeup_reason: "
(it depends on how the makefile was written, I don't see it)
> >> +#include <linux/wakeup_reason.h>
> >> +#include <linux/kernel.h>
[]
> >> +void log_wakeup_reason(int irq)
> >> +{
> >> + struct irq_desc *desc;
> >> + desc = irq_to_desc(irq);
> >> + if (desc && desc->action && desc->action->name)
> >> + printk(KERN_INFO "Resume caused by IRQ %d, %s\n", irq,
> >> + desc->action->name);
> >> + else
> >> + printk(KERN_INFO "Resume caused by IRQ %d\n", irq);
if (desc && desc->action && desc->action->name)
pr_info("Resume caused by IRQ %d, %s\n",
irq, desc->action->name);
else
pr_info("Resume caused by IRQ %d\n", irq);
> >> + if (irqcount == MAX_WAKEUP_REASON_IRQS) {
> >> + spin_unlock(&resume_reason_lock);
> >> + printk(KERN_WARNING "Resume caused by more than %d IRQs\n",
> >> + MAX_WAKEUP_REASON_IRQS);
pr_warn("Resume caused by more than %d IRQs\n",
MAX_WAKEUP_REASON_IRQS);
> >> + return;
> >> + }
> >> +
> >> + irq_list[irqcount++] = irq;
> >> + spin_unlock(&resume_reason_lock);
> >> +}
I'd probably write this with:
if (irqcount >= MAX_REASON_IRQS) {
[]
> >> +int __init wakeup_reason_init(void)
> >> +{
> >> + int retval;
> >> + spin_lock_init(&resume_reason_lock);
> >> + retval = register_pm_notifier(&wakeup_reason_pm_notifier_block);
> >> + if (retval)
> >> + printk(KERN_WARNING "[%s] failed to register PM notifier
> >> %d\n",
> >> + __func__, retval);
Kernel style generally uses function names emitted as "%s: ", __func__
not putting function name in brackets. I suggest using that style:
pr_warn("%s: failed to register PM notifier %d\n",
__func__, retval);
> >> +
> >> + wakeup_reason = kobject_create_and_add("wakeup_reasons",
> >> kernel_kobj);
> >> + if (!wakeup_reason) {
> >> + printk(KERN_WARNING "[%s] failed to create a sysfs
> >> kobject\n",
> >> + __func__);
pr_warn("%s: failed to create a sysfs kobject\n", __func___);
> >> + return 1;
> >> + }
> >> + retval = sysfs_create_group(wakeup_reason, &attr_group);
> >> + if (retval) {
> >> + kobject_put(wakeup_reason);
> >> + printk(KERN_WARNING "[%s] failed to create a sysfs group
> >> %d\n",
> >> + __func__, retval);
pr_warn("%s: failed to create a sysfs group %d\n",
__func___, retval);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
