[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Tue, 5 May 2009 10:40:46 +0200
From: Daniel Mack <daniel@...aq.de>
To: Steve.Glendinning@...c.com
Cc: netdev@...r.kernel.org, David Miller <davem@...emloft.net>,
Ian.Saturley@...c.com
Subject: Re: [PATCH] net: smsc911x: add power management functions
On Tue, May 05, 2009 at 09:37:42AM +0100, Steve.Glendinning@...c.com wrote:
> > This adds a power management implementation for smsc911x.c which assumes
> > the chips remains powered during suspend. The device is put in its D1
> > power saving mode.
>
> [....]
>
> > +static int smsc911x_resume(struct platform_device *pdev)
> > +{
> > + struct net_device *dev = platform_get_drvdata(pdev);
> > + struct smsc911x_data *pdata = netdev_priv(dev);
> > + unsigned int to = 100;
> > +
> > + /* Note 3.11 from the datasheet:
> > + * "When the LAN9220 is in a power saving state, a write of any
> > + * data to the BYTE_TEST register will wake-up the device."
> > + */
> > + smsc911x_reg_write(pdata, BYTE_TEST, 0);
> > +
> > + /* poll the READY bit in PMT_CTRL. Any other access to the device is
> > + * forbidden while this bit isn't set. Try for 100ms and return -EIO
> > + * if it failed. */
> > + while (!(smsc911x_reg_read(pdata, PMT_CTRL) & PMT_CTRL_READY_) &&
> to--)
> > + udelay(1000);
> > +
> > + return (to == 0) ? -EIO : 0;
> > +}
>
> to-- should be --to, otherwise it'll end up as -1 when the loop times out
> (and the error code return below won't work). Roel Kluin recently fixed
> many instances of this.
Yep, that's been pointed out by Enrik Berkhan already. David Miller
offered to fix that on the fly when commiting.
> Other than that, the power saving logic looks fine.
Thanks for your review :)
Daniel
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists