Sorry, got the first one wrong
Hi,
after checking the new 2.6.8 code for drivers/usb/serial/cyberjack.c I
found some problems imho not properly taken care of.
I'm using a ASUS P2B-F board (i440BX chipset), a Cyberjack e-com USB
chipcard drive and a Debian kernel 2.6.8 (kernel-source-2.6.8 Version:
2.6.8-2); cyberjack.c there is unchanged from vanilla 2.6.8.1 from
kernel.org.
As far as I can remember the cyberjack didn't work since 2.6.0, but I
didn't look into all releases. When some problems got addressed in 2.6.7
I found that the changes made there just weren't enough to get my
cyberjack working (again under 2.6).
After some searching on newsgroups, this list, etc. I found
http://www.debianforum.de/forum/viewtopic.php?t=22954&start=15 (German
only) I think the patches mentioned there reverted too much, as I
experienced kernel debugging messages (enabled
CONFIG_DEBUG_SPINLOCK_SLEEP=y) to the effect of "sleeping function
called from invalid context" similar to the one mentioned later, but the
drive worked fine. Hooray!
Contacting the manufacturer (http://reiner-sct.de/) yielded 3 patches
via email (one of them against pre-2.6.7 versions), those described in
the provided URL seemed to derive from. I haven't found a direct link to
them up to this date. With these applied, the drive worked fine and no
more debugging messages showed up.
So, when looking into the new 2.6.8 driver I found that this version
(still/again) led to debugging messages like:
Debug: sleeping function called from invalid context at
arch/i386/lib/usercopy.c:623
in_atomic():1, irqs_disabled():1
[<c0107d9e>] dump_stack+0x1e/0x30
[<c011aa47>] __might_sleep+0xb7/0xe0
[<c01a7617>] copy_from_user+0x27/0x80
[<c8b57782>] cyberjack_write+0x432/0x4f0 [cyberjack]
[<c8b114c8>] serial_write+0x88/0xc0 [usbserial]
[<c01dc9d0>] write_chan+0x210/0x240
[<c01d71d8>] tty_write+0x1d8/0x270
[<c0159a7f>] vfs_write+0xcf/0x140
[<c0159bbb>] sys_write+0x4b/0x80
[<c0106fdb>] syscall_call+0x7/0xb
As the original patch to cyberjack.c (the third one was aimed at
usb-serial.c and could not be applied due to major changes there, which
seem to obsolete it anyway) didn't apply cleanly, I sort of wriggled it
in manually, resulting in the attached diff. This made the drive working
again with no more debugging messages relating to "sleeping functions...".
I hope someone more knowledgeable about kernel (usb) hacking than me
will find this information helpful for further improvement of the
driver. As I don't know what logs, config options etc. might shed
further light on this I didn't wan't to spam the list with lots of
perhaps unrelated/already known details. If You need something specific,
feel free to contact me.
Regards,
Klaus Hörcher
--- kernel-source-2.6.8.orig/drivers/usb/serial/cyberjack.c 2004-08-14 07:37:41.000000000 +0200
+++ kernel-source-2.6.8/drivers/usb/serial/cyberjack.c 2004-08-23 11:32:53.000000000 +0200
@@ -44,7 +44,7 @@
/*
* Version Information
*/
-#define DRIVER_VERSION "v1.0"
+#define DRIVER_VERSION "v1.01"
#define DRIVER_AUTHOR "Matthias Bruestle"
#define DRIVER_DESC "REINER SCT cyberJack pinpad/e-com USB Chipcard Reader Driver"
@@ -111,6 +111,7 @@
static int cyberjack_startup (struct usb_serial *serial)
{
struct cyberjack_private *priv;
+ int i;
dbg("%s", __FUNCTION__);
@@ -128,6 +129,16 @@
init_waitqueue_head(&serial->port[0]->write_wait);
+ for (i = 0; i < serial->num_ports; ++i) {
+ int result;
+ serial->port[i]->interrupt_in_urb->dev = serial->dev;
+ result = usb_submit_urb(serial->port[i]->interrupt_in_urb,
+ GFP_KERNEL);
+ if (result)
+ err(" usb_submit_urb(read int) failed");
+ dbg("%s - usb_submit_urb(int urb)", __FUNCTION__);
+ }
+
return( 0 );
}
@@ -138,6 +149,7 @@
dbg("%s", __FUNCTION__);
for (i=0; i < serial->num_ports; ++i) {
+ usb_unlink_urb (serial->port[i]->interrupt_in_urb);
/* My special items, the standard routines free my urbs */
kfree(usb_get_serial_port_data(serial->port[i]));
usb_set_serial_port_data(serial->port[i], NULL);
@@ -168,17 +180,6 @@
priv->wrsent = 0;
spin_unlock_irqrestore(&priv->lock, flags);
- /* shutdown any bulk reads that might be going on */
- usb_unlink_urb (port->write_urb);
- usb_unlink_urb (port->read_urb);
- usb_unlink_urb (port->interrupt_in_urb);
-
- port->interrupt_in_urb->dev = port->serial->dev;
- result = usb_submit_urb(port->interrupt_in_urb, GFP_KERNEL);
- if (result)
- err(" usb_submit_urb(read int) failed");
- dbg("%s - usb_submit_urb(int urb)", __FUNCTION__);
-
return result;
}
@@ -190,11 +191,6 @@
/* shutdown any bulk reads that might be going on */
usb_unlink_urb (port->write_urb);
usb_unlink_urb (port->read_urb);
- usb_unlink_urb (port->interrupt_in_urb);
- dbg("%s - usb_clear_halt", __FUNCTION__ );
- usb_clear_halt(port->serial->dev, port->write_urb->pipe);
- usb_clear_halt(port->serial->dev, port->read_urb->pipe);
- usb_clear_halt(port->serial->dev, port->interrupt_in_urb->pipe);
}
}
@@ -205,6 +201,7 @@
unsigned long flags;
int result;
int wrexpected;
+ unsigned char localbuf[CYBERJACK_LOCAL_BUF_SIZE]; /* Buffer for collecting data to write */
dbg("%s - port %d", __FUNCTION__, port->number);
dbg("%s - from_user %d", __FUNCTION__, from_user);
@@ -221,23 +218,30 @@
spin_lock_irqsave(&priv->lock, flags);
- if( (count+priv->wrfilled)>sizeof(priv->wrbuf) ) {
+ if( (count+priv->wrfilled)>sizeof(priv->wrbuf) ||
+ (count>sizeof(localbuf)) ) {
/* To much data for buffer. Reset buffer. */
priv->wrfilled=0;
spin_unlock_irqrestore(&priv->lock, flags);
return (0);
}
+ spin_unlock_irqrestore(&priv->lock, flags);
+
+
/* Copy data */
if (from_user) {
- if (copy_from_user(priv->wrbuf+priv->wrfilled, buf, count)) {
- spin_unlock_irqrestore(&priv->lock, flags);
+ if (copy_from_user(localbuf, buf, count)) {
return -EFAULT;
}
} else {
- memcpy (priv->wrbuf+priv->wrfilled, buf, count);
+ memcpy (localbuf, buf, count);
}
+ spin_lock_irqsave(&priv->lock, flags);
+
+ memcpy (priv->wrbuf+priv->wrfilled, localbuf, count);
+
usb_serial_debug_data(debug, &port->dev, __FUNCTION__, count,
priv->wrbuf+priv->wrfilled);
priv->wrfilled += count;
@@ -376,6 +380,10 @@
}
tty = port->tty;
+ if (!tty) {
+ dbg("%s - ignoring since device not open\n", __FUNCTION__);
+ return;
+ }
if (urb->actual_length) {
for (i = 0; i < urb->actual_length ; ++i) {
/* if we insert more than TTY_FLIPBUF_SIZE characters, we drop them. */
