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 <[email protected]>
---
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)))
+#define MTK_KPD_SEL_ROWMASK(r) (MTK_KPD_SEL_ROW >> (6 - (r)))
#define MTK_KPD_NUM_MEMS 5
#define MTK_KPD_NUM_BITS 136 /* 4*32+8 MEM5 only use 8 BITS */
@@ -171,6 +176,11 @@ static int mt6779_keypad_pdrv_probe(struct platform_device *pdev)
regmap_write(keypad->regmap, MTK_KPD_DEBOUNCE,
(debounce * (1 << 5)) & MTK_KPD_DEBOUNCE_MASK);
+ regmap_update_bits(keypad->regmap, MTK_KPD_SEL, MTK_KPD_SEL_ROW,
+ MTK_KPD_SEL_ROWMASK(keypad->n_rows));
+ regmap_update_bits(keypad->regmap, MTK_KPD_SEL, MTK_KPD_SEL_COL,
+ MTK_KPD_SEL_COLMASK(keypad->n_cols));
+
keypad->clk = devm_clk_get(&pdev->dev, "kpd");
if (IS_ERR(keypad->clk))
return PTR_ERR(keypad->clk);
--
2.32.0
Hi Dmitry,
Thank you for your review,
On dim., mai 15, 2022 at 22:21, Dmitry Torokhov <[email protected]> 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 <[email protected]>
>> ---
>> 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
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 <[email protected]>
> ---
> 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)
?
Thanks.
--
Dmitry