On Wed, Apr 15, 2009 at 11:11:41PM -0400, H Hartley Sweeten wrote:
> >> 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,
>
> How's this? I left the ability for the original ABS_* axis
> operation since that might be what Daniel Mack needed.
Yep, thanks. Even though now as axis can be relative, I realize this
actually makes a lot more sense in my application, too :) But we should
still leave it in for others. The more versatile the driver is, the
better.
Acked-by: Daniel Mack <daniel@xxxxxxxx>
> Input: rotary_encoder - add support for REL_* axes
>
> From: H Hartley Sweeten <hartleys@xxxxxxxxxxxxxxxxxxx>
>
> The rotary encoder driver only supports returning input events
> for ABS_* axes, this adds support for REL_* axes. The relative
> axis input event is reported as -1 for each counter-clockwise
> step and +1 for each clockwise step.
>
> The ability to clamp the position of ABS_* axes between 0 and
> a maximum of "steps" has also been added.
>
> Signed-off-by: H Hartley Sweeten <hsweeten@xxxxxxxxxxxxxxxxxxx>
> Cc: Daniel Mack <daniel@xxxxxxxx>
> Signed-off-by: Dmitry Torokhov <dtor@xxxxxxx>
>
> ---
>
> diff --git a/Documentation/input/rotary-encoder.txt
> b/Documentation/input/rotary-encoder.txt
> index 435102a..3a6aec4 100644
> --- a/Documentation/input/rotary-encoder.txt
> +++ b/Documentation/input/rotary-encoder.txt
> @@ -67,7 +67,12 @@ data with it.
> struct rotary_encoder_platform_data is declared in
> include/linux/rotary-encoder.h and needs to be filled with the number
> of
> steps the encoder has and can carry information about externally
> inverted
> -signals (because of used invertig buffer or other reasons).
> +signals (because of an inverting buffer or other reasons). The encoder
> +can be set up to deliver input information as either an absolute or
> relative
> +axes. For relative axes the input event returns +/-1 for each step. For
> +absolute axes the position of the encoder can either roll over between
> zero
> +and the number of steps or will clamp at the maximum and zero depending
> on
> +the configuration.
>
> Because GPIO to IRQ mapping is platform specific, this information must
> be given in seperately to the driver. See the example below.
> @@ -85,6 +90,8 @@ be given in seperately to the driver. See the example
> below.
> static struct rotary_encoder_platform_data my_rotary_encoder_info = {
> .steps = 24,
> .axis = ABS_X,
> + .relative_axis = false,
> + .rollover = false,
> .gpio_a = GPIO_ROTARY_A,
> .gpio_b = GPIO_ROTARY_B,
> .inverted_a = 0,
> diff --git a/drivers/input/misc/rotary_encoder.c
> b/drivers/input/misc/rotary_encoder.c
> index 5bb3ab5..4a6a72a 100644
> --- a/drivers/input/misc/rotary_encoder.c
> +++ b/drivers/input/misc/rotary_encoder.c
> @@ -26,13 +26,17 @@
> #define DRV_NAME "rotary-encoder"
>
> struct rotary_encoder {
> - unsigned int irq_a;
> - unsigned int irq_b;
> - unsigned int pos;
> - unsigned int armed;
> - unsigned int dir;
> struct input_dev *input;
> struct rotary_encoder_platform_data *pdata;
> +
> + unsigned int axis;
> + unsigned int pos;
> +
> + unsigned int irq_a;
> + unsigned int irq_b;
> +
> + bool armed;
> + unsigned char dir; /* 0 - clockwise, 1 - CCW */
> };
>
> static irqreturn_t rotary_encoder_irq(int irq, void *dev_id)
> @@ -53,21 +57,32 @@ static irqreturn_t rotary_encoder_irq(int irq, void
> *dev_id)
> if (!encoder->armed)
> break;
>
> - if (encoder->dir) {
> - /* turning counter-clockwise */
> - encoder->pos += pdata->steps;
> - encoder->pos--;
> - encoder->pos %= pdata->steps;
> + if (pdata->relative_axis) {
> + input_report_rel(encoder->input, pdata->axis,
> + encoder->dir ? -1 : 1);
> } else {
> - /* turning clockwise */
> - encoder->pos++;
> - encoder->pos %= pdata->steps;
> + unsigned int pos = encoder->pos;
> +
> + if (encoder->dir) {
> + /* turning counter-clockwise */
> + if (encoder->rollover)
> + pos += pdata->steps;
> + if (pos)
> + pos--;
> + } else {
> + /* turning clockwise */
> + if (encoder->rollover || pos <
> pdata->steps)
> + pos++;
> + }
> + if (encoder->rollover)
> + pos %= pdata->steps;
> + encoder->pos = pos;
> + input_report_abs(encoder->input, pdata->axis,
> + encoder->pos);
> }
> -
> - input_report_abs(encoder->input, pdata->axis,
> encoder->pos);
> input_sync(encoder->input);
>
> - encoder->armed = 0;
> + encoder->armed = false;
> break;
>
> case 0x1:
> @@ -77,7 +92,7 @@ static irqreturn_t rotary_encoder_irq(int irq, void
> *dev_id)
> break;
>
> case 0x3:
> - encoder->armed = 1;
> + encoder->armed = true;
> break;
> }
>
> @@ -113,9 +128,15 @@ static int __devinit rotary_encoder_probe(struct
> platform_device *pdev)
> input->name = pdev->name;
> input->id.bustype = BUS_HOST;
> input->dev.parent = &pdev->dev;
> - input->evbit[0] = BIT_MASK(EV_ABS);
> - input_set_abs_params(encoder->input,
> - pdata->axis, 0, pdata->steps, 0, 1);
> +
> + if (pdata->relative_axis) {
> + input->evbit[0] = BIT_MASK(EV_ABS);
> + input_set_abs_params(encoder->input,
> + pdata->axis, 0, pdata->steps, 0,
> 1);
> + } else {
> + input->evbit[0] = BIT_MASK(EV_REL);
> + input->relbit[0] = BIT_MASK(pdata->axis);
> + }
>
> err = input_register_device(input);
> if (err) {
> diff --git a/include/linux/rotary_encoder.h
> b/include/linux/rotary_encoder.h
> index 12d63a3..215278b 100644
> --- a/include/linux/rotary_encoder.h
> +++ b/include/linux/rotary_encoder.h
> @@ -8,6 +8,8 @@ struct rotary_encoder_platform_data {
> unsigned int gpio_b;
> unsigned int inverted_a;
> unsigned int inverted_b;
> + bool relative_axis;
> + bool rollover;
> };
>
> #endif /* __ROTARY_ENCODER_H__ */
--
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
