[PATCH] ASoC: rt1011: Fix 'I2S Reference' enum control
There are several things the patch adding the support for 'I2S Reference' got wrong: - "None" selection is in fact equals to last selected reference - The custom put overrides RX/TX len, TDM slot sizes, etc - the enum is useless in most part for the reference tracking - there is no need for EXT control as there is a single bit in RT1011_TDM1_SET_1 register (bit 7) which selects the reference - it was using ucontrol->value.integer.value[0] in the put/get callbacks which causes access to 'I2S Reference' enum with alsamixer to fail: $ alsamixer cannot load mixer controls: Invalid argument
cml_rt1011_rt5682 cml_rt1011_rt5682: control 2:0:0:TL I2S Reference:0: access overflow
It should have used ucontrol->value.enumerated.item[0], but since there is no need for the custom code, it does not really matter.
Fixes: 87f40af26c262 ("ASoC: rt1011: add i2s reference control for rt1011") Reported-by: Seppo Ingalsuo seppo.ingalsuo@linux.intel.com Signed-off-by: Peter Ujfalusi peter.ujfalusi@linux.intel.com --- Hi,
This patch is an iteration on https://lore.kernel.org/alsa-devel/20211011144518.2518-1-peter.ujfalusi@linu...
to fix 87f40af26c262 ("ASoC: rt1011: add i2s reference control for rt1011").
In essence it is reverting the original patch to use SOC_ENUM_SINGLE_DECL/SOC_ENUM to handle the bit to select the I2S reference.
Regards, Peter
sound/soc/codecs/rt1011.c | 56 +++++---------------------------------- sound/soc/codecs/rt1011.h | 7 ----- 2 files changed, 6 insertions(+), 57 deletions(-)
diff --git a/sound/soc/codecs/rt1011.c b/sound/soc/codecs/rt1011.c index 508597866dff..297af7ff824c 100644 --- a/sound/soc/codecs/rt1011.c +++ b/sound/soc/codecs/rt1011.c @@ -1311,56 +1311,13 @@ static int rt1011_r0_load_info(struct snd_kcontrol *kcontrol, .put = rt1011_r0_load_mode_put \ }
-static const char * const rt1011_i2s_ref[] = { - "None", "Left Channel", "Right Channel" +static const char * const rt1011_i2s_ref_texts[] = { + "Left Channel", "Right Channel" };
-static SOC_ENUM_SINGLE_DECL(rt1011_i2s_ref_enum, 0, 0, - rt1011_i2s_ref); - -static int rt1011_i2s_ref_put(struct snd_kcontrol *kcontrol, - struct snd_ctl_elem_value *ucontrol) -{ - struct snd_soc_component *component = - snd_soc_kcontrol_component(kcontrol); - struct rt1011_priv *rt1011 = - snd_soc_component_get_drvdata(component); - int i2s_ref_ch = ucontrol->value.integer.value[0]; - - switch (i2s_ref_ch) { - case RT1011_I2S_REF_LEFT_CH: - regmap_write(rt1011->regmap, RT1011_TDM_TOTAL_SET, 0x0240); - regmap_write(rt1011->regmap, RT1011_TDM1_SET_2, 0x8); - regmap_write(rt1011->regmap, RT1011_TDM1_SET_1, 0x1022); - regmap_write(rt1011->regmap, RT1011_ADCDAT_OUT_SOURCE, 0x4); - break; - case RT1011_I2S_REF_RIGHT_CH: - regmap_write(rt1011->regmap, RT1011_TDM_TOTAL_SET, 0x0240); - regmap_write(rt1011->regmap, RT1011_TDM1_SET_2, 0x8); - regmap_write(rt1011->regmap, RT1011_TDM1_SET_1, 0x10a2); - regmap_write(rt1011->regmap, RT1011_ADCDAT_OUT_SOURCE, 0x4); - break; - default: - dev_info(component->dev, "I2S Reference: Do nothing\n"); - } - - rt1011->i2s_ref = ucontrol->value.integer.value[0]; - - return 0; -} - -static int rt1011_i2s_ref_get(struct snd_kcontrol *kcontrol, - struct snd_ctl_elem_value *ucontrol) -{ - struct snd_soc_component *component = - snd_soc_kcontrol_component(kcontrol); - struct rt1011_priv *rt1011 = - snd_soc_component_get_drvdata(component); - - ucontrol->value.integer.value[0] = rt1011->i2s_ref; - - return 0; -} +static SOC_ENUM_SINGLE_DECL(rt1011_i2s_ref_enum, + RT1011_TDM1_SET_1, 7, + rt1011_i2s_ref_texts);
static const struct snd_kcontrol_new rt1011_snd_controls[] = { /* I2S Data In Selection */ @@ -1401,8 +1358,7 @@ static const struct snd_kcontrol_new rt1011_snd_controls[] = { SOC_SINGLE("R0 Temperature", RT1011_STP_INITIAL_RESISTANCE_TEMP, 2, 255, 0), /* I2S Reference */ - SOC_ENUM_EXT("I2S Reference", rt1011_i2s_ref_enum, - rt1011_i2s_ref_get, rt1011_i2s_ref_put), + SOC_ENUM("I2S Reference", rt1011_i2s_ref_enum), };
static int rt1011_is_sys_clk_from_pll(struct snd_soc_dapm_widget *source, diff --git a/sound/soc/codecs/rt1011.h b/sound/soc/codecs/rt1011.h index afb2fad94216..68fadc15fa8c 100644 --- a/sound/soc/codecs/rt1011.h +++ b/sound/soc/codecs/rt1011.h @@ -654,12 +654,6 @@ enum { RT1011_AIFS };
-enum { - RT1011_I2S_REF_NONE, - RT1011_I2S_REF_LEFT_CH, - RT1011_I2S_REF_RIGHT_CH, -}; - /* BiQual & DRC related settings */ #define RT1011_BQ_DRC_NUM 128 struct rt1011_bq_drc_params { @@ -698,7 +692,6 @@ struct rt1011_priv { unsigned int r0_reg, cali_done; unsigned int r0_calib, temperature_calib; int recv_spk_mode; - unsigned int i2s_ref; };
#endif /* end of _RT1011_H_ */
On Tue, 12 Oct 2021 09:31:13 +0300, Peter Ujfalusi wrote:
There are several things the patch adding the support for 'I2S Reference' got wrong:
- "None" selection is in fact equals to last selected reference
- The custom put overrides RX/TX len, TDM slot sizes, etc
- the enum is useless in most part for the reference tracking
- there is no need for EXT control as there is a single bit in RT1011_TDM1_SET_1 register (bit 7) which selects the reference
- it was using ucontrol->value.integer.value[0] in the put/get callbacks which causes access to 'I2S Reference' enum with alsamixer to fail:
$ alsamixer cannot load mixer controls: Invalid argument
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/1] ASoC: rt1011: Fix 'I2S Reference' enum control commit: f05a9b8552896d95fc22e135eaf9c6be541bfe79
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
participants (2)
-
Mark Brown
-
Peter Ujfalusi