Re: [PATCH RESEND 4/5] USB: serial: cp210x: Prepared get_termios() for adding error handling

[Johan Hovold]

On Sun, Apr 24, 2016 at 12:09:30PM -0500, Konstantin Shkolnyy wrote:
> Replaced several register write calls with one, to simplify adding error
> handling.
> 
> Signed-off-by: Konstantin Shkolnyy 
> ---
>  drivers/usb/serial/cp210x.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
> index b2321a7..58cffc9 100644
> --- a/drivers/usb/serial/cp210x.c
> +++ b/drivers/usb/serial/cp210x.c
> @@ -692,6 +692,7 @@ static void cp210x_get_termios_port(struct 
> usb_serial_port *port,

Instead of cleaning up cp210x_get_termios I think you should consider
removing it.

It is used to read the current settings from the device and in case we
have a tty at open, to update the termios settings to match.

Instead you should just make sure that the device settings match
termios (and this is in fact already implemented, but done after
retrieving the settings).

You can also implement the init_termios callback to provide defaults
that match the hardware reset values.

Thanks,
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH RESEND 4/5] USB: serial: cp210x: Prepared get_termios() for adding error handling

[Konstantin Shkolnyy]

Replaced several register write calls with one, to simplify adding error
handling.

Signed-off-by: Konstantin Shkolnyy 
---
 drivers/usb/serial/cp210x.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
index b2321a7..58cffc9 100644
--- a/drivers/usb/serial/cp210x.c
+++ b/drivers/usb/serial/cp210x.c
@@ -692,6 +692,7 @@ static void cp210x_get_termios_port(struct usb_serial_port 
*port,
unsigned int cflag;
u8 modem_ctl[16];
u32 baud;
+   u16 old_bits;
u16 bits;
 
cp210x_read_u32_reg(port, CP210X_GET_BAUDRATE, );
@@ -702,6 +703,7 @@ static void cp210x_get_termios_port(struct usb_serial_port 
*port,
cflag = *cflagp;
 
cp210x_get_line_ctl(port, );
+   old_bits = bits;
cflag &= ~CSIZE;
switch (bits & BITS_DATA_MASK) {
case BITS_DATA_5:
@@ -725,14 +727,12 @@ static void cp210x_get_termios_port(struct 
usb_serial_port *port,
cflag |= CS8;
bits &= ~BITS_DATA_MASK;
bits |= BITS_DATA_8;
-   cp210x_write_u16_reg(port, CP210X_SET_LINE_CTL, bits);
break;
default:
dev_dbg(dev, "%s - Unknown number of data bits, using 8\n", 
__func__);
cflag |= CS8;
bits &= ~BITS_DATA_MASK;
bits |= BITS_DATA_8;
-   cp210x_write_u16_reg(port, CP210X_SET_LINE_CTL, bits);
break;
}
 
@@ -763,7 +763,6 @@ static void cp210x_get_termios_port(struct usb_serial_port 
*port,
dev_dbg(dev, "%s - Unknown parity mode, disabling parity\n", 
__func__);
cflag &= ~PARENB;
bits &= ~BITS_PARITY_MASK;
-   cp210x_write_u16_reg(port, CP210X_SET_LINE_CTL, bits);
break;
}
 
@@ -775,7 +774,6 @@ static void cp210x_get_termios_port(struct usb_serial_port 
*port,
case BITS_STOP_1_5:
dev_dbg(dev, "%s - stop bits = 1.5 (not supported, using 1 stop 
bit)\n", __func__);
bits &= ~BITS_STOP_MASK;
-   cp210x_write_u16_reg(port, CP210X_SET_LINE_CTL, bits);
break;
case BITS_STOP_2:
dev_dbg(dev, "%s - stop bits = 2\n", __func__);
@@ -784,10 +782,12 @@ static void cp210x_get_termios_port(struct 
usb_serial_port *port,
default:
dev_dbg(dev, "%s - Unknown number of stop bits, using 1 stop 
bit\n", __func__);
bits &= ~BITS_STOP_MASK;
-   cp210x_write_u16_reg(port, CP210X_SET_LINE_CTL, bits);
break;
}
 
+   if (bits != old_bits)
+   cp210x_write_u16_reg(port, CP210X_SET_LINE_CTL, bits);
+
cp210x_read_reg_block(port, CP210X_GET_FLOW, modem_ctl,
sizeof(modem_ctl));
if (modem_ctl[0] & 0x08) { /* if SERIAL_CTS_HANDSHAKE */
-- 
1.8.4.5

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html