Commit 468d1362 authored by Hermann Kneissel's avatar Hermann Kneissel Committed by Greg Kroah-Hartman

USB: serial: garmin_gps: fixes package loss if used from gpsbabel

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.
Signed-off-by: default avatarHermann Kneissel <hermann.kneissel@gmx.de>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@suse.de>
parent c8ba84a0
/* /*
* Garmin GPS driver * Garmin GPS driver
* *
* Copyright (C) 2006 Hermann Kneissel herkne@users.sourceforge.net * Copyright (C) 2006,2007 Hermann Kneissel herkne@users.sourceforge.net
* *
* The latest version of the driver can be found at * The latest version of the driver can be found at
* http://sourceforge.net/projects/garmin-gps/ * http://sourceforge.net/projects/garmin-gps/
...@@ -34,6 +34,7 @@ ...@@ -34,6 +34,7 @@
#include <linux/module.h> #include <linux/module.h>
#include <linux/spinlock.h> #include <linux/spinlock.h>
#include <asm/uaccess.h> #include <asm/uaccess.h>
#include <asm/atomic.h>
#include <linux/usb.h> #include <linux/usb.h>
#include <linux/usb/serial.h> #include <linux/usb/serial.h>
...@@ -52,7 +53,7 @@ static int debug = 0; ...@@ -52,7 +53,7 @@ static int debug = 0;
*/ */
#define VERSION_MAJOR 0 #define VERSION_MAJOR 0
#define VERSION_MINOR 28 #define VERSION_MINOR 31
#define _STR(s) #s #define _STR(s) #s
#define _DRIVER_VERSION(a,b) "v" _STR(a) "." _STR(b) #define _DRIVER_VERSION(a,b) "v" _STR(a) "." _STR(b)
...@@ -141,6 +142,8 @@ struct garmin_data { ...@@ -141,6 +142,8 @@ struct garmin_data {
__u8 inbuffer [GPS_IN_BUFSIZ]; /* tty -> usb */ __u8 inbuffer [GPS_IN_BUFSIZ]; /* tty -> usb */
__u8 outbuffer[GPS_OUT_BUFSIZ]; /* usb -> tty */ __u8 outbuffer[GPS_OUT_BUFSIZ]; /* usb -> tty */
__u8 privpkt[4*6]; __u8 privpkt[4*6];
atomic_t req_count;
atomic_t resp_count;
spinlock_t lock; spinlock_t lock;
struct list_head pktlist; struct list_head pktlist;
}; };
...@@ -171,8 +174,6 @@ struct garmin_data { ...@@ -171,8 +174,6 @@ struct garmin_data {
#define CLEAR_HALT_REQUIRED 0x0001 #define CLEAR_HALT_REQUIRED 0x0001
#define FLAGS_QUEUING 0x0100 #define FLAGS_QUEUING 0x0100
#define FLAGS_APP_RESP_SEEN 0x0200
#define FLAGS_APP_REQ_SEEN 0x0400
#define FLAGS_DROP_DATA 0x0800 #define FLAGS_DROP_DATA 0x0800
#define FLAGS_GSP_SKIP 0x1000 #define FLAGS_GSP_SKIP 0x1000
...@@ -186,7 +187,8 @@ struct garmin_data { ...@@ -186,7 +187,8 @@ struct garmin_data {
/* function prototypes */ /* function prototypes */
static void gsp_next_packet(struct garmin_data * garmin_data_p); static void gsp_next_packet(struct garmin_data * garmin_data_p);
static int garmin_write_bulk(struct usb_serial_port *port, 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 */ /* some special packets to be send or received */
static unsigned char const GARMIN_START_SESSION_REQ[] static unsigned char const GARMIN_START_SESSION_REQ[]
...@@ -233,9 +235,7 @@ static struct usb_driver garmin_driver = { ...@@ -233,9 +235,7 @@ static struct usb_driver garmin_driver = {
static inline int noResponseFromAppLayer(struct garmin_data * garmin_data_p) static inline int noResponseFromAppLayer(struct garmin_data * garmin_data_p)
{ {
return ((garmin_data_p->flags return atomic_read(&garmin_data_p->req_count) == atomic_read(&garmin_data_p->resp_count);
& (FLAGS_APP_REQ_SEEN|FLAGS_APP_RESP_SEEN))
== FLAGS_APP_REQ_SEEN);
} }
...@@ -463,7 +463,7 @@ static int gsp_rec_packet(struct garmin_data * garmin_data_p, int count) ...@@ -463,7 +463,7 @@ static int gsp_rec_packet(struct garmin_data * garmin_data_p, int count)
usbdata[2] = __cpu_to_le32(size); usbdata[2] = __cpu_to_le32(size);
garmin_write_bulk (garmin_data_p->port, garmin_data_p->inbuffer, 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 /* if this was an abort-transfer command, flush all
queued data. */ queued data. */
...@@ -818,7 +818,7 @@ static int nat_receive(struct garmin_data * garmin_data_p, ...@@ -818,7 +818,7 @@ static int nat_receive(struct garmin_data * garmin_data_p,
if (garmin_data_p->insize >= len) { if (garmin_data_p->insize >= len) {
garmin_write_bulk (garmin_data_p->port, garmin_write_bulk (garmin_data_p->port,
garmin_data_p->inbuffer, garmin_data_p->inbuffer,
len); len, 0);
garmin_data_p->insize = 0; garmin_data_p->insize = 0;
/* if this was an abort-transfer command, /* if this was an abort-transfer command,
...@@ -893,10 +893,11 @@ static int garmin_clear(struct garmin_data * garmin_data_p) ...@@ -893,10 +893,11 @@ static int garmin_clear(struct garmin_data * garmin_data_p)
struct usb_serial_port *port = garmin_data_p->port; 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 */ /* send a terminate command */
status = garmin_write_bulk(port, GARMIN_STOP_TRANSFER_REQ, status = garmin_write_bulk(port, GARMIN_STOP_TRANSFER_REQ,
sizeof(GARMIN_STOP_TRANSFER_REQ)); sizeof(GARMIN_STOP_TRANSFER_REQ),
1);
} }
/* flush all queued data */ /* flush all queued data */
...@@ -939,7 +940,8 @@ static int garmin_init_session(struct usb_serial_port *port) ...@@ -939,7 +940,8 @@ static int garmin_init_session(struct usb_serial_port *port)
dbg("%s - starting session ...", __FUNCTION__); dbg("%s - starting session ...", __FUNCTION__);
garmin_data_p->state = STATE_ACTIVE; garmin_data_p->state = STATE_ACTIVE;
status = garmin_write_bulk(port, GARMIN_START_SESSION_REQ, status = garmin_write_bulk(port, GARMIN_START_SESSION_REQ,
sizeof(GARMIN_START_SESSION_REQ)); sizeof(GARMIN_START_SESSION_REQ),
0);
if (status >= 0) { if (status >= 0) {
...@@ -950,7 +952,8 @@ static int garmin_init_session(struct usb_serial_port *port) ...@@ -950,7 +952,8 @@ static int garmin_init_session(struct usb_serial_port *port)
/* not needed, but the win32 driver does it too ... */ /* not needed, but the win32 driver does it too ... */
status = garmin_write_bulk(port, status = garmin_write_bulk(port,
GARMIN_START_SESSION_REQ2, GARMIN_START_SESSION_REQ2,
sizeof(GARMIN_START_SESSION_REQ2)); sizeof(GARMIN_START_SESSION_REQ2),
0);
if (status >= 0) { if (status >= 0) {
status = 0; status = 0;
spin_lock_irqsave(&garmin_data_p->lock, flags); spin_lock_irqsave(&garmin_data_p->lock, flags);
...@@ -987,6 +990,8 @@ static int garmin_open (struct usb_serial_port *port, struct file *filp) ...@@ -987,6 +990,8 @@ static int garmin_open (struct usb_serial_port *port, struct file *filp)
garmin_data_p->mode = initial_mode; garmin_data_p->mode = initial_mode;
garmin_data_p->count = 0; garmin_data_p->count = 0;
garmin_data_p->flags = 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); spin_unlock_irqrestore(&garmin_data_p->lock, flags);
/* shutdown any bulk reads that might be going on */ /* shutdown any bulk reads that might be going on */
...@@ -1035,28 +1040,39 @@ static void garmin_write_bulk_callback (struct urb *urb) ...@@ -1035,28 +1040,39 @@ static void garmin_write_bulk_callback (struct urb *urb)
{ {
unsigned long flags; unsigned long flags;
struct usb_serial_port *port = (struct usb_serial_port *)urb->context; 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; int status = urb->status;
/* free up the transfer buffer, as usb_free_urb() does not do this */ if (port) {
kfree (urb->transfer_buffer); 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 (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) { if (status) {
dbg("%s - nonzero write bulk status received: %d", dbg("%s - nonzero write bulk status received: %d",
__FUNCTION__, status); __FUNCTION__, urb->status);
spin_lock_irqsave(&garmin_data_p->lock, flags); spin_lock_irqsave(&garmin_data_p->lock, flags);
garmin_data_p->flags |= CLEAR_HALT_REQUIRED; garmin_data_p->flags |= CLEAR_HALT_REQUIRED;
spin_unlock_irqrestore(&garmin_data_p->lock, flags); 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, 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; unsigned long flags;
struct usb_serial *serial = port->serial; struct usb_serial *serial = port->serial;
...@@ -1093,13 +1109,12 @@ static int garmin_write_bulk (struct usb_serial_port *port, ...@@ -1093,13 +1109,12 @@ static int garmin_write_bulk (struct usb_serial_port *port,
usb_sndbulkpipe (serial->dev, usb_sndbulkpipe (serial->dev,
port->bulk_out_endpointAddress), port->bulk_out_endpointAddress),
buffer, count, buffer, count,
garmin_write_bulk_callback, port); garmin_write_bulk_callback,
dismiss_ack ? NULL : port);
urb->transfer_flags |= URB_ZERO_PACKET; urb->transfer_flags |= URB_ZERO_PACKET;
if (GARMIN_LAYERID_APPL == getLayerId(buffer)) { if (GARMIN_LAYERID_APPL == getLayerId(buffer)) {
spin_lock_irqsave(&garmin_data_p->lock, flags); atomic_inc(&garmin_data_p->req_count);
garmin_data_p->flags |= FLAGS_APP_REQ_SEEN;
spin_unlock_irqrestore(&garmin_data_p->lock, flags);
if (garmin_data_p->mode == MODE_GARMIN_SERIAL) { if (garmin_data_p->mode == MODE_GARMIN_SERIAL) {
pkt_clear(garmin_data_p); pkt_clear(garmin_data_p);
garmin_data_p->state = STATE_GSP_WAIT_DATA; garmin_data_p->state = STATE_GSP_WAIT_DATA;
...@@ -1114,13 +1129,6 @@ static int garmin_write_bulk (struct usb_serial_port *port, ...@@ -1114,13 +1129,6 @@ static int garmin_write_bulk (struct usb_serial_port *port,
"failed with status = %d\n", "failed with status = %d\n",
__FUNCTION__, status); __FUNCTION__, status);
count = 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 /* we are done with this urb, so let the host driver
...@@ -1135,7 +1143,6 @@ static int garmin_write_bulk (struct usb_serial_port *port, ...@@ -1135,7 +1143,6 @@ static int garmin_write_bulk (struct usb_serial_port *port,
static int garmin_write (struct usb_serial_port *port, static int garmin_write (struct usb_serial_port *port,
const unsigned char *buf, int count) const unsigned char *buf, int count)
{ {
unsigned long flags;
int pktid, pktsiz, len; int pktid, pktsiz, len;
struct garmin_data * garmin_data_p = usb_get_serial_port_data(port); struct garmin_data * garmin_data_p = usb_get_serial_port_data(port);
__le32 *privpkt = (__le32 *)garmin_data_p->privpkt; __le32 *privpkt = (__le32 *)garmin_data_p->privpkt;
...@@ -1186,9 +1193,7 @@ static int garmin_write (struct usb_serial_port *port, ...@@ -1186,9 +1193,7 @@ static int garmin_write (struct usb_serial_port *port,
break; break;
case PRIV_PKTID_RESET_REQ: case PRIV_PKTID_RESET_REQ:
spin_lock_irqsave(&garmin_data_p->lock, flags); atomic_inc(&garmin_data_p->req_count);
garmin_data_p->flags |= FLAGS_APP_REQ_SEEN;
spin_unlock_irqrestore(&garmin_data_p->lock, flags);
break; break;
case PRIV_PKTID_SET_DEF_MODE: case PRIV_PKTID_SET_DEF_MODE:
...@@ -1241,8 +1246,6 @@ static int garmin_chars_in_buffer (struct usb_serial_port *port) ...@@ -1241,8 +1246,6 @@ static int garmin_chars_in_buffer (struct usb_serial_port *port)
static void garmin_read_process(struct garmin_data * garmin_data_p, static void garmin_read_process(struct garmin_data * garmin_data_p,
unsigned char *data, unsigned data_length) unsigned char *data, unsigned data_length)
{ {
unsigned long flags;
if (garmin_data_p->flags & FLAGS_DROP_DATA) { if (garmin_data_p->flags & FLAGS_DROP_DATA) {
/* abort-transfer cmd is actice */ /* abort-transfer cmd is actice */
dbg("%s - pkt dropped", __FUNCTION__); dbg("%s - pkt dropped", __FUNCTION__);
...@@ -1254,9 +1257,7 @@ static void garmin_read_process(struct garmin_data * garmin_data_p, ...@@ -1254,9 +1257,7 @@ static void garmin_read_process(struct garmin_data * garmin_data_p,
the device */ the device */
if (0 == memcmp(data, GARMIN_APP_LAYER_REPLY, if (0 == memcmp(data, GARMIN_APP_LAYER_REPLY,
sizeof(GARMIN_APP_LAYER_REPLY))) { sizeof(GARMIN_APP_LAYER_REPLY))) {
spin_lock_irqsave(&garmin_data_p->lock, flags); atomic_inc(&garmin_data_p->resp_count);
garmin_data_p->flags |= FLAGS_APP_RESP_SEEN;
spin_unlock_irqrestore(&garmin_data_p->lock, flags);
} }
/* if throttling is active or postprecessing is required /* if throttling is active or postprecessing is required
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment