[alsa-devel] [PATCH 3.1]ASoC: ad193x: add spi_hw_read, fix sysclk and register definition

[Scott Jiang]

2011/8/11 Mark Brown <broonie at opensource.wolfsonmicro.com>:
> On Thu, 2011-08-11 at 17:04 +0800, Scott Jiang wrote:
>
>> @@ -56,7 +56,7 @@ static int bf5xx_ad193x_hw_params(struct
>> snd_pcm_substream *substream,
>>
>>       switch (params_rate(params)) {
>>       case 48000:
>> -             clk = 12288000;
>> +             clk = 24576000;
>>               break;
>>       }
>
> Whatever this is for it should be a separate change.
>
>> -     reg = (reg & (~AD193X_DAC_WORD_LEN_MASK)) | word_len;
>> +     reg = (reg & (~AD193X_DAC_WORD_LEN_MASK))
>> +             | (word_len << AD193X_DAC_WORD_LEN_SHFT);
>>       snd_soc_write(codec, AD193X_DAC_CTRL2, reg);
>>
>>       reg = snd_soc_read(codec, AD193X_ADC_CTRL1);
>
> This needs some documentation as it's not clear what it's supposed to
> do.
>
the word_len value forgot to shift, I think it's clear.

>> +#if defined(CONFIG_SPI_MASTER)
>> +     /* asoc cache layer can't support this kind of spi registers now */
>> +     codec->cache_bypass = 1;
>> +#endif
>
> That's not what cache_bypass is for. Just mark the registers as
> volatile, or remove the cache entirely.
>
volatile usually used on the read-only registers, so I think removing
cache is better.

>>  #ifdef CONFIG_SPI_MASTER
>>                 codec->hw_write = do_spi_write;
>>  #endif
>> +               if (io_types[i].spi_read)
>> +                       codec->hw_read = io_types[i].spi_read;
>
> So, if we've got an ifdef on the write...
>
sorry, I can't understand what you mean. I just follow i2c code above.