| Message ID | 20190212125049.12108-1-charlesyeh522@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] USB:serial:pl2303:add new Pull-Up mode to support PL2303HXD (TYPE_HX) | expand |
Hi Johan & Greg, Do you have received a new patch"[PATCH] [v2]USB:serial:pl2303:add new Pull-Up mode to support PL2303HXD (TYPE_HX)"? If you have received a new patch, has the content been confirmed? Or tell me where needs to be modified. Charles Yeh <charlesyeh522@gmail.com> 於 2019年2月12日 週二 下午8:51寫道： > > 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 <charlesyeh522@gmail.com> > --- > drivers/usb/serial/pl2303.c | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c > index bb3f9aa4a909..e5d00e4a495d 100644 > --- a/drivers/usb/serial/pl2303.c > +++ b/drivers/usb/serial/pl2303.c > @@ -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 > +#define TYPE_HX_PULLUP_MODE_DATA 0x08 > +#define TYPE_HX_PULLUP_MODE_REG 0x09 > +#define TYPE_HX_PUM_ADD0 0x00 > +#define TYPE_HX_PUM_DATA0 0x31 > +#define TYPE_HX_PUM_ADD1 0x01 > +#define TYPE_HX_PUM_DATA1 0x08 > + > static void pl2303_set_break(struct usb_serial_port *port, bool enable); > > enum pl2303_type { > @@ -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); > + if (*buf == TYPE_HX_PULLUP_MODE_DATA) { > + pl2303_vendor_write(serial, TYPE_HX_PUM_ADD0, > + TYPE_HX_PUM_DATA0); > + pl2303_vendor_write(serial, TYPE_HX_PUM_ADD1, > + TYPE_HX_PUM_DATA1); > + } > + } > + > kfree(buf); > } > > -- > 2.19.1 >
On Tue, Feb 19, 2019 at 02:41:28PM +0800, Charles Yeh wrote: > Hi Johan & Greg, > > > Do you have received a new patch"[PATCH] [v2]USB:serial:pl2303:add new > Pull-Up mode to support PL2303HXD (TYPE_HX)"? > > If you have received a new patch, has the content been confirmed? > > Or tell me where needs to be modified. Yes, both patches you sent last week are in my queue. I just haven't had time to review them yet. Johan
Hi Johan,
Do you have time to review the two patch?
Johan Hovold <johan@kernel.org> 於 2019年2月19日 週二 下午4:51寫道：
>
> On Tue, Feb 19, 2019 at 02:41:28PM +0800, Charles Yeh wrote:
> > Hi Johan & Greg,
> >
> >
> > Do you have received a new patch"[PATCH] [v2]USB:serial:pl2303:add new
> > Pull-Up mode to support PL2303HXD (TYPE_HX)"?
> >
> > If you have received a new patch, has the content been confirmed?
> >
> > Or tell me where needs to be modified.
>
> Yes, both patches you sent last week are in my queue. I just haven't had
> time to review them yet.
>
> Johan
Hi Johan / Greg, Do you have time to review the patch? If there is no time.. Who else can review this patch? Because my boss has been asking me to update the Linux patch... Charles. Charles Yeh <charlesyeh522@gmail.com> 於 2019年3月5日 週二 上午9:16寫道： > > Hi Johan, > Do you have time to review the two patch? > > Johan Hovold <johan@kernel.org> 於 2019年2月19日 週二 下午4:51寫道： > > > > On Tue, Feb 19, 2019 at 02:41:28PM +0800, Charles Yeh wrote: > > > Hi Johan & Greg, > > > > > > > > > Do you have received a new patch"[PATCH] [v2]USB:serial:pl2303:add new > > > Pull-Up mode to support PL2303HXD (TYPE_HX)"? > > > > > > If you have received a new patch, has the content been confirmed? > > > > > > Or tell me where needs to be modified. > > > > Yes, both patches you sent last week are in my queue. I just haven't had > > time to review them yet. > > > > Johan
On Wed, Mar 06, 2019 at 12:42:29PM +0800, Charles Yeh wrote: > Hi Johan / Greg, > > Do you have time to review the patch? > If there is no time.. Who else can review this patch? > Because my boss has been asking me to update the Linux patch... It is the middle of the merge window and we can not accept any new patches until after 5.1-rc1 is out in 1 1/2 weeks. Please wait until thne before we can get to reviewing this. thanks, greg k-h
Hi Greg / Johan,
Is there free time to check the patch?
Or I have to continue wait.
Thanks!
Charles
Greg KH <gregkh@linuxfoundation.org> 於 2019年3月6日 週三 下午1:25寫道：
>
> On Wed, Mar 06, 2019 at 12:42:29PM +0800, Charles Yeh wrote:
> > Hi Johan / Greg,
> >
> > Do you have time to review the patch?
> > If there is no time.. Who else can review this patch?
> > Because my boss has been asking me to update the Linux patch...
>
> It is the middle of the merge window and we can not accept any new
> patches until after 5.1-rc1 is out in 1 1/2 weeks. Please wait until
> thne before we can get to reviewing this.
>
> thanks,
>
> greg k-h
On Wed, Mar 20, 2019 at 10:43:28AM +0800, Charles Yeh wrote: > Hi Greg / Johan, > Is there free time to check the patch? > Or I have to continue wait. The merge window is just over and I've started to process my queue. Don't worry, I'll get to your patches as well. Johan
On Tue, Feb 12, 2019 at 08:50:49PM +0800, Charles Yeh wrote: > 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 <charlesyeh522@gmail.com> > --- > drivers/usb/serial/pl2303.c | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c > index bb3f9aa4a909..e5d00e4a495d 100644 > --- a/drivers/usb/serial/pl2303.c > +++ b/drivers/usb/serial/pl2303.c > @@ -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? > +#define TYPE_HX_PULLUP_MODE_DATA 0x08 What is bit 0x08? > +#define TYPE_HX_PULLUP_MODE_REG 0x09 And this register isn't just used for pull-up mode as far as I can tell. > +#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? > + > static void pl2303_set_break(struct usb_serial_port *port, bool enable); > > enum pl2303_type { > @@ -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? > + if (*buf == TYPE_HX_PULLUP_MODE_DATA) { Don't you want to just check bit 0x80 here? > + 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? > + 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. > + } > + } > + > kfree(buf); > } Thanks, Johan
Hi Johan, A Patch: [PATCH] USB:serial:pl2303:Add new PID to support PL2303HXN (TYPE_HXN) B Patch: [PATCH] [v2]USB:serial:pl2303:add new Pull-Up mode to support PL2303HXD (TYPE_HX) To save time, let you have more time to process A patch, B patch's reply will wait until A patch passes, then I will repeat the problem of B patch. Charles. Johan Hovold <johan@kernel.org> 於 2019年4月2日 週二 下午3:34寫道： > > On Tue, Feb 12, 2019 at 08:50:49PM +0800, Charles Yeh wrote: > > 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 <charlesyeh522@gmail.com> > > --- > > drivers/usb/serial/pl2303.c | 24 ++++++++++++++++++++++++ > > 1 file changed, 24 insertions(+) > > > > diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c > > index bb3f9aa4a909..e5d00e4a495d 100644 > > --- a/drivers/usb/serial/pl2303.c > > +++ b/drivers/usb/serial/pl2303.c > > @@ -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? > > > +#define TYPE_HX_PULLUP_MODE_DATA 0x08 > > What is bit 0x08? > > > +#define TYPE_HX_PULLUP_MODE_REG 0x09 > > And this register isn't just used for pull-up mode as far as I can tell. > > > +#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? > > > + > > static void pl2303_set_break(struct usb_serial_port *port, bool enable); > > > > enum pl2303_type { > > @@ -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? > > > + if (*buf == TYPE_HX_PULLUP_MODE_DATA) { > > Don't you want to just check bit 0x80 here? > > > + 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? > > > + 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. > > > + } > > + } > > + > > kfree(buf); > > } > > Thanks, > Johan
diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c index bb3f9aa4a909..e5d00e4a495d 100644 --- a/drivers/usb/serial/pl2303.c +++ b/drivers/usb/serial/pl2303.c @@ -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 +#define TYPE_HX_PULLUP_MODE_DATA 0x08 +#define TYPE_HX_PULLUP_MODE_REG 0x09 +#define TYPE_HX_PUM_ADD0 0x00 +#define TYPE_HX_PUM_DATA0 0x31 +#define TYPE_HX_PUM_ADD1 0x01 +#define TYPE_HX_PUM_DATA1 0x08 + static void pl2303_set_break(struct usb_serial_port *port, bool enable); enum pl2303_type { @@ -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); + if (*buf == TYPE_HX_PULLUP_MODE_DATA) { + pl2303_vendor_write(serial, TYPE_HX_PUM_ADD0, + TYPE_HX_PUM_DATA0); + pl2303_vendor_write(serial, TYPE_HX_PUM_ADD1, + TYPE_HX_PUM_DATA1); + } + } + kfree(buf); }
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 <charlesyeh522@gmail.com> --- drivers/usb/serial/pl2303.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)