On Tue, Feb 16, 2016 at 05:32:56PM +0000, Srinivas Kandagatla wrote:
This patch add register ranges for readonly, default and reset register values.
This split of the patches is really not helping at all. The patches cross reference each other which makes things harder to follow and it's not like things can be treated independently or there are detaile changelogs explaining everything separately.
+static int __msm8x16_wcd_reg_write(struct snd_soc_codec *codec,
unsigned short reg, u8 val)+{
- int ret = -EINVAL;
- struct msm8x16_wcd_chip *chip = dev_get_drvdata(codec->dev);
- if (MSM8X16_WCD_IS_TOMBAK_REG(reg)) {
ret = regmap_write(chip->analog_map,chip->analog_base + reg, val);- } else if (MSM8X16_WCD_IS_DIGITAL_REG(reg)) {
u32 temp = val & 0x000000FF;u16 offset = (reg ^ 0x0200) & 0x0FFF;ret = regmap_write(chip->digital_map, offset, temp);- }
- return ret;
+}
I don't really know what this is supposed to be doing but it looks like something is wrong. It seems that it's trying to munge two different register maps together in some undocumented reason.
+static int msm8x16_wcd_write(struct snd_soc_codec *codec, unsigned int reg,
unsigned int value)+{
- if (reg == SND_SOC_NOPM)
return 0;- WARN_ON(reg > MSM8X16_WCD_MAX_REGISTER);
- if (!msm8x16_wcd_volatile(codec, reg))
msm8x16_wcd_reset_reg_defaults[reg] = value;
This appears to be obviously confused. We're writing to a global variable as part of the write routine, and worse that global variable is called _reg_defaults which suggests that it's supposed to be the register default values not what appears to be a cache implemented outside of the existing generic cache code.
- return __msm8x16_wcd_reg_write(codec, reg, (u8)value);
+}
It's also not clear why this is a separate wrapper function.