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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

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