]> git.openfabrics.org - ~emulex/infiniband.git/commitdiff
Staging: quatech_usb2: write_room rewrite
authorRichard Ash <richard@audacityteam.org>
Mon, 17 Aug 2009 09:35:18 +0000 (10:35 +0100)
committerGreg Kroah-Hartman <gregkh@suse.de>
Tue, 15 Sep 2009 19:01:36 +0000 (12:01 -0700)
The following patch re-writes the write_room method to work in the
obvious sort of way, so it keeps it's promises to the layers above.
Based on feedback from Alan Cox.

Signed-off-by: Richard Ash <richard@audacityteam.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
drivers/staging/quatech_usb2/quatech_usb2.c

index bbbf9b88fac01c7cfffc4b9c4f42a654d4127b75..55a25e54aee569049aa29c7cab8a27921536c8d3 100644 (file)
@@ -38,7 +38,7 @@ static int debug;
 #define QU2BOXPWRON 0x8000             /* magic number to turn FPGA power on */
 #define QU2BOX232 0x40                 /* RS232 mode on MEI devices */
 #define QU2BOXSPD9600 0x60             /* set speed to 9600 baud */
-#define FIFO_DEPTH 1024                        /* size of hardware fifos */
+#define QT2_FIFO_DEPTH 1024                    /* size of hardware fifos */
 #define QT2_TX_HEADER_LENGTH   5
 /* length of the header sent to the box with each write URB */
 
