Re: [PATCH] USB: otg: Fix error 32 when enable hardware flow control.
On Mon, Jan 11, 2021 at 04:55:22AM +, Pho Tran wrote: > When hardware flow control is enabled, > don't allow host send MHS command to cp210x. You're commit message needs to explain *why* this is needed (and not just say *what* the patch does). > Signed-off-by: Pho Tran Also fix up the Subject prefix. Johan
Re: [PATCH] USB: otg: Fix error 32 when enable hardware flow control.
On Mon, Jan 11, 2021 at 04:55:22AM +, Pho Tran wrote:
> When hardware flow control is enabled,
> don't allow host send MHS command to cp210x.
>
> Signed-off-by: Pho Tran
Nit, you need a ' ' before the '<' character.
And why didn't you send this to the maintainer of this file as described
by scripts/get_maintainer.pl?
Please do so when you fix things up and send the next version.
> ---
> drivers/usb/serial/cp210x.c | 32 +++-
> 1 file changed, 31 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
> index fbb10dfc56e3..f231cecdaf7d 100644
> --- a/drivers/usb/serial/cp210x.c
> +++ b/drivers/usb/serial/cp210x.c
> @@ -1204,6 +1204,7 @@ static int cp210x_tiocmset(struct tty_struct *tty,
> unsigned int set, unsigned int clear)
> {
> struct usb_serial_port *port = tty->driver_data;
> +
> return cp210x_tiocmset_port(port, set, clear);
> }
>
Unneeded change :(
> @@ -1211,6 +1212,11 @@ static int cp210x_tiocmset_port(struct usb_serial_port
> *port,
> unsigned int set, unsigned int clear)
> {
> u16 control = 0;
> + struct cp210x_flow_ctl flow_ctl;
> + u32 ctl_hs = 0;
> + u32 flow_repl = 0;
> + bool auto_dtr = false;
> + bool auto_rts = false;
>
> if (set & TIOCM_RTS) {
> control |= CONTROL_RTS;
> @@ -1230,6 +1236,30 @@ static int cp210x_tiocmset_port(struct usb_serial_port
> *port,
> }
>
> dev_dbg(>dev, "%s - control = 0x%.4x\n", __func__, control);
> + /*
> + * Don't send MHS command if device in hardware flowcontrol mode
> + */
Why the giant comment?
> + cp210x_read_reg_block(port, CP210X_GET_FLOW, _ctl,
> + sizeof(flow_ctl));
> + ctl_hs = le32_to_cpu(flow_ctl.ulControlHandshake);
> + flow_repl = le32_to_cpu(flow_ctl.ulFlowReplace);
> +
> + if (CP210X_SERIAL_DTR_SHIFT(CP210X_SERIAL_DTR_FLOW_CTL)
> + == (ctl_hs & CP210X_SERIAL_DTR_MASK))
Very odd indentation :(
> + auto_dtr = true;
> + else
> + auto_dtr = false;
> +
> + if (CP210X_SERIAL_RTS_SHIFT(CP210X_SERIAL_RTS_FLOW_CTL)
> + == (flow_repl & CP210X_SERIAL_RTS_MASK))
> + auto_rts = true;
> + else
> + auto_rts = false;
> +
> + if (auto_dtr || auto_rts) {
> + dev_dbg(>dev, "Don't set MHS when while device in flow
> control mode\n");
> + return 0;
> + }
>
> return cp210x_write_u16_reg(port, CP210X_SET_MHS, control);
> }
> @@ -1256,7 +1286,7 @@ static int cp210x_tiocmget(struct tty_struct *tty)
> |((control & CONTROL_RTS) ? TIOCM_RTS : 0)
> |((control & CONTROL_CTS) ? TIOCM_CTS : 0)
> |((control & CONTROL_DSR) ? TIOCM_DSR : 0)
> - |((control & CONTROL_RING)? TIOCM_RI : 0)
> + |((control & CONTROL_RING) ? TIOCM_RI : 0)
Do not mix whitespace changes with logic changes in the same patch, that
is a sure way to get your patch rejected.
thanks,
greg k-h
[PATCH] USB: otg: Fix error 32 when enable hardware flow control.
When hardware flow control is enabled,
don't allow host send MHS command to cp210x.
Signed-off-by: Pho Tran
---
drivers/usb/serial/cp210x.c | 32 +++-
1 file changed, 31 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
index fbb10dfc56e3..f231cecdaf7d 100644
--- a/drivers/usb/serial/cp210x.c
+++ b/drivers/usb/serial/cp210x.c
@@ -1204,6 +1204,7 @@ static int cp210x_tiocmset(struct tty_struct *tty,
unsigned int set, unsigned int clear)
{
struct usb_serial_port *port = tty->driver_data;
+
return cp210x_tiocmset_port(port, set, clear);
}
@@ -1211,6 +1212,11 @@ static int cp210x_tiocmset_port(struct usb_serial_port
*port,
unsigned int set, unsigned int clear)
{
u16 control = 0;
+ struct cp210x_flow_ctl flow_ctl;
+ u32 ctl_hs = 0;
+ u32 flow_repl = 0;
+ bool auto_dtr = false;
+ bool auto_rts = false;
if (set & TIOCM_RTS) {
control |= CONTROL_RTS;
@@ -1230,6 +1236,30 @@ static int cp210x_tiocmset_port(struct usb_serial_port
*port,
}
dev_dbg(>dev, "%s - control = 0x%.4x\n", __func__, control);
+ /*
+* Don't send MHS command if device in hardware flowcontrol mode
+*/
+ cp210x_read_reg_block(port, CP210X_GET_FLOW, _ctl,
+ sizeof(flow_ctl));
+ ctl_hs = le32_to_cpu(flow_ctl.ulControlHandshake);
+ flow_repl = le32_to_cpu(flow_ctl.ulFlowReplace);
+
+ if (CP210X_SERIAL_DTR_SHIFT(CP210X_SERIAL_DTR_FLOW_CTL)
+ == (ctl_hs & CP210X_SERIAL_DTR_MASK))
+ auto_dtr = true;
+ else
+ auto_dtr = false;
+
+ if (CP210X_SERIAL_RTS_SHIFT(CP210X_SERIAL_RTS_FLOW_CTL)
+ == (flow_repl & CP210X_SERIAL_RTS_MASK))
+ auto_rts = true;
+ else
+ auto_rts = false;
+
+ if (auto_dtr || auto_rts) {
+ dev_dbg(>dev, "Don't set MHS when while device in flow
control mode\n");
+ return 0;
+ }
return cp210x_write_u16_reg(port, CP210X_SET_MHS, control);
}
@@ -1256,7 +1286,7 @@ static int cp210x_tiocmget(struct tty_struct *tty)
|((control & CONTROL_RTS) ? TIOCM_RTS : 0)
|((control & CONTROL_CTS) ? TIOCM_CTS : 0)
|((control & CONTROL_DSR) ? TIOCM_DSR : 0)
- |((control & CONTROL_RING)? TIOCM_RI : 0)
+ |((control & CONTROL_RING) ? TIOCM_RI : 0)
|((control & CONTROL_DCD) ? TIOCM_CD : 0);
dev_dbg(>dev, "%s - control = 0x%.2x\n", __func__, control);
--
2.17.1
