Message ID | 20200820075240.13321-1-china_shenglong@163.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | [v3] usb-serial:cp210x: add support to software flow control | expand |
On 20.08.20 09:52, Sheng Long Wang wrote: > From: Wang Sheng Long <shenglong.wang.ext@siemens.com> > > When data is transmitted between two serial ports, > the phenomenon of data loss often occurs. The two kinds > of flow control commonly used in serial communication > are hardware flow control and software flow control. > > In serial communication, If you only use RX/TX/GND Pins, you > can't do hardware flow. So we often used software flow control > and prevent data loss. The user sets the software flow control > through the application program, and the application program > sets the software flow control mode for the serial port > chip through the driver. > > For the cp210 serial port chip, its driver lacks the > software flow control setting code, so the user cannot set > the software flow control function through the application > program. This adds the missing software flow control. > > Signed-off-by: Wang Sheng Long <shenglong.wang.ext@siemens.com> > > Changes in v3: > -fixed code style, It mainly adjusts the code style acccording > to kernel specification. Patch does not apply. You forgot to rebase over latest tty/tty-next or linux master. Jan > --- > drivers/usb/serial/cp210x.c | 118 ++++++++++++++++++++++++++++++++++-- > 1 file changed, 113 insertions(+), 5 deletions(-) > > diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c > index e732949f65..c66a0e0fb9 100644 > --- a/drivers/usb/serial/cp210x.c > +++ b/drivers/usb/serial/cp210x.c > @@ -380,6 +380,9 @@ static struct usb_serial_driver * const serial_drivers[] = { > #define CP210X_PARTNUM_CP2102N_QFN20 0x22 > #define CP210X_PARTNUM_UNKNOWN 0xFF > > +#define CP210X_VSTART 0x11 > +#define CP210X_VSTOP 0x13 > + > /* CP210X_GET_COMM_STATUS returns these 0x13 bytes */ > struct cp210x_comm_status { > __le32 ulErrors; > @@ -391,6 +394,15 @@ struct cp210x_comm_status { > u8 bReserved; > } __packed; > > +struct cp210x_chars_response { > + u8 eofchar; > + u8 errochar; > + u8 breakchar; > + u8 eventchar; > + u8 xonchar; > + u8 xoffchar; > +} __packed; > + > /* > * CP210X_PURGE - 16 bits passed in wValue of USB request. > * SiLabs app note AN571 gives a strange description of the 4 bits: > @@ -624,6 +636,45 @@ static int cp210x_read_vendor_block(struct usb_serial *serial, u8 type, u16 val, > return result; > } > > +/* > + * Read and Write Character Responses operate > + * Register SET_CHARS/GET_CHATS > + */ > +static int cp210x_operate_chars_block(struct usb_serial_port *port, > + u8 req, u8 type, void *buf, int bufsize) > +{ > + struct usb_serial *serial = port->serial; > + struct cp210x_port_private *port_priv = usb_get_serial_port_data(port); > + void *dmabuf; > + int result; > + > + dmabuf = kmemdup(buf, bufsize, GFP_KERNEL); > + if (!dmabuf) > + return -ENOMEM; > + > + result = usb_control_msg(serial->dev, > + usb_rcvctrlpipe(serial->dev, 0), > + req, type, 0, port_priv->bInterfaceNumber, > + dmabuf, bufsize, USB_CTRL_SET_TIMEOUT); > + > + if (result == bufsize) { > + if (type == REQTYPE_DEVICE_TO_HOST) > + memcpy(buf, dmabuf, bufsize); > + > + result = 0; > + } else { > + dev_err(&port->dev, "failed get req 0x%x size %d status: %d\n", > + req, bufsize, result); > + if (result >= 0) > + result = -EIO; > + > + } > + > + kfree(dmabuf); > + > + return result; > +} > + > /* > * Writes any 16-bit CP210X_ register (req) whose value is passed > * entirely in the wValue field of the USB request. > @@ -1134,11 +1185,17 @@ static void cp210x_set_termios(struct tty_struct *tty, > struct usb_serial_port *port, struct ktermios *old_termios) > { > struct device *dev = &port->dev; > - unsigned int cflag, old_cflag; > + struct cp210x_chars_response charsres; > + struct cp210x_flow_ctl flow_ctl; > + unsigned int cflag, old_cflag, iflag; > u16 bits; > + int result; > + u32 ctl_hs; > + u32 flow_repl; > > cflag = tty->termios.c_cflag; > old_cflag = old_termios->c_cflag; > + iflag = tty->termios.c_iflag; > > if (tty->termios.c_ospeed != old_termios->c_ospeed) > cp210x_change_speed(tty, port, old_termios); > @@ -1212,10 +1269,6 @@ static void cp210x_set_termios(struct tty_struct *tty, > } > > if ((cflag & CRTSCTS) != (old_cflag & CRTSCTS)) { > - struct cp210x_flow_ctl flow_ctl; > - u32 ctl_hs; > - u32 flow_repl; > - > cp210x_read_reg_block(port, CP210X_GET_FLOW, &flow_ctl, > sizeof(flow_ctl)); > ctl_hs = le32_to_cpu(flow_ctl.ulControlHandshake); > @@ -1252,6 +1305,61 @@ static void cp210x_set_termios(struct tty_struct *tty, > sizeof(flow_ctl)); > } > > + /* > + * Set Software Flow Control > + * Check the IXOFF/IXON status in the iflag component of the > + * termios structure. > + * > + */ > + if ((iflag & IXOFF) || (iflag & IXON)) { > + > + result = cp210x_operate_chars_block(port, > + CP210X_GET_CHARS, > + REQTYPE_DEVICE_TO_HOST, > + &charsres, > + sizeof(charsres)); > + > + if (result < 0) { > + dev_err(dev, "Read Characrter Responses failed\n"); > + return; > + } > + charsres.xonchar = CP210X_VSTART; > + charsres.xoffchar = CP210X_VSTOP; > + result = cp210x_operate_chars_block(port, > + CP210X_SET_CHARS, > + REQTYPE_HOST_TO_INTERFACE, > + &charsres, > + sizeof(charsres)); > + if (result < 0) { > + memset(&charsres, 0, sizeof(charsres)); > + dev_err(dev, "Write Characrter Responses failed\n"); > + return; > + } > + > + /*Set Rx/Tx Flow Contrl Flag in ulFlowReplace*/ > + cp210x_read_reg_block(port, > + CP210X_GET_FLOW, > + &flow_ctl, > + sizeof(flow_ctl)); > + > + flow_repl = le32_to_cpu(flow_ctl.ulFlowReplace); > + > + if (iflag & IXOFF) > + flow_repl |= CP210X_SERIAL_AUTO_RECEIVE; > + else > + flow_repl &= ~CP210X_SERIAL_AUTO_RECEIVE; > + > + if (iflag & IXON) > + flow_repl |= CP210X_SERIAL_AUTO_TRANSMIT; > + else > + flow_repl &= ~CP210X_SERIAL_AUTO_TRANSMIT; > + > + flow_ctl.ulFlowReplace = cpu_to_le32(flow_repl); > + cp210x_write_reg_block(port, > + CP210X_SET_FLOW, > + &flow_ctl, > + sizeof(flow_ctl)); > + } > } > > static int cp210x_tiocmset(struct tty_struct *tty, >
On Fri, Aug 21, 2020 at 07:32:58AM +0200, Jan Kiszka wrote: > On 20.08.20 09:52, Sheng Long Wang wrote: > > From: Wang Sheng Long <shenglong.wang.ext@siemens.com> > > > > When data is transmitted between two serial ports, > > the phenomenon of data loss often occurs. The two kinds > > of flow control commonly used in serial communication > > are hardware flow control and software flow control. > > > > In serial communication, If you only use RX/TX/GND Pins, you > > can't do hardware flow. So we often used software flow control > > and prevent data loss. The user sets the software flow control > > through the application program, and the application program > > sets the software flow control mode for the serial port > > chip through the driver. > > > > For the cp210 serial port chip, its driver lacks the > > software flow control setting code, so the user cannot set > > the software flow control function through the application > > program. This adds the missing software flow control. > > > > Signed-off-by: Wang Sheng Long <shenglong.wang.ext@siemens.com> > > > > Changes in v3: > > -fixed code style, It mainly adjusts the code style acccording > > to kernel specification. > > Patch does not apply. You forgot to rebase over latest tty/tty-next or > linux master. That should be the usb-next branch of the usb-serial tree: https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git/ or linux-next (or, currently, Linus's master branch). You can use "scripts/get_maintainer.sh --scm" to determine which tree to base your work against. Johan
Hi Johan, Thanks for your reminding. I am adjusting patch according to the latest Linux Master branch. With best regards, Wang Sheng Long Siemens Ltd., China RC-CN DI FA R&D SW Tianyuan road No.99 611731 CHENGDU, China Mobil: +86 15281074996 mailto:shenglong.wang.ext@siemens.com -----Original Message----- From: Johan Hovold <johan@kernel.org> Sent: Monday, August 24, 2020 5:10 PM To: Kiszka, Jan (CT RDA IOT SES-DE) <jan.kiszka@siemens.com> Cc: Sheng Long Wang <china_shenglong@163.com>; Wang, Sheng Long (EXT) (RC-CN DI FA R&D SW) <shenglong.wang.ext@siemens.com>; johan@kernel.org; gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org Subject: Re: [PATCH v3] usb-serial:cp210x: add support to software flow control On Fri, Aug 21, 2020 at 07:32:58AM +0200, Jan Kiszka wrote: > On 20.08.20 09:52, Sheng Long Wang wrote: > > From: Wang Sheng Long <shenglong.wang.ext@siemens.com> > > > > When data is transmitted between two serial ports, the phenomenon of > > data loss often occurs. The two kinds of flow control commonly used > > in serial communication are hardware flow control and software flow > > control. > > > > In serial communication, If you only use RX/TX/GND Pins, you can't > > do hardware flow. So we often used software flow control and prevent > > data loss. The user sets the software flow control through the > > application program, and the application program sets the software > > flow control mode for the serial port chip through the driver. > > > > For the cp210 serial port chip, its driver lacks the software flow > > control setting code, so the user cannot set the software flow > > control function through the application program. This adds the > > missing software flow control. > > > > Signed-off-by: Wang Sheng Long <shenglong.wang.ext@siemens.com> > > > > Changes in v3: > > -fixed code style, It mainly adjusts the code style acccording to > > kernel specification. > > Patch does not apply. You forgot to rebase over latest tty/tty-next or > linux master. That should be the usb-next branch of the usb-serial tree: https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git/ or linux-next (or, currently, Linus's master branch). You can use "scripts/get_maintainer.sh --scm" to determine which tree to base your work against. Johan
On 24.08.20 11:28, Wang, Sheng Long (EXT) (RC-CN DI FA R&D SW) wrote: > Hi Johan, > > Thanks for your reminding. I am adjusting patch according to the latest Linux Master branch. Use git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git, branch is likely usb-linus, as Johan requested - just in case there is another conflict due to a patch not yet in Linux master but in that tree. > > With best regards, > Wang Sheng Long > Siemens Ltd., China > RC-CN DI FA R&D SW > Tianyuan road No.99 > 611731 CHENGDU, China > Mobil: +86 15281074996 > mailto:shenglong.wang.ext@siemens.com > > > > -----Original Message----- > From: Johan Hovold <johan@kernel.org> > Sent: Monday, August 24, 2020 5:10 PM > To: Kiszka, Jan (CT RDA IOT SES-DE) <jan.kiszka@siemens.com> > Cc: Sheng Long Wang <china_shenglong@163.com>; Wang, Sheng Long (EXT) (RC-CN DI FA R&D SW) <shenglong.wang.ext@siemens.com>; johan@kernel.org; gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH v3] usb-serial:cp210x: add support to software flow control > > On Fri, Aug 21, 2020 at 07:32:58AM +0200, Jan Kiszka wrote: >> On 20.08.20 09:52, Sheng Long Wang wrote: >>> From: Wang Sheng Long <shenglong.wang.ext@siemens.com> >>> >>> When data is transmitted between two serial ports, the phenomenon of >>> data loss often occurs. The two kinds of flow control commonly used >>> in serial communication are hardware flow control and software flow >>> control. >>> >>> In serial communication, If you only use RX/TX/GND Pins, you can't >>> do hardware flow. So we often used software flow control and prevent >>> data loss. The user sets the software flow control through the >>> application program, and the application program sets the software >>> flow control mode for the serial port chip through the driver. >>> >>> For the cp210 serial port chip, its driver lacks the software flow >>> control setting code, so the user cannot set the software flow >>> control function through the application program. This adds the >>> missing software flow control. >>> >>> Signed-off-by: Wang Sheng Long <shenglong.wang.ext@siemens.com> >>> >>> Changes in v3: >>> -fixed code style, It mainly adjusts the code style acccording to >>> kernel specification. >> >> Patch does not apply. You forgot to rebase over latest tty/tty-next or >> linux master. > > That should be the usb-next branch of the usb-serial tree: > > https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git/ > > or linux-next (or, currently, Linus's master branch). > > You can use "scripts/get_maintainer.sh --scm" to determine which tree to base your work against. Thanks for correcting! But it's scripts/get_maintainer.pl. ;) Jan
On Thu, Aug 20, 2020 at 03:52:40PM +0800, Sheng Long Wang wrote: > From: Wang Sheng Long <shenglong.wang.ext@siemens.com> > > When data is transmitted between two serial ports, > the phenomenon of data loss often occurs. The two kinds > of flow control commonly used in serial communication > are hardware flow control and software flow control. > > In serial communication, If you only use RX/TX/GND Pins, you > can't do hardware flow. So we often used software flow control > and prevent data loss. The user sets the software flow control > through the application program, and the application program > sets the software flow control mode for the serial port > chip through the driver. > > For the cp210 serial port chip, its driver lacks the > software flow control setting code, so the user cannot set > the software flow control function through the application > program. This adds the missing software flow control. > > Signed-off-by: Wang Sheng Long <shenglong.wang.ext@siemens.com> > > Changes in v3: > -fixed code style, It mainly adjusts the code style acccording > to kernel specification. > --- > drivers/usb/serial/cp210x.c | 118 ++++++++++++++++++++++++++++++++++-- > 1 file changed, 113 insertions(+), 5 deletions(-) > > diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c > index e732949f65..c66a0e0fb9 100644 > --- a/drivers/usb/serial/cp210x.c > +++ b/drivers/usb/serial/cp210x.c > @@ -380,6 +380,9 @@ static struct usb_serial_driver * const serial_drivers[] = { > #define CP210X_PARTNUM_CP2102N_QFN20 0x22 > #define CP210X_PARTNUM_UNKNOWN 0xFF > > +#define CP210X_VSTART 0x11 > +#define CP210X_VSTOP 0x13 These should come from the termios settings. > + > /* CP210X_GET_COMM_STATUS returns these 0x13 bytes */ > struct cp210x_comm_status { > __le32 ulErrors; > @@ -391,6 +394,15 @@ struct cp210x_comm_status { > u8 bReserved; > } __packed; > > +struct cp210x_chars_response { Please rename cp210x_special_chars. > + u8 eofchar; > + u8 errochar; > + u8 breakchar; > + u8 eventchar; > + u8 xonchar; > + u8 xoffchar; Please revert to using the field names from the spec for consistency. > +} __packed; No need for __packed. > + > /* > * CP210X_PURGE - 16 bits passed in wValue of USB request. > * SiLabs app note AN571 gives a strange description of the 4 bits: > @@ -624,6 +636,45 @@ static int cp210x_read_vendor_block(struct usb_serial *serial, u8 type, u16 val, > return result; > } > > +/* > + * Read and Write Character Responses operate > + * Register SET_CHARS/GET_CHATS This comment makes very little sense to me. Please fix up (including typos) or drop. > + */ > +static int cp210x_operate_chars_block(struct usb_serial_port *port, > + u8 req, u8 type, void *buf, int bufsize) > +{ > + struct usb_serial *serial = port->serial; > + struct cp210x_port_private *port_priv = usb_get_serial_port_data(port); > + void *dmabuf; > + int result; > + > + dmabuf = kmemdup(buf, bufsize, GFP_KERNEL); > + if (!dmabuf) > + return -ENOMEM; > + > + result = usb_control_msg(serial->dev, > + usb_rcvctrlpipe(serial->dev, 0), This should be usb_sndctrlpipe() when updating the settings. > + req, type, 0, port_priv->bInterfaceNumber, > + dmabuf, bufsize, USB_CTRL_SET_TIMEOUT); > + > + if (result == bufsize) { > + if (type == REQTYPE_DEVICE_TO_HOST) > + memcpy(buf, dmabuf, bufsize); > + > + result = 0; > + } else { > + dev_err(&port->dev, "failed get req 0x%x size %d status: %d\n", > + req, bufsize, result); > + if (result >= 0) > + result = -EIO; > + Spurious newline. > + } > + > + kfree(dmabuf); > + > + return result; > +} Replace this with two dedicated helpers cp210x_get_chars() and cp210x_set_chars(). > + > /* > * Writes any 16-bit CP210X_ register (req) whose value is passed > * entirely in the wValue field of the USB request. > @@ -1134,11 +1185,17 @@ static void cp210x_set_termios(struct tty_struct *tty, > struct usb_serial_port *port, struct ktermios *old_termios) > { > struct device *dev = &port->dev; > - unsigned int cflag, old_cflag; > + struct cp210x_chars_response charsres; > + struct cp210x_flow_ctl flow_ctl; > + unsigned int cflag, old_cflag, iflag; > u16 bits; > + int result; > + u32 ctl_hs; > + u32 flow_repl; > > cflag = tty->termios.c_cflag; > old_cflag = old_termios->c_cflag; > + iflag = tty->termios.c_iflag; > > if (tty->termios.c_ospeed != old_termios->c_ospeed) > cp210x_change_speed(tty, port, old_termios); > @@ -1212,10 +1269,6 @@ static void cp210x_set_termios(struct tty_struct *tty, > } > > if ((cflag & CRTSCTS) != (old_cflag & CRTSCTS)) { > - struct cp210x_flow_ctl flow_ctl; > - u32 ctl_hs; > - u32 flow_repl; > - > cp210x_read_reg_block(port, CP210X_GET_FLOW, &flow_ctl, > sizeof(flow_ctl)); > ctl_hs = le32_to_cpu(flow_ctl.ulControlHandshake); > @@ -1252,6 +1305,61 @@ static void cp210x_set_termios(struct tty_struct *tty, > sizeof(flow_ctl)); > } > > + /* > + * Set Software Flow Control > + * Check the IXOFF/IXON status in the iflag component of the > + * termios structure. > + * > + */ Drop this comment. > + if ((iflag & IXOFF) || (iflag & IXON)) { > + > + result = cp210x_operate_chars_block(port, > + CP210X_GET_CHARS, > + REQTYPE_DEVICE_TO_HOST, > + &charsres, > + sizeof(charsres)); > + > + if (result < 0) { > + dev_err(dev, "Read Characrter Responses failed\n"); Please run your patch through a spell checker. And no need for Capital letters everywhere. > + return; You shouldn't just return here (more important with latest driver). > + } > + charsres.xonchar = CP210X_VSTART; > + charsres.xoffchar = CP210X_VSTOP; These should come from termios (see START_CHAR() and STOP_CHAR()). > + result = cp210x_operate_chars_block(port, > + CP210X_SET_CHARS, > + REQTYPE_HOST_TO_INTERFACE, > + &charsres, > + sizeof(charsres)); > + if (result < 0) { > + memset(&charsres, 0, sizeof(charsres)); Not needed. > + dev_err(dev, "Write Characrter Responses failed\n"); > + return; > + } > + > + /*Set Rx/Tx Flow Contrl Flag in ulFlowReplace*/ Spaces after and before *. Random spaces mid sentence. Capitalisation... And generally not useful, please drop it. > + cp210x_read_reg_block(port, > + CP210X_GET_FLOW, > + &flow_ctl, > + sizeof(flow_ctl)); Error handling. > + > + flow_repl = le32_to_cpu(flow_ctl.ulFlowReplace); > + > + if (iflag & IXOFF) > + flow_repl |= CP210X_SERIAL_AUTO_RECEIVE; > + else > + flow_repl &= ~CP210X_SERIAL_AUTO_RECEIVE; > + > + if (iflag & IXON) > + flow_repl |= CP210X_SERIAL_AUTO_TRANSMIT; > + else > + flow_repl &= ~CP210X_SERIAL_AUTO_TRANSMIT; > + > + flow_ctl.ulFlowReplace = cpu_to_le32(flow_repl); > + cp210x_write_reg_block(port, > + CP210X_SET_FLOW, > + &flow_ctl, > + sizeof(flow_ctl)); Error handling? > + } > } > > static int cp210x_tiocmset(struct tty_struct *tty, Johan
On Mon, Aug 24, 2020 at 11:36:50AM +0200, Jan Kiszka wrote: > On 24.08.20 11:28, Wang, Sheng Long (EXT) (RC-CN DI FA R&D SW) wrote: > > Hi Johan, > > > > Thanks for your reminding. I am adjusting patch according to the latest Linux Master branch. > > Use git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git, branch > is likely usb-linus, as Johan requested - just in case there is another > conflict due to a patch not yet in Linux master but in that tree. The usb-next is used for new features like this one, but again, currently Linus's master branch works as well. > > -----Original Message----- > > From: Johan Hovold <johan@kernel.org> > > Sent: Monday, August 24, 2020 5:10 PM > > To: Kiszka, Jan (CT RDA IOT SES-DE) <jan.kiszka@siemens.com> > > Cc: Sheng Long Wang <china_shenglong@163.com>; Wang, Sheng Long (EXT) (RC-CN DI FA R&D SW) <shenglong.wang.ext@siemens.com>; johan@kernel.org; gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org > > Subject: Re: [PATCH v3] usb-serial:cp210x: add support to software flow control > > > > On Fri, Aug 21, 2020 at 07:32:58AM +0200, Jan Kiszka wrote: > >> On 20.08.20 09:52, Sheng Long Wang wrote: > >>> From: Wang Sheng Long <shenglong.wang.ext@siemens.com> > >>> > >>> When data is transmitted between two serial ports, the phenomenon of > >>> data loss often occurs. The two kinds of flow control commonly used > >>> in serial communication are hardware flow control and software flow > >>> control. > >>> > >>> In serial communication, If you only use RX/TX/GND Pins, you can't > >>> do hardware flow. So we often used software flow control and prevent > >>> data loss. The user sets the software flow control through the > >>> application program, and the application program sets the software > >>> flow control mode for the serial port chip through the driver. > >>> > >>> For the cp210 serial port chip, its driver lacks the software flow > >>> control setting code, so the user cannot set the software flow > >>> control function through the application program. This adds the > >>> missing software flow control. > >>> > >>> Signed-off-by: Wang Sheng Long <shenglong.wang.ext@siemens.com> > >>> > >>> Changes in v3: > >>> -fixed code style, It mainly adjusts the code style acccording to > >>> kernel specification. > >> > >> Patch does not apply. You forgot to rebase over latest tty/tty-next or > >> linux master. > > > > That should be the usb-next branch of the usb-serial tree: > > > > https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git/ > > > > or linux-next (or, currently, Linus's master branch). > > > > You can use "scripts/get_maintainer.sh --scm" to determine which tree to base your work against. > > Thanks for correcting! But it's scripts/get_maintainer.pl. ;) Heh. Thanks for catching that. Johan
Hi, Johan So, Is it currently possible for me to use which Branch? :) https://github.com/torvalds/linux master branch Or use https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git Thanks! BR/Sheng Long -----Original Message----- From: Johan Hovold <johan@kernel.org> Sent: Monday, August 24, 2020 5:43 PM To: Kiszka, Jan (CT RDA IOT SES-DE) <jan.kiszka@siemens.com> Cc: Wang, Sheng Long (EXT) (RC-CN DI FA R&D SW) <shenglong.wang.ext@siemens.com>; Johan Hovold <johan@kernel.org>; Sheng Long Wang <china_shenglong@163.com>; gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org Subject: Re: [PATCH v3] usb-serial:cp210x: add support to software flow control On Mon, Aug 24, 2020 at 11:36:50AM +0200, Jan Kiszka wrote: > On 24.08.20 11:28, Wang, Sheng Long (EXT) (RC-CN DI FA R&D SW) wrote: > > Hi Johan, > > > > Thanks for your reminding. I am adjusting patch according to the latest Linux Master branch. > > Use git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git, > branch is likely usb-linus, as Johan requested - just in case there is > another conflict due to a patch not yet in Linux master but in that tree. The usb-next is used for new features like this one, but again, currently Linus's master branch works as well. > > -----Original Message----- > > From: Johan Hovold <johan@kernel.org> > > Sent: Monday, August 24, 2020 5:10 PM > > To: Kiszka, Jan (CT RDA IOT SES-DE) <jan.kiszka@siemens.com> > > Cc: Sheng Long Wang <china_shenglong@163.com>; Wang, Sheng Long > > (EXT) (RC-CN DI FA R&D SW) <shenglong.wang.ext@siemens.com>; > > johan@kernel.org; gregkh@linuxfoundation.org; > > linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org > > Subject: Re: [PATCH v3] usb-serial:cp210x: add support to software > > flow control > > > > On Fri, Aug 21, 2020 at 07:32:58AM +0200, Jan Kiszka wrote: > >> On 20.08.20 09:52, Sheng Long Wang wrote: > >>> From: Wang Sheng Long <shenglong.wang.ext@siemens.com> > >>> > >>> When data is transmitted between two serial ports, the phenomenon > >>> of data loss often occurs. The two kinds of flow control commonly > >>> used in serial communication are hardware flow control and > >>> software flow control. > >>> > >>> In serial communication, If you only use RX/TX/GND Pins, you can't > >>> do hardware flow. So we often used software flow control and > >>> prevent data loss. The user sets the software flow control through > >>> the application program, and the application program sets the > >>> software flow control mode for the serial port chip through the driver. > >>> > >>> For the cp210 serial port chip, its driver lacks the software flow > >>> control setting code, so the user cannot set the software flow > >>> control function through the application program. This adds the > >>> missing software flow control. > >>> > >>> Signed-off-by: Wang Sheng Long <shenglong.wang.ext@siemens.com> > >>> > >>> Changes in v3: > >>> -fixed code style, It mainly adjusts the code style acccording to > >>> kernel specification. > >> > >> Patch does not apply. You forgot to rebase over latest tty/tty-next > >> or linux master. > > > > That should be the usb-next branch of the usb-serial tree: > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git > > / > > > > or linux-next (or, currently, Linus's master branch). > > > > You can use "scripts/get_maintainer.sh --scm" to determine which tree to base your work against. > > Thanks for correcting! But it's scripts/get_maintainer.pl. ;) Heh. Thanks for catching that. Johan
On Mon, Aug 24, 2020 at 10:16:13AM +0000, Wang, Sheng Long wrote: > Hi, Johan > > So, Is it currently possible for me to use which Branch? :) > > https://github.com/torvalds/linux master branch > > Or use https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git Please use the usb-next branch from https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git Johan
diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c index e732949f65..c66a0e0fb9 100644 --- a/drivers/usb/serial/cp210x.c +++ b/drivers/usb/serial/cp210x.c @@ -380,6 +380,9 @@ static struct usb_serial_driver * const serial_drivers[] = { #define CP210X_PARTNUM_CP2102N_QFN20 0x22 #define CP210X_PARTNUM_UNKNOWN 0xFF +#define CP210X_VSTART 0x11 +#define CP210X_VSTOP 0x13 + /* CP210X_GET_COMM_STATUS returns these 0x13 bytes */ struct cp210x_comm_status { __le32 ulErrors; @@ -391,6 +394,15 @@ struct cp210x_comm_status { u8 bReserved; } __packed; +struct cp210x_chars_response { + u8 eofchar; + u8 errochar; + u8 breakchar; + u8 eventchar; + u8 xonchar; + u8 xoffchar; +} __packed; + /* * CP210X_PURGE - 16 bits passed in wValue of USB request. * SiLabs app note AN571 gives a strange description of the 4 bits: @@ -624,6 +636,45 @@ static int cp210x_read_vendor_block(struct usb_serial *serial, u8 type, u16 val, return result; } +/* + * Read and Write Character Responses operate + * Register SET_CHARS/GET_CHATS + */ +static int cp210x_operate_chars_block(struct usb_serial_port *port, + u8 req, u8 type, void *buf, int bufsize) +{ + struct usb_serial *serial = port->serial; + struct cp210x_port_private *port_priv = usb_get_serial_port_data(port); + void *dmabuf; + int result; + + dmabuf = kmemdup(buf, bufsize, GFP_KERNEL); + if (!dmabuf) + return -ENOMEM; + + result = usb_control_msg(serial->dev, + usb_rcvctrlpipe(serial->dev, 0), + req, type, 0, port_priv->bInterfaceNumber, + dmabuf, bufsize, USB_CTRL_SET_TIMEOUT); + + if (result == bufsize) { + if (type == REQTYPE_DEVICE_TO_HOST) + memcpy(buf, dmabuf, bufsize); + + result = 0; + } else { + dev_err(&port->dev, "failed get req 0x%x size %d status: %d\n", + req, bufsize, result); + if (result >= 0) + result = -EIO; + + } + + kfree(dmabuf); + + return result; +} + /* * Writes any 16-bit CP210X_ register (req) whose value is passed * entirely in the wValue field of the USB request. @@ -1134,11 +1185,17 @@ static void cp210x_set_termios(struct tty_struct *tty, struct usb_serial_port *port, struct ktermios *old_termios) { struct device *dev = &port->dev; - unsigned int cflag, old_cflag; + struct cp210x_chars_response charsres; + struct cp210x_flow_ctl flow_ctl; + unsigned int cflag, old_cflag, iflag; u16 bits; + int result; + u32 ctl_hs; + u32 flow_repl; cflag = tty->termios.c_cflag; old_cflag = old_termios->c_cflag; + iflag = tty->termios.c_iflag; if (tty->termios.c_ospeed != old_termios->c_ospeed) cp210x_change_speed(tty, port, old_termios); @@ -1212,10 +1269,6 @@ static void cp210x_set_termios(struct tty_struct *tty, } if ((cflag & CRTSCTS) != (old_cflag & CRTSCTS)) { - struct cp210x_flow_ctl flow_ctl; - u32 ctl_hs; - u32 flow_repl; - cp210x_read_reg_block(port, CP210X_GET_FLOW, &flow_ctl, sizeof(flow_ctl)); ctl_hs = le32_to_cpu(flow_ctl.ulControlHandshake); @@ -1252,6 +1305,61 @@ static void cp210x_set_termios(struct tty_struct *tty, sizeof(flow_ctl)); } + /* + * Set Software Flow Control + * Check the IXOFF/IXON status in the iflag component of the + * termios structure. + * + */ + if ((iflag & IXOFF) || (iflag & IXON)) { + + result = cp210x_operate_chars_block(port, + CP210X_GET_CHARS, + REQTYPE_DEVICE_TO_HOST, + &charsres, + sizeof(charsres)); + + if (result < 0) { + dev_err(dev, "Read Characrter Responses failed\n"); + return; + } + charsres.xonchar = CP210X_VSTART; + charsres.xoffchar = CP210X_VSTOP; + result = cp210x_operate_chars_block(port, + CP210X_SET_CHARS, + REQTYPE_HOST_TO_INTERFACE, + &charsres, + sizeof(charsres)); + if (result < 0) { + memset(&charsres, 0, sizeof(charsres)); + dev_err(dev, "Write Characrter Responses failed\n"); + return; + } + + /*Set Rx/Tx Flow Contrl Flag in ulFlowReplace*/ + cp210x_read_reg_block(port, + CP210X_GET_FLOW, + &flow_ctl, + sizeof(flow_ctl)); + + flow_repl = le32_to_cpu(flow_ctl.ulFlowReplace); + + if (iflag & IXOFF) + flow_repl |= CP210X_SERIAL_AUTO_RECEIVE; + else + flow_repl &= ~CP210X_SERIAL_AUTO_RECEIVE; + + if (iflag & IXON) + flow_repl |= CP210X_SERIAL_AUTO_TRANSMIT; + else + flow_repl &= ~CP210X_SERIAL_AUTO_TRANSMIT; + + flow_ctl.ulFlowReplace = cpu_to_le32(flow_repl); + cp210x_write_reg_block(port, + CP210X_SET_FLOW, + &flow_ctl, + sizeof(flow_ctl)); + } } static int cp210x_tiocmset(struct tty_struct *tty,