[PATCH 0/5] ASoC/MFD: twl6040: Remove codec driver local reg cache

[PATCH 0/5] ASoC/MFD: twl6040: Remove codec driver local reg cache

[Peter Ujfalusi]

Hi Mark,

It was not straight forward task to remove the local reg cache from the twl6040
codec driver. I also needed to change the MFD part so it will take over the
caching of registers via regmap.
So far I have the following patches working on top of mainline kernel.

Is this close enough of what you had in mind when asked me to look at this?

twl4030 codec is going to be a bit more complicated AFAIK but it will come
eventually.

Regards,
Peter
---
Peter Ujfalusi (5):
  ASoC: twl6040: Rename twl6040_is_path_unmuted ->
    twl6040_can_write_to_chip
  ASoC: twl6040: Custom caching for sensitive DL1/2 path registers
  MFD: twl6040: reg_defaults support for regmap
  ASoC: twl6040: Remove register restore functionality
  ASoC: twl6040: Remove self managed local reg_cache support

 drivers/mfd/twl6040.c      |  92 +++++++++++++++++-
 sound/soc/codecs/twl6040.c | 227 +++++++++++++--------------------------------
 2 files changed, 155 insertions(+), 164 deletions(-)

-- 
1.8.4.2

[PATCH 1/5] ASoC: twl6040: Rename twl6040_is_path_unmuted -> twl6040_can_write_to_chip

[Peter Ujfalusi]

Matches more precisely of the functionality.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 sound/soc/codecs/twl6040.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/sound/soc/codecs/twl6040.c b/sound/soc/codecs/twl6040.c
index f2f4bcb..ef13a50 100644
--- a/sound/soc/codecs/twl6040.c
+++ b/sound/soc/codecs/twl6040.c
@@ -219,8 +219,8 @@ static int twl6040_read_reg_volatile(struct snd_soc_codec *codec,
 	return value;
 }
 