@@ -146,18 +146,32 @@ static struct usb_driver quausb2_usb_driver = {
  * value of the line status flags from the port
  * @param shadowMSR Last received state of the modem status register, holds
  * the value of the modem status received from the port
- * @param xmit_pending_bytes Number of bytes waiting to be sent out of
- * the serial port
- * @param xmit_fifo_room_bytes free space available in the transmit fifo
- * for this port on the box
  * @param rcv_flush Flag indicating that a receive flush has occured on
  * the hardware.
  * @param xmit_flush Flag indicating that a transmit flush has been processed by
  * the hardware.
- * @param fifo_empty_flag Flag indicating that the (per-port) buffer on the
- * device is empty.
- * @param tx_fifo_room Number of bytes room left in the buffer
- * @param tx_pending_bytes Number of bytes waiting to be sent
+ * @param fifo_empty_flag
+ * - Starts off true when port opened
+ * - set false when a write is submitted to the driver
+ * - as far as I can see not true again until device is re-opened.
+ * - read in a number of places.
+ * @param tx_fifo_room
+ * - set to FIFO_DEPTH when port opened
+ * - decremented by tc_pending_bytes when a new write is submitted.
+ * - set to FIFO_DEPTH when "xmit_empty" is received from the device,
+ * regardless of how many bytes were reported to have been sent (?)
+ *
+ * @param tx_pending_bytes Number of bytes waiting to be sent. This total
+ * includes the size (excluding header) of URBs that have been submitted but
+ * have not yet been sent to to the device, and bytes that have been sent out
+ * of the port but not yet reported sent by the "xmit_empty" messages (which
+ * indicate the number of bytes sent each time they are recieved, despite the
+ * misleading name).
+ * - Starts at zero when port is initialised.
+ * - is incremented by the size of the data to be written (no headers)
+ * each time a write urb is dispatched.
+ * - is decremented each time a "transmit empty" message is received
+ * by the driver in the data stream.
  */
 struct quatech2_port {
        int     magic;
@@ -165,8 +179,7 @@ struct quatech2_port {
        bool    close_pending;
        __u8    shadowLSR;
        __u8    shadowMSR;
-       int     xmit_pending_bytes;
-       int     xmit_fifo_room_bytes;
+       /*int   xmit_fifo_room_bytes;*/
        bool    rcv_flush;
        bool    xmit_flush;
 /*     bool    fifo_empty_flag;
@@ -723,11 +736,10 @@ static int qt2_write(struct tty_struct *tty, struct usb_serial_port *port,
                        "-EINPROGRESS", __func__);
                /* schedule_work(&port->work); commented in vendor driver */
                return 0;
-       } else if (port_extra->tx_pending_bytes >= FIFO_DEPTH) {
-               /* such a lot queued up that we will fill the buffer again as
-                * soon as it does empty? Overflowed buffer? */
-               dbg("%s(): already writing, port_extra->tx_pending_bytes >="
-                       " FIFO_DEPTH", __func__);
+       } else if (port_extra->tx_pending_bytes >= QT2_FIFO_DEPTH) {
+               /* buffer is full (==). > should not occur, but would indicate
+                * that an overflow had occured */
+               dbg("%s(): port transmit buffer is full!", __func__);
                /* schedule_work(&port->work); commented in vendor driver */
                return 0;
        }
@@ -745,8 +757,8 @@ static int qt2_write(struct tty_struct *tty, struct usb_serial_port *port,
        /* we must also ensure that the FIFO at the other end can cope with the
         * URB we send it, otherwise it will have problems. As above, we can
         * restrict the write size by just shrinking count.*/
-       if (count > (FIFO_DEPTH - port_extra->tx_pending_bytes))
-               count = FIFO_DEPTH - port_extra->tx_pending_bytes;
+       if (count > (QT2_FIFO_DEPTH - port_extra->tx_pending_bytes))
+               count = QT2_FIFO_DEPTH - port_extra->tx_pending_bytes;
        /* now build the header for transmission */
        header_array[0] = 0x1b;
        header_array[1] = 0x1b;
@@ -780,12 +792,21 @@ static int qt2_write(struct tty_struct *tty, struct usb_serial_port *port,
                port->xmit_fifo_room_bytes = FIFO_DEPTH -
                port->xmit_pending_bytes;*/
                result = count;
-               dbg("%s(): submitted write urb, returning %d", __func__,
-result);
+               dbg("%s(): submitted write urb, returning %d",
+                       __func__, result);
        }
        return result;
 }
 
+/* This is used by the next layer up to know how much space is available
+ * in the buffer on the device. It is used on a device closure to avoid
+ * calling close() until the buffer is reported to be empty.
+ * The returned value must never go down by more than the number of bytes
+ * written for correct behaviour further up the driver stack, i.e. if I call
+ * it, then write 6 bytes, then call again I should get 6 less, or possibly
+ * only 5 less if one was written in the meantime, etc. I should never get 7
+ * less (or any bigger number) because I only wrote 6 bytes.
+ */
 static int qt2_write_room(struct tty_struct *tty)
 {
        struct usb_serial_port *port = tty->driver_data;
@@ -798,11 +819,28 @@ static int qt2_write_room(struct tty_struct *tty)
                dbg("%s(): port_extra->close_pending == true", __func__);
                return -ENODEV;
        }
+       /* Q: how many bytes would a write() call actually succeed in writing
+        * if it happened now?
+        * A: one QT2_FIFO_DEPTH, less the number of bytes waiting to be sent
+        * out of the port, unless this is more than the size of the
+        * write_urb output buffer less the header, which is the maximum
+        * size write we can do.
+
+        * Most of the implementation of this is done when writes to the device
+        * are started or terminate. When we send a write to the device, we
+        * reduce the free space count by the size of the dispatched write.
+        * When a "transmit empty" message comes back up the USB read stream,
+        * we decrement the count by the number of bytes reported sent, thus
+        * keeping track of the difference between sent and recieved bytes.
+        */
 
-       dbg("%s(): port %d", __func__, port->number);
-       if ((port->write_urb->status != -EINPROGRESS) &&
-               (port_extra->tx_pending_bytes == 0))
+       room = (QT2_FIFO_DEPTH - port_extra->tx_pending_bytes);
+       /* space in FIFO */
+       if (room > port->bulk_out_size - QT2_TX_HEADER_LENGTH)
                room = port->bulk_out_size - QT2_TX_HEADER_LENGTH;
+       /* if more than the URB can hold, then cap to that limit */
+
+       dbg("%s(): port %d: write room is %d", __func__, port->number, room);
        return room;
 }
 
@@ -1303,8 +1341,7 @@ __func__);
                                                break;
                                        }
                                        qt2_process_xmit_empty(active,
-                                               FOURTHCHAR,
-                                                       FIFTHCHAR);
+                                               FOURTHCHAR, FIFTHCHAR);
                                        i += 4;
                                        escapeflag = true;
                                        break;
@@ -1473,8 +1510,16 @@ static void qt2_process_xmit_empty(struct usb_serial_port *port,
 
        byte_count = (int)(fifth_char * 16);
        byte_count +=  (int)fourth_char;
-       port_extra->xmit_pending_bytes -= (int)byte_count;
-       port_extra->xmit_fifo_room_bytes = FIFO_DEPTH;
+       /* byte_count indicates how many bytes the device has written out. This
+        * message appears to occur regularly, and is used in the vendor driver
+        * to keep track of the fill state of the port transmit buffer */
+       port_extra->tx_pending_bytes -= byte_count;
+       /* reduce the stored data queue length by the known number of bytes
+        * sent */
+       dbg("port %d: %d bytes reported sent, %d still pending", port->number,
+                       byte_count, port_extra->tx_pending_bytes);
+
+       /*port_extra->xmit_fifo_room_bytes = FIFO_DEPTH; ???*/
 }
 
 static void qt2_process_port_change(struct usb_serial_port *port,