diff mbox

ASoC: rt5677: Use specific r/w function for DSP mode

Message ID 1414378351-2329-1-git-send-email-oder_chiou@realtek.com (mailing list archive)
State New, archived
Headers show

Commit Message

Oder Chiou Oct. 27, 2014, 2:52 a.m. UTC 
In DSP mode, the register r/w should use the specific function to access
that is invoked by address mapping of the DSP.

The MX-65[1] is for switching DSP or codec mode.

Signed-off-by: Oder Chiou <oder_chiou@realtek.com>
---
 sound/soc/codecs/rt5677.c | 222 ++++++++++++++++++++++++++++++----------------
 sound/soc/codecs/rt5677.h |   2 +
 2 files changed, 149 insertions(+), 75 deletions(-)

Comments

Mark Brown Oct. 28, 2014, 10:29 p.m. UTC | #1 
On Mon, Oct 27, 2014 at 10:52:31AM +0800, Oder Chiou wrote:

> +	/* Write register */
> +	r[0] = reg & 0xff;
> +	xfer[0].addr = client->addr;
> +	xfer[0].flags = 0;
> +	xfer[0].len = 1;
> +	xfer[0].buf = &r[0];
> +
> +	/* Read data */
> +	xfer[1].addr = client->addr;
> +	xfer[1].flags = I2C_M_RD;
> +	xfer[1].len = 2;
> +	xfer[1].buf = data;
> +
> +	ret = i2c_transfer(client->adapter, xfer, 2);
> +	if (ret != 2) {
> +		dev_err(&client->dev, "i2c_transfer() returned %d\n", ret);
> +		return -EIO;
> +	}

This looks like standard regmap stuff - what's different?
Oder Chiou Oct. 29, 2014, 1:44 a.m. UTC | #2 
> -----Original Message-----
> From: Mark Brown [mailto:broonie@kernel.org]
> Sent: Wednesday, October 29, 2014 6:29 AM
> To: Oder Chiou
> Cc: lgirdwood@gmail.com; alsa-devel@alsa-project.org; Bard Liao; Flove;
> anatol@google.com; benzh@google.com
> Subject: Re: [PATCH] ASoC: rt5677: Use specific r/w function for DSP mode
> 
> On Mon, Oct 27, 2014 at 10:52:31AM +0800, Oder Chiou wrote:
> 
> > +	/* Write register */
> > +	r[0] = reg & 0xff;
> > +	xfer[0].addr = client->addr;
> > +	xfer[0].flags = 0;
> > +	xfer[0].len = 1;
> > +	xfer[0].buf = &r[0];
> > +
> > +	/* Read data */
> > +	xfer[1].addr = client->addr;
> > +	xfer[1].flags = I2C_M_RD;
> > +	xfer[1].len = 2;
> > +	xfer[1].buf = data;
> > +
> > +	ret = i2c_transfer(client->adapter, xfer, 2);
> > +	if (ret != 2) {
> > +		dev_err(&client->dev, "i2c_transfer() returned %d\n", ret);
> > +		return -EIO;
> > +	}
> 
> This looks like standard regmap stuff - what's different?

They are not different with regmap stuff, but they are called by
rt5677_dsp_mode_i2c_write_addr(), rt5677_dsp_mode_i2c_read_addr(),
rt5677_read() and rt5677_write().

The customize access function is implemented by rt5677_read() and
rt5677_write() that are assigned to regmap_config "rt5677_regmap" for using
between codec mode or DSP mode.
Mark Brown Oct. 31, 2014, 5:52 p.m. UTC | #3 
On Wed, Oct 29, 2014 at 01:44:57AM +0000, Oder Chiou wrote:

> > This looks like standard regmap stuff - what's different?

> They are not different with regmap stuff, but they are called by
> rt5677_dsp_mode_i2c_write_addr(), rt5677_dsp_mode_i2c_read_addr(),
> rt5677_read() and rt5677_write().

> The customize access function is implemented by rt5677_read() and
> rt5677_write() that are assigned to regmap_config "rt5677_regmap" for using
> between codec mode or DSP mode.

