At Fri, 16 Oct 2009 21:52:48 +0200, LCID Fire wrote:
Signed-off-by: Andreas Bergmeier andreas.bergmeier@gmx.net
Thanks for the renewed patch. Below is a quick review.
diff --git a/sound/usb/usbmixer.c b/sound/usb/usbmixer.c index 00397c8..532d02d 100644 --- a/sound/usb/usbmixer.c +++ b/sound/usb/usbmixer.c @@ -2013,6 +2013,281 @@ static void snd_audigy2nx_proc_read(struct snd_info_entry *entry, } }
+static void snd_usb_cleanup_interrupt_urb(struct urb *pUrb)
Avoid Windows-style variable names as much as possible.
+{
- if (pUrb->transfer_buffer_length > 0)
kfree(pUrb->transfer_buffer);- kfree(pUrb->context);
- usb_free_urb(pUrb);
+}
+/* Wrapper for setting and submitting the interrupt urb().*/ +static int snd_usb_interrupt_trans(struct usb_device *dev,
- unsigned int pipe, void *data, __u16 size, usb_complete_t callback)
No need to use __u16. Here it can be simply unsigned int.
+{
- int err = 0;
- void *buf = NULL;
- struct urb *pUrb = NULL;
No need for initializations of all of these.
(snip)
+/* gets called multiple times! */ +static int snd_sl_controls_create(struct usb_mixer_interface *mixer) +{
- int i, err;
- for (i = 0; i < ARRAY_SIZE(snd_sl_controls); ++i) {
struct snd_kcontrol *kcontrol =snd_ctl_new1(&snd_sl_controls[i], mixer);
Missing NULL check here.
/* Since we only need one control and the routine* is called multiple times we have to ignore all* attempts to attach controls multiple times*/
Hm, why is it called so many times? Can't it be limited with a check of iface or altsetting?
if (snd_ctl_find_id(mixer->chip->card, &kcontrol->id)) {/* we found the control - it is already present* so just continue*/snd_ctl_free_one(kcontrol);continue;}/* add frees the control if on err < 0! */err = snd_ctl_add(mixer->chip->card,kcontrol);if (err < 0)return err;/* Make sure device is set to default */err = snd_sl_phono_set(kcontrol, kcontrol->private_value);if (err < 0) {snd_ctl_free_one(kcontrol);
Don't free this after adding via snd_ctl_add(). Just keep it. The final destructor will clean up everything eventually.
thanks,
Takashi