some fixes of the sierra driver

2008-01-30 Thread Oliver Neukum
Hi,

while I was adding autosuspend to that driver I noticed a few
issues. You were having DMAed buffers as a part of a structure.
This will fail on platforms that are not DMA-coherent (arm, sparc, ppc, ...)
Please test this patch to fix it.

Regards
Oliver



--- linux-2.6.24-sierra/drivers/usb/serial/sierra.c.alt22008-01-30 
12:03:57.0 +0100
+++ linux-2.6.24-sierra/drivers/usb/serial/sierra.c 2008-01-30 
12:11:39.0 +0100
@@ -56,9 +56,9 @@ struct sierra_port_private {
int outstanding_urbs;   /* number of out urbs in flight */
struct usb_anchor transmit_urbs;
 
-   /* Input endpoints and buffer for this port */
+   /* Input endpoints and buffers for this port */
struct urb *in_urbs[N_IN_URB];
-   char in_buffer[N_IN_URB][IN_BUFLEN];
+   char *in_buffer[N_IN_URB];
 
/* Settings for the port */
int rts_state;  /* Handshaking pins (outputs) */
@@ -714,6 +714,15 @@ static int sierra_startup(struct usb_ser
}
spin_lock_init(&portdata->lock);
init_usb_anchor(&portdata->transmit_urbs);
+   for (j = 0; j < N_IN_URB; j++) {
+   portdata->in_buffer[j] = kmalloc(IN_BUFLEN, GFP_KERNEL);
+   if (!portdata->in_buffer[j]) {
+   for ( --j; j >= 0; j--)
+   kfree(portdata->in_buffer[j]);
+   kfree(portdata);
+   return -ENOMEM;
+   } 
+   }
 
usb_set_serial_port_data(port, portdata);
 
@@ -757,7 +766,7 @@ static void sierra_shutdown(struct usb_s
for (j = 0; j < N_IN_URB; j++) {
usb_kill_urb(portdata->in_urbs[j]);
usb_free_urb(portdata->in_urbs[j]);
-   portdata->in_urbs[j] = NULL;
+   kfree(portdata->in_buffer[j]);
}
kfree(portdata);
usb_set_serial_port_data(port, NULL);
-
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: some fixes of the sierra driver

2008-01-30 Thread Kevin Lloyd
Hi Oliver,

I will get to testing this and get back to you with the results asap.

I expect that this will work as I have seen it implemented this way
before.

Thanks,
 -Kevin
-Original Message-
From: Oliver Neukum [mailto:[EMAIL PROTECTED] 
Sent: Wednesday, January 30, 2008 3:22 AM
To: Linux Development Group; linux-usb@vger.kernel.org
Subject: some fixes of the sierra driver

Hi,

while I was adding autosuspend to that driver I noticed a few
issues. You were having DMAed buffers as a part of a structure.
This will fail on platforms that are not DMA-coherent (arm, sparc, ppc,
...)
Please test this patch to fix it.

Regards
Oliver



--- linux-2.6.24-sierra/drivers/usb/serial/sierra.c.alt2
2008-01-30 12:03:57.0 +0100
+++ linux-2.6.24-sierra/drivers/usb/serial/sierra.c 2008-01-30
12:11:39.0 +0100
@@ -56,9 +56,9 @@ struct sierra_port_private {
int outstanding_urbs;   /* number of out urbs in flight */
struct usb_anchor transmit_urbs;
 
-   /* Input endpoints and buffer for this port */
+   /* Input endpoints and buffers for this port */
struct urb *in_urbs[N_IN_URB];
-   char in_buffer[N_IN_URB][IN_BUFLEN];
+   char *in_buffer[N_IN_URB];
 
/* Settings for the port */
int rts_state;  /* Handshaking pins (outputs) */
@@ -714,6 +714,15 @@ static int sierra_startup(struct usb_ser
}
spin_lock_init(&portdata->lock);
init_usb_anchor(&portdata->transmit_urbs);
+   for (j = 0; j < N_IN_URB; j++) {
+   portdata->in_buffer[j] = kmalloc(IN_BUFLEN,
GFP_KERNEL);
+   if (!portdata->in_buffer[j]) {
+   for ( --j; j >= 0; j--)
+   kfree(portdata->in_buffer[j]);
+   kfree(portdata);
+   return -ENOMEM;
+   } 
+   }
 
usb_set_serial_port_data(port, portdata);
 
@@ -757,7 +766,7 @@ static void sierra_shutdown(struct usb_s
for (j = 0; j < N_IN_URB; j++) {
usb_kill_urb(portdata->in_urbs[j]);
usb_free_urb(portdata->in_urbs[j]);
-   portdata->in_urbs[j] = NULL;
+   kfree(portdata->in_buffer[j]);
}
kfree(portdata);
usb_set_serial_port_data(port, NULL);

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