Re: Adding pulse support to gpio(4), gpioctl(8)

[Mindaugas Rasiukevicius <rmind%netbsd.org@localhost>]

Marc Balmer <mbalmer%NetBSD.org@localhost> wrote:
> > 
> >> +  mutex_enter(&sc->sc_mtx);
> >> +  if (sc->sc_opened)
> >> +          return EBUSY;
> > 
> > This leaks the lock.
> 
> Ooops... But it is not needed here anymore, see below...

Are underlying drivers MP-safe?

> >>    int                      sc_opened;
> >> +  kmutex_t                 sc_mtx;
> > 
> > Preferred suffix is _lock, rather than _mtx, so please use sc_lock.
> 
> I have seen both forms in the tree.

The ones which use _mtx should rather be fixed, instead of sprinkling them
even more.  Also, for condvars we use _cv suffix.

> I reworked the patch again.  This version changes the behaviour as well:
>  It is now possible that multiple thready/processes have a gpio device
> open, but only one thread can be in ioctl().  Two new functions are
> added, gpio_lock() and gpio_nolock() which can be used by child drivers
> as well.

Locking everything with a single fat lock is inefficient.  As initial step,
while learning, that is suitable.  Otherwise locking should be fine grained.

-- 
Mindaugas