All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] usb-serial-simple: Add Whistler radio scanners TRX serie support.
@ 2020-08-31  7:21 <Daniel Caujolle-Bert>
  2020-09-18  7:45 ` Johan Hovold
  0 siblings, 1 reply; 6+ messages in thread
From: <Daniel Caujolle-Bert> @ 2020-08-31  7:21 UTC (permalink / raw)
  To: Johan Hovold; +Cc: linux-usb

Hi,

   Whistler TRX serie radio scanners provide 2 USB subclass devices: one
usb mass storage and one serial device.
The problem is USB serial is unusable, as the cdc_acm fails with
error -22.
Enabling the support in the usb_simple_serial driver make it works.


Cheers.
---
Daniel


lsusb -v output:
Bus 003 Device 003: ID 2a59:0012 Whistler Whistler TRX-1e Scanner
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               2.00
  bDeviceClass            0 
  bDeviceSubClass         0 
  bDeviceProtocol         0 
  bMaxPacketSize0         8
  idVendor           0x2a59 
  idProduct          0x0012 
  bcdDevice            0.01
  iManufacturer           1 Whistler
  iProduct                2 Whistler TRX-1e Scanner
  iSerial                 0 
  bNumConfigurations      1
  Configuration Descriptor:
    bLength                 9
    bDescriptorType         2
    wTotalLength       0x0059
    bNumInterfaces          2
    bConfigurationValue     1
    iConfiguration          2 Whistler TRX-1e Scanner
    bmAttributes         0xc0
      Self Powered
    MaxPower              500mA
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       0
      bNumEndpoints           2
      bInterfaceClass         8 Mass Storage
      bInterfaceSubClass      6 SCSI
      bInterfaceProtocol     80 Bulk-Only
      iInterface              0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x81  EP 1 IN
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               1
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x01  EP 1 OUT
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               1
        ** UNRECOGNIZED:  08 11 01 02 02 02 01 00
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        1
      bAlternateSetting       0
      bNumEndpoints           3
      bInterfaceClass         2 Communications
      bInterfaceSubClass      2 Abstract (modem)
      bInterfaceProtocol      1 AT-commands (v.25ter)
      iInterface              0 
      CDC Header:
        bcdCDC               1.10
      CDC ACM:
        bmCapabilities       0x02
          line coding and serial state
      CDC Union:
        bMasterInterface        0
        bSlaveInterface         1 
      CDC Call Management:
        bmCapabilities       0x00
        bDataInterface          1
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x82  EP 2 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0008  1x 8 bytes
        bInterval               2
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x03  EP 3 OUT
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               0
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x83  EP 3 IN
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               0
Device Status:     0x0000
  (Bus Powered)




From 7f47321863c9dd4827460d65e985633c16efbb0f Mon Sep 17 00:00:00 2001
From: Daniel Caujolle-Bert <f1rmb.daniel@gmail.com>
Date: Thu, 27 Aug 2020 10:28:52 +0200
Subject: [PATCH] usb-serial-simple: Add Whistler radio scanners TRX serie support.

Whistler's firmware CDC support seems incomplete or buggy, but usb-serial works.

Signed-off-by: Daniel Caujolle-Bert <f1rmb.daniel@gmail.com>
---
 drivers/usb/class/cdc-acm.c            | 14 ++++++++++++++
 drivers/usb/serial/Kconfig             |  1 +
 drivers/usb/serial/usb-serial-simple.c | 10 ++++++++++
 3 files changed, 25 insertions(+)

diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 991786876dbb..12929f65bcfb 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -1915,6 +1915,20 @@ static const struct usb_device_id acm_ids[] = {
 	.driver_info = SEND_ZERO_PACKET,
 	},
 
+	/* Exclude Whistler radio scanners */
+	{ USB_DEVICE_INTERFACE_CLASS(0x2a59, 0x0010, USB_CLASS_COMM), /* TRX-1  */
+	.driver_info = IGNORE_DEVICE,
+	},
+	{ USB_DEVICE_INTERFACE_CLASS(0x2a59, 0x0011, USB_CLASS_COMM), /* TRX-2  */
+	.driver_info = IGNORE_DEVICE,
+	},
+	{ USB_DEVICE_INTERFACE_CLASS(0x2a59, 0x0012, USB_CLASS_COMM), /* TRX-1e  */
+	.driver_info = IGNORE_DEVICE,
+	},
+ 	{ USB_DEVICE_INTERFACE_CLASS(0x2a59, 0x0013, USB_CLASS_COMM), /* TRX-2e */
+	.driver_info = IGNORE_DEVICE,
+	},
+
 	/* control interfaces without any protocol set */
 	{ USB_INTERFACE_INFO(USB_CLASS_COMM, USB_CDC_SUBCLASS_ACM,
 		USB_CDC_PROTO_NONE) },
diff --git a/drivers/usb/serial/Kconfig b/drivers/usb/serial/Kconfig
index 4007fa25a8ff..cd23e33c0ea4 100644
--- a/drivers/usb/serial/Kconfig
+++ b/drivers/usb/serial/Kconfig
@@ -71,6 +71,7 @@ config USB_SERIAL_SIMPLE
 		- ViVOtech ViVOpay USB device.
 		- Infineon Modem Flashloader USB interface
 		- ZIO Motherboard USB serial interface
+		- Whistler TRX radio scanners
 
 	  To compile this driver as a module, choose M here: the module
 	  will be called usb-serial-simple.
diff --git a/drivers/usb/serial/usb-serial-simple.c b/drivers/usb/serial/usb-serial-simple.c
index bd23a7cb1be2..af32a0fc9447 100644
--- a/drivers/usb/serial/usb-serial-simple.c
+++ b/drivers/usb/serial/usb-serial-simple.c
@@ -112,6 +112,14 @@ DEVICE(suunto, SUUNTO_IDS);
 	{ USB_DEVICE(0x908, 0x0004) }
 DEVICE(siemens_mpi, SIEMENS_IDS);
 
+/* Whistler radio scanners */
+#define WHISTLER_IDS()						\
+	{ USB_DEVICE_INTERFACE_CLASS(0x2a59, 0x0010, USB_CLASS_COMM) }, /* TRX-1  */ \
+	{ USB_DEVICE_INTERFACE_CLASS(0x2a59, 0x0011, USB_CLASS_COMM) }, /* TRX-2  */ \
+	{ USB_DEVICE_INTERFACE_CLASS(0x2a59, 0x0012, USB_CLASS_COMM) }, /* TRX-1e */ \
+	{ USB_DEVICE_INTERFACE_CLASS(0x2a59, 0x0013, USB_CLASS_COMM) }  /* TRX-2e */
+DEVICE(whistler, WHISTLER_IDS);
+
 /* All of the above structures mushed into two lists */
 static struct usb_serial_driver * const serial_drivers[] = {
 	&carelink_device,
@@ -127,6 +135,7 @@ static struct usb_serial_driver * const serial_drivers[] = {
 	&hp4x_device,
 	&suunto_device,
 	&siemens_mpi_device,
+	&whistler_device,
 	NULL
 };
 
@@ -144,6 +153,7 @@ static const struct usb_device_id id_table[] = {
 	HP4X_IDS(),
 	SUUNTO_IDS(),
 	SIEMENS_IDS(),
+	WHISTLER_IDS(),
 	{ },
 };
 MODULE_DEVICE_TABLE(usb, id_table);
-- 
2.25.1





^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] usb-serial-simple: Add Whistler radio scanners TRX serie support.
  2020-08-31  7:21 [PATCH] usb-serial-simple: Add Whistler radio scanners TRX serie support <Daniel Caujolle-Bert>
@ 2020-09-18  7:45 ` Johan Hovold
  2020-09-18  8:30   ` <Daniel Caujolle-Bert>
  2020-09-18 13:14   ` <Daniel Caujolle-Bert>
  0 siblings, 2 replies; 6+ messages in thread
