Re: [PATCH 5/5] brcmfmac: introduce checkdied debugfs functionality

["Arend van Spriel" <arend@broadcom.com>]

On 06/10/2012 04:13 AM, Gábor Stefanik wrote:
> On Sat, Jun 9, 2012 at 10:51 PM, Arend van Spriel <arend@broadcom.com> wrote:
>> The checkdied functionality provides useful information for analyzing
>> firmware crashes. By exposing this information to a debugfs file users
>> can easily provide its content in bug reports. The functionality is
>> available only when CONFIG_BRCMDBG is selected.
>>
>> Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com>
>> Reviewed-by: Franky (Zhenhui) Lin <frankyl@broadcom.com>
>> Signed-off-by: Arend van Spriel <arend@broadcom.com>
>> ---
>>  drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c |  332 +++++++++++++++++++-
>>  1 file changed, 331 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c b/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
>> index a07fb01..5a33b42 100644
>> --- a/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
>> +++ b/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
>> @@ -31,6 +31,7 @@
>>  #include <linux/firmware.h>
>>  #include <linux/module.h>
>>  #include <linux/bcma/bcma.h>
>> +#include <linux/debugfs.h>
>>  #include <asm/unaligned.h>
>>  #include <defs.h>
>>  #include <brcmu_wifi.h>
>> @@ -48,6 +49,9 @@
>>
>>  #define CBUF_LEN       (128)
>>
>> +/* Device console log buffer state */
>> +#define CONSOLE_BUFFER_MAX     2024
> 
> Just out of curiosity; what is the significance of this number?

It is used in the code fragment below. I have to admit the comment is
not explaining its significance.

>> +       /* allocate buffer for console data */
>> +       if (console_size <= CONSOLE_BUFFER_MAX)
>> +               conbuf = kzalloc(console_size+1, GFP_ATOMIC);
>> +
>> +       if (!conbuf)
>> +               return -ENOMEM;

Probably would be better to use vmalloc() here or at least use
GFP_KERNEL here.

John, any recommendations?

Gr. AvS