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 <ra...@milecki.pl>

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 <ra...@milecki.pl>
---
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?

Reply via email to