Re: [PATCH 1/1] Input: rotary-encoder: Add 'stepped' irq handler

[Daniel Mack <zonque@gmail.com>]

On 29.09.2013 19:29, Ezequiel García wrote:
> On 29 September 2013 07:40, Daniel Mack <zonque@gmail.com> wrote:
>> On 28.09.2013 20:26, Ezequiel Garcia wrote:

>>> +     sum = (encoder->last_stable << 2) + state;
>>> +     switch (sum) {
>>> +     case 0b1101:
>>> +     case 0b0100:
>>> +     case 0b0010:
>>> +     case 0b1011:
>>
>> Binary constants are frowned upon, please avoid them in the kernel.
>> checkpatch.pl should have warned you about them as well.
>>
> 
> Well... despite any checkpatch.pl warnings, I think the above is much clear
> to the reader than any alternative.

The problem is that support for that notation is a proprietary gcc
extension that is AFAIK only supported from gcc 4.3 onwards or so.
However, the current minimum gcc version for building the kernel is 3.2.

> If binary values should be avoided by all means, then I would prefer to encode
> the previous and current in different nibbles:
> 
> sum = (encoder->last_stable << 4) + state;
> switch (sum) {
>         case 0x31:
>         case 0x10:
>         case 0x02:
>         case 0x23:
> 
> Maybe this is better?

Either that, or use
	case BIT(3) | BIT(2) | BIT(0):
		...

> I'm really curious about the rotary devices you originally used with
> this driver.
> I guess those have no detents, so there's no mechanical-click on each step?

Some models have detents, some don't. We've used one of this series
which does:


http://de.mouser.com/ProductDetail/Alpha-Taiwan/RE111F-20B3-20F-20P/?qs=yA6kp8fx8Y7KsyMOFz9p0A==



Best regards,
Daniel

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html