[alsa-devel] [PATCH v2 0/3] ASoC: sta32x improvements
Hi,
this is a rework of the "ASoC: sta32x: add workaround for ESD reset issue" patch posted on Nov 9. Since the review of that patch discovered a general issue with the coefficient RAM, I split the patch into three parts: the first fixes the coef reload after power up, the second adds the missing platform configuration (in place of hard-coded values), and the third has the ESD workaround.
I think I followed all review comments, except for the ESD workaround where I decided it is much clearer to call sta32x_cache_sync() directly instead of messing around with sta32x_set_bias_level().
The patches apply to git://opensource.wolfsonmicro.com/linux-2.6-asoc.git for-3.2 and for-3.3. checkpatch.pl result is clean.
include/sound/sta32x.h | 35 +++++++++++ sound/soc/codecs/sta32x.c | 143 +++++++++++++++++++++++++++++++++++++++++---- sound/soc/codecs/sta32x.h | 1 + 3 files changed, 168 insertions(+), 11 deletions(-)
Thanks Johannes
The coefficient RAM must be saved in a shadow so it can be restored when the codec is powered on using regulator_bulk_enable().
Cc: Sven Neumann s.neumann@raumfeld.com Cc: Daniel Mack daniel@zonque.org Signed-off-by: Johannes Stezenbach js@sig21.net --- sound/soc/codecs/sta32x.c | 63 ++++++++++++++++++++++++++++++++++++++++++++- sound/soc/codecs/sta32x.h | 1 + 2 files changed, 63 insertions(+), 1 deletions(-)
diff --git a/sound/soc/codecs/sta32x.c b/sound/soc/codecs/sta32x.c index bb82408..d2f3715 100644 --- a/sound/soc/codecs/sta32x.c +++ b/sound/soc/codecs/sta32x.c @@ -76,6 +76,8 @@ struct sta32x_priv {
unsigned int mclk; unsigned int format; + + u32 coef_shadow[STA32X_COEF_COUNT]; };
static const DECLARE_TLV_DB_SCALE(mvol_tlv, -12700, 50, 1); @@ -227,6 +229,7 @@ static int sta32x_coefficient_put(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) { struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol); + struct sta32x_priv *sta32x = snd_soc_codec_get_drvdata(codec); int numcoef = kcontrol->private_value >> 16; int index = kcontrol->private_value & 0xffff; unsigned int cfud; @@ -239,6 +242,11 @@ static int sta32x_coefficient_put(struct snd_kcontrol *kcontrol, snd_soc_write(codec, STA32X_CFUD, cfud);
snd_soc_write(codec, STA32X_CFADDR2, index); + for (i = 0; i < numcoef && (index + i < STA32X_COEF_COUNT); i++) + sta32x->coef_shadow[index + i] = + (ucontrol->value.bytes.data[3 * i] << 16) + | (ucontrol->value.bytes.data[3 * i + 1] << 8) + | (ucontrol->value.bytes.data[3 * i + 2]); for (i = 0; i < 3 * numcoef; i++) snd_soc_write(codec, STA32X_B1CF1 + i, ucontrol->value.bytes.data[i]); @@ -252,6 +260,48 @@ static int sta32x_coefficient_put(struct snd_kcontrol *kcontrol, return 0; }
+int sta32x_sync_coef_shadow(struct snd_soc_codec *codec) +{ + struct sta32x_priv *sta32x = snd_soc_codec_get_drvdata(codec); + unsigned int cfud; + int i; + + /* preserve reserved bits in STA32X_CFUD */ + cfud = snd_soc_read(codec, STA32X_CFUD) & 0xf0; + + for (i = 0; i < STA32X_COEF_COUNT; i++) { + snd_soc_write(codec, STA32X_CFADDR2, i); + snd_soc_write(codec, STA32X_B1CF1, + (sta32x->coef_shadow[i] >> 16) & 0xff); + snd_soc_write(codec, STA32X_B1CF2, + (sta32x->coef_shadow[i] >> 8) & 0xff); + snd_soc_write(codec, STA32X_B1CF3, + (sta32x->coef_shadow[i]) & 0xff); + /* chip documentation does not say if the bits are + * self-clearing, so do it explicitly */ + snd_soc_write(codec, STA32X_CFUD, cfud); + snd_soc_write(codec, STA32X_CFUD, cfud | 0x01); + } + return 0; +} + +int sta32x_cache_sync(struct snd_soc_codec *codec) +{ + 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); + snd_soc_write(codec, STA32X_MMUTE, mute); + return rc; +} + #define SINGLE_COEF(xname, index) \ { .iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, \ .info = sta32x_coefficient_info, \ @@ -661,7 +711,7 @@ static int sta32x_set_bias_level(struct snd_soc_codec *codec, return ret; }
- snd_soc_cache_sync(codec); + sta32x_cache_sync(codec); }
/* Power up to mute */ @@ -790,6 +840,17 @@ static int sta32x_probe(struct snd_soc_codec *codec) STA32X_CxCFG_OM_MASK, 2 << STA32X_CxCFG_OM_SHIFT);
+ /* initialize coefficient shadow RAM with reset values */ + for (i = 4; i <= 49; i += 5) + sta32x->coef_shadow[i] = 0x400000; + for (i = 50; i <= 54; i++) + sta32x->coef_shadow[i] = 0x7fffff; + sta32x->coef_shadow[55] = 0x5a9df7; + sta32x->coef_shadow[56] = 0x7fffff; + sta32x->coef_shadow[59] = 0x7fffff; + sta32x->coef_shadow[60] = 0x400000; + sta32x->coef_shadow[61] = 0x400000; + sta32x_set_bias_level(codec, SND_SOC_BIAS_STANDBY); /* Bias level configuration will have done an extra enable */ regulator_bulk_disable(ARRAY_SIZE(sta32x->supplies), sta32x->supplies); diff --git a/sound/soc/codecs/sta32x.h b/sound/soc/codecs/sta32x.h index b97ee5a..d8e32a6 100644 --- a/sound/soc/codecs/sta32x.h +++ b/sound/soc/codecs/sta32x.h @@ -19,6 +19,7 @@ /* STA326 register addresses */
#define STA32X_REGISTER_COUNT 0x2d +#define STA32X_COEF_COUNT 62
#define STA32X_CONFA 0x00 #define STA32X_CONFB 0x01
On Mon, Nov 14, 2011 at 05:23:16PM +0100, Johannes Stezenbach wrote:
The coefficient RAM must be saved in a shadow so it can be restored when the codec is powered on using regulator_bulk_enable().
Applied, thanks.
Add a structure for platform specific configuration and use it, thereby removing a few FIXMEs which marked hard-coded values.
Cc: Sven Neumann s.neumann@raumfeld.com Cc: Daniel Mack daniel@zonque.org Signed-off-by: Johannes Stezenbach js@sig21.net --- include/sound/sta32x.h | 34 ++++++++++++++++++++++++++++++++++ sound/soc/codecs/sta32x.c | 30 +++++++++++++++++++++--------- 2 files changed, 55 insertions(+), 9 deletions(-) create mode 100644 include/sound/sta32x.h
diff --git a/include/sound/sta32x.h b/include/sound/sta32x.h new file mode 100644 index 0000000..45d7477 --- /dev/null +++ b/include/sound/sta32x.h @@ -0,0 +1,34 @@ +/* + * Platform data for ST STA32x ASoC codec driver. + * + * Copyright: 2011 Raumfeld GmbH + * Author: Johannes Stezenbach js@sig21.net + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2 of the License, or (at your + * option) any later version. + */ +#ifndef __LINUX_SND__STA32X_H +#define __LINUX_SND__STA32X_H + +#define STA32X_OCFG_2CH 0 +#define STA32X_OCFG_2_1CH 1 +#define STA32X_OCFG_1CH 3 + +#define STA32X_OM_CH1 0 +#define STA32X_OM_CH2 1 +#define STA32X_OM_CH3 2 + +#define STA32X_THERMAL_ADJUSTMENT_ENABLE 1 +#define STA32X_THERMAL_RECOVERY_ENABLE 2 + +struct sta32x_platform_data { + int output_conf; + int ch1_output_mapping; + int ch2_output_mapping; + int ch3_output_mapping; + int thermal_conf; +}; + +#endif /* __LINUX_SND__STA32X_H */ diff --git a/sound/soc/codecs/sta32x.c b/sound/soc/codecs/sta32x.c index d2f3715..97091e3 100644 --- a/sound/soc/codecs/sta32x.c +++ b/sound/soc/codecs/sta32x.c @@ -35,6 +35,7 @@ #include <sound/initval.h> #include <sound/tlv.h>
+#include <sound/sta32x.h> #include "sta32x.h"
#define STA32X_RATES (SNDRV_PCM_RATE_32000 | \ @@ -73,6 +74,7 @@ static const char *sta32x_supply_names[] = { struct sta32x_priv { struct regulator_bulk_data supplies[ARRAY_SIZE(sta32x_supply_names)]; struct snd_soc_codec *codec; + struct sta32x_platform_data *pdata;
unsigned int mclk; unsigned int format; @@ -775,9 +777,10 @@ static int sta32x_resume(struct snd_soc_codec *codec) static int sta32x_probe(struct snd_soc_codec *codec) { struct sta32x_priv *sta32x = snd_soc_codec_get_drvdata(codec); - int i, ret = 0; + int i, ret = 0, thermal = 0;
sta32x->codec = codec; + sta32x->pdata = dev_get_platdata(codec->dev);
/* regulators */ for (i = 0; i < ARRAY_SIZE(sta32x->supplies); i++) @@ -820,25 +823,34 @@ static int sta32x_probe(struct snd_soc_codec *codec) snd_soc_cache_write(codec, STA32X_AUTO3, 0x00); snd_soc_cache_write(codec, STA32X_C3CFG, 0x40);
- /* FIXME enable thermal warning adjustment and recovery */ + /* set thermal warning adjustment and recovery */ + if (!(sta32x->pdata->thermal_conf & STA32X_THERMAL_ADJUSTMENT_ENABLE)) + thermal |= STA32X_CONFA_TWAB; + if (!(sta32x->pdata->thermal_conf & STA32X_THERMAL_RECOVERY_ENABLE)) + thermal |= STA32X_CONFA_TWRB; snd_soc_update_bits(codec, STA32X_CONFA, - STA32X_CONFA_TWAB | STA32X_CONFA_TWRB, 0); + STA32X_CONFA_TWAB | STA32X_CONFA_TWRB, + thermal);
- /* FIXME select 2.1 mode */ + /* select output configuration */ snd_soc_update_bits(codec, STA32X_CONFF, STA32X_CONFF_OCFG_MASK, - 1 << STA32X_CONFF_OCFG_SHIFT); + sta32x->pdata->output_conf + << STA32X_CONFF_OCFG_SHIFT);
- /* FIXME channel to output mapping */ + /* channel to output mapping */ snd_soc_update_bits(codec, STA32X_C1CFG, STA32X_CxCFG_OM_MASK, - 0 << STA32X_CxCFG_OM_SHIFT); + sta32x->pdata->ch1_output_mapping + << STA32X_CxCFG_OM_SHIFT); snd_soc_update_bits(codec, STA32X_C2CFG, STA32X_CxCFG_OM_MASK, - 1 << STA32X_CxCFG_OM_SHIFT); + sta32x->pdata->ch2_output_mapping + << STA32X_CxCFG_OM_SHIFT); snd_soc_update_bits(codec, STA32X_C3CFG, STA32X_CxCFG_OM_MASK, - 2 << STA32X_CxCFG_OM_SHIFT); + sta32x->pdata->ch3_output_mapping + << STA32X_CxCFG_OM_SHIFT);
/* initialize coefficient shadow RAM with reset values */ for (i = 4; i <= 49; i += 5)
On Mon, Nov 14, 2011 at 05:23:17PM +0100, Johannes Stezenbach wrote:
Add a structure for platform specific configuration and use it, thereby removing a few FIXMEs which marked hard-coded values.
Applied, thanks.
sta32x resets and loses all configuration during ESD test. Work around by polling the CONFA register once a second and restore all coeffcients and registers when CONFA changes unexpectedly.
Cc: Sven Neumann s.neumann@raumfeld.com Cc: Daniel Mack daniel@zonque.org Signed-off-by: Johannes Stezenbach js@sig21.net --- include/sound/sta32x.h | 1 + sound/soc/codecs/sta32x.c | 50 ++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 50 insertions(+), 1 deletions(-)
diff --git a/include/sound/sta32x.h b/include/sound/sta32x.h index 45d7477..8d93b03 100644 --- a/include/sound/sta32x.h +++ b/include/sound/sta32x.h @@ -29,6 +29,7 @@ struct sta32x_platform_data { int ch2_output_mapping; int ch3_output_mapping; int thermal_conf; + int needs_esd_watchdog; };
#endif /* __LINUX_SND__STA32X_H */ diff --git a/sound/soc/codecs/sta32x.c b/sound/soc/codecs/sta32x.c index 97091e3..3b0deaf 100644 --- a/sound/soc/codecs/sta32x.c +++ b/sound/soc/codecs/sta32x.c @@ -27,6 +27,7 @@ #include <linux/platform_device.h> #include <linux/regulator/consumer.h> #include <linux/slab.h> +#include <linux/workqueue.h> #include <sound/core.h> #include <sound/pcm.h> #include <sound/pcm_params.h> @@ -80,6 +81,8 @@ struct sta32x_priv { unsigned int format;
u32 coef_shadow[STA32X_COEF_COUNT]; + struct delayed_work watchdog_work; + int shutdown; };
static const DECLARE_TLV_DB_SCALE(mvol_tlv, -12700, 50, 1); @@ -304,6 +307,46 @@ int sta32x_cache_sync(struct snd_soc_codec *codec) return rc; }
+/* work around ESD issue where sta32x resets and loses all configuration */ +static void sta32x_watchdog(struct work_struct *work) +{ + struct sta32x_priv *sta32x = container_of(work, struct sta32x_priv, + watchdog_work.work); + struct snd_soc_codec *codec = sta32x->codec; + unsigned int confa, confa_cached; + + /* check if sta32x has reset itself */ + confa_cached = snd_soc_read(codec, STA32X_CONFA); + codec->cache_bypass = 1; + confa = snd_soc_read(codec, STA32X_CONFA); + codec->cache_bypass = 0; + if (confa != confa_cached) { + codec->cache_sync = 1; + sta32x_cache_sync(codec); + } + + if (!sta32x->shutdown) + schedule_delayed_work(&sta32x->watchdog_work, + round_jiffies_relative(HZ)); +} + +static void sta32x_watchdog_start(struct sta32x_priv *sta32x) +{ + if (sta32x->pdata->needs_esd_watchdog) { + sta32x->shutdown = 0; + schedule_delayed_work(&sta32x->watchdog_work, + round_jiffies_relative(HZ)); + } +} + +static void sta32x_watchdog_stop(struct sta32x_priv *sta32x) +{ + if (sta32x->pdata->needs_esd_watchdog) { + sta32x->shutdown = 1; + cancel_delayed_work_sync(&sta32x->watchdog_work); + } +} + #define SINGLE_COEF(xname, index) \ { .iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, \ .info = sta32x_coefficient_info, \ @@ -714,6 +757,7 @@ static int sta32x_set_bias_level(struct snd_soc_codec *codec, }
sta32x_cache_sync(codec); + sta32x_watchdog_start(sta32x); }
/* Power up to mute */ @@ -730,7 +774,7 @@ static int sta32x_set_bias_level(struct snd_soc_codec *codec, STA32X_CONFF_PWDN | STA32X_CONFF_EAPD, STA32X_CONFF_PWDN); msleep(300); - + sta32x_watchdog_stop(sta32x); regulator_bulk_disable(ARRAY_SIZE(sta32x->supplies), sta32x->supplies); break; @@ -863,6 +907,9 @@ static int sta32x_probe(struct snd_soc_codec *codec) sta32x->coef_shadow[60] = 0x400000; sta32x->coef_shadow[61] = 0x400000;
+ if (sta32x->pdata->needs_esd_watchdog) + INIT_DELAYED_WORK(&sta32x->watchdog_work, sta32x_watchdog); + sta32x_set_bias_level(codec, SND_SOC_BIAS_STANDBY); /* Bias level configuration will have done an extra enable */ regulator_bulk_disable(ARRAY_SIZE(sta32x->supplies), sta32x->supplies); @@ -879,6 +926,7 @@ static int sta32x_remove(struct snd_soc_codec *codec) { struct sta32x_priv *sta32x = snd_soc_codec_get_drvdata(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);
On Mon, Nov 14, 2011 at 05:23:18PM +0100, Johannes Stezenbach wrote:
sta32x resets and loses all configuration during ESD test. Work around by polling the CONFA register once a second and restore all coeffcients and registers when CONFA changes unexpectedly.
Applied, though it would be nicer if idle_bias_off were also enabled so that this only has an effect when audio is active (otherwise I can't imagine anything cares about the reset).
On Mon, Nov 14, 2011 at 09:37:17PM +0000, Mark Brown wrote:
On Mon, Nov 14, 2011 at 05:23:18PM +0100, Johannes Stezenbach wrote:
sta32x resets and loses all configuration during ESD test. Work around by polling the CONFA register once a second and restore all coeffcients and registers when CONFA changes unexpectedly.
Applied, though it would be nicer if idle_bias_off were also enabled so that this only has an effect when audio is active (otherwise I can't imagine anything cares about the reset).
OK, I must admit the meaning of this comment was not clear to me, but after grepping in git log I found a850260e4722706805fda3a0f6e5bc1539e83bac Is that what you mean?
Thanks Johannes
On Tue, Nov 15, 2011 at 12:50:13PM +0100, Johannes Stezenbach wrote:
On Mon, Nov 14, 2011 at 09:37:17PM +0000, Mark Brown wrote:
Applied, though it would be nicer if idle_bias_off were also enabled so that this only has an effect when audio is active (otherwise I can't imagine anything cares about the reset).
OK, I must admit the meaning of this comment was not clear to me, but after grepping in git log I found a850260e4722706805fda3a0f6e5bc1539e83bac
Please always quote human readable text, not just git commit ids.
Is that what you mean?
Yes.
participants (2)
-
Johannes Stezenbach
-
Mark Brown