Re: [PATCH v2 1/4] brcmfmac: Do not print the firmware version as an error

[Arend Van Spriel <arend.vanspriel@broadcom.com>]

On 8-3-2017 9:23, Hans de Goede wrote:
> Hi,
> 
> On 07-03-17 10:59, Arend Van Spriel wrote:
>> On 27-2-2017 22:45, Hans de Goede wrote:
>>> Using pr_err for things which are not errors is a bad idea. E.g. it
>>> will cause the plymouth bootsplash screen to drop back to the text
>>> console so that the user can see the error, which is not what we
>>> normally want to happen.
>>>
>>> Instead add a new brcmf_info macro and use that.
>>>
>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>> ---
>>> Changes in v2:
>>> -Fix brcm_err typo (should be brcmf_err) in CONFIG_BRCM_TRACING case
>>> ---
>>>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 2 +-
>>>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h  | 3 +++
>>>  2 files changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git
>>> a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
>>> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
>>> index 3e15d64..6d565f1 100644
>>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
>>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
>>> @@ -161,7 +161,7 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
>>>      strsep(&ptr, "\n");
>>>
>>>      /* Print fw version info */
>>> -    brcmf_err("Firmware version = %s\n", buf);
>>> +    brcmf_info("Firmware version = %s\n", buf);
>>>
>>>      /* locate firmware version number for ethtool */
>>>      ptr = strrchr(buf, ' ') + 1;
>>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
>>> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
>>> index 6687812..605f260 100644
>>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
>>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
>>> @@ -59,11 +59,14 @@
>>>              pr_err("%s: " fmt, __func__, ##__VA_ARGS__);    \
>>>      } while (0)
>>>  #endif
>>> +#define brcmf_info(fmt, ...)    pr_info("%s: " fmt, __func__,
>>> ##__VA_ARGS__)
>>
>> Prefer using the same style as for brcmf_err, ie. using do {} while (0)
> 
> OK, v3 with this fixed coming up.

I think Kalle prefers the whole series to be resubmitted.

Kalle,

Can you confirm (or deny)?

Regards,
Arend

> Regards,
> 
> Hans
> 
> 
>>
>> Regards,
>> Arend
>>
>>>  #else
>>>  __printf(2, 3)
>>>  void __brcmf_err(const char *func, const char *fmt, ...);
>>>  #define brcmf_err(fmt, ...) \
>>>      __brcmf_err(__func__, fmt, ##__VA_ARGS__)
>>> +/* For tracing purposes treat info messages as errors */
>>> +#define brcmf_info brcmf_err
>>>  #endif
>>>
>>>  #if defined(DEBUG) || defined(CONFIG_BRCM_TRACING)
>>>