Re: [PATCH] generic driver for rotary encoders on GPIOs

[Daniel Mack <daniel@caiaq.de>]

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@caiaq.de>
> > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > Cc: H Hartley Sweeten <hsweeten@visionengravers.com>
> > ---
> >  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