All of lore.kernel.org
 help / color / mirror / Atom feed
* [RESEND PATCH v3 0/2] Input: mt6779-keypad - fix hw code logic and row/col selection
@ 2022-07-07  7:52 ` Mattijs Korpershoek
  0 siblings, 0 replies; 8+ messages in thread
From: Mattijs Korpershoek @ 2022-07-07  7:52 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Matthias Brugger, AngeloGioacchino Del Regno, Kevin Hilman,
	Fabien Parent, linux-input, linux-mediatek, linux-arm-kernel,
	linux-kernel, Mattijs Korpershoek

Input: mt6779-keypad - fix hw code logic and row/col selection

This serie is the first follow-up on the mt6779-keypad in
order to enable it on the MediaTek mt8183-pumpkin board.

To fully enable it on mt8183-pumpkin, we still need:
* double key support
* dts changes

To ease up reviewing, I preferred sending this first.

The first patch fixes a logic bug based on the (non-public) datasheet
I have.
The second patch configures the keypad correctly in order to not
report bogus values.

Thank you,
Mattijs

Changes in v3:
* reworked row/column logic as discussed in [1]
* Dropped Angelo's review since patch 1 changed

Changes in v2:
* Simplified SEL_COL/ROW_MASK macros as suggested by Dmitry
* Added Angelo's Reviewed-by on patch 1

Mattijs Korpershoek (2):
  Input: mt6779-keypad - match hardware matrix organization
  Input: mt6779-keypad - implement row/column selection

 drivers/input/keyboard/mt6779-keypad.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

[1] https://lore.kernel.org/r/YpMDZORAlHmg/x/0@google.com

base-commit: c4bcc1b99b8b8acdfe673e4701a9c2acb6b8b2fb
-- 
2.34.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [RESEND PATCH v3 0/2] Input: mt6779-keypad - fix hw code logic and row/col selection
@ 2022-07-07  7:52 ` Mattijs Korpershoek
  0 siblings, 0 replies; 8+ messages in thread
From: Mattijs Korpershoek @ 2022-07-07  7:52 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Matthias Brugger, AngeloGioacchino Del Regno, Kevin Hilman,
	Fabien Parent, linux-input, linux-mediatek, linux-arm-kernel,
	linux-kernel, Mattijs Korpershoek

Input: mt6779-keypad - fix hw code logic and row/col selection

This serie is the first follow-up on the mt6779-keypad in
order to enable it on the MediaTek mt8183-pumpkin board.

To fully enable it on mt8183-pumpkin, we still need:
* double key support
* dts changes

To ease up reviewing, I preferred sending this first.

The first patch fixes a logic bug based on the (non-public) datasheet
I have.
The second patch configures the keypad correctly in order to not
report bogus values.

Thank you,
Mattijs

Changes in v3:
* reworked row/column logic as discussed in [1]
* Dropped Angelo's review since patch 1 changed

Changes in v2:
* Simplified SEL_COL/ROW_MASK macros as suggested by Dmitry
* Added Angelo's Reviewed-by on patch 1

Mattijs Korpershoek (2):
  Input: mt6779-keypad - match hardware matrix organization
  Input: mt6779-keypad - implement row/column selection

 drivers/input/keyboard/mt6779-keypad.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

[1] https://lore.kernel.org/r/YpMDZORAlHmg/x/0@google.com

base-commit: c4bcc1b99b8b8acdfe673e4701a9c2acb6b8b2fb
-- 
2.34.1


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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [RESEND PATCH v3 1/2] Input: mt6779-keypad - match hardware matrix organization
  2022-07-07  7:52 ` Mattijs Korpershoek
@ 2022-07-07  7:52   ` Mattijs Korpershoek
  -1 siblings, 0 replies; 8+ messages in thread
From: Mattijs Korpershoek @ 2022-07-07  7:52 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Matthias Brugger, AngeloGioacchino Del Regno, Kevin Hilman,
	Fabien Parent, linux-input, linux-mediatek, linux-arm-kernel,
	linux-kernel, Mattijs Korpershoek, Fabien Parent

The MediaTek keypad has a set of bits representing keys,
from KEY0 to KEY77, arranged in 5 chunks of 15 bits split into 5 32-bit
registers.

In our implementation, we simply decided to use register number as row
and offset in the register as column when encoding our "matrix".

Because of this, we can have a 5x32 matrix which does not match the
hardware at all, which is confusing.

Change the row/column calculation to match the hardware.

Fixes: f28af984e771 ("Input: mt6779-keypad - add MediaTek keypad driver")
Co-developed-by: Fabien Parent <fparent@baylibre.com>
Signed-off-by: Fabien Parent <fparent@baylibre.com>
Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
---
 drivers/input/keyboard/mt6779-keypad.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/input/keyboard/mt6779-keypad.c b/drivers/input/keyboard/mt6779-keypad.c
index 2e7c9187c10f..bd86cb95bde3 100644
--- a/drivers/input/keyboard/mt6779-keypad.c
+++ b/drivers/input/keyboard/mt6779-keypad.c
@@ -42,7 +42,7 @@ static irqreturn_t mt6779_keypad_irq_handler(int irq, void *dev_id)
 	const unsigned short *keycode = keypad->input_dev->keycode;
 	DECLARE_BITMAP(new_state, MTK_KPD_NUM_BITS);
 	DECLARE_BITMAP(change, MTK_KPD_NUM_BITS);
-	unsigned int bit_nr;
+	unsigned int bit_nr, key;
 	unsigned int row, col;
 	unsigned int scancode;
 	unsigned int row_shift = get_count_order(keypad->n_cols);
@@ -61,8 +61,10 @@ static irqreturn_t mt6779_keypad_irq_handler(int irq, void *dev_id)
 		if (bit_nr % 32 >= 16)
 			continue;
 
-		row = bit_nr / 32;
-		col = bit_nr % 32;
+		key = bit_nr / 32 * 16 + bit_nr % 32;
+		row = key / 9;
+		col = key % 9;
+
 		scancode = MATRIX_SCAN_CODE(row, col, row_shift);
 		/* 1: not pressed, 0: pressed */
 		pressed = !test_bit(bit_nr, new_state);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [RESEND PATCH v3 1/2] Input: mt6779-keypad - match hardware matrix organization
@ 2022-07-07  7:52   ` Mattijs Korpershoek
  0 siblings, 0 replies; 8+ messages in thread
From: Mattijs Korpershoek @ 2022-07-07  7:52 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Matthias Brugger, AngeloGioacchino Del Regno, Kevin Hilman,
	Fabien Parent, linux-input, linux-mediatek, linux-arm-kernel,
	linux-kernel, Mattijs Korpershoek, Fabien Parent

The MediaTek keypad has a set of bits representing keys,
from KEY0 to KEY77, arranged in 5 chunks of 15 bits split into 5 32-bit
registers.

In our implementation, we simply decided to use register number as row
and offset in the register as column when encoding our "matrix".

Because of this, we can have a 5x32 matrix which does not match the
hardware at all, which is confusing.

Change the row/column calculation to match the hardware.

Fixes: f28af984e771 ("Input: mt6779-keypad - add MediaTek keypad driver")
Co-developed-by: Fabien Parent <fparent@baylibre.com>
Signed-off-by: Fabien Parent <fparent@baylibre.com>
Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
---
 drivers/input/keyboard/mt6779-keypad.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/input/keyboard/mt6779-keypad.c b/drivers/input/keyboard/mt6779-keypad.c
index 2e7c9187c10f..bd86cb95bde3 100644
--- a/drivers/input/keyboard/mt6779-keypad.c
+++ b/drivers/input/keyboard/mt6779-keypad.c
@@ -42,7 +42,7 @@ static irqreturn_t mt6779_keypad_irq_handler(int irq, void *dev_id)
 	const unsigned short *keycode = keypad->input_dev->keycode;
 	DECLARE_BITMAP(new_state, MTK_KPD_NUM_BITS);
 	DECLARE_BITMAP(change, MTK_KPD_NUM_BITS);
-	unsigned int bit_nr;
+	unsigned int bit_nr, key;
 	unsigned int row, col;
 	unsigned int scancode;
 	unsigned int row_shift = get_count_order(keypad->n_cols);
@@ -61,8 +61,10 @@ static irqreturn_t mt6779_keypad_irq_handler(int irq, void *dev_id)
 		if (bit_nr % 32 >= 16)
 			continue;
 
-		row = bit_nr / 32;
-		col = bit_nr % 32;
+		key = bit_nr / 32 * 16 + bit_nr % 32;
+		row = key / 9;
+		col = key % 9;
+
 		scancode = MATRIX_SCAN_CODE(row, col, row_shift);
 		/* 1: not pressed, 0: pressed */
 		pressed = !test_bit(bit_nr, new_state);
-- 
2.34.1


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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [RESEND PATCH v3 2/2] Input: mt6779-keypad - implement row/column selection
  2022-07-07  7:52 ` Mattijs Korpershoek
