[PATCH v1 0/2] Input: Add Nuvoton MA35D1 keypad support

[PATCH v1 0/2] Input: Add Nuvoton MA35D1 keypad support

[Feng-Yi Hsueh]

This series adds support for the Nuvoton MA35D1 hardware
matrix keypad controller.

The controller supports up to eight rows and eight columns.
It provides key press and release event registers, which
the driver handles through the Linux input subsystem.

The driver uses the common clock framework and configures
the controller's scanning and debounce parameters using
fixed register settings.

The series consists of:
- A device tree binding for the MA35D1 keypad controller.
- An input driver with Kconfig and Makefile integration.

Tested on a Nuvoton MA35D1 board with a 4x1 keypad
configuration and a 1 MHz KPI clock.

Tests performed:
- Key press and release events verified using evtest.
- Input device open and close verified using evtest.

An earlier MA35D1 keypad driver was submitted by Ming-jen Chen. This series is a separate v1 submission with a revised implementation, focusing on simpler clock management and hardware event handling

Feng-Yi Hsueh (2):
  dt-bindings: input: Add Nuvoton MA35D1 keypad controller
  Input: keyboard: Add Nuvoton MA35D1 keypad driver

 .../bindings/input/nuvoton,ma35d1-keypad.yaml |  73 ++++
 drivers/input/keyboard/Kconfig                |  11 +
 drivers/input/keyboard/Makefile               |   1 +
 drivers/input/keyboard/ma35d1_keypad.c        | 405 ++++++++++++++++++
 4 files changed, 490 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/nuvoton,ma35d1-keypad.yaml
 create mode 100644 drivers/input/keyboard/ma35d1_keypad.c

-- 
2.43.0

[PATCH v1 1/2] dt-bindings: input: Add Nuvoton MA35D1 keypad controller

[Feng-Yi Hsueh]

Document the Nuvoton MA35D1 hardware matrix keypad controller.

The controller supports up to eight rows and eight columns
and generates interrupts for key press and release events.

Signed-off-by: Feng-Yi Hsueh <fyhsueh4linux@gmail.com>
---
 .../bindings/input/nuvoton,ma35d1-keypad.yaml | 73 +++++++++++++++++++
 1 file changed, 73 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/nuvoton,ma35d1-keypad.yaml

diff --git a/Documentation/devicetree/bindings/input/nuvoton,ma35d1-keypad.yaml b/Documentation/devicetree/bindings/input/nuvoton,ma35d1-keypad.yaml
new file mode 100644
index 000000000000..be678d0b38b9
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/nuvoton,ma35d1-keypad.yaml
@@ -0,0 +1,73 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/nuvoton,ma35d1-keypad.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Nuvoton MA35D1 Matrix Keypad Controller
+
+maintainers:
+  - Feng-Yi Hsueh <fyhsueh4linux@gmail.com>
+
+description: |
+  The Nuvoton MA35D1 keypad controller scans a matrix of up to eight rows
+  and eight columns and reports key press and release interrupts.
+
+allOf:
+  - $ref: /schemas/input/matrix-keymap.yaml#
+
+properties:
+  compatible:
+    const: nuvoton,ma35d1-keypad
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  keypad,num-rows:
+    minimum: 2
+    maximum: 8
+
+  keypad,num-columns:
+    minimum: 1
+    maximum: 8
+
+  autorepeat:
+    type: boolean
+    description: Enable key autorepeat events.
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - keypad,num-rows
+  - keypad,num-columns
+  - linux,keymap
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/input/input.h>
+
+    keypad@404a0000 {
+        compatible = "nuvoton,ma35d1-keypad";
+        reg = <0x404a0000 0x10000>;
+        interrupts = <79>;
+        clocks = <&clk>;
+
+        keypad,num-rows = <4>;
+        keypad,num-columns = <1>;
+        linux,keymap = <
+            MATRIX_KEY(0, 0, KEY_1)
+            MATRIX_KEY(1, 0, KEY_2)
+            MATRIX_KEY(2, 0, KEY_3)
+            MATRIX_KEY(3, 0, KEY_4)
+        >;
+    };
-- 
2.43.0