-static bool twl6040_is_path_unmuted(struct snd_soc_codec *codec,
-				    unsigned int reg)
+static bool twl6040_can_write_to_chip(struct snd_soc_codec *codec,
+				  unsigned int reg)
 {
 	struct twl6040_data *priv = snd_soc_codec_get_drvdata(codec);
 
@@ -250,7 +250,7 @@ static int twl6040_write(struct snd_soc_codec *codec,
 		return -EIO;
 
 	twl6040_write_reg_cache(codec, reg, value);
-	if (twl6040_is_path_unmuted(codec, reg))
+	if (twl6040_can_write_to_chip(codec, reg))
 		return twl6040_reg_write(twl6040, reg, value);
 	else
 		return 0;
-- 
1.8.4.2

[PATCH 2/5] ASoC: twl6040: Custom caching for sensitive DL1/2 path registers

[Peter Ujfalusi]

Introduce a small register cache for registers which needs special caching
to reduce pop noise:
TWL6040_REG_HSLCTL, TWL6040_REG_HSRCTL, TWL6040_REG_EARCTL, TWL6040_REG_HFLCTL
and TWL6040_REG_HFRCTL.
Switch over and use the new small cache for these registers instead of the
main reg_cache.
This is in preparation to remove the local ASoC reg_cache from the driver.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 sound/soc/codecs/twl6040.c | 51 ++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 49 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/twl6040.c b/sound/soc/codecs/twl6040.c
index ef13a50..fb8c65b 100644
--- a/sound/soc/codecs/twl6040.c
+++ b/sound/soc/codecs/twl6040.c
@@ -72,6 +72,7 @@ struct twl6040_data {
 	int hs_power_mode_locked;
 	bool dl1_unmuted;
 	bool dl2_unmuted;
+	u8 dl12_cache[TWL6040_REG_HFRCTL - TWL6040_REG_HSLCTL + 1];
 	unsigned int clk_in;
 	unsigned int sysclk;
 	struct twl6040_jack_data hs_jack;
@@ -174,18 +175,62 @@ static struct snd_pcm_hw_constraint_list sysclk_constraints[] = {
 	{ .count = ARRAY_SIZE(hp_rates), .list = hp_rates, },
 };
 
+static inline int twl6040_read_dl12_cache(struct snd_soc_codec *codec,
+					   u8 reg, u8 *value)
+{
+	struct twl6040_data *priv = snd_soc_codec_get_drvdata(codec);
+	int ret = 0;
+
+	switch (reg) {
+	case TWL6040_REG_HSLCTL:
+	case TWL6040_REG_HSRCTL:
+	case TWL6040_REG_EARCTL:
+	case TWL6040_REG_HFLCTL:
+	case TWL6040_REG_HFRCTL:
+		*value = priv->dl12_cache[reg - TWL6040_REG_HSLCTL];
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	return ret;
+}
+
 /*
  * read twl6040 register cache
  */
 static inline unsigned int twl6040_read_reg_cache(struct snd_soc_codec *codec,
-						unsigned int reg)
+						  unsigned int reg)
 {
 	u8 *cache = codec->reg_cache;
+	u8 value;
 
 	if (reg >= TWL6040_CACHEREGNUM)
 		return -EIO;
 
-	return cache[reg];
+	if (twl6040_read_dl12_cache(codec, reg, &value))
+		value = cache[reg];
+
+	return value;
+}
+
+static inline void twl6040_update_dl12_cache(struct snd_soc_codec *codec,
+					     u8 reg, u8 value)
+{
+	struct twl6040_data *priv = snd_soc_codec_get_drvdata(codec);
+
+	switch (reg) {
+	case TWL6040_REG_HSLCTL:
+	case TWL6040_REG_HSRCTL:
+	case TWL6040_REG_EARCTL:
+	case TWL6040_REG_HFLCTL:
+	case TWL6040_REG_HFRCTL:
+		priv->dl12_cache[reg - TWL6040_REG_HSLCTL] = value;
+		break;
+	default:
+		break;
+	}
 }
 
 /*
@@ -199,6 +244,8 @@ static inline void twl6040_write_reg_cache(struct snd_soc_codec *codec,
 	if (reg >= TWL6040_CACHEREGNUM)
 		return;
 	cache[reg] = value;
+
+	twl6040_update_dl12_cache(codec, reg, value);
 }
 
 /*
-- 
1.8.4.2

[PATCH 3/5] MFD: twl6040: reg_defaults support for regmap

[Peter Ujfalusi]

Add reg_defaults to regmap and at the same time implement proper power state
handling with using regcache_cache_only(), regcache_sync() and
regcache_mark_dirty().
This will make sure that we do not need to do restore operations in child
drivers anymore.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 drivers/mfd/twl6040.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 89 insertions(+), 3 deletions(-)

diff --git a/drivers/mfd/twl6040.c b/drivers/mfd/twl6040.c
index 0779d5a..51b6df1 100644
--- a/drivers/mfd/twl6040.c
+++ b/drivers/mfd/twl6040.c
@@ -44,6 +44,54 @@
 #define VIBRACTRL_MEMBER(reg) ((reg == TWL6040_REG_VIBCTLL) ? 0 : 1)
 #define TWL6040_NUM_SUPPLIES	(2)
 
+static struct reg_default twl6040_defaults[] = {
+	{ 0x01, 0x4B }, /* REG_ASICID	(ro) */
+	{ 0x02, 0x00 }, /* REG_ASICREV	(ro) */
+	{ 0x03, 0x00 }, /* REG_INTID	*/
+	{ 0x04, 0x00 }, /* REG_INTMR	*/
+	{ 0x05, 0x00 }, /* REG_NCPCTRL	*/
+	{ 0x06, 0x00 }, /* REG_LDOCTL	*/
+	{ 0x07, 0x60 }, /* REG_HPPLLCTL	*/
+	{ 0x08, 0x00 }, /* REG_LPPLLCTL	*/
+	{ 0x09, 0x4A }, /* REG_LPPLLDIV	*/
+	{ 0x0A, 0x00 }, /* REG_AMICBCTL	*/
+	{ 0x0B, 0x00 }, /* REG_DMICBCTL	*/
+	{ 0x0C, 0x00 }, /* REG_MICLCTL	*/
+	{ 0x0D, 0x00 }, /* REG_MICRCTL	*/
+	{ 0x0E, 0x00 }, /* REG_MICGAIN	*/
+	{ 0x0F, 0x1B }, /* REG_LINEGAIN	*/
+	{ 0x10, 0x00 }, /* REG_HSLCTL	*/
+	{ 0x11, 0x00 }, /* REG_HSRCTL	*/
+	{ 0x12, 0x00 }, /* REG_HSGAIN	*/
+	{ 0x13, 0x00 }, /* REG_EARCTL	*/
+	{ 0x14, 0x00 }, /* REG_HFLCTL	*/
+	{ 0x15, 0x00 }, /* REG_HFLGAIN	*/
+	{ 0x16, 0x00 }, /* REG_HFRCTL	*/
+	{ 0x17, 0x00 }, /* REG_HFRGAIN	*/
+	{ 0x18, 0x00 }, /* REG_VIBCTLL	*/
+	{ 0x19, 0x00 }, /* REG_VIBDATL	*/
+	{ 0x1A, 0x00 }, /* REG_VIBCTLR	*/
+	{ 0x1B, 0x00 }, /* REG_VIBDATR	*/
+	{ 0x1C, 0x00 }, /* REG_HKCTL1	*/
+	{ 0x1D, 0x00 }, /* REG_HKCTL2	*/
+	{ 0x1E, 0x00 }, /* REG_GPOCTL	*/
+	{ 0x1F, 0x00 }, /* REG_ALB	*/
+	{ 0x20, 0x00 }, /* REG_DLB	*/
+	/* 0x28, REG_TRIM1 */
+	/* 0x29, REG_TRIM2 */
+	/* 0x2A, REG_TRIM3 */
+	/* 0x2B, REG_HSOTRIM */
+	/* 0x2C, REG_HFOTRIM */
+	{ 0x2D, 0x08 }, /* REG_ACCCTL	*/
+	{ 0x2E, 0x00 }, /* REG_STATUS	(ro) */
+};
+
+struct reg_default twl6040_patch[] = {
+	/* Select I2C bus access to dual access registers */
+	{ TWL6040_REG_ACCCTL, 0x09 },
+};
+
+
 static bool twl6040_has_vibra(struct device_node *node)
 {
 #ifdef CONFIG_OF
@@ -238,6 +286,9 @@ int twl6040_power(struct twl6040 *twl6040, int on)
 		if (twl6040->power_count++)
 			goto out;
 
+		/* Allow writes to the chip */
+		regcache_cache_only(twl6040->regmap, false);
+
 		if (gpio_is_valid(twl6040->audpwron)) {
 			/* use automatic power-up sequence */
 			ret = twl6040_power_up_automatic(twl6040);
@@ -253,6 +304,10 @@ int twl6040_power(struct twl6040 *twl6040, int on)
 				goto out;
 			}
 		}
+
+		/* Sync with the HW */
+		regcache_sync(twl6040->regmap);
+
 		/* Default PLL configuration after power up */
 		twl6040->pll = TWL6040_SYSCLK_SEL_LPPLL;
 		twl6040->sysclk = 19200000;
@@ -279,6 +334,11 @@ int twl6040_power(struct twl6040 *twl6040, int on)
 			/* use manual power-down sequence */
 			twl6040_power_down_manual(twl6040);
 		}
+
+		/* Set regmap to cache only and mark it as dirty */
+		regcache_cache_only(twl6040->regmap, true);
+		regcache_mark_dirty(twl6040->regmap);
+
 		twl6040->sysclk = 0;
 		twl6040->mclk = 0;
 	}