@ 2022-07-07  7:52   ` Mattijs Korpershoek
  -1 siblings, 0 replies; 8+ messages in thread
From: Mattijs Korpershoek @ 2022-07-07  7:52 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Matthias Brugger, AngeloGioacchino Del Regno, Kevin Hilman,
	Fabien Parent, linux-input, linux-mediatek, linux-arm-kernel,
	linux-kernel, Mattijs Korpershoek

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 bd86cb95bde3..bf447bf598fb 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)	GENMASK((c) + 9, 10)
+#define MTK_KPD_SEL_ROWMASK(r)	GENMASK((r) + 3, 4)
 #define MTK_KPD_NUM_MEMS	5
 #define MTK_KPD_NUM_BITS	136	/* 4*32+8 MEM5 only use 8 BITS */
 
@@ -161,6 +166,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.34.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [RESEND PATCH v3 2/2] Input: mt6779-keypad - implement row/column selection
@ 2022-07-07  7:52   ` Mattijs Korpershoek
  0 siblings, 0 replies; 8+ messages in thread
From: Mattijs Korpershoek @ 2022-07-07  7:52 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Matthias Brugger, AngeloGioacchino Del Regno, Kevin Hilman,
	Fabien Parent, linux-input, linux-mediatek, linux-arm-kernel,
	linux-kernel, Mattijs Korpershoek

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 bd86cb95bde3..bf447bf598fb 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)	GENMASK((c) + 9, 10)
+#define MTK_KPD_SEL_ROWMASK(r)	GENMASK((r) + 3, 4)
 #define MTK_KPD_NUM_MEMS	5
 #define MTK_KPD_NUM_BITS	136	/* 4*32+8 MEM5 only use 8 BITS */
 
@@ -161,6 +166,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.34.1


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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [RESEND PATCH v3 0/2] Input: mt6779-keypad - fix hw code logic and row/col selection
  2022-07-07  7:52 ` Mattijs Korpershoek
@ 2022-07-08 21:58   ` Dmitry Torokhov
  -1 siblings, 0 replies; 8+ messages in thread
From: Dmitry Torokhov @ 2022-07-08 21:58 UTC (permalink / raw)
  To: Mattijs Korpershoek
  Cc: Matthias Brugger, AngeloGioacchino Del Regno, Kevin Hilman,
	Fabien Parent, linux-input, linux-mediatek, linux-arm-kernel,
	linux-kernel