From: Johan Hovold @ 2020-09-18  7:45 UTC (permalink / raw)
  To: <Daniel Caujolle-Bert>, Oliver Neukum; +Cc: Johan Hovold, linux-usb

On Mon, Aug 31, 2020 at 09:21:27AM +0200, <Daniel Caujolle-Bert> wrote:
> Hi,
> 
>    Whistler TRX serie radio scanners provide 2 USB subclass devices: one
> usb mass storage and one serial device.
> The problem is USB serial is unusable, as the cdc_acm fails with
> error -22.
> Enabling the support in the usb_simple_serial driver make it works.

Thanks for the patch, and sorry about the late reply.

> lsusb -v output:
> Bus 003 Device 003: ID 2a59:0012 Whistler Whistler TRX-1e Scanner
> Device Descriptor:
>   bLength                18
>   bDescriptorType         1
>   bcdUSB               2.00
>   bDeviceClass            0 
>   bDeviceSubClass         0 
>   bDeviceProtocol         0 
>   bMaxPacketSize0         8
>   idVendor           0x2a59 
>   idProduct          0x0012 
>   bcdDevice            0.01
>   iManufacturer           1 Whistler
>   iProduct                2 Whistler TRX-1e Scanner
>   iSerial                 0 
>   bNumConfigurations      1
>   Configuration Descriptor:
>     bLength                 9
>     bDescriptorType         2
>     wTotalLength       0x0059
>     bNumInterfaces          2
>     bConfigurationValue     1
>     iConfiguration          2 Whistler TRX-1e Scanner
>     bmAttributes         0xc0
>       Self Powered
>     MaxPower              500mA
>     Interface Descriptor:
>       bLength                 9
>       bDescriptorType         4
>       bInterfaceNumber        0
>       bAlternateSetting       0
>       bNumEndpoints           2
>       bInterfaceClass         8 Mass Storage
>       bInterfaceSubClass      6 SCSI
>       bInterfaceProtocol     80 Bulk-Only
>       iInterface              0 
>       Endpoint Descriptor:
>         bLength                 7
>         bDescriptorType         5
>         bEndpointAddress     0x81  EP 1 IN
>         bmAttributes            2
>           Transfer Type            Bulk
>           Synch Type               None
>           Usage Type               Data
>         wMaxPacketSize     0x0040  1x 64 bytes
>         bInterval               1
>       Endpoint Descriptor:
>         bLength                 7
>         bDescriptorType         5
>         bEndpointAddress     0x01  EP 1 OUT
>         bmAttributes            2
>           Transfer Type            Bulk
>           Synch Type               None
>           Usage Type               Data
>         wMaxPacketSize     0x0040  1x 64 bytes
>         bInterval               1
>         ** UNRECOGNIZED:  08 11 01 02 02 02 01 00
>     Interface Descriptor:
>       bLength                 9
>       bDescriptorType         4
>       bInterfaceNumber        1
>       bAlternateSetting       0
>       bNumEndpoints           3
>       bInterfaceClass         2 Communications
>       bInterfaceSubClass      2 Abstract (modem)
>       bInterfaceProtocol      1 AT-commands (v.25ter)
>       iInterface              0 
>       CDC Header:
>         bcdCDC               1.10
>       CDC ACM:
>         bmCapabilities       0x02
>           line coding and serial state
>       CDC Union:
>         bMasterInterface        0
>         bSlaveInterface         1 

