On 02/27/2014 04:52 PM, Dan Carpenter wrote:
This isn't a real patch, and it deliberately doesn't compile, but it's
sort of what the patch should look like.
The first thing to do is to get rid of the stupid DGAP_UNLOCK() macro.
Disabling IRQs more than once doesn't help anything and it doesn't make
sense to have lock_flags and lock_flags2. It should just be one
"flags".
I guess the function name should have something to do with "wake_up" but
I just made up a dummy name.
regards,
dan carpenter
diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c
index 7cb1ad597ced..41de09af27d1 100644
--- a/drivers/staging/dgap/dgap.c
+++ b/drivers/staging/dgap/dgap.c
@@ -6278,7 +6278,31 @@ static void dgap_parity_scan(struct channel_t *ch, unsigned char *cbuf, unsigned
}
+static void futz_with_un_flags(struct board_t *bd, struct channel_t *ch,
+ struct un_t *un, unsigned long *irq_flags)
+{
+ if (!(un->un_flags & UN_LOW))
+ return;
+
+ un->un_flags &= ~UN_LOW;
+
+ if (!(un->un_flags & UN_ISOPEN))
+ return;
+
+ if ((un->un_tty->flags & (1 << TTY_DO_WRITE_WAKEUP)) &&
+ un->un_tty->ldisc->ops->write_wakeup) {
+ spin_unlock(&ch->ch_lock);
+ spin_unlock_irqrestore(&bd->bd_lock, *irq_flags);
+ (un->un_tty->ldisc->ops->write_wakeup)(un->un_tty);
+
+ spin_lock_irqsave(&bd->bd_lock, *irq_flags);
+ spin_lock(&ch->ch_lock);
+ }
+ wake_up_interruptible(&un->un_tty->write_wait);
+ wake_up_interruptible(&un->un_flags_wait);
+ DPR_EVENT(("event: Got low event. jiffies: %lu\n", jiffies));
+}
/*=======================================================================
*
@@ -6442,44 +6466,8 @@ static int dgap_event(struct board_t *bd)
DPR_EVENT(("event: got low event\n"));
- if (ch->ch_tun.un_flags & UN_LOW) {
- ch->ch_tun.un_flags &= ~UN_LOW;
-
- if (ch->ch_tun.un_flags & UN_ISOPEN) {
- if ((ch->ch_tun.un_tty->flags &
- (1 << TTY_DO_WRITE_WAKEUP)) &&
- ch->ch_tun.un_tty->ldisc->ops->write_wakeup)
- {
- DGAP_UNLOCK(ch->ch_lock, lock_flags2);
- DGAP_UNLOCK(bd->bd_lock, lock_flags);
- (ch->ch_tun.un_tty->ldisc->ops->write_wakeup)(ch->ch_tun.un_tty);
- DGAP_LOCK(bd->bd_lock, lock_flags);
- DGAP_LOCK(ch->ch_lock, lock_flags2);
- }
- wake_up_interruptible(&ch->ch_tun.un_tty->write_wait);
- wake_up_interruptible(&ch->ch_tun.un_flags_wait);
-
- DPR_EVENT(("event: Got low event. jiffies: %lu\n", jiffies));
- }
- }
-
- if (ch->ch_pun.un_flags & UN_LOW) {
- ch->ch_pun.un_flags &= ~UN_LOW;
- if (ch->ch_pun.un_flags & UN_ISOPEN) {
- if ((ch->ch_pun.un_tty->flags &
- (1 << TTY_DO_WRITE_WAKEUP)) &&
- ch->ch_pun.un_tty->ldisc->ops->write_wakeup)
- {
- DGAP_UNLOCK(ch->ch_lock, lock_flags2);
- DGAP_UNLOCK(bd->bd_lock, lock_flags);
- (ch->ch_pun.un_tty->ldisc->ops->write_wakeup)(ch->ch_pun.un_tty);
- DGAP_LOCK(bd->bd_lock, lock_flags);
- DGAP_LOCK(ch->ch_lock, lock_flags2);
- }
- wake_up_interruptible(&ch->ch_pun.un_tty->write_wait);
- wake_up_interruptible(&ch->ch_pun.un_flags_wait);
- }
- }
+ futz_with_un_flags(bd, ch, &ch->ch_tun, &flags);
+ futz_with_un_flags(bd, ch, &ch->ch_pun, &flags);
if (ch->ch_flags & CH_WLOW) {
ch->ch_flags &= ~CH_WLOW;
Thanks Dan. I see what should be done. I like and can work on this. But
is it OK to save all the 80 char problems until the end of this next
series or more likely a separate patch all together? Since I'm trying to
make individual patches that address specific checkpatch problems I am
running into a chicken and egg sort of thing. Some of the next series
will have checkpatch warnings/errors that are corrected in later patches.
Will this be OK? I think the review process will be much easier this way?
Thanks
Mark
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
