Skip to contentSkip to conversation

Add Proliic new chip: PL2303TB & PL2303N(G)

DISCUSSION Johan Hovold 1 participant 1 message Dec 18, 2018 USB

lore
Share & tools
lore
#0 lore raw
Quoted 6 lines · Charles Yeh

From b9fd71c64d4d0d939a7a27e08a74d81f960ff5ea Mon Sep 17 00:00:00 2001
From: Charles Yeh <[email protected]>
Date: Sat, 15 Dec 2018 07:10:17 +0800
Subject: [PATCH] Add Proliic new chip: PL2303TB & PL2303N(G)

Add new PID to support PL2303TB

Is this also an HXN-type device?

Quoted 14 lines

Add mew PID to support PL2303(N)GC/GB/GS/GT/GL/GE
Add new request to support PL2303N(G)

---
 drivers/usb/serial/pl2303.c | 106 +++++++++++++++++++++++++++++-------
 drivers/usb/serial/pl2303.h |  11 ++++
 2 files changed, 97 insertions(+), 20 deletions(-)
drivers/usb/serial/pl2303.c +0 -0
index a4e0d13fc121..0001b527f07f 100644

drivers/usb/serial/pl2303.c line 35

Quoted 5 lines
drivers/usb/serial/pl2303.c +2 -0
@@ -31,6 +31,8 @@
 #define PL2303_QUIRK_UART_STATE_IDX0        BIT(0)
 #define PL2303_QUIRK_LEGACY            BIT(1)
 #define PL2303_QUIRK_ENDPOINT_HACK        BIT(2)
+#define PL2303_QUIRK_LEGACY_HX            BIT(3)    /* old IC type */
+#define PL2303_QUIRK_LEGACY_N            BIT(4)    /* new IC type */

I don't there's any need to add new "quirk" flags for these types. Just use the type stored in the interface private data directly (struct pl2303_serial_private).