It seems this is the problem right here, that the device designates the
mass-storage interface as master.

I think it would be better if we could teach cdc-acm to handle this
device, for example, by adding a quirk to ignore the union descriptor or
to tell the driver that we're dealing with a combined-interface
directly. In fact, we already have a quirk for the latter, it just needs
to be generalised a bit.

Daniel, could you try the below patch and see if that would work for
you?

Note that adding a IGNORE_UNION quirk allows for a smaller patch, but I
prefer generalising the one we already have instead of adding another. 

Oliver, what do think about this? Do you see any way of autodetecting
broken union descriptors without risking introducing regression? It was
thinking about falling back to combined-interface probing, for example,
if the master interface isn't a Communication or Data Class interface
(as per spec), or perhaps simply if the probed interface has three
endpoints.

>       CDC Call Management:
>         bmCapabilities       0x00
>         bDataInterface          1
>       Endpoint Descriptor:
>         bLength                 7
>         bDescriptorType         5
>         bEndpointAddress     0x82  EP 2 IN
>         bmAttributes            3
>           Transfer Type            Interrupt
>           Synch Type               None
>           Usage Type               Data
>         wMaxPacketSize     0x0008  1x 8 bytes
>         bInterval               2
>       Endpoint Descriptor:
>         bLength                 7
>         bDescriptorType         5
>         bEndpointAddress     0x03  EP 3 OUT
>         bmAttributes            2
>           Transfer Type            Bulk
>           Synch Type               None
>           Usage Type               Data
>         wMaxPacketSize     0x0040  1x 64 bytes
>         bInterval               0
>       Endpoint Descriptor:
>         bLength                 7
>         bDescriptorType         5
>         bEndpointAddress     0x83  EP 3 IN
>         bmAttributes            2
>           Transfer Type            Bulk
>           Synch Type               None
>           Usage Type               Data
>         wMaxPacketSize     0x0040  1x 64 bytes
>         bInterval               0
> Device Status:     0x0000
>   (Bus Powered)

