[PATCH 0/4] ALSA: Cleanup with snd_ctl_add_followers()

[PATCH 0/4] ALSA: Cleanup with snd_ctl_add_followers()

[Takashi Iwai]

This is another set of cleanup patches: introduce a new standard
helper for adding vmaster control elements, and replace the open code
in a few drivers with it.


Takashi

===

Takashi Iwai (4):
  ALSA: vmaster: Add snd_ctl_add_followers() helper
  ALSA: ac97: Use the standard snd_ctl_add_followers() helper
  ALSA: ca0106: Use the standard snd_ctl_add_followers() helper
  ALSA: ice1712: Use the standard snd_ctl_add_followers() helper

 include/sound/control.h         |  3 +++
 sound/core/vmaster.c            | 28 ++++++++++++++++++++++++++++
 sound/pci/ac97/ac97_patch.c     | 16 +++-------------
 sound/pci/ca0106/ca0106_mixer.c | 19 ++++++-------------
 sound/pci/ice1712/juli.c        | 19 +++----------------
 sound/pci/ice1712/quartet.c     | 15 +++------------
 6 files changed, 46 insertions(+), 54 deletions(-)

-- 
2.35.3

[PATCH 1/4] ALSA: vmaster: Add snd_ctl_add_followers() helper

[Takashi Iwai]

Add a new helper to add multiple vmaster followers in a shot.  The
same function was open-coded in various places, and this helper
replaces them.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 include/sound/control.h |  3 +++
 sound/core/vmaster.c    | 28 ++++++++++++++++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/include/sound/control.h b/include/sound/control.h
index 69d950a34ca3..9a4f4f7138da 100644
--- a/include/sound/control.h
+++ b/include/sound/control.h
@@ -262,6 +262,9 @@ snd_ctl_add_follower(struct snd_kcontrol *master, struct snd_kcontrol *follower)
 	return _snd_ctl_add_follower(master, follower, 0);
 }
 
