[alsa-devel] [PATCH 1/4] ASoC: clean up cache read/write functions

[Takashi Iwai]

At Mon, 20 Dec 2010 17:24:05 +0000,
Mark Brown wrote:
> 
> On Mon, Dec 20, 2010 at 05:01:09PM +0100, Takashi Iwai wrote:
> 
> > +static int do_hw_write(struct snd_soc_codec *codec, unsigned int reg,
> > +                      unsigned int value, void *data, unsigned int size)
> > +{
> > +       int ret;
> > +
> > +       if (!snd_soc_codec_volatile_register(codec, reg) &&
> > +           reg < codec->driver->reg_cache_size) {
> > +               ret = snd_soc_cache_write(codec, reg, value);
> > +               if (ret < 0)
> > +                       return -1;
> > +       }
> 
> This isn't actually doing a hardware write, though - it's the entire
> write path which may or may not end up at the hardware.  The whole
> passing of both the mangled and unmangled versions also feels a bit odd
> here.

Yes, this could be done better in the common place before write op is
called.

> I think it'd be clearer to do this by making this a plain function and
> adding a mangle operation set by the cache types which gets called out
> to at the appropriate moment, that'd probably make the code flow more
> naturally.

Sounds reasonable.

> > +static unsigned int snd_soc_4_12_read(struct snd_soc_codec *codec,
> > +				      unsigned int reg)
> > +{
> > +	return do_hw_read(codec, reg);
> > +}
> > +
> 
> >  static unsigned int snd_soc_7_9_read(struct snd_soc_codec *codec,
> >  				     unsigned int reg)
> >  {
> > -	int ret;
> > -	unsigned int val;
> > -
> > -	if (reg >= codec->driver->reg_cache_size ||
> > -		snd_soc_codec_volatile_register(codec, reg)) {
> > -			if (codec->cache_only)
> > -				return -1;
> > -
> > -			BUG_ON(!codec->hw_read);
> > -			return codec->hw_read(codec, reg);
> > -	}
> > -
> > -	ret = snd_soc_cache_read(codec, reg, &val);
> > -	if (ret < 0)
> > -		return -1;
> > -	return val;
> > +	return do_hw_read(codec, reg);
> 
> If this is OK to do we should just be making do_hw_read() the operation
> directly.

My patch is just a clean up, and I kept the code "reg &= 0xff" in some
places.  Without these, all can be the same function.


Takashi