linux-mips
[Top] [All Lists]

Re: [RFC 11/18] spi: add SPI controller driver for the Atheros AR71XX/AR

To: Gabor Juhos <juhosg@openwrt.org>
Subject: Re: [RFC 11/18] spi: add SPI controller driver for the Atheros AR71XX/AR724X/AR913X SoCs
From: Grant Likely <grant.likely@secretlab.ca>
Date: Sun, 14 Nov 2010 21:04:56 -0700
Cc: Ralf Baechle <ralf@linux-mips.org>, linux-mips@linux-mips.org, "Luis R. Rodriguez" <mcgrof@gmail.com>, Cliff Holden <Cliff.Holden@Atheros.com>, David Brownell <dbrownell@users.sourceforge.net>, spi-devel-general@lists.sourceforge.net, Imre Kaloz <kaloz@openwrt.org>
In-reply-to: <4CE04EBC.4080701@openwrt.org>
Original-recipient: rfc822;linux-mips@linux-mips.org
References: <1289598684-30624-1-git-send-email-juhosg@openwrt.org> <1289598684-30624-12-git-send-email-juhosg@openwrt.org> <20101114082242.GA3137@angua.secretlab.ca> <4CE04EBC.4080701@openwrt.org>
Sender: linux-mips-bounce@linux-mips.org
User-agent: Mutt/1.5.20 (2009-06-14)
On Sun, Nov 14, 2010 at 10:03:56PM +0100, Gabor Juhos wrote:
> >> +static inline u32 ath79_spi_rr(struct ath79_spi *sp, unsigned reg)
> >> +{
> >> +  return __raw_readl(sp->base + reg);
> >> +}
> >> +
> >> +static inline void ath79_spi_wr(struct ath79_spi *sp, unsigned reg, u32 
> >> val)
> >> +{
> >> +  __raw_writel(val, sp->base + reg);
> >> +}
> > 
> > This is suspect.  Why is __raw_{readl,writel} being used instead of
> > ioread32/iowrite32?  The __raw versions don't provide any kind of
> > ordering barriers.
> 
> Mainly because the resulting code is smaller, and the performance is a bit
> better with the use of the __raw versions. The controller is embedded into the
> SoC and the registers are memory mapped, so i think it is safe to access them
> with __raw_{readl,writel}. However I can change it if that is the preferred 
> method.
> 

Smaller, yes, because it doesn't have any io barriers; but is it safe?
Do you know whether or not the CPU will reorder the instructions on
you?  Being embedded into the SoC doesn't really mean anything in this
regard.  Unless you really understand all the behaviour of the CPU and
bus, then the safe versions must be used.

If you *do* really understand all the behaviour and decide it is safe
to use the __raw versions, then the driver needs to be well documented
as to the reasons why the __raw versions are safe to use.

g.


<Prev in Thread] Current Thread [Next in Thread>