[alsa-devel] [PATCH] 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
At Wed, 26 Nov 2008 16:19:01 +0100, Daniel Mack wrote:
Hi,
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().
thanks,
Takashi
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 daniel@caiaq.de
diff --git a/sound/usb/caiaq/caiaq-control.c b/sound/usb/caiaq/caiaq-control.c index 798ca12..fc3aed9 100644 --- a/sound/usb/caiaq/caiaq-control.c +++ b/sound/usb/caiaq/caiaq-control.c @@ -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);diff --git a/sound/usb/caiaq/caiaq-device.c b/sound/usb/caiaq/caiaq-device.c index 8317508..b143ef7 100644 --- a/sound/usb/caiaq/caiaq-device.c +++ b/sound/usb/caiaq/caiaq-device.c @@ -42,7 +42,7 @@ #endif
MODULE_AUTHOR("Daniel Mack daniel@caiaq.de"); -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 Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Hi Takashi,
On Wed, Nov 26, 2008 at 04:44:08PM +0100, Takashi Iwai wrote:
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().
Ok, agreed and done. However, I considered it a good idea as it avoided writing the array's name more than once.
Daniel
At Wed, 26 Nov 2008 19:57:20 +0100, Daniel Mack wrote:
Hi Takashi,
On Wed, Nov 26, 2008 at 04:44:08PM +0100, Takashi Iwai wrote:
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().
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.
thanks,
Takashi
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 daniel@caiaq.de
diff --git a/sound/usb/caiaq/caiaq-control.c b/sound/usb/caiaq/caiaq-control.c index 798ca12..ccd763d 100644 --- a/sound/usb/caiaq/caiaq-control.c +++ b/sound/usb/caiaq/caiaq-control.c @@ -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);
- return 0;
- return ret;
}
diff --git a/sound/usb/caiaq/caiaq-device.c b/sound/usb/caiaq/caiaq-device.c index 8317508..b143ef7 100644 --- a/sound/usb/caiaq/caiaq-device.c +++ b/sound/usb/caiaq/caiaq-device.c @@ -42,7 +42,7 @@ #endif
MODULE_AUTHOR("Daniel Mack daniel@caiaq.de"); -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},"
On 26.11.2008, at 21:28, Takashi Iwai wrote:
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.
Ok. Do you want me to do this or are you fine with the code as-is?
Daniel
At Wed, 26 Nov 2008 21:31:59 +0100, Daniel Mack wrote:
On 26.11.2008, at 21:28, Takashi Iwai wrote:
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.
Ok. Do you want me to do this or are you fine with the code as-is?
I have no big preference. But, since it's not bad to have ARRAY_SIZE() explicitly in the code (then you notice that it's no null-terminated array), I applied your previous one now.
thanks,
Takashi
participants (2)
-
Daniel Mack -
Takashi Iwai