[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, 57 insertions, 30 deletions

diff --git a/drivers/isdn/gigaset/ev-layer.c b/drivers/isdn/gigaset/ev-layer.c
index 2e826d051c85..1ba3424a286b 100644
--- a/drivers/isdn/gigaset/ev-layer.c
+++ b/drivers/isdn/gigaset/ev-layer.c
@@ -482,14 +482,6 @@ static int isdn_gethex(char *p)
         return v;
 }
 
-static inline void new_index(atomic_t *index, int max)
-{
-        if (atomic_read(index) == max)  //FIXME race?
-                atomic_set(index, 0);
-        else
-                atomic_inc(index);
-}
-
 /* retrieve CID from parsed response
  * returns 0 if no CID, -1 if invalid CID, or CID value 1..65535
  */
@@ -581,8 +573,8 @@ void gigaset_handle_modem_response(struct cardstate *cs)
         }
 
         spin_lock_irqsave(&cs->ev_lock, flags);
-        head = atomic_read(&cs->ev_head);
-        tail = atomic_read(&cs->ev_tail);
+        head = cs->ev_head;
+        tail = cs->ev_tail;
 
         abort = 1;
         curarg = 0;
@@ -715,7 +707,7 @@ void gigaset_handle_modem_response(struct cardstate *cs)
                         break;
         }
 
-        atomic_set(&cs->ev_tail, tail);
+        cs->ev_tail = tail;
         spin_unlock_irqrestore(&cs->ev_lock, flags);
 
         if (curarg != params)
@@ -734,14 +726,16 @@ static void disconnect(struct at_state_t **at_state_p)
         struct bc_state *bcs = (*at_state_p)->bcs;
         struct cardstate *cs = (*at_state_p)->cs;
 
-        new_index(&(*at_state_p)->seq_index, MAX_SEQ_INDEX);
+        spin_lock_irqsave(&cs->lock, flags);
+        ++(*at_state_p)->seq_index;
 
         /* revert to selected idle mode */
