Re: [PATCH] usb-audio: Add mixer control for Digidesign Mbox 1 clock source

[Takashi Sakamoto <o-takashi@sakamocchi.jp>]

[-- Attachment #1: Type: text/plain, Size: 1479 bytes --]

Hi Damien,

On Oct 31 2014 13:02, Damien Zammit wrote:
> Hi, sorry for the style problems.  See attached for better version.

4 points.

1. please use white-spaces or tabs for indentation to align parameters 
to function-start blacket:

 > +static int snd_mbox1_switch_put(struct snd_kcontrol *kctl,
 > +                                       struct snd_ctl_elem_value 
*ucontrol)

 > +static int snd_mbox1_switch_info(struct snd_kcontrol *kcontrol,
 > +                                       struct snd_ctl_elem_info *uinfo)

 > +static int snd_mbox1_switch_put(struct snd_kcontrol *kctl,
 > +                                       struct snd_ctl_elem_value 
*ucontrol)

2.please use 'const char *const' for immutable arrays for immutable strings:
 > +       static const char *texts[2] = {"Internal",
 > +                                      "S/PDIF"
 > +       };
 > +

3. I think in your case, snd_ctl_enum_info() is available in struct 
snd_kcontrol_new.info() callback. Please read this thread:
[alsa-devel] [PATCH 00/43] Spread usage of snd_ctl_elem_info()
http://mailman.alsa-project.org/pipermail/alsa-devel/2014-October/082573.html

4. I think 'err' local variable in snd_mbox1_create_sync_switch() can be 
removed because it's assign and evaluated at once:

 > +       err = snd_ctl_add(mixer->chip->card, kctl);
 > +       if (err < 0)
 > +               return err;
 > +
 > +       return 0;

See attached patch.


Regards

Takashi Sakamoto
o-takashi@sakamocchi.jp

[-- Attachment #2: 0001-Comments-for-mbox1-spdif-2.patch.patch --]
[-- Type: text/x-patch, Size: 2570 bytes --]

>From 6e889e1fc7d7101cfef14484849a412b002798e9 Mon Sep 17 00:00:00 2001
From: Takashi Sakamoto <mocchi@MocchiSakamoto64.miraclelinux.com>
Date: Fri, 31 Oct 2014 13:17:19 +0900
Subject: [PATCH] Comments for mbox1-spdif-2.patch

See:
[alsa-devel] [PATCH] usb-audio: Add mixer control for Digidesign Mbox 1 clock source
http://mailman.alsa-project.org/pipermail/alsa-devel/2014-October/083264.html
---
 sound/usb/mixer_quirks.c | 28 ++++++++--------------------
 1 file changed, 8 insertions(+), 20 deletions(-)

diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c
index 6a21dec..87f121f 100644
--- a/sound/usb/mixer_quirks.c
+++ b/sound/usb/mixer_quirks.c
@@ -574,7 +574,7 @@ struct snd_mbox1_switch_priv_val {
 };
 
 static int snd_mbox1_switch_get(struct snd_kcontrol *kctl,
-					struct snd_ctl_elem_value *ucontrol)
+				struct snd_ctl_elem_value *ucontrol)
 {
 	struct snd_mbox1_switch_priv_val *pval;
 	unsigned char value;
@@ -597,7 +597,7 @@ static int snd_mbox1_switch_get(struct snd_kcontrol *kctl,
 }
 
 static int snd_mbox1_switch_put(struct snd_kcontrol *kctl,
-					struct snd_ctl_elem_value *ucontrol)
+				struct snd_ctl_elem_value *ucontrol)
 {
 	struct snd_usb_audio *chip;
 	struct snd_mbox1_switch_priv_val *pval;
@@ -692,21 +692,14 @@ static int snd_mbox1_switch_put(struct snd_kcontrol *kctl,
 }
 
 static int snd_mbox1_switch_info(struct snd_kcontrol *kcontrol,
-					struct snd_ctl_elem_info *uinfo)
+				 struct snd_ctl_elem_info *uinfo)
 {
-	static const char *texts[2] = {"Internal",
-				       "S/PDIF"
+	static const char *const texts[2] = {
+		"Internal",
+		"S/PDIF"
 	};
 
-	uinfo->type = SNDRV_CTL_ELEM_TYPE_ENUMERATED;
-	uinfo->count = 1;
-	uinfo->value.enumerated.items = 2;
-	if (uinfo->value.enumerated.item > 1)
-		uinfo->value.enumerated.item = 1;
-	strcpy(uinfo->value.enumerated.name,
-		texts[uinfo->value.enumerated.item]);
-
-	return 0;
+	return snd_ctl_enum_info(uinfo, 1, ARRAY_SIZE(texts), texts);;
 }
 
 static int snd_mbox1_create_sync_switch(struct usb_mixer_interface *mixer)
@@ -722,7 +715,6 @@ static int snd_mbox1_create_sync_switch(struct usb_mixer_interface *mixer)
 		.put = snd_mbox1_switch_put
 	};
 
-	int err;
 	struct snd_kcontrol *kctl;
 	struct snd_mbox1_switch_priv_val *pval;
 
@@ -741,11 +733,7 @@ static int snd_mbox1_create_sync_switch(struct usb_mixer_interface *mixer)
 		return -ENOMEM;
 	}
 
-	err = snd_ctl_add(mixer->chip->card, kctl);
-	if (err < 0)
-		return err;
-
-	return 0;
+	return snd_ctl_add(mixer->chip->card, kctl);
 }
 
 /* Native Instruments device quirks */
-- 
1.9.1


[-- Attachment #3: Type: text/plain, Size: 0 bytes --]