netdev
[Top] [All Lists]

Re: [9/9][PATCH 2.6] Add WOL support

To: Jeff Garzik <jgarzik@xxxxxxxxx>
Subject: Re: [9/9][PATCH 2.6] Add WOL support
From: Roger Luethi <rl@xxxxxxxxxxx>
Date: Sun, 20 Jun 2004 00:15:13 +0200
Cc: Andrew Morton <akpm@xxxxxxxx>, netdev@xxxxxxxxxxx
In-reply-to: <40D4B049.6070508@xxxxxxxxx>
References: <20040615175003.GA11382@xxxxxxxxxxxxxx> <40D4B049.6070508@xxxxxxxxx>
Sender: netdev-bounce@xxxxxxxxxxx
User-agent: Mutt/1.5.6i
On Sat, 19 Jun 2004 17:29:45 -0400, Jeff Garzik wrote:
> >+                            reason = "Unicast packet";
> >+                            break;
> >+                    case WOLbmcast:
> >+                            reason = "Multicast/broadcast packet";
> >+                            break;
> >+                    default:
> >+                            reason = "Unknown";
> >+                    }
> >+                    printk("%s: Woke system up. Reason: %s.\n",
> >+                           DRV_NAME, reason);
> 
> printk needs  KERN_xxx prefix

Oops. Fixed.

> also, use dev->name rather than DRV_NAME, since we are past the probe phase.

Can you define probe phase? ... In the code as is, we haven't called
register_netdev when we execute that part. Getting the probe stuff into
a sane order is on my todo list as well, but ISTR that calling
register_netdev way early is frowned upon. No?

> >+    /* Enable legacy WOL (for old motherboards) */
> >+    writeb(0x01, ioaddr + PwcfgSet);
> >+    writeb(readb(ioaddr + StickyHW) | 0x04, ioaddr + StickyHW);
> >+
> >+    /* Hit power state D3 (sleep) */
> >+    writeb(readb(ioaddr + StickyHW) | 0x03, ioaddr + StickyHW);
> 
> would be nice to eliminate these magic numbers (0x04, 0x04), ...

Mostly agreed. I want to change a bunch of symbol names anyway. However,
I found it easier to document magic numbers that are used only _once_
where they occur instead of giving them a name I have to look up later.
I don't feel strongly about it, though, so feel free to bug me again if
you do :-).

Roger

<Prev in Thread] Current Thread [Next in Thread>