On Wed, 2010-10-06 at 16:24 +0200, Takashi Iwai wrote: > At Wed, 06 Oct 2010 15:13:42 +0100, > Dimitris Papastamos wrote: > > > > On Wed, 2010-10-06 at 15:46 +0200, Takashi Iwai wrote: > > > At Wed, 06 Oct 2010 14:40:04 +0100, > > > Dimitris Papastamos wrote: > > > > > > > > On Wed, 2010-10-06 at 15:30 +0200, Takashi Iwai wrote: > > > > > At Wed, 06 Oct 2010 10:25:40 +0100, > > > > > Dimitris Papastamos wrote: > > > > > > > > > > > > On Wed, 2010-10-06 at 08:31 +0100, Mark Brown wrote: > > > > > > On Wed, Oct 06, 2010 at 09:10:23AM +0200, Takashi Iwai wrote: > > > > > > > > > > > > > > > If the above difference is intentional, it should be commented > > > > > > > > somewhere. > > > > > > > > > > > > > > Meh, yes. Dimitris, please fix or add a comment as appropriate. > > > > > > > > > > > > > > > > > > > In snd_soc_7_9_spi_write we prepare the tx buffer to be register > > > > > > followed by data packed into 16 bits. In snd_soc_4_12_spi_write the tx > > > > > > buffer is swapped. I'd expect this to be consistent between the two > > > > > > transfers. > > > > > > > > > > Sorry, I don't understand your statement clearly. > > > > > So, the byte-swap behavior in snd_soc_4_12_spi_write() is designed? > > > > > > > > I meant to say that snd_soc_4_12_spi_write looks suspicious and that I'd > > > > expect it to behave similarly to snd_soc_7_9_spi_write. I don't see why > > > > the byte swapping is needed. > > > > > > OK, thanks for clarifying :) > > > > > > Looking through git commits, this was introduced by a patch from Barry > > > Song. Barry, could you check whether the current code is correct? > > > > > > I guess this is because the original code accessed unsigned short > > > while the new code is converted to a byte array. Maybe due to the > > > endianess, but it looks wrong. > > > > Looking at the commit now, I think his changes are consistent. Consider > > an example with reg = 0xf and value = 0xa, then his original unsigned > > short buf is equal to 0xf00a. If you follow the logic in > > snd_soc_4_12_write you will find that data[0] and data[1] are 0xf0 and > > 0x0a respectively. However on a say LE machine, his original buf is > > laid out in mem as 0af0 whereas the data buffer is laid out as f00a so > > he has to reverse that by swapping it in snd_soc_4_12_spi_write. > > Yeah, I don't say it's buggy. But it's wrong from a programming POV. > The proper endianess handling seems missing in the current code... Yup, I will wait for Mark to see what he has to comment on this and then I will make the required changes. Thanks, Dimitrios _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
