Subject: Re: [PATCH] drivers/net/r8169.c - trivial - add KERN_DEBUG to dprintk and PFX before KERN_ uses

From: Joe Perches <[email-masked]>
Date: Tue, 02 Oct 2007 10:06:37 -0700

On Tue, 2007-10-02 at 12:57 -0400, Jeff Garzik wrote:
> \> @@ -1924,7 +1925,7 @@ static void rtl_hw_start_8169(struct net_device *dev)
> >  
> >  	if ((tp->mac_version == RTL_GIGA_MAC_VER_02) ||
> >  	    (tp->mac_version == RTL_GIGA_MAC_VER_03)) {
> > -		dprintk(KERN_INFO PFX "Set MAC Reg C+CR Offset 0xE0. "
> > +		dprintk("Set MAC Reg C+CR Offset 0xE0. "
> >  			"Bit-3 and bit-14 MUST be 1\n");
> >  		tp->cp_cmd |= (1 << 14);
> >  	}
> 
> NAK these, the author clearly prefers his debugging output at KERN_INFO 
> level

This fixes an expanded printk(PFX KERN_INFO PFX foo) bug.

That the author "clearly prefers" is dubious.

The rest of the dprintk uses are undefined.

$ grep dprintk drivers/net/r8169.c
        dprintk("mac_version = 0x%02x\n", tp->mac_version);
                        dprintk("phy_version == %s (%04x)\n", p->msg, p->reg);
        dprintk("phy_version == Unknown\n");
        dprintk("MAC version != 0 && PHY version == 0 or 1\n");
        dprintk("Do final_reg2.cfg\n");
        dprintk("Set MAC Reg C+CR Offset 0x82h = 0x01h\n");
                dprintk("Set MAC Reg C+CR Offset 0x82h = 0x01h\n");
                dprintk("Set PHY Reg 0x0bh = 0x00h\n");
                dprintk(KERN_INFO PFX "Set MAC Reg C+CR Offset 0xE0. "

If the author does so prefer KERN_INFO levels, it's better to use:

#define dprintk(fmt, arg...) \
	do { printk(KERN_INFO PFX fmt, ## arg); } while (0)

cheers,  Joe