@@ -490,9 +550,24 @@ static bool twl6040_readable_reg(struct device *dev, unsigned int reg)
 static bool twl6040_volatile_reg(struct device *dev, unsigned int reg)
 {
 	switch (reg) {
-	case TWL6040_REG_VIBCTLL:
-	case TWL6040_REG_VIBCTLR:
-	case TWL6040_REG_INTMR:
+	case TWL6040_REG_ASICID:
+	case TWL6040_REG_ASICREV:
+	case TWL6040_REG_INTID:
+	case TWL6040_REG_LPPLLCTL:
+	case TWL6040_REG_HPPLLCTL:
+	case TWL6040_REG_STATUS:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static bool twl6040_writeable_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case TWL6040_REG_ASICID:
+	case TWL6040_REG_ASICREV:
+	case TWL6040_REG_STATUS:
 		return false;
 	default:
 		return true;
@@ -502,10 +577,15 @@ static bool twl6040_volatile_reg(struct device *dev, unsigned int reg)
 static struct regmap_config twl6040_regmap_config = {
 	.reg_bits = 8,
 	.val_bits = 8,
+
+	.reg_defaults = twl6040_defaults,
+	.num_reg_defaults = ARRAY_SIZE(twl6040_defaults),
+
 	.max_register = TWL6040_REG_STATUS, /* 0x2e */
 
 	.readable_reg = twl6040_readable_reg,
 	.volatile_reg = twl6040_volatile_reg,
+	.writeable_reg = twl6040_writeable_reg,
 
 	.cache_type = REGCACHE_RBTREE,
 };
@@ -624,6 +704,8 @@ static int twl6040_probe(struct i2c_client *client,
 
 	/* dual-access registers controlled by I2C only */
 	twl6040_set_bits(twl6040, TWL6040_REG_ACCCTL, TWL6040_I2CSEL);
+	regmap_register_patch(twl6040->regmap, twl6040_patch,
+			      ARRAY_SIZE(twl6040_patch));
 
 	/*
 	 * The main functionality of twl6040 to provide audio on OMAP4+ systems.
@@ -656,6 +738,10 @@ static int twl6040_probe(struct i2c_client *client,
 	cell->name = "twl6040-gpo";
 	children++;
 
+	/* The chip is powered down so mark regmap to cache only and dirty */
+	regcache_cache_only(twl6040->regmap, true);
+	regcache_mark_dirty(twl6040->regmap);
+
 	ret = mfd_add_devices(&client->dev, -1, twl6040->cells, children,
 			      NULL, 0, NULL);
 	if (ret)
-- 
1.8.4.2

Re: [PATCH 3/5] MFD: twl6040: reg_defaults support for regmap

[Lee Jones]

On Fri, 29 Nov 2013, Peter Ujfalusi wrote:

> Add reg_defaults to regmap and at the same time implement proper power state
> handling with using regcache_cache_only(), regcache_sync() and
> regcache_mark_dirty().
> This will make sure that we do not need to do restore operations in child
> drivers anymore.
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> ---
>  drivers/mfd/twl6040.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 89 insertions(+), 3 deletions(-)

The code looks fine to me (me = !regmap expert), so with Mark's Ack I
will apply.

Mark, is an immutable branch required for this patch?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 3/5] MFD: twl6040: reg_defaults support for regmap

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 217 bytes --]

On Fri, Nov 29, 2013 at 04:00:32PM +0000, Lee Jones wrote:

> The code looks fine to me (me = !regmap expert), so with Mark's Ack I
> will apply.

> Mark, is an immutable branch required for this patch?

Yes, please.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 3/5] MFD: twl6040: reg_defaults support for regmap

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 197 bytes --]

