USB:serial:pl2303:add new Pull-Up mode to support PL2303HXD (TYPE_HX)

PATCH New Johan Hovold 1 participant 1 message Apr 2, 2019 USB

lore

Share & tools

lore

Johan Hovold

author

2019-04-02 07:34 UTC #0 lore raw

PATCH v2 1/1 USB:serial:pl2303:add new Pull-Up mode to support PL2303HXD (TYPE_HX)

Quoted 20 lines · Charles Yeh

Pull-Up mode is disabled (default) in PL2303HXD.
When the Pull-Up mode is activated, its TX/DTR/RTS external resistor will start the output function.
How to enable the Pull-Up mode of PL2303HXD
1.TX/DTR/RTS external resistor is required on the circuit diagram (PCB) 2.PL2303HXD OTP needs to be programmed to have a Pull-Up mode through 6.5V (USB_VCC,5V->6.5V)
The patch driver will read whether the PL2303HXD has a Pull-Up mode,and if so, it will be set to enable Pull-Up mode function.
Signed-off-by Charles Yeh <[email protected]>
---
 drivers/usb/serial/pl2303.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)
drivers/usb/serial/pl2303.c +0 -0
index bb3f9aa4a909..e5d00e4a495d 100644

drivers/usb/serial/pl2303.c MODULE_DEVICE_TABLE(usb, id_table); · line 150

Quoted 6 lines

drivers/usb/serial/pl2303.c +3 -0

@@ -145,6 +145,16 @@ MODULE_DEVICE_TABLE(usb, id_table);
 #define UART_OVERRUN_ERROR		0x40
 #define UART_CTS			0x80
 
+#define	TYPE_HX_READ_PUM_STATUS_REG	0x8484
+#define	TYPE_HX_READ_PUM_ADD		0x0404
+#define	TYPE_HX_READ_PUM_DATA_REG	0x8383

Again, the defines need to reflect what the registers are for, not what use happen to use them for in on specific code path.

These are used to access the EEPROM/OTP, right?

drivers/usb/serial/pl2303.c MODULE_DEVICE_TABLE(usb, id_table); · line 151

Quoted 1 line

drivers/usb/serial/pl2303.c +1 -0
+#define	TYPE_HX_PULLUP_MODE_DATA	0x08

What is bit 0x08?

drivers/usb/serial/pl2303.c MODULE_DEVICE_TABLE(usb, id_table); · line 152

Quoted 1 line

drivers/usb/serial/pl2303.c +1 -0
+#define	TYPE_HX_PULLUP_MODE_REG		0x09

And this register isn't just used for pull-up mode as far as I can tell.

drivers/usb/serial/pl2303.c MODULE_DEVICE_TABLE(usb, id_table); · line 156

Quoted 4 lines

drivers/usb/serial/pl2303.c +4 -0

+#define	TYPE_HX_PUM_ADD0		0x00
+#define	TYPE_HX_PUM_DATA0		0x31
+#define	TYPE_HX_PUM_ADD1		0x01
+#define	TYPE_HX_PUM_DATA1		0x08

Same for these, we don't want multiple defines for register 0 for example. What are bits 0x31 of register 0?

Quoted 5 lines

drivers/usb/serial/pl2303.c +1 -0

+
 static void pl2303_set_break(struct usb_serial_port *port, bool enable);
 
 enum pl2303_type {

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

Quoted 9 lines

drivers/usb/serial/pl2303.c +6 -0

@@ -688,6 +698,20 @@ static void pl2303_set_termios(struct tty_struct *tty,
 		pl2303_vendor_write(serial, 0x0, 0x0);
 	}
 
+	if (spriv->type == &pl2303_type_data[TYPE_HX]) {
+		pl2303_vendor_read(serial, TYPE_HX_READ_PUM_STATUS_REG, buf);
+		pl2303_vendor_write(serial, TYPE_HX_READ_PUM_ADD,
+					TYPE_HX_PULLUP_MODE_REG);
+		pl2303_vendor_read(serial, TYPE_HX_READ_PUM_STATUS_REG, buf);
+		pl2303_vendor_read(serial, TYPE_HX_READ_PUM_DATA_REG, buf);

Why do you need to access the OTP on every set_termios() call? I thought those settings where copied to the corresponding control registers during boot?

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

Quoted 1 line

drivers/usb/serial/pl2303.c +1 -0
+		if (*buf == TYPE_HX_PULLUP_MODE_DATA) {

Don't you want to just check bit 0x80 here?

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

Quoted 2 lines

drivers/usb/serial/pl2303.c +2 -0

+			pl2303_vendor_write(serial, TYPE_HX_PUM_ADD0,
+						TYPE_HX_PUM_DATA0);

So this looks broken since you're overwriting the flow control settings that were just set above. And again, what are bits 0x31 of register 0? Doesn't 0x30 control auto-rts?

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

Quoted 2 lines

drivers/usb/serial/pl2303.c +2 -0

+			pl2303_vendor_write(serial, TYPE_HX_PUM_ADD1,
+						TYPE_HX_PUM_DATA1);

And why do you need to (over-)write register 2?

Either way, configuring pull-up mode should be done at probe and not on every set_termios() call.

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

Quoted 5 lines

drivers/usb/serial/pl2303.c +3 -0
+		}
+	}
+
 	kfree(buf);
 }

Thanks,
Johan