[PATCH RFC v1] ASoC: core: Support for limiting the volume

[PATCH RFC v1] ASoC: core: Support for limiting the volume

[Peter Ujfalusi]

Add support for the core to limit the maximum volume on an
existing control.
The function will modify the soc_mixer_control.max value
of the given control.
The new value must be lower than the original one (chip maximum)

If there is a need for limiting a gain on a given control,
than machine drivers can do the following in their
snd_soc_dai_link.init function:

snd_soc_limit_volume(codec, "TPA6140A2 Headphone Playback Volume", 21);

This will modify the original 31 (chip maximum) to 21, so user
space will not be able to set the gain higher than this.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@nokia.com>
---

Hello Mark, Liam,

So this is my proposed way of doing the volume limiting within the core.
By looking around the code, this should be safe.
With this simple function all machine driver can limit gains on a given
control in a way, that the codec/amp driver does not need to be involved.

What do you think?

---
Peter

 include/sound/soc.h  |    2 ++
 sound/soc/soc-core.c |   38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 0 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 01e9c66..9f306f0 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -326,6 +326,8 @@ int snd_soc_get_volsw_s8(struct snd_kcontrol *kcontrol,
 	struct snd_ctl_elem_value *ucontrol);
 int snd_soc_put_volsw_s8(struct snd_kcontrol *kcontrol,
 	struct snd_ctl_elem_value *ucontrol);
