[PATCH v2 0/3] ASoC: codecs: some more improvements for adau1701

[PATCH v2 0/3] ASoC: codecs: some more improvements for adau1701

[Daniel Mack]

Sorry Lars, it's been a while since the last round.

I hope I addressed all the details you mentioned.

Thanks for having another look,
Daniel

Daniel Mack (3):
  ASoC: codecs: adau1701: allow configuration of PLL mode pins
  ASoC: codecs: adau1701: switch to direct regmap API usage
  ASoC: codecs: adau1701: add support for pin muxing

 .../devicetree/bindings/sound/adi,adau1701.txt     |  18 ++
 sound/soc/codecs/adau1701.c                        | 250 ++++++++++++++++-----
 sound/soc/codecs/adau1701.h                        |   4 +
 3 files changed, 216 insertions(+), 56 deletions(-)

-- 
1.8.1.4

[PATCH 1/3] ASoC: codecs: adau1701: allow configuration of PLL mode pins

[Daniel Mack]

The ADAU1701 has 2 hardware pins to configure the PLL mode in accordance
to the MCLK-to-LRCLK ratio. These pins have to be stable before the chip
is released from reset, and a full reset cycle, including a new firmware
download is needed whenever they change.

This patch adds GPIO properties to the DT bindings of the Codec, and
implements makes the set_sysclk memorize the configured sysclk.

To avoid excessive reset cycles and firmware downloads, the default
clock divider can be specified in DT as well. Whenever a ratio change is
detected in the hw_params callback, the PLL mode lines are updates and a
full reset cycle is issued.

Signed-off-by: Daniel Mack <zonque@gmail.com>
---
 .../devicetree/bindings/sound/adi,adau1701.txt     |  14 +++
 sound/soc/codecs/adau1701.c                        | 107 +++++++++++++++++----
 sound/soc/codecs/adau1701.h                        |   4 +
 3 files changed, 104 insertions(+), 21 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/adi,adau1701.txt b/Documentation/devicetree/bindings/sound/adi,adau1701.txt
index 3afeda7..a0d7e92 100644
--- a/Documentation/devicetree/bindings/sound/adi,adau1701.txt
+++ b/Documentation/devicetree/bindings/sound/adi,adau1701.txt
@@ -11,6 +11,19 @@ Optional properties:
  - reset-gpio: 		A GPIO spec to define which pin is connected to the
 			chip's !RESET pin. If specified, the driver will
 			assert a hardware reset at probe time.
+ - adi,pll-clkdiv: 	The PLL clock divider, specifing the ratio between
+			MCLK and fsclk. The value is used to determine the
+			correct state of the two mode pins below.
+			Note that this value can be overridden at runtime
+			by passing the ADAU1701_CLKDIV_MCLK_LRCLK divider
+			with ASoC calls. However, the chips needs a full
+			reset cycle and a new firmware download each time
+			the configuration changes.
+ - adi,pll-mode-gpios:	An array of two GPIO specs to describe the GPIOs
+			the ADAU's PLL config pins are connected to.
+			The state of the pins are set according to the
+			configured clock divider on ASoC side before the
+			firmware is loaded.
 
 Examples:
 
@@ -19,5 +32,6 @@ Examples:
 			compatible = "adi,adau1701";
 			reg = <0x34>;
 			reset-gpio = <&gpio 23 0>;
+			adi,pll-mode-gpios = <&gpio 24 0 &gpio 25 0>;
 		};
 	};
diff --git a/sound/soc/codecs/adau1701.c b/sound/soc/codecs/adau1701.c
index b6b1a77..e6ce4fe 100644
--- a/sound/soc/codecs/adau1701.c
+++ b/sound/soc/codecs/adau1701.c
@@ -91,7 +91,11 @@
 
 struct adau1701 {
 	int gpio_nreset;
+	int gpio_pll_mode0;
+	int gpio_pll_mode1;
 	unsigned int dai_fmt;
+	unsigned int pll_clkdiv;
+	unsigned int sysclk;
 };
 
 static const struct snd_kcontrol_new adau1701_controls[] = {
@@ -184,13 +188,37 @@ static unsigned int adau1701_read(struct snd_soc_codec *codec, unsigned int reg)
 	return value;
 }
 
-static void adau1701_reset(struct snd_soc_codec *codec)
+static void adau1701_reset(struct snd_soc_codec *codec, unsigned int clkdiv)
 {
 	struct adau1701 *adau1701 = snd_soc_codec_get_drvdata(codec);
 
 	if (!gpio_is_valid(adau1701->gpio_nreset))
 		return;
 
+	if (gpio_is_valid(adau1701->gpio_pll_mode0) &&
+	    gpio_is_valid(adau1701->gpio_pll_mode1)) {
+		switch (adau1701->pll_clkdiv) {
+		case 64:
+			gpio_set_value(adau1701->gpio_pll_mode0, 0);
+			gpio_set_value(adau1701->gpio_pll_mode1, 0);
+			break;
+		case 256:
+			gpio_set_value(adau1701->gpio_pll_mode0, 0);
+			gpio_set_value(adau1701->gpio_pll_mode1, 1);
+			break;
+		case 384:
+			gpio_set_value(adau1701->gpio_pll_mode0, 1);
+			gpio_set_value(adau1701->gpio_pll_mode1, 0);
+			break;
+		case 512:
+			gpio_set_value(adau1701->gpio_pll_mode0, 1);
+			gpio_set_value(adau1701->gpio_pll_mode1, 1);
+			break;
+		}
+	}
+
+	adau1701->pll_clkdiv = clkdiv;
+
 	gpio_set_value(adau1701->gpio_nreset, 0);
 	/* minimum reset time is 20ns */
 	udelay(1);
@@ -199,24 +227,6 @@ static void adau1701_reset(struct snd_soc_codec *codec)
 	mdelay(85);
 }
 
