Skip to content

Input: rotary-encoder - use more than two gpios

Daniel Mack linux-arm-kernel linux-devicetree linux-input 3 versions – New

Get series with b4
b4 am [email protected]

Versions newest first · linked by author and title

  1. v2 Latest posting 2/21 of 3 patches 0 repliesNew
  2. v2 posting 1/22 of 3 patches 0 repliesSuperseded
  3. v1 RFC0 of 3 patches 0 repliesSuperseded

1 of 3 patches is in this archive. The cover letter announces 3.

Patches in v2 posting 2/2 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/3Input: rotary-encoder - make use of devm_* to simplify .probe and .remove+0 -00 replies

Reviews trailers per patch

The patch has no Reviewed-by or Acked-by of their own; the series has one for all of them.

PatchDaniel Mack
SeriesA-b
1/3

Trailers given in replies are filled; those carried in the patches from earlier versions are outlined with dashes.

Read the v2 thread

Cover letter [PATCH v2 0/3] Input: rotary-encoder - use more than two gpios
Quoted 46 lines · Uwe Kleine-K?nig

Hello Daniel,

Quoted 43 lines · Daniel Mack
Quoted 26 lines · Uwe Kleine-K?nig

Some time ago I sent a v1 of this, now after testing the changes more deeply patch 3 changed a bit. The old series started with

	Date: Wed,  2 Dec 2015 11:07:11 +0100
	From: Uwe Kleine-K?nig <[email protected]>
	Subject: [PATCH RFC 0/3] input: rotary_encoder: use more than two gpios as input
	Message-Id: <[email protected]>

The two first patches are just preparation for the third patch.

There is an obvious improvement that allows detection of quick changes more reliably with >2 gpios, but I didn't implement this yet. (With 4 GPIOs you can distinguish a counter clockwise movement of three states from a clock wise movement of a single state. Still the patch is useful as it makes these devices work at all.

My test device looks as follows:

        rotary at 0 {
                compatible = "rotary-encoder";
                gpios = <&gpio4 12 GPIO_ACTIVE_HIGH>, <&gpio4 11 GPIO_ACTIVE_HIGH>, <&gpio4 10 GPIO_ACTIVE_HIGH>, <&gpio4 9 GPIO_ACTIVE_HIGH>;

                rotary-encoder,steps = <16>;
		rotary-encoder,steps-per-period = <16>;
        };
Quoted 5 lines

While Daniel Mack and Rojhalat Ibrahim agreed that this device is an absolute encoder and should be supported by a simpler logic, I still consider it worthwhile to get these patches in as a first step. Also the binding looks right, so IMHO the comments shouldn't stop this series from going in.

I still don't understand why this is implemented that way, rather than going for a much simpler logic of interpretation that also allows users to read out the absolute position.

The code to read the value would be really just as simple as reading all GPIOs and or-ing their values into the result, and skip the state machine completely. This code would be active if a new attribute (something like 'rotary-encoder,hardware-absolute') is set, or even implicitly, when more than 2 GPIOs are specified.

Quoted 7 lines
Quoted 1 line

Is there any reason for not doing that?

Currently the reason is lack of time. And when implementing rotary-encoder,hardware-absolute something similar would be the result for the relative reporting anyhow. So the problem is only that I don't have absolute support yet, but the patches as is would be the base for that anyhow.

Because you would support relative support for such 4-pin encoders as well? I would have thought that absolute encoders would report absolute values only, but I guess you have a point here. Just to make sure we're on the same page: For more than 2 GPIOs, and an absent "rotary-encoder,relative-axis", the driver would switch to a mode in which it bypasses the state machine, right?

If you're planning to implement that eventually, I'm fine with the patches for now :)

Dmitry, if you are apply them, feel free to add my

to all 3 of them.

Thanks,
Daniel