+int snd_soc_limit_volume(struct snd_soc_codec *codec,
+	const char *name, int max);
 
 /**
  * struct snd_soc_jack_pin - Describes a pin to update based on jack detection
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index d59076e..5968499 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2238,6 +2238,44 @@ int snd_soc_put_volsw_s8(struct snd_kcontrol *kcontrol,
 EXPORT_SYMBOL_GPL(snd_soc_put_volsw_s8);
 
 /**
+ * snd_soc_limit_volume - Set new limit to an existing volume control.
+ *
+ * @codec: where to look for the control
+ * @name: Name of the control
+ * @max: new maximum limit
+ *
+ * Return 0 for success, else error.
+ */
+int snd_soc_limit_volume(struct snd_soc_codec *codec,
+	const char *name, int max)
+{
+	struct snd_card *card = codec->card;
+	struct snd_kcontrol *kctl;
+	struct soc_mixer_control *mc;
+	int found = 0;
+	int ret = -EINVAL;
+
+	/* Sanity check for name and max */
+	if (unlikely(!name || max <= 0))
+		return -EINVAL;
+
+	list_for_each_entry(kctl, &card->controls, list) {
+		if (!strncmp(kctl->id.name, name, sizeof(kctl->id.name))) {
+			found = 1;
+			break;
+		}
+	}
+	if (found) {
+		mc = (struct soc_mixer_control *)kctl->private_value;
+		if (max < mc->max)
+			mc->max = max;
+		ret = 0;
+	}
+	return ret;
+}
+EXPORT_SYMBOL_GPL(snd_soc_limit_volume);
+
+/**
  * snd_soc_dai_set_sysclk - configure DAI system or master clock.
  * @dai: DAI
  * @clk_id: DAI specific clock ID
-- 
1.7.1

Re: [PATCH RFC v1] ASoC: core: Support for limiting the volume

[Peter Ujfalusi]

Hi,

On Friday 07 May 2010 09:03:05 Ujfalusi Peter (Nokia-D/Tampere) wrote:
> Add support for the core to limit the maximum volume on an
> existing control.
> The function will modify the soc_mixer_control.max value
> of the given control.
> The new value must be lower than the original one (chip maximum)
> 
> If there is a need for limiting a gain on a given control,
> than machine drivers can do the following in their
> snd_soc_dai_link.init function:
> 
> snd_soc_limit_volume(codec, "TPA6140A2 Headphone Playback Volume", 21);
> 
> This will modify the original 31 (chip maximum) to 21, so user
> space will not be able to set the gain higher than this.
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@nokia.com>
> ---
> 
> Hello Mark, Liam,
> 
> So this is my proposed way of doing the volume limiting within the core.
> By looking around the code, this should be safe.
> With this simple function all machine driver can limit gains on a given
> control in a way, that the codec/amp driver does not need to be involved.
> 
> What do you think?
> 
> ---
> Peter
> 
>  include/sound/soc.h  |    2 ++
>  sound/soc/soc-core.c |   38 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 40 insertions(+), 0 deletions(-)
> 
> diff --git a/include/sound/soc.h b/include/sound/soc.h
> index 01e9c66..9f306f0 100644
> --- a/include/sound/soc.h
> +++ b/include/sound/soc.h
> @@ -326,6 +326,8 @@ int snd_soc_get_volsw_s8(struct snd_kcontrol *kcontrol,
>  	struct snd_ctl_elem_value *ucontrol);
>  int snd_soc_put_volsw_s8(struct snd_kcontrol *kcontrol,
>  	struct snd_ctl_elem_value *ucontrol);
> +int snd_soc_limit_volume(struct snd_soc_codec *codec,
> +	const char *name, int max);
> 
>  /**
>   * struct snd_soc_jack_pin - Describes a pin to update based on jack
> detection diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
> index d59076e..5968499 100644
> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -2238,6 +2238,44 @@ int snd_soc_put_volsw_s8(struct snd_kcontrol
> *kcontrol, EXPORT_SYMBOL_GPL(snd_soc_put_volsw_s8);
> 
>  /**
> + * snd_soc_limit_volume - Set new limit to an existing volume control.
> + *
> + * @codec: where to look for the control
> + * @name: Name of the control
> + * @max: new maximum limit
> + *
> + * Return 0 for success, else error.
> + */
> +int snd_soc_limit_volume(struct snd_soc_codec *codec,
> +	const char *name, int max)
> +{
> +	struct snd_card *card = codec->card;
> +	struct snd_kcontrol *kctl;
> +	struct soc_mixer_control *mc;
> +	int found = 0;
> +	int ret = -EINVAL;
> +
> +	/* Sanity check for name and max */
> +	if (unlikely(!name || max <= 0))
> +		return -EINVAL;
> +
> +	list_for_each_entry(kctl, &card->controls, list) {
> +		if (!strncmp(kctl->id.name, name, sizeof(kctl->id.name))) {
> +			found = 1;
> +			break;
> +		}
> +	}
> +	if (found) {
> +		mc = (struct soc_mixer_control *)kctl->private_value;
> +		if (max < mc->max)
> +			mc->max = max;
> +		ret = 0;

Probably it would be better to return with 0 only, if we _changed_ the maximum.
I'll resend after the comments.

> +	}
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(snd_soc_limit_volume);
> +
> +/**
>   * snd_soc_dai_set_sysclk - configure DAI system or master clock.
>   * @dai: DAI
>   * @clk_id: DAI specific clock ID

-- 
Péter

Re: [PATCH RFC v1] ASoC: core: Support for limiting the volume

[Mark Brown]

On Fri, May 07, 2010 at 10:26:35AM +0300, Peter Ujfalusi wrote:

> Probably it would be better to return with 0 only, if we _changed_ the maximum.
> I'll resend after the comments.

No, I think it's fine to report success so long as the requested change
can be implemented - if it's a noop it might be worth printing a warning
but the requested setting is in effect.

Re: [PATCH RFC v1] ASoC: core: Support for limiting the volume

[Peter Ujfalusi]

On Friday 07 May 2010 12:56:15 ext Mark Brown wrote:
> On Fri, May 07, 2010 at 10:26:35AM +0300, Peter Ujfalusi wrote:
> > Probably it would be better to return with 0 only, if we _changed_ the
> > maximum. I'll resend after the comments.
> 
> No, I think it's fine to report success so long as the requested change
> can be implemented - if it's a noop it might be worth printing a warning
> but the requested setting is in effect.

I think:
if (max <= mc->max) {
	mc->max = max;
	ret = 0;
}

Would be appropriate. We still report error, if the user asked out of limit 
gain, but we report success, if the limit has not been actually changed.

-- 
Péter

Re: [PATCH RFC v1] ASoC: core: Support for limiting the volume

[Mark Brown]

On Fri, May 07, 2010 at 01:03:53PM +0300, Peter Ujfalusi wrote:

> I think:
> if (max <= mc->max) {
> 	mc->max = max;
> 	ret = 0;
> }

> Would be appropriate. We still report error, if the user asked out of limit 
> gain, but we report success, if the limit has not been actually changed.

Yes, reporting an error if the limit is higher than is possible is
sensible.