[PATCH 1/3] mfd: mc13xxx: Don't require lock for simple register I/O

[PATCH 1/3] mfd: mc13xxx: Don't require lock for simple register I/O

[Mark Brown]

From: Mark Brown <broonie@linaro.org>

Since the conversion to regmap there has been no need for device level
locking for I/O as regmap provides locking so remove the locks.

Signed-off-by: Mark Brown <broonie@linaro.org>
---
 drivers/mfd/mc13xxx-core.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/mfd/mc13xxx-core.c b/drivers/mfd/mc13xxx-core.c
index 2a9b100..dbbf8ee 100644
--- a/drivers/mfd/mc13xxx-core.c
+++ b/drivers/mfd/mc13xxx-core.c
@@ -158,8 +158,6 @@ int mc13xxx_reg_read(struct mc13xxx *mc13xxx, unsigned int offset, u32 *val)
 {
 	int ret;
 
-	BUG_ON(!mutex_is_locked(&mc13xxx->lock));
-
 	if (offset > MC13XXX_NUMREGS)
 		return -EINVAL;
 
@@ -172,8 +170,6 @@ EXPORT_SYMBOL(mc13xxx_reg_read);
 
 int mc13xxx_reg_write(struct mc13xxx *mc13xxx, unsigned int offset, u32 val)
 {
-	BUG_ON(!mutex_is_locked(&mc13xxx->lock));
-
 	dev_vdbg(mc13xxx->dev, "[0x%02x] <- 0x%06x\n", offset, val);
 
 	if (offset > MC13XXX_NUMREGS || val > 0xffffff)
@@ -186,7 +182,6 @@ EXPORT_SYMBOL(mc13xxx_reg_write);
 int mc13xxx_reg_rmw(struct mc13xxx *mc13xxx, unsigned int offset,
 		u32 mask, u32 val)
 {
-	BUG_ON(!mutex_is_locked(&mc13xxx->lock));
 	BUG_ON(val & ~mask);
 	dev_vdbg(mc13xxx->dev, "[0x%02x] <- 0x%06x (mask: 0x%06x)\n",
 			offset, val, mask);
-- 
1.8.4.rc3

[PATCH 2/3] mfd: mc13xxx: Move SPI erratum workaround into SPI I/O function

[Mark Brown]

From: Mark Brown <broonie@linaro.org>

Move the workaround for double sending AUDIO_CODEC and AUDIO_DAC writes
into the SPI core, aiding refactoring to eliminate the ASoC custom I/O
functions and avoiding the extra writes for I2C.

Signed-off-by: Mark Brown <broonie@linaro.org>
---
 drivers/mfd/mc13xxx-spi.c   | 5 +++++
 include/linux/mfd/mc13xxx.h | 7 +++++++
 sound/soc/codecs/mc13783.c  | 4 ----
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/mfd/mc13xxx-spi.c b/drivers/mfd/mc13xxx-spi.c
index 77189da..363c359 100644
--- a/drivers/mfd/mc13xxx-spi.c
+++ b/drivers/mfd/mc13xxx-spi.c
@@ -94,10 +94,15 @@ static int mc13xxx_spi_write(void *context, const void *data, size_t count)
 {
 	struct device *dev = context;
 	struct spi_device *spi = to_spi_device(dev);
+	char *reg = data;
 
 	if (count != 4)
 		return -ENOTSUPP;
 
+	/* include errata fix for spi audio problems */
+	if (*reg == MC13783_AUDIO_CODEC || *reg == MC13783_AUDIO_DAC)
+		spi_write(spi, data, count);
+
 	return spi_write(spi, data, count);
 }
 
diff --git a/include/linux/mfd/mc13xxx.h b/include/linux/mfd/mc13xxx.h
index 41ed592..67c17b5 100644
--- a/include/linux/mfd/mc13xxx.h
+++ b/include/linux/mfd/mc13xxx.h
@@ -41,6 +41,13 @@ int mc13xxx_adc_do_conversion(struct mc13xxx *mc13xxx,
 		unsigned int mode, unsigned int channel,
 		u8 ato, bool atox, unsigned int *sample);
 
+#define MC13783_AUDIO_RX0	36
+#define MC13783_AUDIO_RX1	37
+#define MC13783_AUDIO_TX	38
+#define MC13783_SSI_NETWORK	39
+#define MC13783_AUDIO_CODEC	40
+#define MC13783_AUDIO_DAC	41
+
 #define MC13XXX_IRQ_ADCDONE	0
 #define MC13XXX_IRQ_ADCBISDONE	1
 #define MC13XXX_IRQ_TS		2
diff --git a/sound/soc/codecs/mc13783.c b/sound/soc/codecs/mc13783.c
index ea141e1..4d3c8fd 100644
--- a/sound/soc/codecs/mc13783.c
+++ b/sound/soc/codecs/mc13783.c
@@ -125,10 +125,6 @@ static int mc13783_write(struct snd_soc_codec *codec,
 
 	ret = mc13xxx_reg_write(priv->mc13xxx, reg, value);
 
-	/* include errata fix for spi audio problems */
-	if (reg == MC13783_AUDIO_CODEC || reg == MC13783_AUDIO_DAC)
-		ret = mc13xxx_reg_write(priv->mc13xxx, reg, value);
-
 	mc13xxx_unlock(priv->mc13xxx);
 
 	return ret;
-- 
1.8.4.rc3

Re: [PATCH 2/3] mfd: mc13xxx: Move SPI erratum workaround into SPI I/O function

[Lee Jones]

On Wed, 18 Sep 2013, Mark Brown wrote:

> From: Mark Brown <broonie@linaro.org>
> 
> Move the workaround for double sending AUDIO_CODEC and AUDIO_DAC writes
> into the SPI core, aiding refactoring to eliminate the ASoC custom I/O
> functions and avoiding the extra writes for I2C.
> 
> Signed-off-by: Mark Brown <broonie@linaro.org>
> ---
>  drivers/mfd/mc13xxx-spi.c   | 5 +++++
>  include/linux/mfd/mc13xxx.h | 7 +++++++
>  sound/soc/codecs/mc13783.c  | 4 ----
>  3 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mfd/mc13xxx-spi.c b/drivers/mfd/mc13xxx-spi.c
> index 77189da..363c359 100644
> --- a/drivers/mfd/mc13xxx-spi.c
> +++ b/drivers/mfd/mc13xxx-spi.c
> @@ -94,10 +94,15 @@ static int mc13xxx_spi_write(void *context, const void *data, size_t count)
>  {
>  	struct device *dev = context;
>  	struct spi_device *spi = to_spi_device(dev);
> +	char *reg = data;

I think this requires a cast.

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

Re: [PATCH 2/3] mfd: mc13xxx: Move SPI erratum workaround into SPI I/O function

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 568 bytes --]

On Thu, Sep 19, 2013 at 10:13:18AM +0100, Lee Jones wrote:

Please delete irrelevant context from mails, it makes it easier to find
the new content that's beenn added.

> On Wed, 18 Sep 2013, Mark Brown wrote:

> > @@ -94,10 +94,15 @@ static int mc13xxx_spi_write(void *context, const void *data, size_t count)
> >  {
> >  	struct device *dev = context;
> >  	struct spi_device *spi = to_spi_device(dev);
> > +	char *reg = data;

> I think this requires a cast.

No, you should never need a cast to or from void in C - it probably does
want to be a const char though.

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 2/3] mfd: mc13xxx: Move SPI erratum workaround into SPI I/O function

[Lee Jones]

> Please delete irrelevant context from mails, it makes it easier to find
> the new content that's beenn added.

I did, off the bottom.

I only left 22 lines at the top. You're just being pernickety.

> > On Wed, 18 Sep 2013, Mark Brown wrote:
> 
> > > @@ -94,10 +94,15 @@ static int mc13xxx_spi_write(void *context, const void *data, size_t count)
> > >  {
> > >  	struct device *dev = context;
> > >  	struct spi_device *spi = to_spi_device(dev);
> > > +	char *reg = data;
> 
> > I think this requires a cast.
> 
> No, you should never need a cast to or from void in C - it probably does
> want to be a const char though.

Either way, it needs changing.

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

Re: [PATCH 2/3] mfd: mc13xxx: Move SPI erratum workaround into SPI I/O function

[Mark Brown]

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

On Thu, Sep 19, 2013 at 11:04:34AM +0100, Lee Jones wrote:
> > Please delete irrelevant context from mails, it makes it easier to find
> > the new content that's beenn added.

> I did, off the bottom.

> I only left 22 lines at the top. You're just being pernickety.

I tend to say that when I end up having to page over more than a screen
or so of quote to find relevant stuff, especially if the useful content
is much smaller than the fluff (but really basically whenever it looks
like no trimming was done).

> Either way, it needs changing.

Let's see if someone actually tests the code first though.

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

Re: [PATCH 2/3] mfd: mc13xxx: Move SPI erratum workaround into SPI I/O function

[Lee Jones]

> > Either way, it needs changing.
> 
> Let's see if someone actually tests the code first though.

I've taken the liberty of bulding it, but I don't have the h/w to
actually run the code.

I get:
  drivers/mfd/mc13xxx-spi.c: In function ‘mc13xxx_spi_write’:
  drivers/mfd/mc13xxx-spi.c:97:14: warning: initialisation discards ‘const’ qualifier from pointer target type [enabled by default]

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

Re: [PATCH 2/3] mfd: mc13xxx: Move SPI erratum workaround into SPI I/O function

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 549 bytes --]

On Thu, Sep 19, 2013 at 12:49:10PM +0100, Lee Jones wrote:
> > > Either way, it needs changing.

> > Let's see if someone actually tests the code first though.

> I've taken the liberty of bulding it, but I don't have the h/w to
> actually run the code.

> I get:
>   drivers/mfd/mc13xxx-spi.c: In function ‘mc13xxx_spi_write’:
>   drivers/mfd/mc13xxx-spi.c:97:14: warning: initialisation discards ‘const’ qualifier from pointer target type [enabled by default]

Sure, but like I say let's see if the code actually works first.

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[PATCH 3/3] ASoC: mc13783: Use regmap directly from ASoC

[Mark Brown]

From: Mark Brown <broonie@linaro.org>

As part of a push to remove the register I/O functionality from ASoC (since
it is now duplicated in the regmap API) convert the mc13783 driver to use
regmap directly.

Signed-off-by: Mark Brown <broonie@linaro.org>
---
 sound/soc/codecs/mc13783.c | 55 ++++++++--------------------------------------
 1 file changed, 9 insertions(+), 46 deletions(-)

diff --git a/sound/soc/codecs/mc13783.c b/sound/soc/codecs/mc13783.c
index 4d3c8fd..eedbf05 100644
--- a/sound/soc/codecs/mc13783.c
+++ b/sound/soc/codecs/mc13783.c
@@ -30,16 +30,10 @@
 #include <sound/soc.h>
 #include <sound/initval.h>
 #include <sound/soc-dapm.h>
+#include <linux/regmap.h>
 
 #include "mc13783.h"
 
-#define MC13783_AUDIO_RX0	36
-#define MC13783_AUDIO_RX1	37
-#define MC13783_AUDIO_TX	38
-#define MC13783_SSI_NETWORK	39
-#define MC13783_AUDIO_CODEC	40
-#define MC13783_AUDIO_DAC	41
-
 #define AUDIO_RX0_ALSPEN		(1 << 5)
 #define AUDIO_RX0_ALSPSEL		(1 << 7)
 #define AUDIO_RX0_ADDCDC		(1 << 21)
@@ -95,41 +89,12 @@
 
 struct mc13783_priv {
 	struct mc13xxx *mc13xxx;
+	struct regmap *regmap;
 
 	enum mc13783_ssi_port adc_ssi_port;
 	enum mc13783_ssi_port dac_ssi_port;
 };
 
-static unsigned int mc13783_read(struct snd_soc_codec *codec,
-	unsigned int reg)
-{
-	struct mc13783_priv *priv = snd_soc_codec_get_drvdata(codec);
-	unsigned int value = 0;
-
-	mc13xxx_lock(priv->mc13xxx);
-
-	mc13xxx_reg_read(priv->mc13xxx, reg, &value);
-
-	mc13xxx_unlock(priv->mc13xxx);
-
-	return value;
-}
-
-static int mc13783_write(struct snd_soc_codec *codec,
-	unsigned int reg, unsigned int value)
-{
-	struct mc13783_priv *priv = snd_soc_codec_get_drvdata(codec);
-	int ret;
-
-	mc13xxx_lock(priv->mc13xxx);
-
-	ret = mc13xxx_reg_write(priv->mc13xxx, reg, value);
-
-	mc13xxx_unlock(priv->mc13xxx);
-
-	return ret;
-}
-
 /* Mapping between sample rates and register value */
 static unsigned int mc13783_rates[] = {
 	8000, 11025, 12000, 16000,
@@ -583,8 +548,14 @@ static struct snd_kcontrol_new mc13783_control_list[] = {
 static int mc13783_probe(struct snd_soc_codec *codec)
 {
 	struct mc13783_priv *priv = snd_soc_codec_get_drvdata(codec);
+	int ret;
 
-	mc13xxx_lock(priv->mc13xxx);
+	codec->control_data = dev_get_regmap(codec->dev->parent, NULL);
+	ret = snd_soc_codec_set_cache_io(codec, 8, 24, SND_SOC_REGMAP);
+	if (ret != 0) {
+		dev_err(codec->dev, "Failed to set cache I/O: %d\n", ret);
+		return ret;
+	}
 
 	/* these are the reset values */
 	mc13xxx_reg_write(priv->mc13xxx, MC13783_AUDIO_RX0, 0x25893);
@@ -608,8 +579,6 @@ static int mc13783_probe(struct snd_soc_codec *codec)
 		mc13xxx_reg_rmw(priv->mc13xxx, MC13783_AUDIO_DAC,
 				0, AUDIO_SSI_SEL);
 
-	mc13xxx_unlock(priv->mc13xxx);
-
 	return 0;
 }
 
@@ -617,13 +586,9 @@ static int mc13783_remove(struct snd_soc_codec *codec)
 {
 	struct mc13783_priv *priv = snd_soc_codec_get_drvdata(codec);
 
-	mc13xxx_lock(priv->mc13xxx);
-
 	/* Make sure VAUDIOON is off */
 	mc13xxx_reg_rmw(priv->mc13xxx, MC13783_AUDIO_RX0, 0x3, 0);
 
-	mc13xxx_unlock(priv->mc13xxx);
-
 	return 0;
 }
 
@@ -713,8 +678,6 @@ static struct snd_soc_dai_driver mc13783_dai_sync[] = {
 static struct snd_soc_codec_driver soc_codec_dev_mc13783 = {
 	.probe		= mc13783_probe,
 	.remove		= mc13783_remove,
-	.read		= mc13783_read,
-	.write		= mc13783_write,
 	.controls	= mc13783_control_list,
 	.num_controls	= ARRAY_SIZE(mc13783_control_list),
 	.dapm_widgets	= mc13783_dapm_widgets,
-- 
1.8.4.rc3

[PATCH 1/3] mfd: mc13xxx: Don't require lock for simple register I/O

[Mark Brown]

From: Mark Brown <broonie@linaro.org>

Since the conversion to regmap there has been no need for device level
locking for I/O as regmap provides locking so remove the locks.

Signed-off-by: Mark Brown <broonie@linaro.org>
---
 drivers/mfd/mc13xxx-core.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/mfd/mc13xxx-core.c b/drivers/mfd/mc13xxx-core.c
index 2a9b100..dbbf8ee 100644
--- a/drivers/mfd/mc13xxx-core.c
+++ b/drivers/mfd/mc13xxx-core.c
@@ -158,8 +158,6 @@ int mc13xxx_reg_read(struct mc13xxx *mc13xxx, unsigned int offset, u32 *val)
 {
 	int ret;
 
-	BUG_ON(!mutex_is_locked(&mc13xxx->lock));
-
 	if (offset > MC13XXX_NUMREGS)
 		return -EINVAL;
 
@@ -172,8 +170,6 @@ EXPORT_SYMBOL(mc13xxx_reg_read);
 
 int mc13xxx_reg_write(struct mc13xxx *mc13xxx, unsigned int offset, u32 val)
 {
-	BUG_ON(!mutex_is_locked(&mc13xxx->lock));
-
 	dev_vdbg(mc13xxx->dev, "[0x%02x] <- 0x%06x\n", offset, val);
 
 	if (offset > MC13XXX_NUMREGS || val > 0xffffff)
@@ -186,7 +182,6 @@ EXPORT_SYMBOL(mc13xxx_reg_write);
 int mc13xxx_reg_rmw(struct mc13xxx *mc13xxx, unsigned int offset,
 		u32 mask, u32 val)
 {
-	BUG_ON(!mutex_is_locked(&mc13xxx->lock));
 	BUG_ON(val & ~mask);
 	dev_vdbg(mc13xxx->dev, "[0x%02x] <- 0x%06x (mask: 0x%06x)\n",
 			offset, val, mask);
-- 
1.8.4.rc3

[PATCH 3/3] ASoC: mc13783: Use regmap directly from ASoC

[Mark Brown]

From: Mark Brown <broonie@linaro.org>

As part of a push to remove the register I/O functionality from ASoC (since
it is now duplicated in the regmap API) convert the mc13783 driver to use
regmap directly.

Signed-off-by: Mark Brown <broonie@linaro.org>
---
 sound/soc/codecs/mc13783.c | 55 ++++++++--------------------------------------
 1 file changed, 9 insertions(+), 46 deletions(-)

diff --git a/sound/soc/codecs/mc13783.c b/sound/soc/codecs/mc13783.c
index 4d3c8fd..eedbf05 100644
--- a/sound/soc/codecs/mc13783.c
+++ b/sound/soc/codecs/mc13783.c
@@ -30,16 +30,10 @@
 #include <sound/soc.h>
 #include <sound/initval.h>
 #include <sound/soc-dapm.h>
+#include <linux/regmap.h>
 
 #include "mc13783.h"
 
-#define MC13783_AUDIO_RX0	36
-#define MC13783_AUDIO_RX1	37
-#define MC13783_AUDIO_TX	38
-#define MC13783_SSI_NETWORK	39
-#define MC13783_AUDIO_CODEC	40
-#define MC13783_AUDIO_DAC	41
-
 #define AUDIO_RX0_ALSPEN		(1 << 5)
 #define AUDIO_RX0_ALSPSEL		(1 << 7)
 #define AUDIO_RX0_ADDCDC		(1 << 21)
@@ -95,41 +89,12 @@
 
 struct mc13783_priv {
 	struct mc13xxx *mc13xxx;
+	struct regmap *regmap;
 
 	enum mc13783_ssi_port adc_ssi_port;
 	enum mc13783_ssi_port dac_ssi_port;
 };
 
-static unsigned int mc13783_read(struct snd_soc_codec *codec,
-	unsigned int reg)
-{
-	struct mc13783_priv *priv = snd_soc_codec_get_drvdata(codec);
-	unsigned int value = 0;
-
-	mc13xxx_lock(priv->mc13xxx);
-
-	mc13xxx_reg_read(priv->mc13xxx, reg, &value);
-
-	mc13xxx_unlock(priv->mc13xxx);
-
-	return value;
-}
-
-static int mc13783_write(struct snd_soc_codec *codec,
-	unsigned int reg, unsigned int value)
-{
-	struct mc13783_priv *priv = snd_soc_codec_get_drvdata(codec);
-	int ret;
-
-	mc13xxx_lock(priv->mc13xxx);
-
-	ret = mc13xxx_reg_write(priv->mc13xxx, reg, value);
-
-	mc13xxx_unlock(priv->mc13xxx);
-
-	return ret;
-}
-
 /* Mapping between sample rates and register value */
 static unsigned int mc13783_rates[] = {
 	8000, 11025, 12000, 16000,
@@ -583,8 +548,14 @@ static struct snd_kcontrol_new mc13783_control_list[] = {
 static int mc13783_probe(struct snd_soc_codec *codec)
 {
 	struct mc13783_priv *priv = snd_soc_codec_get_drvdata(codec);
+	int ret;
 
-	mc13xxx_lock(priv->mc13xxx);
+	codec->control_data = dev_get_regmap(codec->dev->parent, NULL);
+	ret = snd_soc_codec_set_cache_io(codec, 8, 24, SND_SOC_REGMAP);
+	if (ret != 0) {
+		dev_err(codec->dev, "Failed to set cache I/O: %d\n", ret);
+		return ret;
+	}
 
 	/* these are the reset values */
 	mc13xxx_reg_write(priv->mc13xxx, MC13783_AUDIO_RX0, 0x25893);
@@ -608,8 +579,6 @@ static int mc13783_probe(struct snd_soc_codec *codec)
 		mc13xxx_reg_rmw(priv->mc13xxx, MC13783_AUDIO_DAC,
 				0, AUDIO_SSI_SEL);
 
-	mc13xxx_unlock(priv->mc13xxx);
-
 	return 0;
 }
 
@@ -617,13 +586,9 @@ static int mc13783_remove(struct snd_soc_codec *codec)
 {
 	struct mc13783_priv *priv = snd_soc_codec_get_drvdata(codec);
 
-	mc13xxx_lock(priv->mc13xxx);
-
 	/* Make sure VAUDIOON is off */
 	mc13xxx_reg_rmw(priv->mc13xxx, MC13783_AUDIO_RX0, 0x3, 0);
 
-	mc13xxx_unlock(priv->mc13xxx);
-
 	return 0;
 }
 
@@ -713,8 +678,6 @@ static struct snd_soc_dai_driver mc13783_dai_sync[] = {
 static struct snd_soc_codec_driver soc_codec_dev_mc13783 = {
 	.probe		= mc13783_probe,
 	.remove		= mc13783_remove,
-	.read		= mc13783_read,
-	.write		= mc13783_write,
 	.controls	= mc13783_control_list,
 	.num_controls	= ARRAY_SIZE(mc13783_control_list),
 	.dapm_widgets	= mc13783_dapm_widgets,
-- 
1.8.4.rc3