* [PATCH 1/2] ASoC: sta32x: Move regulator acquisition to I2C probe @ 2012-09-10 3:01 Mark Brown 2012-09-10 3:01 ` [PATCH 2/2] ASoC: sta32x: Convert to regmap Mark Brown 2012-09-10 9:00 ` [PATCH 1/2] ASoC: sta32x: Move regulator acquisition to I2C probe Johannes Stezenbach 0 siblings, 2 replies; 5+ messages in thread From: Mark Brown @ 2012-09-10 3:01 UTC (permalink / raw) To: Liam Girdwood, Johannes Stezenbach; +Cc: alsa-devel, Mark Brown This is better style as it ensures we don't try to do the ASoC probe without required resources. Also convert to devm_ while we're at it, saving a bit of code, and fix a leak of enable on error. Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> --- sound/soc/codecs/sta32x.c | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/sound/soc/codecs/sta32x.c b/sound/soc/codecs/sta32x.c index 51b7313..039597e 100644 --- a/sound/soc/codecs/sta32x.c +++ b/sound/soc/codecs/sta32x.c @@ -825,22 +825,11 @@ static int sta32x_probe(struct snd_soc_codec *codec) sta32x->codec = codec; sta32x->pdata = dev_get_platdata(codec->dev); - /* regulators */ - for (i = 0; i < ARRAY_SIZE(sta32x->supplies); i++) - sta32x->supplies[i].supply = sta32x_supply_names[i]; - - ret = regulator_bulk_get(codec->dev, ARRAY_SIZE(sta32x->supplies), - sta32x->supplies); - if (ret != 0) { - dev_err(codec->dev, "Failed to request supplies: %d\n", ret); - goto err; - } - ret = regulator_bulk_enable(ARRAY_SIZE(sta32x->supplies), sta32x->supplies); if (ret != 0) { dev_err(codec->dev, "Failed to enable supplies: %d\n", ret); - goto err_get; + return ret; } /* Tell ASoC what kind of I/O to use to read the registers. ASoC will @@ -849,7 +838,7 @@ static int sta32x_probe(struct snd_soc_codec *codec) ret = snd_soc_codec_set_cache_io(codec, 8, 8, SND_SOC_I2C); if (ret < 0) { dev_err(codec->dev, "failed to set cache I/O (ret=%i)\n", ret); - return ret; + goto err; } /* Chip documentation explicitly requires that the reset values @@ -915,9 +904,8 @@ static int sta32x_probe(struct snd_soc_codec *codec) return 0; -err_get: - regulator_bulk_free(ARRAY_SIZE(sta32x->supplies), sta32x->supplies); err: + regulator_bulk_disable(ARRAY_SIZE(sta32x->supplies), sta32x->supplies); return ret; } @@ -928,7 +916,6 @@ static int sta32x_remove(struct snd_soc_codec *codec) sta32x_watchdog_stop(sta32x); sta32x_set_bias_level(codec, SND_SOC_BIAS_OFF); regulator_bulk_disable(ARRAY_SIZE(sta32x->supplies), sta32x->supplies); - regulator_bulk_free(ARRAY_SIZE(sta32x->supplies), sta32x->supplies); return 0; } @@ -966,13 +953,24 @@ static __devinit int sta32x_i2c_probe(struct i2c_client *i2c, const struct i2c_device_id *id) { struct sta32x_priv *sta32x; - int ret; + int ret, i; sta32x = devm_kzalloc(&i2c->dev, sizeof(struct sta32x_priv), GFP_KERNEL); if (!sta32x) return -ENOMEM; + /* regulators */ + for (i = 0; i < ARRAY_SIZE(sta32x->supplies); i++) + sta32x->supplies[i].supply = sta32x_supply_names[i]; + + ret = devm_regulator_bulk_get(&i2c->dev, ARRAY_SIZE(sta32x->supplies), + sta32x->supplies); + if (ret != 0) { + dev_err(&i2c->dev, "Failed to request supplies: %d\n", ret); + return ret; + } + i2c_set_clientdata(i2c, sta32x); ret = snd_soc_register_codec(&i2c->dev, &sta32x_codec, &sta32x_dai, 1); -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] ASoC: sta32x: Convert to regmap 2012-09-10 3:01 [PATCH 1/2] ASoC: sta32x: Move regulator acquisition to I2C probe Mark Brown @ 2012-09-10 3:01 ` Mark Brown 2012-09-10 7:49 ` Mark Brown 2012-09-10 9:10 ` Johannes Stezenbach 2012-09-10 9:00 ` [PATCH 1/2] ASoC: sta32x: Move regulator acquisition to I2C probe Johannes Stezenbach 1 sibling, 2 replies; 5+ messages in thread From: Mark Brown @ 2012-09-10 3:01 UTC (permalink / raw) To: Liam Girdwood, Johannes Stezenbach; +Cc: alsa-devel, Mark Brown Long term all drivers should be using regmap directly. This is more idiomatic and moves us towards the removal of the ASoC level cache code. The initialiasation of reserved register bits in probe() is slightly odd as the defaults being written don't appear to match the silicon defaults but the new code should have the same effect as the old code. The watchdog code will now unconditionally do a mute and unmute when resyncing but since we only sync when we are very sure there is something to sync this should have no impact. Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> --- sound/soc/codecs/sta32x.c | 106 +++++++++++++++++++++++++++++++++------------ 1 file changed, 79 insertions(+), 27 deletions(-) diff --git a/sound/soc/codecs/sta32x.c b/sound/soc/codecs/sta32x.c index 039597e..744ec7a 100644 --- a/sound/soc/codecs/sta32x.c +++ b/sound/soc/codecs/sta32x.c @@ -24,6 +24,7 @@ #include <linux/delay.h> #include <linux/pm.h> #include <linux/i2c.h> +#include <linux/regmap.h> #include <linux/regulator/consumer.h> #include <linux/slab.h> #include <linux/workqueue.h> @@ -55,12 +56,50 @@ SNDRV_PCM_FMTBIT_S32_LE | SNDRV_PCM_FMTBIT_S32_BE) /* Power-up register defaults */ -static const u8 sta32x_regs[STA32X_REGISTER_COUNT] = { - 0x63, 0x80, 0xc2, 0x40, 0xc2, 0x5c, 0x10, 0xff, 0x60, 0x60, - 0x60, 0x80, 0x00, 0x00, 0x00, 0x40, 0x80, 0x77, 0x6a, 0x69, - 0x6a, 0x69, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x2d, - 0xc0, 0xf3, 0x33, 0x00, 0x0c, +static const struct reg_default sta32x_regs[] = { + { 0x0, 0x63 }, + { 0x1, 0x80 }, + { 0x2, 0xc2 }, + { 0x3, 0x40 }, + { 0x4, 0xc2 }, + { 0x5, 0x5c }, + { 0x6, 0x10 }, + { 0x7, 0xff }, + { 0x8, 0x60 }, + { 0x9, 0x60 }, + { 0xa, 0x60 }, + { 0xb, 0x80 }, + { 0xc, 0x00 }, + { 0xd, 0x00 }, + { 0xe, 0x00 }, + { 0xf, 0x40 }, + { 0x10, 0x80 }, + { 0x11, 0x77 }, + { 0x12, 0x6a }, + { 0x13, 0x69 }, + { 0x14, 0x6a }, + { 0x15, 0x69 }, + { 0x16, 0x00 }, + { 0x17, 0x00 }, + { 0x18, 0x00 }, + { 0x19, 0x00 }, + { 0x1a, 0x00 }, + { 0x1b, 0x00 }, + { 0x1c, 0x00 }, + { 0x1d, 0x00 }, + { 0x1e, 0x00 }, + { 0x1f, 0x00 }, + { 0x20, 0x00 }, + { 0x21, 0x00 }, + { 0x22, 0x00 }, + { 0x23, 0x00 }, + { 0x24, 0x00 }, + { 0x25, 0x00 }, + { 0x26, 0x00 }, + { 0x27, 0x2d }, + { 0x28, 0xc0 }, + { 0x2b, 0x00 }, + { 0x2c, 0x0c }, }; /* regulator power supply names */ @@ -72,6 +111,7 @@ static const char *sta32x_supply_names[] = { /* codec private data */ struct sta32x_priv { + struct regmap *regmap; struct regulator_bulk_data supplies[ARRAY_SIZE(sta32x_supply_names)]; struct snd_soc_codec *codec; struct sta32x_platform_data *pdata; @@ -291,17 +331,15 @@ static int sta32x_sync_coef_shadow(struct snd_soc_codec *codec) static int sta32x_cache_sync(struct snd_soc_codec *codec) { + struct sta32x_priv *sta32x = codec->control_data; unsigned int mute; int rc; - if (!codec->cache_sync) - return 0; - /* mute during register sync */ mute = snd_soc_read(codec, STA32X_MMUTE); snd_soc_write(codec, STA32X_MMUTE, mute | STA32X_MMUTE_MMUTE); sta32x_sync_coef_shadow(codec); - rc = snd_soc_cache_sync(codec); + rc = regcache_sync(sta32x->regmap); snd_soc_write(codec, STA32X_MMUTE, mute); return rc; } @@ -316,11 +354,11 @@ static void sta32x_watchdog(struct work_struct *work) /* check if sta32x has reset itself */ confa_cached = snd_soc_read(codec, STA32X_CONFA); - codec->cache_bypass = 1; + regcache_cache_bypass(sta32x->regmap, true); confa = snd_soc_read(codec, STA32X_CONFA); - codec->cache_bypass = 0; + regcache_cache_bypass(sta32x->regmap, false); if (confa != confa_cached) { - codec->cache_sync = 1; + regcache_mark_dirty(sta32x->regmap); sta32x_cache_sync(codec); } @@ -835,7 +873,8 @@ static int sta32x_probe(struct snd_soc_codec *codec) /* Tell ASoC what kind of I/O to use to read the registers. ASoC will * then do the I2C transactions itself. */ - ret = snd_soc_codec_set_cache_io(codec, 8, 8, SND_SOC_I2C); + codec->control_data = sta32x->regmap; + ret = snd_soc_codec_set_cache_io(codec, 8, 8, SND_SOC_REGMAP); if (ret < 0) { dev_err(codec->dev, "failed to set cache I/O (ret=%i)\n", ret); goto err; @@ -847,13 +886,15 @@ static int sta32x_probe(struct snd_soc_codec *codec) * so the write to the these registers are suppressed by the cache * restore code when it skips writes of default registers. */ - snd_soc_cache_write(codec, STA32X_CONFC, 0xc2); - snd_soc_cache_write(codec, STA32X_CONFE, 0xc2); - snd_soc_cache_write(codec, STA32X_CONFF, 0x5c); - snd_soc_cache_write(codec, STA32X_MMUTE, 0x10); - snd_soc_cache_write(codec, STA32X_AUTO1, 0x60); - snd_soc_cache_write(codec, STA32X_AUTO3, 0x00); - snd_soc_cache_write(codec, STA32X_C3CFG, 0x40); + regcache_cache_only(sta32x->regmap, true); + snd_soc_write(codec, STA32X_CONFC, 0xc2); + snd_soc_write(codec, STA32X_CONFE, 0xc2); + snd_soc_write(codec, STA32X_CONFF, 0x5c); + snd_soc_write(codec, STA32X_MMUTE, 0x10); + snd_soc_write(codec, STA32X_AUTO1, 0x60); + snd_soc_write(codec, STA32X_AUTO3, 0x00); + snd_soc_write(codec, STA32X_C3CFG, 0x40); + regcache_cache_only(sta32x->regmap, false); /* set thermal warning adjustment and recovery */ if (!(sta32x->pdata->thermal_conf & STA32X_THERMAL_ADJUSTMENT_ENABLE)) @@ -920,8 +961,7 @@ static int sta32x_remove(struct snd_soc_codec *codec) return 0; } -static int sta32x_reg_is_volatile(struct snd_soc_codec *codec, - unsigned int reg) +static bool sta32x_reg_is_volatile(struct device *dev, unsigned int reg) { switch (reg) { case STA32X_CONFA ... STA32X_L2ATRT: @@ -936,10 +976,6 @@ static const struct snd_soc_codec_driver sta32x_codec = { .remove = sta32x_remove, .suspend = sta32x_suspend, .resume = sta32x_resume, - .reg_cache_size = STA32X_REGISTER_COUNT, - .reg_word_size = sizeof(u8), - .reg_cache_default = sta32x_regs, - .volatile_register = sta32x_reg_is_volatile, .set_bias_level = sta32x_set_bias_level, .controls = sta32x_snd_controls, .num_controls = ARRAY_SIZE(sta32x_snd_controls), @@ -949,6 +985,15 @@ static const struct snd_soc_codec_driver sta32x_codec = { .num_dapm_routes = ARRAY_SIZE(sta32x_dapm_routes), }; +static const struct regmap_config sta32x_regmap = { + .reg_bits = 8, + .val_bits = 8, + .max_register = STA32X_FDRC2, + .reg_defaults = sta32x_regs, + .num_reg_defaults = ARRAY_SIZE(sta32x_regs), + .volatile_reg = sta32x_reg_is_volatile, +}; + static __devinit int sta32x_i2c_probe(struct i2c_client *i2c, const struct i2c_device_id *id) { @@ -971,6 +1016,13 @@ static __devinit int sta32x_i2c_probe(struct i2c_client *i2c, return ret; } + sta32x->regmap = devm_regmap_init_i2c(i2c, &sta32x_regmap); + if (IS_ERR(sta32x->regmap)) { + ret = PTR_ERR(sta32x->regmap); + dev_err(&i2c->dev, "Failed to init regmap: %d\n", ret); + return ret; + } + i2c_set_clientdata(i2c, sta32x); ret = snd_soc_register_codec(&i2c->dev, &sta32x_codec, &sta32x_dai, 1); -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] ASoC: sta32x: Convert to regmap 2012-09-10 3:01 ` [PATCH 2/2] ASoC: sta32x: Convert to regmap Mark Brown @ 2012-09-10 7:49 ` Mark Brown 2012-09-10 9:10 ` Johannes Stezenbach 1 sibling, 0 replies; 5+ messages in thread From: Mark Brown @ 2012-09-10 7:49 UTC (permalink / raw) To: Liam Girdwood, Johannes Stezenbach; +Cc: alsa-devel On Mon, Sep 10, 2012 at 11:01:43AM +0800, Mark Brown wrote: > Long term all drivers should be using regmap directly. This is more > idiomatic and moves us towards the removal of the ASoC level cache > code. This also needs .cache_type = REGCACHE_RBTREE in the regmap config. I've fixed up locally. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] ASoC: sta32x: Convert to regmap 2012-09-10 3:01 ` [PATCH 2/2] ASoC: sta32x: Convert to regmap Mark Brown 2012-09-10 7:49 ` Mark Brown @ 2012-09-10 9:10 ` Johannes Stezenbach 1 sibling, 0 replies; 5+ messages in thread From: Johannes Stezenbach @ 2012-09-10 9:10 UTC (permalink / raw) To: Mark Brown; +Cc: alsa-devel, Sven Neumann, Liam Girdwood, Daniel Mack (added some Cc:) On Mon, Sep 10, 2012 at 11:01:43AM +0800, Mark Brown wrote: > Long term all drivers should be using regmap directly. This is more > idiomatic and moves us towards the removal of the ASoC level cache > code. > > The initialiasation of reserved register bits in probe() is slightly odd > as the defaults being written don't appear to match the silicon defaults > but the new code should have the same effect as the old code. > > The watchdog code will now unconditionally do a mute and unmute when > resyncing but since we only sync when we are very sure there is something > to sync this should have no impact. > > Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> Looks good to me, but I wouldn't have spotted the missing ".cache_type = REGCACHE_RBTREE" that you noticed yourself thus my Acked-by doesn't mean that much. You can add it anyway if you want: Acked-by: Johannes Stezenbach <js@sig21.net> Thanks, Johannes > --- > sound/soc/codecs/sta32x.c | 106 +++++++++++++++++++++++++++++++++------------ > 1 file changed, 79 insertions(+), 27 deletions(-) > > diff --git a/sound/soc/codecs/sta32x.c b/sound/soc/codecs/sta32x.c > index 039597e..744ec7a 100644 > --- a/sound/soc/codecs/sta32x.c > +++ b/sound/soc/codecs/sta32x.c > @@ -24,6 +24,7 @@ > #include <linux/delay.h> > #include <linux/pm.h> > #include <linux/i2c.h> > +#include <linux/regmap.h> > #include <linux/regulator/consumer.h> > #include <linux/slab.h> > #include <linux/workqueue.h> > @@ -55,12 +56,50 @@ > SNDRV_PCM_FMTBIT_S32_LE | SNDRV_PCM_FMTBIT_S32_BE) > > /* Power-up register defaults */ > -static const u8 sta32x_regs[STA32X_REGISTER_COUNT] = { > - 0x63, 0x80, 0xc2, 0x40, 0xc2, 0x5c, 0x10, 0xff, 0x60, 0x60, > - 0x60, 0x80, 0x00, 0x00, 0x00, 0x40, 0x80, 0x77, 0x6a, 0x69, > - 0x6a, 0x69, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x2d, > - 0xc0, 0xf3, 0x33, 0x00, 0x0c, > +static const struct reg_default sta32x_regs[] = { > + { 0x0, 0x63 }, > + { 0x1, 0x80 }, > + { 0x2, 0xc2 }, > + { 0x3, 0x40 }, > + { 0x4, 0xc2 }, > + { 0x5, 0x5c }, > + { 0x6, 0x10 }, > + { 0x7, 0xff }, > + { 0x8, 0x60 }, > + { 0x9, 0x60 }, > + { 0xa, 0x60 }, > + { 0xb, 0x80 }, > + { 0xc, 0x00 }, > + { 0xd, 0x00 }, > + { 0xe, 0x00 }, > + { 0xf, 0x40 }, > + { 0x10, 0x80 }, > + { 0x11, 0x77 }, > + { 0x12, 0x6a }, > + { 0x13, 0x69 }, > + { 0x14, 0x6a }, > + { 0x15, 0x69 }, > + { 0x16, 0x00 }, > + { 0x17, 0x00 }, > + { 0x18, 0x00 }, > + { 0x19, 0x00 }, > + { 0x1a, 0x00 }, > + { 0x1b, 0x00 }, > + { 0x1c, 0x00 }, > + { 0x1d, 0x00 }, > + { 0x1e, 0x00 }, > + { 0x1f, 0x00 }, > + { 0x20, 0x00 }, > + { 0x21, 0x00 }, > + { 0x22, 0x00 }, > + { 0x23, 0x00 }, > + { 0x24, 0x00 }, > + { 0x25, 0x00 }, > + { 0x26, 0x00 }, > + { 0x27, 0x2d }, > + { 0x28, 0xc0 }, > + { 0x2b, 0x00 }, > + { 0x2c, 0x0c }, > }; > > /* regulator power supply names */ > @@ -72,6 +111,7 @@ static const char *sta32x_supply_names[] = { > > /* codec private data */ > struct sta32x_priv { > + struct regmap *regmap; > struct regulator_bulk_data supplies[ARRAY_SIZE(sta32x_supply_names)]; > struct snd_soc_codec *codec; > struct sta32x_platform_data *pdata; > @@ -291,17 +331,15 @@ static int sta32x_sync_coef_shadow(struct snd_soc_codec *codec) > > static int sta32x_cache_sync(struct snd_soc_codec *codec) > { > + struct sta32x_priv *sta32x = codec->control_data; > unsigned int mute; > int rc; > > - if (!codec->cache_sync) > - return 0; > - > /* mute during register sync */ > mute = snd_soc_read(codec, STA32X_MMUTE); > snd_soc_write(codec, STA32X_MMUTE, mute | STA32X_MMUTE_MMUTE); > sta32x_sync_coef_shadow(codec); > - rc = snd_soc_cache_sync(codec); > + rc = regcache_sync(sta32x->regmap); > snd_soc_write(codec, STA32X_MMUTE, mute); > return rc; > } > @@ -316,11 +354,11 @@ static void sta32x_watchdog(struct work_struct *work) > > /* check if sta32x has reset itself */ > confa_cached = snd_soc_read(codec, STA32X_CONFA); > - codec->cache_bypass = 1; > + regcache_cache_bypass(sta32x->regmap, true); > confa = snd_soc_read(codec, STA32X_CONFA); > - codec->cache_bypass = 0; > + regcache_cache_bypass(sta32x->regmap, false); > if (confa != confa_cached) { > - codec->cache_sync = 1; > + regcache_mark_dirty(sta32x->regmap); > sta32x_cache_sync(codec); > } > > @@ -835,7 +873,8 @@ static int sta32x_probe(struct snd_soc_codec *codec) > /* Tell ASoC what kind of I/O to use to read the registers. ASoC will > * then do the I2C transactions itself. > */ > - ret = snd_soc_codec_set_cache_io(codec, 8, 8, SND_SOC_I2C); > + codec->control_data = sta32x->regmap; > + ret = snd_soc_codec_set_cache_io(codec, 8, 8, SND_SOC_REGMAP); > if (ret < 0) { > dev_err(codec->dev, "failed to set cache I/O (ret=%i)\n", ret); > goto err; > @@ -847,13 +886,15 @@ static int sta32x_probe(struct snd_soc_codec *codec) > * so the write to the these registers are suppressed by the cache > * restore code when it skips writes of default registers. > */ > - snd_soc_cache_write(codec, STA32X_CONFC, 0xc2); > - snd_soc_cache_write(codec, STA32X_CONFE, 0xc2); > - snd_soc_cache_write(codec, STA32X_CONFF, 0x5c); > - snd_soc_cache_write(codec, STA32X_MMUTE, 0x10); > - snd_soc_cache_write(codec, STA32X_AUTO1, 0x60); > - snd_soc_cache_write(codec, STA32X_AUTO3, 0x00); > - snd_soc_cache_write(codec, STA32X_C3CFG, 0x40); > + regcache_cache_only(sta32x->regmap, true); > + snd_soc_write(codec, STA32X_CONFC, 0xc2); > + snd_soc_write(codec, STA32X_CONFE, 0xc2); > + snd_soc_write(codec, STA32X_CONFF, 0x5c); > + snd_soc_write(codec, STA32X_MMUTE, 0x10); > + snd_soc_write(codec, STA32X_AUTO1, 0x60); > + snd_soc_write(codec, STA32X_AUTO3, 0x00); > + snd_soc_write(codec, STA32X_C3CFG, 0x40); > + regcache_cache_only(sta32x->regmap, false); > > /* set thermal warning adjustment and recovery */ > if (!(sta32x->pdata->thermal_conf & STA32X_THERMAL_ADJUSTMENT_ENABLE)) > @@ -920,8 +961,7 @@ static int sta32x_remove(struct snd_soc_codec *codec) > return 0; > } > > -static int sta32x_reg_is_volatile(struct snd_soc_codec *codec, > - unsigned int reg) > +static bool sta32x_reg_is_volatile(struct device *dev, unsigned int reg) > { > switch (reg) { > case STA32X_CONFA ... STA32X_L2ATRT: > @@ -936,10 +976,6 @@ static const struct snd_soc_codec_driver sta32x_codec = { > .remove = sta32x_remove, > .suspend = sta32x_suspend, > .resume = sta32x_resume, > - .reg_cache_size = STA32X_REGISTER_COUNT, > - .reg_word_size = sizeof(u8), > - .reg_cache_default = sta32x_regs, > - .volatile_register = sta32x_reg_is_volatile, > .set_bias_level = sta32x_set_bias_level, > .controls = sta32x_snd_controls, > .num_controls = ARRAY_SIZE(sta32x_snd_controls), > @@ -949,6 +985,15 @@ static const struct snd_soc_codec_driver sta32x_codec = { > .num_dapm_routes = ARRAY_SIZE(sta32x_dapm_routes), > }; > > +static const struct regmap_config sta32x_regmap = { > + .reg_bits = 8, > + .val_bits = 8, > + .max_register = STA32X_FDRC2, > + .reg_defaults = sta32x_regs, > + .num_reg_defaults = ARRAY_SIZE(sta32x_regs), > + .volatile_reg = sta32x_reg_is_volatile, > +}; > + > static __devinit int sta32x_i2c_probe(struct i2c_client *i2c, > const struct i2c_device_id *id) > { > @@ -971,6 +1016,13 @@ static __devinit int sta32x_i2c_probe(struct i2c_client *i2c, > return ret; > } > > + sta32x->regmap = devm_regmap_init_i2c(i2c, &sta32x_regmap); > + if (IS_ERR(sta32x->regmap)) { > + ret = PTR_ERR(sta32x->regmap); > + dev_err(&i2c->dev, "Failed to init regmap: %d\n", ret); > + return ret; > + } > + > i2c_set_clientdata(i2c, sta32x); > > ret = snd_soc_register_codec(&i2c->dev, &sta32x_codec, &sta32x_dai, 1); > -- > 1.7.10.4 > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] ASoC: sta32x: Move regulator acquisition to I2C probe 2012-09-10 3:01 [PATCH 1/2] ASoC: sta32x: Move regulator acquisition to I2C probe Mark Brown 2012-09-10 3:01 ` [PATCH 2/2] ASoC: sta32x: Convert to regmap Mark Brown @ 2012-09-10 9:00 ` Johannes Stezenbach 1 sibling, 0 replies; 5+ messages in thread From: Johannes Stezenbach @ 2012-09-10 9:00 UTC (permalink / raw) To: Mark Brown; +Cc: alsa-devel, Sven Neumann, Liam Girdwood, Daniel Mack (add some Cc:) On Mon, Sep 10, 2012 at 11:01:42AM +0800, Mark Brown wrote: > This is better style as it ensures we don't try to do the ASoC probe > without required resources. Also convert to devm_ while we're at it, > saving a bit of code, and fix a leak of enable on error. > > Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> I don't have the hw to test it, but it looks good. If you want you can add Acked-by: Johannes Stezenbach <js@sig21.net>. Thanks, Johannes > --- > sound/soc/codecs/sta32x.c | 32 +++++++++++++++----------------- > 1 file changed, 15 insertions(+), 17 deletions(-) > > diff --git a/sound/soc/codecs/sta32x.c b/sound/soc/codecs/sta32x.c > index 51b7313..039597e 100644 > --- a/sound/soc/codecs/sta32x.c > +++ b/sound/soc/codecs/sta32x.c > @@ -825,22 +825,11 @@ static int sta32x_probe(struct snd_soc_codec *codec) > sta32x->codec = codec; > sta32x->pdata = dev_get_platdata(codec->dev); > > - /* regulators */ > - for (i = 0; i < ARRAY_SIZE(sta32x->supplies); i++) > - sta32x->supplies[i].supply = sta32x_supply_names[i]; > - > - ret = regulator_bulk_get(codec->dev, ARRAY_SIZE(sta32x->supplies), > - sta32x->supplies); > - if (ret != 0) { > - dev_err(codec->dev, "Failed to request supplies: %d\n", ret); > - goto err; > - } > - > ret = regulator_bulk_enable(ARRAY_SIZE(sta32x->supplies), > sta32x->supplies); > if (ret != 0) { > dev_err(codec->dev, "Failed to enable supplies: %d\n", ret); > - goto err_get; > + return ret; > } > > /* Tell ASoC what kind of I/O to use to read the registers. ASoC will > @@ -849,7 +838,7 @@ static int sta32x_probe(struct snd_soc_codec *codec) > ret = snd_soc_codec_set_cache_io(codec, 8, 8, SND_SOC_I2C); > if (ret < 0) { > dev_err(codec->dev, "failed to set cache I/O (ret=%i)\n", ret); > - return ret; > + goto err; > } > > /* Chip documentation explicitly requires that the reset values > @@ -915,9 +904,8 @@ static int sta32x_probe(struct snd_soc_codec *codec) > > return 0; > > -err_get: > - regulator_bulk_free(ARRAY_SIZE(sta32x->supplies), sta32x->supplies); > err: > + regulator_bulk_disable(ARRAY_SIZE(sta32x->supplies), sta32x->supplies); > return ret; > } > > @@ -928,7 +916,6 @@ static int sta32x_remove(struct snd_soc_codec *codec) > sta32x_watchdog_stop(sta32x); > sta32x_set_bias_level(codec, SND_SOC_BIAS_OFF); > regulator_bulk_disable(ARRAY_SIZE(sta32x->supplies), sta32x->supplies); > - regulator_bulk_free(ARRAY_SIZE(sta32x->supplies), sta32x->supplies); > > return 0; > } > @@ -966,13 +953,24 @@ static __devinit int sta32x_i2c_probe(struct i2c_client *i2c, > const struct i2c_device_id *id) > { > struct sta32x_priv *sta32x; > - int ret; > + int ret, i; > > sta32x = devm_kzalloc(&i2c->dev, sizeof(struct sta32x_priv), > GFP_KERNEL); > if (!sta32x) > return -ENOMEM; > > + /* regulators */ > + for (i = 0; i < ARRAY_SIZE(sta32x->supplies); i++) > + sta32x->supplies[i].supply = sta32x_supply_names[i]; > + > + ret = devm_regulator_bulk_get(&i2c->dev, ARRAY_SIZE(sta32x->supplies), > + sta32x->supplies); > + if (ret != 0) { > + dev_err(&i2c->dev, "Failed to request supplies: %d\n", ret); > + return ret; > + } > + > i2c_set_clientdata(i2c, sta32x); > > ret = snd_soc_register_codec(&i2c->dev, &sta32x_codec, &sta32x_dai, 1); > -- > 1.7.10.4 > > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-09-10 9:10 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-09-10 3:01 [PATCH 1/2] ASoC: sta32x: Move regulator acquisition to I2C probe Mark Brown 2012-09-10 3:01 ` [PATCH 2/2] ASoC: sta32x: Convert to regmap Mark Brown 2012-09-10 7:49 ` Mark Brown 2012-09-10 9:10 ` Johannes Stezenbach 2012-09-10 9:00 ` [PATCH 1/2] ASoC: sta32x: Move regulator acquisition to I2C probe Johannes Stezenbach
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.