[PATCH 00/16] Yet more cleanups of line6 drivers

[PATCH 00/16] Yet more cleanups of line6 drivers

[Takashi Iwai]

Hi,

here is a series of cleanup patches for line6 stuff.  They are applied
on top of my previous patches.  The latest patchset is found in
test/line6 branch of sound.git tree.


Takashi


Takashi Iwai (16):
  ALSA: line6: Minor refactoring
  ALSA: line6: Fix memory leak at probe error path
  ALSA: line6: Remove unused line6_nop_read()
  ALSA: line6: Reduce superfluous spinlock in midi.c
  ALSA: line6: Fix missing error handling in line6_pcm_acquire()
  ALSA: line6: Use logical OR
  ALSA: line6: Fix the error recovery in line6_pcm_acquire()
  ALSA: line6: Drop superfluous spinlock for trigger
  ALSA: line6: Use incremental loop
  ALSA: line6: Drop voodoo workarounds
  ALSA: line6: Rearrange PCM structure
  ALSA: line6: Consolidate URB unlink and sync helpers
  ALSA: line6: Use dev_err()
  ALSA: line6: Consolidate PCM stream buffer allocation and free
  ALSA: line6: Do clipping in volume / monitor manipulations
  ALSA: line6: Skip volume manipulation during silence copying

 sound/usb/line6/capture.c  | 122 ++++++--------------------
 sound/usb/line6/capture.h  |   5 --
 sound/usb/line6/driver.c   | 157 ++++++++++++++++-----------------
 sound/usb/line6/driver.h   |   2 -
 sound/usb/line6/midi.c     |  18 ++--
 sound/usb/line6/midi.h     |   7 +-
 sound/usb/line6/pcm.c      | 210 +++++++++++++++++++++++++++------------------
 sound/usb/line6/pcm.h      | 163 ++++++++++-------------------------
 sound/usb/line6/playback.c | 159 ++++++++++------------------------
 sound/usb/line6/playback.h |   5 --
 10 files changed, 329 insertions(+), 519 deletions(-)

-- 
2.2.2

[PATCH 01/16] ALSA: line6: Minor refactoring

[Takashi Iwai]

Split some codes in the lengthy line6_probe().

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/usb/line6/driver.c | 94 +++++++++++++++++++++++++-----------------------
 1 file changed, 49 insertions(+), 45 deletions(-)

diff --git a/sound/usb/line6/driver.c b/sound/usb/line6/driver.c
index e7f9a99e1949..b783c0788e45 100644
--- a/sound/usb/line6/driver.c
+++ b/sound/usb/line6/driver.c
@@ -448,6 +448,52 @@ static void line6_destruct(struct snd_card *card)
 	usb_put_dev(usbdev);
 }
 
