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@xxxxxxxxxxxxxx> > >>> 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@xxxxxxxxxxxxxx> > >>> > >>> 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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html