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?