[PATCH 1/3] regmap: Add support for device specific write and read flag masks.

[PATCH 1/3] regmap: Add support for device specific write and read flag masks.

[Lars-Peter Clausen]

Some buses like SPI have no standard notation of read or write operations.
The general scheme here is to set or clear specific bits in the register
address to indicate whether the operation is a read or write. We already
support having a read flag mask per bus, but as there is no standard
the bits which need to be set or cleared differ between devices and vendors,
thus we need a mechanism to specify them per device.

This patch adds two new entries to the regmap_config struct, read_flag_mask and
write_flag_mask. These will be or'ed onto the top byte when doing a read or
write operation. If both masks are empty the device will fallback to the
regmap_bus masks.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/base/regmap/internal.h |    3 +++
 drivers/base/regmap/regmap.c   |   15 ++++++++++++---
 include/linux/regmap.h         |    9 +++++++++
 3 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
index 5ab3fef..7e14d5a 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -46,6 +46,9 @@ struct regmap {
 	bool (*readable_reg)(struct device *dev, unsigned int reg);
 	bool (*volatile_reg)(struct device *dev, unsigned int reg);
 	bool (*precious_reg)(struct device *dev, unsigned int reg);
+
+	u8 read_flag_mask;
+	u8 write_flag_mask;
 };
 
 bool regmap_writeable(struct regmap *map, unsigned int reg);
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index fa2bd89..8ecf8bf 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -147,6 +147,13 @@ struct regmap *regmap_init(struct device *dev,
 	map->volatile_reg = config->volatile_reg;
 	map->precious_reg = config->precious_reg;
 
+	if (config->read_flag_mask || config->write_flag_mask) {
+		map->read_flag_mask = config->read_flag_mask;
+		map->write_flag_mask = config->write_flag_mask;
+	} else {
+		map->read_flag_mask = bus->read_flag_mask;
+	}
+
 	switch (config->reg_bits) {
 	case 4:
 		switch (config->val_bits) {
@@ -229,6 +236,7 @@ EXPORT_SYMBOL_GPL(regmap_exit);
 static int _regmap_raw_write(struct regmap *map, unsigned int reg,
 			     const void *val, size_t val_len)
 {
+	u8 *u8 = map->work_buf;
 	void *buf;
 	int ret = -ENOTSUPP;
 	size_t len;
@@ -242,6 +250,8 @@ static int _regmap_raw_write(struct regmap *map, unsigned int reg,
 
 	map->format.format_reg(map->work_buf, reg);
 
+	u8[0] |= map->write_flag_mask;
+
 	trace_regmap_hw_write_start(map->dev, reg,
 				    val_len / map->format.val_bytes);
 
@@ -369,13 +379,12 @@ static int _regmap_raw_read(struct regmap *map, unsigned int reg, void *val,
 	map->format.format_reg(map->work_buf, reg);
 
 	/*
-	 * Some buses flag reads by setting the high bits in the
+	 * Some buses or devices flag reads by setting the high bits in the
 	 * register addresss; since it's always the high bits for all
 	 * current formats we can do this here rather than in
 	 * formatting.  This may break if we get interesting formats.
 	 */
-	if (map->bus->read_flag_mask)
-		u8[0] |= map->bus->read_flag_mask;
+	u8[0] |= map->read_flag_mask;
 
 	trace_regmap_hw_read_start(map->dev, reg,
 				   val_len / map->format.val_bytes);
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 449e264..f0b8d47 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -53,6 +53,12 @@ struct reg_default {
  * @reg_defaults: Power on reset values for registers (for use with
  *                register cache support).
  * @num_reg_defaults: Number of elements in reg_defaults.
+ *
+ * @read_flag_mask: Mask to be set in the top byte of the register when doing
+ *                  a read.
+ * @write_flag_mask: Mask to be set in the top byte of the register when doing
+ *                   a write. If both read_flag_mask and write_flag_mask are
+ *                   empty the regmap_bus default masks are used.
  */
 struct regmap_config {
 	int reg_bits;
@@ -66,6 +72,9 @@ struct regmap_config {
 	unsigned int max_register;
 	struct reg_default *reg_defaults;
 	int num_reg_defaults;
+
+	u8 read_flag_mask;
+	u8 write_flag_mask;
 };
 
 typedef int (*regmap_hw_write)(struct device *dev, const void *data,
-- 
1.7.2.5

[PATCH 2/3] ASoC: Allow codecs to provide their own regmap configuration

[Lars-Peter Clausen]

Currently a codec can either do the whole regmap initialization on its own or
provide only the register and value bit size and let the core handle
initialization. This patch allows a codec to provide a complete regmap
configuration while still letting ASoC core handle the regmap initialization
and setup.

This is useful for cases where we want to specify more than just the register and
value bit size of the regmap configuration, for example read and write flag masks.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 include/sound/soc.h |    3 +++
 sound/soc/soc-io.c  |   49 +++++++++++++++++++++++++++++++++----------------
 2 files changed, 36 insertions(+), 16 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 6da55a1..fcb0528 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -301,6 +301,9 @@ int snd_soc_codec_writable_register(struct snd_soc_codec *codec,
 int snd_soc_codec_set_cache_io(struct snd_soc_codec *codec,
 			       int addr_bits, int data_bits,
 			       enum snd_soc_control_type control);
+int snd_soc_codec_regmap_init(struct snd_soc_codec *codec,
+			      struct regmap_config *config,
+			      enum snd_soc_control_type control);
 int snd_soc_cache_sync(struct snd_soc_codec *codec);
 int snd_soc_cache_init(struct snd_soc_codec *codec);
 int snd_soc_cache_exit(struct snd_soc_codec *codec);
diff --git a/sound/soc/soc-io.c b/sound/soc/soc-io.c
index 66fcccd..abc55af 100644
--- a/sound/soc/soc-io.c
+++ b/sound/soc/soc-io.c
@@ -97,16 +97,7 @@ static int snd_soc_hw_bulk_write_raw(struct snd_soc_codec *codec,
  * @data_bits: Number of bits of data per register.
  * @control: Control bus used.
  *
- * Register formats are frequently shared between many I2C and SPI
- * devices.  In order to promote code reuse the ASoC core provides
- * some standard implementations of CODEC read and write operations
- * which can be set up using this function.
- *
- * The caller is responsible for allocating and initialising the
- * actual cache.
- *
- * Note that at present this code cannot be used by CODECs with
- * volatile registers.
+ * Legacy and convenience wrapper around snd_soc_codec_regmap_init
  */
 int snd_soc_codec_set_cache_io(struct snd_soc_codec *codec,
 			       int addr_bits, int data_bits,
@@ -115,25 +106,43 @@ int snd_soc_codec_set_cache_io(struct snd_soc_codec *codec,
 	struct regmap_config config;
 
 	memset(&config, 0, sizeof(config));
+	config.reg_bits = addr_bits;
+	config.val_bits = data_bits;
+
+	return snd_soc_codec_regmap_init(codec, &config, control);
+}
+EXPORT_SYMBOL_GPL(snd_soc_codec_set_cache_io);
+
+/**
+ * snd_soc_codec_init_regmap: Set up regmap for the CODEC I/O functions.
+ *
+ * @codec: CODEC to configure.
+ * @config: regmap configuration for the CODEC
+ * @control: Control bus used.
+ *
+ * Initializes regmap for the specified control bus and hooks it up to the
+ * CODEC I/O functions.
+ */
+int snd_soc_codec_regmap_init(struct snd_soc_codec *codec,
+			      struct regmap_config *config,
+			      enum snd_soc_control_type control)
+{
 	codec->write = hw_write;
 	codec->read = hw_read;
 	codec->bulk_write_raw = snd_soc_hw_bulk_write_raw;
 
-	config.reg_bits = addr_bits;
-	config.val_bits = data_bits;
-
 	switch (control) {
 #if defined(CONFIG_REGMAP_I2C) || defined(CONFIG_REGMAP_I2C_MODULE)
 	case SND_SOC_I2C:
 		codec->control_data = regmap_init_i2c(to_i2c_client(codec->dev),
-						      &config);
+						      config);
 		break;
 #endif
 
 #if defined(CONFIG_REGMAP_SPI) || defined(CONFIG_REGMAP_SPI_MODULE)
 	case SND_SOC_SPI:
 		codec->control_data = regmap_init_spi(to_spi_device(codec->dev),
-						      &config);
+						      config);
 		break;
 #endif
 
@@ -150,7 +159,7 @@ int snd_soc_codec_set_cache_io(struct snd_soc_codec *codec,
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(snd_soc_codec_set_cache_io);
+EXPORT_SYMBOL_GPL(snd_soc_codec_regmap_init);
 #else
 int snd_soc_codec_set_cache_io(struct snd_soc_codec *codec,
 			       int addr_bits, int data_bits,
@@ -159,4 +168,12 @@ int snd_soc_codec_set_cache_io(struct snd_soc_codec *codec,
 	return -ENOTSUPP;
 }
 EXPORT_SYMBOL_GPL(snd_soc_codec_set_cache_io);
+
+int snd_soc_codec_regmap_init(struct snd_soc_codec *codec,
+			      struct regmap_config *config,
+			      enum snd_soc_control_type control)
+{
+	return -ENOTSUPP;
+}
+EXPORT_SYMBOL_GPL(snd_soc_codec_regmap_init);
 #endif
-- 
1.7.2.5

[PATCH 3/3] ASoC: ad193x: Setup regmap read and write flag masks for SPI

[Lars-Peter Clausen]

Currently register read-back for the ad193x is broken, because it expects bit 0
of the upper byte to be set to indicate a read operation, while the regmap
default for SPI is to use bit 7.

This patch also addresses another oddity of the device. There are SPI and I2C
versions of this codec. In both cases the registers are 8-bit wide and numbered
from 0x0 to 0x10, but in the SPI case there is also a so called
'global address' which is prefixed in-front of the register address. The global
address mimics I2C behaviour and includes a static device address the and the
read/write flag. This basically extends the register address to an 16-bit value
numbered from 0x800 to 0x810. These are the register numbers which are
currently used by the driver. This works, because I2C will ignore the upper
8 bits of the register, but it is still a bit confusing, as there are no such
register numbers in the I2C case.

The approach taken by this patch is to number the registers from 0x00 to 0x10
and encode the global address for SPI mode into the read and write flag masks.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>

---
This patch depends on the two previous patches, while the first one depends on
regmap/for-next and the second on asoc/for-next, so maybe to avoid having to
pull in regmap/for-next into asoc/for-next we should wait with applying this
patch until the merge window.

Scott, it would be nice if you could test these changes since I don't have a
ad1936 board here right now. But I compared the generated SPI signals with what
is in 3.1 and they look identical, so it should work.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 sound/soc/codecs/ad193x.c |   24 ++++++++++++++++++++----
 sound/soc/codecs/ad193x.h |   34 +++++++++++++++++-----------------
 2 files changed, 37 insertions(+), 21 deletions(-)

diff --git a/sound/soc/codecs/ad193x.c b/sound/soc/codecs/ad193x.c
index eedb6f5..fc03705 100644
--- a/sound/soc/codecs/ad193x.c
+++ b/sound/soc/codecs/ad193x.c
@@ -347,12 +347,28 @@ static int ad193x_probe(struct snd_soc_codec *codec)
 {
 	struct ad193x_priv *ad193x = snd_soc_codec_get_drvdata(codec);
 	struct snd_soc_dapm_context *dapm = &codec->dapm;
+	struct regmap_config config;
 	int ret;
 
-	if (ad193x->control_type == SND_SOC_I2C)
-		ret = snd_soc_codec_set_cache_io(codec, 8, 8, ad193x->control_type);
-	else
-		ret = snd_soc_codec_set_cache_io(codec, 16, 8, ad193x->control_type);
+	memset(&config, 0, sizeof(config));
+
+	config.val_bits = 8;
+
+	switch (ad193x->control_type) {
+	case SND_SOC_I2C:
+		config.reg_bits = 8;
+		break;
+	case SND_SOC_SPI:
+		config.reg_bits = 16;
+		config.read_flag_mask = 0x09;
+		config.write_flag_mask = 0x08;
+		break;
+	default:
+		BUG();
+		return -EINVAL;
+	}
+
+	ret = snd_soc_codec_regmap_init(codec, &config, ad193x->control_type);
 	if (ret < 0) {
 		dev_err(codec->dev, "failed to set cache I/O: %d\n", ret);
 		return ret;
diff --git a/sound/soc/codecs/ad193x.h b/sound/soc/codecs/ad193x.h
index cccc2e8..536e5f2 100644
--- a/sound/soc/codecs/ad193x.h
+++ b/sound/soc/codecs/ad193x.h
@@ -9,20 +9,20 @@
 #ifndef __AD193X_H__
 #define __AD193X_H__
 
-#define AD193X_PLL_CLK_CTRL0    0x800
+#define AD193X_PLL_CLK_CTRL0    0x00
 #define AD193X_PLL_POWERDOWN           0x01
 #define AD193X_PLL_INPUT_MASK   (~0x6)
 #define AD193X_PLL_INPUT_256    (0 << 1)
 #define AD193X_PLL_INPUT_384    (1 << 1)
 #define AD193X_PLL_INPUT_512    (2 << 1)
 #define AD193X_PLL_INPUT_768    (3 << 1)
-#define AD193X_PLL_CLK_CTRL1    0x801
-#define AD193X_DAC_CTRL0        0x802
+#define AD193X_PLL_CLK_CTRL1    0x01
+#define AD193X_DAC_CTRL0        0x02
 #define AD193X_DAC_POWERDOWN           0x01
 #define AD193X_DAC_SERFMT_MASK		0xC0
 #define AD193X_DAC_SERFMT_STEREO	(0 << 6)
 #define AD193X_DAC_SERFMT_TDM		(1 << 6)
-#define AD193X_DAC_CTRL1        0x803
+#define AD193X_DAC_CTRL1        0x03
 #define AD193X_DAC_2_CHANNELS   0
 #define AD193X_DAC_4_CHANNELS   1
 #define AD193X_DAC_8_CHANNELS   2
@@ -33,11 +33,11 @@
 #define AD193X_DAC_BCLK_MASTER  (1 << 5)
 #define AD193X_DAC_LEFT_HIGH    (1 << 3)
 #define AD193X_DAC_BCLK_INV     (1 << 7)
-#define AD193X_DAC_CTRL2        0x804
+#define AD193X_DAC_CTRL2        0x04
 #define AD193X_DAC_WORD_LEN_SHFT        3
 #define AD193X_DAC_WORD_LEN_MASK        0x18
 #define AD193X_DAC_MASTER_MUTE  1
-#define AD193X_DAC_CHNL_MUTE    0x805
+#define AD193X_DAC_CHNL_MUTE    0x05
 #define AD193X_DACL1_MUTE       0
 #define AD193X_DACR1_MUTE       1
 #define AD193X_DACL2_MUTE       2
@@ -46,28 +46,28 @@
 #define AD193X_DACR3_MUTE       5
 #define AD193X_DACL4_MUTE       6
 #define AD193X_DACR4_MUTE       7
-#define AD193X_DAC_L1_VOL       0x806
-#define AD193X_DAC_R1_VOL       0x807
-#define AD193X_DAC_L2_VOL       0x808
-#define AD193X_DAC_R2_VOL       0x809
-#define AD193X_DAC_L3_VOL       0x80a
-#define AD193X_DAC_R3_VOL       0x80b
-#define AD193X_DAC_L4_VOL       0x80c
-#define AD193X_DAC_R4_VOL       0x80d
-#define AD193X_ADC_CTRL0        0x80e
+#define AD193X_DAC_L1_VOL       0x06
+#define AD193X_DAC_R1_VOL       0x07
+#define AD193X_DAC_L2_VOL       0x08
+#define AD193X_DAC_R2_VOL       0x09
+#define AD193X_DAC_L3_VOL       0x0a
+#define AD193X_DAC_R3_VOL       0x0b
+#define AD193X_DAC_L4_VOL       0x0c
+#define AD193X_DAC_R4_VOL       0x0d
+#define AD193X_ADC_CTRL0        0x0e
 #define AD193X_ADC_POWERDOWN           0x01
 #define AD193X_ADC_HIGHPASS_FILTER	1
 #define AD193X_ADCL1_MUTE 		2
 #define AD193X_ADCR1_MUTE 		3
 #define AD193X_ADCL2_MUTE 		4
 #define AD193X_ADCR2_MUTE 		5
-#define AD193X_ADC_CTRL1        0x80f
+#define AD193X_ADC_CTRL1        0x0f
 #define AD193X_ADC_SERFMT_MASK		0x60
 #define AD193X_ADC_SERFMT_STEREO	(0 << 5)
 #define AD193X_ADC_SERFMT_TDM		(1 << 5)
 #define AD193X_ADC_SERFMT_AUX		(2 << 5)
 #define AD193X_ADC_WORD_LEN_MASK	0x3
-#define AD193X_ADC_CTRL2        0x810
+#define AD193X_ADC_CTRL2        0x10
 #define AD193X_ADC_2_CHANNELS   0
 #define AD193X_ADC_4_CHANNELS   1
 #define AD193X_ADC_8_CHANNELS   2
-- 
1.7.2.5

Re: [PATCH 2/3] ASoC: Allow codecs to provide their own regmap configuration

[Mark Brown]

On Mon, Sep 05, 2011 at 03:18:01PM +0200, Lars-Peter Clausen wrote:
> Currently a codec can either do the whole regmap initialization on its own or
> provide only the register and value bit size and let the core handle
> initialization. This patch allows a codec to provide a complete regmap
> configuration while still letting ASoC core handle the regmap initialization
> and setup.

I'd rather not do this, the only value is the copying of the word size
setup out of the regmap config is to allow us to use the ASoC cache code
but the idea is to push the cache down out of ASoC into the regmap code
so this function should just be a trivial indirection for the regmap
init which isn't useful.

> - * Register formats are frequently shared between many I2C and SPI
> - * devices.  In order to promote code reuse the ASoC core provides
> - * some standard implementations of CODEC read and write operations
> - * which can be set up using this function.
> - *
> - * The caller is responsible for allocating and initialising the
> - * actual cache.
> - *
> - * Note that at present this code cannot be used by CODECs with
> - * volatile registers.
> + * Legacy and convenience wrapper around snd_soc_codec_regmap_init
>   */

This is a useful but unrelated change.

Re: [PATCH 2/3] ASoC: Allow codecs to provide their own regmap configuration

[Lars-Peter Clausen]

On 09/05/2011 07:29 PM, Mark Brown wrote:
> On Mon, Sep 05, 2011 at 03:18:01PM +0200, Lars-Peter Clausen wrote:
>> Currently a codec can either do the whole regmap initialization on its own or
>> provide only the register and value bit size and let the core handle
>> initialization. This patch allows a codec to provide a complete regmap
>> configuration while still letting ASoC core handle the regmap initialization
>> and setup.
> 
> I'd rather not do this, the only value is the copying of the word size
> setup out of the regmap config is to allow us to use the ASoC cache code
> but the idea is to push the cache down out of ASoC into the regmap code
> so this function should just be a trivial indirection for the regmap
> init which isn't useful.
> 

I think it is useful, since we avoid duplicating that ugly switch statement
and we also have to wrap regmap_write and regmap_read, duplicating that in each
codec driver doesn't make sense.

Re: [PATCH 2/3] ASoC: Allow codecs to provide their own regmap configuration

[Mark Brown]

On Mon, Sep 05, 2011 at 07:43:22PM +0200, Lars-Peter Clausen wrote:

> I think it is useful, since we avoid duplicating that ugly switch statement
> and we also have to wrap regmap_write and regmap_read, duplicating that in each
> codec driver doesn't make sense.

When the CODEC drivers do the probe they should be setting the regmap up
in their individual bus probe functions so no switch statement.