usb-audio: some cleanup patches for 2.6.36

usb-audio: some cleanup patches for 2.6.36

[Daniel Mack]

Here are five patches that mostly do some basic cleanups. They're
unurgent and should be queued for 2.6.36.

I'm still pondering about the clock topology and how to fully support
all corner cases. That might take awhile to sort out.

Daniel

[PATCH 1/5] ALSA: usb-audio: clean up includes in clock.c
[PATCH 2/5] ALSA: usb-audio: unify UAC macros and struct names
[PATCH 3/5] ALSA: usb-midi: whitespace fixes
[PATCH 4/5] ALSA: usb-audio: move and add some comments
[PATCH 5/5] ALSA: usb-audio: simplify control interface access

[PATCH 1/5] ALSA: usb-audio: clean up includes in clock.c

[Daniel Mack]

Signed-off-by: Daniel Mack <daniel@caiaq.de>
---
 sound/usb/clock.c |   16 +---------------
 1 files changed, 1 insertions(+), 15 deletions(-)

diff --git a/sound/usb/clock.c b/sound/usb/clock.c
index b585511..386b09c 100644
--- a/sound/usb/clock.c
+++ b/sound/usb/clock.c
@@ -19,33 +19,19 @@
 
 #include <linux/bitops.h>
 #include <linux/init.h>
-#include <linux/list.h>
-#include <linux/slab.h>
 #include <linux/string.h>
 #include <linux/usb.h>
-#include <linux/moduleparam.h>
-#include <linux/mutex.h>
 #include <linux/usb/audio.h>
 #include <linux/usb/audio-v2.h>
 
 #include <sound/core.h>
 #include <sound/info.h>
 #include <sound/pcm.h>
-#include <sound/pcm_params.h>
-#include <sound/initval.h>
 
 #include "usbaudio.h"
 #include "card.h"
-#include "midi.h"
-#include "mixer.h"
-#include "proc.h"
-#include "quirks.h"
-#include "endpoint.h"
 #include "helper.h"
-#include "debug.h"
-#include "pcm.h"
-#include "urb.h"
-#include "format.h"
+#include "clock.h"
 
 static struct uac_clock_source_descriptor *
 	snd_usb_find_clock_source(struct usb_host_interface *ctrl_iface,
-- 
1.7.1

[PATCH 2/5] ALSA: usb-audio: unify UAC macros and struct names

[Daniel Mack]

Get rid of the last occurances of _v1 suffixes, and move the version
number right after the "uac" string. Now things are consitent again.

Sorry for the forth and back, but it just looks much nicer this way.

Signed-off-by: Daniel Mack <daniel@caiaq.de>
---
 drivers/usb/gadget/f_audio.c |    6 +++---
 drivers/usb/gadget/gmidi.c   |    2 +-
 include/linux/usb/audio-v2.h |    2 +-
 include/linux/usb/audio.h    |   12 ++++++------
 sound/usb/card.c             |    2 +-
 sound/usb/endpoint.c         |    4 ++--
 sound/usb/mixer.c            |   14 +++++++-------
 7 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/drivers/usb/gadget/f_audio.c b/drivers/usb/gadget/f_audio.c
index b91115f..1f48ceb 100644
--- a/drivers/usb/gadget/f_audio.c
+++ b/drivers/usb/gadget/f_audio.c
@@ -61,7 +61,7 @@ DECLARE_UAC_AC_HEADER_DESCRIPTOR(2);
 #define UAC_DT_TOTAL_LENGTH (UAC_DT_AC_HEADER_LENGTH + UAC_DT_INPUT_TERMINAL_SIZE \
 	+ UAC_DT_OUTPUT_TERMINAL_SIZE + UAC_DT_FEATURE_UNIT_SIZE(0))
 /* B.3.2  Class-Specific AC Interface Descriptor */