+/* get data from endpoint descriptor (see usb_maxpacket): */
+static void line6_get_interval(struct usb_line6 *line6)
+{
+	struct usb_device *usbdev = line6->usbdev;
+	struct usb_host_endpoint *ep;
+	unsigned pipe = usb_rcvintpipe(usbdev, line6->properties->ep_ctrl_r);
+	unsigned epnum = usb_pipeendpoint(pipe);
+
+	ep = usbdev->ep_in[epnum];
+	if (ep) {
+		line6->interval = ep->desc.bInterval;
+		line6->max_packet_size = le16_to_cpu(ep->desc.wMaxPacketSize);
+	} else {
+		dev_err(line6->ifcdev,
+			"endpoint not available, using fallback values");
+		line6->interval = LINE6_FALLBACK_INTERVAL;
+		line6->max_packet_size = LINE6_FALLBACK_MAXPACKETSIZE;
+	}
+}
+
+static int line6_init_cap_control(struct usb_line6 *line6)
+{
+	int ret;
+
+	/* initialize USB buffers: */
+	line6->buffer_listen = kmalloc(LINE6_BUFSIZE_LISTEN, GFP_KERNEL);
+	if (!line6->buffer_listen)
+		return -ENOMEM;
+
+	line6->buffer_message = kmalloc(LINE6_MESSAGE_MAXLEN, GFP_KERNEL);
+	if (!line6->buffer_message)
+		return -ENOMEM;
+
+	line6->urb_listen = usb_alloc_urb(0, GFP_KERNEL);
+	if (!line6->urb_listen)
+		return -ENOMEM;
+
+	ret = line6_start_listen(line6);
+	if (ret < 0) {
+		dev_err(line6->ifcdev, "cannot start listening: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
 /*
 	Probe USB device.
 */
@@ -485,24 +531,7 @@ int line6_probe(struct usb_interface *interface,
 	line6->usbdev = usbdev;
 	line6->ifcdev = &interface->dev;
 
-	/* get data from endpoint descriptor (see usb_maxpacket): */
-	{
-		struct usb_host_endpoint *ep;
-		unsigned pipe = usb_rcvintpipe(usbdev, properties->ep_ctrl_r);
-		unsigned epnum = usb_pipeendpoint(pipe);
-		ep = usbdev->ep_in[epnum];
-
-		if (ep != NULL) {
-			line6->interval = ep->desc.bInterval;
-			line6->max_packet_size =
-			    le16_to_cpu(ep->desc.wMaxPacketSize);
-		} else {
-			line6->interval = LINE6_FALLBACK_INTERVAL;
-			line6->max_packet_size = LINE6_FALLBACK_MAXPACKETSIZE;
-			dev_err(line6->ifcdev,
-				"endpoint not available, using fallback values");
-		}
-	}
+	line6_get_interval(line6);
 
 	ret = snd_card_new(line6->ifcdev,
 			   SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1,
@@ -525,34 +554,9 @@ int line6_probe(struct usb_interface *interface,
 	usb_get_dev(usbdev);
 
 	if (properties->capabilities & LINE6_CAP_CONTROL) {
-		/* initialize USB buffers: */
-		line6->buffer_listen =
-		    kmalloc(LINE6_BUFSIZE_LISTEN, GFP_KERNEL);
-		if (line6->buffer_listen == NULL) {
-			ret = -ENOMEM;
-			goto err_destruct;
-		}
-
-		line6->buffer_message =
-		    kmalloc(LINE6_MESSAGE_MAXLEN, GFP_KERNEL);
-		if (line6->buffer_message == NULL) {
-			ret = -ENOMEM;
+		ret = line6_init_cap_control(line6);
+		if (ret < 0)
 			goto err_destruct;
-		}
-
-		line6->urb_listen = usb_alloc_urb(0, GFP_KERNEL);
-
-		if (line6->urb_listen == NULL) {
-			ret = -ENOMEM;
-			goto err_destruct;
-		}
-
-		ret = line6_start_listen(line6);
-		if (ret < 0) {
-			dev_err(&interface->dev, "%s: usb_submit_urb failed\n",
-				__func__);
-			goto err_destruct;
-		}
 	}
 
 	/* initialize device data based on device: */
-- 
2.2.2

[PATCH 02/16] ALSA: line6: Fix memory leak at probe error path

[Takashi Iwai]

Fix memory leak at probe error path by rearranging the call order in
line6_destruct() so that the common destructor is always called.
Also this simplifies the error path to a single goto label.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/usb/line6/driver.c | 53 ++++++++++++++++++++++++------------------------
 1 file changed, 27 insertions(+), 26 deletions(-)

diff --git a/sound/usb/line6/driver.c b/sound/usb/line6/driver.c
index b783c0788e45..2eed6fbd99c4 100644
--- a/sound/usb/line6/driver.c
+++ b/sound/usb/line6/driver.c
@@ -507,10 +507,32 @@ int line6_probe(struct usb_interface *interface,
 	int interface_number;
 	int ret;
 
+	ret = snd_card_new(line6->ifcdev,
+			   SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1,
+			   THIS_MODULE, 0, &card);
+	if (ret < 0) {
+		kfree(line6);
+		return ret;
+	}
+
+	line6->card = card;
+	strcpy(card->id, line6->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,
+		dev_name(line6->ifcdev));
+	card->private_data = line6;
+	card->private_free = line6_destruct;
+
+	usb_set_intfdata(interface, line6);
+
+	/* increment reference counters: */
+	usb_get_dev(usbdev);
+
 	/* we don't handle multiple configurations */
 	if (usbdev->descriptor.bNumConfigurations != 1) {
 		ret = -ENODEV;
-		goto err_put;
+		goto error;
 	}
 
 	/* initialize device info: */
@@ -523,7 +545,7 @@ int line6_probe(struct usb_interface *interface,
 			properties->altsetting);
 	if (ret < 0) {
 		dev_err(&interface->dev, "set_interface failed\n");
-		goto err_put;
+		goto error;
 	}
 
 	/* store basic data: */
@@ -533,36 +555,16 @@ int line6_probe(struct usb_interface *interface,
 
 	line6_get_interval(line6);
 
-	ret = snd_card_new(line6->ifcdev,
-			   SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1,
-			   THIS_MODULE, 0, &card);
-	if (ret < 0)
-		goto err_put;
-
-	line6->card = card;
-	strcpy(card->id, line6->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,
-		dev_name(line6->ifcdev));
-	card->private_data = line6;
-	card->private_free = line6_destruct;
-
-	usb_set_intfdata(interface, line6);
-
-	/* increment reference counters: */
-	usb_get_dev(usbdev);
-
 	if (properties->capabilities & LINE6_CAP_CONTROL) {
 		ret = line6_init_cap_control(line6);
 		if (ret < 0)
-			goto err_destruct;
+			goto error;
 	}
 
 	/* initialize device data based on device: */
 	ret = private_init(interface, line6);
 	if (ret < 0)
-		goto err_destruct;
+		goto error;
 
 	/* creation of additional special files should go here */
 
@@ -571,11 +573,10 @@ int line6_probe(struct usb_interface *interface,
 
 	return 0;
 
- err_destruct:
+ error:
 	if (line6->disconnect)
 		line6->disconnect(interface);
 	snd_card_free(card);
- err_put:
 	return ret;
 }
 EXPORT_SYMBOL_GPL(line6_probe);
-- 
2.2.2

[PATCH 03/16] ALSA: line6: Remove unused line6_nop_read()

[Takashi Iwai]

The function isn't used any longer after rewriting from sysfs to leds
class in toneport.c.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/usb/line6/driver.c | 10 ----------
 sound/usb/line6/driver.h |  2 --
 2 files changed, 12 deletions(-)

diff --git a/sound/usb/line6/driver.c b/sound/usb/line6/driver.c
index 2eed6fbd99c4..d6023c9a8a61 100644
--- a/sound/usb/line6/driver.c
+++ b/sound/usb/line6/driver.c
@@ -413,16 +413,6 @@ int line6_read_serial_number(struct usb_line6 *line6, int *serial_number)
 EXPORT_SYMBOL_GPL(line6_read_serial_number);
 
 /*
-	No operation (i.e., unsupported).
-*/
-ssize_t line6_nop_read(struct device *dev, struct device_attribute *attr,
-		       char *buf)
-{
-	return 0;
-}
-EXPORT_SYMBOL_GPL(line6_nop_read);
-
-/*
 	Card destructor.
 */
 static void line6_destruct(struct snd_card *card)
diff --git a/sound/usb/line6/driver.h b/sound/usb/line6/driver.h
index efd58ac3215b..a6c0b2f08ba8 100644
--- a/sound/usb/line6/driver.h
+++ b/sound/usb/line6/driver.h
@@ -162,8 +162,6 @@ struct usb_line6 {
 
 extern char *line6_alloc_sysex_buffer(struct usb_line6 *line6, int code1,
 				      int code2, int size);
-extern ssize_t line6_nop_read(struct device *dev,
-			      struct device_attribute *attr, char *buf);
 extern int line6_read_data(struct usb_line6 *line6, int address, void *data,
 			   size_t datalen);
 extern int line6_read_serial_number(struct usb_line6 *line6,
-- 
2.2.2

[PATCH 04/16] ALSA: line6: Reduce superfluous spinlock in midi.c

[Takashi Iwai]

The midi_transmit_lock is used always inside the send_urb_lock, thus
it doesn't play any role.  Let's kill it.  Also, rename
"send_urb_lock" as a more simple name "lock" since this is the only
lock for midi.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/usb/line6/midi.c | 18 ++++++------------
 sound/usb/line6/midi.h |  7 +------
 2 files changed, 7 insertions(+), 18 deletions(-)

diff --git a/sound/usb/line6/midi.c b/sound/usb/line6/midi.c
index b5a58a7fe11a..beeedf9a2cbe 100644
--- a/sound/usb/line6/midi.c
+++ b/sound/usb/line6/midi.c
@@ -45,12 +45,9 @@ static void line6_midi_transmit(struct snd_rawmidi_substream *substream)
 	    line6_rawmidi_substream_midi(substream)->line6;
 	struct snd_line6_midi *line6midi = line6->line6midi;
 	struct midi_buffer *mb = &line6midi->midibuf_out;
-	unsigned long flags;
 	unsigned char chunk[LINE6_FALLBACK_MAXPACKETSIZE];
 	int req, done;
 
-	spin_lock_irqsave(&line6->line6midi->midi_transmit_lock, flags);
-
 	for (;;) {
 		req = min(line6_midibuf_bytes_free(mb), line6->max_packet_size);
 		done = snd_rawmidi_transmit_peek(substream, chunk, req);
@@ -71,8 +68,6 @@ static void line6_midi_transmit(struct snd_rawmidi_substream *substream)
 
 		send_midi_async(line6, chunk, done);
 	}
-
-	spin_unlock_irqrestore(&line6->line6midi->midi_transmit_lock, flags);
 }
 
 /*
@@ -92,7 +87,7 @@ static void midi_sent(struct urb *urb)
 	if (status == -ESHUTDOWN)
 		return;
 
-	spin_lock_irqsave(&line6->line6midi->send_urb_lock, flags);
+	spin_lock_irqsave(&line6->line6midi->lock, flags);
 	num = --line6->line6midi->num_active_send_urbs;
 
 	if (num == 0) {
@@ -103,12 +98,12 @@ static void midi_sent(struct urb *urb)
 	if (num == 0)
 		wake_up(&line6->line6midi->send_wait);
 
-	spin_unlock_irqrestore(&line6->line6midi->send_urb_lock, flags);
+	spin_unlock_irqrestore(&line6->line6midi->lock, flags);
 }
 
 /*
 	Send an asynchronous MIDI message.
-	Assumes that line6->line6midi->send_urb_lock is held
+	Assumes that line6->line6midi->lock is held
 	(i.e., this function is serialized).
 */
 static int send_midi_async(struct usb_line6 *line6, unsigned char *data,
@@ -166,12 +161,12 @@ static void line6_midi_output_trigger(struct snd_rawmidi_substream *substream,
 	    line6_rawmidi_substream_midi(substream)->line6;
 
 	line6->line6midi->substream_transmit = substream;
-	spin_lock_irqsave(&line6->line6midi->send_urb_lock, flags);
+	spin_lock_irqsave(&line6->line6midi->lock, flags);
 
 	if (line6->line6midi->num_active_send_urbs == 0)
 		line6_midi_transmit(substream);
 
-	spin_unlock_irqrestore(&line6->line6midi->send_urb_lock, flags);
+	spin_unlock_irqrestore(&line6->line6midi->lock, flags);
 }
 
 static void line6_midi_output_drain(struct snd_rawmidi_substream *substream)
@@ -281,8 +276,7 @@ int line6_init_midi(struct usb_line6 *line6)
 	rmidi->private_free = snd_line6_midi_free;
 
 	init_waitqueue_head(&line6midi->send_wait);
-	spin_lock_init(&line6midi->send_urb_lock);
-	spin_lock_init(&line6midi->midi_transmit_lock);
+	spin_lock_init(&line6midi->lock);
 	line6midi->line6 = line6;
 
 	err = line6_midibuf_init(&line6midi->midibuf_in, MIDI_BUFFER_SIZE, 0);
diff --git a/sound/usb/line6/midi.h b/sound/usb/line6/midi.h
index ba6bf3828aa5..9d9467b2613c 100644
--- a/sound/usb/line6/midi.h
+++ b/sound/usb/line6/midi.h
@@ -40,14 +40,9 @@ struct snd_line6_midi {
 	int num_active_send_urbs;
 
 	/**
-		 Spin lock to protect updates of send_urb.
-	*/
-	spinlock_t send_urb_lock;
-
-	/**
 		 Spin lock to protect MIDI buffer handling.
 	*/
-	spinlock_t midi_transmit_lock;
+	spinlock_t lock;
 
 	/**
 		 Wait queue for MIDI transmission.
-- 
2.2.2

[PATCH 05/16] ALSA: line6: Fix missing error handling in line6_pcm_acquire()

[Takashi Iwai]

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/usb/line6/pcm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sound/usb/line6/pcm.c b/sound/usb/line6/pcm.c
index 8a6059adef69..826158fe9149 100644
--- a/sound/usb/line6/pcm.c
+++ b/sound/usb/line6/pcm.c
@@ -133,7 +133,8 @@ int line6_pcm_acquire(struct snd_line6_pcm *line6pcm, int channels)
 		 */
 		if (line6pcm->active_urb_in | line6pcm->unlink_urb_in) {
 			dev_err(line6pcm->line6->ifcdev, "Device not yet ready\n");
-			return -EBUSY;
+			err = -EBUSY;
+			goto pcm_acquire_error;
 		}
 
 		line6pcm->count_in = 0;
-- 
2.2.2

[PATCH 06/16] ALSA: line6: Use logical OR

[Takashi Iwai]

Fixed a few places using bits OR wrongly for condition checks.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/usb/line6/pcm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/usb/line6/pcm.c b/sound/usb/line6/pcm.c
index 826158fe9149..f740b4490d75 100644
--- a/sound/usb/line6/pcm.c
+++ b/sound/usb/line6/pcm.c
@@ -131,7 +131,7 @@ int line6_pcm_acquire(struct snd_line6_pcm *line6pcm, int channels)
 		   a bug, we therefore report an error if capturing is restarted
 		   too soon.
 		 */
-		if (line6pcm->active_urb_in | line6pcm->unlink_urb_in) {
+		if (line6pcm->active_urb_in || line6pcm->unlink_urb_in) {
 			dev_err(line6pcm->line6->ifcdev, "Device not yet ready\n");
 			err = -EBUSY;
 			goto pcm_acquire_error;
@@ -166,7 +166,7 @@ int line6_pcm_acquire(struct snd_line6_pcm *line6pcm, int channels)
 		/*
 		  See comment above regarding PCM restart.
 		*/
-		if (line6pcm->active_urb_out | line6pcm->unlink_urb_out) {
+		if (line6pcm->active_urb_out || line6pcm->unlink_urb_out) {
 			dev_err(line6pcm->line6->ifcdev, "Device not yet ready\n");
 			return -EBUSY;
 		}
-- 
2.2.2

[PATCH 07/16] ALSA: line6: Fix the error recovery in line6_pcm_acquire()

[Takashi Iwai]

line6_pcm_acquire() tries to restore the newly obtained resources at
the error path.  But some flags aren't recorded and released properly
when the corresponding buffer is already present.  These bits have to
be cleared in the error recovery, too.

Also, "flags_final" can be initialized to zero since we pass only the
subset of "channels" bits.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/usb/line6/pcm.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/sound/usb/line6/pcm.c b/sound/usb/line6/pcm.c
index f740b4490d75..9a2a15f4c4e4 100644
--- a/sound/usb/line6/pcm.c
+++ b/sound/usb/line6/pcm.c
@@ -106,7 +106,7 @@ int line6_pcm_acquire(struct snd_line6_pcm *line6pcm, int channels)
 		flags_new = flags_old | channels;
 	} while (cmpxchg(&line6pcm->flags, flags_old, flags_new) != flags_old);
 
-	flags_final = flags_old;
+	flags_final = 0;
 
 	line6pcm->prev_fbuf = NULL;
 
@@ -120,9 +120,9 @@ int line6_pcm_acquire(struct snd_line6_pcm *line6pcm, int channels)
 				err = -ENOMEM;
 				goto pcm_acquire_error;
 			}
-
-			flags_final |= channels & LINE6_BITS_CAPTURE_BUFFER;
 		}
+
+		flags_final |= channels & LINE6_BITS_CAPTURE_BUFFER;
 	}
 
 	if (test_flags(flags_old, flags_new, LINE6_BITS_CAPTURE_STREAM)) {
@@ -157,9 +157,9 @@ int line6_pcm_acquire(struct snd_line6_pcm *line6pcm, int channels)
 				err = -ENOMEM;
 				goto pcm_acquire_error;
 			}
-
-			flags_final |= channels & LINE6_BITS_PLAYBACK_BUFFER;
 		}
+
+		flags_final |= channels & LINE6_BITS_PLAYBACK_BUFFER;
 	}
 
 	if (test_flags(flags_old, flags_new, LINE6_BITS_PLAYBACK_STREAM)) {
@@ -187,7 +187,7 @@ pcm_acquire_error:
 	   If not all requested resources/streams could be obtained, release
 	   those which were successfully obtained (if any).
 	*/
-	line6_pcm_release(line6pcm, flags_final & channels);
+	line6_pcm_release(line6pcm, flags_final);
 	return err;
 }
 EXPORT_SYMBOL_GPL(line6_pcm_acquire);
-- 
2.2.2

[PATCH 08/16] ALSA: line6: Drop superfluous spinlock for trigger

[Takashi Iwai]

The trigger callback is already spinlocked, so we need no more lock
here (even for the linked substreams).  Let's drop it.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/usb/line6/pcm.c | 23 ++++++-----------------
 sound/usb/line6/pcm.h |  5 -----
 2 files changed, 6 insertions(+), 22 deletions(-)

diff --git a/sound/usb/line6/pcm.c b/sound/usb/line6/pcm.c
index 9a2a15f4c4e4..adbcac46b785 100644
--- a/sound/usb/line6/pcm.c
+++ b/sound/usb/line6/pcm.c
@@ -226,9 +226,8 @@ int snd_line6_trigger(struct snd_pcm_substream *substream, int cmd)
 {
 	struct snd_line6_pcm *line6pcm = snd_pcm_substream_chip(substream);
 	struct snd_pcm_substream *s;
-	int err;
+	int err = 0;
 
-	spin_lock(&line6pcm->lock_trigger);
 	clear_bit(LINE6_INDEX_PREPARED, &line6pcm->flags);
 
 	snd_pcm_group_for_each_entry(s, substream) {
@@ -237,32 +236,23 @@ int snd_line6_trigger(struct snd_pcm_substream *substream, int cmd)
 		switch (s->stream) {
 		case SNDRV_PCM_STREAM_PLAYBACK:
 			err = snd_line6_playback_trigger(line6pcm, cmd);
-
-			if (err < 0) {
-				spin_unlock(&line6pcm->lock_trigger);
-				return err;
-			}
-
 			break;
 
 		case SNDRV_PCM_STREAM_CAPTURE:
 			err = snd_line6_capture_trigger(line6pcm, cmd);
-
-			if (err < 0) {
-				spin_unlock(&line6pcm->lock_trigger);
-				return err;
-			}
-
 			break;
 
 		default:
 			dev_err(line6pcm->line6->ifcdev,
 				"Unknown stream direction %d\n", s->stream);
+			err = -EINVAL;
+			break;
 		}
+		if (err < 0)
+			break;
 	}
 
-	spin_unlock(&line6pcm->lock_trigger);
-	return 0;
+	return err;
 }
 
 /* control info callback */
@@ -427,7 +417,6 @@ int line6_init_pcm(struct usb_line6 *line6,
 
 	spin_lock_init(&line6pcm->lock_audio_out);
 	spin_lock_init(&line6pcm->lock_audio_in);
-	spin_lock_init(&line6pcm->lock_trigger);
 	line6pcm->impulse_period = LINE6_IMPULSE_DEFAULT_PERIOD;
 
 	line6->line6pcm = line6pcm;
diff --git a/sound/usb/line6/pcm.h b/sound/usb/line6/pcm.h
index c742b33666eb..a84753ee0fa2 100644
--- a/sound/usb/line6/pcm.h
+++ b/sound/usb/line6/pcm.h
@@ -308,11 +308,6 @@ struct snd_line6_pcm {
 	spinlock_t lock_audio_in;
 
 	/**
-		 Spin lock to protect trigger.
-	*/
-	spinlock_t lock_trigger;
-
-	/**
 		 PCM playback volume (left and right).
 	*/
 	int volume_playback[2];
-- 
2.2.2

[PATCH 09/16] ALSA: line6: Use incremental loop

[Takashi Iwai]

Using a decremental loop without particular reasons worsens the
readability a lot.  Use incremental loops instead.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/usb/line6/capture.c  |  4 ++--
 sound/usb/line6/pcm.c      |  6 +++---
 sound/usb/line6/playback.c | 12 ++++++------
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/sound/usb/line6/capture.c b/sound/usb/line6/capture.c
index 5a010ba163fa..97283e631e45 100644
--- a/sound/usb/line6/capture.c
+++ b/sound/usb/line6/capture.c
@@ -91,7 +91,7 @@ void line6_unlink_audio_in_urbs(struct snd_line6_pcm *line6pcm)
 {
 	unsigned int i;
 
-	for (i = LINE6_ISO_BUFFERS; i--;) {
+	for (i = 0; i < LINE6_ISO_BUFFERS; i++) {
 		if (test_bit(i, &line6pcm->active_urb_in)) {
 			if (!test_and_set_bit(i, &line6pcm->unlink_urb_in)) {
 				struct urb *u = line6pcm->urb_audio_in[i];
@@ -114,7 +114,7 @@ void line6_wait_clear_audio_in_urbs(struct snd_line6_pcm *line6pcm)
 
 	do {
 		alive = 0;
-		for (i = LINE6_ISO_BUFFERS; i--;) {
+		for (i = 0; i < LINE6_ISO_BUFFERS; i++) {
 			if (test_bit(i, &line6pcm->active_urb_in))
 				alive++;
 		}
diff --git a/sound/usb/line6/pcm.c b/sound/usb/line6/pcm.c
index adbcac46b785..43474c4ebb6b 100644
--- a/sound/usb/line6/pcm.c
+++ b/sound/usb/line6/pcm.c
@@ -273,7 +273,7 @@ static int snd_line6_control_playback_get(struct snd_kcontrol *kcontrol,
 	int i;
 	struct snd_line6_pcm *line6pcm = snd_kcontrol_chip(kcontrol);
 
-	for (i = 2; i--;)
+	for (i = 0; i < 2; i++)
 		ucontrol->value.integer.value[i] = line6pcm->volume_playback[i];
 
 	return 0;
@@ -286,7 +286,7 @@ static int snd_line6_control_playback_put(struct snd_kcontrol *kcontrol,
 	int i, changed = 0;
 	struct snd_line6_pcm *line6pcm = snd_kcontrol_chip(kcontrol);
 
-	for (i = 2; i--;)
+	for (i = 0; i < 2; i++)
 		if (line6pcm->volume_playback[i] !=
 		    ucontrol->value.integer.value[i]) {
 			line6pcm->volume_playback[i] =
@@ -330,7 +330,7 @@ static void line6_cleanup_pcm(struct snd_pcm *pcm)
 	int i;
 	struct snd_line6_pcm *line6pcm = snd_pcm_chip(pcm);
 
-	for (i = LINE6_ISO_BUFFERS; i--;) {
+	for (i = 0; i < LINE6_ISO_BUFFERS; i++) {
 		if (line6pcm->urb_audio_out[i]) {
 			usb_kill_urb(line6pcm->urb_audio_out[i]);
 			usb_free_urb(line6pcm->urb_audio_out[i]);
diff --git a/sound/usb/line6/playback.c b/sound/usb/line6/playback.c
index 1c9f95a370ff..ab9a83f0f864 100644
--- a/sound/usb/line6/playback.c
+++ b/sound/usb/line6/playback.c
@@ -297,7 +297,7 @@ void line6_unlink_audio_out_urbs(struct snd_line6_pcm *line6pcm)
 {
 	unsigned int i;
 
-	for (i = LINE6_ISO_BUFFERS; i--;) {
+	for (i = 0; i < LINE6_ISO_BUFFERS; i++) {
 		if (test_bit(i, &line6pcm->active_urb_out)) {
 			if (!test_and_set_bit(i, &line6pcm->unlink_urb_out)) {
 				struct urb *u = line6pcm->urb_audio_out[i];
@@ -320,7 +320,7 @@ void line6_wait_clear_audio_out_urbs(struct snd_line6_pcm *line6pcm)
 
 	do {
 		alive = 0;
-		for (i = LINE6_ISO_BUFFERS; i--;) {
+		for (i = 0; i < LINE6_ISO_BUFFERS; i++) {
 			if (test_bit(i, &line6pcm->active_urb_out))
 				alive++;
 		}
@@ -366,14 +366,14 @@ static void audio_out_callback(struct urb *urb)
 	line6pcm->last_frame_out = urb->start_frame;
 
 	/* find index of URB */
-	for (index = LINE6_ISO_BUFFERS; index--;)
+	for (index = 0; index < LINE6_ISO_BUFFERS; index++)
 		if (urb == line6pcm->urb_audio_out[index])
 			break;
 
-	if (index < 0)
+	if (index >= LINE6_ISO_BUFFERS)
 		return;		/* URB has been unlinked asynchronously */
 
-	for (i = LINE6_ISO_PACKETS; i--;)
+	for (i = 0; i < LINE6_ISO_PACKETS; i++)
 		length += urb->iso_frame_desc[i].length;
 
 	spin_lock_irqsave(&line6pcm->lock_audio_out, flags);
@@ -390,7 +390,7 @@ static void audio_out_callback(struct urb *urb)
 
 	clear_bit(index, &line6pcm->active_urb_out);
 
-	for (i = LINE6_ISO_PACKETS; i--;)
+	for (i = 0; i < LINE6_ISO_PACKETS; i++)
 		if (urb->iso_frame_desc[i].status == -EXDEV) {
 			shutdown = 1;
 			break;
-- 
2.2.2

[PATCH 10/16] ALSA: line6: Drop voodoo workarounds

[Takashi Iwai]

If the problem still really remains, we should fix it instead of
papering over it like this...

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/usb/line6/capture.c  | 12 ------------
 sound/usb/line6/playback.c | 12 ------------
 2 files changed, 24 deletions(-)

diff --git a/sound/usb/line6/capture.c b/sound/usb/line6/capture.c
index 97283e631e45..bad1b5b02786 100644
--- a/sound/usb/line6/capture.c
+++ b/sound/usb/line6/capture.c
@@ -297,18 +297,6 @@ static int snd_line6_capture_hw_params(struct snd_pcm_substream *substream,
 	int ret;
 	struct snd_line6_pcm *line6pcm = snd_pcm_substream_chip(substream);
 
-	/* -- Florian Demski [FD] */
-	/* don't ask me why, but this fixes the bug on my machine */
-	if (line6pcm == NULL) {
-		if (substream->pcm == NULL)
-			return -ENOMEM;
-		if (substream->pcm->private_data == NULL)
-			return -ENOMEM;
-		substream->private_data = substream->pcm->private_data;
-		line6pcm = snd_pcm_substream_chip(substream);
-	}
-	/* -- [FD] end */
-
 	ret = line6_pcm_acquire(line6pcm, LINE6_BIT_PCM_ALSA_CAPTURE_BUFFER);
 
 	if (ret < 0)
diff --git a/sound/usb/line6/playback.c b/sound/usb/line6/playback.c
index ab9a83f0f864..7e031b1761aa 100644
--- a/sound/usb/line6/playback.c
+++ b/sound/usb/line6/playback.c
@@ -445,18 +445,6 @@ static int snd_line6_playback_hw_params(struct snd_pcm_substream *substream,
 	int ret;
 	struct snd_line6_pcm *line6pcm = snd_pcm_substream_chip(substream);
 
-	/* -- Florian Demski [FD] */
-	/* don't ask me why, but this fixes the bug on my machine */
-	if (line6pcm == NULL) {
-		if (substream->pcm == NULL)
-			return -ENOMEM;
-		if (substream->pcm->private_data == NULL)
-			return -ENOMEM;
-		substream->private_data = substream->pcm->private_data;
-		line6pcm = snd_pcm_substream_chip(substream);
-	}
-	/* -- [FD] end */
-
 	ret = line6_pcm_acquire(line6pcm, LINE6_BIT_PCM_ALSA_PLAYBACK_BUFFER);
 
 	if (ret < 0)
-- 
2.2.2

[PATCH 11/16] ALSA: line6: Rearrange PCM structure

[Takashi Iwai]

Introduce a new line6_pcm_stream structure and group individual
fields of snd_line6_pcm struct to playback and capture groups.

This patch itself just does rename and nothing else.  More
meaningful cleanups based on these fields shuffling will follow.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/usb/line6/capture.c  |  64 +++++++++---------
 sound/usb/line6/pcm.c      |  50 +++++++-------
 sound/usb/line6/pcm.h      | 158 +++++++++++++--------------------------------
 sound/usb/line6/playback.c |  78 +++++++++++-----------
 4 files changed, 142 insertions(+), 208 deletions(-)

diff --git a/sound/usb/line6/capture.c b/sound/usb/line6/capture.c
index bad1b5b02786..439f1941eb56 100644
--- a/sound/usb/line6/capture.c
+++ b/sound/usb/line6/capture.c
@@ -29,17 +29,17 @@ static int submit_audio_in_urb(struct snd_line6_pcm *line6pcm)
 	int ret;
 	struct urb *urb_in;
 
-	spin_lock_irqsave(&line6pcm->lock_audio_in, flags);
+	spin_lock_irqsave(&line6pcm->in.lock, flags);
 	index =
-	    find_first_zero_bit(&line6pcm->active_urb_in, LINE6_ISO_BUFFERS);
+	    find_first_zero_bit(&line6pcm->in.active_urbs, LINE6_ISO_BUFFERS);
 
 	if (index < 0 || index >= LINE6_ISO_BUFFERS) {
-		spin_unlock_irqrestore(&line6pcm->lock_audio_in, flags);
+		spin_unlock_irqrestore(&line6pcm->in.lock, flags);
 		dev_err(line6pcm->line6->ifcdev, "no free URB found\n");
 		return -EINVAL;
 	}
 
-	urb_in = line6pcm->urb_audio_in[index];
+	urb_in = line6pcm->in.urbs[index];
 	urb_size = 0;
 
 	for (i = 0; i < LINE6_ISO_PACKETS; ++i) {
@@ -51,7 +51,7 @@ static int submit_audio_in_urb(struct snd_line6_pcm *line6pcm)
 	}
 
 	urb_in->transfer_buffer =
-	    line6pcm->buffer_in +
+	    line6pcm->in.buffer +
 	    index * LINE6_ISO_PACKETS * line6pcm->max_packet_size;
 	urb_in->transfer_buffer_length = urb_size;
 	urb_in->context = line6pcm;
@@ -59,12 +59,12 @@ static int submit_audio_in_urb(struct snd_line6_pcm *line6pcm)
 	ret = usb_submit_urb(urb_in, GFP_ATOMIC);
 
 	if (ret == 0)
-		set_bit(index, &line6pcm->active_urb_in);
+		set_bit(index, &line6pcm->in.active_urbs);
 	else
 		dev_err(line6pcm->line6->ifcdev,
 			"URB in #%d submission failed (%d)\n", index, ret);
 
-	spin_unlock_irqrestore(&line6pcm->lock_audio_in, flags);
+	spin_unlock_irqrestore(&line6pcm->in.lock, flags);
 	return 0;
 }
 
@@ -92,9 +92,9 @@ void line6_unlink_audio_in_urbs(struct snd_line6_pcm *line6pcm)
 	unsigned int i;
 
 	for (i = 0; i < LINE6_ISO_BUFFERS; i++) {
-		if (test_bit(i, &line6pcm->active_urb_in)) {
-			if (!test_and_set_bit(i, &line6pcm->unlink_urb_in)) {
-				struct urb *u = line6pcm->urb_audio_in[i];
+		if (test_bit(i, &line6pcm->in.active_urbs)) {
+			if (!test_and_set_bit(i, &line6pcm->in.unlink_urbs)) {
+				struct urb *u = line6pcm->in.urbs[i];
 
 				usb_unlink_urb(u);
 			}
@@ -115,7 +115,7 @@ void line6_wait_clear_audio_in_urbs(struct snd_line6_pcm *line6pcm)
 	do {
 		alive = 0;
 		for (i = 0; i < LINE6_ISO_BUFFERS; i++) {
-			if (test_bit(i, &line6pcm->active_urb_in))
+			if (test_bit(i, &line6pcm->in.active_urbs))
 				alive++;
 		}
 		if (!alive)
@@ -150,18 +150,18 @@ void line6_capture_copy(struct snd_line6_pcm *line6pcm, char *fbuf, int fsize)
 	if (runtime == NULL)
 		return;
 
-	if (line6pcm->pos_in_done + frames > runtime->buffer_size) {
+	if (line6pcm->in.pos_done + frames > runtime->buffer_size) {
 		/*
 		   The transferred area goes over buffer boundary,
 		   copy two separate chunks.
 		 */
 		int len;
 
-		len = runtime->buffer_size - line6pcm->pos_in_done;
+		len = runtime->buffer_size - line6pcm->in.pos_done;
 
 		if (len > 0) {
 			memcpy(runtime->dma_area +
-			       line6pcm->pos_in_done * bytes_per_frame, fbuf,
+			       line6pcm->in.pos_done * bytes_per_frame, fbuf,
 			       len * bytes_per_frame);
 			memcpy(runtime->dma_area, fbuf + len * bytes_per_frame,
 			       (frames - len) * bytes_per_frame);
@@ -173,12 +173,12 @@ void line6_capture_copy(struct snd_line6_pcm *line6pcm, char *fbuf, int fsize)
 	} else {
 		/* copy single chunk */
 		memcpy(runtime->dma_area +
-		       line6pcm->pos_in_done * bytes_per_frame, fbuf, fsize);
+		       line6pcm->in.pos_done * bytes_per_frame, fbuf, fsize);
 	}
 
-	line6pcm->pos_in_done += frames;
-	if (line6pcm->pos_in_done >= runtime->buffer_size)
-		line6pcm->pos_in_done -= runtime->buffer_size;
+	line6pcm->in.pos_done += frames;
+	if (line6pcm->in.pos_done >= runtime->buffer_size)
+		line6pcm->in.pos_done -= runtime->buffer_size;
 }
 
 void line6_capture_check_period(struct snd_line6_pcm *line6pcm, int length)
@@ -186,17 +186,17 @@ void line6_capture_check_period(struct snd_line6_pcm *line6pcm, int length)
 	struct snd_pcm_substream *substream =
 	    get_substream(line6pcm, SNDRV_PCM_STREAM_CAPTURE);
 
-	line6pcm->bytes_in += length;
-	if (line6pcm->bytes_in >= line6pcm->period_in) {
-		line6pcm->bytes_in %= line6pcm->period_in;
+	line6pcm->in.bytes += length;
+	if (line6pcm->in.bytes >= line6pcm->in.period) {
+		line6pcm->in.bytes %= line6pcm->in.period;
 		snd_pcm_period_elapsed(substream);
 	}
 }
 
 void line6_free_capture_buffer(struct snd_line6_pcm *line6pcm)
 {
-	kfree(line6pcm->buffer_in);
-	line6pcm->buffer_in = NULL;
+	kfree(line6pcm->in.buffer);
+	line6pcm->in.buffer = NULL;
 }
 
 /*
@@ -209,14 +209,14 @@ static void audio_in_callback(struct urb *urb)
 
 	struct snd_line6_pcm *line6pcm = (struct snd_line6_pcm *)urb->context;
 
-	line6pcm->last_frame_in = urb->start_frame;
+	line6pcm->in.last_frame = urb->start_frame;
 
 	/* find index of URB */
 	for (index = 0; index < LINE6_ISO_BUFFERS; ++index)
-		if (urb == line6pcm->urb_audio_in[index])
+		if (urb == line6pcm->in.urbs[index])
 			break;
 
-	spin_lock_irqsave(&line6pcm->lock_audio_in, flags);
+	spin_lock_irqsave(&line6pcm->in.lock, flags);
 
 	for (i = 0; i < LINE6_ISO_PACKETS; ++i) {
 		char *fbuf;
@@ -249,12 +249,12 @@ static void audio_in_callback(struct urb *urb)
 				line6_capture_copy(line6pcm, fbuf, fsize);
 	}
 
-	clear_bit(index, &line6pcm->active_urb_in);
+	clear_bit(index, &line6pcm->in.active_urbs);
 
-	if (test_and_clear_bit(index, &line6pcm->unlink_urb_in))
+	if (test_and_clear_bit(index, &line6pcm->in.unlink_urbs))
 		shutdown = 1;
 
-	spin_unlock_irqrestore(&line6pcm->lock_audio_in, flags);
+	spin_unlock_irqrestore(&line6pcm->in.lock, flags);
 
 	if (!shutdown) {
 		submit_audio_in_urb(line6pcm);
@@ -309,7 +309,7 @@ static int snd_line6_capture_hw_params(struct snd_pcm_substream *substream,
 		return ret;
 	}
 
-	line6pcm->period_in = params_period_bytes(hw_params);
+	line6pcm->in.period = params_period_bytes(hw_params);
 	return 0;
 }
 
@@ -361,7 +361,7 @@ snd_line6_capture_pointer(struct snd_pcm_substream *substream)
 {
 	struct snd_line6_pcm *line6pcm = snd_pcm_substream_chip(substream);
 
-	return line6pcm->pos_in_done;
+	return line6pcm->in.pos_done;
 }
 
 /* capture operators */
@@ -386,7 +386,7 @@ int line6_create_audio_in_urbs(struct snd_line6_pcm *line6pcm)
 		struct urb *urb;
 
 		/* URB for audio in: */
-		urb = line6pcm->urb_audio_in[i] =
+		urb = line6pcm->in.urbs[i] =
 		    usb_alloc_urb(LINE6_ISO_PACKETS, GFP_KERNEL);
 
 		if (urb == NULL)
diff --git a/sound/usb/line6/pcm.c b/sound/usb/line6/pcm.c
index 43474c4ebb6b..738bfd82cecd 100644
--- a/sound/usb/line6/pcm.c
+++ b/sound/usb/line6/pcm.c
@@ -112,11 +112,11 @@ int line6_pcm_acquire(struct snd_line6_pcm *line6pcm, int channels)
 
 	if (test_flags(flags_old, flags_new, LINE6_BITS_CAPTURE_BUFFER)) {
 		/* Invoked multiple times in a row so allocate once only */
-		if (!line6pcm->buffer_in) {
-			line6pcm->buffer_in =
+		if (!line6pcm->in.buffer) {
+			line6pcm->in.buffer =
 				kmalloc(LINE6_ISO_BUFFERS * LINE6_ISO_PACKETS *
 					line6pcm->max_packet_size, GFP_KERNEL);
-			if (!line6pcm->buffer_in) {
+			if (!line6pcm->in.buffer) {
 				err = -ENOMEM;
 				goto pcm_acquire_error;
 			}
@@ -131,13 +131,13 @@ int line6_pcm_acquire(struct snd_line6_pcm *line6pcm, int channels)
 		   a bug, we therefore report an error if capturing is restarted
 		   too soon.
 		 */
-		if (line6pcm->active_urb_in || line6pcm->unlink_urb_in) {
+		if (line6pcm->in.active_urbs || line6pcm->in.unlink_urbs) {
 			dev_err(line6pcm->line6->ifcdev, "Device not yet ready\n");
 			err = -EBUSY;
 			goto pcm_acquire_error;
 		}
 
-		line6pcm->count_in = 0;
+		line6pcm->in.count = 0;
 		line6pcm->prev_fsize = 0;
 		err = line6_submit_audio_in_all_urbs(line6pcm);
 
@@ -149,11 +149,11 @@ int line6_pcm_acquire(struct snd_line6_pcm *line6pcm, int channels)
 
 	if (test_flags(flags_old, flags_new, LINE6_BITS_PLAYBACK_BUFFER)) {
 		/* Invoked multiple times in a row so allocate once only */
-		if (!line6pcm->buffer_out) {
-			line6pcm->buffer_out =
+		if (!line6pcm->out.buffer) {
+			line6pcm->out.buffer =
 				kmalloc(LINE6_ISO_BUFFERS * LINE6_ISO_PACKETS *
 					line6pcm->max_packet_size, GFP_KERNEL);
-			if (!line6pcm->buffer_out) {
+			if (!line6pcm->out.buffer) {
 				err = -ENOMEM;
 				goto pcm_acquire_error;
 			}
@@ -166,12 +166,12 @@ int line6_pcm_acquire(struct snd_line6_pcm *line6pcm, int channels)
 		/*
 		  See comment above regarding PCM restart.
 		*/
-		if (line6pcm->active_urb_out || line6pcm->unlink_urb_out) {
+		if (line6pcm->out.active_urbs || line6pcm->out.unlink_urbs) {
 			dev_err(line6pcm->line6->ifcdev, "Device not yet ready\n");
 			return -EBUSY;
 		}
 
-		line6pcm->count_out = 0;
+		line6pcm->out.count = 0;
 		err = line6_submit_audio_out_all_urbs(line6pcm);
 
 		if (err < 0)
@@ -331,13 +331,13 @@ static void line6_cleanup_pcm(struct snd_pcm *pcm)
 	struct snd_line6_pcm *line6pcm = snd_pcm_chip(pcm);
 
 	for (i = 0; i < LINE6_ISO_BUFFERS; i++) {
-		if (line6pcm->urb_audio_out[i]) {
-			usb_kill_urb(line6pcm->urb_audio_out[i]);
-			usb_free_urb(line6pcm->urb_audio_out[i]);
+		if (line6pcm->out.urbs[i]) {
+			usb_kill_urb(line6pcm->out.urbs[i]);
+			usb_free_urb(line6pcm->out.urbs[i]);
 		}
-		if (line6pcm->urb_audio_in[i]) {
-			usb_kill_urb(line6pcm->urb_audio_in[i]);
-			usb_free_urb(line6pcm->urb_audio_in[i]);
+		if (line6pcm->in.urbs[i]) {
+			usb_kill_urb(line6pcm->in.urbs[i]);
+			usb_free_urb(line6pcm->in.urbs[i]);
 		}
 	}
 	kfree(line6pcm);
@@ -415,8 +415,8 @@ int line6_init_pcm(struct usb_line6 *line6,
 			usb_maxpacket(line6->usbdev,
 				usb_sndisocpipe(line6->usbdev, ep_write), 1));
 
-	spin_lock_init(&line6pcm->lock_audio_out);
-	spin_lock_init(&line6pcm->lock_audio_in);
+	spin_lock_init(&line6pcm->out.lock);
+	spin_lock_init(&line6pcm->in.lock);
 	line6pcm->impulse_period = LINE6_IMPULSE_DEFAULT_PERIOD;
 
 	line6->line6pcm = line6pcm;
@@ -464,13 +464,13 @@ int snd_line6_prepare(struct snd_pcm_substream *substream)
 	}
 
 	if (!test_and_set_bit(LINE6_INDEX_PREPARED, &line6pcm->flags)) {
-		line6pcm->count_out = 0;
-		line6pcm->pos_out = 0;
-		line6pcm->pos_out_done = 0;
-		line6pcm->bytes_out = 0;
-		line6pcm->count_in = 0;
-		line6pcm->pos_in_done = 0;
-		line6pcm->bytes_in = 0;
+		line6pcm->out.count = 0;
+		line6pcm->out.pos = 0;
+		line6pcm->out.pos_done = 0;
+		line6pcm->out.bytes = 0;
+		line6pcm->in.count = 0;
+		line6pcm->in.pos_done = 0;
+		line6pcm->in.bytes = 0;
 	}
 
 	return 0;
diff --git a/sound/usb/line6/pcm.h b/sound/usb/line6/pcm.h
index a84753ee0fa2..4c74f4e85074 100644
--- a/sound/usb/line6/pcm.h
+++ b/sound/usb/line6/pcm.h
@@ -165,115 +165,78 @@ struct line6_pcm_properties {
 	int bytes_per_frame;
 };
 
-struct snd_line6_pcm {
-	/**
-		 Pointer back to the Line 6 driver data structure.
-	*/
-	struct usb_line6 *line6;
+struct line6_pcm_stream {
+	/* allocated URBs */
+	struct urb *urbs[LINE6_ISO_BUFFERS];
 
-	/**
-		 Properties.
-	*/
-	struct line6_pcm_properties *properties;
+	/* Temporary buffer;
+	 * Since the packet size is not known in advance, this buffer is
+	 * large enough to store maximum size packets.
+	 */
+	unsigned char *buffer;
 
-	/**
-		 ALSA pcm stream
-	*/
-	struct snd_pcm *pcm;
+	/* Free frame position in the buffer. */
+	snd_pcm_uframes_t pos;
 
-	/**
-		 URBs for audio playback.
-	*/
-	struct urb *urb_audio_out[LINE6_ISO_BUFFERS];
+	/* Count processed bytes;
+	 * This is modulo period size (to determine when a period is finished).
+	 */
+	unsigned bytes;
 
-	/**
-		 URBs for audio capture.
-	*/
-	struct urb *urb_audio_in[LINE6_ISO_BUFFERS];
+	/* Counter to create desired sample rate */
+	unsigned count;
 
-	/**
-		 Temporary buffer for playback.
-		 Since the packet size is not known in advance, this buffer is
-		 large enough to store maximum size packets.
-	*/
-	unsigned char *buffer_out;
+	/* period size in bytes */
+	unsigned period;
 
-	/**
-		 Temporary buffer for capture.
-		 Since the packet size is not known in advance, this buffer is
-		 large enough to store maximum size packets.
-	*/
-	unsigned char *buffer_in;
+	/* Processed frame position in the buffer;
+	 * The contents of the ring buffer have been consumed by the USB
+	 * subsystem (i.e., sent to the USB device) up to this position.
+	 */
+	snd_pcm_uframes_t pos_done;
 
-	/**
-		 Previously captured frame (for software monitoring).
-	*/
-	unsigned char *prev_fbuf;
+	/* Bit mask of active URBs */
+	unsigned long active_urbs;
 
-	/**
-		 Size of previously captured frame (for software monitoring).
-	*/
-	int prev_fsize;
-
-	/**
-		 Free frame position in the playback buffer.
-	*/
-	snd_pcm_uframes_t pos_out;
+	/* Bit mask of URBs currently being unlinked */
+	unsigned long unlink_urbs;
 
-	/**
-		 Count processed bytes for playback.
-		 This is modulo period size (to determine when a period is
-		 finished).
-	*/
-	unsigned bytes_out;
+	/* Spin lock to protect updates of the buffer positions (not contents)
+	 */
+	spinlock_t lock;
 
-	/**
-		 Counter to create desired playback sample rate.
-	*/
-	unsigned count_out;
-
-	/**
-		 Playback period size in bytes
-	*/
-	unsigned period_out;
+	int last_frame;
+};
 
+struct snd_line6_pcm {
 	/**
-		 Processed frame position in the playback buffer.
-		 The contents of the output ring buffer have been consumed by
-		 the USB subsystem (i.e., sent to the USB device) up to this
-		 position.
+		 Pointer back to the Line 6 driver data structure.
 	*/
-	snd_pcm_uframes_t pos_out_done;
+	struct usb_line6 *line6;
 
 	/**
-		 Count processed bytes for capture.
-		 This is modulo period size (to determine when a period is
-		 finished).
+		 Properties.
 	*/
-	unsigned bytes_in;
+	struct line6_pcm_properties *properties;
 
 	/**
-		 Counter to create desired capture sample rate.
+		 ALSA pcm stream
 	*/
-	unsigned count_in;
+	struct snd_pcm *pcm;
 
-	/**
-		 Capture period size in bytes
-	*/
-	unsigned period_in;
+	/* Capture and playback streams */
+	struct line6_pcm_stream in;
+	struct line6_pcm_stream out;
 
 	/**
-		 Processed frame position in the capture buffer.
-		 The contents of the output ring buffer have been consumed by
-		 the USB subsystem (i.e., sent to the USB device) up to this
-		 position.
+		 Previously captured frame (for software monitoring).
 	*/
-	snd_pcm_uframes_t pos_in_done;
+	unsigned char *prev_fbuf;
 
 	/**
-		 Bit mask of active playback URBs.
+		 Size of previously captured frame (for software monitoring).
 	*/
-	unsigned long active_urb_out;
+	int prev_fsize;
 
 	/**
 		 Maximum size of USB packet.
@@ -281,33 +244,6 @@ struct snd_line6_pcm {
 	int max_packet_size;
 
 	/**
-		 Bit mask of active capture URBs.
-	*/
-	unsigned long active_urb_in;
-
-	/**
-		 Bit mask of playback URBs currently being unlinked.
-	*/
-	unsigned long unlink_urb_out;
-
-	/**
-		 Bit mask of capture URBs currently being unlinked.
-	*/
-	unsigned long unlink_urb_in;
-
-	/**
-		 Spin lock to protect updates of the playback buffer positions (not
-		 contents!)
-	*/
-	spinlock_t lock_audio_out;
-
-	/**
-		 Spin lock to protect updates of the capture buffer positions (not
-		 contents!)
-	*/
-	spinlock_t lock_audio_in;
-
-	/**
 		 PCM playback volume (left and right).
 	*/
 	int volume_playback[2];
@@ -336,8 +272,6 @@ struct snd_line6_pcm {
 		 Several status bits (see LINE6_BIT_*).
 	*/
 	unsigned long flags;
-
-	int last_frame_in, last_frame_out;
 };
 
 extern int line6_init_pcm(struct usb_line6 *line6,
diff --git a/sound/usb/line6/playback.c b/sound/usb/line6/playback.c
index 7e031b1761aa..d619c1718306 100644
--- a/sound/usb/line6/playback.c
+++ b/sound/usb/line6/playback.c
@@ -145,17 +145,17 @@ static int submit_audio_out_urb(struct snd_line6_pcm *line6pcm)
 	    (USB_INTERVALS_PER_SECOND / LINE6_ISO_INTERVAL);
 	struct urb *urb_out;
 
-	spin_lock_irqsave(&line6pcm->lock_audio_out, flags);
+	spin_lock_irqsave(&line6pcm->out.lock, flags);
 	index =
-	    find_first_zero_bit(&line6pcm->active_urb_out, LINE6_ISO_BUFFERS);
+	    find_first_zero_bit(&line6pcm->out.active_urbs, LINE6_ISO_BUFFERS);
 
 	if (index < 0 || index >= LINE6_ISO_BUFFERS) {
-		spin_unlock_irqrestore(&line6pcm->lock_audio_out, flags);
+		spin_unlock_irqrestore(&line6pcm->out.lock, flags);
 		dev_err(line6pcm->line6->ifcdev, "no free URB found\n");
 		return -EINVAL;
 	}
 
-	urb_out = line6pcm->urb_audio_out[index];
+	urb_out = line6pcm->out.urbs[index];
 	urb_size = 0;
 
 	for (i = 0; i < LINE6_ISO_PACKETS; ++i) {
@@ -170,9 +170,9 @@ static int submit_audio_out_urb(struct snd_line6_pcm *line6pcm)
 		if (fsize == 0) {
 			int n;
 
-			line6pcm->count_out += frame_increment;
-			n = line6pcm->count_out / frame_factor;
-			line6pcm->count_out -= n * frame_factor;
+			line6pcm->out.count += frame_increment;
+			n = line6pcm->out.count / frame_factor;
+			line6pcm->out.count -= n * frame_factor;
 			fsize = n * bytes_per_frame;
 		}
 
@@ -183,14 +183,14 @@ static int submit_audio_out_urb(struct snd_line6_pcm *line6pcm)
 
 	if (urb_size == 0) {
 		/* can't determine URB size */
-		spin_unlock_irqrestore(&line6pcm->lock_audio_out, flags);
+		spin_unlock_irqrestore(&line6pcm->out.lock, flags);
 		dev_err(line6pcm->line6->ifcdev, "driver bug: urb_size = 0\n");
 		return -EINVAL;
 	}
 
 	urb_frames = urb_size / bytes_per_frame;
 	urb_out->transfer_buffer =
-	    line6pcm->buffer_out +
+	    line6pcm->out.buffer +
 	    index * LINE6_ISO_PACKETS * line6pcm->max_packet_size;
 	urb_out->transfer_buffer_length = urb_size;
 	urb_out->context = line6pcm;
@@ -200,19 +200,19 @@ static int submit_audio_out_urb(struct snd_line6_pcm *line6pcm)
 		struct snd_pcm_runtime *runtime =
 		    get_substream(line6pcm, SNDRV_PCM_STREAM_PLAYBACK)->runtime;
 
-		if (line6pcm->pos_out + urb_frames > runtime->buffer_size) {
+		if (line6pcm->out.pos + urb_frames > runtime->buffer_size) {
 			/*
 			   The transferred area goes over buffer boundary,
 			   copy the data to the temp buffer.
 			 */
 			int len;
 
-			len = runtime->buffer_size - line6pcm->pos_out;
+			len = runtime->buffer_size - line6pcm->out.pos;
 
 			if (len > 0) {
 				memcpy(urb_out->transfer_buffer,
 				       runtime->dma_area +
-				       line6pcm->pos_out * bytes_per_frame,
+				       line6pcm->out.pos * bytes_per_frame,
 				       len * bytes_per_frame);
 				memcpy(urb_out->transfer_buffer +
 				       len * bytes_per_frame, runtime->dma_area,
@@ -223,13 +223,13 @@ static int submit_audio_out_urb(struct snd_line6_pcm *line6pcm)
 		} else {
 			memcpy(urb_out->transfer_buffer,
 			       runtime->dma_area +
-			       line6pcm->pos_out * bytes_per_frame,
+			       line6pcm->out.pos * bytes_per_frame,
 			       urb_out->transfer_buffer_length);
 		}
 
-		line6pcm->pos_out += urb_frames;
-		if (line6pcm->pos_out >= runtime->buffer_size)
-			line6pcm->pos_out -= runtime->buffer_size;
+		line6pcm->out.pos += urb_frames;
+		if (line6pcm->out.pos >= runtime->buffer_size)
+			line6pcm->out.pos -= runtime->buffer_size;
 	} else {
 		memset(urb_out->transfer_buffer, 0,
 		       urb_out->transfer_buffer_length);
@@ -265,12 +265,12 @@ static int submit_audio_out_urb(struct snd_line6_pcm *line6pcm)
 	ret = usb_submit_urb(urb_out, GFP_ATOMIC);
 
 	if (ret == 0)
-		set_bit(index, &line6pcm->active_urb_out);
+		set_bit(index, &line6pcm->out.active_urbs);
 	else
 		dev_err(line6pcm->line6->ifcdev,
 			"URB out #%d submission failed (%d)\n", index, ret);
 
-	spin_unlock_irqrestore(&line6pcm->lock_audio_out, flags);
+	spin_unlock_irqrestore(&line6pcm->out.lock, flags);
 	return 0;
 }
 
@@ -298,9 +298,9 @@ void line6_unlink_audio_out_urbs(struct snd_line6_pcm *line6pcm)
 	unsigned int i;
 
 	for (i = 0; i < LINE6_ISO_BUFFERS; i++) {
-		if (test_bit(i, &line6pcm->active_urb_out)) {
-			if (!test_and_set_bit(i, &line6pcm->unlink_urb_out)) {
-				struct urb *u = line6pcm->urb_audio_out[i];
+		if (test_bit(i, &line6pcm->out.active_urbs)) {
+			if (!test_and_set_bit(i, &line6pcm->out.unlink_urbs)) {
+				struct urb *u = line6pcm->out.urbs[i];
 
 				usb_unlink_urb(u);
 			}
@@ -321,7 +321,7 @@ void line6_wait_clear_audio_out_urbs(struct snd_line6_pcm *line6pcm)
 	do {
 		alive = 0;
 		for (i = 0; i < LINE6_ISO_BUFFERS; i++) {
-			if (test_bit(i, &line6pcm->active_urb_out))
+			if (test_bit(i, &line6pcm->out.active_urbs))
 				alive++;
 		}
 		if (!alive)
@@ -344,8 +344,8 @@ void line6_unlink_wait_clear_audio_out_urbs(struct snd_line6_pcm *line6pcm)
 
 void line6_free_playback_buffer(struct snd_line6_pcm *line6pcm)
 {
-	kfree(line6pcm->buffer_out);
-	line6pcm->buffer_out = NULL;
+	kfree(line6pcm->out.buffer);
+	line6pcm->out.buffer = NULL;
 }
 
 /*
@@ -363,11 +363,11 @@ static void audio_out_callback(struct urb *urb)
 	memset(urb->transfer_buffer, 0, urb->transfer_buffer_length);
 #endif
 
-	line6pcm->last_frame_out = urb->start_frame;
+	line6pcm->out.last_frame = urb->start_frame;
 
 	/* find index of URB */
 	for (index = 0; index < LINE6_ISO_BUFFERS; index++)
-		if (urb == line6pcm->urb_audio_out[index])
+		if (urb == line6pcm->out.urbs[index])
 			break;
 
 	if (index >= LINE6_ISO_BUFFERS)
@@ -376,19 +376,19 @@ static void audio_out_callback(struct urb *urb)
 	for (i = 0; i < LINE6_ISO_PACKETS; i++)
 		length += urb->iso_frame_desc[i].length;
 
-	spin_lock_irqsave(&line6pcm->lock_audio_out, flags);
+	spin_lock_irqsave(&line6pcm->out.lock, flags);
 
 	if (test_bit(LINE6_INDEX_PCM_ALSA_PLAYBACK_STREAM, &line6pcm->flags)) {
 		struct snd_pcm_runtime *runtime = substream->runtime;
 
-		line6pcm->pos_out_done +=
+		line6pcm->out.pos_done +=
 		    length / line6pcm->properties->bytes_per_frame;
 
-		if (line6pcm->pos_out_done >= runtime->buffer_size)
-			line6pcm->pos_out_done -= runtime->buffer_size;
+		if (line6pcm->out.pos_done >= runtime->buffer_size)
+			line6pcm->out.pos_done -= runtime->buffer_size;
 	}
 
-	clear_bit(index, &line6pcm->active_urb_out);
+	clear_bit(index, &line6pcm->out.active_urbs);
 
 	for (i = 0; i < LINE6_ISO_PACKETS; i++)
 		if (urb->iso_frame_desc[i].status == -EXDEV) {
@@ -396,19 +396,19 @@ static void audio_out_callback(struct urb *urb)
 			break;
 		}
 
-	if (test_and_clear_bit(index, &line6pcm->unlink_urb_out))
+	if (test_and_clear_bit(index, &line6pcm->out.unlink_urbs))
 		shutdown = 1;
 
-	spin_unlock_irqrestore(&line6pcm->lock_audio_out, flags);
+	spin_unlock_irqrestore(&line6pcm->out.lock, flags);
 
 	if (!shutdown) {
 		submit_audio_out_urb(line6pcm);
 
 		if (test_bit(LINE6_INDEX_PCM_ALSA_PLAYBACK_STREAM,
 			     &line6pcm->flags)) {
-			line6pcm->bytes_out += length;
-			if (line6pcm->bytes_out >= line6pcm->period_out) {
-				line6pcm->bytes_out %= line6pcm->period_out;
+			line6pcm->out.bytes += length;
+			if (line6pcm->out.bytes >= line6pcm->out.period) {
+				line6pcm->out.bytes %= line6pcm->out.period;
 				snd_pcm_period_elapsed(substream);
 			}
 		}
@@ -457,7 +457,7 @@ static int snd_line6_playback_hw_params(struct snd_pcm_substream *substream,
 		return ret;
 	}
 
-	line6pcm->period_out = params_period_bytes(hw_params);
+	line6pcm->out.period = params_period_bytes(hw_params);
 	return 0;
 }
 
@@ -517,7 +517,7 @@ snd_line6_playback_pointer(struct snd_pcm_substream *substream)
 {
 	struct snd_line6_pcm *line6pcm = snd_pcm_substream_chip(substream);
 
-	return line6pcm->pos_out_done;
+	return line6pcm->out.pos_done;
 }
 
 /* playback operators */
@@ -542,7 +542,7 @@ int line6_create_audio_out_urbs(struct snd_line6_pcm *line6pcm)
 		struct urb *urb;
 
 		/* URB for audio out: */
-		urb = line6pcm->urb_audio_out[i] =
+		urb = line6pcm->out.urbs[i] =
 		    usb_alloc_urb(LINE6_ISO_PACKETS, GFP_KERNEL);
 
 		if (urb == NULL)
-- 
2.2.2

[PATCH 12/16] ALSA: line6: Consolidate URB unlink and sync helpers

[Takashi Iwai]

The codes to unlink and sync URBs are identical for both playback and
capture streams.  Consolidate to single helper functions.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/usb/line6/capture.c  | 52 ---------------------------
 sound/usb/line6/capture.h  |  4 ---
 sound/usb/line6/pcm.c      | 90 +++++++++++++++++++++++++++++++++++-----------
 sound/usb/line6/playback.c | 52 ---------------------------
 sound/usb/line6/playback.h |  4 ---
 5 files changed, 69 insertions(+), 133 deletions(-)

diff --git a/sound/usb/line6/capture.c b/sound/usb/line6/capture.c
index 439f1941eb56..1d477d7a42fb 100644
--- a/sound/usb/line6/capture.c
+++ b/sound/usb/line6/capture.c
@@ -85,58 +85,6 @@ int line6_submit_audio_in_all_urbs(struct snd_line6_pcm *line6pcm)
 }
 
 /*
-	Unlink all currently active capture URBs.
-*/
-void line6_unlink_audio_in_urbs(struct snd_line6_pcm *line6pcm)
-{
-	unsigned int i;
-
-	for (i = 0; i < LINE6_ISO_BUFFERS; i++) {
-		if (test_bit(i, &line6pcm->in.active_urbs)) {
-			if (!test_and_set_bit(i, &line6pcm->in.unlink_urbs)) {
-				struct urb *u = line6pcm->in.urbs[i];
-
-				usb_unlink_urb(u);
-			}
-		}
-	}
-}
-
-/*
-	Wait until unlinking of all currently active capture URBs has been
-	finished.
-*/
-void line6_wait_clear_audio_in_urbs(struct snd_line6_pcm *line6pcm)
-{
-	int timeout = HZ;
-	unsigned int i;
-	int alive;
-
-	do {
-		alive = 0;
-		for (i = 0; i < LINE6_ISO_BUFFERS; i++) {
-			if (test_bit(i, &line6pcm->in.active_urbs))
-				alive++;
-		}
-		if (!alive)
-			break;
-		set_current_state(TASK_UNINTERRUPTIBLE);
-		schedule_timeout(1);
-	} while (--timeout > 0);
-	if (alive)
-		snd_printk(KERN_ERR "timeout: still %d active urbs..\n", alive);
-}
-
-/*
-	Unlink all currently active capture URBs, and wait for finishing.
-*/
-void line6_unlink_wait_clear_audio_in_urbs(struct snd_line6_pcm *line6pcm)
-{
-	line6_unlink_audio_in_urbs(line6pcm);
-	line6_wait_clear_audio_in_urbs(line6pcm);
-}
-
-/*
 	Copy data into ALSA capture buffer.
 */
 void line6_capture_copy(struct snd_line6_pcm *line6pcm, char *fbuf, int fsize)
diff --git a/sound/usb/line6/capture.h b/sound/usb/line6/capture.h
index 0939f400a405..3cc71bc70b21 100644
--- a/sound/usb/line6/capture.h
+++ b/sound/usb/line6/capture.h
@@ -26,10 +26,6 @@ extern void line6_capture_check_period(struct snd_line6_pcm *line6pcm,
 extern int line6_create_audio_in_urbs(struct snd_line6_pcm *line6pcm);
 extern void line6_free_capture_buffer(struct snd_line6_pcm *line6pcm);
 extern int line6_submit_audio_in_all_urbs(struct snd_line6_pcm *line6pcm);
-extern void line6_unlink_audio_in_urbs(struct snd_line6_pcm *line6pcm);
-extern void line6_unlink_wait_clear_audio_in_urbs(struct snd_line6_pcm
-						  *line6pcm);
-extern void line6_wait_clear_audio_in_urbs(struct snd_line6_pcm *line6pcm);
 extern int snd_line6_capture_trigger(struct snd_line6_pcm *line6pcm, int cmd);
 
 #endif
diff --git a/sound/usb/line6/pcm.c b/sound/usb/line6/pcm.c
index 738bfd82cecd..677419dcacf9 100644
--- a/sound/usb/line6/pcm.c
+++ b/sound/usb/line6/pcm.c
@@ -90,6 +90,47 @@ static int snd_line6_impulse_period_put(struct snd_kcontrol *kcontrol,
 	return 1;
 }
 
+/*
+	Unlink all currently active URBs.
+*/
+static void line6_unlink_audio_urbs(struct snd_line6_pcm *line6pcm,
+				    struct line6_pcm_stream *pcms)
+{
+	int i;
+
+	for (i = 0; i < LINE6_ISO_BUFFERS; i++) {
+		if (test_bit(i, &pcms->active_urbs)) {
+			if (!test_and_set_bit(i, &pcms->unlink_urbs))
+				usb_unlink_urb(pcms->urbs[i]);
+		}
+	}
+}
+
+/*
+	Wait until unlinking of all currently active URBs has been finished.
+*/
+static void line6_wait_clear_audio_urbs(struct snd_line6_pcm *line6pcm,
+					struct line6_pcm_stream *pcms)
+{
+	int timeout = HZ;
+	int i;
+	int alive;
+
+	do {
+		alive = 0;
+		for (i = 0; i < LINE6_ISO_BUFFERS; i++) {
+			if (test_bit(i, &pcms->active_urbs))
+				alive++;
+		}
+		if (!alive)
+			break;
+		set_current_state(TASK_UNINTERRUPTIBLE);
+		schedule_timeout(1);
+	} while (--timeout > 0);
+	if (alive)
+		snd_printk(KERN_ERR "timeout: still %d active urbs..\n", alive);
+}
+
 static bool test_flags(unsigned long flags0, unsigned long flags1,
 		       unsigned long mask)
 {
@@ -202,18 +243,18 @@ int line6_pcm_release(struct snd_line6_pcm *line6pcm, int channels)
 	} while (cmpxchg(&line6pcm->flags, flags_old, flags_new) != flags_old);
 
 	if (test_flags(flags_new, flags_old, LINE6_BITS_CAPTURE_STREAM))
-		line6_unlink_audio_in_urbs(line6pcm);
+		line6_unlink_audio_urbs(line6pcm, &line6pcm->in);
 
 	if (test_flags(flags_new, flags_old, LINE6_BITS_CAPTURE_BUFFER)) {
-		line6_wait_clear_audio_in_urbs(line6pcm);
+		line6_wait_clear_audio_urbs(line6pcm, &line6pcm->in);
 		line6_free_capture_buffer(line6pcm);
 	}
 
 	if (test_flags(flags_new, flags_old, LINE6_BITS_PLAYBACK_STREAM))
-		line6_unlink_audio_out_urbs(line6pcm);
+		line6_unlink_audio_urbs(line6pcm, &line6pcm->out);
 
 	if (test_flags(flags_new, flags_old, LINE6_BITS_PLAYBACK_BUFFER)) {
-		line6_wait_clear_audio_out_urbs(line6pcm);
+		line6_wait_clear_audio_urbs(line6pcm, &line6pcm->out);
 		line6_free_playback_buffer(line6pcm);
 	}
 
@@ -325,21 +366,24 @@ static struct snd_kcontrol_new line6_controls[] = {
 /*
 	Cleanup the PCM device.
 */
-static void line6_cleanup_pcm(struct snd_pcm *pcm)
+static void cleanup_urbs(struct line6_pcm_stream *pcms)
 {
 	int i;
-	struct snd_line6_pcm *line6pcm = snd_pcm_chip(pcm);
 
 	for (i = 0; i < LINE6_ISO_BUFFERS; i++) {
-		if (line6pcm->out.urbs[i]) {
-			usb_kill_urb(line6pcm->out.urbs[i]);
-			usb_free_urb(line6pcm->out.urbs[i]);
-		}
-		if (line6pcm->in.urbs[i]) {
-			usb_kill_urb(line6pcm->in.urbs[i]);
-			usb_free_urb(line6pcm->in.urbs[i]);
+		if (pcms->urbs[i]) {
+			usb_kill_urb(pcms->urbs[i]);
+			usb_free_urb(pcms->urbs[i]);
 		}
 	}
+}
+
+static void line6_cleanup_pcm(struct snd_pcm *pcm)
+{
+	struct snd_line6_pcm *line6pcm = snd_pcm_chip(pcm);
+
+	cleanup_urbs(&line6pcm->out);
+	cleanup_urbs(&line6pcm->in);
 	kfree(line6pcm);
 }
 
@@ -374,8 +418,10 @@ static int snd_line6_new_pcm(struct usb_line6 *line6, struct snd_pcm **pcm_ret)
 */
 void line6_pcm_disconnect(struct snd_line6_pcm *line6pcm)
 {
-	line6_unlink_wait_clear_audio_out_urbs(line6pcm);
-	line6_unlink_wait_clear_audio_in_urbs(line6pcm);
+	line6_unlink_audio_urbs(line6pcm, &line6pcm->out);
+	line6_unlink_audio_urbs(line6pcm, &line6pcm->in);
+	line6_wait_clear_audio_urbs(line6pcm, &line6pcm->out);
+	line6_wait_clear_audio_urbs(line6pcm, &line6pcm->in);
 }
 
 /*
@@ -451,15 +497,17 @@ int snd_line6_prepare(struct snd_pcm_substream *substream)
 
 	switch (substream->stream) {
 	case SNDRV_PCM_STREAM_PLAYBACK:
-		if ((line6pcm->flags & LINE6_BITS_PLAYBACK_STREAM) == 0)
-			line6_unlink_wait_clear_audio_out_urbs(line6pcm);
-
+		if ((line6pcm->flags & LINE6_BITS_PLAYBACK_STREAM) == 0) {
+			line6_unlink_audio_urbs(line6pcm, &line6pcm->out);
+			line6_wait_clear_audio_urbs(line6pcm, &line6pcm->out);
+		}
 		break;
 
 	case SNDRV_PCM_STREAM_CAPTURE:
-		if ((line6pcm->flags & LINE6_BITS_CAPTURE_STREAM) == 0)
-			line6_unlink_wait_clear_audio_in_urbs(line6pcm);
-
+		if ((line6pcm->flags & LINE6_BITS_CAPTURE_STREAM) == 0) {
+			line6_unlink_audio_urbs(line6pcm, &line6pcm->in);
+			line6_wait_clear_audio_urbs(line6pcm, &line6pcm->in);
+		}
 		break;
 	}
 
diff --git a/sound/usb/line6/playback.c b/sound/usb/line6/playback.c
index d619c1718306..3820ed08b342 100644
--- a/sound/usb/line6/playback.c
+++ b/sound/usb/line6/playback.c
@@ -290,58 +290,6 @@ int line6_submit_audio_out_all_urbs(struct snd_line6_pcm *line6pcm)
 	return 0;
 }
 
-/*
-	Unlink all currently active playback URBs.
-*/
-void line6_unlink_audio_out_urbs(struct snd_line6_pcm *line6pcm)
-{
-	unsigned int i;
-
-	for (i = 0; i < LINE6_ISO_BUFFERS; i++) {
-		if (test_bit(i, &line6pcm->out.active_urbs)) {
-			if (!test_and_set_bit(i, &line6pcm->out.unlink_urbs)) {
-				struct urb *u = line6pcm->out.urbs[i];
-
-				usb_unlink_urb(u);
-			}
-		}
-	}
-}
-
-/*
-	Wait until unlinking of all currently active playback URBs has been
-	finished.
-*/
-void line6_wait_clear_audio_out_urbs(struct snd_line6_pcm *line6pcm)
-{
-	int timeout = HZ;
-	unsigned int i;
-	int alive;
-
-	do {
-		alive = 0;
-		for (i = 0; i < LINE6_ISO_BUFFERS; i++) {
-			if (test_bit(i, &line6pcm->out.active_urbs))
-				alive++;
-		}
-		if (!alive)
-			break;
-		set_current_state(TASK_UNINTERRUPTIBLE);
-		schedule_timeout(1);
-	} while (--timeout > 0);
-	if (alive)
-		snd_printk(KERN_ERR "timeout: still %d active urbs..\n", alive);
-}
-
-/*
-	Unlink all currently active playback URBs, and wait for finishing.
-*/
-void line6_unlink_wait_clear_audio_out_urbs(struct snd_line6_pcm *line6pcm)
-{
-	line6_unlink_audio_out_urbs(line6pcm);
-	line6_wait_clear_audio_out_urbs(line6pcm);
-}
-
 void line6_free_playback_buffer(struct snd_line6_pcm *line6pcm)
 {
 	kfree(line6pcm->out.buffer);
diff --git a/sound/usb/line6/playback.h b/sound/usb/line6/playback.h
index 78a885113221..52a278353d3b 100644
--- a/sound/usb/line6/playback.h
+++ b/sound/usb/line6/playback.h
@@ -32,10 +32,6 @@ extern struct snd_pcm_ops snd_line6_playback_ops;
 extern int line6_create_audio_out_urbs(struct snd_line6_pcm *line6pcm);
 extern void line6_free_playback_buffer(struct snd_line6_pcm *line6pcm);
 extern int line6_submit_audio_out_all_urbs(struct snd_line6_pcm *line6pcm);
-extern void line6_unlink_audio_out_urbs(struct snd_line6_pcm *line6pcm);
-extern void line6_unlink_wait_clear_audio_out_urbs(struct snd_line6_pcm
-						   *line6pcm);
-extern void line6_wait_clear_audio_out_urbs(struct snd_line6_pcm *line6pcm);
 extern int snd_line6_playback_trigger(struct snd_line6_pcm *line6pcm, int cmd);
 
 #endif
-- 
2.2.2

[PATCH 13/16] ALSA: line6: Use dev_err()

[Takashi Iwai]

This is the last remaining snd_printk() usage in this driver.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/usb/line6/pcm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sound/usb/line6/pcm.c b/sound/usb/line6/pcm.c
index 677419dcacf9..4152d92105b1 100644
--- a/sound/usb/line6/pcm.c
+++ b/sound/usb/line6/pcm.c
@@ -128,7 +128,8 @@ static void line6_wait_clear_audio_urbs(struct snd_line6_pcm *line6pcm,
 		schedule_timeout(1);
 	} while (--timeout > 0);
 	if (alive)
-		snd_printk(KERN_ERR "timeout: still %d active urbs..\n", alive);
+		dev_err(line6pcm->line6->ifcdev,
+			"timeout: still %d active urbs..\n", alive);
 }
 
 static bool test_flags(unsigned long flags0, unsigned long flags1,
-- 
2.2.2

[PATCH 14/16] ALSA: line6: Consolidate PCM stream buffer allocation and free

[Takashi Iwai]

The PCM stream buffer allocation and free are identical for both
playback and capture streams.  Provide single helper functions.
These are used only in pcm.c, thus they can be even static.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/usb/line6/capture.c  |  6 ------
 sound/usb/line6/capture.h  |  1 -
 sound/usb/line6/pcm.c      | 53 +++++++++++++++++++++++++---------------------
 sound/usb/line6/playback.c |  6 ------
 sound/usb/line6/playback.h |  1 -
 5 files changed, 29 insertions(+), 38 deletions(-)

diff --git a/sound/usb/line6/capture.c b/sound/usb/line6/capture.c
index 1d477d7a42fb..47cfcc2ab387 100644
--- a/sound/usb/line6/capture.c
+++ b/sound/usb/line6/capture.c
@@ -141,12 +141,6 @@ void line6_capture_check_period(struct snd_line6_pcm *line6pcm, int length)
 	}
 }
 
-void line6_free_capture_buffer(struct snd_line6_pcm *line6pcm)
-{
-	kfree(line6pcm->in.buffer);
-	line6pcm->in.buffer = NULL;
-}
-
 /*
  * Callback for completed capture URB.
  */
diff --git a/sound/usb/line6/capture.h b/sound/usb/line6/capture.h
index 3cc71bc70b21..db062e7200a6 100644
--- a/sound/usb/line6/capture.h
+++ b/sound/usb/line6/capture.h
@@ -24,7 +24,6 @@ extern void line6_capture_copy(struct snd_line6_pcm *line6pcm, char *fbuf,
 extern void line6_capture_check_period(struct snd_line6_pcm *line6pcm,
 				       int length);
 extern int line6_create_audio_in_urbs(struct snd_line6_pcm *line6pcm);
-extern void line6_free_capture_buffer(struct snd_line6_pcm *line6pcm);
 extern int line6_submit_audio_in_all_urbs(struct snd_line6_pcm *line6pcm);
 extern int snd_line6_capture_trigger(struct snd_line6_pcm *line6pcm, int cmd);
 
diff --git a/sound/usb/line6/pcm.c b/sound/usb/line6/pcm.c
index 4152d92105b1..f75825995e24 100644
--- a/sound/usb/line6/pcm.c
+++ b/sound/usb/line6/pcm.c
@@ -132,6 +132,27 @@ static void line6_wait_clear_audio_urbs(struct snd_line6_pcm *line6pcm,
 			"timeout: still %d active urbs..\n", alive);
 }
 
+static int line6_alloc_stream_buffer(struct snd_line6_pcm *line6pcm,
+				     struct line6_pcm_stream *pcms)
+{
+	/* Invoked multiple times in a row so allocate once only */
+	if (pcms->buffer)
+		return 0;
+
+	pcms->buffer = kmalloc(LINE6_ISO_BUFFERS * LINE6_ISO_PACKETS *
+			       line6pcm->max_packet_size, GFP_KERNEL);
+	if (!pcms->buffer)
+		return -ENOMEM;
+	return 0;
+}
+
+static void line6_free_stream_buffer(struct snd_line6_pcm *line6pcm,
+				     struct line6_pcm_stream *pcms)
+{
+	kfree(pcms->buffer);
+	pcms->buffer = NULL;
+}
+
 static bool test_flags(unsigned long flags0, unsigned long flags1,
 		       unsigned long mask)
 {
@@ -153,17 +174,9 @@ int line6_pcm_acquire(struct snd_line6_pcm *line6pcm, int channels)
 	line6pcm->prev_fbuf = NULL;
 
 	if (test_flags(flags_old, flags_new, LINE6_BITS_CAPTURE_BUFFER)) {
-		/* Invoked multiple times in a row so allocate once only */
-		if (!line6pcm->in.buffer) {
-			line6pcm->in.buffer =
-				kmalloc(LINE6_ISO_BUFFERS * LINE6_ISO_PACKETS *
-					line6pcm->max_packet_size, GFP_KERNEL);
-			if (!line6pcm->in.buffer) {
-				err = -ENOMEM;
-				goto pcm_acquire_error;
-			}
-		}
-
+		err = line6_alloc_stream_buffer(line6pcm, &line6pcm->in);
+		if (err < 0)
+			goto pcm_acquire_error;
 		flags_final |= channels & LINE6_BITS_CAPTURE_BUFFER;
 	}
 
@@ -190,17 +203,9 @@ int line6_pcm_acquire(struct snd_line6_pcm *line6pcm, int channels)
 	}
 
 	if (test_flags(flags_old, flags_new, LINE6_BITS_PLAYBACK_BUFFER)) {
-		/* Invoked multiple times in a row so allocate once only */
-		if (!line6pcm->out.buffer) {
-			line6pcm->out.buffer =
-				kmalloc(LINE6_ISO_BUFFERS * LINE6_ISO_PACKETS *
-					line6pcm->max_packet_size, GFP_KERNEL);
-			if (!line6pcm->out.buffer) {
-				err = -ENOMEM;
-				goto pcm_acquire_error;
-			}
-		}
-
+		err = line6_alloc_stream_buffer(line6pcm, &line6pcm->out);
+		if (err < 0)
+			goto pcm_acquire_error;
 		flags_final |= channels & LINE6_BITS_PLAYBACK_BUFFER;
 	}
 
@@ -248,7 +253,7 @@ int line6_pcm_release(struct snd_line6_pcm *line6pcm, int channels)
 
 	if (test_flags(flags_new, flags_old, LINE6_BITS_CAPTURE_BUFFER)) {
 		line6_wait_clear_audio_urbs(line6pcm, &line6pcm->in);
-		line6_free_capture_buffer(line6pcm);
+		line6_free_stream_buffer(line6pcm, &line6pcm->in);
 	}
 
 	if (test_flags(flags_new, flags_old, LINE6_BITS_PLAYBACK_STREAM))
@@ -256,7 +261,7 @@ int line6_pcm_release(struct snd_line6_pcm *line6pcm, int channels)
 
 	if (test_flags(flags_new, flags_old, LINE6_BITS_PLAYBACK_BUFFER)) {
 		line6_wait_clear_audio_urbs(line6pcm, &line6pcm->out);
-		line6_free_playback_buffer(line6pcm);
+		line6_free_stream_buffer(line6pcm, &line6pcm->out);
 	}
 
 	return 0;
diff --git a/sound/usb/line6/playback.c b/sound/usb/line6/playback.c
index 3820ed08b342..750d91dced57 100644
--- a/sound/usb/line6/playback.c
+++ b/sound/usb/line6/playback.c
@@ -290,12 +290,6 @@ int line6_submit_audio_out_all_urbs(struct snd_line6_pcm *line6pcm)
 	return 0;
 }
 
-void line6_free_playback_buffer(struct snd_line6_pcm *line6pcm)
-{
-	kfree(line6pcm->out.buffer);
-	line6pcm->out.buffer = NULL;
-}
-
 /*
 	Callback for completed playback URB.
 */
diff --git a/sound/usb/line6/playback.h b/sound/usb/line6/playback.h
index 52a278353d3b..f6a9e18f87b6 100644
--- a/sound/usb/line6/playback.h
+++ b/sound/usb/line6/playback.h
@@ -30,7 +30,6 @@
 extern struct snd_pcm_ops snd_line6_playback_ops;
 
 extern int line6_create_audio_out_urbs(struct snd_line6_pcm *line6pcm);
-extern void line6_free_playback_buffer(struct snd_line6_pcm *line6pcm);
 extern int line6_submit_audio_out_all_urbs(struct snd_line6_pcm *line6pcm);
 extern int snd_line6_playback_trigger(struct snd_line6_pcm *line6pcm, int cmd);
 
-- 
2.2.2

[PATCH 15/16] ALSA: line6: Do clipping in volume / monitor manipulations

[Takashi Iwai]

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/usb/line6/playback.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/sound/usb/line6/playback.c b/sound/usb/line6/playback.c
index 750d91dced57..cbcd97f5d629 100644
--- a/sound/usb/line6/playback.c
+++ b/sound/usb/line6/playback.c
@@ -37,7 +37,8 @@ static void change_volume(struct urb *urb_out, int volume[],
 		buf_end = p + urb_out->transfer_buffer_length / sizeof(*p);
 
 		for (; p < buf_end; ++p) {
-			*p = (*p * volume[chn & 1]) >> 8;
+			int val = (*p * volume[chn & 1]) >> 8;
+			*p = clamp(val, 0x7fff, -0x8000);
 			++chn;
 		}
 	} else if (bytes_per_frame == 6) {
@@ -51,6 +52,7 @@ static void change_volume(struct urb *urb_out, int volume[],
 
 			val = p[0] + (p[1] << 8) + ((signed char)p[2] << 16);
 			val = (val * volume[chn & 1]) >> 8;
+			val = clamp(val, 0x7fffff, -0x800000);
 			p[0] = val;
 			p[1] = val >> 8;
 			p[2] = val >> 16;
@@ -118,8 +120,10 @@ static void add_monitor_signal(struct urb *urb_out, unsigned char *signal,
 		po = (short *)urb_out->transfer_buffer;
 		buf_end = po + urb_out->transfer_buffer_length / sizeof(*po);
 
-		for (; po < buf_end; ++pi, ++po)
-			*po += (*pi * volume) >> 8;
+		for (; po < buf_end; ++pi, ++po) {
+			int val = *po + ((*pi * volume) >> 8);
+			*po = clamp(val, 0x7fff, -0x8000);
+		}
 	}
 
 	/*
-- 
2.2.2

[PATCH 16/16] ALSA: line6: Skip volume manipulation during silence copying

[Takashi Iwai]

A minor optimization; while pausing, the driver just copies the zero
that doesn't need any volume changes.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/usb/line6/playback.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/sound/usb/line6/playback.c b/sound/usb/line6/playback.c
index cbcd97f5d629..3762a98026aa 100644
--- a/sound/usb/line6/playback.c
+++ b/sound/usb/line6/playback.c
@@ -234,13 +234,14 @@ static int submit_audio_out_urb(struct snd_line6_pcm *line6pcm)
 		line6pcm->out.pos += urb_frames;
 		if (line6pcm->out.pos >= runtime->buffer_size)
 			line6pcm->out.pos -= runtime->buffer_size;
+
+		change_volume(urb_out, line6pcm->volume_playback,
+			      bytes_per_frame);
 	} else {
 		memset(urb_out->transfer_buffer, 0,
 		       urb_out->transfer_buffer_length);
 	}
 
-	change_volume(urb_out, line6pcm->volume_playback, bytes_per_frame);
-
 	if (line6pcm->prev_fbuf != NULL) {
 		if (line6pcm->flags & LINE6_BITS_PCM_IMPULSE) {
 			create_impulse_test_signal(line6pcm, urb_out,
-- 
2.2.2

Re: [PATCH 02/16] ALSA: line6: Fix memory leak at probe error path

[Chris Rorvick]

On Fri, Jan 23, 2015 at 11:13 AM, Takashi Iwai <tiwai@suse.de> wrote:
> --- a/sound/usb/line6/driver.c
> +++ b/sound/usb/line6/driver.c
> @@ -507,10 +507,32 @@ int line6_probe(struct usb_interface *interface,
>         int interface_number;
>         int ret;
>
> +       ret = snd_card_new(line6->ifcdev,
> +                          SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1,
> +                          THIS_MODULE, 0, &card);

The `ifcdev' member has not been initialized yet.  You need something
like the following squashed in:

>8------------------------------------------------------8<
diff --git a/sound/usb/line6/driver.c b/sound/usb/line6/driver.c
index 2eed6fb..25d6b0f 100644
--- a/sound/usb/line6/driver.c
+++ b/sound/usb/line6/driver.c
@@ -507,6 +507,11 @@ int line6_probe(struct usb_interface *interface,
        int interface_number;
        int ret;

+       /* store basic data: */
+       line6->properties = properties;
+       line6->usbdev = usbdev;
+       line6->ifcdev = &interface->dev;
+
        ret = snd_card_new(line6->ifcdev,
                           SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1,
                           THIS_MODULE, 0, &card);
@@ -548,11 +553,6 @@ int line6_probe(struct usb_interface *interface,
                goto error;
        }

-       /* store basic data: */
-       line6->properties = properties;
-       line6->usbdev = usbdev;
-       line6->ifcdev = &interface->dev;
-
        line6_get_interval(line6);

        if (properties->capabilities & LINE6_CAP_CONTROL) {

Re: [PATCH 02/16] ALSA: line6: Fix memory leak at probe error path

[Takashi Iwai]

At Sun, 25 Jan 2015 02:06:19 -0600,
Chris Rorvick wrote:
> 
> On Fri, Jan 23, 2015 at 11:13 AM, Takashi Iwai <tiwai@suse.de> wrote:
> > --- a/sound/usb/line6/driver.c
> > +++ b/sound/usb/line6/driver.c
> > @@ -507,10 +507,32 @@ int line6_probe(struct usb_interface *interface,
> >         int interface_number;
> >         int ret;
> >
> > +       ret = snd_card_new(line6->ifcdev,
> > +                          SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1,
> > +                          THIS_MODULE, 0, &card);
> 
> The `ifcdev' member has not been initialized yet.

Good catch.

> You need something
> like the following squashed in:
> 
> >8------------------------------------------------------8<
> diff --git a/sound/usb/line6/driver.c b/sound/usb/line6/driver.c
> index 2eed6fb..25d6b0f 100644
> --- a/sound/usb/line6/driver.c
> +++ b/sound/usb/line6/driver.c
> @@ -507,6 +507,11 @@ int line6_probe(struct usb_interface *interface,
>         int interface_number;
>         int ret;
> 
> +       /* store basic data: */
> +       line6->properties = properties;
> +       line6->usbdev = usbdev;
> +       line6->ifcdev = &interface->dev;
> +
>         ret = snd_card_new(line6->ifcdev,
>                            SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1,
>                            THIS_MODULE, 0, &card);
> @@ -548,11 +553,6 @@ int line6_probe(struct usb_interface *interface,
>                 goto error;
>         }
> 
> -       /* store basic data: */
> -       line6->properties = properties;
> -       line6->usbdev = usbdev;
> -       line6->ifcdev = &interface->dev;
> -
>         line6_get_interval(line6);
> 
>         if (properties->capabilities & LINE6_CAP_CONTROL) {

Right, but there will be another round of cleanup patches and the
allocation of line6 will be dropped there, so I took an easier fixup,
just use &interface->dev to the argument.

The revised patch is found below.  The test/line6 branch was updated
with this (and more fixes I'll send shortly later).


thanks,

Takashi

-- 8< --
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] ALSA: line6: Fix memory leak at probe error path

Fix memory leak at probe error path by rearranging the call order in
line6_destruct() so that the common destructor is always called.
Also this simplifies the error path to a single goto label.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/usb/line6/driver.c | 59 ++++++++++++++++++++++++------------------------
 1 file changed, 30 insertions(+), 29 deletions(-)

diff --git a/sound/usb/line6/driver.c b/sound/usb/line6/driver.c
index b783c0788e45..bf9630cd2395 100644
--- a/sound/usb/line6/driver.c
+++ b/sound/usb/line6/driver.c
@@ -507,39 +507,20 @@ int line6_probe(struct usb_interface *interface,
 	int interface_number;
 	int ret;
 
-	/* we don't handle multiple configurations */
-	if (usbdev->descriptor.bNumConfigurations != 1) {
-		ret = -ENODEV;
-		goto err_put;
-	}
-
-	/* initialize device info: */
-	dev_info(&interface->dev, "Line 6 %s found\n", properties->name);
-
-	/* query interface number */
-	interface_number = interface->cur_altsetting->desc.bInterfaceNumber;
-
-	ret = usb_set_interface(usbdev, interface_number,
-			properties->altsetting);
+	ret = snd_card_new(&interface->dev,
+			   SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1,
+			   THIS_MODULE, 0, &card);
 	if (ret < 0) {
-		dev_err(&interface->dev, "set_interface failed\n");
-		goto err_put;
+		kfree(line6);
+		return ret;
 	}
 
 	/* store basic data: */
+	line6->card = card;
 	line6->properties = properties;
 	line6->usbdev = usbdev;
 	line6->ifcdev = &interface->dev;
 
-	line6_get_interval(line6);
-
-	ret = snd_card_new(line6->ifcdev,
-			   SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1,
-			   THIS_MODULE, 0, &card);
-	if (ret < 0)
-		goto err_put;
-
-	line6->card = card;
 	strcpy(card->id, line6->properties->id);
 	strcpy(card->driver, DRIVER_NAME);
 	strcpy(card->shortname, line6->properties->name);
@@ -553,16 +534,37 @@ 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);
+
+	/* query interface number */
+	interface_number = interface->cur_altsetting->desc.bInterfaceNumber;
+
+	ret = usb_set_interface(usbdev, interface_number,
+			properties->altsetting);
+	if (ret < 0) {
+		dev_err(&interface->dev, "set_interface failed\n");
+		goto error;
+	}
+
+	line6_get_interval(line6);
+
 	if (properties->capabilities & LINE6_CAP_CONTROL) {
 		ret = line6_init_cap_control(line6);
 		if (ret < 0)
-			goto err_destruct;
+			goto error;
 	}
 
 	/* initialize device data based on device: */
 	ret = private_init(interface, line6);
 	if (ret < 0)
-		goto err_destruct;
+		goto error;
 
 	/* creation of additional special files should go here */
 
@@ -571,11 +573,10 @@ int line6_probe(struct usb_interface *interface,
 
 	return 0;
 
- err_destruct:
+ error:
 	if (line6->disconnect)
 		line6->disconnect(interface);
 	snd_card_free(card);
- err_put:
 	return ret;
 }
 EXPORT_SYMBOL_GPL(line6_probe);
-- 
2.2.2

Re: [PATCH 10/16] ALSA: line6: Drop voodoo workarounds

[Chris Rorvick]

On Fri, Jan 23, 2015 at 11:13 AM, Takashi Iwai <tiwai@suse.de> wrote:
> If the problem still really remains, we should fix it instead of
> papering over it like this...

FWIW, the commit from the original repository was made in 2007.  So if
the bug is still there, it's an old one.

commit 1e32d598dcf290e5c131a58231c13bfd25812810
Author: fdemski <fdemski@87a33c3d-1c25-0410-a0e4-a977fe0255b3>
Date:   Mon Jul 16 23:51:55 2007 +0000

    bug found and fixed -- maybe some alsa specifics -- did not crosscheck
    substream->private_data seems not to be filled in hw_param function

Re: [PATCH 08/16] ALSA: line6: Drop superfluous spinlock for trigger

[Chris Rorvick]

On Fri, Jan 23, 2015 at 11:13 AM, Takashi Iwai <tiwai@suse.de> wrote:
> The trigger callback is already spinlocked, so we need no more lock
> here (even for the linked substreams).  Let's drop it.

Are they linked?  It doesn't look like they are for my TonePort UX2.
I assumed this is why the trigger lock existed, to synchronize the
trigger callbacks between playback and capture because the group lock
was *not* being acquired.

And the purpose of this synchronization I assumed was to really
synchronize the calls to line6_pcm_acquire/release().  But then
hw_params() and hw_free() are calling into these seemingly without any
lock.

And line6_pcm_acquire/release() are updating line6pcm->flags
atomically, but elsewhere we're just using test_and_set_bit() and
such.

All of this looks racy to me.  Am I missing something?

Finally, what is the point of dispatching through this rather than to
the respective playback/capture callback directly?  Would this come
into play if I was seeing linked substreams?

Sorry, this is way beyond the scope of this patch.  But I've been
spending some time trying to sort this out and haven't answered the
above questions.  If I'm missing anything, a pointer to the relevant
source would be greatly appreciated!

Regards,

Chris

Re: [PATCH 08/16] ALSA: line6: Drop superfluous spinlock for trigger

[Takashi Iwai]

At Tue, 27 Jan 2015 00:22:48 -0600,
Chris Rorvick wrote:
> 
> On Fri, Jan 23, 2015 at 11:13 AM, Takashi Iwai <tiwai@suse.de> wrote:
> > The trigger callback is already spinlocked, so we need no more lock
> > here (even for the linked substreams).  Let's drop it.
> 
> Are they linked?  It doesn't look like they are for my TonePort UX2.

They can be linked when specified by application via snd_pcm_link().
It's not always linked.

> I assumed this is why the trigger lock existed, to synchronize the
> trigger callbacks between playback and capture because the group lock
> was *not* being acquired.

For the trigger part, both LINE6_BITS_PLAYBACK_STREAM and
LINE6_BITS_CAPTURE_STREAM can be dealt really independently.  Thus
there is no reason to protect the parallel triggers for both
directions.

And, a stream lock is always held for the trigger callback (in
addition to a group lock if linked).

> And the purpose of this synchronization I assumed was to really
> synchronize the calls to line6_pcm_acquire/release().  But then
> hw_params() and hw_free() are calling into these seemingly without any
> lock.

Right.  And, reading the code more carefully, you'll find that a lock
can't help there at all.  The current line6_pcm_acquire() and
line6_pcm_release() are already racy.  At the beginning of each
function, it tries to be consistent about the bit flags.  But that's
all.  It sets to the new bits there, and checks the diff between the
old and the new.  This doesn't protect against  the renewal of the
same bit while the bit is being handled.

For example, suppose a task A calls line6_pcm_release(*_BUFFER).  It
clears the *_BUFFER bits and then tries to free a buffer via kfree().
During it, suppose a task B calls line6_pcm_acquire() with the same
*_BUFFER flag.  The bits was already cleared, so it sets again and
processes the *_BUFFER part, where it allocates a buffer via
kmalloc().  If the task B is executed before kfree() call of task A,
kmalloc() may be performed and replace line6pcm->buffer_out.  Then
task B calls kfree() with the newly allocated one.  Ouch.

> And line6_pcm_acquire/release() are updating line6pcm->flags
> atomically, but elsewhere we're just using test_and_set_bit() and
> such.
>
> All of this looks racy to me.  Am I missing something?

Yes, the code *is* already racy :)
And the lock doesn't anything there.  It protects the parallel call of
playback and capture directions, but these parallel triggers are
actually no problem, so it just gives the unnecessary protection.

> Finally, what is the point of dispatching through this rather than to
> the respective playback/capture callback directly?  Would this come
> into play if I was seeing linked substreams?

I can only guess: the reason is likely to simplify the call for
IMPULSE or MONITOR cases.  In the current form, they can be a simple
line6_pcm_acquire() or _release() call with the combined bit flags.
If the handling of streams are separated, it'll need multiple calls.

I don't mean that the current form is good, though.  Rather I find
it makes thing unnecessarily too complex, and as already mentioned,
it's racy, doesn't work as expected.  So I think we should straighten
the code later, in anyway.

> Sorry, this is way beyond the scope of this patch.  But I've been
> spending some time trying to sort this out and haven't answered the
> above questions.  If I'm missing anything, a pointer to the relevant
> source would be greatly appreciated!

HTH,

Takashi

Re: [PATCH 08/16] ALSA: line6: Drop superfluous spinlock for trigger

[Chris Rorvick]

On Tue, Jan 27, 2015 at 4:27 AM, Takashi Iwai <tiwai@suse.de> wrote:
> At Tue, 27 Jan 2015 00:22:48 -0600, Chris Rorvick wrote:
>> All of this looks racy to me.  Am I missing something?
>
> Yes, the code *is* already racy :)

OK, good to know I'm not seeing ghosts.  :-)

>> Sorry, this is way beyond the scope of this patch.  But I've been
>> spending some time trying to sort this out and haven't answered the
>> above questions.  If I'm missing anything, a pointer to the relevant
>> source would be greatly appreciated!
>
> HTH,

Yes, very much.  Thanks!

Regards,

Chris

Re: [PATCH 08/16] ALSA: line6: Drop superfluous spinlock for trigger

[Takashi Iwai]

At Tue, 27 Jan 2015 06:48:22 -0600,
Chris Rorvick wrote:
> 
> On Tue, Jan 27, 2015 at 4:27 AM, Takashi Iwai <tiwai@suse.de> wrote:
> > At Tue, 27 Jan 2015 00:22:48 -0600, Chris Rorvick wrote:
> >> All of this looks racy to me.  Am I missing something?
> >
> > Yes, the code *is* already racy :)
> 
> OK, good to know I'm not seeing ghosts.  :-)

Hehe.

I wrote a few more patches to cover the races in a bit better way
(although it's not perfect yet).  I'm going to send to ML soon.

Could you give tested-by tag if the latest test/line6 branch works --
at least, no serious regression is seen?


thanks,

Takashi

Re: [PATCH 08/16] ALSA: line6: Drop superfluous spinlock for trigger

[Chris Rorvick]

On Tue, Jan 27, 2015 at 10:10 AM, Takashi Iwai <tiwai@suse.de> wrote:
> Could you give tested-by tag if the latest test/line6 branch works --
> at least, no serious regression is seen?

The "Fix racy loopback handling" is broken due to the typo, but that
drops out later and does not affect the final tree.  So, very briefly
...

Tested-by: Chris Rorvick <chris@rorvick.com>

Re: [PATCH 08/16] ALSA: line6: Drop superfluous spinlock for trigger

[Takashi Iwai]

At Wed, 28 Jan 2015 00:03:17 -0600,
Chris Rorvick wrote:
> 
> On Tue, Jan 27, 2015 at 10:10 AM, Takashi Iwai <tiwai@suse.de> wrote:
> > Could you give tested-by tag if the latest test/line6 branch works --
> > at least, no serious regression is seen?
> 
> The "Fix racy loopback handling" is broken due to the typo, but that
> drops out later and does not affect the final tree.  So, very briefly
> ...
> 
> Tested-by: Chris Rorvick <chris@rorvick.com>

Thanks!  I updated the branch and now merged to for-next.


Takashi

Re: [PATCH 08/16] ALSA: line6: Drop superfluous spinlock for trigger

[Chris Rorvick]

On Wed, Jan 28, 2015 at 12:26 AM, Takashi Iwai <tiwai@suse.de> wrote:
> At Wed, 28 Jan 2015 00:03:17 -0600,
> Chris Rorvick wrote:
>>
>> On Tue, Jan 27, 2015 at 10:10 AM, Takashi Iwai <tiwai@suse.de> wrote:
>> > Could you give tested-by tag if the latest test/line6 branch works --
>> > at least, no serious regression is seen?
>>
>> The "Fix racy loopback handling" is broken due to the typo, but that
>> drops out later and does not affect the final tree.  So, very briefly
>> ...
>>
>> Tested-by: Chris Rorvick <chris@rorvick.com>
>
> Thanks!  I updated the branch and now merged to for-next.

You're welcome, looks good!  Nice to see the playback/capture
buffer/stream flags stuff cleaned up.

Chris