[alsa-devel] [Uclinux-dist-devel] [PATCH 2/2] ASoC: Blackfin AD1836/AD1938 machine drivers: require SPI master

[Barry Song]

On Thu, Oct 8, 2009 at 6:01 PM, Mark Brown
<broonie at opensource.wolfsonmicro.com> wrote:
> On Wed, Oct 07, 2009 at 08:27:56PM -0400, Mike Frysinger wrote:
>
>> i dont think the soc-cache could be used currently by the ad1836 as it
>> doesnt use a 7/9 split with the address/data.  it uses like 6/10 (4
>> addr bits, one bit for read/write, one bit is always 0, and 10 data
>> bits).  guessing the write bit can be considered part of the addr as
>> the read always comes from the cache, but that still gives us 5/10
>> split.  maybe a new 6/10 set of funcs should be added ...
>
> That's the idea - add new functions for any new register formats.
we do those based on the idea that it is a waste all codecs need to
use almost same ways to handle register access. If we use soc-cache to
handle register access issues and run as abstract layer for all
codecs, why not rename it to soc-reg or soc-bus? It seems cache is
only a little part of the responsibility of  this module.
It's better that functions like xx_7_9_read xx_7_9_write xx_8_8_read
xx_8_8_write should not become API because we never know what will be
the format for codecs. We should have a xx_n_m_read/xx_n_m_write or
just a xx_read/xx_write, with n and m as parameters?
A codec using snd_soc_7_9_spi_write/snd_soc_8_16_read_i2c should
select SPI and I2C in fact. Otherwise, why does it use these functions
as codec->hw_read()/codec->hw_write()  but not enable SPI and I2C? It
seems that defining snd_soc_7_9_spi_write and so on as NULL when SPI
is not selected is really useless, just let us pass the compiling to
get a module which can't work at all!

>
>> snd_soc_7_9_write() looks like it does a little more bit work than it
>> needs to ?  if data is declared as a u16, then you have:
>> u16 data = (reg << 9) | (value & 0x01ff);
>> this is what the ad1836 driver does now for its data split.
>
> Probably.  I'd need to check but I believe that's there to handle
> endianness variations in the host, though a cpu_to_ in what you have
> above ought to be able to take care of that.  The code was cut'n'pasted
> from what was in the drivers already.
>
>> in the mean time, rather than adding #ifdef to the codec driver, we
>> could create a local header like "bus-stubs.h" that stubs all the
>> relevant functions to an error value.  then all codec drivers that
>> dont use soc-cache can use that instead and the only change needed is
>> to add:
>> #include "bus-stubs.h"
>
> I'm not sure I feel up to doing that locally in ASoC rather than in the
> relevant subsystems.
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>