[Top] [All Lists]

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

To: Gabor Juhos <>, Ralf Baechle <>
Subject: Re: [RFC 11/18] spi: add SPI controller driver for the Atheros AR71XX/AR724X/AR913X SoCs
From: Grant Likely <>
Date: Mon, 15 Nov 2010 09:28:05 -0700
Cc:, "Luis R. Rodriguez" <>, Cliff Holden <>, David Brownell <>,, Imre Kaloz <>
In-reply-to: <>
Original-recipient: rfc822;
References: <> <> <> <> <> <>
On Mon, Nov 15, 2010 at 2:09 AM, Gabor Juhos <> wrote:
> 2010.11.15. 5:04 keltezéssel, Grant Likely írta:
>> 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.
> These SoCs are using the MIPS 24K core. This core is based on an in-order
> architecture, so it is safe to use the __raw versions from the CPU's side.
> To be honest, I have no informations about that the completion of the request 
> is
> always in order that the request are received on the AHB bus between the CPU 
> and
> the SPI controller. However the Atheros' reference code uses the __raw 
> versions
> everywhere to access the registers of the built-in devices, so I assume that 
> no
> out-of-order completion is allowed on that bus.

Ralf, what say you?  I personally don't like this, and it makes for a
bad example of driver code, but I'll accept it if you say that it is
the right thing to do for MIPS device drivers.  (Although I retain my
requirement that the use of __raw accessors needs to be well


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