[alsa-devel] [PATCH 1/6] ASoC: cs42xx8: Mark chip ID as volatile and remove cache bypass
Rather than manually enabling cache bypass when reading the ID registers simply mark them as volatile. The old code worked this is simply the more standard way to implement this. There is a comment included in the code that claims the chip ID register also contains the right input volume, however this is clearly not the case from the rest of the driver. Further investigation reveals exactly the same comment in the wm8962 driver, where this is the case, so this is almost certainly a copy and paste error from when the driver was created.
Signed-off-by: Charles Keepax ckeepax@opensource.wolfsonmicro.com --- sound/soc/codecs/cs42xx8.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/sound/soc/codecs/cs42xx8.c b/sound/soc/codecs/cs42xx8.c index b4d8737..1d50748 100644 --- a/sound/soc/codecs/cs42xx8.c +++ b/sound/soc/codecs/cs42xx8.c @@ -321,7 +321,6 @@ static struct snd_soc_dai_driver cs42xx8_dai = { };
static const struct reg_default cs42xx8_reg[] = { - { 0x01, 0x01 }, /* Chip I.D. and Revision Register */ { 0x02, 0x00 }, /* Power Control */ { 0x03, 0xF0 }, /* Functional Mode */ { 0x04, 0x46 }, /* Interface Formats */ @@ -352,6 +351,7 @@ static const struct reg_default cs42xx8_reg[] = { static bool cs42xx8_volatile_register(struct device *dev, unsigned int reg) { switch (reg) { + case CS42XX8_CHIPID: case CS42XX8_STATUS: return true; default: @@ -498,13 +498,6 @@ int cs42xx8_probe(struct device *dev, struct regmap *regmap) /* Make sure hardware reset done */ msleep(5);
- /* - * We haven't marked the chip revision as volatile due to - * sharing a register with the right input volume; explicitly - * bypass the cache to read it. - */ - regcache_cache_bypass(cs42xx8->regmap, true); - /* Validate the chip ID */ ret = regmap_read(cs42xx8->regmap, CS42XX8_CHIPID, &val); if (ret < 0) { @@ -523,8 +516,6 @@ int cs42xx8_probe(struct device *dev, struct regmap *regmap) dev_info(dev, "found device, revision %X\n", val & CS42XX8_CHIPID_REV_ID_MASK);
- regcache_cache_bypass(cs42xx8->regmap, false); - cs42xx8_dai.name = cs42xx8->drvdata->name;
/* Each adc supports stereo input */
Rather than manually enabling cache bypass when reading the ID registers simply mark them as volatile. The old code worked this is simply the more standard way to implement this.
Signed-off-by: Charles Keepax ckeepax@opensource.wolfsonmicro.com --- sound/soc/codecs/cs42l73.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/sound/soc/codecs/cs42l73.c b/sound/soc/codecs/cs42l73.c index 71ba560..0f9378f 100644 --- a/sound/soc/codecs/cs42l73.c +++ b/sound/soc/codecs/cs42l73.c @@ -142,6 +142,10 @@ static const struct reg_default cs42l73_reg_defaults[] = { static bool cs42l73_volatile_register(struct device *dev, unsigned int reg) { switch (reg) { + case CS42L73_DEVID_AB: + case CS42L73_DEVID_CD: + case CS42L73_DEVID_E: + case CS42L73_REVID: case CS42L73_IS1: case CS42L73_IS2: return true; @@ -1337,8 +1341,6 @@ static int cs42l73_i2c_probe(struct i2c_client *i2c_client, gpio_set_value_cansleep(cs42l73->pdata.reset_gpio, 1); }
- regcache_cache_bypass(cs42l73->regmap, true); - /* initialize codec */ ret = regmap_read(cs42l73->regmap, CS42L73_DEVID_AB, ®); devid = (reg & 0xFF) << 12; @@ -1366,8 +1368,6 @@ static int cs42l73_i2c_probe(struct i2c_client *i2c_client, dev_info(&i2c_client->dev, "Cirrus Logic CS42L73, Revision: %02X\n", reg & 0xFF);
- regcache_cache_bypass(cs42l73->regmap, false); - ret = snd_soc_register_codec(&i2c_client->dev, &soc_codec_dev_cs42l73, cs42l73_dai, ARRAY_SIZE(cs42l73_dai));
Rather than manually enabling cache bypass when reading the ID registers simply mark them as volatile, also remove the default values since defaults for volatile registers don't make sense. The old code worked this is simply the more standard way to implement this.
Signed-off-by: Charles Keepax ckeepax@opensource.wolfsonmicro.com --- sound/soc/codecs/cs42l56.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/sound/soc/codecs/cs42l56.c b/sound/soc/codecs/cs42l56.c index 54c1768..9bfca5d 100644 --- a/sound/soc/codecs/cs42l56.c +++ b/sound/soc/codecs/cs42l56.c @@ -64,8 +64,6 @@ struct cs42l56_private { };
static const struct reg_default cs42l56_reg_defaults[] = { - { 1, 0x56 }, /* r01 - ID 1 */ - { 2, 0x04 }, /* r02 - ID 2 */ { 3, 0x7f }, /* r03 - Power Ctl 1 */ { 4, 0xff }, /* r04 - Power Ctl 2 */ { 5, 0x00 }, /* ro5 - Clocking Ctl 1 */ @@ -125,6 +123,8 @@ static bool cs42l56_readable_register(struct device *dev, unsigned int reg) static bool cs42l56_volatile_register(struct device *dev, unsigned int reg) { switch (reg) { + case CS42L56_CHIP_ID_1: + case CS42L56_CHIP_ID_2: case CS42L56_INT_STATUS: return true; default: @@ -1262,8 +1262,6 @@ static int cs42l56_i2c_probe(struct i2c_client *i2c_client, return ret; }
- regcache_cache_bypass(cs42l56->regmap, true); - ret = regmap_read(cs42l56->regmap, CS42L56_CHIP_ID_1, ®); devid = reg & CS42L56_CHIP_ID_MASK; if (devid != CS42L56_DEVID) { @@ -1279,8 +1277,6 @@ static int cs42l56_i2c_probe(struct i2c_client *i2c_client, dev_info(&i2c_client->dev, "Alpha Rev %X Metal Rev %X\n", alpha_rev, metal_rev);
- regcache_cache_bypass(cs42l56->regmap, false); - if (cs42l56->pdata.ain1a_ref_cfg) regmap_update_bits(cs42l56->regmap, CS42L56_AIN_REFCFG_ADC_MUX, CS42L56_AIN1A_REF_MASK, 1);
Mark the chip ID register as volatile, the current code does work as without a default value regmap will read from the hardware the first time, however, this makes it more explicit that this register should be read from the hardware.
Signed-off-by: Charles Keepax ckeepax@opensource.wolfsonmicro.com --- sound/soc/codecs/cs4265.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/sound/soc/codecs/cs4265.c b/sound/soc/codecs/cs4265.c index fd966bb..f8ad458 100644 --- a/sound/soc/codecs/cs4265.c +++ b/sound/soc/codecs/cs4265.c @@ -70,6 +70,7 @@ static bool cs4265_readable_register(struct device *dev, unsigned int reg) static bool cs4265_volatile_register(struct device *dev, unsigned int reg) { switch (reg) { + case CS4265_CHIP_ID: case CS4265_INT_STATUS: return true; default:
Mark the chip ID register as volatile, the current code does work as without a default value regmap will read from the hardware the first time, however, this makes it more explicit that this register should be read from the hardware.
Signed-off-by: Charles Keepax ckeepax@opensource.wolfsonmicro.com --- sound/soc/codecs/cs42l52.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/sound/soc/codecs/cs42l52.c b/sound/soc/codecs/cs42l52.c index 0d9c4a5..1164990 100644 --- a/sound/soc/codecs/cs42l52.c +++ b/sound/soc/codecs/cs42l52.c @@ -120,6 +120,7 @@ static bool cs42l52_readable_register(struct device *dev, unsigned int reg) static bool cs42l52_volatile_register(struct device *dev, unsigned int reg) { switch (reg) { + case CS42L52_CHIP: case CS42L52_IFACE_CTL2: case CS42L52_CLK_STATUS: case CS42L52_BATT_LEVEL:
Mark the chip ID registers as volatile, the current code does work as without a default value regmap will read from the hardware the first time, however, this makes it more explicit that this register should be read from the hardware.
Signed-off-by: Charles Keepax ckeepax@opensource.wolfsonmicro.com --- sound/soc/codecs/cs53l30.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/cs53l30.c b/sound/soc/codecs/cs53l30.c index cb47fb5..b4f0a43 100644 --- a/sound/soc/codecs/cs53l30.c +++ b/sound/soc/codecs/cs53l30.c @@ -89,10 +89,16 @@ static const struct reg_default cs53l30_reg_defaults[] = {
static bool cs53l30_volatile_register(struct device *dev, unsigned int reg) { - if (reg == CS53L30_IS) + switch (reg) { + case CS53L30_DEVID_AB: + case CS53L30_DEVID_CD: + case CS53L30_DEVID_E: + case CS53L30_REVID: + case CS53L30_IS: return true; - else + default: return false; + } }
static bool cs53l30_writeable_register(struct device *dev, unsigned int reg)
On Mon, 24 Oct 2016, Charles Keepax wrote:
Rather than manually enabling cache bypass when reading the ID registers simply mark them as volatile. The old code worked this is simply the more standard way to implement this. There is a comment included in the code that claims the chip ID register also contains the right input volume, however this is clearly not the case from the rest of the driver. Further investigation reveals exactly the same comment in the wm8962 driver, where this is the case, so this is almost certainly a copy and paste error from when the driver was created.
Signed-off-by: Charles Keepax ckeepax@opensource.wolfsonmicro.com
For the series <Acked-by: Brian Austin brian.austin@cirrus.com
On Mon, Oct 24, 2016 at 10:55:44AM +0100, Charles Keepax wrote:
Rather than manually enabling cache bypass when reading the ID registers simply mark them as volatile. The old code worked this is simply the more standard way to implement this. There is a comment included in the
Even better just remove the register default, with rbtree regmap will do the read and then cache it - no need to mark as volatile.
On Mon, Oct 24, 2016 at 04:33:42PM +0100, Mark Brown wrote:
On Mon, Oct 24, 2016 at 10:55:44AM +0100, Charles Keepax wrote:
Rather than manually enabling cache bypass when reading the ID registers simply mark them as volatile. The old code worked this is simply the more standard way to implement this. There is a comment included in the
Even better just remove the register default, with rbtree regmap will do the read and then cache it - no need to mark as volatile.
:-) well if we are preferring that then some of the series can just be dropped, as they were changing exactly that to be explicitly marked volatile.
I will fixup the remaining patches and resend.
Thanks, Charles
participants (3)
-
Brian Austin -
Charles Keepax -
Mark Brown