Message ID | 20190613134544.6404-1-charlesyeh522@gmail.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | [v5] USB: serial: pl2303: Add new PID to support PL2303HXN (TYPE_HXN) | expand |
Is there any need to modify it? If there is no need to modify, how long does it take to complete REVIEW? Charles Yeh Charles Yeh <charlesyeh522@gmail.com> 於 2019年6月13日 週四 下午9:46寫道: > > Prolific has developed a new USB to UART chip: PL2303HXN > PL2303HXN : PL2303GC/PL2303GS/PL2303GT/PL2303GL/PL2303GE/PL2303GB > The Vendor request used by the PL2303HXN (TYPE_HXN) is different from > the existing PL2303 series (TYPE_HX & TYPE_01). > Therefore, different Vendor requests are used to issue related commands. > > 1. Added a new TYPE_HXN type in pl2303_type_data, and then executes > new Vendor request,new flow control and other related instructions > if TYPE_HXN is recognized. > > 2. Because the new PL2303HXN only accept the new Vendor request, > the old Vendor request cannot be accepted (the error message > will be returned) > So first determine the TYPE_HX or TYPE_HXN through > PL2303_READ_TYPE_HX_STATUS in pl2303_startup. > > 2.1 If the return message is "1", then the PL2303 is the existing > TYPE_HX/ TYPE_01 series. > The other settings in pl2303_startup are to continue execution. > 2.2 If the return message is "not 1", then the PL2303 is the new > TYPE_HXN series. > The other settings in pl2303_startup are ignored. > (PL2303HXN will directly use the default value in the hardware, > no need to add additional settings through the software) > > 3. In pl2303_open: Because TYPE_HXN is different from the instruction of reset > down/up stream used by TYPE_HX. > Therefore, we will also execute different instructions here. > > 4. In pl2303_set_termios: The UART flow control instructions used by > TYPE_HXN/TYPE_HX/TYPE_01 are different. > Therefore, we will also execute different instructions here. > > 5. In pl2303_vendor_read & pl2303_vendor_write, since TYPE_HXN is different > from the vendor request instruction used by TYPE_HX/TYPE_01, > it will also execute different instructions here. > > 6. In pl2303_update_reg: TYPE_HXN used different register for flow control. > Therefore, we will also execute different instructions here. > > Signed-off-by: Charles Yeh <charlesyeh522@gmail.com> > --- > changelog: > v5: > 1. Modify pl2303_update_reg > 2. Add a patch version on subject > 3. Add a space after each colon at subject line > --- > drivers/usb/serial/pl2303.c | 113 +++++++++++++++++++++++++++++------- > drivers/usb/serial/pl2303.h | 7 ++- > 2 files changed, 97 insertions(+), 23 deletions(-) > > diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c > index 55122ac84518..22ad82aa3894 100644 > --- a/drivers/usb/serial/pl2303.c > +++ b/drivers/usb/serial/pl2303.c > @@ -47,6 +47,12 @@ static const struct usb_device_id id_table[] = { > { 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, PL2303_PRODUCT_ID_GC) }, > + { USB_DEVICE(PL2303_VENDOR_ID, PL2303_PRODUCT_ID_GB) }, > + { USB_DEVICE(PL2303_VENDOR_ID, PL2303_PRODUCT_ID_GT) }, > + { USB_DEVICE(PL2303_VENDOR_ID, PL2303_PRODUCT_ID_GL) }, > + { USB_DEVICE(PL2303_VENDOR_ID, PL2303_PRODUCT_ID_GE) }, > + { USB_DEVICE(PL2303_VENDOR_ID, PL2303_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), > @@ -129,9 +135,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 > @@ -146,12 +154,20 @@ MODULE_DEVICE_TABLE(usb, id_table); > #define UART_CTS 0x80 > > #define PL2303_FLOWCTRL_MASK 0xf0 > +#define PL2303_HXN_FLOWCTRL_MASK 0x1C > +#define PL2303_READ_TYPE_HX_STATUS 0x8080 > +#define PL2303_HXN_FLOWCTRL 0x0A > +#define PL2303_HXN_CTRL_RTS_CTS 0x18 > +#define PL2303_HXN_CTRL_XON_XOFF 0x0C > +#define PL2303_HXN_CTRL_NONE 0x1C > +#define PL2303_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 > }; > > @@ -183,16 +199,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__, > @@ -211,12 +237,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__, > @@ -231,12 +264,17 @@ static int pl2303_update_reg(struct usb_serial *serial, u8 reg, u8 mask, u8 val) > { > int ret = 0; > u8 *buf; > + struct pl2303_serial_private *spriv = usb_get_serial_data(serial); > > buf = kmalloc(1, GFP_KERNEL); > if (!buf) > return -ENOMEM; > > - ret = pl2303_vendor_read(serial, reg | 0x80, buf); > + if (spriv->type == &pl2303_type_data[TYPE_HXN]) > + ret = pl2303_vendor_read(serial, reg, buf); > + else > + ret = pl2303_vendor_read(serial, reg | 0x80, buf); > + > if (ret) > goto out_free; > > @@ -319,6 +357,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) > @@ -340,26 +379,37 @@ 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, > + PL2303_READ_TYPE_HX_STATUS, 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); > > @@ -720,12 +770,26 @@ static void pl2303_set_termios(struct tty_struct *tty, > if (C_CRTSCTS(tty)) { > if (spriv->quirks & PL2303_QUIRK_LEGACY) > pl2303_update_reg(serial, 0, PL2303_FLOWCTRL_MASK, 0x40); > - else > + else if (spriv->type == &pl2303_type_data[TYPE_HXN]) { > + pl2303_update_reg(serial, PL2303_HXN_FLOWCTRL, > + PL2303_HXN_FLOWCTRL_MASK, PL2303_HXN_CTRL_RTS_CTS); > + } else { > pl2303_update_reg(serial, 0, PL2303_FLOWCTRL_MASK, 0x60); > + } > } else if (pl2303_enable_xonxoff(tty, spriv->type)) { > - pl2303_update_reg(serial, 0, PL2303_FLOWCTRL_MASK, 0xc0); > + if (spriv->type == &pl2303_type_data[TYPE_HXN]) { > + pl2303_update_reg(serial, PL2303_HXN_FLOWCTRL, > + PL2303_HXN_FLOWCTRL_MASK, PL2303_HXN_CTRL_XON_XOFF); > + } else { > + pl2303_update_reg(serial, 0, PL2303_FLOWCTRL_MASK, 0xc0); > + } > } else { > - pl2303_update_reg(serial, 0, PL2303_FLOWCTRL_MASK, 0); > + if (spriv->type == &pl2303_type_data[TYPE_HXN]) { > + pl2303_update_reg(serial, PL2303_HXN_FLOWCTRL, > + PL2303_HXN_FLOWCTRL_MASK, PL2303_HXN_CTRL_NONE); > + } else { > + pl2303_update_reg(serial, 0, PL2303_FLOWCTRL_MASK, 0); > + } > } > > kfree(buf); > @@ -766,8 +830,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, > + PL2303_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 559941ca884d..f1c8b5a36816 100644 > --- a/drivers/usb/serial/pl2303.h > +++ b/drivers/usb/serial/pl2303.h > @@ -20,7 +20,12 @@ > #define PL2303_PRODUCT_ID_HCR331 0x331a > #define PL2303_PRODUCT_ID_MOTOROLA 0x0307 > #define PL2303_PRODUCT_ID_ZTEK 0xe1f1 > - > +#define PL2303_PRODUCT_ID_GC 0x23A3 > +#define PL2303_PRODUCT_ID_GB 0x23B3 > +#define PL2303_PRODUCT_ID_GT 0x23C3 > +#define PL2303_PRODUCT_ID_GL 0x23D3 > +#define PL2303_PRODUCT_ID_GE 0x23E3 > +#define PL2303_PRODUCT_ID_GS 0x23F3 > > #define ATEN_VENDOR_ID 0x0557 > #define ATEN_VENDOR_ID2 0x0547 > -- > 2.20.1 >
On Thu, Jun 13, 2019 at 09:45:44PM +0800, Charles Yeh wrote: > Prolific has developed a new USB to UART chip: PL2303HXN > PL2303HXN : PL2303GC/PL2303GS/PL2303GT/PL2303GL/PL2303GE/PL2303GB > The Vendor request used by the PL2303HXN (TYPE_HXN) is different from > the existing PL2303 series (TYPE_HX & TYPE_01). > Therefore, different Vendor requests are used to issue related commands. > > 1. Added a new TYPE_HXN type in pl2303_type_data, and then executes > new Vendor request,new flow control and other related instructions > if TYPE_HXN is recognized. > > 2. Because the new PL2303HXN only accept the new Vendor request, > the old Vendor request cannot be accepted (the error message > will be returned) > So first determine the TYPE_HX or TYPE_HXN through > PL2303_READ_TYPE_HX_STATUS in pl2303_startup. > > 2.1 If the return message is "1", then the PL2303 is the existing > TYPE_HX/ TYPE_01 series. > The other settings in pl2303_startup are to continue execution. > 2.2 If the return message is "not 1", then the PL2303 is the new > TYPE_HXN series. > The other settings in pl2303_startup are ignored. > (PL2303HXN will directly use the default value in the hardware, > no need to add additional settings through the software) > > 3. In pl2303_open: Because TYPE_HXN is different from the instruction of reset > down/up stream used by TYPE_HX. > Therefore, we will also execute different instructions here. > > 4. In pl2303_set_termios: The UART flow control instructions used by > TYPE_HXN/TYPE_HX/TYPE_01 are different. > Therefore, we will also execute different instructions here. > > 5. In pl2303_vendor_read & pl2303_vendor_write, since TYPE_HXN is different > from the vendor request instruction used by TYPE_HX/TYPE_01, > it will also execute different instructions here. > > 6. In pl2303_update_reg: TYPE_HXN used different register for flow control. > Therefore, we will also execute different instructions here. > > Signed-off-by: Charles Yeh <charlesyeh522@gmail.com> Sorry for not getting back to you on the clean ups yet. Just really short on time these last few months. We can merge your patch and I can just add those clean ups on top later. But please fix the below first. > --- > changelog: > v5: > 1. Modify pl2303_update_reg Surely you did more than just modify pl2303_update_reg (and that doesn't explain how or why you did it). Please be more specific in your changelogs. > 2. Add a patch version on subject > 3. Add a space after each colon at subject line > --- > drivers/usb/serial/pl2303.c | 113 +++++++++++++++++++++++++++++------- > drivers/usb/serial/pl2303.h | 7 ++- > 2 files changed, 97 insertions(+), 23 deletions(-) > > diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c > index 55122ac84518..22ad82aa3894 100644 > --- a/drivers/usb/serial/pl2303.c > +++ b/drivers/usb/serial/pl2303.c > @@ -47,6 +47,12 @@ static const struct usb_device_id id_table[] = { > { 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, PL2303_PRODUCT_ID_GC) }, > + { USB_DEVICE(PL2303_VENDOR_ID, PL2303_PRODUCT_ID_GB) }, > + { USB_DEVICE(PL2303_VENDOR_ID, PL2303_PRODUCT_ID_GT) }, > + { USB_DEVICE(PL2303_VENDOR_ID, PL2303_PRODUCT_ID_GL) }, > + { USB_DEVICE(PL2303_VENDOR_ID, PL2303_PRODUCT_ID_GE) }, > + { USB_DEVICE(PL2303_VENDOR_ID, PL2303_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), > @@ -129,9 +135,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 > @@ -146,12 +154,20 @@ MODULE_DEVICE_TABLE(usb, id_table); > #define UART_CTS 0x80 > > #define PL2303_FLOWCTRL_MASK 0xf0 > +#define PL2303_HXN_FLOWCTRL_MASK 0x1C > +#define PL2303_READ_TYPE_HX_STATUS 0x8080 Add a newline here to separate the HXN register defines, and... > +#define PL2303_HXN_FLOWCTRL 0x0A ...I'd move the HXN flow control mask here. > +#define PL2303_HXN_CTRL_RTS_CTS 0x18 > +#define PL2303_HXN_CTRL_XON_XOFF 0x0C > +#define PL2303_HXN_CTRL_NONE 0x1C > +#define PL2303_HXN_RESET_DOWN_UPSTREAM 0x07 This register does more than reset the up- and downstream buffers. Your documentation calls it "Chip reset control", so PL2303_HXN_RESET_CONTROL might be a better name. And move it before the flow-control register definition to keep them sorted by address. > 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 > }; > > @@ -183,16 +199,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__, > @@ -211,12 +237,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__, > @@ -231,12 +264,17 @@ static int pl2303_update_reg(struct usb_serial *serial, u8 reg, u8 mask, u8 val) > { > int ret = 0; > u8 *buf; > + struct pl2303_serial_private *spriv = usb_get_serial_data(serial); > > buf = kmalloc(1, GFP_KERNEL); > if (!buf) > return -ENOMEM; > > - ret = pl2303_vendor_read(serial, reg | 0x80, buf); > + if (spriv->type == &pl2303_type_data[TYPE_HXN]) > + ret = pl2303_vendor_read(serial, reg, buf); > + else > + ret = pl2303_vendor_read(serial, reg | 0x80, buf); Ok, this will do for now, but we really should move that OR into the read helper eventually. > + > if (ret) > goto out_free; > > @@ -319,6 +357,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) > @@ -340,26 +379,37 @@ 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, > + PL2303_READ_TYPE_HX_STATUS, 0, buf, 1, 100); Please indent continuation lines at least two tabs further. > + 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); > > @@ -720,12 +770,26 @@ static void pl2303_set_termios(struct tty_struct *tty, > if (C_CRTSCTS(tty)) { > if (spriv->quirks & PL2303_QUIRK_LEGACY) > pl2303_update_reg(serial, 0, PL2303_FLOWCTRL_MASK, 0x40); > - else You need to add bracket {} to all branches when adding to one branch, as I've mentioned before. > + else if (spriv->type == &pl2303_type_data[TYPE_HXN]) { > + pl2303_update_reg(serial, PL2303_HXN_FLOWCTRL, > + PL2303_HXN_FLOWCTRL_MASK, PL2303_HXN_CTRL_RTS_CTS); Odd indentation; again use at least two tabs more for continuation lines. > + } else { > pl2303_update_reg(serial, 0, PL2303_FLOWCTRL_MASK, 0x60); > + } > } else if (pl2303_enable_xonxoff(tty, spriv->type)) { > - pl2303_update_reg(serial, 0, PL2303_FLOWCTRL_MASK, 0xc0); > + if (spriv->type == &pl2303_type_data[TYPE_HXN]) { > + pl2303_update_reg(serial, PL2303_HXN_FLOWCTRL, > + PL2303_HXN_FLOWCTRL_MASK, PL2303_HXN_CTRL_XON_XOFF); Same here. > + } else { > + pl2303_update_reg(serial, 0, PL2303_FLOWCTRL_MASK, 0xc0); > + } > } else { > - pl2303_update_reg(serial, 0, PL2303_FLOWCTRL_MASK, 0); > + if (spriv->type == &pl2303_type_data[TYPE_HXN]) { > + pl2303_update_reg(serial, PL2303_HXN_FLOWCTRL, > + PL2303_HXN_FLOWCTRL_MASK, PL2303_HXN_CTRL_NONE); And here. > + } else { > + pl2303_update_reg(serial, 0, PL2303_FLOWCTRL_MASK, 0); > + } > } > > kfree(buf); > @@ -766,8 +830,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, > + PL2303_HXN_RESET_DOWN_UPSTREAM, 0); Indentation again. Also, as I've asked you already; why do you write 0 here to the reset-control register when your documentation indicates that you should be writing 0x3 to reset the upstream and downstream pipes. Please explain and fix if necessary. > + } 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 559941ca884d..f1c8b5a36816 100644 > --- a/drivers/usb/serial/pl2303.h > +++ b/drivers/usb/serial/pl2303.h > @@ -20,7 +20,12 @@ > #define PL2303_PRODUCT_ID_HCR331 0x331a > #define PL2303_PRODUCT_ID_MOTOROLA 0x0307 > #define PL2303_PRODUCT_ID_ZTEK 0xe1f1 > - > +#define PL2303_PRODUCT_ID_GC 0x23A3 > +#define PL2303_PRODUCT_ID_GB 0x23B3 > +#define PL2303_PRODUCT_ID_GT 0x23C3 > +#define PL2303_PRODUCT_ID_GL 0x23D3 > +#define PL2303_PRODUCT_ID_GE 0x23E3 > +#define PL2303_PRODUCT_ID_GS 0x23F3 > > #define ATEN_VENDOR_ID 0x0557 > #define ATEN_VENDOR_ID2 0x0547 Johan
diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c index 55122ac84518..22ad82aa3894 100644 --- a/drivers/usb/serial/pl2303.c +++ b/drivers/usb/serial/pl2303.c @@ -47,6 +47,12 @@ static const struct usb_device_id id_table[] = { { 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, PL2303_PRODUCT_ID_GC) }, + { USB_DEVICE(PL2303_VENDOR_ID, PL2303_PRODUCT_ID_GB) }, + { USB_DEVICE(PL2303_VENDOR_ID, PL2303_PRODUCT_ID_GT) }, + { USB_DEVICE(PL2303_VENDOR_ID, PL2303_PRODUCT_ID_GL) }, + { USB_DEVICE(PL2303_VENDOR_ID, PL2303_PRODUCT_ID_GE) }, + { USB_DEVICE(PL2303_VENDOR_ID, PL2303_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), @@ -129,9 +135,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 @@ -146,12 +154,20 @@ MODULE_DEVICE_TABLE(usb, id_table); #define UART_CTS 0x80 #define PL2303_FLOWCTRL_MASK 0xf0 +#define PL2303_HXN_FLOWCTRL_MASK 0x1C +#define PL2303_READ_TYPE_HX_STATUS 0x8080 +#define PL2303_HXN_FLOWCTRL 0x0A +#define PL2303_HXN_CTRL_RTS_CTS 0x18 +#define PL2303_HXN_CTRL_XON_XOFF 0x0C +#define PL2303_HXN_CTRL_NONE 0x1C +#define PL2303_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 }; @@ -183,16 +199,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__, @@ -211,12 +237,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__, @@ -231,12 +264,17 @@ static int pl2303_update_reg(struct usb_serial *serial, u8 reg, u8 mask, u8 val) { int ret = 0; u8 *buf; + struct pl2303_serial_private *spriv = usb_get_serial_data(serial); buf = kmalloc(1, GFP_KERNEL); if (!buf) return -ENOMEM; - ret = pl2303_vendor_read(serial, reg | 0x80, buf); + if (spriv->type == &pl2303_type_data[TYPE_HXN]) + ret = pl2303_vendor_read(serial, reg, buf); + else + ret = pl2303_vendor_read(serial, reg | 0x80, buf); + if (ret) goto out_free; @@ -319,6 +357,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) @@ -340,26 +379,37 @@ 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, + PL2303_READ_TYPE_HX_STATUS, 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); @@ -720,12 +770,26 @@ static void pl2303_set_termios(struct tty_struct *tty, if (C_CRTSCTS(tty)) { if (spriv->quirks & PL2303_QUIRK_LEGACY) pl2303_update_reg(serial, 0, PL2303_FLOWCTRL_MASK, 0x40); - else + else if (spriv->type == &pl2303_type_data[TYPE_HXN]) { + pl2303_update_reg(serial, PL2303_HXN_FLOWCTRL, + PL2303_HXN_FLOWCTRL_MASK, PL2303_HXN_CTRL_RTS_CTS); + } else { pl2303_update_reg(serial, 0, PL2303_FLOWCTRL_MASK, 0x60); + } } else if (pl2303_enable_xonxoff(tty, spriv->type)) { - pl2303_update_reg(serial, 0, PL2303_FLOWCTRL_MASK, 0xc0); + if (spriv->type == &pl2303_type_data[TYPE_HXN]) { + pl2303_update_reg(serial, PL2303_HXN_FLOWCTRL, + PL2303_HXN_FLOWCTRL_MASK, PL2303_HXN_CTRL_XON_XOFF); + } else { + pl2303_update_reg(serial, 0, PL2303_FLOWCTRL_MASK, 0xc0); + } } else { - pl2303_update_reg(serial, 0, PL2303_FLOWCTRL_MASK, 0); + if (spriv->type == &pl2303_type_data[TYPE_HXN]) { + pl2303_update_reg(serial, PL2303_HXN_FLOWCTRL, + PL2303_HXN_FLOWCTRL_MASK, PL2303_HXN_CTRL_NONE); + } else { + pl2303_update_reg(serial, 0, PL2303_FLOWCTRL_MASK, 0); + } } kfree(buf); @@ -766,8 +830,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, + PL2303_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 559941ca884d..f1c8b5a36816 100644 --- a/drivers/usb/serial/pl2303.h +++ b/drivers/usb/serial/pl2303.h @@ -20,7 +20,12 @@ #define PL2303_PRODUCT_ID_HCR331 0x331a #define PL2303_PRODUCT_ID_MOTOROLA 0x0307 #define PL2303_PRODUCT_ID_ZTEK 0xe1f1 - +#define PL2303_PRODUCT_ID_GC 0x23A3 +#define PL2303_PRODUCT_ID_GB 0x23B3 +#define PL2303_PRODUCT_ID_GT 0x23C3 +#define PL2303_PRODUCT_ID_GL 0x23D3 +#define PL2303_PRODUCT_ID_GE 0x23E3 +#define PL2303_PRODUCT_ID_GS 0x23F3 #define ATEN_VENDOR_ID 0x0557 #define ATEN_VENDOR_ID2 0x0547
Prolific has developed a new USB to UART chip: PL2303HXN PL2303HXN : PL2303GC/PL2303GS/PL2303GT/PL2303GL/PL2303GE/PL2303GB The Vendor request used by the PL2303HXN (TYPE_HXN) is different from the existing PL2303 series (TYPE_HX & TYPE_01). Therefore, different Vendor requests are used to issue related commands. 1. Added a new TYPE_HXN type in pl2303_type_data, and then executes new Vendor request,new flow control and other related instructions if TYPE_HXN is recognized. 2. Because the new PL2303HXN only accept the new Vendor request, the old Vendor request cannot be accepted (the error message will be returned) So first determine the TYPE_HX or TYPE_HXN through PL2303_READ_TYPE_HX_STATUS in pl2303_startup. 2.1 If the return message is "1", then the PL2303 is the existing TYPE_HX/ TYPE_01 series. The other settings in pl2303_startup are to continue execution. 2.2 If the return message is "not 1", then the PL2303 is the new TYPE_HXN series. The other settings in pl2303_startup are ignored. (PL2303HXN will directly use the default value in the hardware, no need to add additional settings through the software) 3. In pl2303_open: Because TYPE_HXN is different from the instruction of reset down/up stream used by TYPE_HX. Therefore, we will also execute different instructions here. 4. In pl2303_set_termios: The UART flow control instructions used by TYPE_HXN/TYPE_HX/TYPE_01 are different. Therefore, we will also execute different instructions here. 5. In pl2303_vendor_read & pl2303_vendor_write, since TYPE_HXN is different from the vendor request instruction used by TYPE_HX/TYPE_01, it will also execute different instructions here. 6. In pl2303_update_reg: TYPE_HXN used different register for flow control. Therefore, we will also execute different instructions here. Signed-off-by: Charles Yeh <charlesyeh522@gmail.com> --- changelog: v5: 1. Modify pl2303_update_reg 2. Add a patch version on subject 3. Add a space after each colon at subject line --- drivers/usb/serial/pl2303.c | 113 +++++++++++++++++++++++++++++------- drivers/usb/serial/pl2303.h | 7 ++- 2 files changed, 97 insertions(+), 23 deletions(-)