Quoted 51 lines
drivers/usb/serial/pl2303.c +15 -0
 static const struct usb_device_id id_table[] = {
     { USB_DEVICE(PL2303_VENDOR_ID, PL2303_PRODUCT_ID),
@@ -46,6 +48,13 @@ static const struct usb_device_id id_table[] = {
     { USB_DEVICE(PL2303_VENDOR_ID, PL2303_PRODUCT_ID_HCR331) },
     { USB_DEVICE(PL2303_VENDOR_ID, PL2303_PRODUCT_ID_MOTOROLA) },
     { USB_DEVICE(PL2303_VENDOR_ID, PL2303_PRODUCT_ID_ZTEK) },
+    { USB_DEVICE(PL2303_VENDOR_ID, PL2303_PRODUCT_ID_TB) },
+    { USB_DEVICE(PL2303_VENDOR_ID, PL2303G_PRODUCT_ID_GC) },
+    { USB_DEVICE(PL2303_VENDOR_ID, PL2303G_PRODUCT_ID_GB) },
+    { USB_DEVICE(PL2303_VENDOR_ID, PL2303G_PRODUCT_ID_GT) },
+    { USB_DEVICE(PL2303_VENDOR_ID, PL2303G_PRODUCT_ID_GL) },
+    { USB_DEVICE(PL2303_VENDOR_ID, PL2303G_PRODUCT_ID_GE) },
+    { USB_DEVICE(PL2303_VENDOR_ID, PL2303G_PRODUCT_ID_GS) },
     { USB_DEVICE(IODATA_VENDOR_ID, IODATA_PRODUCT_ID) },
     { USB_DEVICE(IODATA_VENDOR_ID, IODATA_PRODUCT_ID_RSAQ5) },
     { USB_DEVICE(ATEN_VENDOR_ID, ATEN_PRODUCT_ID),
@@ -123,9 +132,11 @@ MODULE_DEVICE_TABLE(usb, id_table);
 
 #define VENDOR_WRITE_REQUEST_TYPE    0x40
 #define VENDOR_WRITE_REQUEST        0x01
+#define VENDOR_WRITE_NREQUEST        0x80
 
 #define VENDOR_READ_REQUEST_TYPE    0xc0
 #define VENDOR_READ_REQUEST        0x01
+#define VENDOR_READ_NREQUEST        0x81
 
 #define UART_STATE_INDEX        8
 #define UART_STATE_MSR_MASK        0x8b
@@ -144,6 +155,7 @@ static void pl2303_set_break(struct
 usb_serial_port *port, bool enable);
 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_COUNT
 };
@@ -172,6 +184,11 @@ static const struct pl2303_type_data
 pl2303_type_data[TYPE_COUNT] = {
     },
     [TYPE_HX] = {
         .max_baud_rate =    12000000,
+        .quirks =        PL2303_QUIRK_LEGACY_HX,    /* old chip type */
+    },
+    [TYPE_HXN] = {
+        .max_baud_rate =    12000000,
+        .quirks =        PL2303_QUIRK_LEGACY_N,    /* New chip type */
     },
 };

drivers/usb/serial/pl2303.c static int pl2303_vendor_read(struct usb_serial · line 202

Quoted 7 lines
drivers/usb/serial/pl2303.c +2 -0
@@ -179,10 +196,16 @@ static int pl2303_vendor_read(struct usb_serial
 *serial, u16 value,
                             unsigned char buf[1])
 {
     struct device *dev = &serial->interface->dev;
+    struct pl2303_serial_private *spriv = usb_get_serial_data(serial);
     int res;
+    __u8 request;

This should be u8.

drivers/usb/serial/pl2303.c static int pl2303_vendor_read(struct usb_serial · line 204

Quoted 2 lines
drivers/usb/serial/pl2303.c +2 -0
+
+    /* old / new  read request ?*/

You can just drop the comment.

drivers/usb/serial/pl2303.c static int pl2303_vendor_read(struct usb_serial · line 206

Quoted 2 lines
drivers/usb/serial/pl2303.c +2 -0
+    if (spriv->quirks & PL2303_QUIRK_LEGACY_N) request= VENDOR_READ_NREQUEST;
+    else request= VENDOR_READ_REQUEST;

But please fix up the indentation of this (as Greg already pointed out).

Quoted 8 lines
drivers/usb/serial/pl2303.c +1 -1
     res = usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, 0),
-            VENDOR_READ_REQUEST, VENDOR_READ_REQUEST_TYPE,
+            request, VENDOR_READ_REQUEST_TYPE,
             value, 0, buf, 1, 100);
     if (res != 1) {
         dev_err(dev, "%s - failed to read [%04x]: %d\n", __func__,

drivers/usb/serial/pl2303.c static int pl2303_vendor_read(struct usb_serial · line 230

Quoted 7 lines
drivers/usb/serial/pl2303.c +2 -0
@@ -201,12 +224,18 @@ static int pl2303_vendor_read(struct usb_serial
 *serial, u16 value,
 static int pl2303_vendor_write(struct usb_serial *serial, u16 value, u16 index)
 {
     struct device *dev = &serial->interface->dev;
+    struct pl2303_serial_private *spriv = usb_get_serial_data(serial);
     int res;
+    __u8 request;

u8

drivers/usb/serial/pl2303.c static int pl2303_vendor_read(struct usb_serial · line 235

Quoted 6 lines
drivers/usb/serial/pl2303.c +3 -0
     dev_dbg(dev, "%s - [%04x] = %02x\n", __func__, value, index);
 
+    /* old / new  write request ? */
+    if (spriv->quirks & PL2303_QUIRK_LEGACY_N) request= VENDOR_WRITE_NREQUEST;
+    else request= VENDOR_WRITE_REQUEST;

Same as above.

Quoted 16 lines
drivers/usb/serial/pl2303.c +3 -1
+
     res = usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev, 0),
-            VENDOR_WRITE_REQUEST, VENDOR_WRITE_REQUEST_TYPE,
+            request, VENDOR_WRITE_REQUEST_TYPE,
             value, index, NULL, 0, 100);
     if (res) {
         dev_err(dev, "%s - failed to write [%04x]: %d\n", __func__,
@@ -286,6 +315,7 @@ static int pl2303_startup(struct usb_serial *serial)
     struct pl2303_serial_private *spriv;
     enum pl2303_type type = TYPE_01;
     unsigned char *buf;
+    int res;
 
     spriv = kzalloc(sizeof(*spriv), GFP_KERNEL);
     if (!spriv)

drivers/usb/serial/pl2303.c static int pl2303_startup(struct usb_serial *serial) · line 344

Quoted 8 lines
drivers/usb/serial/pl2303.c +5 -0
@@ -307,26 +337,38 @@ static int pl2303_startup(struct usb_serial *serial)
         type = TYPE_01;        /* type 1 */
     dev_dbg(&serial->interface->dev, "device type: %d\n", type);
 
+    /* new chip ? */
+    if(serial->dev->descriptor.bcdUSB == 0x0200) {
+        res = usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, 0),
+            VENDOR_READ_REQUEST, VENDOR_READ_REQUEST_TYPE,
+            0x8484, 0, buf, 1, 100);

What are you reading here? What is 0x8484?

Quoted 45 lines
drivers/usb/serial/pl2303.c +21 -14
+        if (res != 1) {
+            type = TYPE_HXN;    /* type 2 */
+        }
+    }
+
     spriv->type = &pl2303_type_data[type];
     spriv->quirks = (unsigned long)usb_get_serial_data(serial);
     spriv->quirks |= spriv->type->quirks;
 
     usb_set_serial_data(serial, spriv);
 
-    pl2303_vendor_read(serial, 0x8484, buf);
-    pl2303_vendor_write(serial, 0x0404, 0);
-    pl2303_vendor_read(serial, 0x8484, buf);
-    pl2303_vendor_read(serial, 0x8383, buf);
-    pl2303_vendor_read(serial, 0x8484, buf);
-    pl2303_vendor_write(serial, 0x0404, 1);
-    pl2303_vendor_read(serial, 0x8484, buf);
-    pl2303_vendor_read(serial, 0x8383, buf);
-    pl2303_vendor_write(serial, 0, 1);
-    pl2303_vendor_write(serial, 1, 0);
-    if (spriv->quirks & PL2303_QUIRK_LEGACY)
-        pl2303_vendor_write(serial, 2, 0x24);
-    else
-        pl2303_vendor_write(serial, 2, 0x44);
+    if(type != TYPE_HXN) {
+        pl2303_vendor_read(serial, 0x8484, buf);
+        pl2303_vendor_write(serial, 0x0404, 0);
+        pl2303_vendor_read(serial, 0x8484, buf);
+        pl2303_vendor_read(serial, 0x8383, buf);
+        pl2303_vendor_read(serial, 0x8484, buf);
+        pl2303_vendor_write(serial, 0x0404, 1);
+        pl2303_vendor_read(serial, 0x8484, buf);
+        pl2303_vendor_read(serial, 0x8383, buf);
+        pl2303_vendor_write(serial, 0, 1);
+        pl2303_vendor_write(serial, 1, 0);
+        if (spriv->quirks & PL2303_QUIRK_LEGACY)
+            pl2303_vendor_write(serial, 2, 0x24);
+        else
+            pl2303_vendor_write(serial, 2, 0x44);
+    }
 
     kfree(buf);
 

drivers/usb/serial/pl2303.c static void pl2303_set_termios(struct tty_struct *tty, · line 719

Quoted 5 lines
drivers/usb/serial/pl2303.c +2 -0
@@ -673,13 +715,33 @@ static void pl2303_set_termios(struct tty_struct *tty,
     if (C_CRTSCTS(tty)) {
         if (spriv->quirks & PL2303_QUIRK_LEGACY)
             pl2303_vendor_write(serial, 0x0, 0x41);
+        else if(spriv->quirks & PL2303_QUIRK_LEGACY_N)
+            pl2303_vendor_write(serial, 0x0A, 0xFA);    /* New chip */

What is 0x0a and 0xfa? Please use defines rather than "magical" constants.

drivers/usb/serial/pl2303.c static void pl2303_set_termios(struct tty_struct *tty, · line 725

Quoted 7 lines
drivers/usb/serial/pl2303.c +2 -1
         else
             pl2303_vendor_write(serial, 0x0, 0x61);
     } else if (I_IXON(tty) && !I_IXANY(tty) && START_CHAR(tty) == 0x11 &&
             STOP_CHAR(tty) == 0x13) {
-        pl2303_vendor_write(serial, 0x0, 0xc0);
+        if(spriv->quirks & PL2303_QUIRK_LEGACY_N)
+            pl2303_vendor_write(serial, 0x0A, 0xEE);    /* New chip */

Same here.

Quoted 7 lines
drivers/usb/serial/pl2303.c +5 -1
+        else
+            pl2303_vendor_write(serial, 0x0, 0xc0);
     } else {
-        pl2303_vendor_write(serial, 0x0, 0x0);
+        if(spriv->quirks & PL2303_QUIRK_LEGACY_N)
+            pl2303_vendor_write(serial, 0x0A, 0xFF);    /* New chip */
+        else

drivers/usb/serial/pl2303.c static void pl2303_set_termios(struct tty_struct *tty, · line 741

Quoted 10 lines
drivers/usb/serial/pl2303.c +10 -0
+            pl2303_vendor_write(serial, 0x0, 0x0);
+    }
+
+    /* Old chip, External Pull-Up Mode */
+    if(spriv->quirks & PL2303_QUIRK_LEGACY_HX){
+        pl2303_vendor_read(serial, 0x8484, buf);
+        pl2303_vendor_write(serial, 0x0404, 0x09);
+        pl2303_vendor_read(serial, 0x8484, buf);
+        pl2303_vendor_read(serial, 0x8383, buf);
+        if((u16)*buf & 0x08){

This looks odd as pl2303_vendor_read() only reads one byte.

drivers/usb/serial/pl2303.c static void pl2303_set_termios(struct tty_struct *tty, · line 744

Quoted 3 lines
drivers/usb/serial/pl2303.c +3 -0
+            pl2303_vendor_write(serial, 0x0, 0x31);
+            pl2303_vendor_write(serial, 0x1, 0x01);
+        }

And this looks like an unrelated change since it affects older devices and not HXN.

Please split out in a separate patch and explain why it is needed. Also replace the "magical" constants with descriptive defines.

drivers/usb/serial/pl2303.c static int pl2303_open(struct tty_struct *tty, · line 787

Quoted 12 lines
drivers/usb/serial/pl2303.c +2 -2
     }
 
     kfree(buf);
@@ -720,8 +782,12 @@ static int pl2303_open(struct tty_struct *tty,
 struct usb_serial_port *port)
         usb_clear_halt(serial->dev, port->read_urb->pipe);
     } else {
         /* reset upstream data pipes */
-        pl2303_vendor_write(serial, 8, 0);
-        pl2303_vendor_write(serial, 9, 0);
+        if(spriv->quirks & PL2303_QUIRK_LEGACY_N)
+            pl2303_vendor_write(serial, 7, 0);    /* reset new chip */

Please add a define for 7 here too.

drivers/usb/serial/pl2303.c static int pl2303_open(struct tty_struct *tty, · line 792

Quoted 5 lines
drivers/usb/serial/pl2303.c +4 -0
+        else {
+            pl2303_vendor_write(serial, 8, 0);
+            pl2303_vendor_write(serial, 9, 0);
+        }
     }

Thanks,
Johan