-static int adau1701_init(struct snd_soc_codec *codec)
-{
-	int ret;
-	struct i2c_client *client = to_i2c_client(codec->dev);
-
-	adau1701_reset(codec);
-
-	ret = process_sigma_firmware(client, ADAU1701_FIRMWARE);
-	if (ret) {
-		dev_warn(codec->dev, "Failed to load firmware\n");
-		return ret;
-	}
-
-	snd_soc_write(codec, ADAU1701_DACSET, ADAU1701_DACSET_DACINIT);
-
-	return 0;
-}
-
 static int adau1701_set_capture_pcm_format(struct snd_soc_codec *codec,
 		snd_pcm_format_t format)
 {
@@ -291,9 +301,22 @@ static int adau1701_hw_params(struct snd_pcm_substream *substream,
 		struct snd_pcm_hw_params *params, struct snd_soc_dai *dai)
 {
 	struct snd_soc_codec *codec = dai->codec;
+	struct adau1701 *adau1701 = snd_soc_codec_get_drvdata(codec);
 	snd_pcm_format_t format;
 	unsigned int val;
 
+	if (adau1701->sysclk) {
+		unsigned int clkdiv = adau1701->sysclk / params_rate(params);
+
+		/*
+		 * If the mclk/lrclk ratio changes, the chip needs updated PLL
+		 * mode GPIO settings, and a full reset cycle, including a new
+		 * firmware upload.
+		 */
+		if (clkdiv != adau1701->pll_clkdiv)
+			adau1701_reset(codec, clkdiv);
+	}
+
 	switch (params_rate(params)) {
 	case 192000:
 		val = ADAU1701_DSPCTRL_SR_192;
@@ -435,6 +458,7 @@ static int adau1701_set_sysclk(struct snd_soc_codec *codec, int clk_id,
 	int source, unsigned int freq, int dir)
 {
 	unsigned int val;
+	struct adau1701 *adau1701 = snd_soc_codec_get_drvdata(codec);
 
 	switch (clk_id) {
 	case ADAU1701_CLK_SRC_OSC:
@@ -448,6 +472,7 @@ static int adau1701_set_sysclk(struct snd_soc_codec *codec, int clk_id,
 	}
 
 	snd_soc_update_bits(codec, ADAU1701_OSCIPOW, ADAU1701_OSCIPOW_OPD, val);
+	adau1701->sysclk = freq;
 
 	return 0;
 }
@@ -495,13 +520,21 @@ MODULE_DEVICE_TABLE(of, adau1701_dt_ids);
 static int adau1701_probe(struct snd_soc_codec *codec)
 {
 	int ret;
+	struct i2c_client *client = to_i2c_client(codec->dev);
+	struct adau1701 *adau1701 = snd_soc_codec_get_drvdata(codec);
 
 	codec->control_data = to_i2c_client(codec->dev);
 
-	ret = adau1701_init(codec);
-	if (ret)
+	/* initalize with pre-configured pll mode settings */
+	adau1701_reset(codec, adau1701->pll_clkdiv);
+
+	ret = process_sigma_firmware(client, ADAU1701_FIRMWARE);
+	if (ret) {
+		dev_warn(codec->dev, "Failed to load firmware\n");
 		return ret;
+	}
 
+	snd_soc_write(codec, ADAU1701_DACSET, ADAU1701_DACSET_DACINIT);
 	snd_soc_write(codec, ADAU1701_DSPCTRL, ADAU1701_DSPCTRL_CR);
 
 	return 0;
@@ -534,6 +567,8 @@ static int adau1701_i2c_probe(struct i2c_client *client,
 	struct adau1701 *adau1701;
 	struct device *dev = &client->dev;
 	int gpio_nreset = -EINVAL;
+	int gpio_pll_mode0 = -EINVAL;
+	int gpio_pll_mode1 = -EINVAL;
 	int ret;
 
 	adau1701 = devm_kzalloc(dev, sizeof(*adau1701), GFP_KERNEL);
@@ -544,6 +579,19 @@ static int adau1701_i2c_probe(struct i2c_client *client,
 		gpio_nreset = of_get_named_gpio(dev->of_node, "reset-gpio", 0);
 		if (gpio_nreset < 0 && gpio_nreset != -ENOENT)
 			return gpio_nreset;
+
+		gpio_pll_mode0 = of_get_named_gpio(dev->of_node,
+						   "adi,pll-mode-gpios", 0);
+		if (gpio_pll_mode0 < 0 && gpio_pll_mode0 != -ENOENT)
+			return gpio_pll_mode0;
+
+		gpio_pll_mode1 = of_get_named_gpio(dev->of_node,
+						   "adi,pll-mode-gpios", 1);
+		if (gpio_pll_mode1 < 0 && gpio_pll_mode1 != -ENOENT)
+			return gpio_pll_mode1;
+
+		of_property_read_u32(dev->of_node, "adi,pll-clkdiv",
+				     &adau1701->pll_clkdiv);
 	}
 
 	if (gpio_is_valid(gpio_nreset)) {
@@ -553,7 +601,24 @@ static int adau1701_i2c_probe(struct i2c_client *client,
 			return ret;
 	}
 
+	if (gpio_is_valid(gpio_pll_mode0) &&
+	    gpio_is_valid(gpio_pll_mode1)) {
+		ret = devm_gpio_request_one(dev, gpio_pll_mode0,
+					    GPIOF_OUT_INIT_LOW,
+					    "ADAU1701 PLL mode 0");
+		if (ret < 0)
+			return ret;
+
+		ret = devm_gpio_request_one(dev, gpio_pll_mode1,
+					    GPIOF_OUT_INIT_LOW,
+					    "ADAU1701 PLL mode 1");
+		if (ret < 0)
+			return ret;
+	}
+
 	adau1701->gpio_nreset = gpio_nreset;
+	adau1701->gpio_pll_mode0 = gpio_pll_mode0;
+	adau1701->gpio_pll_mode1 = gpio_pll_mode1;
 
 	i2c_set_clientdata(client, adau1701);
 	ret = snd_soc_register_codec(&client->dev, &adau1701_codec_drv,
diff --git a/sound/soc/codecs/adau1701.h b/sound/soc/codecs/adau1701.h
index 8d0949a..bdcdddc 100644
--- a/sound/soc/codecs/adau1701.h
+++ b/sound/soc/codecs/adau1701.h
@@ -14,4 +14,8 @@ enum adau1701_clk_src {
 	ADAU1701_CLK_SRC_MCLK,
 };
 
+enum adau1701_clkdiv {
+	ADAU1701_CLKDIV_MCLK_LRCLK,
+};
+
 #endif
-- 
1.8.1.4

[PATCH 2/3] ASoC: codecs: adau1701: switch to direct regmap API usage

[Daniel Mack]

The hardware I/O has to be open-coded due to registers of unequal sizes.
Other than that, the transition is straight forward.

Signed-off-by: Daniel Mack <zonque@gmail.com>
---
 sound/soc/codecs/adau1701.c | 118 +++++++++++++++++++++++++++++++-------------
 1 file changed, 83 insertions(+), 35 deletions(-)

diff --git a/sound/soc/codecs/adau1701.c b/sound/soc/codecs/adau1701.c
index e6ce4fe..5b83b64 100644
--- a/sound/soc/codecs/adau1701.c
+++ b/sound/soc/codecs/adau1701.c
@@ -16,6 +16,7 @@
 #include <linux/of.h>
 #include <linux/of_gpio.h>
 #include <linux/of_device.h>
+#include <linux/regmap.h>
 #include <sound/core.h>
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
@@ -24,16 +25,16 @@
 #include "sigmadsp.h"
 #include "adau1701.h"
 
-#define ADAU1701_DSPCTRL	0x1c
-#define ADAU1701_SEROCTL	0x1e
-#define ADAU1701_SERICTL	0x1f
+#define ADAU1701_DSPCTRL	0x081c
+#define ADAU1701_SEROCTL	0x081e
+#define ADAU1701_SERICTL	0x081f
 
-#define ADAU1701_AUXNPOW	0x22
+#define ADAU1701_AUXNPOW	0x0822
 
-#define ADAU1701_OSCIPOW	0x26
-#define ADAU1701_DACSET		0x27
+#define ADAU1701_OSCIPOW	0x0826
+#define ADAU1701_DACSET		0x0827
 
-#define ADAU1701_NUM_REGS	0x28
+#define ADAU1701_MAX_REGISTER	0x0828
 
 #define ADAU1701_DSPCTRL_CR		(1 << 2)
 #define ADAU1701_DSPCTRL_DAM		(1 << 3)
@@ -96,6 +97,7 @@ struct adau1701 {
 	unsigned int dai_fmt;
 	unsigned int pll_clkdiv;
 	unsigned int sysclk;
+	struct regmap *regmap;
 };
 
 static const struct snd_kcontrol_new adau1701_controls[] = {
@@ -127,7 +129,7 @@ static const struct snd_soc_dapm_route adau1701_dapm_routes[] = {
 	{ "ADC", NULL, "IN1" },
 };
 
-static unsigned int adau1701_register_size(struct snd_soc_codec *codec,
+static unsigned int adau1701_register_size(struct device *dev,
 		unsigned int reg)
 {
 	switch (reg) {
@@ -141,33 +143,42 @@ static unsigned int adau1701_register_size(struct snd_soc_codec *codec,
 		return 1;
 	}
 
-	dev_err(codec->dev, "Unsupported register address: %d\n", reg);
+	dev_err(dev, "Unsupported register address: %d\n", reg);
 	return 0;
 }
 
-static int adau1701_write(struct snd_soc_codec *codec, unsigned int reg,
-		unsigned int value)
+static bool adau1701_volatile_reg(struct device *dev, unsigned int reg)
 {
+	switch (reg) {
+	case ADAU1701_DACSET:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static int adau1701_reg_write(void *context, unsigned int reg,
+			      unsigned int value)
+{
+	struct i2c_client *client = context;
 	unsigned int i;
 	unsigned int size;
 	uint8_t buf[4];
 	int ret;
 
-	size = adau1701_register_size(codec, reg);
+	size = adau1701_register_size(&client->dev, reg);
 	if (size == 0)
 		return -EINVAL;
 
-	snd_soc_cache_write(codec, reg, value);
-
-	buf[0] = 0x08;
-	buf[1] = reg;
+	buf[0] = reg >> 8;
+	buf[1] = reg & 0xff;
 
 	for (i = size + 1; i >= 2; --i) {
 		buf[i] = value;
 		value >>= 8;
 	}
 
-	ret = i2c_master_send(to_i2c_client(codec->dev), buf, size + 2);
+	ret = i2c_master_send(client, buf, size + 2);
 	if (ret == size + 2)
 		return 0;
 	else if (ret < 0)
@@ -176,16 +187,45 @@ static int adau1701_write(struct snd_soc_codec *codec, unsigned int reg,
 		return -EIO;
 }
 
-static unsigned int adau1701_read(struct snd_soc_codec *codec, unsigned int reg)
+static int adau1701_reg_read(void *context, unsigned int reg,
+			     unsigned int *value)
 {
-	unsigned int value;
-	unsigned int ret;
+	int ret;
+	unsigned int i;
+	unsigned int size;
+	uint8_t send_buf[2], recv_buf[3];
+	struct i2c_client *client = context;
+	struct i2c_msg msgs[2];
+
+	size = adau1701_register_size(&client->dev, reg);
+	if (size == 0)
+		return -EINVAL;
 
-	ret = snd_soc_cache_read(codec, reg, &value);
-	if (ret)
+	send_buf[0] = reg >> 8;
+	send_buf[1] = reg & 0xff;
+
+	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 | I2C_M_STOP;
+
+	ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
+	if (ret < 0)
 		return ret;
+	else if (ret != ARRAY_SIZE(msgs))
+		return -EIO;
 
-	return value;
+	*value = 0;
+
+	for (i = 0; i < size; i++)
+		*value |= recv_buf[i] << (i * 8);
+
+	return 0;
 }
 
 static void adau1701_reset(struct snd_soc_codec *codec, unsigned int clkdiv)
@@ -407,8 +447,8 @@ static int adau1701_set_dai_fmt(struct snd_soc_dai *codec_dai,
 
 	adau1701->dai_fmt = fmt & SND_SOC_DAIFMT_FORMAT_MASK;
 
-	snd_soc_write(codec, ADAU1701_SERICTL, serictl);
-	snd_soc_update_bits(codec, ADAU1701_SEROCTL,
+	regmap_write(adau1701->regmap, ADAU1701_SERICTL, serictl);
+	regmap_update_bits(adau1701->regmap, ADAU1701_SEROCTL,
 		~ADAU1701_SEROCTL_WORD_LEN_MASK, seroctl);
 
 	return 0;
@@ -523,8 +563,6 @@ static int adau1701_probe(struct snd_soc_codec *codec)
 	struct i2c_client *client = to_i2c_client(codec->dev);
 	struct adau1701 *adau1701 = snd_soc_codec_get_drvdata(codec);
 
-	codec->control_data = to_i2c_client(codec->dev);
-
 	/* initalize with pre-configured pll mode settings */
 	adau1701_reset(codec, adau1701->pll_clkdiv);
 
@@ -534,8 +572,9 @@ static int adau1701_probe(struct snd_soc_codec *codec)
 		return ret;
 	}
 
-	snd_soc_write(codec, ADAU1701_DACSET, ADAU1701_DACSET_DACINIT);
-	snd_soc_write(codec, ADAU1701_DSPCTRL, ADAU1701_DSPCTRL_CR);
+	regcache_sync(adau1701->regmap);
+	regmap_write(adau1701->regmap, ADAU1701_DACSET, ADAU1701_DACSET_DACINIT);
+	regmap_write(adau1701->regmap, ADAU1701_DSPCTRL, ADAU1701_DSPCTRL_CR);
 
 	return 0;
 }
@@ -545,9 +584,6 @@ static struct snd_soc_codec_driver adau1701_codec_drv = {
 	.set_bias_level		= adau1701_set_bias_level,
 	.idle_bias_off		= true,
 
-	.reg_cache_size		= ADAU1701_NUM_REGS,
-	.reg_word_size		= sizeof(u16),
-
 	.controls		= adau1701_controls,
 	.num_controls		= ARRAY_SIZE(adau1701_controls),
 	.dapm_widgets		= adau1701_dapm_widgets,
@@ -555,12 +591,19 @@ static struct snd_soc_codec_driver adau1701_codec_drv = {
 	.dapm_routes		= adau1701_dapm_routes,
 	.num_dapm_routes	= ARRAY_SIZE(adau1701_dapm_routes),
 
-	.write			= adau1701_write,
-	.read			= adau1701_read,
-
 	.set_sysclk		= adau1701_set_sysclk,
 };
 
+static const struct regmap_config adau1701_regmap = {
+	.reg_bits		= 16,
+	.val_bits		= 32,
+	.max_register		= ADAU1701_MAX_REGISTER,
+	.cache_type		= REGCACHE_RBTREE,
+	.volatile_reg		= adau1701_volatile_reg,
+	.reg_write		= adau1701_reg_write,
+	.reg_read		= adau1701_reg_read,
+};
+
 static int adau1701_i2c_probe(struct i2c_client *client,
 			      const struct i2c_device_id *id)
 {
@@ -575,6 +618,11 @@ static int adau1701_i2c_probe(struct i2c_client *client,
 	if (!adau1701)
 		return -ENOMEM;
 
+	adau1701->regmap = devm_regmap_init(dev, NULL, client,
+					    &adau1701_regmap);
+	if (IS_ERR(adau1701->regmap))
+		return PTR_ERR(adau1701->regmap);
+
 	if (dev->of_node) {
 		gpio_nreset = of_get_named_gpio(dev->of_node, "reset-gpio", 0);
 		if (gpio_nreset < 0 && gpio_nreset != -ENOENT)
-- 
1.8.1.4

[PATCH 3/3] ASoC: codecs: adau1701: add support for pin muxing

[Daniel Mack]

The ADAU1701 has 12 pins that can be configured depending on the system
configuration. Allow settting the corresponding registers from DT.

Signed-off-by: Daniel Mack <zonque@gmail.com>
---
 .../devicetree/bindings/sound/adi,adau1701.txt     |  6 +++++
 sound/soc/codecs/adau1701.c                        | 29 ++++++++++++++++++++--
 2 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/adi,adau1701.txt b/Documentation/devicetree/bindings/sound/adi,adau1701.txt
index a0d7e92..7391698 100644
--- a/Documentation/devicetree/bindings/sound/adi,adau1701.txt
+++ b/Documentation/devicetree/bindings/sound/adi,adau1701.txt
@@ -24,6 +24,10 @@ Optional properties:
 			The state of the pins are set according to the
 			configured clock divider on ASoC side before the
 			firmware is loaded.
+ - adi,pin-config:	An array of 12 numerical values selecting one of the
+			pin configurations as described in the datasheet,
+			table 53. Note that the value of this property has
+			to be prefixed with '/bits/ 8'.
 
 Examples:
 
@@ -33,5 +37,7 @@ Examples:
 			reg = <0x34>;
 			reset-gpio = <&gpio 23 0>;
 			adi,pll-mode-gpios = <&gpio 24 0 &gpio 25 0>;
+			adi,pin-config = /bits/ 8 <0x4 0x7 0x5 0x5 0x4 0x4
+                                                           0x4 0x4 0x4 0x4 0x4 0x4>;
 		};
 	};
diff --git a/sound/soc/codecs/adau1701.c b/sound/soc/codecs/adau1701.c
index 5b83b64..b938e57 100644
--- a/sound/soc/codecs/adau1701.c
+++ b/sound/soc/codecs/adau1701.c
@@ -30,6 +30,9 @@
 #define ADAU1701_SERICTL	0x081f
 
 #define ADAU1701_AUXNPOW	0x0822
+#define ADAU1701_PINCONF_0	0x0820
+#define ADAU1701_PINCONF_1	0x0821
+#define ADAU1701_AUXNPOW	0x0822
 
 #define ADAU1701_OSCIPOW	0x0826
 #define ADAU1701_DACSET		0x0827
@@ -98,6 +101,7 @@ struct adau1701 {
 	unsigned int pll_clkdiv;
 	unsigned int sysclk;
 	struct regmap *regmap;
+	u8 pin_config[12];
 };
 
 static const struct snd_kcontrol_new adau1701_controls[] = {
@@ -133,6 +137,9 @@ static unsigned int adau1701_register_size(struct device *dev,
 		unsigned int reg)
 {
 	switch (reg) {
+	case ADAU1701_PINCONF_0:
+	case ADAU1701_PINCONF_1:
+		return 3;
 	case ADAU1701_DSPCTRL:
 	case ADAU1701_SEROCTL:
 	case ADAU1701_AUXNPOW:
@@ -163,7 +170,7 @@ static int adau1701_reg_write(void *context, unsigned int reg,
 	struct i2c_client *client = context;
 	unsigned int i;
 	unsigned int size;
-	uint8_t buf[4];
+	uint8_t buf[5];
 	int ret;
 
 	size = adau1701_register_size(&client->dev, reg);
@@ -559,7 +566,8 @@ MODULE_DEVICE_TABLE(of, adau1701_dt_ids);
 
 static int adau1701_probe(struct snd_soc_codec *codec)
 {
-	int ret;
+	int ret, i;
+	unsigned int val;
 	struct i2c_client *client = to_i2c_client(codec->dev);
 	struct adau1701 *adau1701 = snd_soc_codec_get_drvdata(codec);
 
@@ -576,6 +584,19 @@ static int adau1701_probe(struct snd_soc_codec *codec)
 	regmap_write(adau1701->regmap, ADAU1701_DACSET, ADAU1701_DACSET_DACINIT);
 	regmap_write(adau1701->regmap, ADAU1701_DSPCTRL, ADAU1701_DSPCTRL_CR);
 
+	/* set up pin config */
+	val = 0;
+	for (i = 0; i < 6; i++)
+		val |= adau1701->pin_config[i] << (i * 4);
+
+	regmap_write(adau1701->regmap, ADAU1701_PINCONF_0, val);
+
+	val = 0;
+	for (i = 0; i < 6; i++)
+		val |= adau1701->pin_config[i + 6] << (i * 4);
+
+	regmap_write(adau1701->regmap, ADAU1701_PINCONF_1, val);
+
 	return 0;
 }
 
@@ -640,6 +661,10 @@ static int adau1701_i2c_probe(struct i2c_client *client,
 
 		of_property_read_u32(dev->of_node, "adi,pll-clkdiv",
 				     &adau1701->pll_clkdiv);
+
+		of_property_read_u8_array(dev->of_node, "adi,pin-config",
+					  adau1701->pin_config,
+					  ARRAY_SIZE(adau1701->pin_config));
 	}
 
 	if (gpio_is_valid(gpio_nreset)) {
-- 
1.8.1.4

Re: [PATCH 1/3] ASoC: codecs: adau1701: allow configuration of PLL mode pins

[Rajeev kumar]

Daniel,

On 6/20/2013 10:59 PM, Daniel Mack wrote:
> The ADAU1701 has 2 hardware pins to configure the PLL mode in accordance
> to the MCLK-to-LRCLK ratio. These pins have to be stable before the chip
> is released from reset, and a full reset cycle, including a new firmware
> download is needed whenever they change.
>
> This patch adds GPIO properties to the DT bindings of the Codec, and
> implements makes the set_sysclk memorize the configured sysclk.
>
> To avoid excessive reset cycles and firmware downloads, the default
> clock divider can be specified in DT as well. Whenever a ratio change is
> detected in the hw_params callback, the PLL mode lines are updates and a
> full reset cycle is issued.
>
> Signed-off-by: Daniel Mack<zonque@gmail.com>
> ---
>   .../devicetree/bindings/sound/adi,adau1701.txt     |  14 +++
>   sound/soc/codecs/adau1701.c                        | 107 +++++++++++++++++----
>   sound/soc/codecs/adau1701.h                        |   4 +
>   3 files changed, 104 insertions(+), 21 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/sound/adi,adau1701.txt b/Documentation/devicetree/bindings/sound/adi,adau1701.txt
> index 3afeda7..a0d7e92 100644
> --- a/Documentation/devicetree/bindings/sound/adi,adau1701.txt
> +++ b/Documentation/devicetree/bindings/sound/adi,adau1701.txt
> @@ -11,6 +11,19 @@ Optional properties:
>    - reset-gpio: 		A GPIO spec to define which pin is connected to the
>   			chip's !RESET pin. If specified, the driver will
>   			assert a hardware reset at probe time.
> + - adi,pll-clkdiv: 	The PLL clock divider, specifing the ratio between
> +			MCLK and fsclk. The value is used to determine the
> +			correct state of the two mode pins below.
> +			Note that this value can be overridden at runtime
> +			by passing the ADAU1701_CLKDIV_MCLK_LRCLK divider
> +			with ASoC calls. However, the chips needs a full
> +			reset cycle and a new firmware download each time
> +			the configuration changes.
> + - adi,pll-mode-gpios:	An array of two GPIO specs to describe the GPIOs
> +			the ADAU's PLL config pins are connected to.
> +			The state of the pins are set according to the
> +			configured clock divider on ASoC side before the
> +			firmware is loaded.
>
>   Examples:
>
> @@ -19,5 +32,6 @@ Examples:
>   			compatible = "adi,adau1701";
>   			reg =<0x34>;
>   			reset-gpio =<&gpio 23 0>;
> +			adi,pll-mode-gpios =<&gpio 24 0&gpio 25 0>;
>   		};
>   	};
> diff --git a/sound/soc/codecs/adau1701.c b/sound/soc/codecs/adau1701.c
> index b6b1a77..e6ce4fe 100644
> --- a/sound/soc/codecs/adau1701.c
> +++ b/sound/soc/codecs/adau1701.c
> @@ -91,7 +91,11 @@
>
>   struct adau1701 {
>   	int gpio_nreset;
> +	int gpio_pll_mode0;
> +	int gpio_pll_mode1;

combine in single line.

>   	unsigned int dai_fmt;
> +	unsigned int pll_clkdiv;
> +	unsigned int sysclk;
>   };
>
>   static const struct snd_kcontrol_new adau1701_controls[] = {
> @@ -184,13 +188,37 @@ static unsigned int adau1701_read(struct snd_soc_codec *codec, unsigned int reg)
>   	return value;
>   }
>
> -static void adau1701_reset(struct snd_soc_codec *codec)
> +static void adau1701_reset(struct snd_soc_codec *codec, unsigned int clkdiv)
>   {
>   	struct adau1701 *adau1701 = snd_soc_codec_get_drvdata(codec);
>
>   	if (!gpio_is_valid(adau1701->gpio_nreset))
>   		return;
>
> +	if (gpio_is_valid(adau1701->gpio_pll_mode0)&&
> +	    gpio_is_valid(adau1701->gpio_pll_mode1)) {
> +		switch (adau1701->pll_clkdiv) {
> +		case 64:

magic number?

> +			gpio_set_value(adau1701->gpio_pll_mode0, 0);
> +			gpio_set_value(adau1701->gpio_pll_mode1, 0);
> +			break;
> +		case 256:
> +			gpio_set_value(adau1701->gpio_pll_mode0, 0);
> +			gpio_set_value(adau1701->gpio_pll_mode1, 1);
> +			break;
> +		case 384:
> +			gpio_set_value(adau1701->gpio_pll_mode0, 1);
> +			gpio_set_value(adau1701->gpio_pll_mode1, 0);
> +			break;
> +		case 512:
> +			gpio_set_value(adau1701->gpio_pll_mode0, 1);
> +			gpio_set_value(adau1701->gpio_pll_mode1, 1);
> +			break;
> +		}
> +	}
> +
> +	adau1701->pll_clkdiv = clkdiv;
> +
>   	gpio_set_value(adau1701->gpio_nreset, 0);
>   	/* minimum reset time is 20ns */
>   	udelay(1);
> @@ -199,24 +227,6 @@ static void adau1701_reset(struct snd_soc_codec *codec)
>   	mdelay(85);
>   }
>
> -static int adau1701_init(struct snd_soc_codec *codec)
> -{
> -	int ret;
> -	struct i2c_client *client = to_i2c_client(codec->dev);
> -
> -	adau1701_reset(codec);
> -
> -	ret = process_sigma_firmware(client, ADAU1701_FIRMWARE);
> -	if (ret) {
> -		dev_warn(codec->dev, "Failed to load firmware\n");
> -		return ret;
> -	}
> -
> -	snd_soc_write(codec, ADAU1701_DACSET, ADAU1701_DACSET_DACINIT);
> -
> -	return 0;
> -}
> -
>   static int adau1701_set_capture_pcm_format(struct snd_soc_codec *codec,
>   		snd_pcm_format_t format)
>   {
> @@ -291,9 +301,22 @@ static int adau1701_hw_params(struct snd_pcm_substream *substream,
>   		struct snd_pcm_hw_params *params, struct snd_soc_dai *dai)
>   {
>   	struct snd_soc_codec *codec = dai->codec;
> +	struct adau1701 *adau1701 = snd_soc_codec_get_drvdata(codec);
>   	snd_pcm_format_t format;
>   	unsigned int val;
>
> +	if (adau1701->sysclk) {
> +		unsigned int clkdiv = adau1701->sysclk / params_rate(params);

It will give warning.

Best Regards
Rajeev

> +
> +		/*
> +		 * If the mclk/lrclk ratio changes, the chip needs updated PLL
> +		 * mode GPIO settings, and a full reset cycle, including a new
> +		 * firmware upload.
> +		 */
> +		if (clkdiv != adau1701->pll_clkdiv)
> +			adau1701_reset(codec, clkdiv);
> +	}
> +
>   	switch (params_rate(params)) {
>   	case 192000:
>   		val = ADAU1701_DSPCTRL_SR_192;
> @@ -435,6 +458,7 @@ static int adau1701_set_sysclk(struct snd_soc_codec *codec, int clk_id,
>   	int source, unsigned int freq, int dir)
>   {
>   	unsigned int val;
> +	struct adau1701 *adau1701 = snd_soc_codec_get_drvdata(codec);
>
>   	switch (clk_id) {
>   	case ADAU1701_CLK_SRC_OSC:
> @@ -448,6 +472,7 @@ static int adau1701_set_sysclk(struct snd_soc_codec *codec, int clk_id,
>   	}
>
>   	snd_soc_update_bits(codec, ADAU1701_OSCIPOW, ADAU1701_OSCIPOW_OPD, val);
> +	adau1701->sysclk = freq;
>
>   	return 0;
>   }
> @@ -495,13 +520,21 @@ MODULE_DEVICE_TABLE(of, adau1701_dt_ids);
>   static int adau1701_probe(struct snd_soc_codec *codec)
>   {
>   	int ret;
> +	struct i2c_client *client = to_i2c_client(codec->dev);
> +	struct adau1701 *adau1701 = snd_soc_codec_get_drvdata(codec);
>
>   	codec->control_data = to_i2c_client(codec->dev);
>
> -	ret = adau1701_init(codec);
> -	if (ret)
> +	/* initalize with pre-configured pll mode settings */
> +	adau1701_reset(codec, adau1701->pll_clkdiv);
> +
> +	ret = process_sigma_firmware(client, ADAU1701_FIRMWARE);
> +	if (ret) {
> +		dev_warn(codec->dev, "Failed to load firmware\n");
>   		return ret;
> +	}
>
> +	snd_soc_write(codec, ADAU1701_DACSET, ADAU1701_DACSET_DACINIT);
>   	snd_soc_write(codec, ADAU1701_DSPCTRL, ADAU1701_DSPCTRL_CR);
>
>   	return 0;
> @@ -534,6 +567,8 @@ static int adau1701_i2c_probe(struct i2c_client *client,
>   	struct adau1701 *adau1701;
>   	struct device *dev =&client->dev;
>   	int gpio_nreset = -EINVAL;
> +	int gpio_pll_mode0 = -EINVAL;
> +	int gpio_pll_mode1 = -EINVAL;
>   	int ret;
>
>   	adau1701 = devm_kzalloc(dev, sizeof(*adau1701), GFP_KERNEL);
> @@ -544,6 +579,19 @@ static int adau1701_i2c_probe(struct i2c_client *client,
>   		gpio_nreset = of_get_named_gpio(dev->of_node, "reset-gpio", 0);
>   		if (gpio_nreset<  0&&  gpio_nreset != -ENOENT)
>   			return gpio_nreset;
> +
> +		gpio_pll_mode0 = of_get_named_gpio(dev->of_node,
> +						   "adi,pll-mode-gpios", 0);
> +		if (gpio_pll_mode0<  0&&  gpio_pll_mode0 != -ENOENT)
> +			return gpio_pll_mode0;
> +
> +		gpio_pll_mode1 = of_get_named_gpio(dev->of_node,
> +						   "adi,pll-mode-gpios", 1);
> +		if (gpio_pll_mode1<  0&&  gpio_pll_mode1 != -ENOENT)
> +			return gpio_pll_mode1;
> +
> +		of_property_read_u32(dev->of_node, "adi,pll-clkdiv",
> +				&adau1701->pll_clkdiv);
>   	}
>
>   	if (gpio_is_valid(gpio_nreset)) {
> @@ -553,7 +601,24 @@ static int adau1701_i2c_probe(struct i2c_client *client,
>   			return ret;
>   	}
>
> +	if (gpio_is_valid(gpio_pll_mode0)&&
> +	    gpio_is_valid(gpio_pll_mode1)) {
> +		ret = devm_gpio_request_one(dev, gpio_pll_mode0,
> +					    GPIOF_OUT_INIT_LOW,
> +					    "ADAU1701 PLL mode 0");
> +		if (ret<  0)
> +			return ret;
> +
> +		ret = devm_gpio_request_one(dev, gpio_pll_mode1,
> +					    GPIOF_OUT_INIT_LOW,
> +					    "ADAU1701 PLL mode 1");
> +		if (ret<  0)
> +			return ret;
> +	}
> +
>   	adau1701->gpio_nreset = gpio_nreset;
> +	adau1701->gpio_pll_mode0 = gpio_pll_mode0;
> +	adau1701->gpio_pll_mode1 = gpio_pll_mode1;
>
>   	i2c_set_clientdata(client, adau1701);
>   	ret = snd_soc_register_codec(&client->dev,&adau1701_codec_drv,
> diff --git a/sound/soc/codecs/adau1701.h b/sound/soc/codecs/adau1701.h
> index 8d0949a..bdcdddc 100644
> --- a/sound/soc/codecs/adau1701.h
> +++ b/sound/soc/codecs/adau1701.h
> @@ -14,4 +14,8 @@ enum adau1701_clk_src {
>   	ADAU1701_CLK_SRC_MCLK,
>   };
>
> +enum adau1701_clkdiv {
> +	ADAU1701_CLKDIV_MCLK_LRCLK,
> +};
> +
>   #endif

Re: [PATCH 1/3] ASoC: codecs: adau1701: allow configuration of PLL mode pins

[Daniel Mack]

On 21.06.2013 06:46, Rajeev kumar wrote:
> Daniel,
> 
> On 6/20/2013 10:59 PM, Daniel Mack wrote:
>> The ADAU1701 has 2 hardware pins to configure the PLL mode in accordance
>> to the MCLK-to-LRCLK ratio. These pins have to be stable before the chip
>> is released from reset, and a full reset cycle, including a new firmware
>> download is needed whenever they change.
>>
>> This patch adds GPIO properties to the DT bindings of the Codec, and
>> implements makes the set_sysclk memorize the configured sysclk.
>>
>> To avoid excessive reset cycles and firmware downloads, the default
>> clock divider can be specified in DT as well. Whenever a ratio change is
>> detected in the hw_params callback, the PLL mode lines are updates and a
>> full reset cycle is issued.
>>
>> Signed-off-by: Daniel Mack<zonque@gmail.com>
>> ---
>>   .../devicetree/bindings/sound/adi,adau1701.txt     |  14 +++
>>   sound/soc/codecs/adau1701.c                        | 107 +++++++++++++++++----
>>   sound/soc/codecs/adau1701.h                        |   4 +
>>   3 files changed, 104 insertions(+), 21 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/sound/adi,adau1701.txt b/Documentation/devicetree/bindings/sound/adi,adau1701.txt
>> index 3afeda7..a0d7e92 100644
>> --- a/Documentation/devicetree/bindings/sound/adi,adau1701.txt
>> +++ b/Documentation/devicetree/bindings/sound/adi,adau1701.txt
>> @@ -11,6 +11,19 @@ Optional properties:
>>    - reset-gpio: 		A GPIO spec to define which pin is connected to the
>>   			chip's !RESET pin. If specified, the driver will
>>   			assert a hardware reset at probe time.
>> + - adi,pll-clkdiv: 	The PLL clock divider, specifing the ratio between
>> +			MCLK and fsclk. The value is used to determine the
>> +			correct state of the two mode pins below.
>> +			Note that this value can be overridden at runtime
>> +			by passing the ADAU1701_CLKDIV_MCLK_LRCLK divider
>> +			with ASoC calls. However, the chips needs a full
>> +			reset cycle and a new firmware download each time
>> +			the configuration changes.
>> + - adi,pll-mode-gpios:	An array of two GPIO specs to describe the GPIOs
>> +			the ADAU's PLL config pins are connected to.
>> +			The state of the pins are set according to the
>> +			configured clock divider on ASoC side before the
>> +			firmware is loaded.
>>
>>   Examples:
>>
>> @@ -19,5 +32,6 @@ Examples:
>>   			compatible = "adi,adau1701";
>>   			reg =<0x34>;
>>   			reset-gpio =<&gpio 23 0>;
>> +			adi,pll-mode-gpios =<&gpio 24 0&gpio 25 0>;
>>   		};
>>   	};
>> diff --git a/sound/soc/codecs/adau1701.c b/sound/soc/codecs/adau1701.c
>> index b6b1a77..e6ce4fe 100644
>> --- a/sound/soc/codecs/adau1701.c
>> +++ b/sound/soc/codecs/adau1701.c
>> @@ -91,7 +91,11 @@
>>
>>   struct adau1701 {
>>   	int gpio_nreset;
>> +	int gpio_pll_mode0;
>> +	int gpio_pll_mode1;
> 
> combine in single line.

I disagree for the sake of readability.

> 
>>   	unsigned int dai_fmt;
>> +	unsigned int pll_clkdiv;
>> +	unsigned int sysclk;
>>   };
>>
>>   static const struct snd_kcontrol_new adau1701_controls[] = {
>> @@ -184,13 +188,37 @@ static unsigned int adau1701_read(struct snd_soc_codec *codec, unsigned int reg)
>>   	return value;
>>   }
>>
>> -static void adau1701_reset(struct snd_soc_codec *codec)
>> +static void adau1701_reset(struct snd_soc_codec *codec, unsigned int clkdiv)
>>   {
>>   	struct adau1701 *adau1701 = snd_soc_codec_get_drvdata(codec);
>>
>>   	if (!gpio_is_valid(adau1701->gpio_nreset))
>>   		return;
>>
>> +	if (gpio_is_valid(adau1701->gpio_pll_mode0)&&
>> +	    gpio_is_valid(adau1701->gpio_pll_mode1)) {
>> +		switch (adau1701->pll_clkdiv) {
>> +		case 64:
> 
> magic number?

That's a divider value. How and why would you possibly add a #define for
that?

> 
>> +			gpio_set_value(adau1701->gpio_pll_mode0, 0);
>> +			gpio_set_value(adau1701->gpio_pll_mode1, 0);
>> +			break;
>> +		case 256:
>> +			gpio_set_value(adau1701->gpio_pll_mode0, 0);
>> +			gpio_set_value(adau1701->gpio_pll_mode1, 1);
>> +			break;
>> +		case 384:
>> +			gpio_set_value(adau1701->gpio_pll_mode0, 1);
>> +			gpio_set_value(adau1701->gpio_pll_mode1, 0);
>> +			break;
>> +		case 512:
>> +			gpio_set_value(adau1701->gpio_pll_mode0, 1);
>> +			gpio_set_value(adau1701->gpio_pll_mode1, 1);
>> +			break;
>> +		}
>> +	}
>> +
>> +	adau1701->pll_clkdiv = clkdiv;
>> +
>>   	gpio_set_value(adau1701->gpio_nreset, 0);
>>   	/* minimum reset time is 20ns */
>>   	udelay(1);
>> @@ -199,24 +227,6 @@ static void adau1701_reset(struct snd_soc_codec *codec)
>>   	mdelay(85);
>>   }
>>
>> -static int adau1701_init(struct snd_soc_codec *codec)
>> -{
>> -	int ret;
>> -	struct i2c_client *client = to_i2c_client(codec->dev);
>> -
>> -	adau1701_reset(codec);
>> -
>> -	ret = process_sigma_firmware(client, ADAU1701_FIRMWARE);
>> -	if (ret) {
>> -		dev_warn(codec->dev, "Failed to load firmware\n");
>> -		return ret;
>> -	}
>> -
>> -	snd_soc_write(codec, ADAU1701_DACSET, ADAU1701_DACSET_DACINIT);
>> -
>> -	return 0;
>> -}
>> -
>>   static int adau1701_set_capture_pcm_format(struct snd_soc_codec *codec,
>>   		snd_pcm_format_t format)
>>   {
>> @@ -291,9 +301,22 @@ static int adau1701_hw_params(struct snd_pcm_substream *substream,
>>   		struct snd_pcm_hw_params *params, struct snd_soc_dai *dai)
>>   {
>>   	struct snd_soc_codec *codec = dai->codec;
>> +	struct adau1701 *adau1701 = snd_soc_codec_get_drvdata(codec);
>>   	snd_pcm_format_t format;
>>   	unsigned int val;
>>
>> +	if (adau1701->sysclk) {
>> +		unsigned int clkdiv = adau1701->sysclk / params_rate(params);
> 
> It will give warning.

Please elaborate.



Thanks,
Daniel

Re: [PATCH 1/3] ASoC: codecs: adau1701: allow configuration of PLL mode pins

[Lars-Peter Clausen]

On 06/20/2013 07:29 PM, Daniel Mack wrote:
> The ADAU1701 has 2 hardware pins to configure the PLL mode in accordance
> to the MCLK-to-LRCLK ratio. These pins have to be stable before the chip
> is released from reset, and a full reset cycle, including a new firmware
> download is needed whenever they change.
> 
> This patch adds GPIO properties to the DT bindings of the Codec, and
> implements makes the set_sysclk memorize the configured sysclk.
> 
> To avoid excessive reset cycles and firmware downloads, the default
> clock divider can be specified in DT as well. Whenever a ratio change is
> detected in the hw_params callback, the PLL mode lines are updates and a
> full reset cycle is issued.
> 
> Signed-off-by: Daniel Mack <zonque@gmail.com>
> ---
>  .../devicetree/bindings/sound/adi,adau1701.txt     |  14 +++
>  sound/soc/codecs/adau1701.c                        | 107 +++++++++++++++++----
>  sound/soc/codecs/adau1701.h                        |   4 +
>  3 files changed, 104 insertions(+), 21 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/sound/adi,adau1701.txt b/Documentation/devicetree/bindings/sound/adi,adau1701.txt
> index 3afeda7..a0d7e92 100644
> --- a/Documentation/devicetree/bindings/sound/adi,adau1701.txt
> +++ b/Documentation/devicetree/bindings/sound/adi,adau1701.txt
> @@ -11,6 +11,19 @@ Optional properties:
>   - reset-gpio: 		A GPIO spec to define which pin is connected to the
>  			chip's !RESET pin. If specified, the driver will
>  			assert a hardware reset at probe time.
> + - adi,pll-clkdiv: 	The PLL clock divider, specifing the ratio between
> +			MCLK and fsclk. The value is used to determine the
> +			correct state of the two mode pins below.
> +			Note that this value can be overridden at runtime
> +			by passing the ADAU1701_CLKDIV_MCLK_LRCLK divider
> +			with ASoC calls. However, the chips needs a full
> +			reset cycle and a new firmware download each time
> +			the configuration changes.

Considering that this is now done all automatically at runtime, do we
actually still need this property?

> + - adi,pll-mode-gpios:	An array of two GPIO specs to describe the GPIOs
> +			the ADAU's PLL config pins are connected to.
> +			The state of the pins are set according to the
> +			configured clock divider on ASoC side before the
> +			firmware is loaded.
>  
>  Examples:
>  
> @@ -19,5 +32,6 @@ Examples:
>  			compatible = "adi,adau1701";
>  			reg = <0x34>;
>  			reset-gpio = <&gpio 23 0>;
> +			adi,pll-mode-gpios = <&gpio 24 0 &gpio 25 0>;
>  		};
>  	};
> diff --git a/sound/soc/codecs/adau1701.c b/sound/soc/codecs/adau1701.c
> index b6b1a77..e6ce4fe 100644
> --- a/sound/soc/codecs/adau1701.c
> +++ b/sound/soc/codecs/adau1701.c
> @@ -91,7 +91,11 @@
>  
>  struct adau1701 {
>  	int gpio_nreset;
> +	int gpio_pll_mode0;
> +	int gpio_pll_mode1;

I would have made this an array, but I guess either solution is fine.

>  	unsigned int dai_fmt;
> +	unsigned int pll_clkdiv;
> +	unsigned int sysclk;
>  };
>  
>  static const struct snd_kcontrol_new adau1701_controls[] = {
> @@ -184,13 +188,37 @@ static unsigned int adau1701_read(struct snd_soc_codec *codec, unsigned int reg)
>  	return value;
>  }
>  
> -static void adau1701_reset(struct snd_soc_codec *codec)
> +static void adau1701_reset(struct snd_soc_codec *codec, unsigned int clkdiv)
>  {
>  	struct adau1701 *adau1701 = snd_soc_codec_get_drvdata(codec);
>  
>  	if (!gpio_is_valid(adau1701->gpio_nreset))
>  		return;
>  
> +	if (gpio_is_valid(adau1701->gpio_pll_mode0) &&
> +	    gpio_is_valid(adau1701->gpio_pll_mode1)) {
> +		switch (adau1701->pll_clkdiv) {

Considering that adau1701->pll_clkdiv is only assigned below this should
either be clkdiv, or the assigment should be moved up.

> +		case 64:
> +			gpio_set_value(adau1701->gpio_pll_mode0, 0);
> +			gpio_set_value(adau1701->gpio_pll_mode1, 0);
> +			break;
> +		case 256:
> +			gpio_set_value(adau1701->gpio_pll_mode0, 0);
> +			gpio_set_value(adau1701->gpio_pll_mode1, 1);
> +			break;
> +		case 384:
> +			gpio_set_value(adau1701->gpio_pll_mode0, 1);
> +			gpio_set_value(adau1701->gpio_pll_mode1, 0);
> +			break;
> +		case 512:
> +			gpio_set_value(adau1701->gpio_pll_mode0, 1);
> +			gpio_set_value(adau1701->gpio_pll_mode1, 1);
> +			break;
> +		}
> +	}
> +
> +	adau1701->pll_clkdiv = clkdiv;
> +
>  	gpio_set_value(adau1701->gpio_nreset, 0);
>  	/* minimum reset time is 20ns */
>  	udelay(1);
> @@ -199,24 +227,6 @@ static void adau1701_reset(struct snd_soc_codec *codec)
>  	mdelay(85);
}
[...]
> diff --git a/sound/soc/codecs/adau1701.h b/sound/soc/codecs/adau1701.h
> index 8d0949a..bdcdddc 100644
> --- a/sound/soc/codecs/adau1701.h
> +++ b/sound/soc/codecs/adau1701.h
> @@ -14,4 +14,8 @@ enum adau1701_clk_src {
>  	ADAU1701_CLK_SRC_MCLK,
>  };
>  
> +enum adau1701_clkdiv {
> +	ADAU1701_CLKDIV_MCLK_LRCLK,
> +};
> +

This is unused now.

>  #endif

Re: [PATCH 3/3] ASoC: codecs: adau1701: add support for pin muxing

[Lars-Peter Clausen]

On 06/20/2013 07:29 PM, Daniel Mack wrote:
> [...]
>  static int adau1701_probe(struct snd_soc_codec *codec)
>  {
> -	int ret;
> +	int ret, i;
> +	unsigned int val;
>  	struct i2c_client *client = to_i2c_client(codec->dev);
>  	struct adau1701 *adau1701 = snd_soc_codec_get_drvdata(codec);
>  
> @@ -576,6 +584,19 @@ static int adau1701_probe(struct snd_soc_codec *codec)
>  	regmap_write(adau1701->regmap, ADAU1701_DACSET, ADAU1701_DACSET_DACINIT);
>  	regmap_write(adau1701->regmap, ADAU1701_DSPCTRL, ADAU1701_DSPCTRL_CR);
>  
> +	/* set up pin config */
> +	val = 0;
> +	for (i = 0; i < 6; i++)
> +		val |= adau1701->pin_config[i] << (i * 4);
> +
> +	regmap_write(adau1701->regmap, ADAU1701_PINCONF_0, val);
> +
> +	val = 0;
> +	for (i = 0; i < 6; i++)
> +		val |= adau1701->pin_config[i + 6] << (i * 4);
> +
> +	regmap_write(adau1701->regmap, ADAU1701_PINCONF_1, val);

Will the config survive a reset?

Re: [PATCH 2/3] ASoC: codecs: adau1701: switch to direct regmap API usage

[Lars-Peter Clausen]

On 06/20/2013 07:29 PM, Daniel Mack wrote:
> The hardware I/O has to be open-coded due to registers of unequal sizes.
> Other than that, the transition is straight forward.
> 
> Signed-off-by: Daniel Mack <zonque@gmail.com>

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

Thanks. One minor nitpick though.

> ---
[...]
> +	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 | I2C_M_STOP;

We don't need to set a explicit stop flag for the last message in the transfer.

Re: [PATCH 3/3] ASoC: codecs: adau1701: add support for pin muxing

[Daniel Mack]

On 21.06.2013 09:24, Lars-Peter Clausen wrote:
> On 06/20/2013 07:29 PM, Daniel Mack wrote:
>> [...]
>>  static int adau1701_probe(struct snd_soc_codec *codec)
>>  {
>> -	int ret;
>> +	int ret, i;
>> +	unsigned int val;
>>  	struct i2c_client *client = to_i2c_client(codec->dev);
>>  	struct adau1701 *adau1701 = snd_soc_codec_get_drvdata(codec);
>>  
>> @@ -576,6 +584,19 @@ static int adau1701_probe(struct snd_soc_codec *codec)
>>  	regmap_write(adau1701->regmap, ADAU1701_DACSET, ADAU1701_DACSET_DACINIT);
>>  	regmap_write(adau1701->regmap, ADAU1701_DSPCTRL, ADAU1701_DSPCTRL_CR);
>>  
>> +	/* set up pin config */
>> +	val = 0;
>> +	for (i = 0; i < 6; i++)
>> +		val |= adau1701->pin_config[i] << (i * 4);
>> +
>> +	regmap_write(adau1701->regmap, ADAU1701_PINCONF_0, val);
>> +
>> +	val = 0;
>> +	for (i = 0; i < 6; i++)
>> +		val |= adau1701->pin_config[i + 6] << (i * 4);
>> +
>> +	regmap_write(adau1701->regmap, ADAU1701_PINCONF_1, val);
> 
> Will the config survive a reset?
> 

That will be synced back via regcache_sync(), no?

Re: [PATCH 1/3] ASoC: codecs: adau1701: allow configuration of PLL mode pins

[Daniel Mack]

On 21.06.2013 09:23, Lars-Peter Clausen wrote:
> On 06/20/2013 07:29 PM, Daniel Mack wrote:
>> The ADAU1701 has 2 hardware pins to configure the PLL mode in accordance
>> to the MCLK-to-LRCLK ratio. These pins have to be stable before the chip
>> is released from reset, and a full reset cycle, including a new firmware
>> download is needed whenever they change.
>>
>> This patch adds GPIO properties to the DT bindings of the Codec, and
>> implements makes the set_sysclk memorize the configured sysclk.
>>
>> To avoid excessive reset cycles and firmware downloads, the default
>> clock divider can be specified in DT as well. Whenever a ratio change is
>> detected in the hw_params callback, the PLL mode lines are updates and a
>> full reset cycle is issued.
>>
>> Signed-off-by: Daniel Mack <zonque@gmail.com>
>> ---
>>  .../devicetree/bindings/sound/adi,adau1701.txt     |  14 +++
>>  sound/soc/codecs/adau1701.c                        | 107 +++++++++++++++++----
>>  sound/soc/codecs/adau1701.h                        |   4 +
>>  3 files changed, 104 insertions(+), 21 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/sound/adi,adau1701.txt b/Documentation/devicetree/bindings/sound/adi,adau1701.txt
>> index 3afeda7..a0d7e92 100644
>> --- a/Documentation/devicetree/bindings/sound/adi,adau1701.txt
>> +++ b/Documentation/devicetree/bindings/sound/adi,adau1701.txt
>> @@ -11,6 +11,19 @@ Optional properties:
>>   - reset-gpio: 		A GPIO spec to define which pin is connected to the
>>  			chip's !RESET pin. If specified, the driver will
>>  			assert a hardware reset at probe time.
>> + - adi,pll-clkdiv: 	The PLL clock divider, specifing the ratio between
>> +			MCLK and fsclk. The value is used to determine the
>> +			correct state of the two mode pins below.
>> +			Note that this value can be overridden at runtime
>> +			by passing the ADAU1701_CLKDIV_MCLK_LRCLK divider
>> +			with ASoC calls. However, the chips needs a full
>> +			reset cycle and a new firmware download each time
>> +			the configuration changes.
> 
> Considering that this is now done all automatically at runtime, do we
> actually still need this property?

Yes, I noticed that as well, but given that changing the PLL mode setup
is expensive, and because we need some sort of firmware to start up at
least, I wanted to provide a way to set these values up-front, so
systems with stable clock dividers can minimize the number of firmware
transfers.

>> -static void adau1701_reset(struct snd_soc_codec *codec)
>> +static void adau1701_reset(struct snd_soc_codec *codec, unsigned int clkdiv)
>>  {
>>  	struct adau1701 *adau1701 = snd_soc_codec_get_drvdata(codec);
>>  
>>  	if (!gpio_is_valid(adau1701->gpio_nreset))
>>  		return;
>>  
>> +	if (gpio_is_valid(adau1701->gpio_pll_mode0) &&
>> +	    gpio_is_valid(adau1701->gpio_pll_mode1)) {
>> +		switch (adau1701->pll_clkdiv) {
> 
> Considering that adau1701->pll_clkdiv is only assigned below this should
> either be clkdiv, or the assigment should be moved up.

Very true. It didn't hit me as in my setup, the PLL mode is passed in
via DT.

Re: [PATCH 2/3] ASoC: codecs: adau1701: switch to direct regmap API usage

[Daniel Mack]

On 21.06.2013 09:28, Lars-Peter Clausen wrote:
> On 06/20/2013 07:29 PM, Daniel Mack wrote:
>> The hardware I/O has to be open-coded due to registers of unequal sizes.
>> Other than that, the transition is straight forward.
>>
>> Signed-off-by: Daniel Mack <zonque@gmail.com>
> 
> Acked-by: Lars-Peter Clausen <lars@metafoo.de>
> 
> Thanks. One minor nitpick though.
> 
>> ---
> [...]
>> +	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 | I2C_M_STOP;
> 
> We don't need to set a explicit stop flag for the last message in the transfer.
> 

Right. I'll send a v3 of the entire series. Hang on.

Re: [PATCH 3/3] ASoC: codecs: adau1701: add support for pin muxing

[Lars-Peter Clausen]

On 06/21/2013 09:28 AM, Daniel Mack wrote:
> On 21.06.2013 09:24, Lars-Peter Clausen wrote:
>> On 06/20/2013 07:29 PM, Daniel Mack wrote:
>>> [...]
>>>  static int adau1701_probe(struct snd_soc_codec *codec)
>>>  {
>>> -	int ret;
>>> +	int ret, i;
>>> +	unsigned int val;
>>>  	struct i2c_client *client = to_i2c_client(codec->dev);
>>>  	struct adau1701 *adau1701 = snd_soc_codec_get_drvdata(codec);
>>>  
>>> @@ -576,6 +584,19 @@ static int adau1701_probe(struct snd_soc_codec *codec)
>>>  	regmap_write(adau1701->regmap, ADAU1701_DACSET, ADAU1701_DACSET_DACINIT);
>>>  	regmap_write(adau1701->regmap, ADAU1701_DSPCTRL, ADAU1701_DSPCTRL_CR);
>>>  
>>> +	/* set up pin config */
>>> +	val = 0;
>>> +	for (i = 0; i < 6; i++)
>>> +		val |= adau1701->pin_config[i] << (i * 4);
>>> +
>>> +	regmap_write(adau1701->regmap, ADAU1701_PINCONF_0, val);
>>> +
>>> +	val = 0;
>>> +	for (i = 0; i < 6; i++)
>>> +		val |= adau1701->pin_config[i + 6] << (i * 4);
>>> +
>>> +	regmap_write(adau1701->regmap, ADAU1701_PINCONF_1, val);
>>
>> Will the config survive a reset?
>>
> 
> That will be synced back via regcache_sync(), no?
> 
Oh, sorry I missed the regcache_sync(), but you still need to call
regcache_mark_dirty() first, otherwise regcache_sync() won't do anything.

- Lars

Re: [PATCH 3/3] ASoC: codecs: adau1701: add support for pin muxing

[Lars-Peter Clausen]

On 06/21/2013 09:35 AM, Lars-Peter Clausen wrote:
> On 06/21/2013 09:28 AM, Daniel Mack wrote:
>> On 21.06.2013 09:24, Lars-Peter Clausen wrote:
>>> On 06/20/2013 07:29 PM, Daniel Mack wrote:
>>>> [...]
>>>>  static int adau1701_probe(struct snd_soc_codec *codec)
>>>>  {
>>>> -	int ret;
>>>> +	int ret, i;
>>>> +	unsigned int val;
>>>>  	struct i2c_client *client = to_i2c_client(codec->dev);
>>>>  	struct adau1701 *adau1701 = snd_soc_codec_get_drvdata(codec);
>>>>  
>>>> @@ -576,6 +584,19 @@ static int adau1701_probe(struct snd_soc_codec *codec)
>>>>  	regmap_write(adau1701->regmap, ADAU1701_DACSET, ADAU1701_DACSET_DACINIT);
>>>>  	regmap_write(adau1701->regmap, ADAU1701_DSPCTRL, ADAU1701_DSPCTRL_CR);
>>>>  
>>>> +	/* set up pin config */
>>>> +	val = 0;
>>>> +	for (i = 0; i < 6; i++)
>>>> +		val |= adau1701->pin_config[i] << (i * 4);
>>>> +
>>>> +	regmap_write(adau1701->regmap, ADAU1701_PINCONF_0, val);
>>>> +
>>>> +	val = 0;
>>>> +	for (i = 0; i < 6; i++)
>>>> +		val |= adau1701->pin_config[i + 6] << (i * 4);
>>>> +
>>>> +	regmap_write(adau1701->regmap, ADAU1701_PINCONF_1, val);
>>>
>>> Will the config survive a reset?
>>>
>>
>> That will be synced back via regcache_sync(), no?
>>
> Oh, sorry I missed the regcache_sync(), but you still need to call
> regcache_mark_dirty() first, otherwise regcache_sync() won't do anything.

wait... the regcache_sync() is only in adau1761_probe(), but not in
adau1761_reset().

Re: [PATCH 1/3] ASoC: codecs: adau1701: allow configuration of PLL mode pins

[Lars-Peter Clausen]

On 06/21/2013 09:31 AM, Daniel Mack wrote:
> On 21.06.2013 09:23, Lars-Peter Clausen wrote:
>> On 06/20/2013 07:29 PM, Daniel Mack wrote:
>>> The ADAU1701 has 2 hardware pins to configure the PLL mode in accordance
>>> to the MCLK-to-LRCLK ratio. These pins have to be stable before the chip
>>> is released from reset, and a full reset cycle, including a new firmware
>>> download is needed whenever they change.
>>>
>>> This patch adds GPIO properties to the DT bindings of the Codec, and
>>> implements makes the set_sysclk memorize the configured sysclk.
>>>
>>> To avoid excessive reset cycles and firmware downloads, the default
>>> clock divider can be specified in DT as well. Whenever a ratio change is
>>> detected in the hw_params callback, the PLL mode lines are updates and a
>>> full reset cycle is issued.
>>>
>>> Signed-off-by: Daniel Mack <zonque@gmail.com>
>>> ---
>>>  .../devicetree/bindings/sound/adi,adau1701.txt     |  14 +++
>>>  sound/soc/codecs/adau1701.c                        | 107 +++++++++++++++++----
>>>  sound/soc/codecs/adau1701.h                        |   4 +
>>>  3 files changed, 104 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/sound/adi,adau1701.txt b/Documentation/devicetree/bindings/sound/adi,adau1701.txt
>>> index 3afeda7..a0d7e92 100644
>>> --- a/Documentation/devicetree/bindings/sound/adi,adau1701.txt
>>> +++ b/Documentation/devicetree/bindings/sound/adi,adau1701.txt
>>> @@ -11,6 +11,19 @@ Optional properties:
>>>   - reset-gpio: 		A GPIO spec to define which pin is connected to the
>>>  			chip's !RESET pin. If specified, the driver will
>>>  			assert a hardware reset at probe time.
>>> + - adi,pll-clkdiv: 	The PLL clock divider, specifing the ratio between
>>> +			MCLK and fsclk. The value is used to determine the
>>> +			correct state of the two mode pins below.
>>> +			Note that this value can be overridden at runtime
>>> +			by passing the ADAU1701_CLKDIV_MCLK_LRCLK divider
>>> +			with ASoC calls. However, the chips needs a full
>>> +			reset cycle and a new firmware download each time
>>> +			the configuration changes.
>>
>> Considering that this is now done all automatically at runtime, do we
>> actually still need this property?
> 
> Yes, I noticed that as well, but given that changing the PLL mode setup
> is expensive, and because we need some sort of firmware to start up at
> least, I wanted to provide a way to set these values up-front, so
> systems with stable clock dividers can minimize the number of firmware
> transfers.

Ok, fair enough, but the documentation for the property needs to be updated.

- Lars

Re: [PATCH 1/3] ASoC: codecs: adau1701: allow configuration of PLL mode pins

[Rajeev kumar]

On 6/21/2013 11:36 AM, Daniel Mack wrote:
>>> >>     	struct snd_soc_codec *codec = dai->codec;
>>> >>  +	struct adau1701 *adau1701 = snd_soc_codec_get_drvdata(codec);
>>> >>     	snd_pcm_format_t format;
>>> >>     	unsigned int val;
>>> >>
>>> >>  +	if (adau1701->sysclk) {
>>> >>  +		unsigned int clkdiv = adau1701->sysclk / params_rate(params);
>> >
>> >  It will give warning.
> Please elaborate.

you are declaring something (unsigned int clkdiv) in between code.

~Rajeev

Re: [PATCH 1/3] ASoC: codecs: adau1701: allow configuration of PLL mode pins

[Daniel Mack]

On 21.06.2013 10:14, Rajeev kumar wrote:
> On 6/21/2013 11:36 AM, Daniel Mack wrote:
>>>>>>     	struct snd_soc_codec *codec = dai->codec;
>>>>>>  +	struct adau1701 *adau1701 = snd_soc_codec_get_drvdata(codec);
>>>>>>     	snd_pcm_format_t format;
>>>>>>     	unsigned int val;
>>>>>>
>>>>>>  +	if (adau1701->sysclk) {
>>>>>>  +		unsigned int clkdiv = adau1701->sysclk / params_rate(params);
>>>>
>>>>  It will give warning.
>> Please elaborate.
> 
> you are declaring something (unsigned int clkdiv) in between code.

It's not in between code, it's at the beginning of a new block ('{').
That's totally fine and done everywhere else in the kernel.


Daniel

Re: [PATCH 1/3] ASoC: codecs: adau1701: allow configuration of PLL mode pins

[Lars-Peter Clausen]

On 06/21/2013 10:14 AM, Rajeev kumar wrote:
> On 6/21/2013 11:36 AM, Daniel Mack wrote:
>>>> >>         struct snd_soc_codec *codec = dai->codec;
>>>> >>  +    struct adau1701 *adau1701 = snd_soc_codec_get_drvdata(codec);
>>>> >>         snd_pcm_format_t format;
>>>> >>         unsigned int val;
>>>> >>
>>>> >>  +    if (adau1701->sysclk) {
>>>> >>  +        unsigned int clkdiv = adau1701->sysclk / params_rate(params);
>>> >
>>> >  It will give warning.
>> Please elaborate.
> 
> you are declaring something (unsigned int clkdiv) in between code.
> 

It's at the beginning at a block, that's actually fine. Neither checkpatch
or gcc will generate a warning.