Re: [RESEND PATCH 1/2] Input: mt6779-keypad - fix hardware code mapping

[AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>]

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@gmail.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...

...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.

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel