From: Mattijs Korpershoek <mkorpershoek@baylibre.com>
To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Matthias Brugger <matthias.bgg@gmail.com>,
AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com>,
Kevin Hilman <khilman@baylibre.com>,
Fabien Parent <fparent@baylibre.com>,
linux-input@vger.kernel.org, linux-mediatek@lists.infradead.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Subject: Re: [RESEND PATCH 2/2] Input: mt6779-keypad - implement row/column selection
Date: Mon, 16 May 2022 09:30:49 +0200 [thread overview]
Message-ID: <875ym6kk7q.fsf@baylibre.com> (raw)
In-Reply-To: <YoHfUYmq49V5Bygd@google.com>
Hi Dmitry,
Thank you for your review,
On dim., mai 15, 2022 at 22:21, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> On Fri, May 13, 2022 at 05:18:45PM +0200, Mattijs Korpershoek wrote:
>> The MediaTek keypad has a total of 6 input rows and 6 input columns.
>> By default, rows/columns 0-2 are enabled.
>>
>> This is controlled by the KP_SEL register:
>> - bits[9:4] control row selection
>> - bits[15:10] control column selection
>>
>> Each bit enables the corresponding row/column number (e.g KP_SEL[4]
>> enables ROW0)
>>
>> Depending on how the keypad is wired, this may result in wrong readings
>> of the keypad state.
>>
>> Program the KP_SEL register to limit the key detection to n_rows,
>> n_cols we retrieve from the device tree.
>>
>> Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
>> ---
>> drivers/input/keyboard/mt6779-keypad.c | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/input/keyboard/mt6779-keypad.c b/drivers/input/keyboard/mt6779-keypad.c
>> index 23360de20da5..653dfc619696 100644
>> --- a/drivers/input/keyboard/mt6779-keypad.c
>> +++ b/drivers/input/keyboard/mt6779-keypad.c
>> @@ -17,6 +17,11 @@
>> #define MTK_KPD_DEBOUNCE 0x0018
>> #define MTK_KPD_DEBOUNCE_MASK GENMASK(13, 0)
>> #define MTK_KPD_DEBOUNCE_MAX_MS 256
>> +#define MTK_KPD_SEL 0x0020
>> +#define MTK_KPD_SEL_COL GENMASK(15, 10)
>> +#define MTK_KPD_SEL_ROW GENMASK(9, 4)
>> +#define MTK_KPD_SEL_COLMASK(c) (MTK_KPD_SEL_COL >> (6 - (c)))
>
> Would it be clearer to say
>
> #define MTK_KPD_SEL_COLMASK(c) GENMASK((c) + 3, 4)
Per my understanding, GENMASK is mostly used for build-time constants
since it does compile-time checks with BUILD_BUG_ON_ZERO.
Since "c" is defined at run time (as it comes from keypad->n_cols, which
comes from the device tree), it felt more appropriate to not use GENMASK.
Looking again, i've found examples in sound/soc/codecs/cs35l41-lib.c
that use GENMASK with a run-time value.
Will send a v2 using GENMASK, thank you for the suggestion.
>
> ?
>
> Thanks.
>
> --
> Dmitry
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
prev parent reply other threads:[~2022-05-16 7:32 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-13 15:18 [RESEND PATCH 0/2] Input: mt6779-keypad - fix hw code logic and row/col selection Mattijs Korpershoek
2022-05-13 15:18 ` [RESEND PATCH 1/2] Input: mt6779-keypad - fix hardware code mapping Mattijs Korpershoek
2022-05-16 5:23 ` Dmitry Torokhov
2022-05-16 7:30 ` Mattijs Korpershoek
2022-05-16 11:06 ` AngeloGioacchino Del Regno
2022-05-18 6:56 ` Mattijs Korpershoek
2022-05-23 5:42 ` Dmitry Torokhov
2022-05-24 19:21 ` Mattijs Korpershoek
2022-05-29 5:23 ` Dmitry Torokhov
2022-06-14 9:14 ` Mattijs Korpershoek
2022-05-13 15:18 ` [RESEND PATCH 2/2] Input: mt6779-keypad - implement row/column selection Mattijs Korpershoek
2022-05-16 5:21 ` Dmitry Torokhov
2022-05-16 7:30 ` Mattijs Korpershoek [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=875ym6kk7q.fsf@baylibre.com \
--to=mkorpershoek@baylibre.com \
--cc=angelogioacchino.delregno@collabora.com \
--cc=dmitry.torokhov@gmail.com \
--cc=fparent@baylibre.com \
--cc=khilman@baylibre.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=matthias.bgg@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).