Re: [PATCH] spi: SuperH MSIOF SPI Master driver

[Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>]

On Thu, 26 Nov 2009 15:43:16 +0900 Paul Mundt <lethal-M7jkjyW5wf5g9hUCZPvPmw@public.gmane.org> wrote:

> > > +static void sh_msiof_modify_ctr_wait(struct sh_msiof_spi_priv *p,
> > > +				     unsigned long clr, unsigned long set)
> > > +{
> > > +	unsigned long mask = clr | set;
> > > +	unsigned long data;
> > > +
> > > +	data = sh_msiof_read(p, CTR);
> > > +	data &= ~clr;
> > > +	data |= set;
> > > +	sh_msiof_write(p, CTR, data);
> > > +
> > > +	while ((sh_msiof_read(p, CTR) & mask) != set)
> > > +		;
> > 
> > hm, confidence.  No timeout needed here?
> > 
> This definitely needs a timeout, nothing involving SPI inspires
> confidence. A cpu_relax() to prevent the compiler from optimizing the
> loop out would help, too.

We generally don't bother with the relax in an IO polling loop
like this.  It involves an IO read/write which the compiler cannot fiddle
with and I believe that CPUs will generally take the opportunity to have
a little snooze while the slow IO operation is happening.

Can't hurt though ;)

------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with
Crystal Reports now.  http://p.sf.net/sfu/bobj-july