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 _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel