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

[Carl-Daniel Hailfinger]

Bartlomiej Zolnierkiewicz wrote:
> On Saturday 19 of June 2004 15:55, Francois Romieu wrote:
> 
>>Carl-Daniel Hailfinger <c-d.hailfinger.kernel.2004@xxxxxxx> :
>>[...]
>>
>>
>>>+static int phy_reset(struct net_device *dev)
>>>+{
>>>+	struct fe_priv *np = get_nvpriv(dev);
>>>+	u32 miicontrol;
>>>+	u32 microseconds = 0;
>>>+	u32 milliseconds = 0;
>>>+
>>>+	miicontrol = mii_rw(dev, np->phyaddr, MII_BMCR, MII_READ);
>>>+	miicontrol |= BMCR_RESET;
>>>+	if (mii_rw(dev, np->phyaddr, MII_BMCR, miicontrol)) {
>>>+		return -1;
>>>+	}
>>>+
>>>+	//wait for 500ms
>>>+	mdelay(500);
>>>+
>>>+	//must wait till reset is deasserted
>>>+	while (miicontrol & BMCR_RESET) {
>>>+		udelay(NV_MIIBUSY_DELAY);
>>>+		miicontrol = mii_rw(dev, np->phyaddr, MII_BMCR, MII_READ);
>>>+		microseconds++;
>>>+		if (microseconds == 20) {
>>>+			microseconds = 0;
>>>+			milliseconds++;
>>>+		}
>>>+		if (milliseconds > 50)
>>>+			return -1;
>>>+	}
>>>+	return 0;
>>>+}
>>
>>Afaiks this function is not called from a spinlocked nor is it
>>time-critical. You should make it use schedule_timeout().

Thanks for highlighting the above code. I saw it and wanted to fix it, but
then I got sidetracked and forgot.

> msleep()

Bartlomiej, could you prepare a patch to move the msleep() function from
drivers/scsi/libata-core.c to include/linux/delay.h ? This is going to
benefit users besides libata.


New version:

static int phy_reset(struct net_device *dev)
{
        struct fe_priv *np = get_nvpriv(dev);
        u32 miicontrol;
        unsigned int tries = 0;

        miicontrol = mii_rw(dev, np->phyaddr, MII_BMCR, MII_READ);
        miicontrol |= BMCR_RESET;
        if (mii_rw(dev, np->phyaddr, MII_BMCR, miicontrol)) {
                return -1;
        }

        //wait for 500ms
        msleep(500);

        //must wait till reset is deasserted
        while (miicontrol & BMCR_RESET) {
                udelay(NV_MIIBUSY_DELAY);
                miicontrol = mii_rw(dev, np->phyaddr, MII_BMCR, MII_READ);
                /* FIXME: 1000 tries seem excessive */
                if (tries++ > 1000)
                        return -1;
        }
        return 0;
}


Better?

Regards,
Carl-Daniel
-- 
http://www.hailfinger.org/

-
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/