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.
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.
+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.