On Wed, Mar 04, 2009 at 12:48:52AM -0800, Dmitry Torokhov wrote:
> On Tue, Mar 03, 2009 at 10:59:27AM +0100, Daniel Mack wrote:
> > This patch adds a generic driver for rotary encoders connected to GPIO
> > pins of a system. It relies on gpiolib and generic hardware irqs. The
> > documentation that also comes with this patch explains the concept and
> > how to use the driver.
> >
> > Signed-off-by: Daniel Mack <daniel@xxxxxxxx>
> > Cc: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
> > Cc: H Hartley Sweeten <hsweeten@xxxxxxxxxxxxxxxxxxx>
> > ---
> > New version with Dmitry's change requests included:
> >
> > * calling input_free_device() in case input_register_device() fails
> > * calling input_unregister_device() in all other cases
> > * made the axis information part of the platform data
> >
>
> I fiddled with the driver a little bit more changing formatting, please
> take a look and if you are still OK with it I will apply to 'next'.
Ok for me, except for one thing ...
> + case 0x0:
> + if (encoder->armed) {
> + if (encoder->dir) {
> + /* turning counter-clockwise */
> + encoder->pos += pdata->steps;
> + encoder->pos--;
> + encoder->pos %= pdata->steps;
> + } else {
> + /* turning clockwise */
> + encoder->pos++;
> + encoder->pos %= pdata->steps;
> + }
> + input_report_abs(encoder->input,
> + pdata->axis, encoder->pos);
> + input_sync(encoder->input);
> + encoder->armed = 0;
> + }
I really prefer early exits ("if (!encoder->armed) break;") as it saves
one indentation level and makes the code more readable.
Thanks,
Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