On Fri, Nov 29, 2013 at 04:00:32PM +0000, Lee Jones wrote:

> Mark, is an immutable branch required for this patch?

One is...  I think I did ask for one at the time but didn't hear
anything back?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 3/5] MFD: twl6040: reg_defaults support for regmap

[Lee Jones]

On Wed, 18 Dec 2013, Mark Brown wrote:

> On Fri, Nov 29, 2013 at 04:00:32PM +0000, Lee Jones wrote:
> 
> > Mark, is an immutable branch required for this patch?
> 
> One is...  I think I did ask for one at the time but didn't hear
> anything back?

I must have missed your reply.

What -rc are you currently based on?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 3/5] MFD: twl6040: reg_defaults support for regmap

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 153 bytes --]

On Thu, Dec 19, 2013 at 09:06:32AM +0000, Lee Jones wrote:

> What -rc are you currently based on?

Using -rc1 makes life easier, it'll be a new branch.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 3/5] MFD: twl6040: reg_defaults support for regmap

[Lee Jones]

Enjoy!

> > What -rc are you currently based on?
> 
> Using -rc1 makes life easier, it'll be a new branch.

The following changes since commit 6ce4eac1f600b34f2f7f58f9cd8f0503d79e42ae:

  Linux 3.13-rc1 (2013-11-22 11:30:55 -0800)

are available in the git repository at:

  git://git.linaro.org/people/ljones/mfd.git tags/ib-asoc-3.14-1

for you to fetch changes up to c7f9129d22940720141d1f1e958a51142eff9d21:

  mfd: twl6040: reg_defaults support for regmap (2013-12-19 17:01:11 +0000)

----------------------------------------------------------------
Immutable branch between ASoC and MFD - for v3.14 merge window

----------------------------------------------------------------
Peter Ujfalusi (1):
      mfd: twl6040: reg_defaults support for regmap

 drivers/mfd/twl6040.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 89 insertions(+), 3 deletions(-)

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 3/5] MFD: twl6040: reg_defaults support for regmap

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 244 bytes --]

On Thu, Dec 19, 2013 at 05:20:36PM +0000, Lee Jones wrote:

> for you to fetch changes up to c7f9129d22940720141d1f1e958a51142eff9d21:
> 
>   mfd: twl6040: reg_defaults support for regmap (2013-12-19 17:01:11 +0000)

Pulled now, thanks.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 4/5] ASoC: twl6040: Remove register restore functionality

[Peter Ujfalusi]

The MFD core takes care of the restore via standard regmap API, no need to
do this anymore here.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 sound/soc/codecs/twl6040.c | 29 -----------------------------
 1 file changed, 29 deletions(-)

diff --git a/sound/soc/codecs/twl6040.c b/sound/soc/codecs/twl6040.c
index fb8c65b..b07839b 100644
--- a/sound/soc/codecs/twl6040.c
+++ b/sound/soc/codecs/twl6040.c
@@ -133,22 +133,6 @@ static const u8 twl6040_reg[TWL6040_CACHEREGNUM] = {
 	0x00, /* REG_STATUS	0x2E (ro) */
 };
 
-/* List of registers to be restored after power up */
-static const int twl6040_restore_list[] = {
-	TWL6040_REG_MICLCTL,
-	TWL6040_REG_MICRCTL,
-	TWL6040_REG_MICGAIN,
-	TWL6040_REG_LINEGAIN,
-	TWL6040_REG_HSLCTL,
-	TWL6040_REG_HSRCTL,
-	TWL6040_REG_HSGAIN,
-	TWL6040_REG_EARCTL,
-	TWL6040_REG_HFLCTL,
-	TWL6040_REG_HFLGAIN,
-	TWL6040_REG_HFRCTL,
-	TWL6040_REG_HFRGAIN,
-};
-
 /* set of rates for each pll: low-power and high-performance */
 static unsigned int lp_rates[] = {
 	8000,
@@ -335,17 +319,6 @@ static void twl6040_init_chip(struct snd_soc_codec *codec)
 	twl6040_write_reg_cache(codec, TWL6040_REG_LINEGAIN, 0);
 }
 