Johan


From 3a2bf4810ae8bb9266d13359e3cc0ed3425560c4 Mon Sep 17 00:00:00 2001
From: Johan Hovold <johan@kernel.org>
Date: Thu, 17 Sep 2020 10:30:20 +0200
Subject: [PATCH] USB: cdc-acm: add Whistler radio scanners TRX series support

Add support for Whistler radio scanners TRX series, which have a union
descriptor that designates a mass-storage interface as master. Handle
that by generalising the NO_DATA_INTERFACE quirk to allow us to fall
back to using the combined-interface detection.

Link: https://lore.kernel.org/r/5f4ca4f8.1c69fb81.a4487.0f5f@mx.google.com
Reported-by: Daniel Caujolle-Bert <f1rmb.daniel@gmail.com>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/class/cdc-acm.c | 36 ++++++++++++++++++++++++------------
 1 file changed, 24 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 7f6f3ab5b8a6..dc63a3bcccb2 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -1220,27 +1220,26 @@ static int acm_probe(struct usb_interface *intf,
 	if (cmgmd)
 		call_intf_num = cmgmd->bDataInterface;
 
-	if (!union_header) {
-		if (call_intf_num > 0) {
+	combined_interfaces = (quirks & NO_DATA_INTERFACE) != 0;
+
+	if (!union_header || combined_interfaces) {
+		if (call_intf_num > 0 && !combined_interfaces) {
 			dev_dbg(&intf->dev, "No union descriptor, using call management descriptor\n");
-			/* quirks for Droids MuIn LCD */
-			if (quirks & NO_DATA_INTERFACE) {
-				data_interface = usb_ifnum_to_if(usb_dev, 0);
-			} else {
-				data_intf_num = call_intf_num;
-				data_interface = usb_ifnum_to_if(usb_dev, data_intf_num);
-			}
+			data_intf_num = call_intf_num;
+			data_interface = usb_ifnum_to_if(usb_dev, data_intf_num);
 			control_interface = intf;
 		} else {
 			if (intf->cur_altsetting->desc.bNumEndpoints != 3) {
 				dev_dbg(&intf->dev,"No union descriptor, giving up\n");
 				return -ENODEV;
-			} else {
+			}
+
+			if (!combined_interfaces) {
 				dev_warn(&intf->dev,"No union descriptor, testing for castrated device\n");
 				combined_interfaces = 1;
-				control_interface = data_interface = intf;
-				goto look_for_collapsed_interface;
 			}
+			control_interface = data_interface = intf;
+			goto look_for_collapsed_interface;
 		}
 	} else {
 		data_intf_num = union_header->bSlaveInterface0;
@@ -1807,6 +1806,19 @@ static const struct usb_device_id acm_ids[] = {
 	.driver_info = CLEAR_HALT_CONDITIONS,
 	},
 
+	{ USB_DEVICE_INTERFACE_CLASS(0x2a59, 0x0010, USB_CDC_SUBCLASS_ACM),
+	.driver_info = NO_DATA_INTERFACE,
+	},
+	{ USB_DEVICE_INTERFACE_CLASS(0x2a59, 0x0011, USB_CDC_SUBCLASS_ACM),
+	.driver_info = NO_DATA_INTERFACE,
+	},
+	{ USB_DEVICE_INTERFACE_CLASS(0x2a59, 0x0012, USB_CDC_SUBCLASS_ACM),
+	.driver_info = NO_DATA_INTERFACE,
+	},
+	{ USB_DEVICE_INTERFACE_CLASS(0x2a59, 0x0013, USB_CDC_SUBCLASS_ACM),
+	.driver_info = NO_DATA_INTERFACE,
+	},
+
 	/* Nokia S60 phones expose two ACM channels. The first is
 	 * a modem and is picked up by the standard AT-command
 	 * information below. The second is 'vendor-specific' but
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] usb-serial-simple: Add Whistler radio scanners TRX serie support.
  2020-09-18  7:45 ` Johan Hovold
@ 2020-09-18  8:30   ` <Daniel Caujolle-Bert>
  2020-09-18 13:14   ` <Daniel Caujolle-Bert>
  1 sibling, 0 replies; 6+ messages in thread
From: <Daniel Caujolle-Bert> @ 2020-09-18  8:30 UTC (permalink / raw)
  To: Johan Hovold; +Cc: <Daniel Caujolle-Bert>, Oliver Neukum, linux-usb

Hi Johan,


   I will compile right now and test today.


Cheers.
---
Daniel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] usb-serial-simple: Add Whistler radio scanners TRX serie support.
  2020-09-18  7:45 ` Johan Hovold
  2020-09-18  8:30   ` <Daniel Caujolle-Bert>
@ 2020-09-18 13:14   ` <Daniel Caujolle-Bert>
  2020-09-18 14:37     ` Johan Hovold
  1 sibling, 1 reply; 6+ messages in thread
From: <Daniel Caujolle-Bert> @ 2020-09-18 13:14 UTC (permalink / raw)
  To: Johan Hovold; +Cc: <Daniel Caujolle-Bert>, Oliver Neukum, linux-usb

Hi Johan,

  I just tried your patch, and it works, /dev/ttyACM0 is there and
  usable.


Cheers.
---
Daniel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] usb-serial-simple: Add Whistler radio scanners TRX serie support.
  2020-09-18 13:14   ` <Daniel Caujolle-Bert>
@ 2020-09-18 14:37     ` Johan Hovold
  2020-09-18 14:46       ` <Daniel Caujolle-Bert>
  0 siblings, 1 reply; 6+ messages in thread
From: Johan Hovold @ 2020-09-18 14:37 UTC (permalink / raw)
  To: <Daniel Caujolle-Bert>; +Cc: Johan Hovold, Oliver Neukum, linux-usb

On Fri, Sep 18, 2020 at 03:14:50PM +0200, <Daniel Caujolle-Bert> wrote:
> Hi Johan,
> 
>   I just tried your patch, and it works, /dev/ttyACM0 is there and
>   usable.

Great, thanks for testing!

Can I add your Tested-by tag to the resulting patch?

I'll try to get this sorted as soon as possible.

Johan

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] usb-serial-simple: Add Whistler radio scanners TRX serie support.
  2020-09-18 14:37     ` Johan Hovold
@ 2020-09-18 14:46       ` <Daniel Caujolle-Bert>
  0 siblings, 0 replies; 6+ messages in thread
From: <Daniel Caujolle-Bert> @ 2020-09-18 14:46 UTC (permalink / raw)
  To: Johan Hovold; +Cc: <Daniel Caujolle-Bert>, Oliver Neukum, linux-usb

Yes, sure you can add it.

Thanks again.


Cheers.
---
Daniel

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-09-18 14:46 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-31  7:21 [PATCH] usb-serial-simple: Add Whistler radio scanners TRX serie support <Daniel Caujolle-Bert>
2020-09-18  7:45 ` Johan Hovold
2020-09-18  8:30   ` <Daniel Caujolle-Bert>
2020-09-18 13:14   ` <Daniel Caujolle-Bert>
2020-09-18 14:37     ` Johan Hovold
2020-09-18 14:46       ` <Daniel Caujolle-Bert>

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.