Messages in this thread | | | Date | Fri, 16 Nov 2007 00:51:45 +0100 | From | Tilman Schmidt <> | Subject | Re: [PATCH 3/4] bas_gigaset: suspend support (v2) |
| |
Am 15.11.2007 23:50 schrieb Andrew Morton: >> ... >> >> + if (atomic_read(&cs->hw.bas->basstate) & BS_SUSPEND) { > > that's pretty peculiar. We'd only expect to see atomics being used in > conjunction with atomic_add/sub/inc/etc. Here the driver is using an > atomic_t as a state variable. And here's the magic bit: > > spin_lock_irqsave(&ucs->lock, flags); > state = atomic_read(&ucs->basstate); > atomic_set(&ucs->basstate, (state & ~clear) | set); > spin_unlock_irqrestore(&ucs->lock, flags); > > I'm suspecting that a plain old `int' would be more appropriate here.
You're right. That's a prehistoric leftover. That variable was originally accessed using atomic_set_mask() and atomic_clear_mask() which are unfortunately x86 platform specific.
I'll prepare a cleanup patch.
Thanks, Tilman
-- Tilman Schmidt E-Mail: tilman@imap.cc Bonn, Germany Diese Nachricht besteht zu 100% aus wiederverwerteten Bits. Ungeöffnet mindestens haltbar bis: (siehe Rückseite)
[unhandled content-type:application/pgp-signature] | |