Sorry, I'm not quite following what you're saying here - if you're
saying that they're slightly different to the main register map I/O for
this device then it's OK to create two regmaps and only access the DSP
one through separate functions.
Oder Chiou Nov. 3, 2014, 2:31 a.m. UTC | #4 
> -----Original Message-----
> From: Mark Brown [mailto:broonie@kernel.org]
> Sent: Saturday, November 01, 2014 1:52 AM
> To: Oder Chiou
> Cc: lgirdwood@gmail.com; alsa-devel@alsa-project.org; Bard Liao; Flove;
> anatol@google.com; benzh@google.com
> Subject: Re: [PATCH] ASoC: rt5677: Use specific r/w function for DSP mode
> 
> On Wed, Oct 29, 2014 at 01:44:57AM +0000, Oder Chiou wrote:
> 
> > > This looks like standard regmap stuff - what's different?
> 
> > They are not different with regmap stuff, but they are called by
> > rt5677_dsp_mode_i2c_write_addr(), rt5677_dsp_mode_i2c_read_addr(),
> > rt5677_read() and rt5677_write().
> 
> > The customize access function is implemented by rt5677_read() and
> > rt5677_write() that are assigned to regmap_config "rt5677_regmap" for
> > using between codec mode or DSP mode.
> 
> Sorry, I'm not quite following what you're saying here - if you're saying that they're
> slightly different to the main register map I/O for this device then it's OK to create
> two regmaps and only access the DSP one through separate functions.
> 
Sorry for my weak explanations. In this codec, there are 2 access modes, and it
only can be chosen one mode at the same time. In codec mode, we use the normal
access method to access codec settings, and in DSP mode, we also can access the
codec settings, but it should use the method of memory address map to access.
Ex. If we want to access the MX-01 in the DSP mode, we need to convert to
memory address 0x18020000 + (01 * 2). We want to use the same regmap table to
handle the codec settings, so the customize r/w functions are needed in it.
In the DSP mode, the registers MX-00 ~ MX-04 are used for DSP address access,
so it also need the native function to avoid the regmap handle, thanks.
Mark Brown Nov. 4, 2014, 8:40 p.m. UTC | #5 
On Mon, Nov 03, 2014 at 02:31:41AM +0000, Oder Chiou wrote:

> > Sorry, I'm not quite following what you're saying here - if you're saying that they're
> > slightly different to the main register map I/O for this device then it's OK to create
> > two regmaps and only access the DSP one through separate functions.

> Sorry for my weak explanations. In this codec, there are 2 access modes, and it
> only can be chosen one mode at the same time. In codec mode, we use the normal
> access method to access codec settings, and in DSP mode, we also can access the
> codec settings, but it should use the method of memory address map to access.
> Ex. If we want to access the MX-01 in the DSP mode, we need to convert to
> memory address 0x18020000 + (01 * 2). We want to use the same regmap table to
> handle the codec settings, so the customize r/w functions are needed in it.
> In the DSP mode, the registers MX-00 ~ MX-04 are used for DSP address access,
> so it also need the native function to avoid the regmap handle, thanks.

So, I think what's being said here is that the CODEC has two I/O modes
which you can switch between at runtime and you want to do that
switching.  One way to deal with this would be to just go into DSP mode
immediately on power up since it's hard to see a *big* cost in that.

Another way which is more in line with what you have would be to have a
regmap like you do with reg_read() and reg_write() operations but
instead of switching between open coded functions switch between two
regmaps which just have physical I/O defined (no register cache or
anything).  You then have three regmaps - the master regmap that ASoC
sees and which has the cache and the two physical regmaps which do the
actual I/O.
diff mbox

Patch

diff --git a/sound/soc/codecs/rt5677.c b/sound/soc/codecs/rt5677.c
index 413bccb..74f65ff 100644
--- a/sound/soc/codecs/rt5677.c
+++ b/sound/soc/codecs/rt5677.c
@@ -539,53 +539,113 @@  static bool rt5677_readable_register(struct device *dev, unsigned int reg)
 	}
 }
 
