[alsa-devel] [PATCH v2 0/3] 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(-)
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:
On 12.05.2017 15:34, Johan Hovold wrote:
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:
Sorry for the delay... Looks good to me, tested to work OK.
- Jussi Laako
On Tue, May 30, 2017 at 02:40:09AM +0300, Jussi Laako wrote:
On 12.05.2017 15:34, Johan Hovold wrote:
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:Sorry for the delay... Looks good to me, tested to work OK.
Thanks for testing. Patch is in Linus' tree now.
Johan
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 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 johan@kernel.org --- sound/usb/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/usb/Kconfig b/sound/usb/Kconfig index a452ad7cec40..f61b5662bb89 100644 --- a/sound/usb/Kconfig +++ b/sound/usb/Kconfig @@ -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
participants (2)
-
Johan Hovold -
Jussi Laako