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) &® < 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