Re: [PATCH 3/3] Staging: rt2860: include KERN_* in printk

From: L. Alberto Giménez
Date: Sun Dec 12 2010 - 14:32:59 EST


On Sun, Dec 12, 2010 at 11:04:28AM -0800, Joe Perches wrote:
> On Sun, 2010-12-12 at 18:56 +0100, L. Alberto Giménez wrote:
> > Fix checkpatch complains
> > diff --git a/drivers/staging/rt2860/common/ee_efuse.c b/drivers/staging/rt2860/common/ee_efuse.c
> []
> > @@ -281,8 +281,8 @@ int set_eFusedump_Proc(struct rt_rtmp_adapter *pAd, char *arg)
> >
> > eFuseReadPhysical(pAd, &InBuf[0], 4, &InBuf[2], 2);
> > if (i % 4 == 0)
> > - printk("\nBlock %x:", i / 8);
> > - printk("%04x ", InBuf[2]);
> > + printk(KERN_DEBUG "\nBlock %x:", i / 8);
> > + printk(KERN_DEBUG "%04x ", InBuf[2]);
> > }
> > return TRUE;
> > }
>
> Not quite. Use:
> [...]

Ok.

> > diff --git a/drivers/staging/rt2860/pci_main_dev.c b/drivers/staging/rt2860/pci_main_dev.c
> >[...]
> > -#ifdef DEBUG
> > - printk("RTMPAllocateNdisPacket Fail\n");
> > -#endif
> > + printk(KERN_DEBUG "RTMPAllocateNdisPacket Fail\n");
>
> Behavior change.
> Using pr_devel would be exactly the same.
> Using pr_debug would enable dynamic debug.

You are completely right. I guessed that DEBUG would be a developer-defined
definition. Will fix with pr_devel, makes more sense to me.

>
> > @@ -601,15 +600,15 @@ void hex_dump(char *str, unsigned char *pSrcBufVA, unsigned int SrcBufLen)
> >[...]
> This should use print_hex_dump

Ok, but I'll have to figure out how to translate the arguments without losing
the original developers intended format.

>
> > diff --git a/drivers/staging/rt2860/rt_linux.h b/drivers/staging/rt2860/rt_linux.h
> []
> > #define DBGPRINT_ERR(Fmt) \
> > { \
> > - printk("ERROR! "); \
> > + printk(KERN_ERR "ERROR! "); \
> > printk Fmt; \
> > }
>
> Better ways to do this:
>
> #define DBGPRINT_ERR(fmt, args...) printk(KERN_ERR fmt, ##args)
> or
> #define DBGPRINT_ERR(fmt, ...) printk(KERN_ERR fmt, ##__VA_ARGS__)

Thanks for your review. Will resend.


Regards,

--
L. Alberto Giménez
JabberID agimenez@xxxxxxxxxxxxxxxxxx
GnuPG key ID 0x3BAABDE1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/