linux-kernel - Re: [RESEND PATCH 1/2] Input: mt6779-keypad

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87bkvve3cc.fsf@baylibre.com>
Date:   Wed, 18 May 2022 08:56:19 +0200
From:   Mattijs Korpershoek <mkorpershoek@...libre.com>
To:     AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...labora.com>,
        Dmitry Torokhov <dmitry.torokhov@...il.com>
Cc:     Matthias Brugger <matthias.bgg@...il.com>,
        Kevin Hilman <khilman@...libre.com>,
        Fabien Parent <fparent@...libre.com>,
        linux-input@...r.kernel.org, linux-mediatek@...ts.infradead.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [RESEND PATCH 1/2] Input: mt6779-keypad - fix hardware code
 mapping

On Mon, May 16, 2022 at 13:06, AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com> wrote:

> Il 16/05/22 09:30, Mattijs Korpershoek ha scritto:
>> Hi Dmitry,
>> 
>> Thank you for your review,
>> 
>> On dim., mai 15, 2022 at 22:23, Dmitry Torokhov <dmitry.torokhov@...il.com> wrote:
>> 
>>> On Fri, May 13, 2022 at 05:18:44PM +0200, Mattijs Korpershoek wrote:
>>>> In mt6779_keypad_irq_handler(), we
>>>> 1. Read a hardware code from KPD_MEM1 -> KPD_MEM5
>>>> 2. Use that hardware code to compute columns/rows for the standard
>>>>     keyboard matrix.
>>>>
>>>> According to the (non-public) datasheet, the
>>>> map between the hardware code and the cols/rows is:
>>>>
>>>>          |(0)  |(1)  |(2)
>>>>      ----*-----*-----*-----
>>>>          |     |     |
>>>>          |(9)  |(10) |(11)
>>>>      ----*-----*-----*-----
>>>>          |     |     |
>>>>          |(18) |(19) |(20)
>>>>      ----*-----*-----*-----
>>>>          |     |     |
>>>>
>>>> This brings us to another formula:
>>>> -> row = code / 9;
>>>> -> col = code % 3;
>>>
>>> What if there are more than 3 columns?
>> That's not supported, in hardware, according to the datasheet.
>> 
>> The datasheet I have states that "The interface of MT6763 only supports
>> 3*3 single or 2*2 double, but internal ASIC still detects keys in the
>> manner of 8*8 single, and 3*3 double. The registers and key codes still
>> follows the legacy naming".
>> 
>> Should I add another patch in this series to add that limitation in the
>> probe? There are no checks done in the probe() right now.
>> 
>
> I've just checked a downstream kernel for MT6795 and that one looks like
> being fully compatible with this driver as well... and as far as downstream
> is concerned, apparently, mt6735, 6739, 6755, 6757, 6758, 6763, 6771, 6775
> all have the same register layout and the downstream driver for these is
> always the very same one...
Thank you for taking the time to check in your downstream kernels, I
really appreciate it.

>
> ...so, I don't think that there's currently any SoC that supports more than
> three columns. Besides, a fast check shows that MT8195 also has the same.
> At this point, I'd say that assuming that there are 3 columns, nor less, not
> more, is just fine.
>
> To stay on the safe side, though, perhaps add a comment explaining that
> this driver works on that assumption? ..but that's clear, anyway, if you
> actually read the code.
>
>  From my perspective, this commit is good to go.
I will keep as is for v2 series and apply your review tag. thanks again !

>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>