[alsa-devel] [PATCH 0/7] More line6 fixes / rewrites
Hi,
this is yet another series of patches for line6 drivers. Most of patches are cleanups and trivial fixes while the last one is to reimplement the LED control via the standard LED class.
As usual, patches are found in test/line6 branch for now.
Takashi
... in line6_disconnect() as well.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/line6/driver.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/sound/usb/line6/driver.c b/sound/usb/line6/driver.c index 93cd4daa56bc..e8d51381ffdb 100644 --- a/sound/usb/line6/driver.c +++ b/sound/usb/line6/driver.c @@ -579,18 +579,9 @@ EXPORT_SYMBOL_GPL(line6_probe); */ void line6_disconnect(struct usb_interface *interface) { - struct usb_line6 *line6; - struct usb_device *usbdev; - int interface_number; - - if (interface == NULL) - return; - usbdev = interface_to_usbdev(interface); - if (usbdev == NULL) - return; + struct usb_line6 *line6 = usb_get_intfdata(interface); + struct usb_device *usbdev = interface_to_usbdev(interface);
- interface_number = interface->cur_altsetting->desc.bInterfaceNumber; - line6 = usb_get_intfdata(interface); if (!line6) return;
It's utterly unsafe to proceed further the disconnect procedure if the assigned usbdev is inconsistent with the expected object. Better to put a WARN_ON() for more cautions and abort immediately.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/line6/driver.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/sound/usb/line6/driver.c b/sound/usb/line6/driver.c index e8d51381ffdb..625272fe227a 100644 --- a/sound/usb/line6/driver.c +++ b/sound/usb/line6/driver.c @@ -585,12 +585,12 @@ void line6_disconnect(struct usb_interface *interface) if (!line6) return;
+ if (WARN_ON(usbdev != line6->usbdev)) + return; + if (line6->urb_listen != NULL) line6_stop_listen(line6);
- if (usbdev != line6->usbdev) - dev_err(line6->ifcdev, "driver bug: inconsistent usb device\n"); - snd_card_disconnect(line6->card); if (line6->line6pcm) line6_pcm_disconnect(line6->line6pcm);
The interface and driver objects are always set when callbacks are called. Drop such superfluous NULL checks in init and disconnect calls of each driver.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/line6/pod.c | 28 ++++++++-------------------- sound/usb/line6/podhd.c | 4 ---- sound/usb/line6/toneport.c | 6 ------ sound/usb/line6/variax.c | 8 -------- 4 files changed, 8 insertions(+), 38 deletions(-)
diff --git a/sound/usb/line6/pod.c b/sound/usb/line6/pod.c index bf027fc70cba..26ecf26a33ec 100644 --- a/sound/usb/line6/pod.c +++ b/sound/usb/line6/pod.c @@ -401,25 +401,16 @@ static struct snd_kcontrol_new pod_control_monitor = { */ static void line6_pod_disconnect(struct usb_interface *interface) { - struct usb_line6_pod *pod; - - if (interface == NULL) - return; - pod = usb_get_intfdata(interface); - - if (pod != NULL) { - struct device *dev = &interface->dev; + struct usb_line6_pod *pod = usb_get_intfdata(interface); + struct device *dev = &interface->dev;
- if (dev != NULL) { - /* remove sysfs entries: */ - device_remove_file(dev, &dev_attr_device_id); - device_remove_file(dev, &dev_attr_firmware_version); - device_remove_file(dev, &dev_attr_serial_number); - } + /* remove sysfs entries: */ + device_remove_file(dev, &dev_attr_device_id); + device_remove_file(dev, &dev_attr_firmware_version); + device_remove_file(dev, &dev_attr_serial_number);
- del_timer_sync(&pod->startup_timer); - cancel_work_sync(&pod->startup_work); - } + del_timer_sync(&pod->startup_timer); + cancel_work_sync(&pod->startup_work); }
/* @@ -456,9 +447,6 @@ static int pod_init(struct usb_interface *interface, init_timer(&pod->startup_timer); INIT_WORK(&pod->startup_work, pod_startup4);
- if ((interface == NULL) || (pod == NULL)) - return -ENODEV; - /* create sysfs entries: */ err = pod_create_files2(&interface->dev); if (err < 0) diff --git a/sound/usb/line6/podhd.c b/sound/usb/line6/podhd.c index 7217fa7e5db1..59abbd92624e 100644 --- a/sound/usb/line6/podhd.c +++ b/sound/usb/line6/podhd.c @@ -90,12 +90,8 @@ static struct line6_pcm_properties podhd_pcm_properties = { static int podhd_init(struct usb_interface *interface, struct usb_line6 *line6) { - struct usb_line6_podhd *podhd = (struct usb_line6_podhd *) line6; int err;
- if ((interface == NULL) || (podhd == NULL)) - return -ENODEV; - /* initialize MIDI subsystem: */ err = line6_init_midi(line6); if (err < 0) diff --git a/sound/usb/line6/toneport.c b/sound/usb/line6/toneport.c index c1f61cde52ab..e5669bd3233a 100644 --- a/sound/usb/line6/toneport.c +++ b/sound/usb/line6/toneport.c @@ -370,9 +370,6 @@ static void line6_toneport_disconnect(struct usb_interface *interface) struct usb_line6_toneport *toneport; u16 idProduct;
- if (interface == NULL) - return; - toneport = usb_get_intfdata(interface); del_timer_sync(&toneport->timer); idProduct = le16_to_cpu(toneport->line6.usbdev->descriptor.idProduct); @@ -393,9 +390,6 @@ static int toneport_init(struct usb_interface *interface, int err; struct usb_line6_toneport *toneport = (struct usb_line6_toneport *) line6;
- if ((interface == NULL) || (toneport == NULL)) - return -ENODEV; - line6->disconnect = line6_toneport_disconnect;
/* initialize PCM subsystem: */ diff --git a/sound/usb/line6/variax.c b/sound/usb/line6/variax.c index 99a58cbfd2da..cd3adeffde02 100644 --- a/sound/usb/line6/variax.c +++ b/sound/usb/line6/variax.c @@ -214,12 +214,7 @@ static void line6_variax_disconnect(struct usb_interface *interface) { struct usb_line6_variax *variax;
- if (!interface) - return; - variax = usb_get_intfdata(interface); - if (!variax) - return;
del_timer(&variax->startup_timer1); del_timer(&variax->startup_timer2); @@ -244,9 +239,6 @@ static int variax_init(struct usb_interface *interface, init_timer(&variax->startup_timer2); INIT_WORK(&variax->startup_work, variax_startup6);
- if ((interface == NULL) || (variax == NULL)) - return -ENODEV; - /* initialize USB buffers: */ variax->buffer_activate = kmemdup(variax_activate, sizeof(variax_activate), GFP_KERNEL);
... so that timer_del_sync() in the destructor can be called safely at any time. Also move the mod_timer() call in toneport_setup(), which is a bit clearer place.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/line6/toneport.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/sound/usb/line6/toneport.c b/sound/usb/line6/toneport.c index e5669bd3233a..fb6d5e1e2ade 100644 --- a/sound/usb/line6/toneport.c +++ b/sound/usb/line6/toneport.c @@ -360,6 +360,8 @@ static void toneport_setup(struct usb_line6_toneport *toneport)
if (toneport_has_led(toneport->type)) toneport_update_led(&usbdev->dev); + + mod_timer(&toneport->timer, jiffies + TONEPORT_PCM_DELAY * HZ); }
/* @@ -390,6 +392,9 @@ static int toneport_init(struct usb_interface *interface, int err; struct usb_line6_toneport *toneport = (struct usb_line6_toneport *) line6;
+ setup_timer(&toneport->timer, toneport_start_pcm, + (unsigned long)toneport); + line6->disconnect = line6_toneport_disconnect;
/* initialize PCM subsystem: */ @@ -435,10 +440,6 @@ static int toneport_init(struct usb_interface *interface,
toneport_setup(toneport);
- setup_timer(&toneport->timer, toneport_start_pcm, - (unsigned long)toneport); - mod_timer(&toneport->timer, jiffies + TONEPORT_PCM_DELAY * HZ); - /* register audio system: */ return snd_card_register(line6->card); }
Currently disconnect callback is used as a driver's destructor, and this has to be called not only at the disconnection time but also at the error paths during probe.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/line6/driver.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/sound/usb/line6/driver.c b/sound/usb/line6/driver.c index 625272fe227a..e7f9a99e1949 100644 --- a/sound/usb/line6/driver.c +++ b/sound/usb/line6/driver.c @@ -568,6 +568,8 @@ int line6_probe(struct usb_interface *interface, return 0;
err_destruct: + if (line6->disconnect) + line6->disconnect(interface); snd_card_free(card); err_put: return ret;
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/line6/toneport.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/sound/usb/line6/toneport.c b/sound/usb/line6/toneport.c index fb6d5e1e2ade..9a4f5403569e 100644 --- a/sound/usb/line6/toneport.c +++ b/sound/usb/line6/toneport.c @@ -370,13 +370,11 @@ static void toneport_setup(struct usb_line6_toneport *toneport) static void line6_toneport_disconnect(struct usb_interface *interface) { struct usb_line6_toneport *toneport; - u16 idProduct;
toneport = usb_get_intfdata(interface); del_timer_sync(&toneport->timer); - idProduct = le16_to_cpu(toneport->line6.usbdev->descriptor.idProduct);
- if (toneport_has_led(idProduct)) { + if (toneport_has_led(toneport->type)) { device_remove_file(&interface->dev, &dev_attr_led_red); device_remove_file(&interface->dev, &dev_attr_led_green); }
Instead of non-standard sysfs, reimplement the LED controls on TonePort as LED class devices.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/line6/Kconfig | 2 + sound/usb/line6/toneport.c | 163 +++++++++++++++++++++++++-------------------- 2 files changed, 91 insertions(+), 74 deletions(-)
diff --git a/sound/usb/line6/Kconfig b/sound/usb/line6/Kconfig index af20947e0bda..f4585d378ef3 100644 --- a/sound/usb/line6/Kconfig +++ b/sound/usb/line6/Kconfig @@ -29,6 +29,8 @@ config SND_USB_PODHD config SND_USB_TONEPORT tristate "TonePort GX, UX1 and UX2 USB support" select SND_USB_LINE6 + select NEW_LEDS + select LEDS_CLASS help This is a driver for TonePort GX, UX1 and UX2 devices.
diff --git a/sound/usb/line6/toneport.c b/sound/usb/line6/toneport.c index 9a4f5403569e..9a769463f7bf 100644 --- a/sound/usb/line6/toneport.c +++ b/sound/usb/line6/toneport.c @@ -14,6 +14,7 @@ #include <linux/usb.h> #include <linux/slab.h> #include <linux/module.h> +#include <linux/leds.h> #include <sound/core.h> #include <sound/control.h>
@@ -32,6 +33,15 @@ enum line6_device_type { LINE6_TONEPORT_UX2, };
+struct usb_line6_toneport; + +struct toneport_led { + struct led_classdev dev; + char name[64]; + struct usb_line6_toneport *toneport; + bool registered; +}; + struct usb_line6_toneport { /** Generic Line 6 USB data. @@ -62,6 +72,9 @@ struct usb_line6_toneport { Device type. */ enum line6_device_type type; + + /* LED instances */ + struct toneport_led leds[2]; };
static int toneport_send_cmd(struct usb_device *usbdev, int cmd1, int cmd2); @@ -117,15 +130,6 @@ static struct line6_pcm_properties toneport_pcm_properties = { .bytes_per_frame = 4 };
-/* - For the led on Guitarport. - Brightness goes from 0x00 to 0x26. Set a value above this to have led - blink. - (void cmd_0x02(byte red, byte green) -*/ -static int led_red = 0x00; -static int led_green = 0x26; - static const struct { const char *name; int code; @@ -136,62 +140,6 @@ static const struct { {"Inst & Mic", 0x0901} };
-static bool toneport_has_led(enum line6_device_type type) -{ - return - (type == LINE6_GUITARPORT) || - (type == LINE6_TONEPORT_GX); - /* add your device here if you are missing support for the LEDs */ -} - -static void toneport_update_led(struct device *dev) -{ - struct usb_interface *interface = to_usb_interface(dev); - struct usb_line6_toneport *tp = usb_get_intfdata(interface); - struct usb_line6 *line6; - - if (!tp) - return; - - line6 = &tp->line6; - if (line6) - toneport_send_cmd(line6->usbdev, (led_red << 8) | 0x0002, - led_green); -} - -static ssize_t toneport_set_led_red(struct device *dev, - struct device_attribute *attr, - const char *buf, size_t count) -{ - int retval; - - retval = kstrtoint(buf, 10, &led_red); - if (retval) - return retval; - - toneport_update_led(dev); - return count; -} - -static ssize_t toneport_set_led_green(struct device *dev, - struct device_attribute *attr, - const char *buf, size_t count) -{ - int retval; - - retval = kstrtoint(buf, 10, &led_green); - if (retval) - return retval; - - toneport_update_led(dev); - return count; -} - -static DEVICE_ATTR(led_red, S_IWUSR | S_IRUGO, line6_nop_read, - toneport_set_led_red); -static DEVICE_ATTR(led_green, S_IWUSR | S_IRUGO, line6_nop_read, - toneport_set_led_green); - static int toneport_send_cmd(struct usb_device *usbdev, int cmd1, int cmd2) { int ret; @@ -330,6 +278,78 @@ static struct snd_kcontrol_new toneport_control_source = { };
/* + For the led on Guitarport. + Brightness goes from 0x00 to 0x26. Set a value above this to have led + blink. + (void cmd_0x02(byte red, byte green) +*/ + +static bool toneport_has_led(enum line6_device_type type) +{ + return + (type == LINE6_GUITARPORT) || + (type == LINE6_TONEPORT_GX); + /* add your device here if you are missing support for the LEDs */ +} + +static const char * const led_colors[2] = { "red", "green" }; +static const int led_init_vals[2] = { 0x00, 0x26 }; + +static void toneport_update_led(struct usb_line6_toneport *toneport) +{ + toneport_send_cmd(toneport->line6.usbdev, + (toneport->leds[0].dev.brightness << 8) | 0x0002, + toneport->leds[1].dev.brightness); +} + +static void toneport_led_brightness_set(struct led_classdev *led_cdev, + enum led_brightness brightness) +{ + struct toneport_led *leds = + container_of(led_cdev, struct toneport_led, dev); + toneport_update_led(leds->toneport); +} + +static int toneport_init_leds(struct usb_line6_toneport *toneport) +{ + struct device *dev = &toneport->line6.usbdev->dev; + int i, err; + + for (i = 0; i < 2; i++) { + struct toneport_led *led = &toneport->leds[i]; + struct led_classdev *leddev = &led->dev; + + led->toneport = toneport; + snprintf(led->name, sizeof(led->name), "%s::%s", + dev_name(dev), led_colors[i]); + leddev->name = led->name; + leddev->brightness = led_init_vals[i]; + leddev->max_brightness = 0x26; + leddev->brightness_set = toneport_led_brightness_set; + err = led_classdev_register(dev, leddev); + if (err) + return err; + led->registered = true; + } + + return 0; +} + +static void toneport_remove_leds(struct usb_line6_toneport *toneport) +{ + struct toneport_led *led; + int i; + + for (i = 0; i < 2; i++) { + led = &toneport->leds[i]; + if (!led->registered) + break; + led_classdev_unregister(&led->dev); + led->registered = false; + } +} + +/* Setup Toneport device. */ static void toneport_setup(struct usb_line6_toneport *toneport) @@ -359,7 +379,7 @@ static void toneport_setup(struct usb_line6_toneport *toneport) }
if (toneport_has_led(toneport->type)) - toneport_update_led(&usbdev->dev); + toneport_update_led(toneport);
mod_timer(&toneport->timer, jiffies + TONEPORT_PCM_DELAY * HZ); } @@ -374,10 +394,8 @@ static void line6_toneport_disconnect(struct usb_interface *interface) toneport = usb_get_intfdata(interface); del_timer_sync(&toneport->timer);
- if (toneport_has_led(toneport->type)) { - device_remove_file(&interface->dev, &dev_attr_led_red); - device_remove_file(&interface->dev, &dev_attr_led_green); - } + if (toneport_has_led(toneport->type)) + toneport_remove_leds(toneport); }
@@ -428,10 +446,7 @@ static int toneport_init(struct usb_interface *interface, line6_read_data(line6, 0x80c2, &toneport->firmware_version, 1);
if (toneport_has_led(toneport->type)) { - err = device_create_file(&interface->dev, &dev_attr_led_red); - if (err < 0) - return err; - err = device_create_file(&interface->dev, &dev_attr_led_green); + err = toneport_init_leds(toneport); if (err < 0) return err; }
participants (1)
-
Takashi Iwai