Re: [PATCH] new device support for forcedeth.c second try

[Manfred Spraul <manfred@colorfullife.com>]

Carl-Daniel Hailfinger wrote:

>Hi,
>
>Brian, thank you very much for contributing to forcedeth.
>
I agree, thanks a lot.

> 	NvRegOffloadConfig = 0x90,
> #define NVREG_OFFLOAD_HOMEPHY	0x601
>-#define NVREG_OFFLOAD_NORMAL	0x5ee
>+#define NVREG_OFFLOAD_NORMAL	RX_NIC_BUFSIZE
>  
>
Interesting - does that explain why VLAN doesn't work properly? I have a 
report that maximum sized packets are rejected.

>+		struct {
>+			u32 Length:14;
>+			u32 Flags:18;
>+		} v2;
>  
>
Bitfields for hw access are evil, it caused problems before. I'd prefer 
a macro with explicit shifts.

>+
>+	//wait for 500ms
>+	mdelay(500);
>  
>
Waiting for phy reset is also evil - it should be done either in a 
separate thread or asynchroneously. Not urgent, we can fix it later.

>+
>+	// check auto negotiation is complete
>+	mii_status = mii_rw(dev, np->phyaddr, MII_BMSR, MII_READ);
>+	while (!(mii_status & BMSR_ANEGCOMPLETE)) {
>+		udelay(NV_MIIBUSY_DELAY);
>+		mii_status = mii_rw(dev, np->phyaddr, MII_BMSR, MII_READ);
>+		microseconds++;
>+		if (microseconds == 20) {
>+			microseconds = 0;
>+			milliseconds++;
>+		}
>+		if (milliseconds > 1200) {
>+			printk(KERN_INFO "%s: phy init failed to autoneg.\n", dev->name);
>+			return PHY_TIMEOUT;
>+		}
>  
>
Dito.

The phy code needs a big rewrite and support for ethtool anyway. I'd 
propose to merge the patch after removing the bitfields - everything 
else looks good. Perhaps there is a bit too much code duplication with 
the v1/v2 functions, but that's also not fatal.

--
    Manfred