+static int rt5677_hw_read(void *context, unsigned int reg, unsigned int *val)
+{
+	struct i2c_client *client = context;
+	struct i2c_msg xfer[2];
+	u8 r[1];
+	u8 data[2];
+	int ret;
+
+	/* Write register */
+	r[0] = reg & 0xff;
+	xfer[0].addr = client->addr;
+	xfer[0].flags = 0;
+	xfer[0].len = 1;
+	xfer[0].buf = &r[0];
+
+	/* Read data */
+	xfer[1].addr = client->addr;
+	xfer[1].flags = I2C_M_RD;
+	xfer[1].len = 2;
+	xfer[1].buf = data;
+
+	ret = i2c_transfer(client->adapter, xfer, 2);
+	if (ret != 2) {
+		dev_err(&client->dev, "i2c_transfer() returned %d\n", ret);
+		return -EIO;
+	}
+
+	*val = (data[0] << 8) | data[1];
+
+	dev_dbg(&client->dev, "read %02x => %04x\n", reg, *val);
+
+	return 0;
+}
+
+static int rt5677_hw_write(void *context, unsigned int reg, unsigned int val)
+{
+	struct i2c_client *client = context;
+	u8 data[3];
+	int ret;
+
+	data[0] = reg & 0xff;
+	data[1] = (0xff00 & val) >> 8;
+	data[2] = val & 0xff;
+
+	dev_dbg(&client->dev, "write %02x = %04x\n", reg, val);
+
+	ret = i2c_master_send(client, data, 3);
+	if (ret == 3)
+		return 0;
+	if (ret < 0)
+		return ret;
+	else
+		return -EIO;
+}
+
 /**
  * rt5677_dsp_mode_i2c_write_addr - Write value to address on DSP mode.
- * @codec: SoC audio codec device.
+ * @rt5677: Private Data.
  * @addr: Address index.
  * @value: Address data.
  *
  *
  * Returns 0 for success or negative error code.
  */
