diff mbox series

[v3,12/16] ASoC: tegra: Fix kcontrol put callback in MVC

Message ID 1637219231-406-13-git-send-email-spujar@nvidia.com
State Accepted
Headers show
Series Kcontrol get/put cleanup in Tegra drivers | expand

Commit Message

Sameer Pujar Nov. 18, 2021, 7:07 a.m. UTC
The kcontrol put callback is expected to return 1 when there is change
in HW or when the update is acknowledged by driver. This would ensure
that change notifications are sent to subscribed applications. Filter
out duplicate updates in MVC driver.

Fixes: e539891f9687 ("ASoC: tegra: Add Tegra210 based MVC driver")
Signed-off-by: Sameer Pujar <spujar@nvidia.com>
Suggested-by: Jaroslav Kysela <perex@perex.cz>
Suggested-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/tegra/tegra210_mvc.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

Comments

Takashi Iwai Nov. 18, 2021, 1:32 p.m. UTC | #1
On Thu, 18 Nov 2021 08:07:07 +0100,
Sameer Pujar wrote:
> 
> The kcontrol put callback is expected to return 1 when there is change
> in HW or when the update is acknowledged by driver. This would ensure
> that change notifications are sent to subscribed applications. Filter
> out duplicate updates in MVC driver.
> 
> Fixes: e539891f9687 ("ASoC: tegra: Add Tegra210 based MVC driver")
> Signed-off-by: Sameer Pujar <spujar@nvidia.com>
> Suggested-by: Jaroslav Kysela <perex@perex.cz>
> Suggested-by: Mark Brown <broonie@kernel.org>
> ---
>  sound/soc/tegra/tegra210_mvc.c | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/sound/soc/tegra/tegra210_mvc.c b/sound/soc/tegra/tegra210_mvc.c
> index b7e3170..85b1558 100644
> --- a/sound/soc/tegra/tegra210_mvc.c
> +++ b/sound/soc/tegra/tegra210_mvc.c
> @@ -136,7 +136,7 @@ static int tegra210_mvc_put_mute(struct snd_kcontrol *kcontrol,
>  	struct snd_soc_component *cmpnt = snd_soc_kcontrol_component(kcontrol);
>  	struct tegra210_mvc *mvc = snd_soc_component_get_drvdata(cmpnt);
>  	unsigned int value;
> -	u8 mute_mask;
> +	u8 new_mask, old_mask;
>  	int err;
>  
>  	pm_runtime_get_sync(cmpnt->dev);
> @@ -148,11 +148,19 @@ static int tegra210_mvc_put_mute(struct snd_kcontrol *kcontrol,
>  	if (err < 0)
>  		goto end;
>  
> -	mute_mask = ucontrol->value.integer.value[0];
> +	regmap_read(mvc->regmap, TEGRA210_MVC_CTRL, &value);
> +
> +	old_mask = (value >> TEGRA210_MVC_MUTE_SHIFT) & TEGRA210_MUTE_MASK_EN;
> +	new_mask = ucontrol->value.integer.value[0];
> +
> +	if (new_mask == old_mask) {
> +		err = 0;
> +		goto end;
> +	}
>  
>  	err = regmap_update_bits(mvc->regmap, mc->reg,
>  				 TEGRA210_MVC_MUTE_MASK,
> -				 mute_mask << TEGRA210_MVC_MUTE_SHIFT);
> +				 new_mask << TEGRA210_MVC_MUTE_SHIFT);

I guess this test-and-update procedure can be simplified with
regmap_update_bits_check().


thanks,

Takashi
Mark Brown Nov. 18, 2021, 1:49 p.m. UTC | #2
On Thu, Nov 18, 2021 at 02:32:07PM +0100, Takashi Iwai wrote:
> Sameer Pujar wrote:

> > +	old_mask = (value >> TEGRA210_MVC_MUTE_SHIFT) & TEGRA210_MUTE_MASK_EN;
> > +	new_mask = ucontrol->value.integer.value[0];
> > +
> > +	if (new_mask == old_mask) {
> > +		err = 0;
> > +		goto end;
> > +	}
> >  
> >  	err = regmap_update_bits(mvc->regmap, mc->reg,
> >  				 TEGRA210_MVC_MUTE_MASK,
> > -				 mute_mask << TEGRA210_MVC_MUTE_SHIFT);
> > +				 new_mask << TEGRA210_MVC_MUTE_SHIFT);

> I guess this test-and-update procedure can be simplified with
> regmap_update_bits_check().

Yes, this is exactly the application that _update_bits_check() was
written for.
diff mbox series

Patch

diff --git a/sound/soc/tegra/tegra210_mvc.c b/sound/soc/tegra/tegra210_mvc.c
index b7e3170..85b1558 100644
--- a/sound/soc/tegra/tegra210_mvc.c
+++ b/sound/soc/tegra/tegra210_mvc.c
@@ -136,7 +136,7 @@  static int tegra210_mvc_put_mute(struct snd_kcontrol *kcontrol,
 	struct snd_soc_component *cmpnt = snd_soc_kcontrol_component(kcontrol);
 	struct tegra210_mvc *mvc = snd_soc_component_get_drvdata(cmpnt);
 	unsigned int value;
-	u8 mute_mask;
+	u8 new_mask, old_mask;
 	int err;
 
 	pm_runtime_get_sync(cmpnt->dev);
@@ -148,11 +148,19 @@  static int tegra210_mvc_put_mute(struct snd_kcontrol *kcontrol,
 	if (err < 0)
 		goto end;
 
-	mute_mask = ucontrol->value.integer.value[0];
+	regmap_read(mvc->regmap, TEGRA210_MVC_CTRL, &value);
+
+	old_mask = (value >> TEGRA210_MVC_MUTE_SHIFT) & TEGRA210_MUTE_MASK_EN;
+	new_mask = ucontrol->value.integer.value[0];
+
+	if (new_mask == old_mask) {
+		err = 0;
+		goto end;
+	}
 
 	err = regmap_update_bits(mvc->regmap, mc->reg,
 				 TEGRA210_MVC_MUTE_MASK,
-				 mute_mask << TEGRA210_MVC_MUTE_SHIFT);
+				 new_mask << TEGRA210_MVC_MUTE_SHIFT);
 	if (err < 0)
 		goto end;
 
@@ -195,7 +203,7 @@  static int tegra210_mvc_put_vol(struct snd_kcontrol *kcontrol,
 	unsigned int reg = mc->reg;
 	unsigned int value;
 	u8 chan;
-	int err;
+	int err, old_volume;
 
 	pm_runtime_get_sync(cmpnt->dev);
 
@@ -207,10 +215,16 @@  static int tegra210_mvc_put_vol(struct snd_kcontrol *kcontrol,
 		goto end;
 
 	chan = (reg - TEGRA210_MVC_TARGET_VOL) / REG_SIZE;
+	old_volume = mvc->volume[chan];
 
 	tegra210_mvc_conv_vol(mvc, chan,
 			      ucontrol->value.integer.value[0]);
 
+	if (mvc->volume[chan] == old_volume) {
+		err = 0;
+		goto end;
+	}
+
 	/* Configure init volume same as target volume */
 	regmap_write(mvc->regmap,
 		TEGRA210_MVC_REG_OFFSET(TEGRA210_MVC_INIT_VOL, chan),