-static struct uac_ac_header_descriptor_v1_2 ac_header_desc = {
+static struct uac1_ac_header_descriptor_2 ac_header_desc = {
 	.bLength =		UAC_DT_AC_HEADER_LENGTH,
 	.bDescriptorType =	USB_DT_CS_INTERFACE,
 	.bDescriptorSubtype =	UAC_HEADER,
@@ -125,7 +125,7 @@ static struct usb_audio_control_selector feature_unit = {
 };
 
 #define OUTPUT_TERMINAL_ID	3
-static struct uac_output_terminal_descriptor_v1 output_terminal_desc = {
+static struct uac1_output_terminal_descriptor output_terminal_desc = {
 	.bLength		= UAC_DT_OUTPUT_TERMINAL_SIZE,
 	.bDescriptorType	= USB_DT_CS_INTERFACE,
 	.bDescriptorSubtype	= UAC_OUTPUT_TERMINAL,
@@ -155,7 +155,7 @@ static struct usb_interface_descriptor as_interface_alt_1_desc = {
 };
 
 /* B.4.2  Class-Specific AS Interface Descriptor */
-static struct uac_as_header_descriptor_v1 as_header_desc = {
+static struct uac1_as_header_descriptor as_header_desc = {
 	.bLength =		UAC_DT_AS_HEADER_SIZE,
 	.bDescriptorType =	USB_DT_CS_INTERFACE,
 	.bDescriptorSubtype =	UAC_AS_GENERAL,
diff --git a/drivers/usb/gadget/gmidi.c b/drivers/usb/gadget/gmidi.c
index 2b56ce6..b7bf880 100644
--- a/drivers/usb/gadget/gmidi.c
+++ b/drivers/usb/gadget/gmidi.c
@@ -238,7 +238,7 @@ static const struct usb_interface_descriptor ac_interface_desc = {
 };
 
 /* B.3.2  Class-Specific AC Interface Descriptor */
-static const struct uac_ac_header_descriptor_v1_1 ac_header_desc = {
+static const struct uac1_ac_header_descriptor_1 ac_header_desc = {
 	.bLength =		UAC_DT_AC_HEADER_SIZE(1),
 	.bDescriptorType =	USB_DT_CS_INTERFACE,
 	.bDescriptorSubtype =	USB_MS_HEADER,
diff --git a/include/linux/usb/audio-v2.h b/include/linux/usb/audio-v2.h
index 383b94b..716aebe 100644
--- a/include/linux/usb/audio-v2.h
+++ b/include/linux/usb/audio-v2.h
@@ -121,7 +121,7 @@ struct uac2_feature_unit_descriptor {
 
 /* 4.9.2 Class-Specific AS Interface Descriptor */
 
-struct uac_as_header_descriptor_v2 {
+struct uac2_as_header_descriptor {
 	__u8 bLength;
 	__u8 bDescriptorType;
 	__u8 bDescriptorSubtype;
diff --git a/include/linux/usb/audio.h b/include/linux/usb/audio.h
index c51200c..a54b825 100644
--- a/include/linux/usb/audio.h
+++ b/include/linux/usb/audio.h
@@ -39,8 +39,8 @@
 #define UAC_MIXER_UNIT			0x04
 #define UAC_SELECTOR_UNIT		0x05
 #define UAC_FEATURE_UNIT		0x06
-#define UAC_PROCESSING_UNIT_V1		0x07
-#define UAC_EXTENSION_UNIT_V1		0x08
+#define UAC1_PROCESSING_UNIT		0x07
+#define UAC1_EXTENSION_UNIT		0x08
 
 /* A.6 Audio Class-Specific AS Interface Descriptor Subtypes */
 #define UAC_AS_GENERAL			0x01
@@ -151,7 +151,7 @@
 
 /* Terminal Control Selectors */
 /* 4.3.2  Class-Specific AC Interface Descriptor */
-struct uac_ac_header_descriptor_v1 {
+struct uac1_ac_header_descriptor {
 	__u8  bLength;			/* 8 + n */
 	__u8  bDescriptorType;		/* USB_DT_CS_INTERFACE */
 	__u8  bDescriptorSubtype;	/* UAC_MS_HEADER */
@@ -165,7 +165,7 @@ struct uac_ac_header_descriptor_v1 {
 
 /* As above, but more useful for defining your own descriptors: */
 #define DECLARE_UAC_AC_HEADER_DESCRIPTOR(n)			\
-struct uac_ac_header_descriptor_v1_##n {			\
+struct uac1_ac_header_descriptor_##n {			\
 	__u8  bLength;						\
 	__u8  bDescriptorType;					\
 	__u8  bDescriptorSubtype;				\
@@ -205,7 +205,7 @@ struct uac_input_terminal_descriptor {
 #define UAC_TERMINAL_CS_COPY_PROTECT_CONTROL		0x01
 
 /* 4.3.2.2 Output Terminal Descriptor */
-struct uac_output_terminal_descriptor_v1 {
+struct uac1_output_terminal_descriptor {
 	__u8  bLength;			/* in bytes: 9 */
 	__u8  bDescriptorType;		/* CS_INTERFACE descriptor type */
 	__u8  bDescriptorSubtype;	/* OUTPUT_TERMINAL descriptor subtype */
@@ -395,7 +395,7 @@ static inline __u8 *uac_processing_unit_specific(struct uac_processing_unit_desc
 }
 
 /* 4.5.2 Class-Specific AS Interface Descriptor */
-struct uac_as_header_descriptor_v1 {
+struct uac1_as_header_descriptor {
 	__u8  bLength;			/* in bytes: 7 */
 	__u8  bDescriptorType;		/* USB_DT_CS_INTERFACE */
 	__u8  bDescriptorSubtype;	/* AS_GENERAL */
diff --git a/sound/usb/card.c b/sound/usb/card.c
index 7a8ac1d..9feb00c 100644
--- a/sound/usb/card.c
+++ b/sound/usb/card.c
@@ -217,7 +217,7 @@ static int snd_usb_create_streams(struct snd_usb_audio *chip, int ctrlif)
 
 	switch (protocol) {
 	case UAC_VERSION_1: {
-		struct uac_ac_header_descriptor_v1 *h1 = control_header;
+		struct uac1_ac_header_descriptor *h1 = control_header;
 
 		if (!h1->bInCollection) {
 			snd_printk(KERN_INFO "skipping empty audio interface (v1)\n");
diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
index 9593b91..289a165 100644
--- a/sound/usb/endpoint.c
+++ b/sound/usb/endpoint.c
@@ -275,7 +275,7 @@ int snd_usb_parse_audio_endpoints(struct snd_usb_audio *chip, int iface_no)
 		/* get audio formats */
 		switch (protocol) {
 		case UAC_VERSION_1: {
-			struct uac_as_header_descriptor_v1 *as =
+			struct uac1_as_header_descriptor *as =
 				snd_usb_find_csint_desc(alts->extra, alts->extralen, NULL, UAC_AS_GENERAL);
 
 			if (!as) {
@@ -297,7 +297,7 @@ int snd_usb_parse_audio_endpoints(struct snd_usb_audio *chip, int iface_no)
 		case UAC_VERSION_2: {
 			struct uac2_input_terminal_descriptor *input_term;
 			struct uac2_output_terminal_descriptor *output_term;
-			struct uac_as_header_descriptor_v2 *as =
+			struct uac2_as_header_descriptor *as =
 				snd_usb_find_csint_desc(alts->extra, alts->extralen, NULL, UAC_AS_GENERAL);
 
 			if (!as) {
diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index 6939d0f..b6a261f 100644
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -582,9 +582,9 @@ static int get_term_name(struct mixer_build *state, struct usb_audio_term *iterm
 		switch (iterm->type >> 16) {
 		case UAC_SELECTOR_UNIT:
 			strcpy(name, "Selector"); return 8;
-		case UAC_PROCESSING_UNIT_V1:
+		case UAC1_PROCESSING_UNIT:
 			strcpy(name, "Process Unit"); return 12;
-		case UAC_EXTENSION_UNIT_V1:
+		case UAC1_EXTENSION_UNIT:
 			strcpy(name, "Ext Unit"); return 8;
 		case UAC_MIXER_UNIT:
 			strcpy(name, "Mixer"); return 5;
@@ -672,8 +672,8 @@ static int check_input_term(struct mixer_build *state, int id, struct usb_audio_
 			term->name = uac_selector_unit_iSelector(d);
 			return 0;
 		}
-		case UAC_PROCESSING_UNIT_V1:
-		case UAC_EXTENSION_UNIT_V1: {
+		case UAC1_PROCESSING_UNIT:
+		case UAC1_EXTENSION_UNIT: {
 			struct uac_processing_unit_descriptor *d = p1;
 			if (d->bNrInPins) {
 				id = d->baSourceID[0];
@@ -1842,13 +1842,13 @@ static int parse_audio_unit(struct mixer_build *state, int unitid)
 		return parse_audio_selector_unit(state, unitid, p1);
 	case UAC_FEATURE_UNIT:
 		return parse_audio_feature_unit(state, unitid, p1);
-	case UAC_PROCESSING_UNIT_V1:
+	case UAC1_PROCESSING_UNIT:
 	/*   UAC2_EFFECT_UNIT has the same value */
 		if (state->mixer->protocol == UAC_VERSION_1)
 			return parse_audio_processing_unit(state, unitid, p1);
 		else
 			return 0; /* FIXME - effect units not implemented yet */
-	case UAC_EXTENSION_UNIT_V1:
+	case UAC1_EXTENSION_UNIT:
 	/*   UAC2_PROCESSING_UNIT_V2 has the same value */
 		if (state->mixer->protocol == UAC_VERSION_1)
 			return parse_audio_extension_unit(state, unitid, p1);
@@ -1912,7 +1912,7 @@ static int snd_usb_mixer_controls(struct usb_mixer_interface *mixer)
 	p = NULL;
 	while ((p = snd_usb_find_csint_desc(hostif->extra, hostif->extralen, p, UAC_OUTPUT_TERMINAL)) != NULL) {
 		if (mixer->protocol == UAC_VERSION_1) {
-			struct uac_output_terminal_descriptor_v1 *desc = p;
+			struct uac1_output_terminal_descriptor *desc = p;
 
 			if (desc->bLength < sizeof(*desc))
 				continue; /* invalid descriptor? */
-- 
1.7.1

[PATCH 3/5] ALSA: usb-midi: whitespace fixes

[Daniel Mack]

Signed-off-by: Daniel Mack <daniel@caiaq.de>
---
 sound/usb/midi.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/sound/usb/midi.c b/sound/usb/midi.c
index 4678564..b9c2bc6 100644
--- a/sound/usb/midi.c
+++ b/sound/usb/midi.c
@@ -434,7 +434,7 @@ static void snd_usbmidi_maudio_broken_running_status_input(
 			u8 cin = buffer[i] & 0x0f;
 			struct usbmidi_in_port *port = &ep->ports[cable];
 			int length;
-			
+
 			length = snd_usbmidi_cin_length[cin];
 			if (cin == 0xf && buffer[i + 1] >= 0xf8)
 				; /* realtime msg: no running status change */
@@ -628,13 +628,13 @@ static struct usb_protocol_ops snd_usbmidi_standard_ops = {
 
 static struct usb_protocol_ops snd_usbmidi_midiman_ops = {
 	.input = snd_usbmidi_midiman_input,
-	.output = snd_usbmidi_standard_output, 
+	.output = snd_usbmidi_standard_output,
 	.output_packet = snd_usbmidi_output_midiman_packet,
 };
 
 static struct usb_protocol_ops snd_usbmidi_maudio_broken_running_status_ops = {
 	.input = snd_usbmidi_maudio_broken_running_status_input,
-	.output = snd_usbmidi_standard_output, 
+	.output = snd_usbmidi_standard_output,
 	.output_packet = snd_usbmidi_output_standard_packet,
 };
 
@@ -1248,7 +1248,7 @@ static void snd_usbmidi_out_endpoint_delete(struct snd_usb_midi_out_endpoint *ep
  */
 static int snd_usbmidi_out_endpoint_create(struct snd_usb_midi* umidi,
 					   struct snd_usb_midi_endpoint_info* ep_info,
-			 		   struct snd_usb_midi_endpoint* rep)
+					   struct snd_usb_midi_endpoint* rep)
 {
 	struct snd_usb_midi_out_endpoint* ep;
 	unsigned int i;
@@ -1398,7 +1398,7 @@ static void snd_usbmidi_rawmidi_free(struct snd_rawmidi *rmidi)
 }
 
 static struct snd_rawmidi_substream *snd_usbmidi_find_substream(struct snd_usb_midi* umidi,
-							   int stream, int number)
+								int stream, int number)
 {
 	struct list_head* list;
 
@@ -1811,7 +1811,7 @@ static int snd_usbmidi_detect_endpoints(struct snd_usb_midi* umidi,
 		snd_usbmidi_switch_roland_altsetting(umidi);
 
 	if (endpoint[0].out_ep || endpoint[0].in_ep)
-		return 0;	
+		return 0;
 
 	intf = umidi->iface;
 	if (!intf || intf->num_altsetting < 1)
@@ -1849,7 +1849,7 @@ static int snd_usbmidi_detect_per_port_endpoints(struct snd_usb_midi* umidi,
 						 struct snd_usb_midi_endpoint_info* endpoints)
 {
 	int err, i;
-	
+
 	err = snd_usbmidi_detect_endpoints(umidi, endpoints, MIDI_MAX_ENDPOINTS);
 	for (i = 0; i < MIDI_MAX_ENDPOINTS; ++i) {
 		if (endpoints[i].out_ep)
-- 
1.7.1

[PATCH 4/5] ALSA: usb-audio: move and add some comments

[Daniel Mack]

Also add a list of open topics.

Signed-off-by: Daniel Mack <daniel@caiaq.de>
---
 include/linux/usb/audio-v2.h |   15 +++++++++++++++
 sound/usb/clock.c            |   16 ++++++++++++++--
 sound/usb/mixer.c            |   24 ++++++++++++++++--------
 3 files changed, 45 insertions(+), 10 deletions(-)

diff --git a/include/linux/usb/audio-v2.h b/include/linux/usb/audio-v2.h
index 716aebe..964cb60 100644
--- a/include/linux/usb/audio-v2.h
+++ b/include/linux/usb/audio-v2.h
@@ -18,6 +18,21 @@
 /* v1.0 and v2.0 of this standard have many things in common. For the rest
  * of the definitions, please refer to audio.h */
 
+/*
+ * bmControl field decoders
+ *
+ * From the USB Audio spec v2.0:
+ *
+ *   bmaControls() is a (ch+1)-element array of 4-byte bitmaps,
+ *   each containing a set of bit pairs. If a Control is present,
+ *   it must be Host readable. If a certain Control is not
+ *   present then the bit pair must be set to 0b00.
+ *   If a Control is present but read-only, the bit pair must be
+ *   set to 0b01. If a Control is also Host programmable, the bit
+ *   pair must be set to 0b11. The value 0b10 is not allowed.
+ *
+ */
+
 static inline bool uac2_control_is_readable(u32 bmControls, u8 control)
 {
 	return (bmControls >> (control * 2)) & 0x1;
diff --git a/sound/usb/clock.c b/sound/usb/clock.c
index 386b09c..7279d61 100644
--- a/sound/usb/clock.c
+++ b/sound/usb/clock.c
@@ -120,8 +120,6 @@ static bool uac_clock_source_is_valid(struct snd_usb_audio *chip, int source_id)
 	return !!data;
 }
 
-/* Try to find the clock source ID of a given clock entity */
-
 static int __uac_clock_find_source(struct snd_usb_audio *chip,
 				   struct usb_host_interface *host_iface,
 				   int entity_id, unsigned long *visited)
@@ -154,6 +152,8 @@ static int __uac_clock_find_source(struct snd_usb_audio *chip,
 		if (ret < 0)
 			return ret;
 
+		/* Selector values are one-based */
+
 		if (ret > selector->bNrInPins || ret < 1) {
 			printk(KERN_ERR
 				"%s(): selector reported illegal value, id %d, ret %d\n",
@@ -176,6 +176,17 @@ static int __uac_clock_find_source(struct snd_usb_audio *chip,
 	return -EINVAL;
 }
 
+/*
+ * For all kinds of sample rate settings and other device queries,
+ * the clock source (end-leaf) must be used. However, clock selectors,
+ * clock multipliers and sample rate converters may be specified as
+ * clock source input to terminal. This functions walks the clock path
+ * to its end and tries to find the source.
+ *
+ * The 'visited' bitfield is used internally to detect recursive loops.
+ *
+ * Returns the clock source UnitID (>=0) on success, or an error.
+ */
 int snd_usb_clock_find_source(struct snd_usb_audio *chip,
 			      struct usb_host_interface *host_iface,
 			      int entity_id)
@@ -246,6 +257,7 @@ static int set_sample_rate_v2(struct snd_usb_audio *chip, int iface,
 		return clock;
 
 	if (!uac_clock_source_is_valid(chip, clock)) {
+		/* TODO: should we try to find valid clock setups by ourself? */
 		snd_printk(KERN_ERR "%d:%d:%d: clock source %d is not valid, cannot use\n",
 			   dev->devnum, iface, fmt->altsetting, clock);
 		return -ENXIO;
diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index b6a261f..7ffea71 100644
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -26,6 +26,22 @@
  *
  */
 
+/*
+ * TODOs, for both the mixer and the streaming interfaces:
+ *
+ *  - support for UAC2 effect units
+ *  - support for graphical equalizers
+ *  - RANGE and MEM set commands (UAC2)
+ *  - RANGE and MEM interrupt dispatchers (UAC2)
+ *  - audio channel clustering (UAC2)
+ *  - audio sample rate converter units (UAC2)
+ *  - proper handling of clock multipliers (UAC2)
+ *  - dispatch clock change notifications (UAC2)
+ *  	- stop PCM streams which use a clock that became invalid
+ *  	- stop PCM streams which use a clock selector that has changed
+ *  	- parse available sample rates again when clock sources changed
+ */
+
 #include <linux/bitops.h>
 #include <linux/init.h>
 #include <linux/list.h>
@@ -1186,14 +1202,6 @@ static int parse_audio_feature_unit(struct mixer_build *state, int unitid, void
 		}
 	} else { /* UAC_VERSION_2 */
 		for (i = 0; i < 30/2; i++) {
-			/* From the USB Audio spec v2.0:
-			   bmaControls() is a (ch+1)-element array of 4-byte bitmaps,
-			   each containing a set of bit pairs. If a Control is present,
-			   it must be Host readable. If a certain Control is not
-			   present then the bit pair must be set to 0b00.
-			   If a Control is present but read-only, the bit pair must be
-			   set to 0b01. If a Control is also Host programmable, the bit
-			   pair must be set to 0b11. The value 0b10 is not allowed. */
 			unsigned int ch_bits = 0;
 			unsigned int ch_read_only = 0;
 
-- 
1.7.1

[PATCH 5/5] ALSA: usb-audio: simplify control interface access

[Daniel Mack]

As the control interface is now carried in struct snd_usb_audio, we can
simplify the API a little and also drop the private ctrlif field from
struct usb_mixer_interface.

Also remove a left-over function prototype in pcm.h.

Signed-off-by: Daniel Mack <daniel@caiaq.de>
---
 sound/usb/clock.c    |   22 +++++++++-------------
 sound/usb/clock.h    |    4 +---
 sound/usb/endpoint.c |    1 +
 sound/usb/format.c   |    9 ++++-----
 sound/usb/mixer.c    |   37 ++++++++++++++++++-------------------
 sound/usb/mixer.h    |    1 -
 sound/usb/pcm.h      |    3 ---
 sound/usb/quirks.c   |    1 +
 8 files changed, 34 insertions(+), 44 deletions(-)

diff --git a/sound/usb/clock.c b/sound/usb/clock.c
index 7279d61..66bd157 100644
--- a/sound/usb/clock.c
+++ b/sound/usb/clock.c
@@ -121,7 +121,6 @@ static bool uac_clock_source_is_valid(struct snd_usb_audio *chip, int source_id)
 }
 
 static int __uac_clock_find_source(struct snd_usb_audio *chip,
-				   struct usb_host_interface *host_iface,
 				   int entity_id, unsigned long *visited)
 {
 	struct uac_clock_source_descriptor *source;
@@ -138,11 +137,11 @@ static int __uac_clock_find_source(struct snd_usb_audio *chip,
 	}
 
 	/* first, see if the ID we're looking for is a clock source already */
-	source = snd_usb_find_clock_source(host_iface, entity_id);
+	source = snd_usb_find_clock_source(chip->ctrl_intf, entity_id);
 	if (source)
 		return source->bClockID;
 
-	selector = snd_usb_find_clock_selector(host_iface, entity_id);
+	selector = snd_usb_find_clock_selector(chip->ctrl_intf, entity_id);
 	if (selector) {
 		int ret;
 
@@ -162,16 +161,15 @@ static int __uac_clock_find_source(struct snd_usb_audio *chip,
 			return -EINVAL;
 		}
 
-		return __uac_clock_find_source(chip, host_iface,
-					       selector->baCSourceID[ret-1],
+		return __uac_clock_find_source(chip, selector->baCSourceID[ret-1],
 					       visited);
 	}
 
 	/* FIXME: multipliers only act as pass-thru element for now */
-	multiplier = snd_usb_find_clock_multiplier(host_iface, entity_id);
+	multiplier = snd_usb_find_clock_multiplier(chip->ctrl_intf, entity_id);
 	if (multiplier)
-		return __uac_clock_find_source(chip, host_iface,
-					       multiplier->bCSourceID, visited);
+		return __uac_clock_find_source(chip, multiplier->bCSourceID,
+						visited);
 
 	return -EINVAL;
 }
@@ -187,13 +185,11 @@ static int __uac_clock_find_source(struct snd_usb_audio *chip,
  *
  * Returns the clock source UnitID (>=0) on success, or an error.
  */
-int snd_usb_clock_find_source(struct snd_usb_audio *chip,
-			      struct usb_host_interface *host_iface,
-			      int entity_id)
+int snd_usb_clock_find_source(struct snd_usb_audio *chip, int entity_id)
 {
 	DECLARE_BITMAP(visited, 256);
 	memset(visited, 0, sizeof(visited));
-	return __uac_clock_find_source(chip, host_iface, entity_id, visited);
+	return __uac_clock_find_source(chip, entity_id, visited);
 }
 
 static int set_sample_rate_v1(struct snd_usb_audio *chip, int iface,
@@ -251,7 +247,7 @@ static int set_sample_rate_v2(struct snd_usb_audio *chip, int iface,
 	struct usb_device *dev = chip->dev;
 	unsigned char data[4];
 	int err, crate;
-	int clock = snd_usb_clock_find_source(chip, chip->ctrl_intf, fmt->clock);
+	int clock = snd_usb_clock_find_source(chip, fmt->clock);
 
 	if (clock < 0)
 		return clock;
diff --git a/sound/usb/clock.h b/sound/usb/clock.h
index beb2536..4663093 100644
--- a/sound/usb/clock.h
+++ b/sound/usb/clock.h
@@ -5,8 +5,6 @@ int snd_usb_init_sample_rate(struct snd_usb_audio *chip, int iface,
 			     struct usb_host_interface *alts,
 			     struct audioformat *fmt, int rate);
 
-int snd_usb_clock_find_source(struct snd_usb_audio *chip,
-			      struct usb_host_interface *host_iface,
-			      int entity_id);
+int snd_usb_clock_find_source(struct snd_usb_audio *chip, int entity_id);
 
 #endif /* __USBAUDIO_CLOCK_H */
diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
index 289a165..6ce4ed1 100644
--- a/sound/usb/endpoint.c
+++ b/sound/usb/endpoint.c
@@ -33,6 +33,7 @@
 #include "pcm.h"
 #include "helper.h"
 #include "format.h"
+#include "clock.h"
 
 /*
  * free a substream
diff --git a/sound/usb/format.c b/sound/usb/format.c
index 30364ab..4387f54 100644
--- a/sound/usb/format.c
+++ b/sound/usb/format.c
@@ -264,13 +264,12 @@ static int parse_uac2_sample_rate_range(struct audioformat *fp, int nr_triplets,
  * on the audioformat table (audio class v2).
  */
 static int parse_audio_format_rates_v2(struct snd_usb_audio *chip,
-				       struct audioformat *fp,
-				       struct usb_host_interface *iface)
+				       struct audioformat *fp)
 {
 	struct usb_device *dev = chip->dev;
 	unsigned char tmp[2], *data;
 	int nr_triplets, data_size, ret = 0;
-	int clock = snd_usb_clock_find_source(chip, chip->ctrl_intf, fp->clock);
+	int clock = snd_usb_clock_find_source(chip, fp->clock);
 
 	if (clock < 0) {
 		snd_printk(KERN_ERR "%s(): unable to find clock source (clock %d)\n",
@@ -391,7 +390,7 @@ static int parse_audio_format_i(struct snd_usb_audio *chip,
 		break;
 	case UAC_VERSION_2:
 		/* fp->channels is already set in this case */
-		ret = parse_audio_format_rates_v2(chip, fp, iface);
+		ret = parse_audio_format_rates_v2(chip, fp);
 		break;
 	}
 
@@ -450,7 +449,7 @@ static int parse_audio_format_ii(struct snd_usb_audio *chip,
 		framesize = le16_to_cpu(fmt->wSamplesPerFrame);
 		snd_printd(KERN_INFO "found format II with max.bitrate = %d, frame size=%d\n", brate, framesize);
 		fp->frame_size = framesize;
-		ret = parse_audio_format_rates_v2(chip, fp, iface);
+		ret = parse_audio_format_rates_v2(chip, fp);
 		break;
 	}
 	}
diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index 7ffea71..a8911d9 100644
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -291,16 +291,15 @@ static int get_abs_value(struct usb_mixer_elem_info *cval, int val)
 
 static int get_ctl_value_v1(struct usb_mixer_elem_info *cval, int request, int validx, int *value_ret)
 {
+	struct snd_usb_audio *chip = cval->mixer->chip;
 	unsigned char buf[2];
 	int val_len = cval->val_type >= USB_MIXER_S16 ? 2 : 1;
 	int timeout = 10;
 
 	while (timeout-- > 0) {
-		if (snd_usb_ctl_msg(cval->mixer->chip->dev,
-				    usb_rcvctrlpipe(cval->mixer->chip->dev, 0),
-				    request,
+		if (snd_usb_ctl_msg(chip->dev, usb_rcvctrlpipe(chip->dev, 0), request,
 				    USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,
-				    validx, cval->mixer->ctrlif | (cval->id << 8),
+				    validx, snd_usb_ctrl_intf(chip) | (cval->id << 8),
 				    buf, val_len, 100) >= val_len) {
 			*value_ret = convert_signed_value(cval, snd_usb_combine_bytes(buf, val_len));
 			return 0;
@@ -313,6 +312,7 @@ static int get_ctl_value_v1(struct usb_mixer_elem_info *cval, int request, int v
 
 static int get_ctl_value_v2(struct usb_mixer_elem_info *cval, int request, int validx, int *value_ret)
 {
+	struct snd_usb_audio *chip = cval->mixer->chip;
 	unsigned char buf[2 + 3*sizeof(__u16)]; /* enough space for one range */
 	unsigned char *val;
 	int ret, size;
@@ -328,16 +328,14 @@ static int get_ctl_value_v2(struct usb_mixer_elem_info *cval, int request, int v
 
 	memset(buf, 0, sizeof(buf));
 
-	ret = snd_usb_ctl_msg(cval->mixer->chip->dev,
-			      usb_rcvctrlpipe(cval->mixer->chip->dev, 0),
-			      bRequest,
+	ret = snd_usb_ctl_msg(chip->dev, usb_rcvctrlpipe(chip->dev, 0), bRequest,
 			      USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,
-			      validx, cval->mixer->ctrlif | (cval->id << 8),
+			      validx, snd_usb_ctrl_intf(chip) | (cval->id << 8),
 			      buf, size, 1000);
 
 	if (ret < 0) {
 		snd_printk(KERN_ERR "cannot get ctl value: req = %#x, wValue = %#x, wIndex = %#x, type = %d\n",
-			   request, validx, cval->mixer->ctrlif | (cval->id << 8), cval->val_type);
+			   request, validx, snd_usb_ctrl_intf(chip) | (cval->id << 8), cval->val_type);
 		return ret;
 	}
 
@@ -413,6 +411,7 @@ static int get_cur_mix_value(struct usb_mixer_elem_info *cval,
 int snd_usb_mixer_set_ctl_value(struct usb_mixer_elem_info *cval,
 				int request, int validx, int value_set)
 {
+	struct snd_usb_audio *chip = cval->mixer->chip;
 	unsigned char buf[2];
 	int val_len, timeout = 10;
 
@@ -435,15 +434,14 @@ int snd_usb_mixer_set_ctl_value(struct usb_mixer_elem_info *cval,
 	buf[0] = value_set & 0xff;
 	buf[1] = (value_set >> 8) & 0xff;
 	while (timeout-- > 0)
-		if (snd_usb_ctl_msg(cval->mixer->chip->dev,
-				    usb_sndctrlpipe(cval->mixer->chip->dev, 0),
-				    request,
+		if (snd_usb_ctl_msg(chip->dev,
+				    usb_sndctrlpipe(chip->dev, 0), request,
 				    USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_OUT,
-				    validx, cval->mixer->ctrlif | (cval->id << 8),
+				    validx, snd_usb_ctrl_intf(chip) | (cval->id << 8),
 				    buf, val_len, 100) >= 0)
 			return 0;
 	snd_printdd(KERN_ERR "cannot set ctl value: req = %#x, wValue = %#x, wIndex = %#x, type = %d, data = %#x/%#x\n",
-		    request, validx, cval->mixer->ctrlif | (cval->id << 8), cval->val_type, buf[0], buf[1]);
+		    request, validx, snd_usb_ctrl_intf(chip) | (cval->id << 8), cval->val_type, buf[0], buf[1]);
 	return -EINVAL;
 }
 
@@ -761,6 +759,8 @@ static void usb_mixer_elem_free(struct snd_kcontrol *kctl)
  */
 static int get_min_max(struct usb_mixer_elem_info *cval, int default_min)
 {
+	struct snd_usb_audio *chip = cval->mixer->chip;
+
 	/* for failsafe */
 	cval->min = default_min;
 	cval->max = cval->min + 1;
@@ -783,7 +783,7 @@ static int get_min_max(struct usb_mixer_elem_info *cval, int default_min)
 		if (get_ctl_value(cval, UAC_GET_MAX, (cval->control << 8) | minchn, &cval->max) < 0 ||
 		    get_ctl_value(cval, UAC_GET_MIN, (cval->control << 8) | minchn, &cval->min) < 0) {
 			snd_printd(KERN_ERR "%d:%d: cannot get min/max values for control %d (id %d)\n",
-				   cval->id, cval->mixer->ctrlif, cval->control, cval->id);
+				   cval->id, snd_usb_ctrl_intf(chip), cval->control, cval->id);
 			return -EINVAL;
 		}
 		if (get_ctl_value(cval, UAC_GET_RES, (cval->control << 8) | minchn, &cval->res) < 0) {
@@ -1900,7 +1900,7 @@ static int snd_usb_mixer_controls(struct usb_mixer_interface *mixer)
 	struct usb_host_interface *hostif;
 	void *p;
 
-	hostif = &usb_ifnum_to_if(mixer->chip->dev, mixer->ctrlif)->altsetting[0];
+	hostif = mixer->chip->ctrl_intf;
 	memset(&state, 0, sizeof(state));
 	state.chip = mixer->chip;
 	state.mixer = mixer;
@@ -1992,7 +1992,7 @@ static void snd_usb_mixer_proc_read(struct snd_info_entry *entry,
 	list_for_each_entry(mixer, &chip->mixer_list, list) {
 		snd_iprintf(buffer,
 			"USB Mixer: usb_id=0x%08x, ctrlif=%i, ctlerr=%i\n",
-				chip->usb_id, mixer->ctrlif,
+				chip->usb_id, snd_usb_ctrl_intf(chip),
 				mixer->ignore_ctl_error);
 		snd_iprintf(buffer, "Card: %s\n", chip->card->longname);
 		for (unitid = 0; unitid < MAX_ID_ELEMS; unitid++) {
@@ -2110,7 +2110,7 @@ static int snd_usb_mixer_status_create(struct usb_mixer_interface *mixer)
 	int buffer_length;
 	unsigned int epnum;
 
-	hostif = &usb_ifnum_to_if(mixer->chip->dev, mixer->ctrlif)->altsetting[0];
+	hostif = mixer->chip->ctrl_intf;
 	/* we need one interrupt input endpoint */
 	if (get_iface_desc(hostif)->bNumEndpoints < 1)
 		return 0;
@@ -2153,7 +2153,6 @@ int snd_usb_create_mixer(struct snd_usb_audio *chip, int ctrlif,
 	if (!mixer)
 		return -ENOMEM;
 	mixer->chip = chip;
-	mixer->ctrlif = ctrlif;
 	mixer->ignore_ctl_error = ignore_error;
 	mixer->id_elems = kcalloc(MAX_ID_ELEMS, sizeof(*mixer->id_elems),
 				  GFP_KERNEL);
diff --git a/sound/usb/mixer.h b/sound/usb/mixer.h
index a7cf100..26c636c 100644
--- a/sound/usb/mixer.h
+++ b/sound/usb/mixer.h
@@ -3,7 +3,6 @@
 
 struct usb_mixer_interface {
 	struct snd_usb_audio *chip;
-	unsigned int ctrlif;
 	struct list_head list;
 	unsigned int ignore_ctl_error;
 	struct urb *urb;
diff --git a/sound/usb/pcm.h b/sound/usb/pcm.h
index 1c931b6..ed3e283 100644
--- a/sound/usb/pcm.h
+++ b/sound/usb/pcm.h
@@ -7,8 +7,5 @@ int snd_usb_init_pitch(struct snd_usb_audio *chip, int iface,
 		       struct usb_host_interface *alts,
 		       struct audioformat *fmt);
 
-int snd_usb_init_sample_rate(struct snd_usb_audio *chip, int iface,
-			     struct usb_host_interface *alts,
-			     struct audioformat *fmt, int rate);
 
 #endif /* __USBAUDIO_PCM_H */
diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c
index b45e54c..9a9da09 100644
--- a/sound/usb/quirks.c
+++ b/sound/usb/quirks.c
@@ -32,6 +32,7 @@
 #include "helper.h"
 #include "endpoint.h"
 #include "pcm.h"
+#include "clock.h"
 
 /*
  * handle the quirks for the contained interfaces
-- 
1.7.1

Re: [PATCH 2/5] ALSA: usb-audio: unify UAC macros and struct names

[Takashi Iwai]

At Wed, 16 Jun 2010 17:57:28 +0200,
Daniel Mack wrote:
> 
> diff --git a/include/linux/usb/audio.h b/include/linux/usb/audio.h
> index c51200c..a54b825 100644
> --- a/include/linux/usb/audio.h
> +++ b/include/linux/usb/audio.h
> @@ -39,8 +39,8 @@
>  #define UAC_MIXER_UNIT			0x04
>  #define UAC_SELECTOR_UNIT		0x05
>  #define UAC_FEATURE_UNIT		0x06
> -#define UAC_PROCESSING_UNIT_V1		0x07
> -#define UAC_EXTENSION_UNIT_V1		0x08
> +#define UAC1_PROCESSING_UNIT		0x07
> +#define UAC1_EXTENSION_UNIT		0x08

So now we have mixed prefix here, UAC_ and UAC1_.
Isn't it a bit confusing, too?

Honestly, I have no much preference about this name-ruling.
But it's of course better if it's stabilized :)


thanks,

Takashi

Re: [PATCH 2/5] ALSA: usb-audio: unify UAC macros and struct names

[Daniel Mack]

On Wed, Jun 16, 2010 at 07:34:49PM +0200, Takashi Iwai wrote:
> At Wed, 16 Jun 2010 17:57:28 +0200,
> Daniel Mack wrote:
> > 
> > diff --git a/include/linux/usb/audio.h b/include/linux/usb/audio.h
> > index c51200c..a54b825 100644
> > --- a/include/linux/usb/audio.h
> > +++ b/include/linux/usb/audio.h
> > @@ -39,8 +39,8 @@
> >  #define UAC_MIXER_UNIT			0x04
> >  #define UAC_SELECTOR_UNIT		0x05
> >  #define UAC_FEATURE_UNIT		0x06
> > -#define UAC_PROCESSING_UNIT_V1		0x07
> > -#define UAC_EXTENSION_UNIT_V1		0x08
> > +#define UAC1_PROCESSING_UNIT		0x07
> > +#define UAC1_EXTENSION_UNIT		0x08
> 
> So now we have mixed prefix here, UAC_ and UAC1_.
> Isn't it a bit confusing, too?
> 
> Honestly, I have no much preference about this name-ruling.
> But it's of course better if it's stabilized :)

Well yeah, I hate that too, especially as it is a matter of taste
eventually. However, the idea is: things that are common for both UAC1
and UAC2 are prefixed with UAC_, and only those things that are special
get a number suffix. Which is the case in the block you quoted above.

(This perticular detail is really the greatest unnecessary confusion in
the UAC2 spec, btw. They just drop one enumeration value and shuffled
two others around for no obvious reason. Now we have to live with that.)

Thanks,
Daniel

Re: [PATCH 2/5] ALSA: usb-audio: unify UAC macros and struct names

[Takashi Iwai]

At Wed, 16 Jun 2010 19:40:44 +0200,
Daniel Mack wrote:
> 
> On Wed, Jun 16, 2010 at 07:34:49PM +0200, Takashi Iwai wrote:
> > At Wed, 16 Jun 2010 17:57:28 +0200,
> > Daniel Mack wrote:
> > > 
> > > diff --git a/include/linux/usb/audio.h b/include/linux/usb/audio.h
> > > index c51200c..a54b825 100644
> > > --- a/include/linux/usb/audio.h
> > > +++ b/include/linux/usb/audio.h
> > > @@ -39,8 +39,8 @@
> > >  #define UAC_MIXER_UNIT			0x04
> > >  #define UAC_SELECTOR_UNIT		0x05
> > >  #define UAC_FEATURE_UNIT		0x06
> > > -#define UAC_PROCESSING_UNIT_V1		0x07
> > > -#define UAC_EXTENSION_UNIT_V1		0x08
> > > +#define UAC1_PROCESSING_UNIT		0x07
> > > +#define UAC1_EXTENSION_UNIT		0x08
> > 
> > So now we have mixed prefix here, UAC_ and UAC1_.
> > Isn't it a bit confusing, too?
> > 
> > Honestly, I have no much preference about this name-ruling.
> > But it's of course better if it's stabilized :)
> 
> Well yeah, I hate that too, especially as it is a matter of taste
> eventually.

Indeed, it's just a matter of taste.

> However, the idea is: things that are common for both UAC1
> and UAC2 are prefixed with UAC_, and only those things that are special
> get a number suffix. Which is the case in the block you quoted above.

Yeah, that I understood.  It's just that I feel something not clear
around this...  It might be simply because of the salad I ate today,
though.  But I'd like to hear opinions of others before merging.
If nothing comes up, I'm willing to apply as is.

> (This perticular detail is really the greatest unnecessary confusion in
> the UAC2 spec, btw. They just drop one enumeration value and shuffled
> two others around for no obvious reason. Now we have to live with that.)

There are always enough examples how to behave rude :)


thanks,

Takashi

Re: [PATCH 2/5] ALSA: usb-audio: unify UAC macros and struct names

[Daniel Mack]

On Wed, Jun 16, 2010 at 01:31:17PM -0700, Alex Lee wrote:
> It will be painful, but what about defining a whole new set of
> UAC2_xxx constants even if it is the same value as UAC1 ?

That doesn't really help much, as the code using these macros is hybrid,
for both versions. Just grep for them in the sources to see what I mean
:)

Daniel


> On 16-Jun-2010, at 1:21 PM, Takashi Iwai <tiwai@suse.de> wrote:
> 
> >At Wed, 16 Jun 2010 19:40:44 +0200,
> >Daniel Mack wrote:
> >>
> >>On Wed, Jun 16, 2010 at 07:34:49PM +0200, Takashi Iwai wrote:
> >>>At Wed, 16 Jun 2010 17:57:28 +0200,
> >>>Daniel Mack wrote:
> >>>>
> >>>>diff --git a/include/linux/usb/audio.h b/include/linux/usb/audio.h
> >>>>index c51200c..a54b825 100644
> >>>>--- a/include/linux/usb/audio.h
> >>>>+++ b/include/linux/usb/audio.h
> >>>>@@ -39,8 +39,8 @@
> >>>>#define UAC_MIXER_UNIT            0x04
> >>>>#define UAC_SELECTOR_UNIT        0x05
> >>>>#define UAC_FEATURE_UNIT        0x06
> >>>>-#define UAC_PROCESSING_UNIT_V1        0x07
> >>>>-#define UAC_EXTENSION_UNIT_V1        0x08
> >>>>+#define UAC1_PROCESSING_UNIT        0x07
> >>>>+#define UAC1_EXTENSION_UNIT        0x08
> >>>
> >>>So now we have mixed prefix here, UAC_ and UAC1_.
> >>>Isn't it a bit confusing, too?
> >>>
> >>>Honestly, I have no much preference about this name-ruling.
> >>>But it's of course better if it's stabilized :)
> >>
> >>Well yeah, I hate that too, especially as it is a matter of taste
> >>eventually.
> >
> >Indeed, it's just a matter of taste.
> >
> >>However, the idea is: things that are common for both UAC1
> >>and UAC2 are prefixed with UAC_, and only those things that are
> >>special
> >>get a number suffix. Which is the case in the block you quoted above.
> >
> >Yeah, that I understood.  It's just that I feel something not clear
> >around this...  It might be simply because of the salad I ate today,
> >though.  But I'd like to hear opinions of others before merging.
> >If nothing comes up, I'm willing to apply as is.
> >
> >>(This perticular detail is really the greatest unnecessary
> >>confusion in
> >>the UAC2 spec, btw. They just drop one enumeration value and shuffled
> >>two others around for no obvious reason. Now we have to live
> >>with that.)
> >
> >There are always enough examples how to behave rude :)
> >
> >
> >thanks,
> >
> >Takashi

Re: [PATCH 2/5] ALSA: usb-audio: unify UAC macros and struct names

[Daniel Mack]

(hint, hint: don't top-post :))

On Thu, Jun 17, 2010 at 08:21:22AM -0700, Alex Lee wrote:
> It will not help existing codes, I agree.  But it may make writing new UAC2
> specific code easier?

Not as long as most of the code stays hybrid for both versions. If there
were versions of these enum values for both UAC1 and UAC2, you would
need to choose one of them in the parser codes. Which one you go for
doesn't actually make any difference as they're both the same, but the
code would read as if it was specific to any version.

I'd say this would add more to the confusion, and not ease any.


Daniel

Re: usb-audio: some cleanup patches for 2.6.36

[Takashi Iwai]

At Wed, 16 Jun 2010 17:57:26 +0200,
Daniel Mack wrote:
> 
> Here are five patches that mostly do some basic cleanups. They're
> unurgent and should be queued for 2.6.36.
> 
> I'm still pondering about the clock topology and how to fully support
> all corner cases. That might take awhile to sort out.
> 
> Daniel
> 
> [PATCH 1/5] ALSA: usb-audio: clean up includes in clock.c
> [PATCH 2/5] ALSA: usb-audio: unify UAC macros and struct names
> [PATCH 3/5] ALSA: usb-midi: whitespace fixes
> [PATCH 4/5] ALSA: usb-audio: move and add some comments
> [PATCH 5/5] ALSA: usb-audio: simplify control interface access

As there are no strong objections, I applied all patches now.
Thanks!


Takashi