cleanup controllers in snd-usb-caiaq

Hi,

this patch does some code cleanup in snd-usb-caiaq. No functional change whatsoever. Version number bumped to 1.3.9.

Best regards,
Daniel

[2 caiaq-controls-cleanup.diff <text/x-diff; us-ascii (7bit)>] Cleanup controler registration in snd-usb-caiaq.

Signed-off-by Daniel Mack <[email protected]>

sound/usb/caiaq/caiaq-control.c +14 -40

index 798ca12..fc3aed9 100644
@@ -247,6 +247,15 @@ static struct caiaq_controller a8dj_controller[] = {
 	{ "Software lock", 			40 		}
 };
 
+#define ADD_ALL_CONTROLS(a) \
+	for (i = 0; i < ARRAY_SIZE(a); i++) {				\
+		struct caiaq_controller *c = a + i;			\
+		kcontrol_template.name = c->name;			\
+		kcontrol_template.private_value = c->index;		\
+		kc = snd_ctl_new1(&kcontrol_template, dev);		\
+		snd_ctl_add(dev->chip.card, kc);			\
+	}
+
 int __devinit snd_usb_caiaq_control_init(struct snd_usb_caiaqdev *dev)
 {
 	int i;

@@ -254,59 +263,24 @@ int __devinit snd_usb_caiaq_control_init(struct snd_usb_caiaqdev *dev)
 
 	switch (dev->chip.usb_id) {
 	case USB_ID(USB_VID_NATIVEINSTRUMENTS, USB_PID_AK1):
-		for (i = 0; i < ARRAY_SIZE(ak1_controller); i++) {
-			struct caiaq_controller *c = ak1_controller + i;
-			kcontrol_template.name = c->name;
-			kcontrol_template.private_value = c->index;
-			kc = snd_ctl_new1(&kcontrol_template, dev);
-			snd_ctl_add(dev->chip.card, kc);
-		}
-
+		ADD_ALL_CONTROLS(ak1_controller);
 		break;
 
 	case USB_ID(USB_VID_NATIVEINSTRUMENTS, USB_PID_RIGKONTROL2):
-		for (i = 0; i < ARRAY_SIZE(rk2_controller); i++) {
-			struct caiaq_controller *c = rk2_controller + i;
-			kcontrol_template.name = c->name;
-			kcontrol_template.private_value = c->index;
-			kc = snd_ctl_new1(&kcontrol_template, dev);
-			snd_ctl_add(dev->chip.card, kc);
-		}
-
+		ADD_ALL_CONTROLS(rk2_controller);
 		break;
 
 	case USB_ID(USB_VID_NATIVEINSTRUMENTS, USB_PID_RIGKONTROL3):
-		for (i = 0; i < ARRAY_SIZE(rk3_controller); i++) {
-			struct caiaq_controller *c = rk3_controller + i;
-			kcontrol_template.name = c->name;
-			kcontrol_template.private_value = c->index;
-			kc = snd_ctl_new1(&kcontrol_template, dev);
-			snd_ctl_add(dev->chip.card, kc);
-		}
-
+		ADD_ALL_CONTROLS(rk3_controller);
 		break;
 
 	case USB_ID(USB_VID_NATIVEINSTRUMENTS, USB_PID_KORECONTROLLER):
 	case USB_ID(USB_VID_NATIVEINSTRUMENTS, USB_PID_KORECONTROLLER2):
-		for (i = 0; i < ARRAY_SIZE(kore_controller); i++) {
-			struct caiaq_controller *c = kore_controller + i;
-			kcontrol_template.name = c->name;
-			kcontrol_template.private_value = c->index;
-			kc = snd_ctl_new1(&kcontrol_template, dev);
-			snd_ctl_add(dev->chip.card, kc);
-		}
-
+		ADD_ALL_CONTROLS(kore_controller);
 		break;
 
 	case USB_ID(USB_VID_NATIVEINSTRUMENTS, USB_PID_AUDIO8DJ):
-		for (i = 0; i < ARRAY_SIZE(a8dj_controller); i++) {
-			struct caiaq_controller *c = a8dj_controller + i;
-			kcontrol_template.name = c->name;
-			kcontrol_template.private_value = c->index;
-			kc = snd_ctl_new1(&kcontrol_template, dev);
-			snd_ctl_add(dev->chip.card, kc);
-		}
-
+		ADD_ALL_CONTROLS(a8dj_controller);
 		break;
 	}
 

sound/usb/caiaq/caiaq-device.c +1 -1

index 8317508..b143ef7 100644
@@ -42,7 +42,7 @@
 #endif
 
 MODULE_AUTHOR("Daniel Mack <[email protected]>");
-MODULE_DESCRIPTION("caiaq USB audio, version 1.3.8");
+MODULE_DESCRIPTION("caiaq USB audio, version 1.3.9");
 MODULE_LICENSE("GPL");
 MODULE_SUPPORTED_DEVICE("{{Native Instruments, RigKontrol2},"
 			 "{Native Instruments, RigKontrol3},"

[3 <text/plain; us-ascii (7bit)>]
_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Quoted 2 lines

this patch does some code cleanup in snd-usb-caiaq. No functional change whatsoever. Version number bumped to 1.3.9.

A macro for such is ugly and potentially dangerous, IMO. Make a function instead.

Also, please add an error check of the return value from snd_ctl_add().

Hi Takashi,

Quoted 3 lines · Takashi Iwai

Quoted 2 lines

this patch does some code cleanup in snd-usb-caiaq. No functional change whatsoever. Version number bumped to 1.3.9.

Quoted 5 lines

A macro for such is ugly and potentially dangerous, IMO. Make a function instead.

Also, please add an error check of the return value from snd_ctl_add().

Ok, agreed and done. However, I considered it a good idea as it avoided writing the array's name more than once.

Daniel

[2 caiaq-controls-cleanup.diff <text/x-diff; us-ascii (7bit)>]
snd-usb-caiaq: clean up the control adding code by moving dulpicate code to a function.

Signed-off-by Daniel Mack <[email protected]>

sound/usb/caiaq/caiaq-control.c +30 -43

index 798ca12..ccd763d 100644
@@ -247,69 +247,56 @@ static struct caiaq_controller a8dj_controller[] = {
 	{ "Software lock", 			40 		}
 };
 
-int __devinit snd_usb_caiaq_control_init(struct snd_usb_caiaqdev *dev)
+static int __devinit add_controls(struct caiaq_controller *c, int num,
+				  struct snd_usb_caiaqdev *dev)
 {
-	int i;
+	int i, ret;
 	struct snd_kcontrol *kc;
 
+	for (i = 0; i < num; i++, c++) {
+		kcontrol_template.name = c->name;
+		kcontrol_template.private_value = c->index;
+		kc = snd_ctl_new1(&kcontrol_template, dev);
+		ret = snd_ctl_add(dev->chip.card, kc);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
+int __devinit snd_usb_caiaq_control_init(struct snd_usb_caiaqdev *dev)
+{
+	int ret = 0;
+
 	switch (dev->chip.usb_id) {
 	case USB_ID(USB_VID_NATIVEINSTRUMENTS, USB_PID_AK1):
-		for (i = 0; i < ARRAY_SIZE(ak1_controller); i++) {
-			struct caiaq_controller *c = ak1_controller + i;
-			kcontrol_template.name = c->name;
-			kcontrol_template.private_value = c->index;
-			kc = snd_ctl_new1(&kcontrol_template, dev);
-			snd_ctl_add(dev->chip.card, kc);
-		}
-
+		ret = add_controls(ak1_controller,
+			ARRAY_SIZE(ak1_controller), dev);
 		break;
 
 	case USB_ID(USB_VID_NATIVEINSTRUMENTS, USB_PID_RIGKONTROL2):
-		for (i = 0; i < ARRAY_SIZE(rk2_controller); i++) {
-			struct caiaq_controller *c = rk2_controller + i;
-			kcontrol_template.name = c->name;
-			kcontrol_template.private_value = c->index;
-			kc = snd_ctl_new1(&kcontrol_template, dev);
-			snd_ctl_add(dev->chip.card, kc);
-		}
-
+		ret = add_controls(rk2_controller,
+			ARRAY_SIZE(rk2_controller), dev);
 		break;
 
 	case USB_ID(USB_VID_NATIVEINSTRUMENTS, USB_PID_RIGKONTROL3):
-		for (i = 0; i < ARRAY_SIZE(rk3_controller); i++) {
-			struct caiaq_controller *c = rk3_controller + i;
-			kcontrol_template.name = c->name;
-			kcontrol_template.private_value = c->index;
-			kc = snd_ctl_new1(&kcontrol_template, dev);
-			snd_ctl_add(dev->chip.card, kc);
-		}
-
+		ret = add_controls(rk3_controller,
+			ARRAY_SIZE(rk3_controller), dev);
 		break;
 
 	case USB_ID(USB_VID_NATIVEINSTRUMENTS, USB_PID_KORECONTROLLER):
 	case USB_ID(USB_VID_NATIVEINSTRUMENTS, USB_PID_KORECONTROLLER2):
-		for (i = 0; i < ARRAY_SIZE(kore_controller); i++) {
-			struct caiaq_controller *c = kore_controller + i;
-			kcontrol_template.name = c->name;
-			kcontrol_template.private_value = c->index;
-			kc = snd_ctl_new1(&kcontrol_template, dev);
-			snd_ctl_add(dev->chip.card, kc);
-		}
-
+		ret = add_controls(kore_controller,
+			ARRAY_SIZE(kore_controller), dev);
 		break;
 
 	case USB_ID(USB_VID_NATIVEINSTRUMENTS, USB_PID_AUDIO8DJ):
-		for (i = 0; i < ARRAY_SIZE(a8dj_controller); i++) {
-			struct caiaq_controller *c = a8dj_controller + i;
-			kcontrol_template.name = c->name;
-			kcontrol_template.private_value = c->index;
-			kc = snd_ctl_new1(&kcontrol_template, dev);
-			snd_ctl_add(dev->chip.card, kc);
-		}
-
+		ret = add_controls(a8dj_controller,
+			ARRAY_SIZE(a8dj_controller), dev);
 		break;
 	}
 
-	return 0;
+	return ret;
 }
 

sound/usb/caiaq/caiaq-device.c +1 -1

index 8317508..b143ef7 100644
@@ -42,7 +42,7 @@
 #endif
 
 MODULE_AUTHOR("Daniel Mack <[email protected]>");
-MODULE_DESCRIPTION("caiaq USB audio, version 1.3.8");
+MODULE_DESCRIPTION("caiaq USB audio, version 1.3.9");
 MODULE_LICENSE("GPL");
 MODULE_SUPPORTED_DEVICE("{{Native Instruments, RigKontrol2},"
 			 "{Native Instruments, RigKontrol3},"

Quoted 3 lines

Ok, agreed and done. However, I considered it a good idea as it avoided
writing the array's name more than once.

In that case, you can use a macro, such as,

#define ADD_CTRLS(c, list) add_controls(c, ARRAY_SIZE(list), (list))

But, don't make a too complex block as a macro. A function often gives a better code (in this case it'll get smaller) in addition to a better readability than a big macro.

Quoted 6 lines · Takashi Iwai

Quoted 3 lines

Ok, agreed and done. However, I considered it a good idea as it avoided
writing the array's name more than once.

In that case, you can use a macro, such as,

Quoted 5 lines

#define ADD_CTRLS(c, list) add_controls(c, ARRAY_SIZE(list), (list))

But, don't make a too complex block as a macro. A function often gives a better code (in this case it'll get smaller) in addition to a better readability than a big macro.

Ok. Do you want me to do this or are you fine with the code as-is?