[PATCH v2] ASoC: max9867: shutdown codec when changing filter type
Changing filter type without disabling codec results in filter malfunction. Disable codec when changing filter type.
Signed-off-by: Pavel Dobias dobias@2n.cz --- Changes in v2: - don't allow change if component active - avoid codec shutdown in max9867_filter_set if value not changed
sound/soc/codecs/max9867.c | 80 +++++++++++++++++++++++++++++++++----- sound/soc/codecs/max9867.h | 4 +- 2 files changed, 73 insertions(+), 11 deletions(-)
diff --git a/sound/soc/codecs/max9867.c b/sound/soc/codecs/max9867.c index fcb31144d69c..749de1ab1093 100644 --- a/sound/soc/codecs/max9867.c +++ b/sound/soc/codecs/max9867.c @@ -15,6 +15,13 @@ #include <sound/tlv.h> #include "max9867.h"
+struct max9867_priv { + struct regmap *regmap; + const struct snd_pcm_hw_constraint_list *constraints; + unsigned int sysclk, pclk; + bool master, dsp_a; +}; + static const char *const max9867_spmode[] = { "Stereo Diff", "Mono Diff", "Stereo Cap", "Mono Cap", @@ -32,8 +39,68 @@ static const char *const max9867_adc_dac_filter_text[] = { "Butterworth/8-24" };
-static SOC_ENUM_SINGLE_DECL(max9867_filter, MAX9867_CODECFLTR, 7, - max9867_filter_text); +static int max9867_filter_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_soc_component *component = snd_soc_kcontrol_component(kcontrol); + struct max9867_priv *max9867 = snd_soc_component_get_drvdata(component); + unsigned int reg; + int ret; + + ret = regmap_read(max9867->regmap, MAX9867_CODECFLTR, ®); + if (ret) + return -EINVAL; + + if (reg & MAX9867_CODECFLTR_MODE) + ucontrol->value.enumerated.item[0] = 1; + else + ucontrol->value.enumerated.item[0] = 0; + + return 0; +} + +static int max9867_filter_set(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_soc_component *component = snd_soc_kcontrol_component(kcontrol); + struct max9867_priv *max9867 = snd_soc_component_get_drvdata(component); + unsigned int reg, mode = ucontrol->value.enumerated.item[0]; + int ret; + + /* don't allow change if component active */ + if (snd_soc_component_active(component)) { + return -EBUSY; + } + + if (mode > 1) + return -EINVAL; + mode = mode ? MAX9867_CODECFLTR_MODE : 0; + + /* read current filter mode */ + ret = regmap_read(max9867->regmap, MAX9867_CODECFLTR, ®); + if (ret) + return -EINVAL; + + /* check if change is needed */ + if ((reg & MAX9867_CODECFLTR_MODE) == mode) + return 0; + + /* shutdown codec before switching filter mode */ + regmap_update_bits(max9867->regmap, MAX9867_PWRMAN, + MAX9867_PWRMAN_SHDN, 0); + + /* switch filter mode */ + regmap_update_bits(max9867->regmap, MAX9867_CODECFLTR, + MAX9867_CODECFLTR_MODE, mode); + + /* out of shutdown now */ + regmap_update_bits(max9867->regmap, MAX9867_PWRMAN, + MAX9867_PWRMAN_SHDN, MAX9867_PWRMAN_SHDN); + + return 0; +} + +static SOC_ENUM_SINGLE_EXT_DECL(max9867_filter, max9867_filter_text); static SOC_ENUM_SINGLE_DECL(max9867_dac_filter, MAX9867_CODECFLTR, 0, max9867_adc_dac_filter_text); static SOC_ENUM_SINGLE_DECL(max9867_adc_filter, MAX9867_CODECFLTR, 4, @@ -76,7 +143,7 @@ static const struct snd_kcontrol_new max9867_snd_controls[] = { SOC_ENUM("Speaker Mode", max9867_spkmode), SOC_SINGLE("Volume Smoothing Switch", MAX9867_MODECONFIG, 6, 1, 0), SOC_SINGLE("Line ZC Switch", MAX9867_MODECONFIG, 5, 1, 0), - SOC_ENUM("DSP Filter", max9867_filter), + SOC_ENUM_EXT("DSP Filter", max9867_filter, max9867_filter_get, max9867_filter_set), SOC_ENUM("ADC Filter", max9867_adc_filter), SOC_ENUM("DAC Filter", max9867_dac_filter), SOC_SINGLE("Mono Playback Switch", MAX9867_IFC1B, 3, 1, 0), @@ -197,13 +264,6 @@ static const struct snd_pcm_hw_constraint_list max9867_constraints_48k = { .count = ARRAY_SIZE(max9867_rates_48k), };
-struct max9867_priv { - struct regmap *regmap; - const struct snd_pcm_hw_constraint_list *constraints; - unsigned int sysclk, pclk; - bool master, dsp_a; -}; - static int max9867_startup(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { diff --git a/sound/soc/codecs/max9867.h b/sound/soc/codecs/max9867.h index 3092c3b99075..b6b880631b13 100644 --- a/sound/soc/codecs/max9867.h +++ b/sound/soc/codecs/max9867.h @@ -44,7 +44,8 @@ #define MAX9867_IFC1B_PCLK_4 0x05 #define MAX9867_IFC1B_PCLK_8 0x06 #define MAX9867_IFC1B_PCLK_16 0x07 -#define MAX9867_CODECFLTR 0x0a +#define MAX9867_CODECFLTR 0x0a +#define MAX9867_CODECFLTR_MODE (1<<7) #define MAX9867_SIDETONE 0x0b #define MAX9867_DACLEVEL 0x0c #define MAX9867_ADCLEVEL 0x0d @@ -58,6 +59,7 @@ #define MAX9867_MICCONFIG 0x15 #define MAX9867_MODECONFIG 0x16 #define MAX9867_PWRMAN 0x17 +#define MAX9867_PWRMAN_SHDN (1<<7) #define MAX9867_REVISION 0xff
#define MAX9867_CACHEREGNUM 10
On Wed, Aug 26, 2020 at 05:07:24PM +0200, Pavel Dobias wrote:
- /* don't allow change if component active */
- if (snd_soc_component_active(component)) {
return -EBUSY;
- }
AFAICT the filter only applies to the DAC/ADC but this is checking if the component is in use at all and there's bypass paths - we should only restrict this when the DAC and ADC are in use.
- if (mode > 1)
return -EINVAL;
- mode = mode ? MAX9867_CODECFLTR_MODE : 0;
Please write normal conditional statements to improve readability.
participants (2)
-
Mark Brown
-
Pavel Dobias