Skip to contentSkip to conversation

ALSA: USB-descriptor endianness fixes

PATCH In review Johan Hovold 2 participants 6 messages May 12, 2017 – May 30, 2017 Sound

lore
Share & tools
lore

Johan Hovold

author
#0 lore raw

v2 cover ALSA: USB-descriptor endianness fixes

These patches add missing endianness conversions when accessing the USB device-descriptor fields. In the process, clean up the us122l driver which had product-id conditionals sprinkled throughout.

The final patch enables compile testing for us122l which currently depends on x86 but compiles just fine for arm.

Note that these patches have only been compile tested.

Johan

v2
- enable compile testing rather than drop the us122l x86-dependency

Johan Hovold (3):
ALSA: usb-audio: fix Amanero Combo384 quirk on big-endian hosts
ALSA: us122l: clean up US144 handling
ALSA: us122l: enable compile testing

 sound/usb/Kconfig        |  2 +-
 sound/usb/quirks.c       |  2 +-
 sound/usb/usx2y/us122l.c | 36 ++++++++++++++++++------------------
 sound/usb/usx2y/us122l.h |  2 ++
 4 files changed, 22 insertions(+), 20 deletions(-)
Signature · 1 line
2.13.0
author
#1 lore raw

PATCH v2 1/3 ALSA: usb-audio: fix Amanero Combo384 quirk on big-endian hosts

Add missing endianness conversion when using the USB device-descriptor bcdDevice field when applying the Amanero Combo384 (endianness!) quirk.

Fixes 3eff682d765b ("ALSA: usb-audio: Support both DSD LE/BE Amanero firmware versions")
Cc Jussi Laako <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
 sound/usb/quirks.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
