Thread (12 messages) 12 messages, 4 authors, 2015-10-08

Re: [PATCH 3/3] Input: adp5589-keys: fix event count mask

From: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
Date: 2015-10-08 14:21:13

On 5 October 2015 at 21:29, Dmitry Torokhov [off-list ref] wrote:
Hi Ezequiel,

On Fri, Oct 2, 2015 at 2:28 PM, Ezequiel Garcia
[off-list ref] wrote:
quoted
On 6 May 2015 at 20:36, Dmitry Torokhov [off-list ref] wrote:
quoted
On Tue, Apr 21, 2015 at 04:36:44PM +0200, Michael Hennerich wrote:
quoted
On 04/21/2015 04:21 PM, Guido Martínez wrote:
quoted
The event mask was specified as 0xF (4 bits) when in reality is 0x1F (5
bits) in order to be capable of representing all FIFO length values from
0 to 16.

This caused a problem: when the keypad reported 16 pending events the
driver took it as 0, and did nothing. This in turn caused the keypad to
re-issue the interrupt over and over again.

Fixes: 9d2e173644bb ('Input: ADP5589 - new driver for I2C Keypad Decoder and I/O Expander')
Signed-off-by: Guido Martínez <redacted>
Acked-by: Michael Hennerich <michael.hennerich@analog.com>
Dmitry,

Going through this patchset and noticed the "Fixes" tag got dropped when
the last two fixes were pushed.
I believe "Fixes" tag is interesting when there was a patch that
changed behavior and broke things, not when there is a mistake that
was present since very beginning.
If anything else, the Fixes tag clearly states the commit fixes something
(if the commit log is not clear enough). And if the bug was there from the
very beginning, so be it. The tag is only clarifying that.

I'd say the more information about the commit, the better.
quoted
More importantly, the patches haven't been queued for -stable, despite it
fixes a serious issue.

Can you take care of sending them to -stable or should I do it?
Upstream commits are:
c615dcb6d13e Input: adp5589-keys - fix event count mask
195e610bf710 Input: adp5589-keys - fix pull mask setting
Given that there were no complaints about this issues ever since the
driver has been accepted into main line (3.0 kernel) and that the
device is not very widely used I do not believe this has to go to
stable. Affected parties can cherry-pick from mainline.
Well, the event triggering the bug was quite marginal, so it's fairly
hard to hit
it in real life.

On the other side, the bug itself was a very subtle typo
in the mask value, and took a lot of effort to catch. I thought that effort
worth a simple backport to stable.

Having a bug in mainline, and deciding not to backport it to stable,
seems wrong to me, despite lack of (known) complaints and
despite how unused a driver may appear to be.

Anyway, it's your call. I'm happy with whatever you decide.
-- 
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar
--
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help