Marc Balmer <mbal...@netbsd.org> wrote:
> Here comes the third iteration of the gpio(4) patch. In addition to
> whjat I already described, u_int_XXX types have been replaced by
> uint_XXX and gpio(4) is made MPSAFE. Comments?
>
> Furthermore I am thinking if it would be useful if more than one process
> could open a gpio device, as a long as they use different pins, e.g. one
> process controlls a stepper motor using some of the pins while another
> process drives some LEDs.
There is nothing what prevents from multiple threads calling gpioioctl(),
which is obviously not MP-safe. As soon as you will start fixing this, it
will bring you back to the point I have already stated - the design needs
to be MP-safe in general.
> + mutex_enter(&sc->sc_mtx);
> + if (sc->sc_opened)
> + return EBUSY;
This leaks the lock.
> int sc_opened;
> + kmutex_t sc_mtx;
Preferred suffix is _lock, rather than _mtx, so please use sc_lock.
> + sc->sc_pins[pin].pin_ticks_on = tvtohz(&pulse->gp_pulse_on);
> + sc->sc_pins[pin].pin_ticks_off = tvtohz(&pulse->gp_pulse_off);
> + if (sc->sc_pins[pin].pin_ticks_on == 0
> + || sc->sc_pins[pin].pin_ticks_off == 0) {
> + sc->sc_pins[pin].pin_ticks_on = hz / 2;
> + sc->sc_pins[pin].pin_ticks_off = hz / 2;
> + }
Use gpio_pin_t *gpin = &sc->sc_pins[pin]; and gpin variable instead of
sc->sc_pins[pin] each time. Apart from better readability, GCC will most
likely generate a better code.
--
Mindaugas