+int snd_ctl_add_followers(struct snd_card *card, struct snd_kcontrol *master,
+			  const char * const *list);
+
 /**
  * snd_ctl_add_follower_uncached - Add a virtual follower control
  * @master: vmaster element
diff --git a/sound/core/vmaster.c b/sound/core/vmaster.c
index d0f11f37889b..378d2c7c3d4a 100644
--- a/sound/core/vmaster.c
+++ b/sound/core/vmaster.c
@@ -280,6 +280,34 @@ int _snd_ctl_add_follower(struct snd_kcontrol *master,
 }
 EXPORT_SYMBOL(_snd_ctl_add_follower);
 
+/**
+ * snd_ctl_add_followers - add multiple followers to vmaster
+ * @card: card instance
+ * @master: the target vmaster kcontrol object
+ * @list: NULL-terminated list of name strings of followers to be added
+ *
+ * Adds the multiple follower kcontrols with the given names.
+ * Returns 0 for success or a negative error code.
+ */
+int snd_ctl_add_followers(struct snd_card *card, struct snd_kcontrol *master,
+			  const char * const *list)
+{
+	struct snd_kcontrol *follower;
+	int err;
+
+	for (; *list; list++) {
+		follower = snd_ctl_find_id_mixer(card, *list);
+		if (follower) {
+			err = snd_ctl_add_follower(master, follower);
+			if (err < 0)
+				return err;
+		}
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(snd_ctl_add_followers);
+
 /*
  * ctl callbacks for master controls
  */
-- 
2.35.3

[PATCH 2/4] ALSA: ac97: Use the standard snd_ctl_add_followers() helper

[Takashi Iwai]

Instead of open-code, use the new standard helper to manage vmaster
stuff for code simplification.

Except for a debug print, there should be no functional change.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/pci/ac97/ac97_patch.c | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/sound/pci/ac97/ac97_patch.c b/sound/pci/ac97/ac97_patch.c
index ccfd9c7bf900..09626aed967d 100644
--- a/sound/pci/ac97/ac97_patch.c
+++ b/sound/pci/ac97/ac97_patch.c
@@ -3440,7 +3440,6 @@ static int snd_ac97_add_vmaster(struct snd_ac97 *ac97, char *name,
 				const char * const *followers)
 {
 	struct snd_kcontrol *kctl;
-	const char * const *s;
 	int err;
 
 	kctl = snd_ctl_make_virtual_master(name, tlv);
@@ -3450,19 +3449,10 @@ static int snd_ac97_add_vmaster(struct snd_ac97 *ac97, char *name,
 	if (err < 0)
 		return err;
 
-	for (s = followers; *s; s++) {
-		struct snd_kcontrol *sctl;
+	err = snd_ctl_add_followers(ac97->bus->card, kctl, followers);
+	if (err < 0)
+		return err;
 
-		sctl = snd_ac97_find_mixer_ctl(ac97, *s);
-		if (!sctl) {
-			dev_dbg(ac97->bus->card->dev,
-				"Cannot find follower %s, skipped\n", *s);
-			continue;
-		}
-		err = snd_ctl_add_follower(kctl, sctl);
-		if (err < 0)
-			return err;
-	}
 	return 0;
 }
 
-- 
2.35.3

Re: [PATCH 2/4] ALSA: ac97: Use the standard snd_ctl_add_followers() helper

[Oswald Buddenhagen]

On Thu, Jul 20, 2023 at 10:25:52AM +0200, Takashi Iwai wrote:
>+	err = snd_ctl_add_followers(ac97->bus->card, kctl, followers);
>+	if (err < 0)
>+		return err;
> 
> 	return 0;
>
this seems a bit pointless - you could just
   return snd_ctl_add_followers(...
directly.

(more instances may be affected, but i don't see it due to little diff 
context.)

> }

regards

Re: [PATCH 2/4] ALSA: ac97: Use the standard snd_ctl_add_followers() helper

[Takashi Iwai]

On Thu, 20 Jul 2023 12:35:22 +0200,
Oswald Buddenhagen wrote:
> 
> On Thu, Jul 20, 2023 at 10:25:52AM +0200, Takashi Iwai wrote:
> > +	err = snd_ctl_add_followers(ac97->bus->card, kctl, followers);
> > +	if (err < 0)
> > +		return err;
> > 
> > 	return 0;
> > 
> this seems a bit pointless - you could just
>   return snd_ctl_add_followers(...
> directly.

Right, this can be simplified.  Will fold into the v2 patch.


thanks,

Takashi

[PATCH 3/4] ALSA: ca0106: Use the standard snd_ctl_add_followers() helper

[Takashi Iwai]

Instead of open-code, use the new standard helper to manage vmaster
stuff for code simplification.

Also, handle the errors from the helper more properly instead of
silently ignoring.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/pci/ca0106/ca0106_mixer.c | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/sound/pci/ca0106/ca0106_mixer.c b/sound/pci/ca0106/ca0106_mixer.c
index 2f37d2c3dd38..1d5a899b2c24 100644
--- a/sound/pci/ca0106/ca0106_mixer.c
+++ b/sound/pci/ca0106/ca0106_mixer.c
@@ -751,17 +751,6 @@ static const char * const follower_sws[] = {
 	NULL
 };
 
-static void add_followers(struct snd_card *card,
-			  struct snd_kcontrol *master, const char * const *list)
-{
-	for (; *list; list++) {
-		struct snd_kcontrol *follower =
-			snd_ctl_find_id_mixer(card, *list);
-		if (follower)
-			snd_ctl_add_follower(master, follower);
-	}
-}
-
 int snd_ca0106_mixer(struct snd_ca0106 *emu)
 {
 	int err;
@@ -843,7 +832,9 @@ int snd_ca0106_mixer(struct snd_ca0106 *emu)
 	err = snd_ctl_add(card, vmaster);
 	if (err < 0)
 		return err;
-	add_followers(card, vmaster, follower_vols);
+	err = snd_ctl_add_followers(card, vmaster, follower_vols);
+	if (err < 0)
+		return err;
 
 	if (emu->details->spi_dac) {
 		vmaster = snd_ctl_make_virtual_master("Master Playback Switch",
@@ -853,7 +844,9 @@ int snd_ca0106_mixer(struct snd_ca0106 *emu)
 		err = snd_ctl_add(card, vmaster);
 		if (err < 0)
 			return err;
-		add_followers(card, vmaster, follower_sws);
+		err = snd_ctl_add_followers(card, vmaster, follower_sws);
+		if (err < 0)
+			return err;
 	}
 
 	strcpy(card->mixername, "CA0106");
-- 
2.35.3

[PATCH 4/4] ALSA: ice1712: Use the standard snd_ctl_add_followers() helper

[Takashi Iwai]

Instead of open-code, use the new standard helper to manage vmaster
stuff for code simplification.

Also, handle the errors from the helper more properly instead of
silently ignoring.

The code changes the call order of snd_ctl_add() of the vmaster object
and its followers for avoiding the possible memory leaks at error
path.  But there should be no difference in the functionality.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/pci/ice1712/juli.c    | 19 +++----------------
 sound/pci/ice1712/quartet.c | 15 +++------------
 2 files changed, 6 insertions(+), 28 deletions(-)

diff --git a/sound/pci/ice1712/juli.c b/sound/pci/ice1712/juli.c
index d80ecf1edc16..d679842ae1bd 100644
--- a/sound/pci/ice1712/juli.c
+++ b/sound/pci/ice1712/juli.c
@@ -408,21 +408,6 @@ static const char * const follower_vols[] = {
 static
 DECLARE_TLV_DB_SCALE(juli_master_db_scale, -6350, 50, 1);
 
-static void add_followers(struct snd_card *card,
-			  struct snd_kcontrol *master,
-			  const char * const *list)
-{
-	for (; *list; list++) {
-		struct snd_kcontrol *follower =
-			snd_ctl_find_id_mixer(card, *list);
-		/* dev_dbg(card->dev, "add_followers - %s\n", *list); */
-		if (follower) {
-			/* dev_dbg(card->dev, "follower %s found\n", *list); */
-			snd_ctl_add_follower(master, follower);
-		}
-	}
-}
-
 static int juli_add_controls(struct snd_ice1712 *ice)
 {
 	struct juli_spec *spec = ice->spec;
@@ -445,8 +430,10 @@ static int juli_add_controls(struct snd_ice1712 *ice)
 					      juli_master_db_scale);
 	if (!vmaster)
 		return -ENOMEM;
-	add_followers(ice->card, vmaster, follower_vols);
 	err = snd_ctl_add(ice->card, vmaster);
+	if (err < 0)
+		return err;
+	err = snd_ctl_add_followers(ice->card, vmaster, follower_vols);
 	if (err < 0)
 		return err;
 
diff --git a/sound/pci/ice1712/quartet.c b/sound/pci/ice1712/quartet.c
index 9450c4b104f7..f61ee9f5c754 100644
--- a/sound/pci/ice1712/quartet.c
+++ b/sound/pci/ice1712/quartet.c
@@ -766,17 +766,6 @@ static const char * const follower_vols[] = {
 static
 DECLARE_TLV_DB_SCALE(qtet_master_db_scale, -6350, 50, 1);
 
-static void add_followers(struct snd_card *card,
-			  struct snd_kcontrol *master, const char * const *list)
-{
-	for (; *list; list++) {
-		struct snd_kcontrol *follower =
-			snd_ctl_find_id_mixer(card, *list);
-		if (follower)
-			snd_ctl_add_follower(master, follower);
-	}
-}
-
 static int qtet_add_controls(struct snd_ice1712 *ice)
 {
 	struct qtet_spec *spec = ice->spec;
@@ -797,8 +786,10 @@ static int qtet_add_controls(struct snd_ice1712 *ice)
 			qtet_master_db_scale);
 	if (!vmaster)
 		return -ENOMEM;
-	add_followers(ice->card, vmaster, follower_vols);
 	err = snd_ctl_add(ice->card, vmaster);
+	if (err < 0)
+		return err;
+	err = snd_ctl_add_followers(ice->card, vmaster, follower_vols);
 	if (err < 0)
 		return err;
 	/* only capture SPDIF over AK4113 */
-- 
2.35.3