On Wed, Nov 11, 2015 at 03:47:21PM -0600, Konstantin Shkolnyy wrote:
Please make the commit message self-contained even if it means repeating
what callback you're implementing here.
> Without this function, when the port is closed the data in the chip's
> transmit FIFO are lost. If the actual byte count is reported the close
> can be delayed until all data are sent.
You could mention that the tx_empty callback is used to implement
generic wait-until-sent support.
> Signed-off-by: Konstantin Shkolnyy <konstantin.shkol...@gmail.com>
> ---
> drivers/usb/serial/cp210x.c | 60
> +++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 60 insertions(+)
>
> diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
> index e91b654..756e432 100644
> --- a/drivers/usb/serial/cp210x.c
> +++ b/drivers/usb/serial/cp210x.c
> @@ -38,6 +38,7 @@ static void cp210x_change_speed(struct tty_struct *, struct
> usb_serial_port *,
> struct ktermios *);
> static void cp210x_set_termios(struct tty_struct *, struct usb_serial_port *,
> struct ktermios*);
> +static bool cp210x_tx_empty(struct usb_serial_port *port);
> static int cp210x_tiocmget(struct tty_struct *);
> static int cp210x_tiocmset(struct tty_struct *, unsigned int, unsigned int);
> static int cp210x_tiocmset_port(struct usb_serial_port *port,
> @@ -215,6 +216,7 @@ static struct usb_serial_driver cp210x_device = {
> .close = cp210x_close,
> .break_ctl = cp210x_break_ctl,
> .set_termios = cp210x_set_termios,
> + .tx_empty = cp210x_tx_empty,
> .tiocmget = cp210x_tiocmget,
> .tiocmset = cp210x_tiocmset,
> .port_probe = cp210x_port_probe,
> @@ -301,6 +303,18 @@ static struct usb_serial_driver * const serial_drivers[]
> = {
> #define CONTROL_WRITE_DTR 0x0100
> #define CONTROL_WRITE_RTS 0x0200
>
> +/* CP210X_GET_COMM_STATUS returns these 0x13 bytes */
> +#define CP210X_COMM_STATUS_SIZE 0x13
> +struct cp210x_comm_status {
> + __le32 ulErrors;
> + __le32 ulHoldReasons;
> + __le32 ulAmountInInQueue;
> + __le32 ulAmountInOutQueue;
> + u8 bEofReceived;
> + u8 bWaitForImmediate;
> + u8 bReserved;
> +};
Add a __packed attribute here so that you can use sizeof and drop the
size-define.
> +
> /*
> * CP210X_PURGE - 16 bits passed in wValue of USB request.
> * SiLabs app note AN571 gives a strange description of the 4 bits:
> @@ -551,6 +565,52 @@ static void cp210x_close(struct usb_serial_port *port)
> }
>
> /*
> + * Read how many bytes are waiting in the TX queue.
> + */
> +static int cp210x_get_tx_queue_byte_count(struct usb_serial_port *port,
> + u32 *count)
Indent continuation lines at least two tabs (also below).
> +{
> + struct usb_serial *serial = port->serial;
> + struct cp210x_port_private *port_priv = usb_get_serial_port_data(port);
> + struct cp210x_comm_status *sts;
> + int result;
> +
> + sts = kmalloc(CP210X_COMM_STATUS_SIZE, GFP_KERNEL);
> + if (!sts)
> + return -ENOMEM;
> +
> + result = usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, 0),
> + CP210X_GET_COMM_STATUS, REQTYPE_INTERFACE_TO_HOST, 0x0000,
Just use 0 here.
> + port_priv->bInterfaceNumber, sts, CP210X_COMM_STATUS_SIZE,
And sizeof(*sts) here (and below).
> + USB_CTRL_GET_TIMEOUT);
> + if (result == CP210X_COMM_STATUS_SIZE) {
> + *count = le32_to_cpu(sts->ulAmountInOutQueue);
> + result = 0;
> + } else {
> + dev_dbg(&port->dev, "%s error: size=%d result=%d\n",
> + __func__, CP210X_COMM_STATUS_SIZE, result);
This should be a dev_err, skip the __func__ and spell out the error
(e.g. "failed to get comm status: %d\n"). No need to report the constant
size.
> + if (result >= 0)
> + result = -EPROTO;
> + }
> +
> + kfree(sts);
> +
> + return result;
> +}
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
