* Add Proliic new chip: PL2303TB & PL2303N(G)
@ 2018-12-17 4:51 Charles Yeh
0 siblings, 0 replies; 9+ messages in thread
From: Charles Yeh @ 2018-12-17 4:51 UTC (permalink / raw)
To: gregkh; +Cc: johan, charles-yeh, linux-usb
From b9fd71c64d4d0d939a7a27e08a74d81f960ff5ea Mon Sep 17 00:00:00 2001
From: Charles Yeh <charlesyeh522@gmail.com>
Date: Sat, 15 Dec 2018 07:10:17 +0800
Subject: [PATCH] Add Proliic new chip: PL2303TB & PL2303N(G)
Add new PID to support PL2303TB
Add mew PID to support PL2303(N)GC/GB/GS/GT/GL/GE
Add new request to support PL2303N(G)
Signed-off-by: Charles Yeh <charlesyeh522@gmail.com>
---
drivers/usb/serial/pl2303.c | 106 +++++++++++++++++++++++++++++-------
drivers/usb/serial/pl2303.h | 11 ++++
2 files changed, 97 insertions(+), 20 deletions(-)
diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c
index a4e0d13fc121..0001b527f07f 100644
--- a/drivers/usb/serial/pl2303.c
+++ b/drivers/usb/serial/pl2303.c
@@ -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 */
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 */
},
};
@@ -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;
+
+ /* old / new read request ?*/
+ if (spriv->quirks & PL2303_QUIRK_LEGACY_N) request= VENDOR_READ_NREQUEST;
+ else request= VENDOR_READ_REQUEST;
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__,
@@ -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;
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;
+
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)
@@ -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);
+ 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);
@@ -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 */
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 */
+ 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
+ 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){
+ pl2303_vendor_write(serial, 0x0, 0x31);
+ pl2303_vendor_write(serial, 0x1, 0x01);
+ }
}
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 */
+ else {
+ pl2303_vendor_write(serial, 8, 0);
+ pl2303_vendor_write(serial, 9, 0);
+ }
}
/* Setup termios */
diff --git a/drivers/usb/serial/pl2303.h b/drivers/usb/serial/pl2303.h
index 26965cc23c17..686f1cfca564 100644
--- a/drivers/usb/serial/pl2303.h
+++ b/drivers/usb/serial/pl2303.h
@@ -20,6 +20,17 @@
#define PL2303_PRODUCT_ID_MOTOROLA 0x0307
#define PL2303_PRODUCT_ID_ZTEK 0xe1f1
+/* PL2303TB */
+#define PL2303_PRODUCT_ID_TB 0x2304
+
+/* PL2303N */
+#define PL2303G_PRODUCT_ID_GC 0x23A3
+#define PL2303G_PRODUCT_ID_GB 0x23B3
+#define PL2303G_PRODUCT_ID_GT 0x23C3
+#define PL2303G_PRODUCT_ID_GL 0x23D3
+#define PL2303G_PRODUCT_ID_GE 0x23E3
+#define PL2303G_PRODUCT_ID_GS 0x23F3
+
#define ATEN_VENDOR_ID 0x0557
#define ATEN_VENDOR_ID2 0x0547
#define ATEN_PRODUCT_ID 0x2008
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Add Proliic new chip: PL2303TB & PL2303N(G)
@ 2018-12-17 13:03 Greg Kroah-Hartman
0 siblings, 0 replies; 9+ messages in thread
From: Greg Kroah-Hartman @ 2018-12-17 13:03 UTC (permalink / raw)
To: Charles Yeh; +Cc: johan, charles-yeh, linux-usb
On Mon, Dec 17, 2018 at 12:51:24PM +0800, Charles Yeh wrote:
> >From b9fd71c64d4d0d939a7a27e08a74d81f960ff5ea Mon Sep 17 00:00:00 2001
> From: Charles Yeh <charlesyeh522@gmail.com>
> Date: Sat, 15 Dec 2018 07:10:17 +0800
> Subject: [PATCH] Add Proliic new chip: PL2303TB & PL2303N(G)
Much better, but why is this all in the body of the patch? It doesn't
need to be here.
>
> Add new PID to support PL2303TB
> Add mew PID to support PL2303(N)GC/GB/GS/GT/GL/GE
> Add new request to support PL2303N(G)
>
> Signed-off-by: Charles Yeh <charlesyeh522@gmail.com>
Only one space after the : is needed.
> ---
> drivers/usb/serial/pl2303.c | 106 +++++++++++++++++++++++++++++-------
> drivers/usb/serial/pl2303.h | 11 ++++
> 2 files changed, 97 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c
> index a4e0d13fc121..0001b527f07f 100644
> --- a/drivers/usb/serial/pl2303.c
> +++ b/drivers/usb/serial/pl2303.c
> @@ -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 */
>
> 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) },
Tabs all got turned into spaces here, making the patch impossible to
apply.
Can you just use 'git send-email' to send the patch? That should
preserve it.
> + /* old / new write request ? */
> + if (spriv->quirks & PL2303_QUIRK_LEGACY_N) request= VENDOR_WRITE_NREQUEST;
> + else request= VENDOR_WRITE_REQUEST;
scripts/checkpatch.pl should have complained about this style. It
should look like:
if (spriv->quirks & PL2303_QUIRK_LEGACY_N)
request = VENDOR_WRITE_NREQUEST;
else
request = VENDOR_WRITE_REQUEST;
> + /* 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);
> + if (res != 1) {
> + type = TYPE_HXN; /* type 2 */
> + }
> + }
> +
Tiny style issues with those lines as well. Run your patch through
scripts/checkpatch.pl to catch all of these before resending it.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 9+ messages in thread
* Add Proliic new chip: PL2303TB & PL2303N(G)
@ 2018-12-18 10:02 Johan Hovold
0 siblings, 0 replies; 9+ messages in thread
From: Johan Hovold @ 2018-12-18 10:02 UTC (permalink / raw)
To: Charles Yeh; +Cc: gregkh, johan, charles-yeh, linux-usb
On Mon, Dec 17, 2018 at 12:51:24PM +0800, Charles Yeh wrote:
> From b9fd71c64d4d0d939a7a27e08a74d81f960ff5ea Mon Sep 17 00:00:00 2001
> From: Charles Yeh <charlesyeh522@gmail.com>
> 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?
> Add mew PID to support PL2303(N)GC/GB/GS/GT/GL/GE
> Add new request to support PL2303N(G)
>
> Signed-off-by: Charles Yeh <charlesyeh522@gmail.com>
> ---
> drivers/usb/serial/pl2303.c | 106 +++++++++++++++++++++++++++++-------
> drivers/usb/serial/pl2303.h | 11 ++++
> 2 files changed, 97 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c
> index a4e0d13fc121..0001b527f07f 100644
> --- a/drivers/usb/serial/pl2303.c
> +++ b/drivers/usb/serial/pl2303.c
> @@ -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).
> 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 */
> },
> };
>
> @@ -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.
> +
> + /* old / new read request ?*/
You can just drop the comment.
> + 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).
>
> 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__,
> @@ -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
>
> 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.
> +
> 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)
> @@ -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?
> + 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);
>
> @@ -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.
> 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.
> + 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
> + 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.
> + 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.
> }
>
> 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.
> + else {
> + pl2303_vendor_write(serial, 8, 0);
> + pl2303_vendor_write(serial, 9, 0);
> + }
> }
Thanks,
Johan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Add Proliic new chip: PL2303TB & PL2303N(G)
@ 2018-12-19 10:26 Charles Yeh
0 siblings, 0 replies; 9+ messages in thread
From: Charles Yeh @ 2018-12-19 10:26 UTC (permalink / raw)
To: Johan Hovold; +Cc: gregkh, charles-yeh, linux-usb
Hi Johan & Greg,
Thansk for you check the patch file..
PL2303 is a general name on the market. In fact, PL2303 has the
following various ICs.
A type: PL2303H,
B type: PL2303XA, PL2303HXA, PL2303HXB, PL2303HXC, PL2303HXD,
PL2303EA, PL2303SA, PL2303RA, PL2303TA, PL2303TB
C type:PL2303HXN: PL2303GC, PL2303GS, PL2303GB, PL2303GT, PL2303GL,
PL2303GE. <<--used different PID, different VENDOR_REQUEST
"Please split out in a separate patch and explain why it is needed.
Alsoreplace the "magical" constants with descriptive defines."
I will push 3 patch file.
1. Support PL2303TB(old chip, B type), only add new VID_PID
2. Support PL2303HXD(old chip, B type), /* only support External
Pull-Up Mode on PL2303HXD chip*. /
3. Support PL2303HXN(new chip, C type), add new VID_PID, new
VENDOR_WRITE_REQUEST/ VENDOR_READ_REQUEST, new H/W, S/W flow control
setting.
I will use "sudo ./scripts/checkpatch.pl --file
drivers/usb/serial/pl2303.c" & use 'git send-email' to send the
patch( avoid Tabs all got turned into spaces)
Johan Hovold <johan@kernel.org> 於 2018年12月18日 週二 下午6:02寫道：
>
> On Mon, Dec 17, 2018 at 12:51:24PM +0800, Charles Yeh wrote:
> > From b9fd71c64d4d0d939a7a27e08a74d81f960ff5ea Mon Sep 17 00:00:00 2001
> > From: Charles Yeh <charlesyeh522@gmail.com>
> > 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?
>
> > Add mew PID to support PL2303(N)GC/GB/GS/GT/GL/GE
> > Add new request to support PL2303N(G)
> >
> > Signed-off-by: Charles Yeh <charlesyeh522@gmail.com>
> > ---
> > drivers/usb/serial/pl2303.c | 106 +++++++++++++++++++++++++++++-------
> > drivers/usb/serial/pl2303.h | 11 ++++
> > 2 files changed, 97 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c
> > index a4e0d13fc121..0001b527f07f 100644
> > --- a/drivers/usb/serial/pl2303.c
> > +++ b/drivers/usb/serial/pl2303.c
> > @@ -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).
>
> > 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 */
> > },
> > };
> >
> > @@ -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.
>
> > +
> > + /* old / new read request ?*/
>
> You can just drop the comment.
>
> > + 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).
>
> >
> > 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__,
> > @@ -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
>
> >
> > 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.
>
> > +
> > 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)
> > @@ -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?
>
> > + 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);
> >
> > @@ -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.
>
> > 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.
>
> > + 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
> > + 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.
>
> > + 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.
>
> > }
> >
> > 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.
>
> > + else {
> > + pl2303_vendor_write(serial, 8, 0);
> > + pl2303_vendor_write(serial, 9, 0);
> > + }
> > }
>
> Thanks,
> Johan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Add Proliic new chip: PL2303TB & PL2303N(G)
@ 2018-12-19 10:35 Greg Kroah-Hartman
0 siblings, 0 replies; 9+ messages in thread
From: Greg Kroah-Hartman @ 2018-12-19 10:35 UTC (permalink / raw)
To: Charles Yeh; +Cc: Johan Hovold, charles-yeh, linux-usb
On Wed, Dec 19, 2018 at 06:26:06PM +0800, Charles Yeh wrote:
> Hi Johan & Greg,
> Thansk for you check the patch file..
>
> PL2303 is a general name on the market. In fact, PL2303 has the
> following various ICs.
> A type: PL2303H,
> B type: PL2303XA, PL2303HXA, PL2303HXB, PL2303HXC, PL2303HXD,
> PL2303EA, PL2303SA, PL2303RA, PL2303TA, PL2303TB
> C type:PL2303HXN: PL2303GC, PL2303GS, PL2303GB, PL2303GT, PL2303GL,
> PL2303GE. <<--used different PID, different VENDOR_REQUEST
>
> "Please split out in a separate patch and explain why it is needed.
> Alsoreplace the "magical" constants with descriptive defines."
> I will push 3 patch file.
>
> 1. Support PL2303TB(old chip, B type), only add new VID_PID
> 2. Support PL2303HXD(old chip, B type), /* only support External
> Pull-Up Mode on PL2303HXD chip*. /
> 3. Support PL2303HXN(new chip, C type), add new VID_PID, new
> VENDOR_WRITE_REQUEST/ VENDOR_READ_REQUEST, new H/W, S/W flow control
> setting.
>
>
> I will use "sudo ./scripts/checkpatch.pl --file
> drivers/usb/serial/pl2303.c" & use 'git send-email' to send the
> patch( avoid Tabs all got turned into spaces)
No need to run 'sudu' for checkpatch.pl, and run it on the patches you
generate, not the whole file after you have modified it. That way it
will tell you what is wrong, if anything, with the individual patches
you have.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 9+ messages in thread
* Add Proliic new chip: PL2303TB & PL2303N(G)
@ 2018-12-26 12:28 Charles Yeh
0 siblings, 0 replies; 9+ messages in thread
From: Charles Yeh @ 2018-12-26 12:28 UTC (permalink / raw)
To: lkp; +Cc: kbuild-all, gregkh, johan, linux-usb, charles-yeh, Charles Yeh
Add new PID to support PL2303TB (TYPE_HX)
Add new PID to support PL2303(N)GC/GB/GS/GT/GL/GE (TYPE_HXN)
Add new Flow Control to support PL2303(N)GC/GB/GS/GT/GL/GE (TYPE_HXN)
Add new Pull-Up Mode to support PL2303HXD (TYPE_HX)
Fixed warning:restricted__le16 degrades to integer
Signed-off-by: Charles Yeh <charlesyeh522@gmail.com>
---
drivers/usb/serial/pl2303.c | 116 +++++++++++++++++++++++++++++-------
drivers/usb/serial/pl2303.h | 11 ++++
2 files changed, 106 insertions(+), 21 deletions(-)
diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c
index a4e0d13fc121..8c71612e1811 100644
--- a/drivers/usb/serial/pl2303.c
+++ b/drivers/usb/serial/pl2303.c
@@ -46,6 +46,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 +130,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
@@ -139,11 +148,21 @@ MODULE_DEVICE_TABLE(usb, id_table);
#define UART_OVERRUN_ERROR 0x40
#define UART_CTS 0x80
+#define TYPE_HX_READ_OTP_STATUS_REGISTER 0x8484
+#define TYPE_HX_EXTERNAL_PULLUP_MODE 0x08
+#define TYPE_HX_PULLUP_MODE_REG 0x09
+#define TYPE_HXN_UART_FLOWCONTROL 0x0A
+#define TYPE_HXN_HARDWAREFLOW 0xFA
+#define TYPE_HXN_SOFTWAREFLOW 0xEE
+#define TYPE_HXN_NOFLOW 0xFF
+#define TYPE_HXN_RESET_DOWN_UPSTREAM 0x07
+
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
};
@@ -173,16 +192,26 @@ static const struct pl2303_type_data pl2303_type_data[TYPE_COUNT] = {
[TYPE_HX] = {
.max_baud_rate = 12000000,
},
+ [TYPE_HXN] = {
+ .max_baud_rate = 12000000,
+ },
};
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;
+
+ if (spriv->type == &pl2303_type_data[TYPE_HXN])
+ request = VENDOR_READ_NREQUEST;
+ else
+ request = VENDOR_READ_REQUEST;
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__,
@@ -201,12 +230,19 @@ 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;
dev_dbg(dev, "%s - [%04x] = %02x\n", __func__, value, index);
+ if (spriv->type == &pl2303_type_data[TYPE_HXN])
+ request = VENDOR_WRITE_NREQUEST;
+ else
+ request = VENDOR_WRITE_REQUEST;
+
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 +322,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)
@@ -307,26 +344,36 @@ static int pl2303_startup(struct usb_serial *serial)
type = TYPE_01; /* type 1 */
dev_dbg(&serial->interface->dev, "device type: %d\n", type);
+ if (type == TYPE_HX) {
+ res = usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, 0),
+ VENDOR_READ_REQUEST, VENDOR_READ_REQUEST_TYPE,
+ TYPE_HX_READ_OTP_STATUS_REGISTER, 0, buf, 1, 100);
+ if (res != 1)
+ type = TYPE_HXN;
+ }
+
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);
@@ -671,15 +718,37 @@ static void pl2303_set_termios(struct tty_struct *tty,
}
if (C_CRTSCTS(tty)) {
- if (spriv->quirks & PL2303_QUIRK_LEGACY)
+ if (spriv->type == &pl2303_type_data[TYPE_01])
pl2303_vendor_write(serial, 0x0, 0x41);
+ else if (spriv->type == &pl2303_type_data[TYPE_HXN])
+ pl2303_vendor_write(serial, TYPE_HXN_UART_FLOWCONTROL,
+ TYPE_HXN_HARDWAREFLOW);
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->type == &pl2303_type_data[TYPE_HXN])
+ pl2303_vendor_write(serial, TYPE_HXN_UART_FLOWCONTROL,
+ TYPE_HXN_SOFTWAREFLOW);
+ else
+ pl2303_vendor_write(serial, 0x0, 0xc0);
} else {
- pl2303_vendor_write(serial, 0x0, 0x0);
+ if (spriv->type == &pl2303_type_data[TYPE_HXN])
+ pl2303_vendor_write(serial, TYPE_HXN_UART_FLOWCONTROL,
+ TYPE_HXN_NOFLOW);
+ else
+ pl2303_vendor_write(serial, 0x0, 0x0);
+ }
+
+ if (spriv->type == &pl2303_type_data[TYPE_HX]) {
+ pl2303_vendor_read(serial, 0x8484, buf);
+ pl2303_vendor_write(serial, 0x0404, TYPE_HX_PULLUP_MODE_REG);
+ pl2303_vendor_read(serial, 0x8484, buf);
+ pl2303_vendor_read(serial, 0x8383, buf);
+ if ((u16)*buf & TYPE_HX_EXTERNAL_PULLUP_MODE) {
+ pl2303_vendor_write(serial, 0x0, 0x31);
+ pl2303_vendor_write(serial, 0x1, 0x01);
+ }
}
kfree(buf);
@@ -720,8 +789,13 @@ 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->type == &pl2303_type_data[TYPE_HXN])
+ pl2303_vendor_write(serial,
+ TYPE_HXN_RESET_DOWN_UPSTREAM, 0);
+ else {
+ pl2303_vendor_write(serial, 8, 0);
+ pl2303_vendor_write(serial, 9, 0);
+ }
}
/* Setup termios */
diff --git a/drivers/usb/serial/pl2303.h b/drivers/usb/serial/pl2303.h
index 26965cc23c17..f7c500622f6b 100644
--- a/drivers/usb/serial/pl2303.h
+++ b/drivers/usb/serial/pl2303.h
@@ -20,6 +20,17 @@
#define PL2303_PRODUCT_ID_MOTOROLA 0x0307
#define PL2303_PRODUCT_ID_ZTEK 0xe1f1
+/* PL2303TB , TYPE_HX */
+#define PL2303_PRODUCT_ID_TB 0x2304
+
+/* PL2303HXN , TYPE_HXN */
+#define PL2303G_PRODUCT_ID_GC 0x23A3
+#define PL2303G_PRODUCT_ID_GB 0x23B3
+#define PL2303G_PRODUCT_ID_GT 0x23C3
+#define PL2303G_PRODUCT_ID_GL 0x23D3
+#define PL2303G_PRODUCT_ID_GE 0x23E3
+#define PL2303G_PRODUCT_ID_GS 0x23F3
+
#define ATEN_VENDOR_ID 0x0557
#define ATEN_VENDOR_ID2 0x0547
#define ATEN_PRODUCT_ID 0x2008
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Add Proliic new chip: PL2303TB & PL2303N(G)
@ 2019-01-08 14:16 Johan Hovold
0 siblings, 0 replies; 9+ messages in thread
From: Johan Hovold @ 2019-01-08 14:16 UTC (permalink / raw)
To: Charles Yeh; +Cc: Johan Hovold, gregkh, charles-yeh, linux-usb
On Wed, Dec 19, 2018 at 06:26:06PM +0800, Charles Yeh wrote:
> Hi Johan & Greg,
> Thansk for you check the patch file..
>
> PL2303 is a general name on the market. In fact, PL2303 has the
> following various ICs.
> A type: PL2303H,
> B type: PL2303XA, PL2303HXA, PL2303HXB, PL2303HXC, PL2303HXD,
> PL2303EA, PL2303SA, PL2303RA, PL2303TA, PL2303TB
> C type:PL2303HXN: PL2303GC, PL2303GS, PL2303GB, PL2303GT, PL2303GL,
> PL2303GE. <<--used different PID, different VENDOR_REQUEST
Thanks for that summary. Much appreciated.
Does what the current driver calls TYPE_01 correspond to your "A type"
above?
> "Please split out in a separate patch and explain why it is needed.
> Alsoreplace the "magical" constants with descriptive defines."
> I will push 3 patch file.
>
> 1. Support PL2303TB(old chip, B type), only add new VID_PID
> 2. Support PL2303HXD(old chip, B type), /* only support External
> Pull-Up Mode on PL2303HXD chip*. /
> 3. Support PL2303HXN(new chip, C type), add new VID_PID, new
> VENDOR_WRITE_REQUEST/ VENDOR_READ_REQUEST, new H/W, S/W flow control
> setting.
Sounds good, but looks like your latest revision of the patch still does
all of the above in one patch...
Thanks,
Johan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Add Proliic new chip: PL2303TB & PL2303N(G)
@ 2019-01-08 17:01 Johan Hovold
0 siblings, 0 replies; 9+ messages in thread
From: Johan Hovold @ 2019-01-08 17:01 UTC (permalink / raw)
To: Charles Yeh; +Cc: lkp, kbuild-all, gregkh, johan, linux-usb, charles-yeh
On Wed, Dec 26, 2018 at 08:28:10PM +0800, Charles Yeh wrote:
> Add new PID to support PL2303TB (TYPE_HX)
> Add new PID to support PL2303(N)GC/GB/GS/GT/GL/GE (TYPE_HXN)
> Add new Flow Control to support PL2303(N)GC/GB/GS/GT/GL/GE (TYPE_HXN)
> Add new Pull-Up Mode to support PL2303HXD (TYPE_HX)
So the above should go in three separate patches as we already discussed
(PL2303TB PID, TYPE_HXN support + PIDs, "pull-up mode").
> Fixed warning:restricted__le16 degrades to integer
When you update and resend a patch, it's good to include changelog
information like this, but please put it below the cut-off line (---)
below so that it doesn't end up in the commit log.
Also include a patch revision in the Subject of the mail. Let's call
this one v2, so next time use the following Subject prefix:
[PATCH v3]: USB: serial: pl2303: add ...
(also after breaking the current patch into a series of three or more
patches).
> Signed-off-by: Charles Yeh <charlesyeh522@gmail.com>
> ---
> drivers/usb/serial/pl2303.c | 116 +++++++++++++++++++++++++++++-------
> drivers/usb/serial/pl2303.h | 11 ++++
> 2 files changed, 106 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c
> index a4e0d13fc121..8c71612e1811 100644
> --- a/drivers/usb/serial/pl2303.c
> +++ b/drivers/usb/serial/pl2303.c
> @@ -46,6 +46,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 +130,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
> @@ -139,11 +148,21 @@ MODULE_DEVICE_TABLE(usb, id_table);
> #define UART_OVERRUN_ERROR 0x40
> #define UART_CTS 0x80
>
> +#define TYPE_HX_READ_OTP_STATUS_REGISTER 0x8484
> +#define TYPE_HX_EXTERNAL_PULLUP_MODE 0x08
> +#define TYPE_HX_PULLUP_MODE_REG 0x09
> +#define TYPE_HXN_UART_FLOWCONTROL 0x0A
> +#define TYPE_HXN_HARDWAREFLOW 0xFA
> +#define TYPE_HXN_SOFTWAREFLOW 0xEE
> +#define TYPE_HXN_NOFLOW 0xFF
> +#define TYPE_HXN_RESET_DOWN_UPSTREAM 0x07
> +
> 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
> };
>
> @@ -173,16 +192,26 @@ static const struct pl2303_type_data pl2303_type_data[TYPE_COUNT] = {
> [TYPE_HX] = {
> .max_baud_rate = 12000000,
> },
> + [TYPE_HXN] = {
> + .max_baud_rate = 12000000,
> + },
> };
>
> 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;
> +
> + if (spriv->type == &pl2303_type_data[TYPE_HXN])
> + request = VENDOR_READ_NREQUEST;
> + else
> + request = VENDOR_READ_REQUEST;
>
> 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__,
> @@ -201,12 +230,19 @@ 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;
>
> dev_dbg(dev, "%s - [%04x] = %02x\n", __func__, value, index);
>
> + if (spriv->type == &pl2303_type_data[TYPE_HXN])
> + request = VENDOR_WRITE_NREQUEST;
> + else
> + request = VENDOR_WRITE_REQUEST;
> +
> 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 +322,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)
> @@ -307,26 +344,36 @@ static int pl2303_startup(struct usb_serial *serial)
> type = TYPE_01; /* type 1 */
> dev_dbg(&serial->interface->dev, "device type: %d\n", type);
>
> + if (type == TYPE_HX) {
> + res = usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, 0),
> + VENDOR_READ_REQUEST, VENDOR_READ_REQUEST_TYPE,
> + TYPE_HX_READ_OTP_STATUS_REGISTER, 0, buf, 1, 100);
> + if (res != 1)
> + type = TYPE_HXN;
> + }
So HXN looks like an HX, but responds with an error when trying to read
any (?) register using VENDOR_READ_REQUEST? Isn't there any more direct
way of determining the device type?
My old HXD device report bcdUSB as 0x0110, and you tested you also
tested against 0x0200 in a previous version of this patch. How does
bcdUSB relate to the various types?
Note that you should use le16_to_cpu() to access bcdUSB safely also on
big-endian systems (which the kbuild test robot somewhat cryptically
reported).
> +
> 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);
> + }
Is this even needed for pre-HXN devices, or could perhaps some of it
just be removed? Can you explain what it does?
> kfree(buf);
>
> @@ -671,15 +718,37 @@ static void pl2303_set_termios(struct tty_struct *tty,
> }
>
> if (C_CRTSCTS(tty)) {
> - if (spriv->quirks & PL2303_QUIRK_LEGACY)
> + if (spriv->type == &pl2303_type_data[TYPE_01])
> pl2303_vendor_write(serial, 0x0, 0x41);
> + else if (spriv->type == &pl2303_type_data[TYPE_HXN])
> + pl2303_vendor_write(serial, TYPE_HXN_UART_FLOWCONTROL,
> + TYPE_HXN_HARDWAREFLOW);
> 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->type == &pl2303_type_data[TYPE_HXN])
> + pl2303_vendor_write(serial, TYPE_HXN_UART_FLOWCONTROL,
> + TYPE_HXN_SOFTWAREFLOW);
> + else
> + pl2303_vendor_write(serial, 0x0, 0xc0);
> } else {
> - pl2303_vendor_write(serial, 0x0, 0x0);
> + if (spriv->type == &pl2303_type_data[TYPE_HXN])
> + pl2303_vendor_write(serial, TYPE_HXN_UART_FLOWCONTROL,
> + TYPE_HXN_NOFLOW);
> + else
> + pl2303_vendor_write(serial, 0x0, 0x0);
> + }
So this is becoming a bit hard to read. The idea with the type struct
was to abstract the differences. We could add something like
u8 uart_flowctrl_reg;
u8 uart_flowctrl_hw;
u8 uart_flowctrl_sw;
u8 uart_flowctrl_none;
to struct pl2303_type_data, and replace the above with just three calls
to pl2303_vendor_write().
If you do this, then do this as a preparatory patch before adding HXN
support.
We should probably do something similar with the read and write requests
instead of adding conditionals in those paths.
> +
> + if (spriv->type == &pl2303_type_data[TYPE_HX]) {
> + pl2303_vendor_read(serial, 0x8484, buf);
> + pl2303_vendor_write(serial, 0x0404, TYPE_HX_PULLUP_MODE_REG);
> + pl2303_vendor_read(serial, 0x8484, buf);
> + pl2303_vendor_read(serial, 0x8383, buf);
> + if ((u16)*buf & TYPE_HX_EXTERNAL_PULLUP_MODE) {
> + pl2303_vendor_write(serial, 0x0, 0x31);
> + pl2303_vendor_write(serial, 0x1, 0x01);
> + }
So this bit needs to go in it's own patch with a commit message
explaining why it is needed. Don't forget to replace the "magic
constants" with descriptive defines.
*buf is u8 and no need to cast to u16, as I also already pointed out in
my comments to an earlier version of the patch.
> }
>
> kfree(buf);
> @@ -720,8 +789,13 @@ 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->type == &pl2303_type_data[TYPE_HXN])
> + pl2303_vendor_write(serial,
> + TYPE_HXN_RESET_DOWN_UPSTREAM, 0);
> + else {
> + pl2303_vendor_write(serial, 8, 0);
> + pl2303_vendor_write(serial, 9, 0);
> + }
So this is a bit harder to abstract, but could be done using function
pointers although it's probably not worth it just yet.
Just make sure to do add bracket ({}) also to the branch you add here.
Thanks,
Johan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Add Proliic new chip: PL2303TB & PL2303N(G)
@ 2019-01-09 1:12 Yeh.Charles [葉榮鑫]
0 siblings, 0 replies; 9+ messages in thread
From: Yeh.Charles [葉榮鑫] @ 2019-01-09 1:12 UTC (permalink / raw)
To: Johan Hovold, Charles Yeh
Cc: lkp@intel.com, kbuild-all@01.org, gregkh@linuxfoundation.org,
linux-usb@vger.kernel.org
Hi Johan,
OK, I will release three separate patches on next week.
1. PL2303TB PID : [PATCH ]: USB: serial: pl2303: Add new PID to support PL2303TB (TYPE_HX)
2. pull-up mode: [PATCH ]: USB: serial: pl2303: Add new Pull-Up Mode to support PL2303HXD (TYPE_HX)
3. TYPE_HXN support + PIDs : [PATCH ]: USB: serial: pl2303: Add new PID & flow Control to support PL2303HXN (TYPE_HXN)
Thank for you check the patch code..
Charles Yeh(葉榮鑫)
TeL: +886-2-26546363 ext.522
Prolific Technology Inc. (旺玖科技股份有限公司)
-----Original Message-----
From: Johan Hovold [mailto:jhovold@gmail.com] On Behalf Of Johan Hovold
Sent: Wednesday, January 9, 2019 1:01 AM
To: Charles Yeh
Cc: lkp@intel.com; kbuild-all@01.org; gregkh@linuxfoundation.org; johan@kernel.org; linux-usb@vger.kernel.org; Yeh.Charles [葉榮鑫]
Subject: Re: [PATCH] Add Proliic new chip: PL2303TB & PL2303N(G)
On Wed, Dec 26, 2018 at 08:28:10PM +0800, Charles Yeh wrote:
> Add new PID to support PL2303TB (TYPE_HX)
> Add new PID to support PL2303(N)GC/GB/GS/GT/GL/GE (TYPE_HXN)
> Add new Flow Control to support PL2303(N)GC/GB/GS/GT/GL/GE (TYPE_HXN)
> Add new Pull-Up Mode to support PL2303HXD (TYPE_HX)
So the above should go in three separate patches as we already discussed
(PL2303TB PID, TYPE_HXN support + PIDs, "pull-up mode").
> Fixed warning:restricted__le16 degrades to integer
When you update and resend a patch, it's good to include changelog
information like this, but please put it below the cut-off line (---)
below so that it doesn't end up in the commit log.
Also include a patch revision in the Subject of the mail. Let's call
this one v2, so next time use the following Subject prefix:
[PATCH v3]: USB: serial: pl2303: add ...
(also after breaking the current patch into a series of three or more
patches).
> Signed-off-by: Charles Yeh <charlesyeh522@gmail.com>
> ---
> drivers/usb/serial/pl2303.c | 116 +++++++++++++++++++++++++++++-------
> drivers/usb/serial/pl2303.h | 11 ++++
> 2 files changed, 106 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c
> index a4e0d13fc121..8c71612e1811 100644
> --- a/drivers/usb/serial/pl2303.c
> +++ b/drivers/usb/serial/pl2303.c
> @@ -46,6 +46,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 +130,11 @@ MODULE_DEVICE_TABLE(usb, id_table);
>
> #define VENDOR_WRITE_REQUEST_TYPE0x40
> #define VENDOR_WRITE_REQUEST0x01
> +#define VENDOR_WRITE_NREQUEST0x80
>
> #define VENDOR_READ_REQUEST_TYPE0xc0
> #define VENDOR_READ_REQUEST0x01
> +#define VENDOR_READ_NREQUEST0x81
>
> #define UART_STATE_INDEX8
> #define UART_STATE_MSR_MASK0x8b
> @@ -139,11 +148,21 @@ MODULE_DEVICE_TABLE(usb, id_table);
> #define UART_OVERRUN_ERROR0x40
> #define UART_CTS0x80
>
> +#define TYPE_HX_READ_OTP_STATUS_REGISTER0x8484
> +#define TYPE_HX_EXTERNAL_PULLUP_MODE0x08
> +#define TYPE_HX_PULLUP_MODE_REG0x09
> +#define TYPE_HXN_UART_FLOWCONTROL0x0A
> +#define TYPE_HXN_HARDWAREFLOW0xFA
> +#define TYPE_HXN_SOFTWAREFLOW0xEE
> +#define TYPE_HXN_NOFLOW0xFF
> +#define TYPE_HXN_RESET_DOWN_UPSTREAM0x07
> +
> 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
> };
>
> @@ -173,16 +192,26 @@ static const struct pl2303_type_data pl2303_type_data[TYPE_COUNT] = {
> [TYPE_HX] = {
> .max_baud_rate =12000000,
> },
> +[TYPE_HXN] = {
> +.max_baud_rate =12000000,
> +},
> };
>
> 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;
> +
> +if (spriv->type == &pl2303_type_data[TYPE_HXN])
> +request = VENDOR_READ_NREQUEST;
> +else
> +request = VENDOR_READ_REQUEST;
>
> 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__,
> @@ -201,12 +230,19 @@ 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;
>
> dev_dbg(dev, "%s - [%04x] = %02x\n", __func__, value, index);
>
> +if (spriv->type == &pl2303_type_data[TYPE_HXN])
> +request = VENDOR_WRITE_NREQUEST;
> +else
> +request = VENDOR_WRITE_REQUEST;
> +
> 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 +322,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)
> @@ -307,26 +344,36 @@ static int pl2303_startup(struct usb_serial *serial)
> type = TYPE_01;/* type 1 */
> dev_dbg(&serial->interface->dev, "device type: %d\n", type);
>
> +if (type == TYPE_HX) {
> +res = usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, 0),
> +VENDOR_READ_REQUEST, VENDOR_READ_REQUEST_TYPE,
> +TYPE_HX_READ_OTP_STATUS_REGISTER, 0, buf, 1, 100);
> +if (res != 1)
> +type = TYPE_HXN;
> +}
So HXN looks like an HX, but responds with an error when trying to read
any (?) register using VENDOR_READ_REQUEST? Isn't there any more direct
way of determining the device type?
My old HXD device report bcdUSB as 0x0110, and you tested you also
tested against 0x0200 in a previous version of this patch. How does
bcdUSB relate to the various types?
Note that you should use le16_to_cpu() to access bcdUSB safely also on
big-endian systems (which the kbuild test robot somewhat cryptically
reported).
> +
> 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);
> +}
Is this even needed for pre-HXN devices, or could perhaps some of it
just be removed? Can you explain what it does?
> kfree(buf);
>
> @@ -671,15 +718,37 @@ static void pl2303_set_termios(struct tty_struct *tty,
> }
>
> if (C_CRTSCTS(tty)) {
> -if (spriv->quirks & PL2303_QUIRK_LEGACY)
> +if (spriv->type == &pl2303_type_data[TYPE_01])
> pl2303_vendor_write(serial, 0x0, 0x41);
> +else if (spriv->type == &pl2303_type_data[TYPE_HXN])
> +pl2303_vendor_write(serial, TYPE_HXN_UART_FLOWCONTROL,
> + TYPE_HXN_HARDWAREFLOW);
> 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->type == &pl2303_type_data[TYPE_HXN])
> +pl2303_vendor_write(serial, TYPE_HXN_UART_FLOWCONTROL,
> + TYPE_HXN_SOFTWAREFLOW);
> +else
> +pl2303_vendor_write(serial, 0x0, 0xc0);
> } else {
> -pl2303_vendor_write(serial, 0x0, 0x0);
> +if (spriv->type == &pl2303_type_data[TYPE_HXN])
> +pl2303_vendor_write(serial, TYPE_HXN_UART_FLOWCONTROL,
> + TYPE_HXN_NOFLOW);
> +else
> +pl2303_vendor_write(serial, 0x0, 0x0);
> +}
So this is becoming a bit hard to read. The idea with the type struct
was to abstract the differences. We could add something like
u8 uart_flowctrl_reg;
u8 uart_flowctrl_hw;
u8 uart_flowctrl_sw;
u8 uart_flowctrl_none;
to struct pl2303_type_data, and replace the above with just three calls
to pl2303_vendor_write().
If you do this, then do this as a preparatory patch before adding HXN
support.
We should probably do something similar with the read and write requests
instead of adding conditionals in those paths.
> +
> +if (spriv->type == &pl2303_type_data[TYPE_HX]) {
> +pl2303_vendor_read(serial, 0x8484, buf);
> +pl2303_vendor_write(serial, 0x0404, TYPE_HX_PULLUP_MODE_REG);
> +pl2303_vendor_read(serial, 0x8484, buf);
> +pl2303_vendor_read(serial, 0x8383, buf);
> +if ((u16)*buf & TYPE_HX_EXTERNAL_PULLUP_MODE) {
> +pl2303_vendor_write(serial, 0x0, 0x31);
> +pl2303_vendor_write(serial, 0x1, 0x01);
> +}
So this bit needs to go in it's own patch with a commit message
explaining why it is needed. Don't forget to replace the "magic
constants" with descriptive defines.
*buf is u8 and no need to cast to u16, as I also already pointed out in
my comments to an earlier version of the patch.
> }
>
> kfree(buf);
> @@ -720,8 +789,13 @@ 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->type == &pl2303_type_data[TYPE_HXN])
> +pl2303_vendor_write(serial,
> +TYPE_HXN_RESET_DOWN_UPSTREAM, 0);
> +else {
> +pl2303_vendor_write(serial, 8, 0);
> +pl2303_vendor_write(serial, 9, 0);
> +}
So this is a bit harder to abstract, but could be done using function
pointers although it's probably not worth it just yet.
Just make sure to do add bracket ({}) also to the branch you add here.
Thanks,
Johan
保密警語: 本電子郵件內容及其附加檔案均視為機密資料，受保密合約保護或依法不得洩漏。其內容僅供指定收件人按限定範圍或特殊目的合法使用，未經授權者收到此信息均無權閱讀、使用、複製、洩漏或散佈。若您並非本郵件之指定收件人，請即刻回覆郵件並永久刪除此郵件及其附件和銷毀所有複印文件。電子郵件的傳輸可能遭攔截、損毀、遺失、破壞、遲到或不完整、或包含病毒，無法保證其安全或無誤。寄件人不承擔因本電子郵件的錯誤或遺漏所產生的任何損害賠償責任。 Confidentiality Notice: This e-mail message together with any attachments thereto (if any) is confidential, protected under an enforceable non-disclosure agreement, intended only for the use of the named recipient(s) above and may contain information that is privileged, belonging to professional work products or exempt from disclosure under applicable laws. Any unauthorized review, use, copying, disclosure, or distribution of any information contained in or attached to this transmission is strictly prohibited and may be against the laws. If you have received this message in error, or are not the intended recipient(s), please immediately notify the sender by e-mail and delete this e-mail message, all copies, and any attached documentation from your computer. E-mail transmission cannot be guaranteed to be secure or error-free as information could be intercepted, corrupted, lost, destroyed, arrive late or incomplete, or contain viruses. The sender therefore does not accept liability for any damage caused by any errors or omissions in the contents of this email.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-01-09 1:12 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-01-08 17:01 Add Proliic new chip: PL2303TB & PL2303N(G) Johan Hovold
-- strict thread matches above, loose matches on Subject: below --
2019-01-09 1:12 Yeh.Charles [葉榮鑫]
2019-01-08 14:16 Johan Hovold
2018-12-26 12:28 Charles Yeh
2018-12-19 10:35 Greg Kroah-Hartman
2018-12-19 10:26 Charles Yeh
2018-12-18 10:02 Johan Hovold
2018-12-17 13:03 Greg Kroah-Hartman
2018-12-17 4:51 Charles Yeh
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).