Re: [PATCH] add REL_* axes support to the rotary encoder driver

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

On Wednesday 15 April 2009 19:24:58 H Hartley Sweeten wrote:
> On Wednesday, April 15, 2009 7:09 PM, Dmitry Torokhov wrote:
> > Hi,
> >
> > On Monday 13 April 2009 16:06:24 H Hartley Sweeten wrote:
> >> +
> >> +	unsigned int	axis;
> >> +	unsigned int	pos;
> >> +
> >> +	unsigned int	irq_a;
> >> +	unsigned int	irq_b;
> >> +
> >> +	unsigned int	abs:1;
> >> +	unsigned int	dir:1;
> >> +	unsigned int	armed:1;
> >
> > Doing it this way makes us go from simple store to read/modify/write
> > cycle in the interrupt handler.
>
> Didn't think of that. Good catch.
>
> >> +#define ROTARY_ENCODER_REL	(0<<31)
> >> +#define ROTARY_ENCODER_ABS	(1<<31)
> >
> > Meh... How about below instead?
>
> Your changes look good to me.
>
> The only issue I can see is that absolute axis encoders wrap.
>
> If you have an encoder of something like ABS_VOLUME you would probably
> want the "pos" to go from 0 to "steps" and then clamp. The encoder
> could actually rotate multiple times depending on it's actual line
> count, but the effect would be correct.

I see. Care to prepare a patch? Thanks!

-- 
Dmitry