[alsa-devel] [PATCH 7/8] ASoC: Add SPI support for WM8731

[Alan Horstmann]

On Thursday 04 September 2008 02:46, you wrote:
> On Thu, Sep 4, 2008 at 12:01 AM, Alan Horstmann <gineera at aspect135.co.uk> 
wrote:
> > On Monday 01 September 2008 18:47, Mark Brown wrote:
> >> [Modified to allow runtime selection between I2C and SPI and to select
> >> SPI_MASTER for all codecs build so this is included. -- broonie]
> > What architecture has this SPI code been tested on?  Specifically, what
> > endianess?
>
> We tested on Blackfin machine and it's little-endian.
>
> -Bryan

Thanks, Mark, Bryan.  We are trying this on AT91.  What we see is the 2 bytes 
of the SPI message swapped on the bus (data then address) and I am suspicious 
of this _spi_write function.  As I am in unfamiliar territory, please be 
patient if I am mistaken.  Analysis below...

> >> +static int wm8731_spi_write(struct spi_device *spi, const char *data,
> >> int len) +{
> >> +     struct spi_transfer t;
> >> +     struct spi_message m;
> >> +     u16 msg[2];
> >> +
> >> +     if (len <= 0)
> >> +             return 0;
> >> +
> >> +     msg[0] = (data[0] << 8) + data[1];
> >> +
> >> +     spi_message_init(&m);
> >> +     memset(&t, 0, (sizeof t));
> >> +
> >> +     t.tx_buf = &msg[0];
> >> +     t.len = len;
> >> +
> >> +     spi_message_add_tail(&t, &m);
> >> +     spi_sync(spi, &m);
> >> +
> >> +     return len;
> >> +}

I assume that spi_transfer sends bytes in order, from the tx_buf position, and 
it seems clear that data[0] is the high byte that should go first.  As msg[2] 
is u16 (why?), I would have thought that the byte at &msg[0] would be the low 
byte on LE, high byte on BE so it would only be correct on BE.

Changing the code to use 
	u8 msg[2];
and simply, but explicitly
	msg[0] = data[0];
	msg[1] = data[1];

fixes the problem here.

Comments?

Alan