[PATCH v1 2/2] Input: keyboard: Add Nuvoton MA35D1 keypad driver

[Feng-Yi Hsueh]

Add support for the Nuvoton MA35D1 hardware matrix keypad
controller.

The driver handles key press and release interrupts, reports
key events through the Linux input subsystem, and manages
the peripheral clock during device open, close, and system
suspend and resume.

Signed-off-by: Feng-Yi Hsueh <fyhsueh4linux@gmail.com>
---
 drivers/input/keyboard/Kconfig         |  11 +
 drivers/input/keyboard/Makefile        |   1 +
 drivers/input/keyboard/ma35d1_keypad.c | 405 +++++++++++++++++++++++++
 3 files changed, 417 insertions(+)
 create mode 100644 drivers/input/keyboard/ma35d1_keypad.c

diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index d8c64f333462..9bbcbed3d494 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -358,6 +358,17 @@ config KEYBOARD_LPC32XX
 	  To compile this driver as a module, choose M here: the
 	  module will be called lpc32xx-keys.
 
+config KEYBOARD_MA35D1
+	tristate "Nuvoton MA35D1 keypad controller"
+	depends on ARCH_MA35 || COMPILE_TEST
+	select INPUT_MATRIXKMAP
+	help
+	  Say Y here to enable support for the Nuvoton MA35D1
+	  hardware matrix keypad controller.
+
+	  To compile this driver as a module, choose M here.
+	  The module will be called ma35d1_keypad.
+
 config KEYBOARD_MAPLE
 	tristate "Maple bus keyboard"
 	depends on SH_DREAMCAST && MAPLE
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index 60bb7baf802f..fad34b7afff7 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -39,6 +39,7 @@ obj-$(CONFIG_KEYBOARD_LM8323)		+= lm8323.o
 obj-$(CONFIG_KEYBOARD_LM8333)		+= lm8333.o
 obj-$(CONFIG_KEYBOARD_LOCOMO)		+= locomokbd.o
 obj-$(CONFIG_KEYBOARD_LPC32XX)		+= lpc32xx-keys.o
+obj-$(CONFIG_KEYBOARD_MA35D1) 		+= ma35d1_keypad.o
 obj-$(CONFIG_KEYBOARD_MAPLE)		+= maple_keyb.o
 obj-$(CONFIG_KEYBOARD_MATRIX)		+= matrix_keypad.o
 obj-$(CONFIG_KEYBOARD_MAX7359)		+= max7359_keypad.o
