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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 5 October 2015 at 21:29, Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> wrote:
> Hi Ezequiel,
>
> On Fri, Oct 2, 2015 at 2:28 PM, Ezequiel Garcia
> <ezequiel@xxxxxxxxxxxxxxxxxxxx> wrote:
>> On 6 May 2015 at 20:36, Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> wrote:
>>> On Tue, Apr 21, 2015 at 04:36:44PM +0200, Michael Hennerich wrote:
>>>> On 04/21/2015 04:21 PM, Guido Martínez wrote:
>>>> >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 <guido@xxxxxxxxxxxxxxxxxxxx>
>>>> Acked-by: Michael Hennerich <michael.hennerich@xxxxxxxxxx>
>>>
>>
>> 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.

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



[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux