[alsa-devel] [PATCH 1/3] mfd: mc13xxx: Don't require lock for simple register I/O
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);
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;
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.
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.
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.
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.
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]
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.
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,
participants (2)
-
Lee Jones -
Mark Brown