[PATCH 1/2 v2] ALSA: Add snd_ctl_update() to dynamically update a control

[PATCH 1/2 v2] ALSA: Add snd_ctl_update() to dynamically update a control

[Dimitris Papastamos]

Add a function to dynamically update a given control.  If the
control does not already exist, a third parameter is used to determine
whether to actually add that control.  This is useful in cases where
downloadable firmware at runtime can add or update existing controls.
A separate patch needs to be made to make ALSA Mixer cope with this.

Signed-off-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>
---
 include/sound/control.h |    1 +
 sound/core/control.c    |   46 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 0 deletions(-)

diff --git a/include/sound/control.h b/include/sound/control.h
index e67db28..12e4de7 100644
--- a/include/sound/control.h
+++ b/include/sound/control.h
@@ -113,6 +113,7 @@ struct snd_kcontrol *snd_ctl_new1(const struct snd_kcontrol_new * kcontrolnew, v
 void snd_ctl_free_one(struct snd_kcontrol * kcontrol);
 int snd_ctl_add(struct snd_card * card, struct snd_kcontrol * kcontrol);
 int snd_ctl_remove(struct snd_card * card, struct snd_kcontrol * kcontrol);
+int snd_ctl_update(struct snd_card * card, struct snd_kcontrol * kcontrol, int add_on_update);
 int snd_ctl_remove_id(struct snd_card * card, struct snd_ctl_elem_id *id);
 int snd_ctl_rename_id(struct snd_card * card, struct snd_ctl_elem_id *src_id, struct snd_ctl_elem_id *dst_id);
 int snd_ctl_activate_id(struct snd_card *card, struct snd_ctl_elem_id *id,
diff --git a/sound/core/control.c b/sound/core/control.c
index db51e4e..9c5d7a9 100644
--- a/sound/core/control.c
+++ b/sound/core/control.c
@@ -397,6 +397,52 @@ int snd_ctl_remove(struct snd_card *card, struct snd_kcontrol *kcontrol)
 EXPORT_SYMBOL(snd_ctl_remove);
 
 /**
+ * snd_ctl_update - update the control instance of the card
+ * @card: the card instance
+ * @kcontrol: the control instance to update
+ * @add_on_update: add the control if not already added
+ *
+ * Updates the control instance created via snd_ctl_new() or
+ * snd_ctl_new1() of the given card.
+ *
+ * Returns zero if successful, or a negative error code on failure.
+ *
+ */
+int snd_ctl_update(struct snd_card *card, struct snd_kcontrol *kcontrol,
+		   int add_on_update)
+{
+	struct snd_ctl_elem_id id;
+	struct snd_kcontrol *old;
+	int ret;
+
+	if (!kcontrol)
+		return -EINVAL;
+	if (snd_BUG_ON(!card || !kcontrol->info))
+		return -EINVAL;
+	id = kcontrol->id;
+	down_write(&card->controls_rwsem);
+	old = snd_ctl_find_id(card, &id);
+	if (!old) {
+		up_write(&card->controls_rwsem);
+		if (add_on_update)
+			return snd_ctl_add(card, kcontrol);
+		return -EINVAL;
+	}
+	ret = snd_ctl_remove(card, old);
+	if (ret < 0) {
+		up_write(&card->controls_rwsem);
+		return ret;
+	}
+	up_write(&card->controls_rwsem);
+	ret = snd_ctl_add(card, kcontrol);
+	if (ret < 0)
+		return ret;
+	return 0;
+}
+
+EXPORT_SYMBOL(snd_ctl_update);
+
+/**
  * snd_ctl_remove_id - remove the control of the given id and release it
  * @card: the card instance
  * @id: the control id to remove
-- 
1.7.4.1

[PATCH 2/2 v2] ASoC: soc-core: Add helper function to handle dynamic update of controls

[Dimitris Papastamos]

This function can be used to update or add controls at runtime.  It will
always try to add a given control if it does not already exist.

Signed-off-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>
---
 include/sound/soc.h  |    2 ++
 sound/soc/soc-core.c |   33 +++++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index bfa4836..509c1ea 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -344,6 +344,8 @@ struct snd_kcontrol *snd_soc_cnew(const struct snd_kcontrol_new *_template,
 				  const char *prefix);
 int snd_soc_add_controls(struct snd_soc_codec *codec,
 	const struct snd_kcontrol_new *controls, int num_controls);
+int snd_soc_update_controls(struct snd_soc_codec *codec,
+	const struct snd_kcontrol_new *controls, int num_controls);
 int snd_soc_info_enum_double(struct snd_kcontrol *kcontrol,
 	struct snd_ctl_elem_info *uinfo);
 int snd_soc_info_enum_ext(struct snd_kcontrol *kcontrol,
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 17efacd..815103a 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2420,6 +2420,39 @@ int snd_soc_add_controls(struct snd_soc_codec *codec,
 EXPORT_SYMBOL_GPL(snd_soc_add_controls);
 
 /**
+ * snd_soc_update_controls - update an array of controls
+ * Convienience function to update a list of controls.
+ *
+ * @codec: codec to update controls of
+ * @controls: array of controls to update
+ * @num_controls: number of elements in the array
+ *
+ * Return 0 for success, else error.
+ */
+int snd_soc_update_controls(struct snd_soc_codec *codec,
+	const struct snd_kcontrol_new *controls, int num_controls)
+{
+	struct snd_card *card = codec->card->snd_card;
+	int err, i;
+
+	for (i = 0; i < num_controls; i++) {
+		const struct snd_kcontrol_new *control = &controls[i];
+		/* always add on update if not already added */
+		err = snd_ctl_update(card, snd_soc_cnew(control, codec,
+							control->name,
+							codec->name_prefix), 1);
+		if (err < 0) {
+			dev_err(codec->dev, "%s: Failed to update %s: %d\n",
+				codec->name, control->name, err);
+			return err;
+		}
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(snd_soc_update_controls);
+
+/**
  * snd_soc_info_enum_double - enumerated double mixer info callback
  * @kcontrol: mixer control
  * @uinfo: control element information
-- 
1.7.4.1

Re: [PATCH 1/2 v2] ALSA: Add snd_ctl_update() to dynamically update a control

[Clemens Ladisch]

Dimitris Papastamos wrote:
> Add a function to dynamically update a given control.  If the
> control does not already exist, a third parameter is used to determine
> whether to actually add that control.  This is useful in cases where
> downloadable firmware at runtime can add or update existing controls.

> + * Updates the control instance created via snd_ctl_new() or
> + * snd_ctl_new1() of the given card.

This description was copied from snd_ctl_add(), and the "created ..."
part isn't really useful here.

It should be mentioned that the old control is destroyed, because some
drivers store volatile state in the control's private_data/_value, or
keep a pointer to the control('s ID) to be able to call snd_ctl_notify()
for it later.

> + * Returns zero if successful, or a negative error code on failure.
> + *

superfluous empty line

> +int snd_ctl_update(struct snd_card *card, struct snd_kcontrol *kcontrol,
> +		   int add_on_update)

add_on_update should be bool

> +	up_write(&card->controls_rwsem);
> +	ret = snd_ctl_add(card, kcontrol);

Doing this outside the semaphore doesn't look good.

I wonder whether snd_ctl_add() is the right function to use, because it
will assign a different numid event if the control didn't change.

> A separate patch needs to be made to make ALSA Mixer cope with this.

What happens?


Regards,
Clemens

Re: [PATCH 1/2 v2] ALSA: Add snd_ctl_update() to dynamically update a control

[Takashi Iwai]

At Tue, 15 Mar 2011 15:17:58 +0100,
Clemens Ladisch wrote:
> 
> > +	up_write(&card->controls_rwsem);
> > +	ret = snd_ctl_add(card, kcontrol);
> 
> Doing this outside the semaphore doesn't look good.

Using an internal function without semaphore would be preferred, yes.

> I wonder whether snd_ctl_add() is the right function to use, because it
> will assign a different numid event if the control didn't change.

Hm, actually the function can compare whether it's really changed or
not.  If any change, it makes really sense to assign a different
numid.


Takashi

Re: [PATCH 1/2 v2] ALSA: Add snd_ctl_update() to dynamically update a control

[Dimitris Papastamos]

On Tue, Mar 15, 2011 at 03:17:58PM +0100, Clemens Ladisch wrote:
> Doing this outside the semaphore doesn't look good.

Yes, I was thinking about the same thing, wasn't sure about nesting rw
semaphores though.

> I wonder whether snd_ctl_add() is the right function to use, because it
> will assign a different numid event if the control didn't change.

Hm, indeed.  I can probably inline a slightly different version of
snd_ctl_add()?

> > A separate patch needs to be made to make ALSA Mixer cope with this.
> 
> What happens?

It doesn't break or anything hopefully.  It will probably need support for
handling the update of the controls (without exiting and starting it
again).  I've only tested it against alsa-utils 1.0.20 and it worked
fine with a few changes (updating the controls on the fly).

Thanks,
Dimitris

Re: [PATCH 1/2 v2] ALSA: Add snd_ctl_update() to dynamically update a control

[Raymond Yau]

2011/3/15 Takashi Iwai <tiwai@suse.de>

> At Tue, 15 Mar 2011 15:17:58 +0100,
> Clemens Ladisch wrote:
> >
> > > +   up_write(&card->controls_rwsem);
> > > +   ret = snd_ctl_add(card, kcontrol);
> >
> > Doing this outside the semaphore doesn't look good.
>
> Using an internal function without semaphore would be preferred, yes.
>
> > I wonder whether snd_ctl_add() is the right function to use, because it
> > will assign a different numid event if the control didn't change.
>
> Hm, actually the function can compare whether it's really changed or
> not.  If any change, it makes really sense to assign a different
> numid.
>
>
>

does it mean that the driver can update "Input Source" controls for those
HDA codec on a motherboard with 3 audio jacks at rear panel ?

i.e. remove/add "Rear Mic" and "Line in" from/to "Input Source" when user
switch "channel_mode" between 2 and 6 or  switch"smart51" on/off

Re: [PATCH 1/2 v2] ALSA: Add snd_ctl_update() to dynamically update a control

[Takashi Iwai]

At Wed, 16 Mar 2011 08:56:36 +0800,
Raymond Yau wrote:
> 
> 2011/3/15 Takashi Iwai <tiwai@suse.de>
> 
> > At Tue, 15 Mar 2011 15:17:58 +0100,
> > Clemens Ladisch wrote:
> > >
> > > > +   up_write(&card->controls_rwsem);
> > > > +   ret = snd_ctl_add(card, kcontrol);
> > >
> > > Doing this outside the semaphore doesn't look good.
> >
> > Using an internal function without semaphore would be preferred, yes.
> >
> > > I wonder whether snd_ctl_add() is the right function to use, because it
> > > will assign a different numid event if the control didn't change.
> >
> > Hm, actually the function can compare whether it's really changed or
> > not.  If any change, it makes really sense to assign a different
> > numid.
> >
> >
> >
> 
> does it mean that the driver can update "Input Source" controls for those
> HDA codec on a motherboard with 3 audio jacks at rear panel ?
> 
> i.e. remove/add "Rear Mic" and "Line in" from/to "Input Source" when user
> switch "channel_mode" between 2 and 6 or  switch"smart51" on/off

In theory, yes.  But, practically, I'm afraid that this is rather
confusing for normal desktop apps.  Most of mixer apps aren't written
robustly for handling the dynamically changed control elements.


Takashi

Re: [PATCH 1/2 v2] ALSA: Add snd_ctl_update() to dynamically update a control

[Raymond Yau]

2011/3/16 Dimitris Papastamos <dp@opensource.wolfsonmicro.com>

> On Tue, Mar 15, 2011 at 03:17:58PM +0100, Clemens Ladisch wrote:
>
>
> > > A separate patch needs to be made to make ALSA Mixer cope with this.
> >
> > What happens?
>
> It doesn't break or anything hopefully.  It will probably need support for
> handling the update of the controls (without exiting and starting it
> again).  I've only tested it against alsa-utils 1.0.20 and it worked
> fine with a few changes (updating the controls on the fly).
>
>
It depend on how the driver update control, since snd_mixer_selem just
combine "volume" and "switch" controls of same name into a single selem

if the driver just change the name of volume control, I guess alsamixer
still need to reload the mixer