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.
Regards,
Gabor
