Thread (9 messages) 9 messages, 4 authors, 2012-06-28

Re: [PATCH option A] bcma: use custom printing functions

From: Hauke Mehrtens <hauke@hauke-m.de>
Date: 2012-06-28 20:29:53

On 06/28/2012 10:18 PM, Rafał Miłecki wrote:
2012/6/28 Joe Perches [off-list ref]:
quoted
On Thu, 2012-06-28 at 21:56 +0200, Rafał Miłecki wrote:
quoted
2012/6/28 Johannes Berg [off-list ref]:
quoted
On Thu, 2012-06-28 at 21:44 +0200, Rafał Miłecki wrote:
quoted
+#define bcma_err(fmt, ...) \
+     pr_err(KBUILD_MODNAME "-%d: " fmt, bus->num, ##__VA_ARGS__)
both of your options seem to rely on "bus" being a variable in the
context, is that really a good idea?
Yeah, I made that assumption to make calls nicer & shorter. We may
need to get reference to "bus" in function or two.

I saw such a solution in "radeon" gpu driver, example:
value = RREG32(R600_AUDIO_STATUS_BITS);
(they assume "rdev" is available in every function calling RREG32).
I think that radeon use is ugly myself.
quoted
If you believe it's ugly, I can change that. I also wonder what Joe
will respond.

P.S.
Both patches are not signed yet and they are supposed to be RFC. Sorry
for missing that in subject line.
I think it'd be better to add and use:

#define bcma_bus_err(bus, fmt, ...)                             \
       pr_err("bus %d: " fmt, (bus)->num, ##__VA_ARGS__)

#define bcma_bus_info(bus, fmt, ...)                            \
       pr_info("bus %d: " fmt, (bus)->num, ##__VA_ARGS__)

or some other equivalent use if you're wedded
to wanting "bcma-%d:" prefixed output.

I'd rather have the prefix be something like "bcma: <bus#>: ",
so a dmesg grep pattern can be "^bcma:" but hey, it ain't my code.
OK, thanks for your opinion. Just to be sure, did you mean:
bcma: bus0: FOO BAR
or
bcma: <bus0>: FOO BAR
?

Personally I don't really care, but maybe there is already similar
case in some other driver you know about? It could be nice to be
consistent across the kernel.
In b43 it looks like this if you have multiple devices logging, but if
there is some common way of doing it bcma should use that.

b43-phy0 debug: Found PHY: Analog 8, Type 4, Revision 6
b43-phy0 debug: Found Radio: Manuf 0x17F, Version 0x2056, Revision 11
b43-phy1: Broadcom 4716 WLAN found (core revision 17)
b43-phy1 debug: Found PHY: Analog 8, Type 4, Revision 5
b43-phy1 debug: Found Radio: Manuf 0x17F, Version 0x2056, Revision 7

Hauke
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help