Search Linux Wireless

Re: [PATCH RFC 2/2] brcmfmac: add brcmf_err_pub macro for better error messages

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 01/20/2017 11:26 AM, Arend Van Spriel wrote:
On 18-1-2017 15:27, Rafał Miłecki wrote:
From: Rafał Miłecki <rafal@xxxxxxxxxx>

This macro accepts an extra argument of struct brcmf_pub pointer. It
allows referencing struct device and printing error using dev_err. This
is very useful on devices with more than one wireless card suppored by
brcmfmac. Thanks for dev_err it's possible to identify device that error
is related to.

Signed-off-by: Rafał Miłecki <rafal@xxxxxxxxxx>
---
I could convert few more brcmf_err calls to this new brcmf_err_pub but I don't
want to spent too much time on this in case someone has a better idea.

If you do, comment! :)

I do not, but still comment ;-). Like the idea. Was on our list because
it is damn convenient when testing on router with multiple devices. I
would prefer to replace the existing brcmf_err() rather than introducing
a new one and would like to do the same for brcmf_dbg(). However, not
all call sites have a struct brcmf_pub instance available. Everywhere
before completing brcmf_attach() that is.

I decided to try new macro because:
1) I was too lazy to convert all calls at once
2) I could stick to brcmf_err when struct brcmf_pub isn't available yet

In theory we could pass NULL to dev_err, I'd result in something like:
[   14.746754] (NULL device *): brcmf_c_preinit_dcmds: Firmware version = wl0: Sep 18 2015 03:30:01 version 7.35.177.56 (r587209) FWID 01-6cb8e269

Unfortunately AFAIK it's not possible to handle
brcmf_err(NULL "Foo\n");
calls while keeping brcmf_err a macro.

I was thinking about something like:
#define brcmf_err(pub, fmt, ...)					\
	do {								\
		if (IS_ENABLED(CONFIG_BRCMDBG) || net_ratelimit())	\
			dev_err(pub ? pub->bus_if->dev : NULL,		\
				"%s: " fmt, __func__,			\
				##__VA_ARGS__);				\
	} while (0)

but this wouldn't compile because for brcmf_err(NULL "Foo\n"); it would roll
out to:
dev_err(NULL ? NULL->bus_if->dev : NULL, "Foo\n");

I *can* support brcmf_err(NULL "Foo\n"); calls if I change brcmf_err from
macro to a function (inline probably). Do you think it's a good idea?



[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux