[PATCH] isdn4linux: Siemens Gigaset drivers: make some variables non-atomic

With Hansjoerg Lipp <hjlipp@web.de>

Replace some atomic_t variables in the Gigaset drivers by non-atomic ones,
using spinlocks instead to assure atomicity, as proposed in discussions on the
linux-kernel mailing list.

Signed-off-by: Hansjoerg Lipp <hjlipp@web.de>
Signed-off-by: Tilman Schmidt <tilman@imap.cc>
Cc: Karsten Keil <kkeil@suse.de>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

1 files changed, 52 insertions, 34 deletions

diff --git a/drivers/isdn/gigaset/usb-gigaset.c b/drivers/isdn/gigaset/usb-gigaset.c
index fe8435b8fa97..bfb73fd5077e 100644
--- a/drivers/isdn/gigaset/usb-gigaset.c
+++ b/drivers/isdn/gigaset/usb-gigaset.c
@@ -371,13 +371,14 @@ static void gigaset_read_int_callback(struct urb *urb, struct pt_regs *regs)
 	int r;
 	unsigned numbytes;
 	unsigned char *src;
-
-	if (!atomic_read(&cs->connected)) {
-		err("%s: disconnected", __func__);
-		return;
-	}
+	unsigned long flags;
 
 	if (!urb->status) {
+		if (!cs->connected) {
+			err("%s: disconnected", __func__); /* should never happen */
+			return;
+		}
+
 		numbytes = urb->actual_length;
 
 		if (numbytes) {
@@ -399,12 +400,19 @@ static void gigaset_read_int_callback(struct urb *urb, struct pt_regs *regs)
 		/* The urb might have been killed. */
 		gig_dbg(DEBUG_ANY, "%s - nonzero read bulk status received: %d",
 			__func__, urb->status);
-		if (urb->status != -ENOENT) /* not killed */
+		if (urb->status != -ENOENT) { /* not killed */
+			if (!cs->connected) {
+				err("%s: disconnected", __func__); /* should never happen */
+				return;
+			}
 			resubmit = 1;
+		}
 	}
 
 	if (resubmit) {
-		r = usb_submit_urb(urb, SLAB_ATOMIC);
+		spin_lock_irqsave(&cs->lock, flags);
+		r = cs->connected ? usb_submit_urb(urb, SLAB_ATOMIC) : -ENODEV;
+		spin_unlock_irqrestore(&cs->lock, flags);
 		if (r)
 			dev_err(cs->dev, "error %d when resubmitting urb.\n",
 				-r);
@@ -416,21 +424,22 @@ static void gigaset_read_int_callback(struct urb *urb, struct pt_regs *regs)
 static void gigaset_write_bulk_callback(struct urb *urb, struct pt_regs *regs)
 {
 	struct cardstate *cs = urb->context;
+	unsigned long flags;
 
-#ifdef CONFIG_GIGASET_DEBUG
-	if (!atomic_read(&cs->connected)) {
-		err("%s: not connected", __func__);
-		return;
-	}
-#endif
 	if (urb->status)
 		dev_err(cs->dev, "bulk transfer failed (status %d)\n",
 			-urb->status);
 		/* That's all we can do. Communication problems
 		   are handled by timeouts or network protocols. */
 
-	atomic_set(&cs->hw.usb->busy, 0);
-	tasklet_schedule(&cs->write_tasklet);
+	spin_lock_irqsave(&cs->lock, flags);
+	if (!cs->connected) {
+		err("%s: not connected", __func__);
+	} else {
+		atomic_set(&cs->hw.usb->busy, 0);
+		tasklet_schedule(&cs->write_tasklet);
+	}
+	spin_unlock_irqrestore(&cs->lock, flags);
 }
 
 static int send_cb(struct cardstate *cs, struct cmdbuf_t *cb)
@@ -465,6 +474,8 @@ static int send_cb(struct cardstate *cs, struct cmdbuf_t *cb)
 		}
 		if (cb) {
 			count = min(cb->len, ucs->bulk_out_size);
+			gig_dbg(DEBUG_OUTPUT, "send_cb: send %d bytes", count);
+
 			usb_fill_bulk_urb(ucs->bulk_out_urb, ucs->udev,
 					  usb_sndbulkpipe(ucs->udev,
 					     ucs->bulk_out_endpointAddr & 0x0f),
@@ -474,14 +485,15 @@ static int send_cb(struct cardstate *cs, struct cmdbuf_t *cb)
 			cb->offset += count;
 			cb->len -= count;
 			atomic_set(&ucs->busy, 1);
-			gig_dbg(DEBUG_OUTPUT, "send_cb: send %d bytes", count);
 
-			status = usb_submit_urb(ucs->bulk_out_urb, SLAB_ATOMIC);
+			spin_lock_irqsave(&cs->lock, flags);
+			status = cs->connected ? usb_submit_urb(ucs->bulk_out_urb, SLAB_ATOMIC) : -ENODEV;
+			spin_unlock_irqrestore(&cs->lock, flags);
+
 			if (status) {
 				atomic_set(&ucs->busy, 0);
-				dev_err(cs->dev,
-					"could not submit urb (error %d)\n",
-					-status);
+				err("could not submit urb (error %d)\n",
+				    -status);
 				cb->len = 0; /* skip urb => remove cb+wakeup
 						in next loop cycle */
 			}
@@ -502,11 +514,6 @@ static int gigaset_write_cmd(struct cardstate *cs, const unsigned char *buf,
 			     DEBUG_TRANSCMD : DEBUG_LOCKCMD,
 			   "CMD Transmit", len, buf);
 
-	if (!atomic_read(&cs->connected)) {
-		err("%s: not connected", __func__);
-		return -ENODEV;
-	}
-
 	if (len <= 0)
 		return 0;
 
@@ -533,7 +540,10 @@ static int gigaset_write_cmd(struct cardstate *cs, const unsigned char *buf,
 	cs->lastcmdbuf = cb;
 	spin_unlock_irqrestore(&cs->cmdlock, flags);
 
-	tasklet_schedule(&cs->write_tasklet);
+	spin_lock_irqsave(&cs->lock, flags);
+	if (cs->connected)
+		tasklet_schedule(&cs->write_tasklet);
+	spin_unlock_irqrestore(&cs->lock, flags);
 	return len;
 }
 
@@ -629,6 +639,7 @@ static int write_modem(struct cardstate *cs)
 	int count;
 	struct bc_state *bcs = &cs->bcs[0]; /* only one channel */
 	struct usb_cardstate *ucs = cs->hw.usb;
+	unsigned long flags;
 
 	gig_dbg(DEBUG_WRITE, "len: %d...", bcs->tx_skb->len);
 
@@ -644,20 +655,27 @@ static int write_modem(struct cardstate *cs)
 	count = min(bcs->tx_skb->len, (unsigned) ucs->bulk_out_size);
 	memcpy(ucs->bulk_out_buffer, bcs->tx_skb->data, count);
 	skb_pull(bcs->tx_skb, count);
-
-	usb_fill_bulk_urb(ucs->bulk_out_urb, ucs->udev,
-			  usb_sndbulkpipe(ucs->udev,
-					  ucs->bulk_out_endpointAddr & 0x0f),
-			  ucs->bulk_out_buffer, count,
-			  gigaset_write_bulk_callback, cs);
 	atomic_set(&ucs->busy, 1);
 	gig_dbg(DEBUG_OUTPUT, "write_modem: send %d bytes", count);
 
-	ret = usb_submit_urb(ucs->bulk_out_urb, SLAB_ATOMIC);
+	spin_lock_irqsave(&cs->lock, flags);
+	if (cs->connected) {
+		usb_fill_bulk_urb(ucs->bulk_out_urb, ucs->udev,
+				  usb_sndbulkpipe(ucs->udev,
+						  ucs->bulk_out_endpointAddr & 0x0f),
+				  ucs->bulk_out_buffer, count,
+				  gigaset_write_bulk_callback, cs);
+		ret = usb_submit_urb(ucs->bulk_out_urb, SLAB_ATOMIC);
+	} else {
+		ret = -ENODEV;
+	}
+	spin_unlock_irqrestore(&cs->lock, flags);
+
 	if (ret) {
-		dev_err(cs->dev, "could not submit urb (error %d)\n", -ret);
+		err("could not submit urb (error %d)\n", -ret);
 		atomic_set(&ucs->busy, 0);
 	}
+
 	if (!bcs->tx_skb->len) {
 		/* skb sent completely */
 		gigaset_skb_sent(bcs, bcs->tx_skb); //FIXME also, when ret<0?