lkml.org 
[lkml]   [2006]   [Feb]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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]
\
 
 \ /
  Last update: 2006-02-15 02:56    [from the cache]
©2003-2011 Jasper Spaans