Re: [PATCH] bcma: use custom printing functions
From: Joe Perches <joe@perches.com>
Date: 2012-06-29 10:39:13
On Fri, 2012-06-29 at 12:00 +0200, Rafał Miłecki wrote:
Having bus number printed makes it much easier to anaylze logs on systems with more buses. For example Netgear WNDR4500 has 3 AMBA buses in total, which makes standard log really messy.
Hi again Rafał
quoted hunk ↗ jump to hunk
diff --git a/drivers/bcma/bcma_private.h b/drivers/bcma/bcma_private.h
[]
quoted hunk ↗ jump to hunk
@@ -10,6 +10,14 @@ #define BCMA_CORE_SIZE 0x1000 +/* We use pr_fmt, so call printk directly */ +#define bcma_err(bus, fmt, ...) \ + printk(KERN_ERR KBUILD_MODNAME " bus%d: " fmt, (bus)->num, ##__VA_ARGS__) +#define bcma_warn(bus, fmt, ...) \ + printk(KERN_WARNING KBUILD_MODNAME " bus%d: " fmt, (bus)->num, ##__VA_ARGS__) +#define bcma_info(bus, fmt, ...) \ + printk(KERN_INFO KBUILD_MODNAME " bus%d: " fmt, (bus)->num, ##__VA_ARGS__)
seems fine.
quoted hunk ↗ jump to hunk
diff --git a/drivers/bcma/core.c b/drivers/bcma/core.c
[]
quoted hunk ↗ jump to hunk
@@ -56,6 +56,7 @@ EXPORT_SYMBOL_GPL(bcma_core_enable); void bcma_core_set_clockmode(struct bcma_device *core, enum bcma_clkmode clkmode) { + struct bcma_bus *bus = core->bus;
It seems unnecessary to me to do this assignment here and in lots of other places when it's only used once. It could cause unnecessary stack pressure.
quoted hunk ↗ jump to hunk
@@ -75,7 +76,7 @@ void bcma_core_set_clockmode(struct bcma_device *core, udelay(10); } if (i) - pr_err("HT force timeout\n"); + bcma_err(bus, "HT force timeout\n");
These individual style uses could become bcma_err(core->bus, "HT force timeout\n"); []
quoted hunk ↗ jump to hunk
@@ -137,8 +138,8 @@ void bcma_chipco_serial_init(struct bcma_drv_cc *cc) | BCMA_CC_CORECTL_UARTCLKEN); } } else { - pr_err("serial not supported on this device ccrev: 0x%x\n", - ccrev); + bcma_err(bus, "serial not supported on this device " + "ccrev: 0x%x\n", ccrev);
bcma_err(cc->core->bus, "serial not supported on this device ccrev: 0x%x\n", ccrev); Please don't split format strings. Ignore 80 column lengths for the formats only.
From Documentatation/CodingStyle chapter 2:
The limit on the length of lines is 80 columns and this is a strongly preferred limit. [] However, never break user-visible strings such as printk messages, because that breaks the ability to grep for them. cheers, Joe