[alsa-devel] [PATCH 1/1] ASoC: core: cache index fix

[Takashi Iwai]

At Wed, 3 Aug 2011 01:40:06 +0900,
Mark Brown wrote:
> 
> On Tue, Aug 02, 2011 at 06:13:11PM +0200, Takashi Iwai wrote:
> > Mark Brown wrote:
> 
> > > Like I've indicated several times now we should just get rid of the code
> > > or hide it from the rest of the subsystem, it's being too cute for
> > > vanishingly little value.  The register maps for these devices are
> > > usually at most 255 registers so the memory savings are really not
> > > meaningful.  I'm hoping the guys working with this device will find time
> > > to look at fixing things, but if not I'd imagine we'll get to it at some
> > > point in the release cycle.
> 
> > Well, there aren't so many drivers suffering from this bug, so a
> > temporary fix would be easy like below (totally untested).
> 
> If we're going to do something like this I'd preserve the driver
> interface that's there rather than fiddling with their reg_cache_sizes -
> half the trouble here is that the meaning of that has become a bit
> slippery, the current code used to be correct.

I don't mind either way as long as it gets fixed in way applicable
to stable kernel tree.

> > @@ -421,7 +422,9 @@ static int snd_soc_rbtree_cache_init(struct snd_soc_codec *codec)
> >  		return 0;
> >  
> >  	word_size = codec->driver->reg_word_size;
> > -	for (i = 0; i < codec->driver->reg_cache_size; ++i) {
> > +	if (codec->driver->reg_cache_step)
> > +		step = codec->driver->reg_cache_step;
> > +	for (i = 0; i < codec->driver->reg_cache_size; i += step) {
> >  		val = snd_soc_get_cache_val(codec->reg_def_copy, i,
> >  					    word_size);
> >  		if (!val)
> 
> I'm also really unhappy with handling this in the complex caches, I'd be
> much more inclined to just disallow their use with devices with step
> sizes than to add any complexity to them.

Yeah, I find it's ugly, too.  OTOH, reg_cache_step defines the
validity of the register access, so we can't drop it completely.
Accessing the odd number of register index is invalid when step=2, for
example.  So, alternatively, we can put the condition in some generic
filter function like readable/writeble things.


thanks,

Takashi