[alsa-devel] snd soc spi read/write

[Takashi Iwai]

At Fri, 5 Aug 2011 10:24:43 +0800,
Scott Jiang wrote:
> 
> 2011/8/4 Mark Brown <broonie at opensource.wolfsonmicro.com>:
> > On Thu, Aug 04, 2011 at 06:24:04PM +0800, Scott Jiang wrote:
> >
> >> My version is linux 3.0, I found snd_soc_spi_read/write didn't work at
> >> 16-8 bit mode.
> >> The addr is 16bit, the function compares reg with reg_cache_size. This
> >> certainly causes failure.
> >> In the former version, reg &= 0xff seems right. So how to use
> >> snd_soc_16_8_write now?
> >
> > reg &= 0xff is clearly broken for 16 bit register values...
> 
> hw_write use 16bit reg, cache use 8bit reg, in former version
> 
>         data[0] = (reg >> 8) & 0xff;
>         data[1] = reg & 0xff;
>         data[2] = value;
> 
>         reg &= 0xff;
>         if (reg < codec->reg_cache_size)
>                 cache[reg] = value;
> 
>         ret = codec->hw_write(codec->control_data, data, 3);
> 
> now in do_hw_write
>         if (!snd_soc_codec_volatile_register(codec, reg) &&
>             reg < codec->driver->reg_cache_size &&
>             !codec->cache_bypass) {
>                 ret = snd_soc_cache_write(codec, reg, value);
>                 if (ret < 0)
>                         return -1;
>         }
> reg > reg_cache_size, so will not write to cache

The problem is that the current cache code doesn't take care of
special register-index values like 16_8 case.  In 16-8 case, usually
only the lower 8 bit indicates the real register index while the upper
8 bit is some (usually constant) control bits.  Thus, it worked well
in the former version by masking with 0xff.

A quick fix would to introduce again the 0xff masking.  Of course,
this isn't a right thing, but would be enough for the current code,
as regmap will replace in future anyway.

> > Note that all this code will be replaced with regmap for 3.2.
> >
> 
> Do you mean I must update to 3.2 to solve this problem?

Not yet :)  regmap has the same issue for now, as far as I see.

I guess reg_mask_bits is needed in struct regmap_config in addition
for supporting such a case.


thanks,

Takashi