On Mon, Jan 11, 2021 at 04:55:22AM +0000, Pho Tran wrote:
> When hardware flow control is enabled,
> don't allow host send MHS command to cp210x.
>
> Signed-off-by: Pho Tran<pho.tran@xxxxxxxxxx>
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(&port->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, &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(&port->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