diff --git a/drivers/input/keyboard/ma35d1_keypad.c b/drivers/input/keyboard/ma35d1_keypad.c
new file mode 100644
index 000000000000..673ad8b9704b
--- /dev/null
+++ b/drivers/input/keyboard/ma35d1_keypad.c
@@ -0,0 +1,405 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Nuvoton MA35D1 hardware matrix keypad controller
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/input.h>
+#include <linux/input/matrix_keypad.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/platform_device.h>
+#include <linux/pm.h>
+#include <linux/property.h>
+#include <linux/types.h>
+
+#define MA35D1_KPI_CONF			0x00
+#define MA35D1_KPI_3KCONF		0x04
+#define MA35D1_KPI_STATUS		0x08
+#define MA35D1_KPI_KEST0		0x10
+#define MA35D1_KPI_KPE0			0x18
+#define MA35D1_KPI_KRE0			0x20
+#define MA35D1_KPI_PRESCALDIV		0x28
+
+#define MA35D1_KPI_KROW			GENMASK(30, 28)
+#define MA35D1_KPI_KCOL			GENMASK(26, 24)
+#define MA35D1_KPI_DBCLKSEL		GENMASK(19, 16)
+#define MA35D1_KPI_PRESCALE		GENMASK(15, 8)
+#define MA35D1_KPI_INTEN		BIT(3)
+#define MA35D1_KPI_RKINTEN		BIT(2)
+#define MA35D1_KPI_PKINTEN		BIT(1)
+#define MA35D1_KPI_ENKP			BIT(0)
+
+#define MA35D1_KPI_RST3KEY		BIT(1)
+#define MA35D1_KPI_PDWAKE		BIT(0)
+#define MA35D1_KPI_IRQ_MASK		GENMASK(4, 0)
+
+#define MA35D1_KPI_MAX_ROWS		8
+#define MA35D1_KPI_MAX_COLS		8
+#define MA35D1_KPI_BANKS		2
+#define MA35D1_KPI_ROWS_PER_BANK	(MA35D1_KPI_MAX_ROWS / MA35D1_KPI_BANKS)
+
+/*
+ * Scan timing, in KPI engine clock cycles.  PRESCALE and PRESCALDIV are
+ * encoded as the count minus one:
+ *
+ *   row scan cycles = (PRESCALE + 1) * (PRESCALDIV + 1) = 2080
+ *   debounce sample cycles = 2^DBCLKSEL = 1024
+ *
+ * With SCANROWD at its reset value (4 cycles) the TRM requires
+ * row scan time > 4 + 2 * 1024 + 3 = 2055 cycles.  These limits are expressed
+ * in cycles and therefore hold at different input rates. The actual scan
+ * and debounce intervals depend on the engine clock rate; boards should
+ * select a rate suitable for their switches (1 MHz has been validated).
+ */
+#define MA35D1_KPI_PRESCALE_VAL		0x40
+#define MA35D1_KPI_DIV_VAL		0x1f
+#define MA35D1_KPI_DEBOUNCE_SEL		10
+
+struct ma35d1_keypad {
+	void __iomem *base;
+	struct clk *clk;
+	struct input_dev *input;
+	unsigned short *keycodes;
+	unsigned int rows;
+	unsigned int cols;
+	unsigned int row_shift;
+	u32 conf;
+	int irq;
+	bool running;	/* protected by input->mutex */
+};
+
+static void ma35d1_keypad_clear_events(struct ma35d1_keypad *keypad)
+{
+	unsigned int bank;
+	u32 press, release;
+
+	for (bank = 0; bank < MA35D1_KPI_BANKS; bank++) {
+		press = readl(keypad->base + MA35D1_KPI_KPE0 + bank * 4);
+		release = readl(keypad->base + MA35D1_KPI_KRE0 + bank * 4);
+		/* The event registers are write-one-to-clear. */
+		if (press)
+			writel(press, keypad->base + MA35D1_KPI_KPE0 + bank * 4);
+		if (release)
+			writel(release, keypad->base + MA35D1_KPI_KRE0 + bank * 4);
+	}
+
+	/* Only these two KPISTATUS flags have W1C semantics. */
+	writel(MA35D1_KPI_RST3KEY | MA35D1_KPI_PDWAKE,
+	       keypad->base + MA35D1_KPI_STATUS);
+}
+
+/* Release software key states when hardware scanning is stopped. */
+static void ma35d1_keypad_release_keys(struct ma35d1_keypad *keypad)
+{
+	struct input_dev *input = keypad->input;
+	unsigned int row, col, scan, code;
+	bool reported = false;
+
+	for (row = 0; row < keypad->rows; row++) {
+		for (col = 0; col < keypad->cols; col++) {
+			scan = MATRIX_SCAN_CODE(row, col, keypad->row_shift);
+			code = keypad->keycodes[scan];
+			if (code == KEY_RESERVED || !test_bit(code, input->key))
+				continue;
+
+			input_event(input, EV_MSC, MSC_SCAN, scan);
+			input_report_key(input, code, 0);
+			reported = true;
+		}
+	}
+
+	if (reported)
+		input_sync(input);
+}
+
+/* Called with input->mutex held (open, PM resume). */
+static int ma35d1_keypad_enable(struct ma35d1_keypad *keypad)
+{
+	int ret;
+
+	if (keypad->running)
+		return 0;
+
+	ret = clk_prepare_enable(keypad->clk);
+	if (ret)
+		return ret;
+
+	/* Keep hardware three-key reset disabled for a normal input device. */
+	writel(0, keypad->base + MA35D1_KPI_CONF);
+	writel(0, keypad->base + MA35D1_KPI_3KCONF);
+	writel(MA35D1_KPI_DIV_VAL, keypad->base + MA35D1_KPI_PRESCALDIV);
+	ma35d1_keypad_clear_events(keypad);
+	writel(keypad->conf, keypad->base + MA35D1_KPI_CONF);
+
+	keypad->running = true;
+	enable_irq(keypad->irq);
+
+	return 0;
+}
+
+/* Called with input->mutex held (close, PM suspend). */
+static void ma35d1_keypad_disable(struct ma35d1_keypad *keypad)
+{
+	if (!keypad->running)
+		return;
+
+	/* Wait for the IRQ handler while register access is still clocked. */
+	disable_irq(keypad->irq);
+	writel(0, keypad->base + MA35D1_KPI_CONF);
+	ma35d1_keypad_clear_events(keypad);
+	ma35d1_keypad_release_keys(keypad);
+	clk_disable_unprepare(keypad->clk);
+
+	keypad->running = false;
+}
+
+static int ma35d1_keypad_open(struct input_dev *input)
+{
+	struct ma35d1_keypad *keypad = input_get_drvdata(input);
+
+	return ma35d1_keypad_enable(keypad);
+}
+
+static void ma35d1_keypad_close(struct input_dev *input)
+{
+	struct ma35d1_keypad *keypad = input_get_drvdata(input);
+
+	ma35d1_keypad_disable(keypad);
+}
+
+static void ma35d1_keypad_report(struct ma35d1_keypad *keypad,
+				 unsigned int scan, unsigned short keycode,
+				 bool pressed)
+{
+	input_event(keypad->input, EV_MSC, MSC_SCAN, scan);
+	input_report_key(keypad->input, keycode, pressed);
+}
+
+static irqreturn_t ma35d1_keypad_irq(int irq, void *data)
+{
+	struct ma35d1_keypad *keypad = data;
+	struct input_dev *input = keypad->input;
+	bool reported = false;
+	u32 status, pressed, released, events, state;
+	unsigned int bank, bit, row, col, scan;
+	unsigned short keycode;
+	bool key_is_up;
+
+	status = readl(keypad->base + MA35D1_KPI_STATUS);
+	if (!(status & MA35D1_KPI_IRQ_MASK))
+		return IRQ_NONE;
+
+	for (bank = 0; bank < MA35D1_KPI_BANKS; bank++) {
+		pressed = readl(keypad->base + MA35D1_KPI_KPE0 + bank * 4);
+		released = readl(keypad->base + MA35D1_KPI_KRE0 + bank * 4);
+
+		/* Acknowledge the exact snapshots, not KPISTATUS[4:2]. */
+		if (pressed)
+			writel(pressed, keypad->base + MA35D1_KPI_KPE0 + bank * 4);
+		if (released)
+			writel(released, keypad->base + MA35D1_KPI_KRE0 + bank * 4);
+
+		events = pressed | released;
+		if (!events)
+			continue;
+
+		/* The live state is only needed to order a press/release pair. */
+		state = 0;
+		if (pressed & released)
+			state = readl(keypad->base + MA35D1_KPI_KEST0 + bank * 4);
+
+		while (events) {
+			bit = __ffs(events);
+			events &= ~BIT(bit);
+			row = bank * MA35D1_KPI_ROWS_PER_BANK +
+			      bit / MA35D1_KPI_MAX_COLS;
+			col = bit % MA35D1_KPI_MAX_COLS;
+
+			if (row >= keypad->rows || col >= keypad->cols)
+				continue;
+
+			scan = MATRIX_SCAN_CODE(row, col, keypad->row_shift);
+			keycode = keypad->keycodes[scan];
+			if (keycode == KEY_RESERVED)
+				continue;
+
+			if ((pressed & released) & BIT(bit)) {
+				/*
+				 * Both edges were latched before this interrupt
+				 * was serviced.  KEST is active-low (1 means
+				 * released), so use the final state to order
+				 * them: up -> press then release, down ->
+				 * release then press.  Sync in between so user
+				 * space sees two separate frames.
+				 */
+				key_is_up = !!(state & BIT(bit));
+
+				ma35d1_keypad_report(keypad, scan, keycode,
+						     key_is_up);
+				input_sync(input);
+				ma35d1_keypad_report(keypad, scan, keycode,
+						     !key_is_up);
+			} else {
+				ma35d1_keypad_report(keypad, scan, keycode,
+						     !!(pressed & BIT(bit)));
+			}
+			reported = true;
+		}
+	}
+
+	if (status & (MA35D1_KPI_RST3KEY | MA35D1_KPI_PDWAKE))
+		writel(status & (MA35D1_KPI_RST3KEY | MA35D1_KPI_PDWAKE),
+		       keypad->base + MA35D1_KPI_STATUS);
+
+	if (reported)
+		input_sync(input);
+
+	return IRQ_HANDLED;
+}
+
+static int ma35d1_keypad_suspend(struct device *dev)
+{
+	struct ma35d1_keypad *keypad = dev_get_drvdata(dev);
+	struct input_dev *input = keypad->input;
+
+	mutex_lock(&input->mutex);
+	if (input_device_enabled(input))
+		ma35d1_keypad_disable(keypad);
+	mutex_unlock(&input->mutex);
+
+	return 0;
+}
+
+static int ma35d1_keypad_resume(struct device *dev)
+{
+	struct ma35d1_keypad *keypad = dev_get_drvdata(dev);
+	struct input_dev *input = keypad->input;
+	int ret = 0;
+
+	mutex_lock(&input->mutex);
+	if (input_device_enabled(input))
+		ret = ma35d1_keypad_enable(keypad);
+	mutex_unlock(&input->mutex);
+
+	return ret;
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(ma35d1_keypad_pm_ops,
+				ma35d1_keypad_suspend, ma35d1_keypad_resume);
+
+static int ma35d1_keypad_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct ma35d1_keypad *keypad;
+	struct input_dev *input;
+	int ret;
+
+	keypad = devm_kzalloc(dev, sizeof(*keypad), GFP_KERNEL);
+	if (!keypad)
+		return -ENOMEM;
+
+	ret = matrix_keypad_parse_properties(dev, &keypad->rows, &keypad->cols);
+	if (ret)
+		return ret;
+
+	/* The controller supports 2x1 up to 8x8 keys (KROW = 0 is reserved). */
+	if (keypad->rows < 2 || keypad->rows > MA35D1_KPI_MAX_ROWS ||
+	    !keypad->cols || keypad->cols > MA35D1_KPI_MAX_COLS)
+		return dev_err_probe(dev, -EINVAL, "invalid keypad geometry\n");
+
+	keypad->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(keypad->base))
+		return PTR_ERR(keypad->base);
+
+	keypad->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(keypad->clk))
+		return dev_err_probe(dev, PTR_ERR(keypad->clk),
+				     "failed to get KPI clock\n");
+
+	keypad->irq = platform_get_irq(pdev, 0);
+	if (keypad->irq < 0)
+		return keypad->irq;
+
+	input = devm_input_allocate_device(dev);
+	if (!input)
+		return -ENOMEM;
+
+	keypad->input = input;
+	keypad->row_shift = get_count_order(keypad->cols);
+	keypad->conf = FIELD_PREP(MA35D1_KPI_KROW, keypad->rows - 1) |
+		       FIELD_PREP(MA35D1_KPI_KCOL, keypad->cols - 1) |
+		       FIELD_PREP(MA35D1_KPI_DBCLKSEL, MA35D1_KPI_DEBOUNCE_SEL) |
+		       FIELD_PREP(MA35D1_KPI_PRESCALE, MA35D1_KPI_PRESCALE_VAL) |
+		       MA35D1_KPI_INTEN | MA35D1_KPI_RKINTEN |
+		       MA35D1_KPI_PKINTEN | MA35D1_KPI_ENKP;
+
+	input->name = "Nuvoton MA35D1 Keypad";
+	input->id.bustype = BUS_HOST;
+	input->open = ma35d1_keypad_open;
+	input->close = ma35d1_keypad_close;
+
+	ret = matrix_keypad_build_keymap(NULL, NULL, keypad->rows,
+					 keypad->cols, NULL, input);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to build keymap\n");
+
+	keypad->keycodes = input->keycode;
+	input_set_capability(input, EV_MSC, MSC_SCAN);
+	if (device_property_read_bool(dev, "autorepeat"))
+		__set_bit(EV_REP, input->evbit);
+
+	input_set_drvdata(input, keypad);
+	platform_set_drvdata(pdev, keypad);
+
+	/*
+	 * Clear whatever the bootloader left behind before the IRQ can be
+	 * requested.  The clock must run for the registers to be accessible.
+	 */
+	ret = clk_prepare_enable(keypad->clk);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to enable KPI clock\n");
+
+	writel(0, keypad->base + MA35D1_KPI_CONF);
+	ma35d1_keypad_clear_events(keypad);
+
+	clk_disable_unprepare(keypad->clk);
+
+	/* The IRQ stays disabled while the peripheral clock is gated. */
+	ret = devm_request_irq(dev, keypad->irq, ma35d1_keypad_irq,
+			       IRQF_NO_AUTOEN, dev_name(dev), keypad);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to request IRQ\n");
+
+	ret = input_register_device(input);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to register input device\n");
+
+	return 0;
+}
+
+static const struct of_device_id ma35d1_keypad_of_match[] = {
+	{ .compatible = "nuvoton,ma35d1-keypad" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ma35d1_keypad_of_match);
+
+static struct platform_driver ma35d1_keypad_driver = {
+	.probe = ma35d1_keypad_probe,
+	.driver = {
+		.name = "ma35d1-keypad",
+		.of_match_table = ma35d1_keypad_of_match,
+		.pm = pm_sleep_ptr(&ma35d1_keypad_pm_ops),
+	},
+};
+module_platform_driver(ma35d1_keypad_driver);
+
+MODULE_DESCRIPTION("Nuvoton MA35D1 hardware matrix keypad controller");
+MODULE_LICENSE("GPL");
-- 
2.43.0

Re: [PATCH v1 2/2] Input: keyboard: Add Nuvoton MA35D1 keypad driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Low] Input: ma35d1_keypad - use guard(mutex) in suspend and resume
  callbacks