-        if (!atomic_read(&cs->cidmode)) {
+        if (!cs->cidmode) {
                 cs->at_state.pending_commands |= PC_UMMODE;
                 atomic_set(&cs->commands_pending, 1); //FIXME
                 gig_dbg(DEBUG_CMD, "Scheduling PC_UMMODE");
         }
+        spin_unlock_irqrestore(&cs->lock, flags);
 
         if (bcs) {
                 /* B channel assigned: invoke hardware specific handler */
@@ -933,17 +927,21 @@ static void bchannel_up(struct bc_state *bcs)
         gigaset_i4l_channel_cmd(bcs, ISDN_STAT_BCONN);
 }
 
-static void start_dial(struct at_state_t *at_state, void *data, int seq_index)
+static void start_dial(struct at_state_t *at_state, void *data, unsigned seq_index)
 {
         struct bc_state *bcs = at_state->bcs;
         struct cardstate *cs = at_state->cs;
         int retval;
+        unsigned long flags;
 
         bcs->chstate |= CHS_NOTIFY_LL;
-        //atomic_set(&bcs->status, BCS_INIT);
 
-        if (atomic_read(&at_state->seq_index) != seq_index)
+        spin_lock_irqsave(&cs->lock, flags);
+        if (at_state->seq_index != seq_index) {
+                spin_unlock_irqrestore(&cs->lock, flags);
                 goto error;
+        }
+        spin_unlock_irqrestore(&cs->lock, flags);
 
         retval = gigaset_isdn_setup_dial(at_state, data);
         if (retval != 0)
@@ -988,6 +986,7 @@ static void do_start(struct cardstate *cs)
         if (atomic_read(&cs->mstate) != MS_LOCKED)
                 schedule_init(cs, MS_INIT);
 
+        cs->isdn_up = 1;
         gigaset_i4l_cmd(cs, ISDN_STAT_RUN);
                                         // FIXME: not in locked mode
                                         // FIXME 2: only after init sequence
@@ -1003,6 +1002,12 @@ static void finish_shutdown(struct cardstate *cs)
                 atomic_set(&cs->mode, M_UNKNOWN);
         }
 
+        /* Tell the LL that the device is not available .. */
+        if (cs->isdn_up) {
+                cs->isdn_up = 0;
+                gigaset_i4l_cmd(cs, ISDN_STAT_STOP);
+        }
+
         /* The rest is done by cleanup_cs () in user mode. */
 
         cs->cmd_result = -ENODEV;
@@ -1025,6 +1030,12 @@ static void do_shutdown(struct cardstate *cs)
 
 static void do_stop(struct cardstate *cs)
 {
+        unsigned long flags;
+
+        spin_lock_irqsave(&cs->lock, flags);
+        cs->connected = 0;
+        spin_unlock_irqrestore(&cs->lock, flags);
+
         do_shutdown(cs);
 }
 
@@ -1153,7 +1164,7 @@ static int do_unlock(struct cardstate *cs)
         atomic_set(&cs->mstate, MS_UNINITIALIZED);
         atomic_set(&cs->mode, M_UNKNOWN);
         gigaset_free_channels(cs);
-        if (atomic_read(&cs->connected))
+        if (cs->connected)
                 schedule_init(cs, MS_INIT);
 
         return 0;
@@ -1185,11 +1196,14 @@ static void do_action(int action, struct cardstate *cs,
                 cs->at_state.pending_commands &= ~PC_INIT;
                 cs->cur_at_seq = SEQ_NONE;
                 atomic_set(&cs->mode, M_UNIMODEM);
-                if (!atomic_read(&cs->cidmode)) {
+                spin_lock_irqsave(&cs->lock, flags);
+                if (!cs->cidmode) {
+                        spin_unlock_irqrestore(&cs->lock, flags);
                         gigaset_free_channels(cs);
                         atomic_set(&cs->mstate, MS_READY);
                         break;
                 }
+                spin_unlock_irqrestore(&cs->lock, flags);
                 cs->at_state.pending_commands |= PC_CIDMODE;
                 atomic_set(&cs->commands_pending, 1);
                 gig_dbg(DEBUG_CMD, "Scheduling PC_CIDMODE");
@@ -1536,8 +1550,9 @@ static void do_action(int action, struct cardstate *cs,
 
         /* events from the proc file system */ // FIXME without ACT_xxxx?
         case ACT_PROC_CIDMODE:
-                if (ev->parameter != atomic_read(&cs->cidmode)) {
-                        atomic_set(&cs->cidmode, ev->parameter);
+                spin_lock_irqsave(&cs->lock, flags);
+                if (ev->parameter != cs->cidmode) {
+                        cs->cidmode = ev->parameter;
                         if (ev->parameter) {
                                 cs->at_state.pending_commands |= PC_CIDMODE;
                                 gig_dbg(DEBUG_CMD, "Scheduling PC_CIDMODE");
@@ -1547,6 +1562,7 @@ static void do_action(int action, struct cardstate *cs,
                         }
                         atomic_set(&cs->commands_pending, 1);
                 }
+                spin_unlock_irqrestore(&cs->lock, flags);
                 cs->waiting = 0;
                 wake_up(&cs->waitqueue);
                 break;
@@ -1615,8 +1631,9 @@ static void process_event(struct cardstate *cs, struct event_t *ev)
         /* Setting the pointer to the dial array */
         rep = at_state->replystruct;
 
+        spin_lock_irqsave(&cs->lock, flags);
         if (ev->type == EV_TIMEOUT) {
-                if (ev->parameter != atomic_read(&at_state->timer_index)
+                if (ev->parameter != at_state->timer_index
                     || !at_state->timer_active) {
                         ev->type = RSP_NONE; /* old timeout */
                         gig_dbg(DEBUG_ANY, "old timeout");
@@ -1625,6 +1642,7 @@ static void process_event(struct cardstate *cs, struct event_t *ev)
                 else
                         gig_dbg(DEBUG_ANY, "stopped waiting");
         }
+        spin_unlock_irqrestore(&cs->lock, flags);
 
         /* if the response belongs to a variable in at_state->int_var[VAR_XXXX]
            or at_state->str_var[STR_XXXX], set it */
@@ -1686,7 +1704,7 @@ static void process_event(struct cardstate *cs, struct event_t *ev)
                 } else {
                         /* Send command to modem if not NULL... */
                         if (p_command/*rep->command*/) {
-                                if (atomic_read(&cs->connected))
+                                if (cs->connected)
                                         send_command(cs, p_command,
                                                      sendcid, cs->dle,
                                                      GFP_ATOMIC);
@@ -1703,8 +1721,7 @@ static void process_event(struct cardstate *cs, struct event_t *ev)
                         } else if (rep->timeout > 0) { /* new timeout */
                                 at_state->timer_expires = rep->timeout * 10;
                                 at_state->timer_active = 1;
-                                new_index(&at_state->timer_index,
-                                          MAX_TIMER_INDEX);
+                                ++at_state->timer_index;
                         }
                         spin_unlock_irqrestore(&cs->lock, flags);
                 }
@@ -1724,6 +1741,7 @@ static void process_command_flags(struct cardstate *cs)
         struct bc_state *bcs;
         int i;
         int sequence;
+        unsigned long flags;
 
         atomic_set(&cs->commands_pending, 0);
 
@@ -1773,8 +1791,9 @@ static void process_command_flags(struct cardstate *cs)
         }
 
         /* only switch back to unimodem mode, if no commands are pending and no channels are up */
+        spin_lock_irqsave(&cs->lock, flags);
         if (cs->at_state.pending_commands == PC_UMMODE
-            && !atomic_read(&cs->cidmode)
+            && !cs->cidmode
             && list_empty(&cs->temp_at_states)
             && atomic_read(&cs->mode) == M_CID) {
                 sequence = SEQ_UMMODE;
@@ -1788,6 +1807,7 @@ static void process_command_flags(struct cardstate *cs)
                         }
                 }
         }
+        spin_unlock_irqrestore(&cs->lock, flags);
         cs->at_state.pending_commands &= ~PC_UMMODE;
         if (sequence != SEQ_NONE) {
                 schedule_sequence(cs, at_state, sequence);
@@ -1900,18 +1920,21 @@ static void process_events(struct cardstate *cs)
         int i;
         int check_flags = 0;
         int was_busy;
+        unsigned long flags;
 
-        /* no locking needed (only one reader) */
-        head = atomic_read(&cs->ev_head);
+        spin_lock_irqsave(&cs->ev_lock, flags);
+        head = cs->ev_head;
 
         for (i = 0; i < 2 * MAX_EVENTS; ++i) {
-                tail = atomic_read(&cs->ev_tail);
+                tail = cs->ev_tail;
                 if (tail == head) {
                         if (!check_flags && !atomic_read(&cs->commands_pending))
                                 break;
                         check_flags = 0;
+                        spin_unlock_irqrestore(&cs->ev_lock, flags);
                         process_command_flags(cs);
-                        tail = atomic_read(&cs->ev_tail);
+                        spin_lock_irqsave(&cs->ev_lock, flags);
+                        tail = cs->ev_tail;
                         if (tail == head) {
                                 if (!atomic_read(&cs->commands_pending))
                                         break;
@@ -1921,16 +1944,20 @@ static void process_events(struct cardstate *cs)
 
                 ev = cs->events + head;
                 was_busy = cs->cur_at_seq != SEQ_NONE;
+                spin_unlock_irqrestore(&cs->ev_lock, flags);
                 process_event(cs, ev);
+                spin_lock_irqsave(&cs->ev_lock, flags);
                 kfree(ev->ptr);
                 ev->ptr = NULL;
                 if (was_busy && cs->cur_at_seq == SEQ_NONE)
                         check_flags = 1;
 
                 head = (head + 1) % MAX_EVENTS;
-                atomic_set(&cs->ev_head, head);
+                cs->ev_head = head;
         }
 
+        spin_unlock_irqrestore(&cs->ev_lock, flags);
+
         if (i == 2 * MAX_EVENTS) {
                 dev_err(cs->dev,
                         "infinite loop in process_events; aborting.\n");