Re: [PATCH v2 0/3] Input: rotary-encoder - use more than two gpios

["Uwe Kleine-König" <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>]

Hello Daniel,

On Tue, Feb 02, 2016 at 02:14:49PM +0100, Daniel Mack wrote:
> On 02/02/2016 01:56 PM, Uwe Kleine-König wrote:
> > Hello Daniel,
> > 
> > On Tue, Feb 02, 2016 at 01:08:07PM +0100, Daniel Mack wrote:
> >> On 02/02/2016 11:24 AM, Uwe Kleine-König wrote:
> >>> 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 <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> >>> 	Subject: [PATCH RFC 0/3] input: rotary_encoder: use more than two gpios as input
> >>> 	Message-Id: <1449050834-31779-1-git-send-email-u.kleine-koenig@pengutronix.de>
> >>>
> >>> 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@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>;
> >>>         };
> >>>
> >>> 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.
> >>
> >> 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?

Not sure about how this will be represented in the device tree, but yes,
that's more or less how I imagine to implement this. (And in the end
having support for a 4-line relative device is just a side effect of
having support for relative devices and for 4-line devices.)

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html