- [Low] Input: ma35d1_keypad - name errno variables error and return 0
  explicitly
--

commit ca9c8a019e4d03872087b6ba2c747cf4ce6faf45
Author: Feng-Yi Hsueh <fyhsueh4linux@gmail.com>

Input: keyboard: Add Nuvoton MA35D1 keypad driver

This commit adds an input driver for the Nuvoton MA35D1 hardware matrix
keypad controller, handling key press/release interrupts, input event
reporting, and clock management across open, close, suspend, and resume.

> diff --git a/drivers/input/keyboard/ma35d1_keypad.c b/drivers/input/keyboard/ma35d1_keypad.c
> new file mode 100644
> index 0000000000000..673ad8b9704bd
> --- /dev/null
> +++ b/drivers/input/keyboard/ma35d1_keypad.c

[ ... ]

> +/* Called with input->mutex held (open, PM resume). */
> +static int ma35d1_keypad_enable(struct ma35d1_keypad *keypad)
> +{
> +	int ret;
> +
> +	if (keypad->running)
> +		return 0;
> +
> +	ret = clk_prepare_enable(keypad->clk);
> +	if (ret)
> +		return ret;

[Severity: Low]
This isn't a bug, but in the input subsystem, variables that only hold 0
or a negative error code are conventionally named error or err rather than
ret.  The same applies to ma35d1_keypad_resume() and ma35d1_keypad_probe(),
and ma35d1_keypad_resume() should also explicitly return 0 on the success
path rather than returning the error variable directly.

[ ... ]

> +static int ma35d1_keypad_suspend(struct device *dev)
> +{
> +	struct ma35d1_keypad *keypad = dev_get_drvdata(dev);
> +	struct input_dev *input = keypad->input;
> +
> +	mutex_lock(&input->mutex);
> +	if (input_device_enabled(input))
> +		ma35d1_keypad_disable(keypad);
> +	mutex_unlock(&input->mutex);
> +
> +	return 0;
> +}
> +
> +static int ma35d1_keypad_resume(struct device *dev)
> +{
> +	struct ma35d1_keypad *keypad = dev_get_drvdata(dev);
> +	struct input_dev *input = keypad->input;
> +	int ret = 0;
> +
> +	mutex_lock(&input->mutex);
> +	if (input_device_enabled(input))
> +		ret = ma35d1_keypad_enable(keypad);
> +	mutex_unlock(&input->mutex);
> +
> +	return ret;
> +}

[Severity: Low]
This isn't a bug, but new input drivers prefer guard(mutex)(&input->mutex)
over manual mutex_lock() and mutex_unlock() calls in
ma35d1_keypad_suspend() and ma35d1_keypad_resume().

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110111419.89535-1-fyhsueh4linux@gmail.com?part=2

Re: [PATCH v1 1/2] dt-bindings: input: Add Nuvoton MA35D1 keypad controller

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential
issues to consider:
- [Low] dt-bindings: input: ma35d1-keypad: redefine autorepeat instead
  of input.yaml#
- [Low] dt-bindings: input: ma35d1-keypad: omit resets property for
  MA35D1_RESET_KPI
- [Low] dt-bindings: input: ma35d1-keypad: bad clock/IRQ cells and align
  in example
--

commit 6eade3d3eb06d707c8d9f0ddfa65827ec7270074
Author: Feng-Yi Hsueh <fyhsueh4linux@gmail.com>

dt-bindings: input: Add Nuvoton MA35D1 keypad controller

This commit adds the Device Tree binding schema for the Nuvoton MA35D1
matrix keypad controller.

> diff --git a/Documentation/devicetree/bindings/input/nuvoton,ma35d1-keypad.yaml b/Documentation/devicetree/bindings/input/nuvoton,ma35d1-keypad.yaml
> new file mode 100644
> index 0000000000000..be678d0b38b98
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/nuvoton,ma35d1-keypad.yaml
> @@ -0,0 +1,73 @@
[ ... ]
> +allOf:
> +  - $ref: /schemas/input/matrix-keymap.yaml#
> +
> +properties:
> +  compatible:
> +    const: nuvoton,ma35d1-keypad
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1

[Severity: Low]
Should properties also document resets (with maxItems: 1)?

include/dt-bindings/reset/nuvoton,ma35d1-reset.h defines MA35D1_RESET_KPI for
this controller, and drivers/reset/reset-ma35d1.c implements it in
ma35d1_reset_map.  Because this schema sets unevaluatedProperties: false, any
board or SoC devicetree node specifying resets = <&sys MA35D1_RESET_KPI> will
fail dtbs_check validation.

> +
> +  keypad,num-rows:
> +    minimum: 2
> +    maximum: 8
> +
> +  keypad,num-columns:
> +    minimum: 1
> +    maximum: 8
> +
> +  autorepeat:
> +    type: boolean
> +    description: Enable key autorepeat events.

[Severity: Low]
Should this schema reference input.yaml# in allOf instead of redefining
autorepeat locally?

Documentation/devicetree/bindings/input/input.yaml already defines autorepeat
with its type and description, so referencing input.yaml# in allOf (and at
most listing autorepeat: true here) avoids duplicating the common property
definition.

> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - keypad,num-rows
> +  - keypad,num-columns
> +  - linux,keymap
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/input/input.h>
> +
> +    keypad@404a0000 {
> +        compatible = "nuvoton,ma35d1-keypad";
> +        reg = <0x404a0000 0x10000>;
> +        interrupts = <79>;
> +        clocks = <&clk>;
> +
> +        keypad,num-rows = <4>;
> +        keypad,num-columns = <1>;
> +        linux,keymap = <
> +            MATRIX_KEY(0, 0, KEY_1)
> +            MATRIX_KEY(1, 0, KEY_2)
> +            MATRIX_KEY(2, 0, KEY_3)
> +            MATRIX_KEY(3, 0, KEY_4)
> +        >;
> +    };

[Severity: Low]
Should the example include the GIC interrupt specifier and the KPI_GATE clock
cell, and align the linux,keymap entries?

On MA35D1, the interrupt controller uses three GIC cells (such as
<GIC_SPI 79 IRQ_TYPE_LEVEL_HIGH>) and &clk takes a clock specifier cell
(<&clk KPI_GATE> from dt-bindings/clock/nuvoton,ma35d1-clk.h).  Also, the
multi-line linux,keymap entries are indented by four extra spaces rather than
aligned with the opening bracket, which triggers continuation-alignment
warnings from scripts/dtc/dt-check-style --mode=strict.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110111419.89535-1-fyhsueh4linux@gmail.com?part=1

