Re: [PATCH] net: smsc911x: add power management functions

[Daniel Mack <daniel@caiaq.de>]

On Tue, May 05, 2009 at 09:37:42AM +0100, Steve.Glendinning@smsc.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