| Message ID | 20181226122810.1716-1-charlesyeh522@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add Proliic new chip: PL2303TB & PL2303N(G) | expand |
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
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.
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
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(-)