-static void twl6040_restore_regs(struct snd_soc_codec *codec)
-{
-	u8 *cache = codec->reg_cache;
-	int reg, i;
-
-	for (i = 0; i < ARRAY_SIZE(twl6040_restore_list); i++) {
-		reg = twl6040_restore_list[i];
-		twl6040_write(codec, reg, cache[reg]);
-	}
-}
-
 /* set headset dac and driver power mode */
 static int headset_power_mode(struct snd_soc_codec *codec, int high_perf)
 {
@@ -978,8 +951,6 @@ static int twl6040_set_bias_level(struct snd_soc_codec *codec,
 
 		priv->codec_powered = 1;
 
-		twl6040_restore_regs(codec);
-
 		/* Set external boost GPO */
 		twl6040_write(codec, TWL6040_REG_GPOCTL, 0x02);
 		break;
-- 
1.8.4.2

[PATCH 5/5] ASoC: twl6040: Remove self managed local reg_cache support

[Peter Ujfalusi]

We can rely on mfd driver to manage the register caching via regmap. The
driver still need to cache some registers associated with DL1/2 routes.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 sound/soc/codecs/twl6040.c | 211 +++++++++++----------------------------------
 1 file changed, 49 insertions(+), 162 deletions(-)

diff --git a/sound/soc/codecs/twl6040.c b/sound/soc/codecs/twl6040.c
index b07839b..0afe8be 100644
--- a/sound/soc/codecs/twl6040.c
+++ b/sound/soc/codecs/twl6040.c
@@ -80,59 +80,6 @@ struct twl6040_data {
 	struct mutex mutex;
 };
 
-/*
- * twl6040 register cache & default register settings
- */
-static const u8 twl6040_reg[TWL6040_CACHEREGNUM] = {
-	0x00, /* not used	0x00	*/
-	0x4B, /* REG_ASICID	0x01 (ro) */
-	0x00, /* REG_ASICREV	0x02 (ro) */
-	0x00, /* REG_INTID	0x03	*/
-	0x00, /* REG_INTMR	0x04	*/
-	0x00, /* REG_NCPCTRL	0x05	*/
-	0x00, /* REG_LDOCTL	0x06	*/
-	0x60, /* REG_HPPLLCTL	0x07	*/
-	0x00, /* REG_LPPLLCTL	0x08	*/
-	0x4A, /* REG_LPPLLDIV	0x09	*/
-	0x00, /* REG_AMICBCTL	0x0A	*/
-	0x00, /* REG_DMICBCTL	0x0B	*/
-	0x00, /* REG_MICLCTL	0x0C	*/
-	0x00, /* REG_MICRCTL	0x0D	*/
-	0x00, /* REG_MICGAIN	0x0E	*/
-	0x1B, /* REG_LINEGAIN	0x0F	*/
-	0x00, /* REG_HSLCTL	0x10	*/
-	0x00, /* REG_HSRCTL	0x11	*/
-	0x00, /* REG_HSGAIN	0x12	*/
-	0x00, /* REG_EARCTL	0x13	*/
-	0x00, /* REG_HFLCTL	0x14	*/
-	0x00, /* REG_HFLGAIN	0x15	*/
-	0x00, /* REG_HFRCTL	0x16	*/
-	0x00, /* REG_HFRGAIN	0x17	*/
-	0x00, /* REG_VIBCTLL	0x18	*/
-	0x00, /* REG_VIBDATL	0x19	*/
-	0x00, /* REG_VIBCTLR	0x1A	*/
-	0x00, /* REG_VIBDATR	0x1B	*/
-	0x00, /* REG_HKCTL1	0x1C	*/
-	0x00, /* REG_HKCTL2	0x1D	*/
-	0x00, /* REG_GPOCTL	0x1E	*/
-	0x00, /* REG_ALB	0x1F	*/
-	0x00, /* REG_DLB	0x20	*/
-	0x00, /* not used	0x21	*/
-	0x00, /* not used	0x22	*/
-	0x00, /* not used	0x23	*/
-	0x00, /* not used	0x24	*/
-	0x00, /* not used	0x25	*/
-	0x00, /* not used	0x26	*/
-	0x00, /* not used	0x27	*/
-	0x00, /* REG_TRIM1	0x28	*/
-	0x00, /* REG_TRIM2	0x29	*/
-	0x00, /* REG_TRIM3	0x2A	*/
-	0x00, /* REG_HSOTRIM	0x2B	*/
-	0x00, /* REG_HFOTRIM	0x2C	*/
-	0x09, /* REG_ACCCTL	0x2D	*/
-	0x00, /* REG_STATUS	0x2E (ro) */
-};
-
 /* set of rates for each pll: low-power and high-performance */
 static unsigned int lp_rates[] = {
 	8000,
@@ -159,11 +106,14 @@ static struct snd_pcm_hw_constraint_list sysclk_constraints[] = {
 	{ .count = ARRAY_SIZE(hp_rates), .list = hp_rates, },
 };
 
-static inline int twl6040_read_dl12_cache(struct snd_soc_codec *codec,
-					   u8 reg, u8 *value)
+static unsigned int twl6040_read(struct snd_soc_codec *codec, unsigned int reg)
 {
 	struct twl6040_data *priv = snd_soc_codec_get_drvdata(codec);
-	int ret = 0;
+	struct twl6040 *twl6040 = codec->control_data;
+	u8 value;
+
+	if (reg >= TWL6040_CACHEREGNUM)
+		return -EIO;
 
 	switch (reg) {
 	case TWL6040_REG_HSLCTL:
@@ -171,36 +121,18 @@ static inline int twl6040_read_dl12_cache(struct snd_soc_codec *codec,
 	case TWL6040_REG_EARCTL:
 	case TWL6040_REG_HFLCTL:
 	case TWL6040_REG_HFRCTL:
-		*value = priv->dl12_cache[reg - TWL6040_REG_HSLCTL];
+		value = priv->dl12_cache[reg - TWL6040_REG_HSLCTL];
 		break;
 	default:
-		ret = -EINVAL;
+		value = twl6040_reg_read(twl6040, reg);
 		break;
 	}
 
-	return ret;
-}
-
-/*
- * read twl6040 register cache
- */
-static inline unsigned int twl6040_read_reg_cache(struct snd_soc_codec *codec,
-						  unsigned int reg)
-{
-	u8 *cache = codec->reg_cache;
-	u8 value;
-
-	if (reg >= TWL6040_CACHEREGNUM)
-		return -EIO;
-
-	if (twl6040_read_dl12_cache(codec, reg, &value))
-		value = cache[reg];
-
 	return value;
 }
 
-static inline void twl6040_update_dl12_cache(struct snd_soc_codec *codec,
-					     u8 reg, u8 value)
+static bool twl6040_can_write_to_chip(struct snd_soc_codec *codec,
+				  unsigned int reg)
 {
 	struct twl6040_data *priv = snd_soc_codec_get_drvdata(codec);
 
@@ -208,50 +140,18 @@ static inline void twl6040_update_dl12_cache(struct snd_soc_codec *codec,
 	case TWL6040_REG_HSLCTL:
 	case TWL6040_REG_HSRCTL:
 	case TWL6040_REG_EARCTL:
+		/* DL1 path */
+		return priv->dl1_unmuted;
 	case TWL6040_REG_HFLCTL:
 	case TWL6040_REG_HFRCTL:
-		priv->dl12_cache[reg - TWL6040_REG_HSLCTL] = value;
-		break;
+		return priv->dl2_unmuted;
 	default:
-		break;
+		return 1;
 	}
 }
 
-/*
- * write twl6040 register cache
- */
-static inline void twl6040_write_reg_cache(struct snd_soc_codec *codec,
-						u8 reg, u8 value)
-{
-	u8 *cache = codec->reg_cache;
-
-	if (reg >= TWL6040_CACHEREGNUM)
-		return;
-	cache[reg] = value;
-
-	twl6040_update_dl12_cache(codec, reg, value);
-}
-
-/*
- * read from twl6040 hardware register
- */
-static int twl6040_read_reg_volatile(struct snd_soc_codec *codec,
-			unsigned int reg)
-{
-	struct twl6040 *twl6040 = codec->control_data;
-	u8 value;
-
-	if (reg >= TWL6040_CACHEREGNUM)
-		return -EIO;
-
-	value = twl6040_reg_read(twl6040, reg);
-	twl6040_write_reg_cache(codec, reg, value);
-
-	return value;
-}
-
-static bool twl6040_can_write_to_chip(struct snd_soc_codec *codec,
-				  unsigned int reg)
+static inline void twl6040_update_dl12_cache(struct snd_soc_codec *codec,
+					     u8 reg, u8 value)
 {
 	struct twl6040_data *priv = snd_soc_codec_get_drvdata(codec);
 
@@ -259,19 +159,15 @@ static bool twl6040_can_write_to_chip(struct snd_soc_codec *codec,
 	case TWL6040_REG_HSLCTL:
 	case TWL6040_REG_HSRCTL:
 	case TWL6040_REG_EARCTL:
-		/* DL1 path */
-		return priv->dl1_unmuted;
 	case TWL6040_REG_HFLCTL:
 	case TWL6040_REG_HFRCTL:
-		return priv->dl2_unmuted;
+		priv->dl12_cache[reg - TWL6040_REG_HSLCTL] = value;
+		break;
 	default:
-		return 1;
+		break;
 	}
 }
 
-/*
- * write to the twl6040 register space
- */
 static int twl6040_write(struct snd_soc_codec *codec,
 			unsigned int reg, unsigned int value)
 {
@@ -280,7 +176,7 @@ static int twl6040_write(struct snd_soc_codec *codec,
 	if (reg >= TWL6040_CACHEREGNUM)
 		return -EIO;
 
-	twl6040_write_reg_cache(codec, reg, value);
+	twl6040_update_dl12_cache(codec, reg, value);
 	if (twl6040_can_write_to_chip(codec, reg))
 		return twl6040_reg_write(twl6040, reg, value);
 	else
@@ -289,34 +185,27 @@ static int twl6040_write(struct snd_soc_codec *codec,
 
 static void twl6040_init_chip(struct snd_soc_codec *codec)
 {
-	struct twl6040 *twl6040 = codec->control_data;
-	u8 val;
-
-	/* Update reg_cache: ASICREV, and TRIM values */
-	val = twl6040_get_revid(twl6040);
-	twl6040_write_reg_cache(codec, TWL6040_REG_ASICREV, val);
-
-	twl6040_read_reg_volatile(codec, TWL6040_REG_TRIM1);
-	twl6040_read_reg_volatile(codec, TWL6040_REG_TRIM2);
-	twl6040_read_reg_volatile(codec, TWL6040_REG_TRIM3);
-	twl6040_read_reg_volatile(codec, TWL6040_REG_HSOTRIM);
-	twl6040_read_reg_volatile(codec, TWL6040_REG_HFOTRIM);
+	twl6040_read(codec, TWL6040_REG_TRIM1);
+	twl6040_read(codec, TWL6040_REG_TRIM2);
+	twl6040_read(codec, TWL6040_REG_TRIM3);
+	twl6040_read(codec, TWL6040_REG_HSOTRIM);
+	twl6040_read(codec, TWL6040_REG_HFOTRIM);
 
 	/* Change chip defaults */
 	/* No imput selected for microphone amplifiers */
-	twl6040_write_reg_cache(codec, TWL6040_REG_MICLCTL, 0x18);
-	twl6040_write_reg_cache(codec, TWL6040_REG_MICRCTL, 0x18);
+	twl6040_write(codec, TWL6040_REG_MICLCTL, 0x18);
+	twl6040_write(codec, TWL6040_REG_MICRCTL, 0x18);
 
 	/*
 	 * We need to lower the default gain values, so the ramp code
 	 * can work correctly for the first playback.
 	 * This reduces the pop noise heard at the first playback.
 	 */
-	twl6040_write_reg_cache(codec, TWL6040_REG_HSGAIN, 0xff);
-	twl6040_write_reg_cache(codec, TWL6040_REG_EARCTL, 0x1e);
-	twl6040_write_reg_cache(codec, TWL6040_REG_HFLGAIN, 0x1d);
-	twl6040_write_reg_cache(codec, TWL6040_REG_HFRGAIN, 0x1d);
-	twl6040_write_reg_cache(codec, TWL6040_REG_LINEGAIN, 0);
+	twl6040_write(codec, TWL6040_REG_HSGAIN, 0xff);
+	twl6040_write(codec, TWL6040_REG_EARCTL, 0x1e);
+	twl6040_write(codec, TWL6040_REG_HFLGAIN, 0x1d);
+	twl6040_write(codec, TWL6040_REG_HFRGAIN, 0x1d);
+	twl6040_write(codec, TWL6040_REG_LINEGAIN, 0);
 }
 
 /* set headset dac and driver power mode */
@@ -325,8 +214,8 @@ static int headset_power_mode(struct snd_soc_codec *codec, int high_perf)
 	int hslctl, hsrctl;
 	int mask = TWL6040_HSDRVMODE | TWL6040_HSDACMODE;
 
-	hslctl = twl6040_read_reg_cache(codec, TWL6040_REG_HSLCTL);
-	hsrctl = twl6040_read_reg_cache(codec, TWL6040_REG_HSRCTL);
+	hslctl = twl6040_read(codec, TWL6040_REG_HSLCTL);
+	hsrctl = twl6040_read(codec, TWL6040_REG_HSRCTL);
 
 	if (high_perf) {
 		hslctl &= ~mask;
@@ -353,8 +242,8 @@ static int twl6040_hs_dac_event(struct snd_soc_dapm_widget *w,
 	 * Both HS DAC need to be turned on (before the HS driver) and off at
 	 * the same time.
 	 */
-	hslctl = twl6040_read_reg_cache(codec, TWL6040_REG_HSLCTL);
-	hsrctl = twl6040_read_reg_cache(codec, TWL6040_REG_HSRCTL);
+	hslctl = twl6040_read(codec, TWL6040_REG_HSLCTL);
+	hsrctl = twl6040_read(codec, TWL6040_REG_HSRCTL);
 	if (SND_SOC_DAPM_EVENT_ON(event)) {
 		hslctl |= TWL6040_HSDACENA;
 		hsrctl |= TWL6040_HSDACENA;
@@ -399,7 +288,7 @@ static void twl6040_hs_jack_report(struct snd_soc_codec *codec,
 	mutex_lock(&priv->mutex);
 
 	/* Sync status */
-	status = twl6040_read_reg_volatile(codec, TWL6040_REG_STATUS);
+	status = twl6040_read(codec, TWL6040_REG_STATUS);
 	if (status & TWL6040_PLUGCOMP)
 		snd_soc_jack_report(jack, report, report);
 	else
@@ -451,7 +340,7 @@ static int twl6040_soc_dapm_put_vibra_enum(struct snd_kcontrol *kcontrol,
 	unsigned int val;
 
 	/* Do not allow changes while Input/FF efect is running */
-	val = twl6040_read_reg_volatile(codec, e->reg);
+	val = twl6040_read(codec, e->reg);
 	if (val & TWL6040_VIBENA && !(val & TWL6040_VIBSEL))
 		return -EBUSY;
 
@@ -676,7 +565,7 @@ int twl6040_get_trim_value(struct snd_soc_codec *codec, enum twl6040_trim trim)
 	if (unlikely(trim >= TWL6040_TRIM_INVAL))
 		return -EINVAL;
 
-	return twl6040_read_reg_cache(codec, TWL6040_REG_TRIM1 + trim);
+	return twl6040_read(codec, TWL6040_REG_TRIM1 + trim);
 }
 EXPORT_SYMBOL_GPL(twl6040_get_trim_value);
 
@@ -1071,9 +960,9 @@ static void twl6040_mute_path(struct snd_soc_codec *codec, enum twl6040_dai_id i
 
 	switch (id) {
 	case TWL6040_DAI_DL1:
-		hslctl = twl6040_read_reg_cache(codec, TWL6040_REG_HSLCTL);
-		hsrctl = twl6040_read_reg_cache(codec, TWL6040_REG_HSRCTL);
-		earctl = twl6040_read_reg_cache(codec, TWL6040_REG_EARCTL);
+		hslctl = twl6040_read(codec, TWL6040_REG_HSLCTL);
+		hsrctl = twl6040_read(codec, TWL6040_REG_HSRCTL);
+		earctl = twl6040_read(codec, TWL6040_REG_EARCTL);
 
 		if (mute) {
 			/* Power down drivers and DACs */
@@ -1089,8 +978,8 @@ static void twl6040_mute_path(struct snd_soc_codec *codec, enum twl6040_dai_id i
 		priv->dl1_unmuted = !mute;
 		break;
 	case TWL6040_DAI_DL2:
-		hflctl = twl6040_read_reg_cache(codec, TWL6040_REG_HFLCTL);
-		hfrctl = twl6040_read_reg_cache(codec, TWL6040_REG_HFRCTL);
+		hflctl = twl6040_read(codec, TWL6040_REG_HFLCTL);
+		hfrctl = twl6040_read(codec, TWL6040_REG_HFRCTL);
 
 		if (mute) {
 			/* Power down drivers and DACs */
@@ -1227,6 +1116,7 @@ static int twl6040_resume(struct snd_soc_codec *codec)
 static int twl6040_probe(struct snd_soc_codec *codec)
 {
 	struct twl6040_data *priv;
+	struct twl6040 *twl6040 = dev_get_drvdata(codec->dev->parent);
 	struct platform_device *pdev = container_of(codec->dev,
 						   struct platform_device, dev);
 	int ret = 0;
@@ -1238,7 +1128,7 @@ static int twl6040_probe(struct snd_soc_codec *codec)
 	snd_soc_codec_set_drvdata(codec, priv);
 
 	priv->codec = codec;
-	codec->control_data = dev_get_drvdata(codec->dev->parent);
+	codec->control_data = twl6040;
 
 	priv->plug_irq = platform_get_irq(pdev, 0);
 	if (priv->plug_irq < 0) {
@@ -1258,10 +1148,10 @@ static int twl6040_probe(struct snd_soc_codec *codec)
 		return ret;
 	}
 
+	twl6040_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
 	twl6040_init_chip(codec);
 
-	/* power on device */
-	return twl6040_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
+	return 0;
 }
 
 static int twl6040_remove(struct snd_soc_codec *codec)
@@ -1279,12 +1169,9 @@ static struct snd_soc_codec_driver soc_codec_dev_twl6040 = {
 	.remove = twl6040_remove,
 	.suspend = twl6040_suspend,
 	.resume = twl6040_resume,
-	.read = twl6040_read_reg_cache,
+	.read = twl6040_read,
 	.write = twl6040_write,
 	.set_bias_level = twl6040_set_bias_level,
-	.reg_cache_size = ARRAY_SIZE(twl6040_reg),
-	.reg_word_size = sizeof(u8),
-	.reg_cache_default = twl6040_reg,
 	.ignore_pmdown_time = true,
 
 	.controls = twl6040_snd_controls,
-- 
1.8.4.2

Re: [PATCH 0/5] ASoC/MFD: twl6040: Remove codec driver local reg cache

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 504 bytes --]

On Fri, Nov 29, 2013 at 04:03:42PM +0200, Peter Ujfalusi wrote:

> Is this close enough of what you had in mind when asked me to look at this?

Yes, this is exactly the sort of thing I was looking for - thanks!

> twl4030 codec is going to be a bit more complicated AFAIK but it will come
> eventually.

Yup, they're both more complicated than most of these devices otherwise
I'd have thrown some changes out myself - all the fiddling around inside
the cache data makes them more difficult to work with.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 0/5] ASoC/MFD: twl6040: Remove codec driver local reg cache

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 303 bytes --]

On Fri, Nov 29, 2013 at 04:03:42PM +0200, Peter Ujfalusi wrote:
> Hi Mark,
> 
> It was not straight forward task to remove the local reg cache from the twl6040
> codec driver. I also needed to change the MFD part so it will take over the
> caching of registers via regmap.

Applied all, thanks.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]