linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V3 0/4] tas571x amplifier driver
@ 2015-05-04  0:00 Kevin Cernekee
  2015-05-04  0:00 ` [PATCH V3 1/4] regmap: Use regcache_mark_dirty() to indicate power loss or reset Kevin Cernekee
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Kevin Cernekee @ 2015-05-04  0:00 UTC (permalink / raw)
  To: lgirdwood, broonie
  Cc: lars, dgreid, abrestic, olofj, alsa-devel, devicetree, linux-kernel

V2->V3:

Revert earlier changes to the regcache APIs.

Change regcache_sync() so that regcache_mark_dirty() is used to indicate
that the HW was reset.  This allows tas571x (which does NOT call
regcache_mark_dirty()) to use regcache_sync() to write back any register
changes that occurred while in cache_only (tas571x pdn) mode.

One assumption here is that regcache_sync() will be called once on
resume, before any other writes.  If it fails, the no_sync_defaults flag
will be cleared and the next sync attempt, if any, will try to write
out all contents of the cache.


Kevin Cernekee (4):
  regmap: Use regcache_mark_dirty() to indicate power loss or reset
  ASoC: tas571x: Add DT binding document
  ASoC: tas571x: New driver for TI TAS571x power amplifiers
  MAINTAINERS: Add entry for tas571x ASoC codec driver

 .../devicetree/bindings/sound/tas571x.txt          |  41 ++
 MAINTAINERS                                        |   6 +
 drivers/base/regmap/internal.h                     |   3 +
 drivers/base/regmap/regcache.c                     |  61 ++-
 sound/soc/codecs/Kconfig                           |   5 +
 sound/soc/codecs/Makefile                          |   2 +
 sound/soc/codecs/tas571x.c                         | 520 +++++++++++++++++++++
 sound/soc/codecs/tas571x.h                         |  33 ++
 8 files changed, 651 insertions(+), 20 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/tas571x.txt
 create mode 100644 sound/soc/codecs/tas571x.c
 create mode 100644 sound/soc/codecs/tas571x.h

-- 
2.2.0.rc0.207.ga3a616c


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH V3 1/4] regmap: Use regcache_mark_dirty() to indicate power loss or reset
  2015-05-04  0:00 [PATCH V3 0/4] tas571x amplifier driver Kevin Cernekee
@ 2015-05-04  0:00 ` Kevin Cernekee
  2015-05-04  6:38   ` Lars-Peter Clausen
  2015-05-04  0:00 ` [PATCH V3 2/4] ASoC: tas571x: Add DT binding document Kevin Cernekee
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Kevin Cernekee @ 2015-05-04  0:00 UTC (permalink / raw)
  To: lgirdwood, broonie
  Cc: lars, dgreid, abrestic, olofj, alsa-devel, devicetree, linux-kernel

Existing regmap users call regcache_mark_dirty() as part of the
suspend/resume sequence, to tell regcache that non-default values need to
be resynced post-resume.  Add an internal "no_sync_defaults" regmap flag
to remember this state, so that regcache_sync() can differentiate between
these two cases:

1) HW was reset, so any cache values that match map->reg_defaults can be
safely skipped.  On some chips there are a lot of registers in the
reg_defaults list, so this optimization speeds things up quite a bit.

2) HW was not reset (maybe it was just clock-gated), so if we cached
any writes, they should be sent to the hardware regardless of whether
they match the HW default.  Currently this will write out all values in
the regcache, since we don't maintain per-register dirty bits.

Suggested-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Kevin Cernekee <cernekee@chromium.org>
---
 drivers/base/regmap/internal.h |  3 +++
 drivers/base/regmap/regcache.c | 61 ++++++++++++++++++++++++++++--------------
 2 files changed, 44 insertions(+), 20 deletions(-)

diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
index a13587b5c2be..b2b2849fc6d3 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -131,7 +131,10 @@ struct regmap {
 	struct reg_default *reg_defaults;
 	const void *reg_defaults_raw;
 	void *cache;
+	/* if set, the cache contains newer data than the HW */
 	u32 cache_dirty;
+	/* if set, the HW registers are known to match map->reg_defaults */
+	bool no_sync_defaults;
 
 	struct reg_default *patch;
 	int patch_regs;
diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
index 7eb7b3b98794..63af3103d0c6 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -253,6 +253,9 @@ static int regcache_default_sync(struct regmap *map, unsigned int min,
 				 unsigned int max)
 {
 	unsigned int reg;
+	bool no_sync_defaults = map->no_sync_defaults;
+
+	map->no_sync_defaults = false;
 
 	for (reg = min; reg <= max; reg += map->reg_stride) {
 		unsigned int val;
@@ -266,10 +269,12 @@ static int regcache_default_sync(struct regmap *map, unsigned int min,
 		if (ret)
 			return ret;
 
-		/* Is this the hardware default?  If so skip. */
-		ret = regcache_lookup_reg(map, reg);
-		if (ret >= 0 && val == map->reg_defaults[ret].def)
-			continue;
+		if (no_sync_defaults) {
+			/* Is this the hardware default?  If so skip. */
+			ret = regcache_lookup_reg(map, reg);
+			if (ret >= 0 && val == map->reg_defaults[ret].def)
+				continue;
+		}
 
 		map->cache_bypass = 1;
 		ret = _regmap_write(map, reg, val);
@@ -461,18 +466,23 @@ void regcache_cache_only(struct regmap *map, bool enable)
 EXPORT_SYMBOL_GPL(regcache_cache_only);
 
 /**
- * regcache_mark_dirty: Mark the register cache as dirty
+ * regcache_mark_dirty: Indicate that HW registers were reset to default values
  *
  * @map: map to mark
  *
- * Mark the register cache as dirty, for example due to the device
- * having been powered down for suspend.  If the cache is not marked
- * as dirty then the cache sync will be suppressed.
+ * Inform regcache that the device has been powered down or reset, so that
+ * on resume, regcache_sync() knows to write out all non-default values
+ * stored in the cache.
+ *
+ * If this function is not called, regcache_sync() will assume that
+ * the hardware state still matches the cache state, modulo any writes that
+ * happened when cache_only was true.
  */
 void regcache_mark_dirty(struct regmap *map)
 {
 	map->lock(map->lock_arg);
 	map->cache_dirty = true;
+	map->no_sync_defaults = true;
 	map->unlock(map->lock_arg);
 }
 EXPORT_SYMBOL_GPL(regcache_mark_dirty);
@@ -604,6 +614,9 @@ static int regcache_sync_block_single(struct regmap *map, void *block,
 {
 	unsigned int i, regtmp, val;
 	int ret;
+	bool no_sync_defaults = map->no_sync_defaults;
+
+	map->no_sync_defaults = false;
 
 	for (i = start; i < end; i++) {
 		regtmp = block_base + (i * map->reg_stride);
@@ -614,10 +627,12 @@ static int regcache_sync_block_single(struct regmap *map, void *block,
 
 		val = regcache_get_val(map, block, i);
 
-		/* Is this the hardware default?  If so skip. */
-		ret = regcache_lookup_reg(map, regtmp);
-		if (ret >= 0 && val == map->reg_defaults[ret].def)
-			continue;
+		if (no_sync_defaults) {
+			/* Is this the hardware default?  If so skip. */
+			ret = regcache_lookup_reg(map, regtmp);
+			if (ret >= 0 && val == map->reg_defaults[ret].def)
+				continue;
+		}
 
 		map->cache_bypass = 1;
 
@@ -674,6 +689,9 @@ static int regcache_sync_block_raw(struct regmap *map, void *block,
 	unsigned int base = 0;
 	const void *data = NULL;
 	int ret;
+	bool no_sync_defaults = map->no_sync_defaults;
+
+	map->no_sync_defaults = false;
 
 	for (i = start; i < end; i++) {
 		regtmp = block_base + (i * map->reg_stride);
@@ -689,14 +707,17 @@ static int regcache_sync_block_raw(struct regmap *map, void *block,
 
 		val = regcache_get_val(map, block, i);
 
-		/* Is this the hardware default?  If so skip. */
-		ret = regcache_lookup_reg(map, regtmp);
-		if (ret >= 0 && val == map->reg_defaults[ret].def) {
-			ret = regcache_sync_block_raw_flush(map, &data,
-							    base, regtmp);
-			if (ret != 0)
-				return ret;
-			continue;
+		if (no_sync_defaults) {
+			/* Is this the hardware default?  If so skip. */
+			ret = regcache_lookup_reg(map, regtmp);
+			if (ret >= 0 && val == map->reg_defaults[ret].def) {
+				ret = regcache_sync_block_raw_flush(map, &data,
+								    base,
+								    regtmp);
+				if (ret != 0)
+					return ret;
+				continue;
+			}
 		}
 
 		if (!data) {
-- 
2.2.0.rc0.207.ga3a616c


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH V3 2/4] ASoC: tas571x: Add DT binding document
  2015-05-04  0:00 [PATCH V3 0/4] tas571x amplifier driver Kevin Cernekee
  2015-05-04  0:00 ` [PATCH V3 1/4] regmap: Use regcache_mark_dirty() to indicate power loss or reset Kevin Cernekee
@ 2015-05-04  0:00 ` Kevin Cernekee
  2015-05-04  0:00 ` [PATCH V3 3/4] ASoC: tas571x: New driver for TI TAS571x power amplifiers Kevin Cernekee
  2015-05-04  0:00 ` [PATCH V3 4/4] MAINTAINERS: Add entry for tas571x ASoC codec driver Kevin Cernekee
  3 siblings, 0 replies; 11+ messages in thread
From: Kevin Cernekee @ 2015-05-04  0:00 UTC (permalink / raw)
  To: lgirdwood, broonie
  Cc: lars, dgreid, abrestic, olofj, alsa-devel, devicetree, linux-kernel

Document the bindings for the soon-to-be-added tas571x driver.

Signed-off-by: Kevin Cernekee <cernekee@chromium.org>
---
 .../devicetree/bindings/sound/tas571x.txt          | 41 ++++++++++++++++++++++
 1 file changed, 41 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/tas571x.txt

diff --git a/Documentation/devicetree/bindings/sound/tas571x.txt b/Documentation/devicetree/bindings/sound/tas571x.txt
new file mode 100644
index 000000000000..0ac31d8d5ac4
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/tas571x.txt
@@ -0,0 +1,41 @@
+Texas Instruments TAS5711/TAS5717/TAS5719 stereo power amplifiers
+
+The codec is controlled through an I2C interface.  It also has two other
+signals that can be wired up to GPIOs: reset (strongly recommended), and
+powerdown (optional).
+
+Required properties:
+
+- compatible: "ti,tas5711", "ti,tas5717", or "ti,tas5719"
+- reg: The I2C address of the device
+- #sound-dai-cells: must be equal to 0
+
+Optional properties:
+
+- reset-gpios: GPIO specifier for the TAS571x's active low reset line
+- pdn-gpios: GPIO specifier for the TAS571x's active low powerdown line
+- clocks: clock phandle for the MCLK input
+- clock-names: should be "mclk"
+- AVDD-supply: regulator phandle for the AVDD supply (all chips)
+- DVDD-supply: regulator phandle for the DVDD supply (all chips)
+- HPVDD-supply: regulator phandle for the HPVDD supply (5717/5719)
+- PVDD_AB-supply: regulator phandle for the PVDD_AB supply (5717/5719)
+- PVDD_CD-supply: regulator phandle for the PVDD_CD supply (5717/5719)
+- PVDD_A-supply: regulator phandle for the PVDD_A supply (5711)
+- PVDD_B-supply: regulator phandle for the PVDD_B supply (5711)
+- PVDD_C-supply: regulator phandle for the PVDD_C supply (5711)
+- PVDD_D-supply: regulator phandle for the PVDD_D supply (5711)
+
+Example:
+
+	tas5717: audio-codec@2a {
+		compatible = "ti,tas5717";
+		reg = <0x2a>;
+		#sound-dai-cells = <0>;
+
+		reset-gpios = <&gpio5 1 GPIO_ACTIVE_LOW>;
+		pdn-gpios = <&gpio5 2 GPIO_ACTIVE_LOW>;
+
+		clocks = <&clk_core CLK_I2S>;
+		clock-names = "mclk";
+	};
-- 
2.2.0.rc0.207.ga3a616c


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH V3 3/4] ASoC: tas571x: New driver for TI TAS571x power amplifiers
  2015-05-04  0:00 [PATCH V3 0/4] tas571x amplifier driver Kevin Cernekee
  2015-05-04  0:00 ` [PATCH V3 1/4] regmap: Use regcache_mark_dirty() to indicate power loss or reset Kevin Cernekee
  2015-05-04  0:00 ` [PATCH V3 2/4] ASoC: tas571x: Add DT binding document Kevin Cernekee
@ 2015-05-04  0:00 ` Kevin Cernekee
  2015-05-04 11:45   ` Mark Brown
  2015-05-04  0:00 ` [PATCH V3 4/4] MAINTAINERS: Add entry for tas571x ASoC codec driver Kevin Cernekee
  3 siblings, 1 reply; 11+ messages in thread
From: Kevin Cernekee @ 2015-05-04  0:00 UTC (permalink / raw)
  To: lgirdwood, broonie
  Cc: lars, dgreid, abrestic, olofj, alsa-devel, devicetree, linux-kernel

Introduce a new codec driver for the Texas Instruments
TAS5711/TAS5717/TAS5719 power amplifier chips.  These chips are typically
used to take an I2S digital audio input and drive 10-20W into a pair of
speakers.

Signed-off-by: Kevin Cernekee <cernekee@chromium.org>
---
 sound/soc/codecs/Kconfig   |   5 +
 sound/soc/codecs/Makefile  |   2 +
 sound/soc/codecs/tas571x.c | 520 +++++++++++++++++++++++++++++++++++++++++++++
 sound/soc/codecs/tas571x.h |  33 +++
 4 files changed, 560 insertions(+)
 create mode 100644 sound/soc/codecs/tas571x.c
 create mode 100644 sound/soc/codecs/tas571x.h

diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index 061c46587628..befff910d71a 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -104,6 +104,7 @@ config SND_SOC_ALL_CODECS
 	select SND_SOC_STAC9766 if SND_SOC_AC97_BUS
 	select SND_SOC_TAS2552 if I2C
 	select SND_SOC_TAS5086 if I2C
+	select SND_SOC_TAS571X if I2C
 	select SND_SOC_TFA9879 if I2C
 	select SND_SOC_TLV320AIC23_I2C if I2C
 	select SND_SOC_TLV320AIC23_SPI if SPI_MASTER
@@ -611,6 +612,10 @@ config SND_SOC_TAS5086
 	tristate "Texas Instruments TAS5086 speaker amplifier"
 	depends on I2C
 
+config SND_SOC_TAS571X
+	tristate "Texas Instruments TAS5711/TAS5717/TAS5719 power amplifiers"
+	depends on I2C
+
 config SND_SOC_TFA9879
 	tristate "NXP Semiconductors TFA9879 amplifier"
 	depends on I2C
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index abe2d7edf65c..3dcf5ac85e89 100644
--- a/sound/soc/codecs/Makefile
+++ b/sound/soc/codecs/Makefile
@@ -106,6 +106,7 @@ snd-soc-sta350-objs := sta350.o
 snd-soc-sta529-objs := sta529.o
 snd-soc-stac9766-objs := stac9766.o
 snd-soc-tas5086-objs := tas5086.o
+snd-soc-tas571x-objs := tas571x.o
 snd-soc-tfa9879-objs := tfa9879.o
 snd-soc-tlv320aic23-objs := tlv320aic23.o
 snd-soc-tlv320aic23-i2c-objs := tlv320aic23-i2c.o
@@ -288,6 +289,7 @@ obj-$(CONFIG_SND_SOC_STA529)   += snd-soc-sta529.o
 obj-$(CONFIG_SND_SOC_STAC9766)	+= snd-soc-stac9766.o
 obj-$(CONFIG_SND_SOC_TAS2552)	+= snd-soc-tas2552.o
 obj-$(CONFIG_SND_SOC_TAS5086)	+= snd-soc-tas5086.o
+obj-$(CONFIG_SND_SOC_TAS571X)	+= snd-soc-tas571x.o
 obj-$(CONFIG_SND_SOC_TFA9879)	+= snd-soc-tfa9879.o
 obj-$(CONFIG_SND_SOC_TLV320AIC23)	+= snd-soc-tlv320aic23.o
 obj-$(CONFIG_SND_SOC_TLV320AIC23_I2C)	+= snd-soc-tlv320aic23-i2c.o
diff --git a/sound/soc/codecs/tas571x.c b/sound/soc/codecs/tas571x.c
new file mode 100644
index 000000000000..ffdf48397491
--- /dev/null
+++ b/sound/soc/codecs/tas571x.c
@@ -0,0 +1,520 @@
+/*
+ * TAS571x amplifier audio driver
+ *
+ * Copyright (C) 2015 Google, Inc.
+ * Copyright (c) 2013 Daniel Mack <zonque@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/stddef.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+#include <sound/tlv.h>
+
+#include "tas571x.h"
+
+#define TAS571X_MAX_SUPPLIES		6
+
+struct tas571x_chip {
+	const char			*const *supply_names;
+	int				num_supply_names;
+	const struct snd_kcontrol_new	*controls;
+	int				num_controls;
+	const struct regmap_config	*regmap_config;
+	int				vol_reg_size;
+};
+
+struct tas571x_private {
+	const struct tas571x_chip	*chip;
+	struct regmap			*regmap;
+	struct regulator_bulk_data	supplies[TAS571X_MAX_SUPPLIES];
+	struct clk			*mclk;
+	unsigned int			format;
+	struct gpio_desc		*reset_gpio;
+	struct gpio_desc		*pdn_gpio;
+	struct snd_soc_codec_driver	codec_driver;
+};
+
+static int tas571x_register_size(struct tas571x_private *priv, unsigned int reg)
+{
+	switch (reg) {
+	case TAS571X_MVOL_REG:
+	case TAS571X_CH1_VOL_REG:
+	case TAS571X_CH2_VOL_REG:
+		return priv->chip->vol_reg_size;
+	default:
+		return 1;
+	}
+}
+
+static int tas571x_reg_write(void *context, unsigned int reg,
+			     unsigned int value)
+{
+	struct i2c_client *client = context;
+	struct tas571x_private *priv = i2c_get_clientdata(client);
+	unsigned int i, size;
+	uint8_t buf[5];
+	int ret;
+
+	size = tas571x_register_size(priv, reg);
+	buf[0] = reg;
+
+	for (i = size; i >= 1; --i) {
+		buf[i] = value;
+		value >>= 8;
+	}
+
+	ret = i2c_master_send(client, buf, size + 1);
+	if (ret == size + 1)
+		return 0;
+	else if (ret < 0)
+		return ret;
+	else
+		return -EIO;
+}
+
+static int tas571x_reg_read(void *context, unsigned int reg,
+			    unsigned int *value)
+{
+	struct i2c_client *client = context;
+	struct tas571x_private *priv = i2c_get_clientdata(client);
+	uint8_t send_buf, recv_buf[4];
+	struct i2c_msg msgs[2];
+	unsigned int size;
+	unsigned int i;
+	int ret;
+
+	size = tas571x_register_size(priv, reg);
+	send_buf = reg;
+
+	msgs[0].addr = client->addr;
+	msgs[0].len = sizeof(send_buf);
+	msgs[0].buf = &send_buf;
+	msgs[0].flags = 0;
+
+	msgs[1].addr = client->addr;
+	msgs[1].len = size;
+	msgs[1].buf = recv_buf;
+	msgs[1].flags = I2C_M_RD;
+
+	ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
+	if (ret < 0)
+		return ret;
+	else if (ret != ARRAY_SIZE(msgs))
+		return -EIO;
+
+	*value = 0;
+
+	for (i = 0; i < size; i++) {
+		*value <<= 8;
+		*value |= recv_buf[i];
+	}
+
+	return 0;
+}
+
+static int tas571x_set_dai_fmt(struct snd_soc_dai *dai, unsigned int format)
+{
+	struct tas571x_private *priv = snd_soc_codec_get_drvdata(dai->codec);
+
+	priv->format = format;
+
+	return 0;
+}
+
+static int tas571x_hw_params(struct snd_pcm_substream *substream,
+			     struct snd_pcm_hw_params *params,
+			     struct snd_soc_dai *dai)
+{
+	struct tas571x_private *priv = snd_soc_codec_get_drvdata(dai->codec);
+	u32 val;
+
+	switch (priv->format & SND_SOC_DAIFMT_FORMAT_MASK) {
+	case SND_SOC_DAIFMT_RIGHT_J:
+		val = 0x00;
+		break;
+	case SND_SOC_DAIFMT_I2S:
+		val = 0x03;
+		break;
+	case SND_SOC_DAIFMT_LEFT_J:
+		val = 0x06;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (params_width(params) >= 24)
+		val += 2;
+	else if (params_width(params) >= 20)
+		val += 1;
+
+	return regmap_update_bits(priv->regmap, TAS571X_SDI_REG,
+				  TAS571X_SDI_FMT_MASK, val);
+}
+
+static int tas571x_set_bias_level(struct snd_soc_codec *codec,
+				  enum snd_soc_bias_level level)
+{
+	struct tas571x_private *priv = snd_soc_codec_get_drvdata(codec);
+	int ret;
+
+	switch (level) {
+	case SND_SOC_BIAS_ON:
+		break;
+	case SND_SOC_BIAS_PREPARE:
+		break;
+	case SND_SOC_BIAS_STANDBY:
+		if (codec->dapm.bias_level == SND_SOC_BIAS_OFF) {
+			if (!IS_ERR(priv->mclk)) {
+				ret = clk_prepare_enable(priv->mclk);
+				if (ret) {
+					dev_err(codec->dev,
+						"Failed to enable master clock: %d\n",
+						ret);
+					return ret;
+				}
+			}
+
+			gpiod_set_value(priv->pdn_gpio, 0);
+			usleep_range(5000, 6000);
+
+			regcache_cache_only(priv->regmap, false);
+			ret = regcache_sync(priv->regmap);
+			if (ret)
+				return ret;
+		}
+		break;
+	case SND_SOC_BIAS_OFF:
+		regcache_cache_only(priv->regmap, true);
+		gpiod_set_value(priv->pdn_gpio, 1);
+
+		if (!IS_ERR(priv->mclk))
+			clk_disable_unprepare(priv->mclk);
+		break;
+	}
+
+	codec->dapm.bias_level = level;
+	return 0;
+}
+
+static const struct snd_soc_dai_ops tas571x_dai_ops = {
+	.set_fmt	= tas571x_set_dai_fmt,
+	.hw_params	= tas571x_hw_params,
+};
+
+static const char *const tas5711_supply_names[] = {
+	"AVDD",
+	"DVDD",
+	"PVDD_A",
+	"PVDD_B",
+	"PVDD_C",
+	"PVDD_D",
+};
+
+static const DECLARE_TLV_DB_SCALE(tas5711_volume_tlv, -10350, 50, 1);
+
+static const struct snd_kcontrol_new tas5711_controls[] = {
+	SOC_SINGLE_TLV("Master Volume",
+		       TAS571X_MVOL_REG,
+		       0, 0xff, 1, tas5711_volume_tlv),
+	SOC_DOUBLE_R_TLV("Speaker Volume",
+			 TAS571X_CH1_VOL_REG,
+			 TAS571X_CH2_VOL_REG,
+			 0, 0xff, 1, tas5711_volume_tlv),
+	SOC_DOUBLE("Speaker Switch",
+		   TAS571X_SOFT_MUTE_REG,
+		   TAS571X_SOFT_MUTE_CH1_SHIFT, TAS571X_SOFT_MUTE_CH2_SHIFT,
+		   1, 1),
+};
+
+static const struct reg_default tas5711_reg_defaults[] = {
+	{ 0x04, 0x05 },
+	{ 0x05, 0x40 },
+	{ 0x06, 0x00 },
+	{ 0x07, 0xff },
+	{ 0x08, 0x30 },
+	{ 0x09, 0x30 },
+	{ 0x1b, 0x82 },
+};
+
+static const struct regmap_config tas5711_regmap_config = {
+	.reg_bits			= 8,
+	.val_bits			= 32,
+	.max_register			= 0xff,
+	.reg_read			= tas571x_reg_read,
+	.reg_write			= tas571x_reg_write,
+	.reg_defaults			= tas5711_reg_defaults,
+	.num_reg_defaults		= ARRAY_SIZE(tas5711_reg_defaults),
+	.cache_type			= REGCACHE_RBTREE,
+};
+
+static const struct tas571x_chip tas5711_chip = {
+	.supply_names			= tas5711_supply_names,
+	.num_supply_names		= ARRAY_SIZE(tas5711_supply_names),
+	.controls			= tas5711_controls,
+	.num_controls			= ARRAY_SIZE(tas5711_controls),
+	.regmap_config			= &tas5711_regmap_config,
+	.vol_reg_size			= 1,
+};
+
+static const char *const tas5717_supply_names[] = {
+	"AVDD",
+	"DVDD",
+	"HPVDD",
+	"PVDD_AB",
+	"PVDD_CD",
+};
+
+static const DECLARE_TLV_DB_SCALE(tas5717_volume_tlv, -10375, 25, 0);
+
+static const struct snd_kcontrol_new tas5717_controls[] = {
+	/* MVOL LSB is ignored - see comments in tas571x_i2c_probe() */
+	SOC_SINGLE_TLV("Master Volume",
+		       TAS571X_MVOL_REG, 1, 0x1ff, 1,
+		       tas5717_volume_tlv),
+	SOC_DOUBLE_R_TLV("Speaker Volume",
+			 TAS571X_CH1_VOL_REG, TAS571X_CH2_VOL_REG,
+			 1, 0x1ff, 1, tas5717_volume_tlv),
+	SOC_DOUBLE("Speaker Switch",
+		   TAS571X_SOFT_MUTE_REG,
+		   TAS571X_SOFT_MUTE_CH1_SHIFT, TAS571X_SOFT_MUTE_CH2_SHIFT,
+		   1, 1),
+};
+
+static const struct reg_default tas5717_reg_defaults[] = {
+	{ 0x04, 0x05 },
+	{ 0x05, 0x40 },
+	{ 0x06, 0x00 },
+	{ 0x07, 0x03ff },
+	{ 0x08, 0x00c0 },
+	{ 0x09, 0x00c0 },
+	{ 0x1b, 0x82 },
+};
+
+static const struct regmap_config tas5717_regmap_config = {
+	.reg_bits			= 8,
+	.val_bits			= 32,
+	.max_register			= 0xff,
+	.reg_read			= tas571x_reg_read,
+	.reg_write			= tas571x_reg_write,
+	.reg_defaults			= tas5717_reg_defaults,
+	.num_reg_defaults		= ARRAY_SIZE(tas5717_reg_defaults),
+	.cache_type			= REGCACHE_RBTREE,
+};
+
+/* This entry is reused for tas5719 as the software interface is identical. */
+static const struct tas571x_chip tas5717_chip = {
+	.supply_names			= tas5717_supply_names,
+	.num_supply_names		= ARRAY_SIZE(tas5717_supply_names),
+	.controls			= tas5717_controls,
+	.num_controls			= ARRAY_SIZE(tas5717_controls),
+	.regmap_config			= &tas5717_regmap_config,
+	.vol_reg_size			= 2,
+};
+
+static const struct snd_soc_dapm_widget tas571x_dapm_widgets[] = {
+	SND_SOC_DAPM_DAC("DACL", NULL, SND_SOC_NOPM, 0, 0),
+	SND_SOC_DAPM_DAC("DACR", NULL, SND_SOC_NOPM, 0, 0),
+
+	SND_SOC_DAPM_OUTPUT("OUT_A"),
+	SND_SOC_DAPM_OUTPUT("OUT_B"),
+	SND_SOC_DAPM_OUTPUT("OUT_C"),
+	SND_SOC_DAPM_OUTPUT("OUT_D"),
+};
+
+static const struct snd_soc_dapm_route tas571x_dapm_routes[] = {
+	{ "DACL",  NULL, "Playback" },
+	{ "DACR",  NULL, "Playback" },
+
+	{ "OUT_A", NULL, "DACL" },
+	{ "OUT_B", NULL, "DACL" },
+	{ "OUT_C", NULL, "DACR" },
+	{ "OUT_D", NULL, "DACR" },
+};
+
+static const struct snd_soc_codec_driver tas571x_codec = {
+	.set_bias_level = tas571x_set_bias_level,
+	.idle_bias_off = true,
+
+	.dapm_widgets = tas571x_dapm_widgets,
+	.num_dapm_widgets = ARRAY_SIZE(tas571x_dapm_widgets),
+	.dapm_routes = tas571x_dapm_routes,
+	.num_dapm_routes = ARRAY_SIZE(tas571x_dapm_routes),
+};
+
+static struct snd_soc_dai_driver tas571x_dai = {
+	.name = "tas571x-hifi",
+	.playback = {
+		.stream_name = "Playback",
+		.channels_min = 2,
+		.channels_max = 2,
+		.rates = SNDRV_PCM_RATE_8000_48000,
+		.formats = SNDRV_PCM_FMTBIT_S32_LE |
+			   SNDRV_PCM_FMTBIT_S24_LE |
+			   SNDRV_PCM_FMTBIT_S16_LE,
+	},
+	.ops = &tas571x_dai_ops,
+};
+
+static const struct of_device_id tas571x_of_match[];
+
+static int tas571x_i2c_probe(struct i2c_client *client,
+			     const struct i2c_device_id *id)
+{
+	struct tas571x_private *priv;
+	struct device *dev = &client->dev;
+	int i, ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+	i2c_set_clientdata(client, priv);
+
+	if (dev->of_node) {
+		const struct of_device_id *of_id;
+
+		of_id = of_match_device(tas571x_of_match, dev);
+		if (of_id)
+			priv->chip = of_id->data;
+	}
+
+	if (!priv->chip) {
+		dev_err(dev, "Unknown device type\n");
+		return -EINVAL;
+	}
+
+	priv->mclk = devm_clk_get(dev, "mclk");
+	if (IS_ERR(priv->mclk) && PTR_ERR(priv->mclk) != -ENOENT) {
+		dev_err(dev, "Failed to request mclk: %ld\n",
+			PTR_ERR(priv->mclk));
+		return PTR_ERR(priv->mclk);
+	}
+
+	BUG_ON(priv->chip->num_supply_names > TAS571X_MAX_SUPPLIES);
+	for (i = 0; i < priv->chip->num_supply_names; i++)
+		priv->supplies[i].supply = priv->chip->supply_names[i];
+
+	ret = devm_regulator_bulk_get(dev, priv->chip->num_supply_names,
+				      priv->supplies);
+	if (ret) {
+		dev_err(dev, "Failed to get supplies: %d\n", ret);
+		return ret;
+	}
+	ret = regulator_bulk_enable(priv->chip->num_supply_names,
+				    priv->supplies);
+	if (ret) {
+		dev_err(dev, "Failed to enable supplies: %d\n", ret);
+		return ret;
+	}
+
+	priv->regmap = devm_regmap_init(dev, NULL, client,
+					priv->chip->regmap_config);
+	if (IS_ERR(priv->regmap))
+		return PTR_ERR(priv->regmap);
+
+	priv->pdn_gpio = devm_gpiod_get_optional(dev, "pdn", GPIOD_OUT_LOW);
+	if (IS_ERR(priv->pdn_gpio)) {
+		dev_err(dev, "error requesting pdn_gpio: %ld\n",
+			PTR_ERR(priv->pdn_gpio));
+		return PTR_ERR(priv->pdn_gpio);
+	}
+
+	priv->reset_gpio = devm_gpiod_get_optional(dev, "reset",
+						   GPIOD_OUT_HIGH);
+	if (IS_ERR(priv->reset_gpio)) {
+		dev_err(dev, "error requesting reset_gpio: %ld\n",
+			PTR_ERR(priv->reset_gpio));
+		return PTR_ERR(priv->reset_gpio);
+	} else if (priv->reset_gpio) {
+		/* pulse the active low reset line for ~100us */
+		usleep_range(100, 200);
+		gpiod_set_value(priv->reset_gpio, 0);
+		usleep_range(12000, 20000);
+	}
+
+	ret = regmap_write(priv->regmap, TAS571X_OSC_TRIM_REG, 0);
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(priv->regmap, TAS571X_SYS_CTRL_2_REG,
+				 TAS571X_SYS_CTRL_2_SDN_MASK, 0);
+	if (ret)
+		return ret;
+
+	memcpy(&priv->codec_driver, &tas571x_codec, sizeof(priv->codec_driver));
+	priv->codec_driver.controls = priv->chip->controls;
+	priv->codec_driver.num_controls = priv->chip->num_controls;
+
+	if (priv->chip->vol_reg_size == 2) {
+		/*
+		 * The master volume defaults to 0x3ff (mute), but we ignore
+		 * (zero) the LSB because the hardware step size is 0.125 dB
+		 * and TLV_DB_SCALE_ITEM has a resolution of 0.01 dB.
+		 */
+		ret = regmap_update_bits(priv->regmap, TAS571X_MVOL_REG, 1, 0);
+		if (ret)
+			return ret;
+	}
+
+	regcache_cache_only(priv->regmap, true);
+	gpiod_set_value(priv->pdn_gpio, 1);
+
+	return snd_soc_register_codec(&client->dev, &priv->codec_driver,
+				      &tas571x_dai, 1);
+}
+
+static int tas571x_i2c_remove(struct i2c_client *client)
+{
+	struct tas571x_private *priv = i2c_get_clientdata(client);
+
+	snd_soc_unregister_codec(&client->dev);
+	regulator_bulk_disable(priv->chip->num_supply_names, priv->supplies);
+
+	return 0;
+}
+
+static const struct of_device_id tas571x_of_match[] = {
+	{ .compatible = "ti,tas5711", .data = &tas5711_chip, },
+	{ .compatible = "ti,tas5717", .data = &tas5717_chip, },
+	{ .compatible = "ti,tas5719", .data = &tas5717_chip, },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, tas571x_of_match);
+
+static const struct i2c_device_id tas571x_i2c_id[] = {
+	{ "tas5711", 0 },
+	{ "tas5717", 0 },
+	{ "tas5719", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, tas571x_i2c_id);
+
+static struct i2c_driver tas571x_i2c_driver = {
+	.driver = {
+		.name = "tas571x",
+		.of_match_table = of_match_ptr(tas571x_of_match),
+	},
+	.probe = tas571x_i2c_probe,
+	.remove = tas571x_i2c_remove,
+	.id_table = tas571x_i2c_id,
+};
+module_i2c_driver(tas571x_i2c_driver);
+
+MODULE_DESCRIPTION("ASoC TAS571x driver");
+MODULE_AUTHOR("Kevin Cernekee <cernekee@chromium.org>");
+MODULE_LICENSE("GPL");
diff --git a/sound/soc/codecs/tas571x.h b/sound/soc/codecs/tas571x.h
new file mode 100644
index 000000000000..0aee471232cd
--- /dev/null
+++ b/sound/soc/codecs/tas571x.h
@@ -0,0 +1,33 @@
+/*
+ * TAS571x amplifier audio driver
+ *
+ * Copyright (C) 2015 Google, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#ifndef _TAS571X_H
+#define _TAS571X_H
+
+/* device registers */
+#define TAS571X_SDI_REG			0x04
+#define TAS571X_SDI_FMT_MASK		0x0f
+
+#define TAS571X_SYS_CTRL_2_REG		0x05
+#define TAS571X_SYS_CTRL_2_SDN_MASK	0x40
+
+#define TAS571X_SOFT_MUTE_REG		0x06
+#define TAS571X_SOFT_MUTE_CH1_SHIFT	0
+#define TAS571X_SOFT_MUTE_CH2_SHIFT	1
+#define TAS571X_SOFT_MUTE_CH3_SHIFT	2
+
+#define TAS571X_MVOL_REG		0x07
+#define TAS571X_CH1_VOL_REG		0x08
+#define TAS571X_CH2_VOL_REG		0x09
+
+#define TAS571X_OSC_TRIM_REG		0x1b
+
+#endif /* _TAS571X_H */
-- 
2.2.0.rc0.207.ga3a616c


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH V3 4/4] MAINTAINERS: Add entry for tas571x ASoC codec driver
  2015-05-04  0:00 [PATCH V3 0/4] tas571x amplifier driver Kevin Cernekee
                   ` (2 preceding siblings ...)
  2015-05-04  0:00 ` [PATCH V3 3/4] ASoC: tas571x: New driver for TI TAS571x power amplifiers Kevin Cernekee
@ 2015-05-04  0:00 ` Kevin Cernekee
  3 siblings, 0 replies; 11+ messages in thread
From: Kevin Cernekee @ 2015-05-04  0:00 UTC (permalink / raw)
  To: lgirdwood, broonie
  Cc: lars, dgreid, abrestic, olofj, alsa-devel, devicetree, linux-kernel

Add self as maintainer for the new driver.

Signed-off-by: Kevin Cernekee <cernekee@chromium.org>
---
 MAINTAINERS | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index ea0001760035..15153fc37cc4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9878,6 +9878,12 @@ L:	netdev@vger.kernel.org
 S:	Maintained
 F:	drivers/net/ethernet/ti/netcp*
 
+TI TAS571X FAMILY ASoC CODEC DRIVER
+M:	Kevin Cernekee <cernekee@chromium.org>
+L:	alsa-devel@alsa-project.org (moderated for non-subscribers)
+S:	Odd Fixes
+F:	sound/soc/codecs/tas571x*
+
 TI TWL4030 SERIES SOC CODEC DRIVER
 M:	Peter Ujfalusi <peter.ujfalusi@ti.com>
 L:	alsa-devel@alsa-project.org (moderated for non-subscribers)
-- 
2.2.0.rc0.207.ga3a616c


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH V3 1/4] regmap: Use regcache_mark_dirty() to indicate power loss or reset
  2015-05-04  0:00 ` [PATCH V3 1/4] regmap: Use regcache_mark_dirty() to indicate power loss or reset Kevin Cernekee
@ 2015-05-04  6:38   ` Lars-Peter Clausen
  2015-05-04 14:05     ` Kevin Cernekee
  0 siblings, 1 reply; 11+ messages in thread
From: Lars-Peter Clausen @ 2015-05-04  6:38 UTC (permalink / raw)
  To: Kevin Cernekee, lgirdwood, broonie
  Cc: dgreid, abrestic, olofj, alsa-devel, devicetree, linux-kernel

On 05/04/2015 02:00 AM, Kevin Cernekee wrote:
> Existing regmap users call regcache_mark_dirty() as part of the
> suspend/resume sequence, to tell regcache that non-default values need to
> be resynced post-resume.  Add an internal "no_sync_defaults" regmap flag
> to remember this state, so that regcache_sync() can differentiate between
> these two cases:
>
> 1) HW was reset, so any cache values that match map->reg_defaults can be
> safely skipped.  On some chips there are a lot of registers in the
> reg_defaults list, so this optimization speeds things up quite a bit.
>
> 2) HW was not reset (maybe it was just clock-gated), so if we cached
> any writes, they should be sent to the hardware regardless of whether
> they match the HW default.  Currently this will write out all values in
> the regcache, since we don't maintain per-register dirty bits.
>
> Suggested-by: Mark Brown <broonie@kernel.org>
> Signed-off-by: Kevin Cernekee <cernekee@chromium.org>
> ---
>   drivers/base/regmap/internal.h |  3 +++
>   drivers/base/regmap/regcache.c | 61 ++++++++++++++++++++++++++++--------------
>   2 files changed, 44 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
> index a13587b5c2be..b2b2849fc6d3 100644
> --- a/drivers/base/regmap/internal.h
> +++ b/drivers/base/regmap/internal.h
> @@ -131,7 +131,10 @@ struct regmap {
>   	struct reg_default *reg_defaults;
>   	const void *reg_defaults_raw;
>   	void *cache;
> +	/* if set, the cache contains newer data than the HW */
>   	u32 cache_dirty;
> +	/* if set, the HW registers are known to match map->reg_defaults */
> +	bool no_sync_defaults;
>
>   	struct reg_default *patch;
>   	int patch_regs;
> diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
> index 7eb7b3b98794..63af3103d0c6 100644
> --- a/drivers/base/regmap/regcache.c
> +++ b/drivers/base/regmap/regcache.c
> @@ -253,6 +253,9 @@ static int regcache_default_sync(struct regmap *map, unsigned int min,
>   				 unsigned int max)
>   {
>   	unsigned int reg;
> +	bool no_sync_defaults = map->no_sync_defaults;
> +
> +	map->no_sync_defaults = false;

This needs to be done at the end in regcache_sync(), the same place where 
dirty is set to false.

>
>   	for (reg = min; reg <= max; reg += map->reg_stride) {
>   		unsigned int val;
> @@ -266,10 +269,12 @@ static int regcache_default_sync(struct regmap *map, unsigned int min,
>   		if (ret)
>   			return ret;
>
> -		/* Is this the hardware default?  If so skip. */
> -		ret = regcache_lookup_reg(map, reg);
> -		if (ret >= 0 && val == map->reg_defaults[ret].def)
> -			continue;
> +		if (no_sync_defaults) {
> +			/* Is this the hardware default?  If so skip. */
> +			ret = regcache_lookup_reg(map, reg);
> +			if (ret >= 0 && val == map->reg_defaults[ret].def)
> +				continue;
> +		}

This should go into a helper function regacache_reg_needs_sync() so it can 
be reused at the other places where the same logic is needed.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH V3 3/4] ASoC: tas571x: New driver for TI TAS571x power amplifiers
  2015-05-04  0:00 ` [PATCH V3 3/4] ASoC: tas571x: New driver for TI TAS571x power amplifiers Kevin Cernekee
@ 2015-05-04 11:45   ` Mark Brown
  2015-05-04 13:51     ` Kevin Cernekee
  0 siblings, 1 reply; 11+ messages in thread
From: Mark Brown @ 2015-05-04 11:45 UTC (permalink / raw)
  To: Kevin Cernekee
  Cc: lgirdwood, lars, dgreid, abrestic, olofj, alsa-devel, devicetree,
	linux-kernel

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

On Sun, May 03, 2015 at 05:00:18PM -0700, Kevin Cernekee wrote:

> +	}
> +
> +	codec->dapm.bias_level = level;
> +	return 0;
> +}

This assignment was factored out into the core last week, I'll fix up
for that.

> +	if (dev->of_node) {
> +		const struct of_device_id *of_id;
> +
> +		of_id = of_match_device(tas571x_of_match, dev);
> +		if (of_id)
> +			priv->chip = of_id->data;
> +	}

Doesn't of_match_device() do the right thing with devices not registered
from DT?

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH V3 3/4] ASoC: tas571x: New driver for TI TAS571x power amplifiers
  2015-05-04 11:45   ` Mark Brown
@ 2015-05-04 13:51     ` Kevin Cernekee
  2015-05-04 13:57       ` Mark Brown
  0 siblings, 1 reply; 11+ messages in thread
From: Kevin Cernekee @ 2015-05-04 13:51 UTC (permalink / raw)
  To: Mark Brown
  Cc: Liam Girdwood, Lars-Peter Clausen, dgreid, Andrew Bresticker,
	Olof Johansson, alsa-devel, devicetree, linux-kernel

On Mon, May 4, 2015 at 4:45 AM, Mark Brown <broonie@kernel.org> wrote:
> On Sun, May 03, 2015 at 05:00:18PM -0700, Kevin Cernekee wrote:
>> +     if (dev->of_node) {
>> +             const struct of_device_id *of_id;
>> +
>> +             of_id = of_match_device(tas571x_of_match, dev);
>> +             if (of_id)
>> +                     priv->chip = of_id->data;
>> +     }
>
> Doesn't of_match_device() do the right thing with devices not registered
> from DT?

Not sure.  What kinds of situations are you concerned about?

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH V3 3/4] ASoC: tas571x: New driver for TI TAS571x power amplifiers
  2015-05-04 13:51     ` Kevin Cernekee
@ 2015-05-04 13:57       ` Mark Brown
  0 siblings, 0 replies; 11+ messages in thread
From: Mark Brown @ 2015-05-04 13:57 UTC (permalink / raw)
  To: Kevin Cernekee
  Cc: Liam Girdwood, Lars-Peter Clausen, dgreid, Andrew Bresticker,
	Olof Johansson, alsa-devel, devicetree, linux-kernel

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

On Mon, May 04, 2015 at 06:51:32AM -0700, Kevin Cernekee wrote:
> On Mon, May 4, 2015 at 4:45 AM, Mark Brown <broonie@kernel.org> wrote:
> > On Sun, May 03, 2015 at 05:00:18PM -0700, Kevin Cernekee wrote:
> >> +     if (dev->of_node) {
> >> +             const struct of_device_id *of_id;
> >> +
> >> +             of_id = of_match_device(tas571x_of_match, dev);
> >> +             if (of_id)
> >> +                     priv->chip = of_id->data;
> >> +     }

> > Doesn't of_match_device() do the right thing with devices not registered
> > from DT?

> Not sure.  What kinds of situations are you concerned about?

I'm wondering why we need to bother checking for dev->of_node before
calling it.

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH V3 1/4] regmap: Use regcache_mark_dirty() to indicate power loss or reset
  2015-05-04  6:38   ` Lars-Peter Clausen
@ 2015-05-04 14:05     ` Kevin Cernekee
  2015-05-04 17:33       ` Lars-Peter Clausen
  0 siblings, 1 reply; 11+ messages in thread
From: Kevin Cernekee @ 2015-05-04 14:05 UTC (permalink / raw)
  To: Lars-Peter Clausen
  Cc: Liam Girdwood, Mark Brown, dgreid, Andrew Bresticker,
	Olof Johansson, alsa-devel, devicetree, linux-kernel

On Sun, May 3, 2015 at 11:38 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
>> diff --git a/drivers/base/regmap/regcache.c
>> b/drivers/base/regmap/regcache.c
>> index 7eb7b3b98794..63af3103d0c6 100644
>> --- a/drivers/base/regmap/regcache.c
>> +++ b/drivers/base/regmap/regcache.c
>> @@ -253,6 +253,9 @@ static int regcache_default_sync(struct regmap *map,
>> unsigned int min,
>>                                  unsigned int max)
>>   {
>>         unsigned int reg;
>> +       bool no_sync_defaults = map->no_sync_defaults;
>> +
>> +       map->no_sync_defaults = false;
>
>
> This needs to be done at the end in regcache_sync(), the same place where
> dirty is set to false.

But map->cache_dirty means "any register is dirty," not "all registers
are dirty."  So it can only be cleared after a successful flush.

If one of the writes fails and regcache_sync() has to return
prematurely, we probably don't want no_sync_defaults to stay true
because some of the HW registers might not match map->reg_defaults
anymore.

Leaving it set to true wouldn't break the current regcache_sync()
implementation, but no_sync_defaults is documented as: "if set, the HW
registers are known to match map->reg_defaults".  Writing any register
to a non-default value makes it false.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH V3 1/4] regmap: Use regcache_mark_dirty() to indicate power loss or reset
  2015-05-04 14:05     ` Kevin Cernekee
@ 2015-05-04 17:33       ` Lars-Peter Clausen
  0 siblings, 0 replies; 11+ messages in thread
From: Lars-Peter Clausen @ 2015-05-04 17:33 UTC (permalink / raw)
  To: Kevin Cernekee
  Cc: Liam Girdwood, Mark Brown, dgreid, Andrew Bresticker,
	Olof Johansson, alsa-devel, devicetree, linux-kernel

On 05/04/2015 04:05 PM, Kevin Cernekee wrote:
> On Sun, May 3, 2015 at 11:38 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
>>> diff --git a/drivers/base/regmap/regcache.c
>>> b/drivers/base/regmap/regcache.c
>>> index 7eb7b3b98794..63af3103d0c6 100644
>>> --- a/drivers/base/regmap/regcache.c
>>> +++ b/drivers/base/regmap/regcache.c
>>> @@ -253,6 +253,9 @@ static int regcache_default_sync(struct regmap *map,
>>> unsigned int min,
>>>                                   unsigned int max)
>>>    {
>>>          unsigned int reg;
>>> +       bool no_sync_defaults = map->no_sync_defaults;
>>> +
>>> +       map->no_sync_defaults = false;
>>
>>
>> This needs to be done at the end in regcache_sync(), the same place where
>> dirty is set to false.
>
> But map->cache_dirty means "any register is dirty," not "all registers
> are dirty."  So it can only be cleared after a successful flush.
>
> If one of the writes fails and regcache_sync() has to return
> prematurely, we probably don't want no_sync_defaults to stay true
> because some of the HW registers might not match map->reg_defaults
> anymore.

Makes sense. But it should still be done in a central place rather than 
repeating it in every sync implementation. You can still put it at the end 
of regcache_sync(), just clear it regardless of whether it succeeded or not.


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2015-05-04 17:33 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-04  0:00 [PATCH V3 0/4] tas571x amplifier driver Kevin Cernekee
2015-05-04  0:00 ` [PATCH V3 1/4] regmap: Use regcache_mark_dirty() to indicate power loss or reset Kevin Cernekee
2015-05-04  6:38   ` Lars-Peter Clausen
2015-05-04 14:05     ` Kevin Cernekee
2015-05-04 17:33       ` Lars-Peter Clausen
2015-05-04  0:00 ` [PATCH V3 2/4] ASoC: tas571x: Add DT binding document Kevin Cernekee
2015-05-04  0:00 ` [PATCH V3 3/4] ASoC: tas571x: New driver for TI TAS571x power amplifiers Kevin Cernekee
2015-05-04 11:45   ` Mark Brown
2015-05-04 13:51     ` Kevin Cernekee
2015-05-04 13:57       ` Mark Brown
2015-05-04  0:00 ` [PATCH V3 4/4] MAINTAINERS: Add entry for tas571x ASoC codec driver Kevin Cernekee

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).