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

[PATCH v2 0/3] ALSA: USB-descriptor endianness 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 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(-)

-- 
2.13.0

[PATCH v2 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 v2 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 v2 3/3] ALSA: us122l: enable compile testing

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

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

[Jussi Laako]

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

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

[Johan Hovold]

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