On Thu, Jul 07, 2022 at 09:52:34AM +0200, Mattijs Korpershoek wrote:
> Input: mt6779-keypad - fix hw code logic and row/col selection
> 
> This serie is the first follow-up on the mt6779-keypad in
> order to enable it on the MediaTek mt8183-pumpkin board.
> 
> To fully enable it on mt8183-pumpkin, we still need:
> * double key support
> * dts changes
> 
> To ease up reviewing, I preferred sending this first.
> 
> The first patch fixes a logic bug based on the (non-public) datasheet
> I have.
> The second patch configures the keypad correctly in order to not
> report bogus values.
> 
> Thank you,
> Mattijs
> 
> Changes in v3:
> * reworked row/column logic as discussed in [1]
> * Dropped Angelo's review since patch 1 changed
> 
> Changes in v2:
> * Simplified SEL_COL/ROW_MASK macros as suggested by Dmitry
> * Added Angelo's Reviewed-by on patch 1
> 
> Mattijs Korpershoek (2):
>   Input: mt6779-keypad - match hardware matrix organization
>   Input: mt6779-keypad - implement row/column selection
> 
>  drivers/input/keyboard/mt6779-keypad.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> [1] https://lore.kernel.org/r/YpMDZORAlHmg/x/0@google.com
> 
> base-commit: c4bcc1b99b8b8acdfe673e4701a9c2acb6b8b2fb
> -- 
> 2.34.1
> 

Applied the series, thank you.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RESEND PATCH v3 0/2] Input: mt6779-keypad - fix hw code logic and row/col selection
@ 2022-07-08 21:58   ` Dmitry Torokhov
  0 siblings, 0 replies; 8+ messages in thread
From: Dmitry Torokhov @ 2022-07-08 21:58 UTC (permalink / raw)
  To: Mattijs Korpershoek
  Cc: Matthias Brugger, AngeloGioacchino Del Regno, Kevin Hilman,
	Fabien Parent, linux-input, linux-mediatek, linux-arm-kernel,
	linux-kernel

On Thu, Jul 07, 2022 at 09:52:34AM +0200, Mattijs Korpershoek wrote:
> Input: mt6779-keypad - fix hw code logic and row/col selection
> 
> This serie is the first follow-up on the mt6779-keypad in
> order to enable it on the MediaTek mt8183-pumpkin board.
> 
> To fully enable it on mt8183-pumpkin, we still need:
> * double key support
> * dts changes
> 
> To ease up reviewing, I preferred sending this first.
> 
> The first patch fixes a logic bug based on the (non-public) datasheet
> I have.
> The second patch configures the keypad correctly in order to not
> report bogus values.
> 
> Thank you,
> Mattijs
> 
> Changes in v3:
> * reworked row/column logic as discussed in [1]
> * Dropped Angelo's review since patch 1 changed
> 
> Changes in v2:
> * Simplified SEL_COL/ROW_MASK macros as suggested by Dmitry
> * Added Angelo's Reviewed-by on patch 1
> 
> Mattijs Korpershoek (2):
>   Input: mt6779-keypad - match hardware matrix organization
>   Input: mt6779-keypad - implement row/column selection
> 
>  drivers/input/keyboard/mt6779-keypad.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> [1] https://lore.kernel.org/r/YpMDZORAlHmg/x/0@google.com
> 
> base-commit: c4bcc1b99b8b8acdfe673e4701a9c2acb6b8b2fb
> -- 
> 2.34.1
> 

Applied the series, thank you.

-- 
Dmitry

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-07-08 21:59 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-07  7:52 [RESEND PATCH v3 0/2] Input: mt6779-keypad - fix hw code logic and row/col selection Mattijs Korpershoek
2022-07-07  7:52 ` Mattijs Korpershoek
2022-07-07  7:52 ` [RESEND PATCH v3 1/2] Input: mt6779-keypad - match hardware matrix organization Mattijs Korpershoek
2022-07-07  7:52   ` Mattijs Korpershoek
2022-07-07  7:52 ` [RESEND PATCH v3 2/2] Input: mt6779-keypad - implement row/column selection Mattijs Korpershoek
2022-07-07  7:52   ` Mattijs Korpershoek
2022-07-08 21:58 ` [RESEND PATCH v3 0/2] Input: mt6779-keypad - fix hw code logic and row/col selection Dmitry Torokhov
2022-07-08 21:58   ` Dmitry Torokhov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.