This patch contains two fixes submitted by Ondrej Palkovsky: - the 'ACK' packet is sent after the transfer of the USB packet is completed, i.e. in the write_callback function. Because the close function sends the 'abort' command, a parameter is added that allows the caller of garmin_write_bulk to specify, if the 'ack' should be propagated to the serial link or dimissed. This fixes the problem with gpsbabel, it has sent several packets that were acknowledged before they were sent to the GPS and GpsBabel closed the device - thus effectively cancelled all outstanding requests in the queue. - removed the APP_RESP_SEEN and APP_REQ_SEEN flags and changed them into counters. It evades USB reset of the gps on every device close.
The patch is done against 2.6.21.1 with the 'clean up urb->status usage' patch applied. Signed-off-by: Hermann Kneissel <[EMAIL PROTECTED]> --- --- linux-2.6.22.1/drivers/usb/serial/garmin_gps.c.orig 2007-07-10 20:56:30.000000000 +0200 +++ linux-2.6.22.1/drivers/usb/serial/garmin_gps.c 2007-07-23 19:37:20.129305281 +0200 @@ -1,7 +1,7 @@ /* * Garmin GPS driver * - * Copyright (C) 2006 Hermann Kneissel [EMAIL PROTECTED] + * Copyright (C) 2007 Hermann Kneissel [EMAIL PROTECTED] * * The latest version of the driver can be found at * http://sourceforge.net/projects/garmin-gps/ @@ -21,6 +21,21 @@ * You should have received a copy of the GNU General Public License * along with this program; if not, write to the Free Software * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111 USA + * + * Change History + * + * 0.31 Added a patch from Ondrej Palkovsky <[EMAIL PROTECTED]> + * - the 'ACK' packet is sent after the transfer of the USB packet is + * completed, i.e. in the write_callback function. Because the close + * function sends the 'abort' command, a parameter is added that allows + * the caller of garmin_write_bulk to specify, if the 'ack' should be + * propagated to the serial link or dimissed. + * This fixes the problem with gpsbabel, it has sent several packets that + * were acknowledged before they were sent to the GPS and GpsBabel closed + * the device - thus effectively cancelled all outstanding requests in the + * queue. + * - removed the APP_RESP_SEEN and APP_REQ_SEEN flags and changed them + * into counters. It evades USB reset of the gps on every device close. */ #include <linux/kernel.h> @@ -34,6 +49,7 @@ #include <linux/module.h> #include <linux/spinlock.h> #include <asm/uaccess.h> +#include <asm/atomic.h> #include <linux/usb.h> #include <linux/usb/serial.h> @@ -52,7 +68,7 @@ static int debug = 0; */ #define VERSION_MAJOR 0 -#define VERSION_MINOR 28 +#define VERSION_MINOR 31 #define _STR(s) #s #define _DRIVER_VERSION(a,b) "v" _STR(a) "." _STR(b) @@ -141,6 +157,8 @@ struct garmin_data { __u8 inbuffer [GPS_IN_BUFSIZ]; /* tty -> usb */ __u8 outbuffer[GPS_OUT_BUFSIZ]; /* usb -> tty */ __u8 privpkt[4*6]; + atomic_t req_count; + atomic_t resp_count; spinlock_t lock; struct list_head pktlist; }; @@ -171,8 +189,6 @@ struct garmin_data { #define CLEAR_HALT_REQUIRED 0x0001 #define FLAGS_QUEUING 0x0100 -#define FLAGS_APP_RESP_SEEN 0x0200 -#define FLAGS_APP_REQ_SEEN 0x0400 #define FLAGS_DROP_DATA 0x0800 #define FLAGS_GSP_SKIP 0x1000 @@ -186,7 +202,8 @@ struct garmin_data { /* function prototypes */ static void gsp_next_packet(struct garmin_data * garmin_data_p); static int garmin_write_bulk(struct usb_serial_port *port, - const unsigned char *buf, int count); + const unsigned char *buf, int count, + int dismiss_ack); /* some special packets to be send or received */ static unsigned char const GARMIN_START_SESSION_REQ[] @@ -233,9 +250,7 @@ static struct usb_driver garmin_driver = static inline int noResponseFromAppLayer(struct garmin_data * garmin_data_p) { - return ((garmin_data_p->flags - & (FLAGS_APP_REQ_SEEN|FLAGS_APP_RESP_SEEN)) - == FLAGS_APP_REQ_SEEN); + return atomic_read(&garmin_data_p->req_count) == atomic_read(&garmin_data_p->resp_count); } @@ -463,7 +478,7 @@ static int gsp_rec_packet(struct garmin_ usbdata[2] = __cpu_to_le32(size); garmin_write_bulk (garmin_data_p->port, garmin_data_p->inbuffer, - GARMIN_PKTHDR_LENGTH+size); + GARMIN_PKTHDR_LENGTH+size, 0); /* if this was an abort-transfer command, flush all queued data. */ @@ -818,7 +833,7 @@ static int nat_receive(struct garmin_dat if (garmin_data_p->insize >= len) { garmin_write_bulk (garmin_data_p->port, garmin_data_p->inbuffer, - len); + len, 0); garmin_data_p->insize = 0; /* if this was an abort-transfer command, @@ -893,10 +908,11 @@ static int garmin_clear(struct garmin_da struct usb_serial_port *port = garmin_data_p->port; - if (port != NULL && garmin_data_p->flags & FLAGS_APP_RESP_SEEN) { + if (port != NULL && atomic_read(&garmin_data_p->resp_count)) { /* send a terminate command */ status = garmin_write_bulk(port, GARMIN_STOP_TRANSFER_REQ, - sizeof(GARMIN_STOP_TRANSFER_REQ)); + sizeof(GARMIN_STOP_TRANSFER_REQ), + 1); } /* flush all queued data */ @@ -939,7 +955,8 @@ static int garmin_init_session(struct us dbg("%s - starting session ...", __FUNCTION__); garmin_data_p->state = STATE_ACTIVE; status = garmin_write_bulk(port, GARMIN_START_SESSION_REQ, - sizeof(GARMIN_START_SESSION_REQ)); + sizeof(GARMIN_START_SESSION_REQ), + 0); if (status >= 0) { @@ -950,7 +967,8 @@ static int garmin_init_session(struct us /* not needed, but the win32 driver does it too ... */ status = garmin_write_bulk(port, GARMIN_START_SESSION_REQ2, - sizeof(GARMIN_START_SESSION_REQ2)); + sizeof(GARMIN_START_SESSION_REQ2), + 0); if (status >= 0) { status = 0; spin_lock_irqsave(&garmin_data_p->lock, flags); @@ -987,6 +1005,8 @@ static int garmin_open (struct usb_seria garmin_data_p->mode = initial_mode; garmin_data_p->count = 0; garmin_data_p->flags = 0; + atomic_set(&garmin_data_p->req_count, 0); + atomic_set(&garmin_data_p->resp_count, 0); spin_unlock_irqrestore(&garmin_data_p->lock, flags); /* shutdown any bulk reads that might be going on */ @@ -1035,27 +1055,39 @@ static void garmin_write_bulk_callback ( { unsigned long flags; struct usb_serial_port *port = (struct usb_serial_port *)urb->context; - struct garmin_data * garmin_data_p = usb_get_serial_port_data(port); + int status = urb->status; - /* free up the transfer buffer, as usb_free_urb() does not do this */ - kfree (urb->transfer_buffer); + if (port) { + struct garmin_data * garmin_data_p = usb_get_serial_port_data(port); - dbg("%s - port %d", __FUNCTION__, port->number); + dbg("%s - port %d", __FUNCTION__, port->number); - if (urb->status) { - dbg("%s - nonzero write bulk status received: %d", - __FUNCTION__, urb->status); - spin_lock_irqsave(&garmin_data_p->lock, flags); - garmin_data_p->flags |= CLEAR_HALT_REQUIRED; - spin_unlock_irqrestore(&garmin_data_p->lock, flags); + if (GARMIN_LAYERID_APPL == getLayerId(urb->transfer_buffer) + && (garmin_data_p->mode == MODE_GARMIN_SERIAL)) { + gsp_send_ack(garmin_data_p, ((__u8 *)urb->transfer_buffer)[4]); + } + + if (status) { + dbg("%s - nonzero write bulk status received: %d", + __FUNCTION__, urb->status); + spin_lock_irqsave(&garmin_data_p->lock, flags); + garmin_data_p->flags |= CLEAR_HALT_REQUIRED; + spin_unlock_irqrestore(&garmin_data_p->lock, flags); + } + + usb_serial_port_softint(port); } - usb_serial_port_softint(port); + /* Ignore errors that resulted from garmin_write_bulk with dismiss_ack=1 */ + + /* free up the transfer buffer, as usb_free_urb() does not do this */ + kfree (urb->transfer_buffer); } static int garmin_write_bulk (struct usb_serial_port *port, - const unsigned char *buf, int count) + const unsigned char *buf, int count, + int dismiss_ack) { unsigned long flags; struct usb_serial *serial = port->serial; @@ -1092,13 +1124,12 @@ static int garmin_write_bulk (struct usb usb_sndbulkpipe (serial->dev, port->bulk_out_endpointAddress), buffer, count, - garmin_write_bulk_callback, port); + garmin_write_bulk_callback, + dismiss_ack ? NULL : port); urb->transfer_flags |= URB_ZERO_PACKET; if (GARMIN_LAYERID_APPL == getLayerId(buffer)) { - spin_lock_irqsave(&garmin_data_p->lock, flags); - garmin_data_p->flags |= FLAGS_APP_REQ_SEEN; - spin_unlock_irqrestore(&garmin_data_p->lock, flags); + atomic_inc(&garmin_data_p->req_count); if (garmin_data_p->mode == MODE_GARMIN_SERIAL) { pkt_clear(garmin_data_p); garmin_data_p->state = STATE_GSP_WAIT_DATA; @@ -1113,13 +1144,6 @@ static int garmin_write_bulk (struct usb "failed with status = %d\n", __FUNCTION__, status); count = status; - } else { - - if (GARMIN_LAYERID_APPL == getLayerId(buffer) - && (garmin_data_p->mode == MODE_GARMIN_SERIAL)) { - - gsp_send_ack(garmin_data_p, buffer[4]); - } } /* we are done with this urb, so let the host driver @@ -1134,7 +1158,6 @@ static int garmin_write_bulk (struct usb static int garmin_write (struct usb_serial_port *port, const unsigned char *buf, int count) { - unsigned long flags; int pktid, pktsiz, len; struct garmin_data * garmin_data_p = usb_get_serial_port_data(port); __le32 *privpkt = (__le32 *)garmin_data_p->privpkt; @@ -1185,9 +1208,7 @@ static int garmin_write (struct usb_seri break; case PRIV_PKTID_RESET_REQ: - spin_lock_irqsave(&garmin_data_p->lock, flags); - garmin_data_p->flags |= FLAGS_APP_REQ_SEEN; - spin_unlock_irqrestore(&garmin_data_p->lock, flags); + atomic_inc(&garmin_data_p->req_count); break; case PRIV_PKTID_SET_DEF_MODE: @@ -1240,8 +1261,6 @@ static int garmin_chars_in_buffer (struc static void garmin_read_process(struct garmin_data * garmin_data_p, unsigned char *data, unsigned data_length) { - unsigned long flags; - if (garmin_data_p->flags & FLAGS_DROP_DATA) { /* abort-transfer cmd is actice */ dbg("%s - pkt dropped", __FUNCTION__); @@ -1253,9 +1272,7 @@ static void garmin_read_process(struct g the device */ if (0 == memcmp(data, GARMIN_APP_LAYER_REPLY, sizeof(GARMIN_APP_LAYER_REPLY))) { - spin_lock_irqsave(&garmin_data_p->lock, flags); - garmin_data_p->flags |= FLAGS_APP_RESP_SEEN; - spin_unlock_irqrestore(&garmin_data_p->lock, flags); + atomic_inc(&garmin_data_p->resp_count); } /* if throttling is active or postprecessing is required @@ -1281,7 +1298,8 @@ static void garmin_read_bulk_callback (s struct usb_serial *serial = port->serial; struct garmin_data * garmin_data_p = usb_get_serial_port_data(port); unsigned char *data = urb->transfer_buffer; - int status; + int status = urb->status; + int retval; dbg("%s - port %d", __FUNCTION__, port->number); @@ -1290,9 +1308,9 @@ static void garmin_read_bulk_callback (s return; } - if (urb->status) { + if (status) { dbg("%s - nonzero read bulk status received: %d", - __FUNCTION__, urb->status); + __FUNCTION__, status); return; } @@ -1306,19 +1324,19 @@ static void garmin_read_bulk_callback (s spin_lock_irqsave(&garmin_data_p->lock, flags); garmin_data_p->flags &= ~FLAGS_BULK_IN_RESTART; spin_unlock_irqrestore(&garmin_data_p->lock, flags); - status = usb_submit_urb(port->read_urb, GFP_ATOMIC); - if (status) + retval = usb_submit_urb(port->read_urb, GFP_ATOMIC); + if (retval) dev_err(&port->dev, "%s - failed resubmitting read urb, error %d\n", - __FUNCTION__, status); + __FUNCTION__, retval); } else if (urb->actual_length > 0) { /* Continue trying to read until nothing more is received */ if (0 == (garmin_data_p->flags & FLAGS_THROTTLED)) { - status = usb_submit_urb(port->read_urb, GFP_ATOMIC); - if (status) + retval = usb_submit_urb(port->read_urb, GFP_ATOMIC); + if (retval) dev_err(&port->dev, - "%s - failed resubmitting read urb, error %d\n", - __FUNCTION__, status); + "%s - failed resubmitting read urb, " + "error %d\n", __FUNCTION__, retval); } } else { dbg("%s - end of bulk data", __FUNCTION__); @@ -1333,13 +1351,14 @@ static void garmin_read_bulk_callback (s static void garmin_read_int_callback (struct urb *urb) { unsigned long flags; - int status; + int retval; struct usb_serial_port *port = (struct usb_serial_port *)urb->context; struct usb_serial *serial = port->serial; struct garmin_data * garmin_data_p = usb_get_serial_port_data(port); unsigned char *data = urb->transfer_buffer; + int status = urb->status; - switch (urb->status) { + switch (status) { case 0: /* success */ break; @@ -1348,11 +1367,11 @@ static void garmin_read_int_callback (st case -ESHUTDOWN: /* this urb is terminated, clean up */ dbg("%s - urb shutting down with status: %d", - __FUNCTION__, urb->status); + __FUNCTION__, status); return; default: dbg("%s - nonzero urb status received: %d", - __FUNCTION__, urb->status); + __FUNCTION__, status); return; } @@ -1374,11 +1393,11 @@ static void garmin_read_int_callback (st port->read_urb->transfer_buffer, port->read_urb->transfer_buffer_length, garmin_read_bulk_callback, port); - status = usb_submit_urb(port->read_urb, GFP_ATOMIC); - if (status) { + retval = usb_submit_urb(port->read_urb, GFP_ATOMIC); + if (retval) { dev_err(&port->dev, "%s - failed submitting read urb, error %d\n", - __FUNCTION__, status); + __FUNCTION__, retval); } else { spin_lock_irqsave(&garmin_data_p->lock, flags); garmin_data_p->flags |= FLAGS_BULK_IN_ACTIVE; @@ -1422,11 +1441,11 @@ static void garmin_read_int_callback (st } port->interrupt_in_urb->dev = port->serial->dev; - status = usb_submit_urb (urb, GFP_ATOMIC); - if (status) + retval = usb_submit_urb (urb, GFP_ATOMIC); + if (retval) dev_err(&urb->dev->dev, "%s - Error %d submitting interrupt urb\n", - __FUNCTION__, status); + __FUNCTION__, retval); } -- ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
