[PATCH v2] ASoC: rt1011: Fix 'I2S Reference' enum control

[PATCH v2] ASoC: rt1011: Fix 'I2S Reference' enum control

[Peter Ujfalusi]

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 causesed access to 'I2S Reference' enum with alsamixer to fail

Complements: c3de683c4d1d ("ASoC: rt1011: Fix 'I2S Reference' enum control caused error")
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
---
Hi,

Changes since v1:
- Rebased on top of next
- changed the tag from Fixes to Complements to make sure that the crash fix and
  the fix for the implementation stays together.

This patch is an iteration on
https://lore.kernel.org/alsa-devel/20211011144518.2518-1-peter.ujfalusi@linux.intel.com/

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 | 53 ++++-----------------------------------
 sound/soc/codecs/rt1011.h |  7 ------
 2 files changed, 5 insertions(+), 55 deletions(-)

diff --git a/sound/soc/codecs/rt1011.c b/sound/soc/codecs/rt1011.c
index bdfcbb81fa19..297af7ff824c 100644
--- a/sound/soc/codecs/rt1011.c
+++ b/sound/soc/codecs/rt1011.c
@@ -1312,54 +1312,12 @@ static int rt1011_r0_load_info(struct snd_kcontrol *kcontrol,
 }
 
 static const char * const rt1011_i2s_ref_texts[] = {
-	"None", "Left Channel", "Right Channel"
+	"Left Channel", "Right Channel"
 };
 
-static SOC_ENUM_SINGLE_EXT_DECL(rt1011_i2s_ref_enum, rt1011_i2s_ref_texts);
-
-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.enumerated.item[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.enumerated.item[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.enumerated.item[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 */
@@ -1400,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_ */
-- 
2.33.0

Re: [PATCH v2] ASoC: rt1011: Fix 'I2S Reference' enum control

[Mark Brown]

On Wed, 13 Oct 2021 15:33:00 +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 causesed access to 'I2S Reference' enum with alsamixer to fail
> 
> [...]

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