[alsa-devel] [PATCH 0/3] ALSA: USB-descriptor endianess 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 drops the Kconfig dependency on x86 since it's a USB driver that compiles just fine on non-x86. Perhaps this should be a (X86 || COMPILE_TEST) dependency instead in case there are external dependencies that warrants this limitation.
Note that these patches have only been compile-tested.
Johan
Johan Hovold (3): ALSA: usb-audio: fix Amanero Combo384 quirk on big-endian hosts ALSA: us122l: clean up US144 handling ALSA: us122l: drop x86 dependency
sound/usb/Kconfig | 1 - sound/usb/quirks.c | 2 +- sound/usb/usx2y/us122l.c | 36 ++++++++++++++++++------------------ sound/usb/usx2y/us122l.h | 2 ++ 4 files changed, 21 insertions(+), 20 deletions(-)
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 jussi@sonarnerd.net Signed-off-by: Johan Hovold johan@kernel.org --- sound/usb/quirks.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c index 01eff6ce6401..d7b0b0a3a2db 100644 --- a/sound/usb/quirks.c +++ b/sound/usb/quirks.c @@ -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:
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 johan@kernel.org --- sound/usb/usx2y/us122l.c | 36 ++++++++++++++++++------------------ sound/usb/usx2y/us122l.h | 2 ++ 2 files changed, 20 insertions(+), 18 deletions(-)
diff --git a/sound/usb/usx2y/us122l.c b/sound/usb/usx2y/us122l.c index e118bdca983d..a33e31b2fc2f 100644 --- a/sound/usb/usx2y/us122l.c +++ b/sound/usb/usx2y/us122l.c @@ -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 */ } }; diff --git a/sound/usb/usx2y/us122l.h b/sound/usb/usx2y/us122l.h index f263b3f96c86..3e2a2d0041ee 100644 --- a/sound/usb/usx2y/us122l.h +++ b/sound/usb/usx2y/us122l.h @@ -16,6 +16,8 @@ struct us122l { struct list_head midi_list;
atomic_t mmap_count; + + bool is_us144; };
This is a driver for a USB device and compiles just fine on non-x86 so drop the x86 dependency.
Signed-off-by: Johan Hovold johan@kernel.org --- sound/usb/Kconfig | 1 - 1 file changed, 1 deletion(-)
diff --git a/sound/usb/Kconfig b/sound/usb/Kconfig index a452ad7cec40..d6417cef226b 100644 --- a/sound/usb/Kconfig +++ b/sound/usb/Kconfig @@ -91,7 +91,6 @@ config SND_USB_CAIAQ_INPUT
config SND_USB_US122L tristate "Tascam US-122L USB driver" - depends on X86 select SND_HWDEP select SND_RAWMIDI help
On Fri, 12 May 2017 12:28:18 +0200, Johan Hovold wrote:
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 drops the Kconfig dependency on x86 since it's a USB driver that compiles just fine on non-x86. Perhaps this should be a (X86 || COMPILE_TEST) dependency instead in case there are external dependencies that warrants this limitation.
Yeah, I vaguely remember that the driver is specific to x86 because of its particular usage of mmap. Maybe some other architectures may match, but not all. So, X86 || COMPILE_TEST would be more sensible.
thanks,
Takashi
On Fri, May 12, 2017 at 01:18:16PM +0200, Takashi Iwai wrote:
On Fri, 12 May 2017 12:28:18 +0200, Johan Hovold wrote:
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 drops the Kconfig dependency on x86 since it's a USB driver that compiles just fine on non-x86. Perhaps this should be a (X86 || COMPILE_TEST) dependency instead in case there are external dependencies that warrants this limitation.
Yeah, I vaguely remember that the driver is specific to x86 because of its particular usage of mmap. Maybe some other architectures may match, but not all. So, X86 || COMPILE_TEST would be more sensible.
Thanks for clarifying. I'll respin in a v2.
Johan
participants (2)
-
Johan Hovold -
Takashi Iwai