From: Johan Hovold <johan@kernel.org> Date: 2021-02-22 15:35:23
On Mon, Feb 22, 2021 at 09:53:39AM -0500, Michael G. Katzmann wrote:
On 2/22/21 8:18 AM, Johan Hovold wrote:
I tried hardcoding buf[6-0] in pl2303_set_termios as
a8 a6 01 80 00 02 07 and got a bitrate of ~200kb
so, no these settings do not work in my case (or I missunderstood your
instructions 8-))
Thanks for testing (and that was with 0xa8 in byte 0, right?)
So it seems we have three devices with bcdDevice 0x0300 encoding the
divisors in slightly different ways and that are all still supported by
the vendor's Windows driver.
Unless Prolific are willing to shed some light on this, I guess someone
needs to try to figure out how the Windows driver determines which
encoding to use.
Is your device supposedly also a PL2303 TA? Could you post the output of
lsusb -v for completeness?
Johan
From: Michael G. Katzmann <hidden> Date: 2021-02-22 15:43:26
Sorry, my mistake .. when I put it in the right order it does indeed also give 110Bd !
On 2/22/21 10:34 AM, Johan Hovold wrote:
On Mon, Feb 22, 2021 at 09:53:39AM -0500, Michael G. Katzmann wrote:
quoted
On 2/22/21 8:18 AM, Johan Hovold wrote:
I tried hardcoding buf[6-0] in pl2303_set_termios as
a8 a6 01 80 00 02 07 and got a bitrate of ~200kb
so, no these settings do not work in my case (or I missunderstood your
instructions 8-))
Thanks for testing (and that was with 0xa8 in byte 0, right?)
So it seems we have three devices with bcdDevice 0x0300 encoding the
divisors in slightly different ways and that are all still supported by
the vendor's Windows driver.
Unless Prolific are willing to shed some light on this, I guess someone
needs to try to figure out how the Windows driver determines which
encoding to use.
Is your device supposedly also a PL2303 TA? Could you post the output of
lsusb -v for completeness?
Johan
From: Johan Hovold <johan@kernel.org> Date: 2021-02-22 15:51:33
[ Please try to avoid top posting to the lists. ]
On Mon, Feb 22, 2021 at 10:42:25AM -0500, Michael G. Katzmann wrote:
Sorry, my mistake .. when I put it in the right order it does indeed
also give 110Bd !
Heh, thanks for verifying that. So two ways of encoding the divisors
then. :)
On 2/22/21 10:34 AM, Johan Hovold wrote:
quoted
On Mon, Feb 22, 2021 at 09:53:39AM -0500, Michael G. Katzmann wrote:
quoted
On 2/22/21 8:18 AM, Johan Hovold wrote:
I tried hardcoding buf[6-0] in pl2303_set_termios as
a8 a6 01 80 00 02 07 and got a bitrate of ~200kb
so, no these settings do not work in my case (or I missunderstood your
instructions 8-))
Thanks for testing (and that was with 0xa8 in byte 0, right?)
So it seems we have three devices with bcdDevice 0x0300 encoding the
divisors in slightly different ways and that are all still supported by
the vendor's Windows driver.
Unless Prolific are willing to shed some light on this, I guess someone
needs to try to figure out how the Windows driver determines which
encoding to use.
Is your device supposedly also a PL2303 TA? Could you post the output of
lsusb -v for completeness?
From: Johan Hovold <johan@kernel.org> Date: 2021-02-22 16:49:07
On Mon, Feb 22, 2021 at 11:37:30AM -0500, Michael G. Katzmann wrote:
For reference this is the device I have ...
Bus 001 Device 011: ID 067b:2303 Prolific Technology, Inc. PL2303 Serial Port
Device Descriptor:
bLength 18
bDescriptorType 1
bcdUSB 2.00
bDeviceClass 0
bDeviceSubClass 0
bDeviceProtocol 0
bMaxPacketSize0 64
idVendor 0x067b Prolific Technology, Inc.
idProduct 0x2303 PL2303 Serial Port
bcdDevice 3.00
iManufacturer 1
iProduct 2
Would be interesting to compare with a real TA in case this one happens
to be a clone after all.
My HXD has
bcdDevice 4.00
iManufacturer 1 Prolific Technology Inc.
iProduct 2 USB-Serial Controller D
here.
iSerial 0
bNumConfigurations 1
Thanks!
Joe, would you mind posting the output of "lsusb -v" for your device as
well?
Johan
From: Michael G. Katzmann <hidden> Date: 2021-02-23 15:00:46
On 2/22/21 10:50 AM, Johan Hovold wrote:
Well I found I have another Prolific device here that I got from Adafruit (https://www.adafruit.com/product/954)
It also has a Prolific 2303 and behaves exactly like the other one. i.e. with the current Linux driver it gives a very high baudrate when set to 110 bd and gives 110bd when the modifications are made to the driver that I found.
Is it that we are presuming that what Prolific is telling us is true and only Joe and I are actually measuring the data rate? (i.e. why does the Prolific Windows driver set the values as Joe found ???)
Adafruit device:
Bus 001 Device 013: ID 067b:2303 Prolific Technology, Inc. PL2303 Serial Port
Device Descriptor:
bLength 18
bDescriptorType 1
bcdUSB 2.00
bDeviceClass 0
bDeviceSubClass 0
bDeviceProtocol 0
bMaxPacketSize0 64
idVendor 0x067b Prolific Technology, Inc.
idProduct 0x2303 PL2303 Serial Port
bcdDevice 3.00
iManufacturer 1 Prolific Technology Inc.
iProduct 2 USB-Serial Controller
iSerial 0
bNumConfigurations 1
Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength 0x0027
bNumInterfaces 1
bConfigurationValue 1
iConfiguration 0
bmAttributes 0xa0
(Bus Powered)
Remote Wakeup
MaxPower 100mA
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 0
bAlternateSetting 0
bNumEndpoints 3
bInterfaceClass 255 Vendor Specific Class
bInterfaceSubClass 0
bInterfaceProtocol 0
iInterface 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x81 EP 1 IN
bmAttributes 3
Transfer Type Interrupt
Synch Type None
Usage Type Data
wMaxPacketSize 0x000a 1x 10 bytes
bInterval 1
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x02 EP 2 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
--
|\ _,,,---,,_ Michael Katzmann
/,`.-'`' -. ;-;;,_ NV3Z / VK2BEA / G4NYV
|,4- ) )-,_. ,\ ( `'-'
'---''(_/--' `-'\_) MichaelK@IEEE.org
From: Johan Hovold <johan@kernel.org> Date: 2021-02-23 15:44:31
On Tue, Feb 23, 2021 at 09:58:47AM -0500, Michael G. Katzmann wrote:
Well I found I have another Prolific device here that I got from
Adafruit (https://www.adafruit.com/product/954)
It also has a Prolific 2303 and behaves exactly like the other one.
i.e. with the current Linux driver it gives a very high baudrate when
set to 110 bd and gives 110bd when the modifications are made to the
driver that I found.
Interesting.
Is it that we are presuming that what Prolific is telling us is true
and only Joe and I are actually measuring the data rate? (i.e. why
does the Prolific Windows driver set the values as Joe found ???)
I'm starting to think they've added some alternate baud rate encoding in
order to make life harder for the people pushing (or unknowingly buying)
counterfeit devices.
As you say, why else would the Windows driver support this encoding?
The driver might be triggering on the device descriptors or some control
request so having a packet trace might provide some insight.
I think Joe may already have collected one?
Adafruit device:
Bus 001 Device 013: ID 067b:2303 Prolific Technology, Inc. PL2303 Serial Port
Device Descriptor:
bLength 18
bDescriptorType 1
bcdUSB 2.00
bDeviceClass 0
bDeviceSubClass 0
bDeviceProtocol 0
bMaxPacketSize0 64
idVendor 0x067b Prolific Technology, Inc.
idProduct 0x2303 PL2303 Serial Port
bcdDevice 3.00
iManufacturer 1 Prolific Technology Inc.
iProduct 2 USB-Serial Controller
iSerial 0
bNumConfigurations 1
From: Michael G. Katzmann <hidden> Date: 2021-02-23 15:59:18
On 2/23/21 10:43 AM, Johan Hovold wrote:
On Tue, Feb 23, 2021 at 09:58:47AM -0500, Michael G. Katzmann wrote:
quoted
Is it that we are presuming that what Prolific is telling us is true
and only Joe and I are actually measuring the data rate? (i.e. why
does the Prolific Windows driver set the values as Joe found ???)
I'm starting to think they've added some alternate baud rate encoding in
order to make life harder for the people pushing (or unknowingly buying)
counterfeit devices.
As you say, why else would the Windows driver support this encoding?
I find that 'Halon;'s razor' is helpful in these situations... I can't think that messing with people who use old teleprinters would be useful in protecting one's products 8-)
If Joe has some wireshark traces we can see if there are any vendor specific USB packets. If not I can try it (I'd be starting from scratch as I've only use wireshark on Linux).
I presume you can't see any differentiators in the normal USB identifiers that we can use.
If someone has a device that works under the existing driver, it would be helpful to see if the modified scheme also works on those devices?
From: Johan Hovold <johan@kernel.org> Date: 2021-02-23 16:14:41
On Tue, Feb 23, 2021 at 10:57:09AM -0500, Michael G. Katzmann wrote:
On 2/23/21 10:43 AM, Johan Hovold wrote:
quoted
On Tue, Feb 23, 2021 at 09:58:47AM -0500, Michael G. Katzmann wrote:
quoted
Is it that we are presuming that what Prolific is telling us is true
and only Joe and I are actually measuring the data rate? (i.e. why
does the Prolific Windows driver set the values as Joe found ???)
I'm starting to think they've added some alternate baud rate encoding in
order to make life harder for the people pushing (or unknowingly buying)
counterfeit devices.
As you say, why else would the Windows driver support this encoding?
I find that 'Halon;'s razor' is helpful in these situations... I
can't think that messing with people who use old teleprinters would be
useful in protecting one's products 8-)
Heh, guess you're right.
If Joe has some wireshark traces we can see if there are any vendor
specific USB packets. If not I can try it (I'd be starting from
scratch as I've only use wireshark on Linux).
I presume you can't see any differentiators in the normal USB
identifiers that we can use.
I only have an HXD (and a GC) here.
The HXD has bcdUSB as 1.10 unlike your TA with 2.00, but not sure that
helps.
If someone has a device that works under the existing driver, it would
be helpful to see if the modified scheme also works on those devices?
I tried with both of mine when Joe reported this and neither works with
the alternate scheme (and the GC turned out not to even support divisor
encoding).
Johan
From: Johan Hovold <johan@kernel.org> Date: 2021-02-23 16:52:27
On Tue, Feb 23, 2021 at 11:30:41AM -0500, Michael G. Katzmann wrote:
On 2/23/21 11:14 AM, Johan Hovold wrote:
quoted
I only have an HXD (and a GC) here.
The HXD has bcdUSB as 1.10 unlike your TA with 2.00, but not sure that
helps.
Sound promising .. why do you think this is this not reliable?
Perhaps it is. Perhaps even bcdDevice of 3.00 is enough (includes some
older variants that the TA replaced supposedly). Not sure anyone ever
tried the current scheme on those older models.
Charles, could you post the output of "lsusb -v" for your PL2303TA? And
did you verify that you actually got 110 Bd with the current Linux
driver?
Johan
From: Michael G. Katzmann <hidden> Date: 2021-02-23 19:16:12
On 2/23/21 11:52 AM, Johan Hovold wrote:
On Tue, Feb 23, 2021 at 11:30:41AM -0500, Michael G. Katzmann wrote:
quoted
On 2/23/21 11:14 AM, Johan Hovold wrote:
quoted
I only have an HXD (and a GC) here.
The HXD has bcdUSB as 1.10 unlike your TA with 2.00, but not sure that
helps.
Sound promising .. why do you think this is this not reliable?
Perhaps it is. Perhaps even bcdDevice of 3.00 is enough (includes some
older variants that the TA replaced supposedly). Not sure anyone ever
tried the current scheme on those older models.
Charles, could you post the output of "lsusb -v" for your PL2303TA? And
did you verify that you actually got 110 Bd with the current Linux
driver?
Johan
Here is the USB packet capture from Wireshark oon Windows 10 when:
1) plugging in pl2303 (packets 18-393)
2) setting the device wia cmd line to 110/even/7 bits/2 stop (packets 393-690)
(device is on port 1.7)
https://drive.google.com/file/d/17TkV9JB2iFNdr4LvRftBnV3_DgITGvUH/view?usp=sharing
There are orders of magnitude more traffic than in Linux!
--
|\ _,,,---,,_ Michael Katzmann
/,`.-'`' -. ;-;;,_ NV3Z / VK2BEA / G4NYV
|,4- ) )-,_. ,\ ( `'-'
'---''(_/--' `-'\_) MichaelK@IEEE.org
From: Charles Yeh <hidden> Date: 2021-02-24 07:36:19
PL2303HXD
===>Device Descriptor<===
bLength: 0x12
bDescriptorType: 0x01
bcdUSB: 0x0110
bDeviceClass: 0x00 -> This is an Interface Class
Defined Device
bDeviceSubClass: 0x00
bDeviceProtocol: 0x00
bMaxPacketSize0: 0x40 = (64) Bytes
idVendor: 0x067B = Prolific Technology, Inc.
idProduct: 0x2303
bcdDevice: 0x0400
PL2303TA
===>Device Descriptor<===
bLength: 0x12
bDescriptorType: 0x01
bcdUSB: 0x0200
bDeviceClass: 0x00 -> This is an Interface Class
Defined Device
bDeviceSubClass: 0x00
bDeviceProtocol: 0x00
bMaxPacketSize0: 0x40 = (64) Bytes
idVendor: 0x067B = Prolific Technology, Inc.
idProduct: 0x2303
bcdDevice: 0x0300
PL2303HX(A)/XA ( EOL : PHASED OUT SINCE 2012 )
===>Device Descriptor<===
bLength: 0x12
bDescriptorType: 0x01
bcdUSB: 0x0110
bDeviceClass: 0x00 -> This is an Interface Class
Defined Device
bDeviceSubClass: 0x00
bDeviceProtocol: 0x00
bMaxPacketSize0: 0x40 = (64) Bytes
idVendor: 0x067B = Prolific Technology, Inc.
idProduct: 0x2303
bcdDevice: 0x0300
You can use the two fields bcdUSB & bcdDevice to distinguish PL2303HXD
/ PL2303TA / PL2303HX(A)/XA
Johan Hovold [off-list ref] 於 2021年2月24日 週三 上午12:51寫道：
On Tue, Feb 23, 2021 at 11:30:41AM -0500, Michael G. Katzmann wrote:
quoted
On 2/23/21 11:14 AM, Johan Hovold wrote:
quoted
I only have an HXD (and a GC) here.
The HXD has bcdUSB as 1.10 unlike your TA with 2.00, but not sure that
helps.
Sound promising .. why do you think this is this not reliable?
Perhaps it is. Perhaps even bcdDevice of 3.00 is enough (includes some
older variants that the TA replaced supposedly). Not sure anyone ever
tried the current scheme on those older models.
Charles, could you post the output of "lsusb -v" for your PL2303TA? And
did you verify that you actually got 110 Bd with the current Linux
driver?
Johan
From: Johan Hovold <johan@kernel.org> Date: 2021-02-24 17:04:01
On Wed, Feb 24, 2021 at 03:34:43PM +0800, Charles Yeh wrote:
PL2303HXD
===>Device Descriptor<===
bLength: 0x12
bDescriptorType: 0x01
bcdUSB: 0x0110
bDeviceClass: 0x00 -> This is an Interface Class
Defined Device
bDeviceSubClass: 0x00
bDeviceProtocol: 0x00
bMaxPacketSize0: 0x40 = (64) Bytes
idVendor: 0x067B = Prolific Technology, Inc.
idProduct: 0x2303
bcdDevice: 0x0400
PL2303TA
===>Device Descriptor<===
bLength: 0x12
bDescriptorType: 0x01
bcdUSB: 0x0200
bDeviceClass: 0x00 -> This is an Interface Class
Defined Device
bDeviceSubClass: 0x00
bDeviceProtocol: 0x00
bMaxPacketSize0: 0x40 = (64) Bytes
idVendor: 0x067B = Prolific Technology, Inc.
idProduct: 0x2303
bcdDevice: 0x0300
PL2303HX(A)/XA ( EOL : PHASED OUT SINCE 2012 )
===>Device Descriptor<===
bLength: 0x12
bDescriptorType: 0x01
bcdUSB: 0x0110
bDeviceClass: 0x00 -> This is an Interface Class
Defined Device
bDeviceSubClass: 0x00
bDeviceProtocol: 0x00
bMaxPacketSize0: 0x40 = (64) Bytes
idVendor: 0x067B = Prolific Technology, Inc.
idProduct: 0x2303
bcdDevice: 0x0300
You can use the two fields bcdUSB & bcdDevice to distinguish PL2303HXD
/ PL2303TA / PL2303HX(A)/XA
Thanks a lot, Charles, then we know how to detect the TA.
But can you confirm that your PL2303TA works with the current Linux
driver at 110 Bd (and doesn't need the alternate divisor encoding)?
Johan
From: Johan Hovold <johan@kernel.org> Date: 2021-02-24 17:05:10
On Tue, Feb 23, 2021 at 02:15:06PM -0500, Michael G. Katzmann wrote:
On 2/23/21 11:52 AM, Johan Hovold wrote:
quoted
On Tue, Feb 23, 2021 at 11:30:41AM -0500, Michael G. Katzmann wrote:
quoted
On 2/23/21 11:14 AM, Johan Hovold wrote:
quoted
I only have an HXD (and a GC) here.
The HXD has bcdUSB as 1.10 unlike your TA with 2.00, but not sure that
helps.
Sound promising .. why do you think this is this not reliable?
Perhaps it is. Perhaps even bcdDevice of 3.00 is enough (includes some
older variants that the TA replaced supposedly). Not sure anyone ever
tried the current scheme on those older models.
Charles, could you post the output of "lsusb -v" for your PL2303TA? And
did you verify that you actually got 110 Bd with the current Linux
driver?
Johan
Here is the USB packet capture from Wireshark oon Windows 10 when:
1) plugging in pl2303 (packets 18-393)
2) setting the device wia cmd line to 110/even/7 bits/2 stop (packets 393-690)
(device is on port 1.7)
https://drive.google.com/file/d/17TkV9JB2iFNdr4LvRftBnV3_DgITGvUH/view?usp=sharing
There are orders of magnitude more traffic than in Linux!
Thanks, I'm sure this will come in handy, but I haven't had time to dig
into it myself yet.
Best case, all TAs need the alternate encoding, but Charles seems to
suggest it isn't that easy and in which case the answer probably lies in
these traces.
Perhaps you can even figure out how to poll for an empty TX FIFO from
it, unless Charles is able to provide some details on that separate
matter?
Johan
From: Michael G. Katzmann <hidden> Date: 2021-02-24 17:21:20
On 2/24/21 12:00 PM, Johan Hovold wrote:
On Wed, Feb 24, 2021 at 03:34:43PM +0800, Charles Yeh wrote:
But can you confirm that your PL2303TA works with the current Linux
driver at 110 Bd (and doesn't need the alternate divisor encoding)?
Johan
And by works ... please confirm that the PL2303TA is ACTUALLY outputting data at 110bd (9.09ms bit width) with the current setting provided by the Linux driver.
Michael
From: Michael G. Katzmann <hidden> Date: 2021-02-24 18:14:38
On 2/24/21 12:04 PM, Johan Hovold wrote:
Perhaps you can even figure out how to poll for an empty TX FIFO from
it, unless Charles is able to provide some details on that separate
matter?
I presume from the code below, that when the device is closed, all data waiting to send is clobbered (if so, so the problem is the driver and not the device)
I would have thought that the driver should drain the buffers. I can see that this might be a problem if there is flow control (it may never drain) but the current method seems pretty brutal.
119 void usb_serial_generic_close(struct usb_serial_port *port)
120 {
121 unsigned long flags;
122 int i;
123
124 if (port->bulk_out_size) {
125 for (i = 0; i < ARRAY_SIZE(port->write_urbs); ++i)
126 usb_kill_urb(port->write_urbs[i]);
127
128 spin_lock_irqsave(&port->lock, flags);
129 kfifo_reset_out(&port->write_fifo);
130 spin_unlock_irqrestore(&port->lock, flags);
131 }
132 if (port->bulk_in_size) {
133 for (i = 0; i < ARRAY_SIZE(port->read_urbs); ++i)
134 usb_kill_urb(port->read_urbs[i]);
135 }
136 }
From: Johan Hovold <johan@kernel.org> Date: 2021-02-25 08:43:01
On Wed, Feb 24, 2021 at 01:13:39PM -0500, Michael G. Katzmann wrote:
On 2/24/21 12:04 PM, Johan Hovold wrote:
quoted
Perhaps you can even figure out how to poll for an empty TX FIFO from
it, unless Charles is able to provide some details on that separate
matter?
I presume from the code below, that when the device is closed, all
data waiting to send is clobbered (if so, so the problem is the driver
and not the device)
I would have thought that the driver should drain the buffers. I can
see that this might be a problem if there is flow control (it may
never drain) but the current method seems pretty brutal.
We do; the code below isn't called until after we've waited for the
buffers to drain (driver buffers + device FIFO).
I'll provide a patch so that you can extend the timeout for draining the
driver buffers (defaults to 30 s), but the main problem is that we don't
know how to query the PL2303 FIFO fill level.
119 void usb_serial_generic_close(struct usb_serial_port *port)
120 {
121 unsigned long flags;
122 int i;
123
124 if (port->bulk_out_size) {
125 for (i = 0; i < ARRAY_SIZE(port->write_urbs); ++i)
126 usb_kill_urb(port->write_urbs[i]);
127
128 spin_lock_irqsave(&port->lock, flags);
129 kfifo_reset_out(&port->write_fifo);
130 spin_unlock_irqrestore(&port->lock, flags);
131 }
132 if (port->bulk_in_size) {
133 for (i = 0; i < ARRAY_SIZE(port->read_urbs); ++i)
134 usb_kill_urb(port->read_urbs[i]);
135 }
136 }
From: Charles Yeh <hidden> Date: 2021-03-05 09:33:41
110 bps is not the standard Baud rate,
PL2303TA don't work with the current Linux driver (d5 0e 00 80), It
needs to "a8 a6 01 80"
Charles.
Johan Hovold [off-list ref] 於 2021年2月25日 週四 上午1:00寫道：
But can you confirm that your PL2303TA works with the current Linux
driver at 110 Bd (and doesn't need the alternate divisor encoding)?
From: Johan Hovold <johan@kernel.org> Date: 2021-03-11 16:08:43
On Sat, Mar 06, 2021 at 11:15:52PM -0500, Michael G. Katzmann wrote:
On 3/5/21 4:36 AM, Johan Hovold wrote:
oops I should have looked at the previous code determining variants...
take 2...
#define PL2303_QUIRK_DIVISOR_TA BIT(3)
enum pl2303_type {
TYPE_01, /* Type 0 and 1 (difference unknown) */
TYPE_HX, /* HX version of the pl2303 chip */
TYPE_HXN, /* HXN version of the pl2303 chip */
TYPE_TA, /* TA version of the pl2303 chip */
TYPE_COUNT
};
static const struct pl2303_type_data pl2303_type_data[TYPE_COUNT] =
....
[TYPE_TA] = {
.max_baud_rate = 6000000,
.quirks = PL2303_QUIRK_DIVISOR_TA,
I think we should just a new flag for the alternate divisor encoding
named "alt_divisors" (cf. no_divisors).
Chances are this encoding is used for other types as well.
},
};
static int pl2303_startup(struct usb_serial *serial)
{
....
if ( serial->dev->descriptor.bcdDevice == 0x0300 && serial->dev->descriptor.bcdUSB == 0x0200 )
type = TYPE_TA;
This needs to go after the bDeviceClass == 0x02 check, and the
descriptor fields need to be accessed using le16_to_cpu().
I've prepared a patch series that clean up and tighten the type
detection which I suggest you built upon instead.
I'll post the series after replying here.
else if (serial->dev->descriptor.bDeviceClass == 0x02)
....
}
static speed_t pl2303_encode_baud_rate_divisor( struct usb_serial_port *port,
unsigned char buf[4],
speed_t baud)
{
unsigned int baseline, mantissa, exponent;
struct usb_serial *serial = port->serial;
struct pl2303_serial_private *spriv = usb_get_serial_data(serial);
/*
* Apparently the formula is:
* baudrate = 12M * 32 / (mantissa * 4^exponent)
* where
* mantissa = buf[8:0]
* exponent = buf[11:9]
*
* TA version has more precision
* uses mantissa = buf[bits 10:0 ]
So you discovered that there were even more bits here? Your first
version used ten bits, I believe.
I got an offline mail from a third person having problems with the TA
and who had also verified eleven bits here.
* exponent = buf[bits 15:13]
* and x2 prescaler enable by buf[bit 16]
*/
baseline = 12000000 * 32;
mantissa = baseline / baud;
if (mantissa == 0)
mantissa = 1; /* Avoid dividing by zero if baud > 32*12M. */
exponent = 0;
if (spriv->quirks & PL2303_QUIRK_DIVISOR_TA) {
while (mantissa >= 2048) {
// exponent is three bits (after shifting right)
if (exponent < 15) { // we are going to divide this by 2 later
mantissa >>= 1; // divide by 2
exponent++; // currently log2 ... will become log4
} else {
/* Exponent is maxed. Trim mantissa and leave. */
mantissa = 2047 ;
break;
}
}
buf[2] = exponent & 0x01; // activate x2 prescaler if needed
exponent >>= 1; // now log base 4 (losing LSB)
buf[1] = (exponent << 5) | (mantissa >> 8);
Again, this is really nice work.
But it seems to me that we should simply think about the encoding as
using base-2 with the LSB of the exponent in bit 16. That should make it
easier to follow what's going on here.
I've been thinking about ways of merging the two schemes (both using
base 2), and I even checked if my HXD happened to have a 2-prescaler bit
somewhere as well but I couldn't find one.
It's probably not worth it at this point (and may not end up being more
readable anyway) so I therefore suggest adding a separate function for
the alternate scheme for now. You can just call it at the start of the
"default" function:
if (spriv->type->alt_divisors)
return pl2303_encode_baud_rate_divisor_alt(buf, baud);
From: Michael G. Katzmann <hidden> Date: 2021-03-12 13:19:12
On 3/11/21 11:08 AM, Johan Hovold wrote:
I think we should just a new flag for the alternate divisor encoding
named "alt_divisors" (cf. no_divisors).
Chances are this encoding is used for other types as well.
quoted
},
};
static int pl2303_startup(struct usb_serial *serial)
{
....
if ( serial->dev->descriptor.bcdDevice == 0x0300 && serial->dev->descriptor.bcdUSB == 0x0200 )
type = TYPE_TA;
This needs to go after the bDeviceClass == 0x02 check, and the
descriptor fields need to be accessed using le16_to_cpu().
I've prepared a patch series that clean up and tighten the type
detection which I suggest you built upon instead.
I'll post the series after replying here.
quoted
else if (serial->dev->descriptor.bDeviceClass == 0x02)
....
}
static speed_t pl2303_encode_baud_rate_divisor( struct usb_serial_port *port,
unsigned char buf[4],
speed_t baud)
{
unsigned int baseline, mantissa, exponent;
struct usb_serial *serial = port->serial;
struct pl2303_serial_private *spriv = usb_get_serial_data(serial);
/*
* Apparently the formula is:
* baudrate = 12M * 32 / (mantissa * 4^exponent)
* where
* mantissa = buf[8:0]
* exponent = buf[11:9]
*
* TA version has more precision
* uses mantissa = buf[bits 10:0 ]
So you discovered that there were even more bits here? Your first
version used ten bits, I believe.
I got an offline mail from a third person having problems with the TA
and who had also verified eleven bits here.
I was basing this on Joe's discovery of the value used for 110 bd by the windows driver (confirmed by Charles). The sequence 80 01 a6 a8 implies that the mantissa is 0x6a8 (i.e. 11 bits). The tests that I did seemed to confirm this.
quoted
* exponent = buf[bits 15:13]
* and x2 prescaler enable by buf[bit 16]
*/
baseline = 12000000 * 32;
mantissa = baseline / baud;
if (mantissa == 0)
mantissa = 1; /* Avoid dividing by zero if baud > 32*12M. */
exponent = 0;
if (spriv->quirks & PL2303_QUIRK_DIVISOR_TA) {
while (mantissa >= 2048) {
// exponent is three bits (after shifting right)
if (exponent < 15) { // we are going to divide this by 2 later
mantissa >>= 1; // divide by 2
exponent++; // currently log2 ... will become log4
} else {
/* Exponent is maxed. Trim mantissa and leave. */
mantissa = 2047 ;
break;
}
}
buf[2] = exponent & 0x01; // activate x2 prescaler if needed
exponent >>= 1; // now log base 4 (losing LSB)
buf[1] = (exponent << 5) | (mantissa >> 8);
Again, this is really nice work.
But it seems to me that we should simply think about the encoding as
using base-2 with the LSB of the exponent in bit 16. That should make it
easier to follow what's going on here.
I quite agree. It would be cleaner.
I've been thinking about ways of merging the two schemes (both using
base 2), and I even checked if my HXD happened to have a 2-prescaler bit
somewhere as well but I couldn't find one.
It's probably not worth it at this point (and may not end up being more
readable anyway) so I therefore suggest adding a separate function for
the alternate scheme for now. You can just call it at the start of the
"default" function:
if (spriv->type->alt_divisors)
return pl2303_encode_baud_rate_divisor_alt(buf, baud);
Pardon my ignorance of the process but where is the git repo for this development branch?
Michael
From: Johan Hovold <johan@kernel.org> Date: 2021-03-12 13:44:46
On Fri, Mar 12, 2021 at 08:17:55AM -0500, Michael G. Katzmann wrote:
On 3/11/21 11:08 AM, Johan Hovold wrote:
quoted
quoted
static speed_t pl2303_encode_baud_rate_divisor( struct usb_serial_port *port,
unsigned char buf[4],
speed_t baud)
{
unsigned int baseline, mantissa, exponent;
struct usb_serial *serial = port->serial;
struct pl2303_serial_private *spriv = usb_get_serial_data(serial);
/*
* Apparently the formula is:
* baudrate = 12M * 32 / (mantissa * 4^exponent)
* where
* mantissa = buf[8:0]
* exponent = buf[11:9]
*
* TA version has more precision
* uses mantissa = buf[bits 10:0 ]
So you discovered that there were even more bits here? Your first
version used ten bits, I believe.
I got an offline mail from a third person having problems with the TA
and who had also verified eleven bits here.
I was basing this on Joe's discovery of the value used for 110 bd by
the windows driver (confirmed by Charles). The sequence 80 01 a6 a8
implies that the mantissa is 0x6a8 (i.e. 11 bits). The tests that I
did seemed to confirm this.
Ah, of course.
Pardon my ignorance of the process but where is the git repo for this
development branch?
From: Michael G. Katzmann <hidden> Date: 2021-03-12 20:30:20
On 3/12/21 8:44 AM, Johan Hovold wrote:
See diff to your branch below.
Unless we do what I originally did and pass 'port' to pl2303_encode_baud_rate_divisor we cannot test for 'alt_divisor there.
I did this test in pl2303_encode_baud_rate instead so it looks like ...
if (baud == baud_sup)
baud = pl2303_encode_baud_rate_direct(buf, baud);
else if (spriv->type->alt_divisors)
baud = pl2303_encode_baud_rate_divisor_alt(buf, baud);
else
baud = pl2303_encode_baud_rate_divisor(buf, baud);
Michael
191d190
< unsigned int alt_divisors:1;
221d219
< .alt_divisors = true,
226d223
< .alt_divisors = true,
625,664d620
< static speed_t pl2303_encode_baud_rate_divisor_alt(unsigned char buf[4],
< speed_t baud)
< {
< unsigned int baseline, mantissa, exponent;
<
< /*
< * Apparently, for the TA version the formula is:
< * baudrate = 12M * 32 / (mantissa * 2^exponent)
< * where
< * mantissa = buf[10:0]
< * exponent = buf[15:13 16]
< */
< baseline = 12000000 * 32;
< mantissa = baseline / baud;
< if (mantissa == 0)
< mantissa = 1; /* Avoid dividing by zero if baud > 32*12M. */
< exponent = 0;
< while (mantissa >= 2048) {
< if (exponent < 15) {
< mantissa >>= 1; /* divide by 2 */
< exponent++;
< } else {
< /* Exponent is maxed. Trim mantissa and leave. */
< mantissa = 2047;
< break;
< }
< }
<
< buf[3] = 0x80;
< buf[2] = exponent & 0x01; // LS bit of exponent
< buf[1] = (exponent & ~0x01) << 4 | mantissa >> 8; // 3 bits of the exponent and MS 3 bits of the mantissa
< buf[0] = mantissa & 0xff; // LS 8 bits of the mantissa
<
< /* Calculate and return the exact baud rate. */
< baud = (baseline / mantissa) >> exponent;
<
< return baud;
< }
<
<
692,693d647
< else if (spriv->type->alt_divisors)
< baud = pl2303_encode_baud_rate_divisor_alt(buf, baud);
From: Johan Hovold <johan@kernel.org> Date: 2021-03-15 09:07:56
On Fri, Mar 12, 2021 at 08:28:04PM -0500, Michael G. Katzmann wrote:
On 3/12/21 8:44 AM, Johan Hovold wrote:
Let me try that patch in the right format 8-)
That's better, but we also need a commit message and and Signed-off-by
line from you. Take a look at the general format of the patches I CCed
you on and there's some more information under Documentation/:
Documentation/process/submitting-patches.rst
The easiest is probably to use git-format-patch and git-send-email to
prevent the patch from being corrupted (e.g. the tabs below have been
replaced by spaces).
Are you sure that the TB uses the alternate encoding?
Charles, could you help us out here? Which other device types use the
alternate encoding (e.g. HX(A) or TB) if any?
quoted hunk
},
[TYPE_HXD] = {
.name = "HXD",
@@ -618,6 +621,46 @@
return baud;
}
+static speed_t pl2303_encode_baud_rate_divisor_alt(unsigned char buf[4],
+ speed_t baud)
+{
+ unsigned int baseline, mantissa, exponent;
+
+ /*
+ * Apparently, for the TA version the formula is:
+ * baudrate = 12M * 32 / (mantissa * 2^exponent)
+ * where
+ * mantissa = buf[10:0]
+ * exponent = buf[15:13 16]
+ */
+ baseline = 12000000 * 32;
+ mantissa = baseline / baud;
+ if (mantissa == 0)
+ mantissa = 1; /* Avoid dividing by zero if baud > 32*12M. */
+ exponent = 0;
+ while (mantissa >= 2048) {
+ if (exponent < 15) {
+ mantissa >>= 1; /* divide by 2 */
+ exponent++;
+ } else {
+ /* Exponent is maxed. Trim mantissa and leave. */
+ mantissa = 2047;
+ break;
+ }
+ }
+
+ buf[3] = 0x80;
+ buf[2] = exponent & 0x01; // LS bit of exponent
+ buf[1] = (exponent & ~0x01) << 4 | mantissa >> 8; // 3 bits of the exponent and MS 3 bits of the mantissa
+ buf[0] = mantissa & 0xff; // LS 8 bits of the mantissa
Please avoid c99-style comments, but perhaps the comment at the start of
function is sufficient here.
Looks great otherwise.
This works too if you don't want to pass in the port to
pl2303_encode_baud_rate_divisor() and hide the device-type differences
there (e.g. to make the logic in pl2303_encode_baud_rate() easier to
follow).
Johan
From: Johan Hovold <johan@kernel.org> Date: 2021-04-08 15:35:33
Hi Michael,
On Thu, Feb 25, 2021 at 09:42:20AM +0100, Johan Hovold wrote:
On Wed, Feb 24, 2021 at 01:13:39PM -0500, Michael G. Katzmann wrote:
quoted
On 2/24/21 12:04 PM, Johan Hovold wrote:
quoted
Perhaps you can even figure out how to poll for an empty TX FIFO from
it, unless Charles is able to provide some details on that separate
matter?
I presume from the code below, that when the device is closed, all
data waiting to send is clobbered (if so, so the problem is the driver
and not the device)
I would have thought that the driver should drain the buffers. I can
see that this might be a problem if there is flow control (it may
never drain) but the current method seems pretty brutal.
We do; the code below isn't called until after we've waited for the
buffers to drain (driver buffers + device FIFO).
I'll provide a patch so that you can extend the timeout for draining the
driver buffers (defaults to 30 s), but the main problem is that we don't
know how to query the PL2303 FIFO fill level.
I've added generic support to USB serial for setting the closing_wait
parameter through TIOCSSERIAL (e.g. setserial) so that you can change
the default 30 second timeout also with pl2303:
https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git/commit/?h=usb-next&id=01fd45f676f1b3785b7cdd5d815f9c31ddcd9dd1
With the 4k driver buffer and two bulk-out URBs with 256 bytes of data
each you need to set the timeout to something like 420 seconds at 110
bps to allow those buffers to drain (when not using flow control).
On top of that there's the 256 byte device FIFO, which we not yet know
how to query. At 110 bps that one takes about 23 seconds to drain, but
as I mentioned elsewhere we cap the time-based delay at 2 seconds
currently.
Charles, is there a way to check if the device transmit FIFO has
emptied?
Johan