| Message ID | 20220513151845.2802795-3-mkorpershoek@baylibre.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Input: mt6779-keypad - fix hw code logic and row/col selection | expand |
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) ? Thanks.
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
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);
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(+)