[PATCH 0/3] ALSA: USB-descriptor endianess fixes

[PATCH 0/3] ALSA: USB-descriptor endianess fixes

[Johan Hovold]

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(-)

-- 
2.13.0

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

[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 <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:
-- 
2.13.0

[PATCH 2/3] ALSA: us122l: clean up US144 handling

[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 <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;
 };
 
 
-- 
2.13.0

[PATCH 3/3] ALSA: us122l: drop x86 dependency

[Johan Hovold]

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
-- 
2.13.0

Re: [PATCH 0/3] ALSA: USB-descriptor endianess fixes

[Takashi Iwai]

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

Re: [PATCH 0/3] ALSA: USB-descriptor endianess fixes

[Johan Hovold]

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