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

["John W. Linville" <linville@tuxdriver.com>]

On Sun, Jun 10, 2012 at 06:57:29AM +0200, Arend van Spriel wrote:
> 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?

vmalloc seems reasonable...

-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.