sound/usb/quirks.c +1 -1
index 01eff6ce6401..d7b0b0a3a2db 100644
@@ -1364,7 +1364,7 @@ u64 snd_usb_interface_dsd_format_quirks(struct snd_usb_audio *chip,
 	/* Amanero Combo384 USB interface with native DSD support */
 	case USB_ID(0x16d0, 0x071a):
 		if (fp->altsetting == 2) {
-			switch (chip->dev->descriptor.bcdDevice) {
+			switch (le16_to_cpu(chip->dev->descriptor.bcdDevice)) {
 			case 0x199:
 				return SNDRV_PCM_FMTBIT_DSD_U32_LE;
 			case 0x19b:
Signature · 1 line
2.13.0
#2 lore raw

Re: [PATCH v2 1/3] ALSA: usb-audio: fix Amanero Combo384 quirk on big-endian hosts

Quoted 16 lines · Johan Hovold

Add missing endianness conversion when using the USB device-descriptor bcdDevice field when applying the Amanero Combo384 (endianness!) quirk.

Fixes 3eff682d765b ("ALSA: usb-audio: Support both DSD LE/BE Amanero firmware versions")
Cc Jussi Laako <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
  sound/usb/quirks.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
sound/usb/quirks.c +0 -0
index 01eff6ce6401..d7b0b0a3a2db 100644

sound/usb/quirks.c u64 snd_usb_interface_dsd_format_quirks(struct snd_usb_audio *chip, · line 1370

Quoted 8 lines
sound/usb/quirks.c +1 -1
@@ -1364,7 +1364,7 @@ u64 snd_usb_interface_dsd_format_quirks(struct snd_usb_audio *chip,
  	/* Amanero Combo384 USB interface with native DSD support */
  	case USB_ID(0x16d0, 0x071a):
  		if (fp->altsetting == 2) {
-			switch (chip->dev->descriptor.bcdDevice) {
+			switch (le16_to_cpu(chip->dev->descriptor.bcdDevice)) {
  			case 0x199:
  				return SNDRV_PCM_FMTBIT_DSD_U32_LE;
  			case 0x19b:

Sorry for the delay... Looks good to me, tested to work OK.

- Jussi Laako

author
#3 lore raw

Re: [PATCH v2 1/3] ALSA: usb-audio: fix Amanero Combo384 quirk on big-endian hosts

Quoted 20 lines · Jussi Laako
Quoted 19 lines · Johan Hovold

Add missing endianness conversion when using the USB device-descriptor bcdDevice field when applying the Amanero Combo384 (endianness!) quirk.

Fixes 3eff682d765b ("ALSA: usb-audio: Support both DSD LE/BE Amanero firmware versions")
Cc Jussi Laako <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
  sound/usb/quirks.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
sound/usb/quirks.c +0 -1
index 01eff6ce6401..d7b0b0a3a2db 100644
@@ -1364,7 +1364,7 @@ u64 snd_usb_interface_dsd_format_quirks(struct snd_usb_audio *chip,
  	/* Amanero Combo384 USB interface with native DSD support */
  	case USB_ID(0x16d0, 0x071a):
  		if (fp->altsetting == 2) {
-			switch (chip->dev->descriptor.bcdDevice) {
Quoted 8 lines

sound/usb/quirks.c u64 snd_usb_interface_dsd_format_quirks(struct snd_usb_audio *chip, · line 1370

Quoted 5 lines
sound/usb/quirks.c +1 -0
+			switch (le16_to_cpu(chip->dev->descriptor.bcdDevice)) {
  			case 0x199:
  				return SNDRV_PCM_FMTBIT_DSD_U32_LE;
  			case 0x19b:

Sorry for the delay... Looks good to me, tested to work OK.

Thanks for testing. Patch is in Linus' tree now.

Johan

author
#4 lore raw

PATCH v2 2/3 ALSA: us122l: clean up US144 handling

In reply to Johan Hovold

Use the device-id table and a private flag to determine the device type (US122 or US144) rather than spreading product-id conditionals throughout the driver.

This USB driver currently depends on X86 (why?), but we should still add the missing endianness conversions when accessing the USB device-descriptor fields.

Compile-tested only.

Signed-off-by Johan Hovold <[email protected]>
---
 sound/usb/usx2y/us122l.c | 36 ++++++++++++++++++------------------
 sound/usb/usx2y/us122l.h |  2 ++
 2 files changed, 20 insertions(+), 18 deletions(-)
sound/usb/usx2y/us122l.c +18 -18
index e118bdca983d..a33e31b2fc2f 100644
@@ -46,8 +46,10 @@ MODULE_PARM_DESC(id, "ID string for "NAME_ALLCAPS".");
 module_param_array(enable, bool, NULL, 0444);
 MODULE_PARM_DESC(enable, "Enable "NAME_ALLCAPS".");
 
-static int snd_us122l_card_used[SNDRV_CARDS];
+/* driver_info flags */
+#define US122L_FLAG_US144	BIT(0)
 
+static int snd_us122l_card_used[SNDRV_CARDS];
 
 static int us122l_create_usbmidi(struct snd_card *card)
 {

@@ -198,8 +200,7 @@ static int usb_stream_hwdep_open(struct snd_hwdep *hw, struct file *file)
 	if (!us122l->first)
 		us122l->first = file;
 
-	if (us122l->dev->descriptor.idProduct == USB_ID_US144 ||
-	    us122l->dev->descriptor.idProduct == USB_ID_US144MKII) {
+	if (us122l->is_us144) {
 		iface = usb_ifnum_to_if(us122l->dev, 0);
 		usb_autopm_get_interface(iface);
 	}

@@ -214,8 +215,7 @@ static int usb_stream_hwdep_release(struct snd_hwdep *hw, struct file *file)
 	struct usb_interface *iface;
 	snd_printdd(KERN_DEBUG "%p %p\n", hw, file);
 
-	if (us122l->dev->descriptor.idProduct == USB_ID_US144 ||
-	    us122l->dev->descriptor.idProduct == USB_ID_US144MKII) {
+	if (us122l->is_us144) {
 		iface = usb_ifnum_to_if(us122l->dev, 0);
 		usb_autopm_put_interface(iface);
 	}

@@ -483,8 +483,7 @@ static bool us122l_create_card(struct snd_card *card)
 	int err;
 	struct us122l *us122l = US122L(card);
 
-	if (us122l->dev->descriptor.idProduct == USB_ID_US144 ||
-	    us122l->dev->descriptor.idProduct == USB_ID_US144MKII) {
+	if (us122l->is_us144) {
 		err = usb_set_interface(us122l->dev, 0, 1);
 		if (err) {
 			snd_printk(KERN_ERR "usb_set_interface error \n");

@@ -503,8 +502,7 @@ static bool us122l_create_card(struct snd_card *card)
 	if (!us122l_start(us122l, 44100, 256))
 		return false;
 
-	if (us122l->dev->descriptor.idProduct == USB_ID_US144 ||
-	    us122l->dev->descriptor.idProduct == USB_ID_US144MKII)
+	if (us122l->is_us144)
 		err = us144_create_usbmidi(card);
 	else
 		err = us122l_create_usbmidi(card);

@@ -536,7 +534,8 @@ static void snd_us122l_free(struct snd_card *card)
 
 static int usx2y_create_card(struct usb_device *device,
 			     struct usb_interface *intf,
-			     struct snd_card **cardp)
+			     struct snd_card **cardp,
+			     unsigned long flags)
 {
 	int		dev;
 	struct snd_card *card;

@@ -556,6 +555,7 @@ static int usx2y_create_card(struct usb_device *device,
 	US122L(card)->dev = device;
 	mutex_init(&US122L(card)->mutex);
 	init_waitqueue_head(&US122L(card)->sk.sleep);
+	US122L(card)->is_us144 = flags & US122L_FLAG_US144;
 	INIT_LIST_HEAD(&US122L(card)->midi_list);
 	strcpy(card->driver, "USB "NAME_ALLCAPS"");
 	sprintf(card->shortname, "TASCAM "NAME_ALLCAPS"");

@@ -579,7 +579,7 @@ static int us122l_usb_probe(struct usb_interface *intf,
 	struct snd_card *card;
 	int err;
 
-	err = usx2y_create_card(device, intf, &card);
+	err = usx2y_create_card(device, intf, &card, device_id->driver_info);
 	if (err < 0)
 		return err;
 

@@ -607,9 +607,8 @@ static int snd_us122l_probe(struct usb_interface *intf,
 	struct snd_card *card;
 	int err;
 
-	if ((device->descriptor.idProduct == USB_ID_US144 ||
-	     device->descriptor.idProduct == USB_ID_US144MKII)
-		&& device->speed == USB_SPEED_HIGH) {
+	if (id->driver_info & US122L_FLAG_US144 &&
+			device->speed == USB_SPEED_HIGH) {
 		snd_printk(KERN_ERR "disable ehci-hcd to run US-144 \n");
 		return -ENODEV;
 	}

@@ -703,8 +702,7 @@ static int snd_us122l_resume(struct usb_interface *intf)
 
 	mutex_lock(&us122l->mutex);
 	/* needed, doesn't restart without: */
-	if (us122l->dev->descriptor.idProduct == USB_ID_US144 ||
-	    us122l->dev->descriptor.idProduct == USB_ID_US144MKII) {
+	if (us122l->is_us144) {
 		err = usb_set_interface(us122l->dev, 0, 1);
 		if (err) {
 			snd_printk(KERN_ERR "usb_set_interface error \n");

@@ -747,7 +745,8 @@ static struct usb_device_id snd_us122l_usb_id_table[] = {
 	{	/* US-144 only works at USB1.1! Disable module ehci-hcd. */
 		.match_flags =	USB_DEVICE_ID_MATCH_DEVICE,
 		.idVendor =	0x0644,
-		.idProduct =	USB_ID_US144
+		.idProduct =	USB_ID_US144,
+		.driver_info =	US122L_FLAG_US144
 	},
 	{
 		.match_flags =	USB_DEVICE_ID_MATCH_DEVICE,

@@ -757,7 +756,8 @@ static struct usb_device_id snd_us122l_usb_id_table[] = {
 	{
 		.match_flags =	USB_DEVICE_ID_MATCH_DEVICE,
 		.idVendor =	0x0644,
-		.idProduct =	USB_ID_US144MKII
+		.idProduct =	USB_ID_US144MKII,
+		.driver_info =	US122L_FLAG_US144
 	},
 	{ /* terminator */ }
 };
sound/usb/usx2y/us122l.h +2 -0
index f263b3f96c86..3e2a2d0041ee 100644
@@ -16,6 +16,8 @@ struct us122l {
 	struct list_head	midi_list;
 
 	atomic_t		mmap_count;
+
+	bool			is_us144;
 };
Signature · 1 line
2.13.0
author
#5 lore raw

PATCH v2 3/3 ALSA: us122l: enable compile testing

In reply to Johan Hovold

This USB driver currently depends on X86 for some undocumented reason. Since it compiles just fine for arm, we can at least enable compile testing.

Signed-off-by Johan Hovold <[email protected]>
---
 sound/usb/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
sound/usb/Kconfig +1 -1
index a452ad7cec40..f61b5662bb89 100644
@@ -91,7 +91,7 @@ config SND_USB_CAIAQ_INPUT
 
 config SND_USB_US122L
 	tristate "Tascam US-122L USB driver"
-	depends on X86
+	depends on X86 || COMPILE_TEST
 	select SND_HWDEP
 	select SND_RAWMIDI
 	help
Signature · 1 line
2.13.0