Re: [PATCH 2/6] mv643xx_eth: replace fixed-count spin delays

["Dale Farnsworth" <dale@farnsworth.org>]

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