[alsa-devel] [PATCH 0/3] Another round of line6 cleanup
Hi,
here is yet another series of cleanup patches for line6 drivers. This time, the extra allocation for struct usb_line6 was removed by using snd_card_new()'s allocation feature. In addition to that, a few trivial cleanups.
The test/line6 branch of sound git tree was updated with these patches, too.
Takashi
===
Takashi Iwai (3): ALSA: line6: Drop interface argument from private_init and disconnect callbacks ALSA: line6: Let snd_card_new() allocate private data ALSA: line6: Minor tidy up in line6_probe()
sound/usb/line6/driver.c | 52 ++++++++++++++++++++-------------------------- sound/usb/line6/driver.h | 8 ++++--- sound/usb/line6/pod.c | 21 +++++++------------ sound/usb/line6/podhd.c | 13 ++++-------- sound/usb/line6/toneport.c | 21 +++++++------------ sound/usb/line6/variax.c | 19 ++++++----------- 6 files changed, 54 insertions(+), 80 deletions(-)
The interface argument is used just for retrieving the assigned device, which can be already found in line6->ifcdev. Drop them from the callbacks. Also, pass the usb id to private_init so that the driver can deal with it there. This is a preliminary work for the further cleanup to move the whole allocation into driver.c.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/line6/driver.c | 9 +++++---- sound/usb/line6/driver.h | 6 ++++-- sound/usb/line6/pod.c | 14 +++++++------- sound/usb/line6/podhd.c | 6 +++--- sound/usb/line6/toneport.c | 14 +++++++------- sound/usb/line6/variax.c | 12 +++++------- 6 files changed, 31 insertions(+), 30 deletions(-)
diff --git a/sound/usb/line6/driver.c b/sound/usb/line6/driver.c index 4d2d233d0505..e2fbff05c1b1 100644 --- a/sound/usb/line6/driver.c +++ b/sound/usb/line6/driver.c @@ -488,9 +488,10 @@ static int line6_init_cap_control(struct usb_line6 *line6) Probe USB device. */ int line6_probe(struct usb_interface *interface, + const struct usb_device_id *id, struct usb_line6 *line6, const struct line6_properties *properties, - int (*private_init)(struct usb_interface *, struct usb_line6 *)) + int (*private_init)(struct usb_line6 *, const struct usb_device_id *id)) { struct usb_device *usbdev = interface_to_usbdev(interface); struct snd_card *card; @@ -552,7 +553,7 @@ int line6_probe(struct usb_interface *interface, }
/* initialize device data based on device: */ - ret = private_init(interface, line6); + ret = private_init(line6, id); if (ret < 0) goto error;
@@ -565,7 +566,7 @@ int line6_probe(struct usb_interface *interface,
error: if (line6->disconnect) - line6->disconnect(interface); + line6->disconnect(line6); snd_card_free(card); return ret; } @@ -592,7 +593,7 @@ void line6_disconnect(struct usb_interface *interface) if (line6->line6pcm) line6_pcm_disconnect(line6->line6pcm); if (line6->disconnect) - line6->disconnect(interface); + line6->disconnect(line6);
dev_info(&interface->dev, "Line 6 %s now disconnected\n", line6->properties->name); diff --git a/sound/usb/line6/driver.h b/sound/usb/line6/driver.h index a6c0b2f08ba8..4dc6c28e8224 100644 --- a/sound/usb/line6/driver.h +++ b/sound/usb/line6/driver.h @@ -157,7 +157,7 @@ struct usb_line6 { int message_length;
void (*process_message)(struct usb_line6 *); - void (*disconnect)(struct usb_interface *); + void (*disconnect)(struct usb_line6 *line6); };
extern char *line6_alloc_sysex_buffer(struct usb_line6 *line6, int code1, @@ -180,9 +180,11 @@ extern int line6_write_data(struct usb_line6 *line6, int address, void *data, size_t datalen);
int line6_probe(struct usb_interface *interface, + const struct usb_device_id *id, struct usb_line6 *line6, const struct line6_properties *properties, - int (*private_init)(struct usb_interface *, struct usb_line6 *)); + int (*private_init)(struct usb_line6 *, const struct usb_device_id *id)); + void line6_disconnect(struct usb_interface *interface);
#ifdef CONFIG_PM diff --git a/sound/usb/line6/pod.c b/sound/usb/line6/pod.c index 26ecf26a33ec..d1e952fbcae7 100644 --- a/sound/usb/line6/pod.c +++ b/sound/usb/line6/pod.c @@ -399,10 +399,10 @@ static struct snd_kcontrol_new pod_control_monitor = { /* POD device disconnected. */ -static void line6_pod_disconnect(struct usb_interface *interface) +static void line6_pod_disconnect(struct usb_line6 *line6) { - struct usb_line6_pod *pod = usb_get_intfdata(interface); - struct device *dev = &interface->dev; + struct usb_line6_pod *pod = (struct usb_line6_pod *)line6; + struct device *dev = line6->ifcdev;
/* remove sysfs entries: */ device_remove_file(dev, &dev_attr_device_id); @@ -435,8 +435,8 @@ static int pod_create_files2(struct device *dev) /* Try to init POD device. */ -static int pod_init(struct usb_interface *interface, - struct usb_line6 *line6) +static int pod_init(struct usb_line6 *line6, + const struct usb_device_id *id) { int err; struct usb_line6_pod *pod = (struct usb_line6_pod *) line6; @@ -448,7 +448,7 @@ static int pod_init(struct usb_interface *interface, INIT_WORK(&pod->startup_work, pod_startup4);
/* create sysfs entries: */ - err = pod_create_files2(&interface->dev); + err = pod_create_files2(line6->ifcdev); if (err < 0) return err;
@@ -596,7 +596,7 @@ static int pod_probe(struct usb_interface *interface, pod = kzalloc(sizeof(*pod), GFP_KERNEL); if (!pod) return -ENODEV; - return line6_probe(interface, &pod->line6, + return line6_probe(interface, id, &pod->line6, &pod_properties_table[id->driver_info], pod_init); } diff --git a/sound/usb/line6/podhd.c b/sound/usb/line6/podhd.c index 59abbd92624e..21d7edcfa272 100644 --- a/sound/usb/line6/podhd.c +++ b/sound/usb/line6/podhd.c @@ -87,8 +87,8 @@ static struct line6_pcm_properties podhd_pcm_properties = { /* Try to init POD HD device. */ -static int podhd_init(struct usb_interface *interface, - struct usb_line6 *line6) +static int podhd_init(struct usb_line6 *line6, + const struct usb_device_id *id) { int err;
@@ -182,7 +182,7 @@ static int podhd_probe(struct usb_interface *interface, podhd = kzalloc(sizeof(*podhd), GFP_KERNEL); if (!podhd) return -ENODEV; - return line6_probe(interface, &podhd->line6, + return line6_probe(interface, id, &podhd->line6, &podhd_properties_table[id->driver_info], podhd_init); } diff --git a/sound/usb/line6/toneport.c b/sound/usb/line6/toneport.c index 9a769463f7bf..8e7020df0d10 100644 --- a/sound/usb/line6/toneport.c +++ b/sound/usb/line6/toneport.c @@ -387,11 +387,11 @@ static void toneport_setup(struct usb_line6_toneport *toneport) /* Toneport device disconnected. */ -static void line6_toneport_disconnect(struct usb_interface *interface) +static void line6_toneport_disconnect(struct usb_line6 *line6) { - struct usb_line6_toneport *toneport; + struct usb_line6_toneport *toneport = + (struct usb_line6_toneport *)line6;
- toneport = usb_get_intfdata(interface); del_timer_sync(&toneport->timer);
if (toneport_has_led(toneport->type)) @@ -402,12 +402,13 @@ static void line6_toneport_disconnect(struct usb_interface *interface) /* Try to init Toneport device. */ -static int toneport_init(struct usb_interface *interface, - struct usb_line6 *line6) +static int toneport_init(struct usb_line6 *line6, + const struct usb_device_id *id) { int err; struct usb_line6_toneport *toneport = (struct usb_line6_toneport *) line6;
+ toneport->type = id->driver_info; setup_timer(&toneport->timer, toneport_start_pcm, (unsigned long)toneport);
@@ -562,8 +563,7 @@ static int toneport_probe(struct usb_interface *interface, toneport = kzalloc(sizeof(*toneport), GFP_KERNEL); if (!toneport) return -ENODEV; - toneport->type = id->driver_info; - return line6_probe(interface, &toneport->line6, + return line6_probe(interface, id, &toneport->line6, &toneport_properties_table[id->driver_info], toneport_init); } diff --git a/sound/usb/line6/variax.c b/sound/usb/line6/variax.c index cd3adeffde02..ba6e85eed2ba 100644 --- a/sound/usb/line6/variax.c +++ b/sound/usb/line6/variax.c @@ -210,11 +210,9 @@ static void line6_variax_process_message(struct usb_line6 *line6) /* Variax destructor. */ -static void line6_variax_disconnect(struct usb_interface *interface) +static void line6_variax_disconnect(struct usb_line6 *line6) { - struct usb_line6_variax *variax; - - variax = usb_get_intfdata(interface); + struct usb_line6_variax *variax = (struct usb_line6_variax *)line6;
del_timer(&variax->startup_timer1); del_timer(&variax->startup_timer2); @@ -226,8 +224,8 @@ static void line6_variax_disconnect(struct usb_interface *interface) /* Try to init workbench device. */ -static int variax_init(struct usb_interface *interface, - struct usb_line6 *line6) +static int variax_init(struct usb_line6 *line6, + const struct usb_device_id *id) { struct usb_line6_variax *variax = (struct usb_line6_variax *) line6; int err; @@ -303,7 +301,7 @@ static int variax_probe(struct usb_interface *interface, variax = kzalloc(sizeof(*variax), GFP_KERNEL); if (!variax) return -ENODEV; - return line6_probe(interface, &variax->line6, + return line6_probe(interface, id, &variax->line6, &variax_properties_table[id->driver_info], variax_init); }
Instead of allocating the private data individually in each driver's probe at first, let snd_card_new() allocate the data that is called in line6_probe(). This simplifies the primary probe functions.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/line6/driver.c | 25 ++++++++++--------------- sound/usb/line6/driver.h | 4 ++-- sound/usb/line6/pod.c | 9 ++------- sound/usb/line6/podhd.c | 9 ++------- sound/usb/line6/toneport.c | 9 ++------- sound/usb/line6/variax.c | 9 ++------- 6 files changed, 20 insertions(+), 45 deletions(-)
diff --git a/sound/usb/line6/driver.c b/sound/usb/line6/driver.c index e2fbff05c1b1..c696f9773cbb 100644 --- a/sound/usb/line6/driver.c +++ b/sound/usb/line6/driver.c @@ -418,11 +418,7 @@ EXPORT_SYMBOL_GPL(line6_read_serial_number); static void line6_destruct(struct snd_card *card) { struct usb_line6 *line6 = card->private_data; - struct usb_device *usbdev; - - if (!line6) - return; - usbdev = line6->usbdev; + struct usb_device *usbdev = line6->usbdev;
/* free buffer memory first: */ kfree(line6->buffer_message); @@ -431,9 +427,6 @@ static void line6_destruct(struct snd_card *card) /* then free URBs: */ usb_free_urb(line6->urb_listen);
- /* free interface data: */ - kfree(line6); - /* decrement reference counters: */ usb_put_dev(usbdev); } @@ -489,24 +482,27 @@ static int line6_init_cap_control(struct usb_line6 *line6) */ int line6_probe(struct usb_interface *interface, const struct usb_device_id *id, - struct usb_line6 *line6, const struct line6_properties *properties, - int (*private_init)(struct usb_line6 *, const struct usb_device_id *id)) + int (*private_init)(struct usb_line6 *, const struct usb_device_id *id), + size_t data_size) { struct usb_device *usbdev = interface_to_usbdev(interface); struct snd_card *card; + struct usb_line6 *line6; int interface_number; int ret;
+ if (WARN_ON(data_size < sizeof(*line6))) + return -EINVAL; + ret = snd_card_new(&interface->dev, SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1, - THIS_MODULE, 0, &card); - if (ret < 0) { - kfree(line6); + THIS_MODULE, data_size, &card); + if (ret < 0) return ret; - }
/* store basic data: */ + line6 = card->private_data; line6->card = card; line6->properties = properties; line6->usbdev = usbdev; @@ -517,7 +513,6 @@ int line6_probe(struct usb_interface *interface, strcpy(card->shortname, line6->properties->name); sprintf(card->longname, "Line 6 %s at USB %s", line6->properties->name, dev_name(line6->ifcdev)); - card->private_data = line6; card->private_free = line6_destruct;
usb_set_intfdata(interface, line6); diff --git a/sound/usb/line6/driver.h b/sound/usb/line6/driver.h index 4dc6c28e8224..fce10f12f0d3 100644 --- a/sound/usb/line6/driver.h +++ b/sound/usb/line6/driver.h @@ -181,9 +181,9 @@ extern int line6_write_data(struct usb_line6 *line6, int address, void *data,
int line6_probe(struct usb_interface *interface, const struct usb_device_id *id, - struct usb_line6 *line6, const struct line6_properties *properties, - int (*private_init)(struct usb_line6 *, const struct usb_device_id *id)); + int (*private_init)(struct usb_line6 *, const struct usb_device_id *id), + size_t data_size);
void line6_disconnect(struct usb_interface *interface);
diff --git a/sound/usb/line6/pod.c b/sound/usb/line6/pod.c index d1e952fbcae7..6f7cd585f2d8 100644 --- a/sound/usb/line6/pod.c +++ b/sound/usb/line6/pod.c @@ -591,14 +591,9 @@ static const struct line6_properties pod_properties_table[] = { static int pod_probe(struct usb_interface *interface, const struct usb_device_id *id) { - struct usb_line6_pod *pod; - - pod = kzalloc(sizeof(*pod), GFP_KERNEL); - if (!pod) - return -ENODEV; - return line6_probe(interface, id, &pod->line6, + return line6_probe(interface, id, &pod_properties_table[id->driver_info], - pod_init); + pod_init, sizeof(struct usb_line6_pod)); }
static struct usb_driver pod_driver = { diff --git a/sound/usb/line6/podhd.c b/sound/usb/line6/podhd.c index 21d7edcfa272..43c39886597e 100644 --- a/sound/usb/line6/podhd.c +++ b/sound/usb/line6/podhd.c @@ -177,14 +177,9 @@ static const struct line6_properties podhd_properties_table[] = { static int podhd_probe(struct usb_interface *interface, const struct usb_device_id *id) { - struct usb_line6_podhd *podhd; - - podhd = kzalloc(sizeof(*podhd), GFP_KERNEL); - if (!podhd) - return -ENODEV; - return line6_probe(interface, id, &podhd->line6, + return line6_probe(interface, id, &podhd_properties_table[id->driver_info], - podhd_init); + podhd_init, sizeof(struct usb_line6_podhd)); }
static struct usb_driver podhd_driver = { diff --git a/sound/usb/line6/toneport.c b/sound/usb/line6/toneport.c index 8e7020df0d10..33d16ecf18a0 100644 --- a/sound/usb/line6/toneport.c +++ b/sound/usb/line6/toneport.c @@ -558,14 +558,9 @@ static const struct line6_properties toneport_properties_table[] = { static int toneport_probe(struct usb_interface *interface, const struct usb_device_id *id) { - struct usb_line6_toneport *toneport; - - toneport = kzalloc(sizeof(*toneport), GFP_KERNEL); - if (!toneport) - return -ENODEV; - return line6_probe(interface, id, &toneport->line6, + return line6_probe(interface, id, &toneport_properties_table[id->driver_info], - toneport_init); + toneport_init, sizeof(struct usb_line6_toneport)); }
static struct usb_driver toneport_driver = { diff --git a/sound/usb/line6/variax.c b/sound/usb/line6/variax.c index ba6e85eed2ba..9701ffa61365 100644 --- a/sound/usb/line6/variax.c +++ b/sound/usb/line6/variax.c @@ -296,14 +296,9 @@ static const struct line6_properties variax_properties_table[] = { static int variax_probe(struct usb_interface *interface, const struct usb_device_id *id) { - struct usb_line6_variax *variax; - - variax = kzalloc(sizeof(*variax), GFP_KERNEL); - if (!variax) - return -ENODEV; - return line6_probe(interface, id, &variax->line6, + return line6_probe(interface, id, &variax_properties_table[id->driver_info], - variax_init); + variax_init, sizeof(struct usb_line6_variax)); }
static struct usb_driver variax_driver = {
Move the check of multi configurations before snd_card_new() as a short path, and reduce superfluous pointer references.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/line6/driver.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/sound/usb/line6/driver.c b/sound/usb/line6/driver.c index c696f9773cbb..a0436993a167 100644 --- a/sound/usb/line6/driver.c +++ b/sound/usb/line6/driver.c @@ -495,6 +495,10 @@ int line6_probe(struct usb_interface *interface, if (WARN_ON(data_size < sizeof(*line6))) return -EINVAL;
+ /* we don't handle multiple configurations */ + if (usbdev->descriptor.bNumConfigurations != 1) + return -ENODEV; + ret = snd_card_new(&interface->dev, SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1, THIS_MODULE, data_size, &card); @@ -508,10 +512,10 @@ int line6_probe(struct usb_interface *interface, line6->usbdev = usbdev; line6->ifcdev = &interface->dev;
- strcpy(card->id, line6->properties->id); + strcpy(card->id, properties->id); strcpy(card->driver, DRIVER_NAME); - strcpy(card->shortname, line6->properties->name); - sprintf(card->longname, "Line 6 %s at USB %s", line6->properties->name, + strcpy(card->shortname, properties->name); + sprintf(card->longname, "Line 6 %s at USB %s", properties->name, dev_name(line6->ifcdev)); card->private_free = line6_destruct;
@@ -520,12 +524,6 @@ int line6_probe(struct usb_interface *interface, /* increment reference counters: */ usb_get_dev(usbdev);
- /* we don't handle multiple configurations */ - if (usbdev->descriptor.bNumConfigurations != 1) { - ret = -ENODEV; - goto error; - } - /* initialize device info: */ dev_info(&interface->dev, "Line 6 %s found\n", properties->name);
@@ -533,7 +531,7 @@ int line6_probe(struct usb_interface *interface, interface_number = interface->cur_altsetting->desc.bInterfaceNumber;
ret = usb_set_interface(usbdev, interface_number, - properties->altsetting); + properties->altsetting); if (ret < 0) { dev_err(&interface->dev, "set_interface failed\n"); goto error; @@ -555,7 +553,7 @@ int line6_probe(struct usb_interface *interface, /* creation of additional special files should go here */
dev_info(&interface->dev, "Line 6 %s now attached\n", - line6->properties->name); + properties->name);
return 0;
participants (1)
-
Takashi Iwai