Date: Wed, 25 Nov 2009 23:07:48 -0800
From: Andrew Morton <akpm@linux-foundation.org>
To: Paul Mundt <lethal@linux-sh.org>
Cc: Magnus Damm <magnus.damm@gmail.com>,
       spi-devel-general@lists.sourceforge.net,
       dbrownell@users.sourceforge.net, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] spi: SuperH MSIOF SPI Master driver
Message-Id: <20091125230748.dbb7cf09.akpm@linux-foundation.org>
In-Reply-To: <20091126064315.GA6580@linux-sh.org>
References: <20091124125531.14957.41264.sendpatchset@rxone.opensource.se>
	<20091125121152.4ccf03dc.akpm@linux-foundation.org>
	<20091126064315.GA6580@linux-sh.org>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1237
Lines: 33

On Thu, 26 Nov 2009 15:43:16 +0900 Paul Mundt <lethal@linux-sh.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 ;)
--
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/