Re: [PATCH v1 1/2] dt-bindings: input: Add Nuvoton MA35D1 keypad controller

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2728 bytes --]

On Tue, Nov 10, 2026 at 07:14:18PM +0800, Feng-Yi Hsueh wrote:
> Document the Nuvoton MA35D1 hardware matrix keypad controller.
> 
> The controller supports up to eight rows and eight columns
> and generates interrupts for key press and release events.
> 
> Signed-off-by: Feng-Yi Hsueh <fyhsueh4linux@gmail.com>

Most of what the bot says seems valid to implement,
pw-bot: changes-requested

Thanks,
Conor.

> ---
>  .../bindings/input/nuvoton,ma35d1-keypad.yaml | 73 +++++++++++++++++++
>  1 file changed, 73 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/nuvoton,ma35d1-keypad.yaml
> 
> diff --git a/Documentation/devicetree/bindings/input/nuvoton,ma35d1-keypad.yaml b/Documentation/devicetree/bindings/input/nuvoton,ma35d1-keypad.yaml
> new file mode 100644
> index 000000000000..be678d0b38b9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/nuvoton,ma35d1-keypad.yaml
> @@ -0,0 +1,73 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/nuvoton,ma35d1-keypad.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Nuvoton MA35D1 Matrix Keypad Controller
> +
> +maintainers:
> +  - Feng-Yi Hsueh <fyhsueh4linux@gmail.com>
> +
> +description: |
> +  The Nuvoton MA35D1 keypad controller scans a matrix of up to eight rows
> +  and eight columns and reports key press and release interrupts.
> +
> +allOf:
> +  - $ref: /schemas/input/matrix-keymap.yaml#
> +
> +properties:
> +  compatible:
> +    const: nuvoton,ma35d1-keypad
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  keypad,num-rows:
> +    minimum: 2
> +    maximum: 8
> +
> +  keypad,num-columns:
> +    minimum: 1
> +    maximum: 8
> +
> +  autorepeat:
> +    type: boolean
> +    description: Enable key autorepeat events.
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - keypad,num-rows
> +  - keypad,num-columns
> +  - linux,keymap
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/input/input.h>
> +
> +    keypad@404a0000 {
> +        compatible = "nuvoton,ma35d1-keypad";
> +        reg = <0x404a0000 0x10000>;
> +        interrupts = <79>;
> +        clocks = <&clk>;
> +
> +        keypad,num-rows = <4>;
> +        keypad,num-columns = <1>;
> +        linux,keymap = <
> +            MATRIX_KEY(0, 0, KEY_1)
> +            MATRIX_KEY(1, 0, KEY_2)
> +            MATRIX_KEY(2, 0, KEY_3)
> +            MATRIX_KEY(3, 0, KEY_4)
> +        >;
> +    };
> -- 
> 2.43.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]