-static int rt5677_dsp_mode_i2c_write_addr(struct snd_soc_codec *codec,
+static int rt5677_dsp_mode_i2c_write_addr(struct rt5677_priv *rt5677,
 		unsigned int addr, unsigned int value, unsigned int opcode)
 {
-	struct rt5677_priv *rt5677 = snd_soc_codec_get_drvdata(codec);
+	struct i2c_client *client = rt5677->i2c;
 	int ret;
 
 	mutex_lock(&rt5677->dsp_cmd_lock);
 
-	ret = regmap_write(rt5677->regmap, RT5677_DSP_I2C_ADDR_MSB, addr >> 16);
+	ret = rt5677_hw_write(client, RT5677_DSP_I2C_ADDR_MSB, addr >> 16);
 	if (ret < 0) {
-		dev_err(codec->dev, "Failed to set addr msb value: %d\n", ret);
+		dev_err(&client->dev, "Failed to set addr msb value: %d\n",
+			ret);
 		goto err;
 	}
 
-	ret = regmap_write(rt5677->regmap, RT5677_DSP_I2C_ADDR_LSB,
+	ret = rt5677_hw_write(client, RT5677_DSP_I2C_ADDR_LSB,
 		addr & 0xffff);
 	if (ret < 0) {
-		dev_err(codec->dev, "Failed to set addr lsb value: %d\n", ret);
+		dev_err(&client->dev, "Failed to set addr lsb value: %d\n",
+			ret);
 		goto err;
 	}
 
-	ret = regmap_write(rt5677->regmap, RT5677_DSP_I2C_DATA_MSB,
+	ret = rt5677_hw_write(client, RT5677_DSP_I2C_DATA_MSB,
 		value >> 16);
 	if (ret < 0) {
-		dev_err(codec->dev, "Failed to set data msb value: %d\n", ret);
+		dev_err(&client->dev, "Failed to set data msb value: %d\n",
+			ret);
 		goto err;
 	}
 
-	ret = regmap_write(rt5677->regmap, RT5677_DSP_I2C_DATA_LSB,
+	ret = rt5677_hw_write(client, RT5677_DSP_I2C_DATA_LSB,
 		value & 0xffff);
 	if (ret < 0) {
-		dev_err(codec->dev, "Failed to set data lsb value: %d\n", ret);
+		dev_err(&client->dev, "Failed to set data lsb value: %d\n",
+			ret);
 		goto err;
 	}
 
-	ret = regmap_write(rt5677->regmap, RT5677_DSP_I2C_OP_CODE, opcode);
+	ret = rt5677_hw_write(client, RT5677_DSP_I2C_OP_CODE, opcode);
 	if (ret < 0) {
-		dev_err(codec->dev, "Failed to set op code value: %d\n", ret);
+		dev_err(&client->dev, "Failed to set op code value: %d\n",
+			ret);
 		goto err;
 	}
 
@@ -597,42 +657,45 @@  err:
 
 /**
  * rt5677_dsp_mode_i2c_read_addr - Read value from address on DSP mode.
- * @codec: SoC audio codec device.
+ * rt5677: Private Data.
  * @addr: Address index.
  * @value: Address data.
  *
+ *
  * Returns 0 for success or negative error code.
  */
 static int rt5677_dsp_mode_i2c_read_addr(
-	struct snd_soc_codec *codec, unsigned int addr, unsigned int *value)
+	struct rt5677_priv *rt5677, unsigned int addr, unsigned int *value)
 {
-	struct rt5677_priv *rt5677 = snd_soc_codec_get_drvdata(codec);
+	struct i2c_client *client = rt5677->i2c;
 	int ret;
 	unsigned int msb, lsb;
 
 	mutex_lock(&rt5677->dsp_cmd_lock);
 
-	ret = regmap_write(rt5677->regmap, RT5677_DSP_I2C_ADDR_MSB, addr >> 16);
+	ret = rt5677_hw_write(client, RT5677_DSP_I2C_ADDR_MSB, addr >> 16);
 	if (ret < 0) {
-		dev_err(codec->dev, "Failed to set addr msb value: %d\n", ret);
+		dev_err(&client->dev, "Failed to set addr msb value: %d\n",
+			ret);
 		goto err;
 	}
 
-	ret = regmap_write(rt5677->regmap, RT5677_DSP_I2C_ADDR_LSB,
+	ret = rt5677_hw_write(client, RT5677_DSP_I2C_ADDR_LSB,
 		addr & 0xffff);
 	if (ret < 0) {
-		dev_err(codec->dev, "Failed to set addr lsb value: %d\n", ret);
+		dev_err(&client->dev, "Failed to set addr lsb value: %d\n",
+			ret);
 		goto err;
 	}
 
-	ret = regmap_write(rt5677->regmap, RT5677_DSP_I2C_OP_CODE , 0x0002);
+	ret = rt5677_hw_write(client, RT5677_DSP_I2C_OP_CODE , 0x0002);
 	if (ret < 0) {
-		dev_err(codec->dev, "Failed to set op code value: %d\n", ret);
+		dev_err(&client->dev, "Failed to set op code value: %d\n", ret);
 		goto err;
 	}
 
-	regmap_read(rt5677->regmap, RT5677_DSP_I2C_DATA_MSB, &msb);
-	regmap_read(rt5677->regmap, RT5677_DSP_I2C_DATA_LSB, &lsb);
+	rt5677_hw_read(client, RT5677_DSP_I2C_DATA_MSB, &msb);
+	rt5677_hw_read(client, RT5677_DSP_I2C_DATA_LSB, &lsb);
 	*value = (msb << 16) | lsb;
 
 err:
@@ -643,17 +706,17 @@  err:
 
 /**
  * rt5677_dsp_mode_i2c_write - Write register on DSP mode.
- * @codec: SoC audio codec device.
+ * rt5677: Private Data.
  * @reg: Register index.
  * @value: Register data.
  *
  *
  * Returns 0 for success or negative error code.
  */
-static int rt5677_dsp_mode_i2c_write(struct snd_soc_codec *codec,
+static int rt5677_dsp_mode_i2c_write(struct rt5677_priv *rt5677,
 		unsigned int reg, unsigned int value)
 {
-	return rt5677_dsp_mode_i2c_write_addr(codec, 0x18020000 + reg * 2,
+	return rt5677_dsp_mode_i2c_write_addr(rt5677, 0x18020000 + reg * 2,
 		value, 0x0001);
 }
 
@@ -661,57 +724,33 @@  static int rt5677_dsp_mode_i2c_write(struct snd_soc_codec *codec,
  * rt5677_dsp_mode_i2c_read - Read register on DSP mode.
  * @codec: SoC audio codec device.
  * @reg: Register index.
+ * @value: Register data.
  *
  *
- * Returns Register value.
+ * Returns 0 for success or negative error code.
  */
-static unsigned int rt5677_dsp_mode_i2c_read(
-	struct snd_soc_codec *codec, unsigned int reg)
+static int rt5677_dsp_mode_i2c_read(
+	struct rt5677_priv *rt5677, unsigned int reg, unsigned int *value)
 {
-	unsigned int value = 0;
+	int ret = rt5677_dsp_mode_i2c_read_addr(rt5677, 0x18020000 + reg * 2,
+		value);
 
-	rt5677_dsp_mode_i2c_read_addr(codec, 0x18020000 + reg * 2, &value);
+	*value &= 0xffff;
 
-	return value;
+	return ret;
 }
 
-/**
- * rt5677_dsp_mode_i2c_update_bits - update register on DSP mode.
- * @codec: audio codec
- * @reg: register index.
- * @mask: register mask
- * @value: new value
- *
- *
- * Returns 1 for change, 0 for no change, or negative error code.
- */
-static int rt5677_dsp_mode_i2c_update_bits(struct snd_soc_codec *codec,
-	unsigned int reg, unsigned int mask, unsigned int value)
+static void rt5677_set_dsp_mode(struct snd_soc_codec *codec, bool on)
 {
-	unsigned int old, new;
-	int change, ret;
-
-	ret = rt5677_dsp_mode_i2c_read(codec, reg);
-	if (ret < 0) {
-		dev_err(codec->dev, "Failed to read reg: %d\n", ret);
-		goto err;
-	}
+	struct rt5677_priv *rt5677 = snd_soc_codec_get_drvdata(codec);
 
-	old = ret;
-	new = (old & ~mask) | (value & mask);
-	change = old != new;
-	if (change) {
-		ret = rt5677_dsp_mode_i2c_write(codec, reg, new);
-		if (ret < 0) {
-			dev_err(codec->dev,
-				"Failed to write reg: %d\n", ret);
-			goto err;
-		}
+	if (on) {
+		regmap_update_bits(rt5677->regmap, RT5677_PWR_DSP1, 0x2, 0x2);
+		rt5677->is_dsp_mode = true;
+	} else {
+		regmap_update_bits(rt5677->regmap, RT5677_PWR_DSP1, 0x2, 0x0);
+		rt5677->is_dsp_mode = false;
 	}
-	return change;
-
-err:
-	return ret;
 }
 
 static int rt5677_set_dsp_vad(struct snd_soc_codec *codec, bool on)
@@ -733,9 +772,14 @@  static int rt5677_set_dsp_vad(struct snd_soc_codec *codec, bool on)
 			RT5677_LDO1_SEL_MASK, 0x0);
 		regmap_update_bits(rt5677->regmap, RT5677_PWR_ANLG2,
 			RT5677_PWR_LDO1, RT5677_PWR_LDO1);
-		regmap_write(rt5677->regmap, RT5677_GLB_CLK2, 0x0080);
+		regmap_update_bits(rt5677->regmap, RT5677_GLB_CLK1,
+			RT5677_MCLK_SRC_MASK, RT5677_MCLK2_SRC);
+		regmap_update_bits(rt5677->regmap, RT5677_GLB_CLK2,
+			RT5677_PLL2_PR_SRC_MASK | RT5677_DSP_CLK_SRC_MASK,
+			RT5677_PLL2_PR_SRC_MCLK2 | RT5677_DSP_CLK_SRC_BYPASS);
 		regmap_write(rt5677->regmap, RT5677_PWR_DSP2, 0x07ff);
-		regmap_write(rt5677->regmap, RT5677_PWR_DSP1, 0x07ff);
+		regmap_write(rt5677->regmap, RT5677_PWR_DSP1, 0x07fd);
+		rt5677_set_dsp_mode(codec, true);
 
 		ret = request_firmware(&rt5677->fw1, RT5677_FIRMWARE1,
 			codec->dev);
@@ -751,8 +795,7 @@  static int rt5677_set_dsp_vad(struct snd_soc_codec *codec, bool on)
 			release_firmware(rt5677->fw2);
 		}
 
-		rt5677_dsp_mode_i2c_update_bits(codec, RT5677_PWR_DSP1, 0x1,
-			0x0);
+		regmap_update_bits(rt5677->regmap, RT5677_PWR_DSP1, 0x1, 0x0);
 
 		regcache_cache_bypass(rt5677->regmap, false);
 		regcache_cache_only(rt5677->regmap, true);
@@ -762,9 +805,9 @@  static int rt5677_set_dsp_vad(struct snd_soc_codec *codec, bool on)
 		regcache_cache_only(rt5677->regmap, false);
 		regcache_cache_bypass(rt5677->regmap, true);
 
-		rt5677_dsp_mode_i2c_update_bits(codec, RT5677_PWR_DSP1, 0x1,
-			0x1);
-		rt5677_dsp_mode_i2c_write(codec, RT5677_PWR_DSP1, 0x0001);
+		regmap_update_bits(rt5677->regmap, RT5677_PWR_DSP1, 0x1, 0x1);
+		rt5677_set_dsp_mode(codec, false);
+		regmap_write(rt5677->regmap, RT5677_PWR_DSP1, 0x0001);
 
 		regmap_write(rt5677->regmap, RT5677_RESET, 0x10ec);
 
@@ -3804,6 +3847,32 @@  static int rt5677_resume(struct snd_soc_codec *codec)
 #define rt5677_resume NULL
 #endif
 
+static int rt5677_read(void *context, unsigned int reg, unsigned int *val)
+{
+	struct i2c_client *client = context;
+	struct rt5677_priv *rt5677 = i2c_get_clientdata(client);
+
+	if (rt5677->is_dsp_mode)
+		rt5677_dsp_mode_i2c_read(rt5677, reg, val);
+	else
+		rt5677_hw_read(context, reg, val);
+
+	return 0;
+}
+
+static int rt5677_write(void *context, unsigned int reg, unsigned int val)
+{
+	struct i2c_client *client = context;
+	struct rt5677_priv *rt5677 = i2c_get_clientdata(client);
+
+	if (rt5677->is_dsp_mode)
+		rt5677_dsp_mode_i2c_write(rt5677, reg, val);
+	else
+		rt5677_hw_write(context, reg, val);
+
+	return 0;
+}
+
 #define RT5677_STEREO_RATES SNDRV_PCM_RATE_8000_96000
 #define RT5677_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S20_3LE | \
 			SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S8)
@@ -3938,6 +4007,8 @@  static const struct regmap_config rt5677_regmap = {
 
 	.volatile_reg = rt5677_volatile_register,
 	.readable_reg = rt5677_readable_register,
+	.reg_read = rt5677_read,
+	.reg_write = rt5677_write,
 
 	.cache_type = REGCACHE_RBTREE,
 	.reg_defaults = rt5677_reg,
@@ -4063,6 +4134,7 @@  static int rt5677_i2c_probe(struct i2c_client *i2c,
 		return -ENOMEM;
 
 	i2c_set_clientdata(i2c, rt5677);
+	rt5677->i2c = i2c;
 
 	if (pdata)
 		rt5677->pdata = *pdata;
@@ -4094,7 +4166,7 @@  static int rt5677_i2c_probe(struct i2c_client *i2c,
 		msleep(10);
 	}
 
-	rt5677->regmap = devm_regmap_init_i2c(i2c, &rt5677_regmap);
+	rt5677->regmap = devm_regmap_init(&i2c->dev, NULL, i2c, &rt5677_regmap);
 	if (IS_ERR(rt5677->regmap)) {
 		ret = PTR_ERR(rt5677->regmap);
 		dev_err(&i2c->dev, "Failed to allocate register map: %d\n",
diff --git a/sound/soc/codecs/rt5677.h b/sound/soc/codecs/rt5677.h
index d2c743c..ab81659f 100644
--- a/sound/soc/codecs/rt5677.h
+++ b/sound/soc/codecs/rt5677.h
@@ -1599,6 +1599,7 @@  struct rt5677_priv {
 	struct regmap *regmap;
 	const struct firmware *fw1, *fw2;
 	struct mutex dsp_cmd_lock;
+	struct i2c_client *i2c;
 
 	int sysclk;
 	int sysclk_src;
@@ -1614,6 +1615,7 @@  struct rt5677_priv {
 #endif
 	bool dsp_vad_en;
 	struct regmap_irq_chip_data *irq_data;
+	bool is_dsp_mode;
 };
 
 #endif /* __RT5677_H__ */