Messages in this thread | | | From | "Dale Farnsworth" <> | Date | Wed, 15 Dec 2004 11:03:37 -0700 | Subject | Re: [PATCH 2/6] mv643xx_eth: replace fixed-count spin delays |
| |
On Tue, Dec 14, 2004 at 11:11:01PM +0000, Christoph Hellwig wrote: > On Mon, Dec 13, 2004 at 03:14:31PM -0700, Dale Farnsworth wrote: > > This patch removes spin delays (count to 1000000, ugh) and instead waits > > with udelay or msleep for hardware flags to change. > > > > It also adds a spinlock to protect access to the MV64340_ETH_SMI_REG, > > which is shared across ports. > > Care to add a comment with this information? Driver-global locks are > something we tend to avoid, and cases like this one where it's actually > nessecary should be properly documented.
I did have comments where the spinlock is used. I'll add one at the spinlock definition as well.
> > > + for (i=0; i<10; i++) { > > This is missing some space, should be: > > for (i = 0; i < 10; i++) {
Ok. I'll fix.
> > +#define PHY_WAIT_ITERATIONS 1000 /* 1000 iterations * 10uS = 10mS max */ > > Put this into the header or at least ontop of the file?
Will do.
> > + /* wait for the SMI register to become available */ > > + for (i=0; MV_READ(MV64340_ETH_SMI_REG) & ETH_SMI_BUSY; i++) { > > Missing spaces again. > > > + for (i=0; !(MV_READ(MV64340_ETH_SMI_REG) & ETH_SMI_READ_VALID); i++) { > > Dito. (And a few more)
I'll fix them all.
Thanks, -Dale - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |