Messages in this thread | | | Date | Wed, 15 Feb 2006 02:55:01 +0100 | From | Tilman Schmidt <> | Subject | Re: [PATCH 6/9] isdn4linux: Siemens Gigaset drivers - procfs interface |
| |
Andrew,
thank you very much for your comments.
On 12.02.2006 11:27, Andrew Morton wrote: > >>This patch adds the procfs interface to the gigaset module. > > sysfs, actually.
Yes, sorry, the comment didn't get updated when we did the move from procfs to sysfs.
> - The ringbuffer head and tail indexes are atomic_t's, but always seem to > be manipulated inside the lock. Perhaps they can become integers.
We use ringbuffers in two places. I suppose you are talking about the one implementing our event queue through the ev_head and ev_tail members of the cardstate structure. You have a point there, although I'm not sure if it wouldn't be better to take advantage of the atomic_t and do a little less locking instead.
The other one, within the inbuf_t structure, used for capturing characters received from the device, is accessed without locking from the gigaset_handle_event() tasklet and therefore needs atomic_t indexes, IMHO.
> - You did the ringbuffer the wrong way. Don't constrain the head and > tail to be within 0..MAX_EVENTS. Instead, just let them wrap right up to > 0xffffffff. Apply the masking when you actually _use_ them. > > That way, empty is (head == tail) and full is (tail - head == MAX_EVENTS).
Interesting idea. I have to admit it's rather new to me. I have always done ringbuffers the way they are done in the Gigaset driver now. Can you point me to some example code done the way you propose, so I can familiarize myself with its advantages?
> - Could use kstrdup() in a few places.
Thanks for the hint. Will watch out for these.
> - A few unneeded casts of void*'s, but everyone does that.
:-)
> - There are a lot of global symbols in there. Perhaps they don't all > need to be global.
Well, I *hope* there aren't any unnecessary ones, but we'll re-check.
Regards 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] | |