[PATCH v2 1/2] spi:fsl-dspi:add support of DSPI IP in big endian

Chao Fu B44548 at freescale.com
Fri Jan 10 03:11:33 EST 2014


> This looks a lot nicer - a few things below though.
> 
> > +#define DSPI_BITWISE16(d, v)	(d->big_endian ? cpu_to_be16(v) :
> cpu_to_le16(v))
> > +#define DSPI_BITWISE32(d, v)	(d->big_endian ? cpu_to_be32(v) :
> cpu_to_le32(v))
> 
> These should probably be inline for the same reason as the I/O functions,
> perhaps even being done as part of the I/O functions.
> 
[Chao Fu] Thank you, Mark! I wiil put them into inline functions.

> > +static inline u16 dspi_readw(void __iomem *addr) {
> > +	u16 __v = (__force u16) __raw_readw(addr);
> > +	__iormb();
> > +
> > +	return __v;
> > +}
> 
> Why does this need a barrier and why does it not take care of the
> endianness translation?  A quick glance through shows most if not all of
> the callers doing the translation.
>
[Chao Fu] Our CPUs are working on only ARM architecture. But DSPI have two endianness 
In different series CPU, so we use __raw_read that do not take of endianness.
We need observe ARM io methods  make sure avoid instructions executing reorder .
ARM IO methods :
#define readb(c)                ({ u8  __v = readb_relaxed(c); __iormb(); __v; })
#define readw(c)                ({ u16 __v = readw_relaxed(c); __iormb(); __v; })
#define readl(c)                ({ u32 __v = readl_relaxed(c); __iormb(); __v; })

#define writeb(v,c)             ({ __iowmb(); writeb_relaxed(v,c); })
#define writew(v,c)             ({ __iowmb(); writew_relaxed(v,c); })
#define writel(v,c)             ({ __iowmb(); writel_relaxed(v,c); })

So add barrier here. Could you give some suggestions? Many thanks!
 
> > +static inline void dspi_writew(u16 val, void __iomem *addr) {
> > +	__iowmb();
> > +	__raw_writew((__force u16) val, addr); }
> 
> Again the memory barrier seems odd, especially the barrier *before* doing
> the write.



More information about the linux-arm-kernel mailing list