On Mon, Nov 15, 2010 at 2:09 AM, Gabor Juhos <juh...@openwrt.org> 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
documented).
g.
------------------------------------------------------------------------------
Centralized Desktop Delivery: Dell and VMware Reference Architecture
Simplifying enterprise desktop deployment and management using
Dell EqualLogic storage and VMware View: A highly scalable, end-to-end
client virtualization framework. Read more!
http://p.sf.net/sfu/dell-eql-dev2dev
_______________________________________________
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
