usb-serial: fix regression in Visor/Palm OS module for kernels >= 2.6.24

Fixes a bug/inconsistency revealed by the additional sanity checking in
commit 063a2da8f01806906f7d7b1a1424b9afddebc443
introduced in the original 2.6.24 branch.

The Handspring Visor / PalmOS 4 device structure defines .num_bulk_out=2 but the usb-serial probe returns num_bulk_out=3, triggering the check in the above commit and forcing a bail out when the device (a Garmin iQue in my case) attempts to connect. The patch bumps the expected number of endpoints to 3.

I suppose it's possible that the kernel is identifying 3 bulk endpoints when there should only be 2 and there is some issue with the lower level endpoint probe?

FWIW, this patch will probably solve the following kernel bug report for Treo users (identical symptoms, different model PalmOS units):
<http://bugzilla.kernel.org/show_bug.cgi?id=10118>

---

@@ -191,7 +191,7 @@ static struct usb_serial_driver handspri
 	.id_table =		id_table,
 	.num_interrupt_in =	NUM_DONT_CARE,
 	.num_bulk_in =		2,
-	.num_bulk_out =		2,
+	.num_bulk_out =		3,
 	.num_ports =		2,
 	.open =			visor_open,
 	.close =		visor_close,

Am Mittwoch, 26. März 2008 03:32:43 schrieb Brad Sawatzky:

Send in "lsusb -v". Are you sure all devices have 3 endpoints?

Regards
Oliver

[ . . . ]

Seems to:

% sudo lsusb -v
Bus 002 Device 016: ID 091e:0004 Garmin International Device Descriptor:
bLength 18
bDescriptorType 1
bcdUSB 1.00

  bDeviceClass            0 (Defined at Interface level)
  bDeviceSubClass         0 
  bDeviceProtocol         0 
  bMaxPacketSize0         8
  idVendor           0x091e Garmin International
  idProduct          0x0004 
  bcdDevice            1.00
  iManufacturer           1 Palm, Inc.
  iProduct                2 Palm Handheld
  iSerial                 5 PalmSN12345678
  bNumConfigurations      1

Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength 53
bNumInterfaces 1
bConfigurationValue 1
iConfiguration 0
bmAttributes 0xc0
Self Powered
MaxPower 2mA
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 0
bAlternateSetting 0
bNumEndpoints 5
bInterfaceClass 255 Vendor Specific Class
bInterfaceSubClass 0
bInterfaceProtocol 0
iInterface 0
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 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x82 EP 2 IN
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 0x03 EP 3 OUT
bmAttributes 2
Transfer Type Bulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0020 1x 32 bytes
bInterval 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x84 EP 4 IN
bmAttributes 2
Transfer Type Bulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0020 1x 32 bytes
bInterval 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x05 EP 5 OUT
bmAttributes 2
Transfer Type Bulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0020 1x 32 bytes
bInterval 0
Device Status: 0x0001
Self Powered

[ Trimmed out other devices ]

-- Brad

You did not understand Oliver's question. Yes, your device has 3 bulk-OUT endpoints (and 2 bulk-IN). But do you know whether _all_ Visor/Palm OS devices do? If they don't, your patch will cause the driver to stop working when someone plugs in a device with only 2 endpoints.

Alan Stern

[ . . . ]

You're absolutely right -- a very legitimate point.

I only have one PalmOS USB device and can only provide data for that unit. As mentioned in the initial post I think it is very suggestive that there is a kernel bug report saying that both a Treo90 and a Treo750p that exhibited identical symptoms. I'd bet a beer that that the kernel also reports 3 bulk out endpoints for those devices, but I can't prove it.

I'm more concerned that the kernel is creating a bogus 3rd endpoint (for all devices assigned to the so-called "handspring_device") when it should not. Any suggestions on how to check that with my particular unit?

I was hoping that whomever authored the original code (Greg?) might know where the original num_bulk_out=2 value came from: reverse engineering, best guess, or honest-to-god documentation from Palm?

FWIW, another option I considered was patching the test added in commit 063a2da8f01806906f7d7b1a1424b9afddebc443 to use '<=' instead of '!=' in the obvious places. That is, test if the usb-serial subsystem reports at least as many endpoints as the device definition "requires" rather than checking for an exact number. That behaviour is closer to the pre-2.6.24 code (ie. no check at all) and might be a better fix (provided this 3rd bulk-out endpoint I'm seeing is 'real' and not due to a lower-level bug).

-- Brad

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Brad Sawatzky wrote:
| I only have one PalmOS USB device and can only provide data for that unit. | As mentioned in the initial post I think it is very suggestive that there is
| a kernel bug report saying that both a Treo90 and a Treo750p that exhibited

| identical symptoms.  I'd bet a beer that that the kernel also reports 3
| bulk out endpoints for those devices, but I can't prove it.
|

Attached is the lspci -v output for my Treo 90. I see two bulk out endpoints.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.7 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFH60Gxi1yS1BuzIvgRAuh7AKCR/JXrulXICE33R8pXKvie8VZIbwCeOZa1 zjFCIEC3DaOTHPplI0iWAfg=
=c6GN
-----END PGP SIGNATURE-----

Bus 002 Device 003: ID 082d:0200 Handspring
Device Descriptor:
bLength 18
bDescriptorType 1
bcdUSB 1.00

  bDeviceClass            0 (Defined at Interface level)
  bDeviceSubClass         0 
  bDeviceProtocol         0 
  bMaxPacketSize0         8
  idVendor           0x082d Handspring
  idProduct          0x0200 
  bcdDevice            1.00
  iManufacturer           0 
  iProduct                0 
  iSerial                 0 
  bNumConfigurations      1

Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength 46
bNumInterfaces 1
bConfigurationValue 1
iConfiguration 0
bmAttributes 0xa0
(Bus Powered)
Remote Wakeup
MaxPower 2mA
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 0
bAlternateSetting 0
bNumEndpoints 4
bInterfaceClass 255 Vendor Specific Class
bInterfaceSubClass 0
bInterfaceProtocol 0
iInterface 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x01 EP 1 OUT
bmAttributes 2
Transfer Type Bulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0008 1x 8 bytes
bInterval 0
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 1
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 0x84 EP 4 IN
bmAttributes 2
Transfer Type Bulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0040 1x 64 bytes
bInterval 0
Device Status: 0x0001
Self Powered

Am Donnerstag, 27. März 2008 06:07:26 schrieb Brad Sawatzky:

The problem with that is that if a device adds an endpoint we may use the wrong endpoints in drivers, depending on how the endpoints in devices are numbered. As we have no idea what the consequences would be, we rather fail cleanly.

Regards
Oliver