diff mbox series

[6/8] input: keyboard: Add support for MAX7360 keypad

Message ID 20241219-mdb-max7360-support-v1-6-8e8317584121@bootlin.com
State New
Headers show
Series Add support for MAX7360 multifunction device | expand

Commit Message

Mathieu Dubois-Briand Dec. 19, 2024, 4:21 p.m. UTC
Add driver for Maxim Integrated MAX7360 keypad controller, providing
support for up to 64 keys, with a matrix of 8 columns and 8 rows.

Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
---
 drivers/input/keyboard/Kconfig          |  12 ++
 drivers/input/keyboard/Makefile         |   1 +
 drivers/input/keyboard/max7360-keypad.c | 297 ++++++++++++++++++++++++++++++++
 3 files changed, 310 insertions(+)

Comments

kernel test robot Dec. 20, 2024, 5:12 p.m. UTC | #1
Hi Mathieu,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 78d4f34e2115b517bcbfe7ec0d018bbbb6f9b0b8]

url:    https://github.com/intel-lab-lkp/linux/commits/Mathieu-Dubois-Briand/dt-bindings-Add-MAX7360-MFD-device/20241220-002541
base:   78d4f34e2115b517bcbfe7ec0d018bbbb6f9b0b8
patch link:    https://lore.kernel.org/r/20241219-mdb-max7360-support-v1-6-8e8317584121%40bootlin.com
patch subject: [PATCH 6/8] input: keyboard: Add support for MAX7360 keypad
config: arm64-allmodconfig (https://download.01.org/0day-ci/archive/20241221/202412210024.DavK6KEl-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241221/202412210024.DavK6KEl-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202412210024.DavK6KEl-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/input/keyboard/max7360-keypad.c:105:24: warning: implicit conversion from 'unsigned long' to 'unsigned int' changes value from 18446744073709551487 to 4294967167 [-Wconstant-conversion]
     104 |         ret = regmap_write_bits(max7360_keypad->regmap, MAX7360_REG_CONFIG,
         |               ~~~~~~~~~~~~~~~~~
     105 |                                 MAX7360_CFG_SLEEP, ~MAX7360_CFG_SLEEP);
         |                                                    ^~~~~~~~~~~~~~~~~~
   1 warning generated.


vim +105 drivers/input/keyboard/max7360-keypad.c

    95	
    96	static void max7360_keypad_close(struct input_dev *pdev)
    97	{
    98		struct max7360_keypad *max7360_keypad = input_get_drvdata(pdev);
    99		int ret;
   100	
   101		/*
   102		 * Nobody is using the device anymore: go to sleep.
   103		 */
   104		ret = regmap_write_bits(max7360_keypad->regmap, MAX7360_REG_CONFIG,
 > 105					MAX7360_CFG_SLEEP, ~MAX7360_CFG_SLEEP);
   106		if (ret) {
   107			dev_err(&max7360_keypad->input->dev,
   108				"Failed to write max7360 configuration");
   109		}
   110	}
   111
Mathieu Dubois-Briand Dec. 23, 2024, 3:38 p.m. UTC | #2
On Thu Dec 19, 2024 at 8:35 PM CET, Dmitry Torokhov wrote:
> Hi Mathieu,
>
> On Thu, Dec 19, 2024 at 05:21:23PM +0100, Mathieu Dubois-Briand wrote:
> > +
> > +	input_set_capability(input, EV_MSC, MSC_SCAN);
> > +	if (!max7360_keypad->no_autorepeat)
> > +		__set_bit(EV_REP, input->evbit);
> > +
> > +	input_set_drvdata(input, max7360_keypad);
> > +
> > +	flags = IRQF_TRIGGER_LOW | IRQF_ONESHOT | IRQF_SHARED;
>
> Shared? Why? And why do you need a temp variable?
>

Ok, this is probably wrong.

I have a board using the MAX7360, were both interrupt lines of the
chipset (INTI, interrupt for GPIO and rotary encoder and INTK, interrupt
for the keypad) have been both wired to the same pin on the CPU side. So
on this board, the interrupt line is indeed shared between the three
corresponding drivers.

Yet this is probably very specific to my board, as I have seen no data
about MAXIM suggesting this design. So having IRQF_SHARED is probably
more a hack than a valid implementation. I will drop it in my next
series.

>
> Thanks.

Thanks a lot for you review, I am preparing a new version of this series
that should address all your comments.
diff mbox series

Patch

diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index 721ab69e84ac..bba029f65cfa 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -421,6 +421,18 @@  config KEYBOARD_MAX7359
 	  To compile this driver as a module, choose M here: the
 	  module will be called max7359_keypad.
 
+config KEYBOARD_MAX7360
+	tristate "Maxim MAX7360 Key Switch Controller"
+	select INPUT_MATRIXKMAP
+	depends on I2C
+	depends on MFD_MAX7360
+	help
+	  If you say yes here you get support for the keypad controller on the
+	  Maxim MAX7360 I/O Expander.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called max7360_keypad.
+
 config KEYBOARD_MPR121
 	tristate "Freescale MPR121 Touchkey"
 	depends on I2C
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index 1e0721c30709..b49d32d4003d 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -42,6 +42,7 @@  obj-$(CONFIG_KEYBOARD_LPC32XX)		+= lpc32xx-keys.o
 obj-$(CONFIG_KEYBOARD_MAPLE)		+= maple_keyb.o
 obj-$(CONFIG_KEYBOARD_MATRIX)		+= matrix_keypad.o
 obj-$(CONFIG_KEYBOARD_MAX7359)		+= max7359_keypad.o
+obj-$(CONFIG_KEYBOARD_MAX7360)		+= max7360-keypad.o
 obj-$(CONFIG_KEYBOARD_MPR121)		+= mpr121_touchkey.o
 obj-$(CONFIG_KEYBOARD_MT6779)		+= mt6779-keypad.o
 obj-$(CONFIG_KEYBOARD_MTK_PMIC) 	+= mtk-pmic-keys.o
diff --git a/drivers/input/keyboard/max7360-keypad.c b/drivers/input/keyboard/max7360-keypad.c
new file mode 100644
index 000000000000..fbc51c89dba1
--- /dev/null
+++ b/drivers/input/keyboard/max7360-keypad.c
@@ -0,0 +1,297 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2024 Bootlin
+ *
+ * Author: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
+ */
+
+#include <linux/init.h>
+#include <linux/input.h>
+#include <linux/input/matrix_keypad.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/max7360.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_wakeirq.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+struct max7360_keypad {
+	struct input_dev *input;
+	unsigned int rows;
+	unsigned int cols;
+	unsigned int debounce_ms;
+	int irq;
+	bool no_autorepeat;
+	struct regmap *regmap;
+	unsigned short keycodes[MAX7360_MAX_KEY_ROWS * MAX7360_MAX_KEY_COLS];
+};
+
+static irqreturn_t max7360_keypad_irq(int irq, void *data)
+{
+	struct max7360_keypad *max7360_keypad = data;
+	unsigned int val;
+	unsigned int row, col;
+	unsigned int release;
+	unsigned int code;
+	int ret;
+
+	ret = regmap_read(max7360_keypad->regmap, MAX7360_REG_KEYFIFO, &val);
+	if (!ret && val == MAX7360_FIFO_OVERFLOW) {
+		/* FIFO overflow: ignore it and get next event. */
+		dev_err(&max7360_keypad->input->dev, "max7360 FIFO overflow");
+		ret = regmap_read(max7360_keypad->regmap, MAX7360_REG_KEYFIFO,
+				  &val);
+	}
+	if (ret) {
+		dev_err(&max7360_keypad->input->dev,
+			"Failed to read max7360 FIFO");
+		return ret;
+	}
+
+	if (val == MAX7360_FIFO_EMPTY) {
+		dev_dbg(&max7360_keypad->input->dev,
+			"Got a spurious interrupt");
+
+		return IRQ_NONE;
+	}
+
+	row = FIELD_GET(MAX7360_FIFO_ROW, val);
+	col = FIELD_GET(MAX7360_FIFO_COL, val);
+	release = val & MAX7360_FIFO_RELEASE;
+
+	code = MATRIX_SCAN_CODE(row, col, MAX7360_ROW_SHIFT);
+
+	dev_dbg(&max7360_keypad->input->dev,
+		"key[%d:%d] %s\n", row, col, release ? "release" : "press");
+
+	input_event(max7360_keypad->input, EV_MSC, MSC_SCAN, code);
+	input_report_key(max7360_keypad->input, max7360_keypad->keycodes[code],
+			 !release);
+	input_sync(max7360_keypad->input);
+
+	return IRQ_HANDLED;
+}
+
+static int max7360_keypad_open(struct input_dev *pdev)
+{
+	struct max7360_keypad *max7360_keypad = input_get_drvdata(pdev);
+	int ret;
+
+	/*
+	 * Somebody is using the device: get out of sleep.
+	 */
+	ret = regmap_write_bits(max7360_keypad->regmap, MAX7360_REG_CONFIG,
+				MAX7360_CFG_SLEEP, MAX7360_CFG_SLEEP);
+	if (ret) {
+		dev_err(&max7360_keypad->input->dev,
+			"Failed to write max7360 configuration");
+		return ret;
+	}
+
+	return 0;
+}
+
+static void max7360_keypad_close(struct input_dev *pdev)
+{
+	struct max7360_keypad *max7360_keypad = input_get_drvdata(pdev);
+	int ret;
+
+	/*
+	 * Nobody is using the device anymore: go to sleep.
+	 */
+	ret = regmap_write_bits(max7360_keypad->regmap, MAX7360_REG_CONFIG,
+				MAX7360_CFG_SLEEP, ~MAX7360_CFG_SLEEP);
+	if (ret) {
+		dev_err(&max7360_keypad->input->dev,
+			"Failed to write max7360 configuration");
+	}
+}
+
+static int max7360_keypad_hw_init(struct max7360_keypad *max7360_keypad)
+{
+	unsigned int val;
+	int ret;
+
+	val = max7360_keypad->debounce_ms - MAX7360_DEBOUNCE_MIN;
+	ret = regmap_write_bits(max7360_keypad->regmap, MAX7360_REG_DEBOUNCE,
+				MAX7360_DEBOUNCE,
+				FIELD_PREP(MAX7360_DEBOUNCE, val));
+	if (ret) {
+		dev_err(&max7360_keypad->input->dev,
+			"Failed to write max7360 debounce configuration");
+		return ret;
+	}
+
+	ret = regmap_write_bits(max7360_keypad->regmap, MAX7360_REG_INTERRUPT,
+				MAX7360_INTERRUPT_TIME_MASK,
+				FIELD_PREP(MAX7360_INTERRUPT_TIME_MASK, 1));
+	if (ret) {
+		dev_err(&max7360_keypad->input->dev,
+			"Failed to write max7360 keypad interrupt configuration");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int max7360_keypad_parse_dt(struct platform_device *pdev,
+				   struct max7360_keypad *max7360_keypad)
+{
+	bool no_autorepeat;
+	int ret;
+
+	ret = matrix_keypad_parse_properties(&pdev->dev, &max7360_keypad->rows,
+					     &max7360_keypad->cols);
+	if (ret)
+		return ret;
+
+	if (!max7360_keypad->rows || !max7360_keypad->cols ||
+	    max7360_keypad->rows > MAX7360_MAX_KEY_ROWS ||
+	    max7360_keypad->cols > MAX7360_MAX_KEY_COLS) {
+		dev_err(&pdev->dev,
+			"Invalid number of columns or rows (%ux%u)\n",
+			max7360_keypad->cols, max7360_keypad->rows);
+		return -EINVAL;
+	}
+
+	max7360_keypad->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+	if (!max7360_keypad->regmap) {
+		dev_err(&pdev->dev, "Could not get parent regmap\n");
+		return -ENODEV;
+	}
+
+	no_autorepeat = of_property_read_bool(pdev->dev.of_node,
+					      "linux,input-no-autorepeat");
+	max7360_keypad->no_autorepeat = no_autorepeat;
+
+	max7360_keypad->debounce_ms = MAX7360_DEBOUNCE_MIN;
+	ret = of_property_read_u32(pdev->dev.of_node, "debounce-delay-ms",
+				   &max7360_keypad->debounce_ms);
+	if (ret == -EINVAL) {
+		dev_info(&pdev->dev, "Using default debounce-delay-ms: %u\n",
+			 max7360_keypad->debounce_ms);
+	} else if (ret < 0) {
+		dev_err(&pdev->dev, "Failed to read debounce-delay-ms property\n");
+		return ret;
+	} else if (max7360_keypad->debounce_ms < MAX7360_DEBOUNCE_MIN ||
+		   max7360_keypad->debounce_ms > MAX7360_DEBOUNCE_MAX) {
+		dev_err(&pdev->dev,
+			"Invalid debounce-delay-ms: %u, should be between %u and %u.\n",
+			max7360_keypad->debounce_ms, MAX7360_DEBOUNCE_MIN,
+			MAX7360_DEBOUNCE_MAX);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int max7360_keypad_probe(struct platform_device *pdev)
+{
+	struct max7360_keypad *max7360_keypad;
+	struct input_dev *input;
+	unsigned long flags;
+	int irq;
+	int ret;
+
+	if (!pdev->dev.parent)
+		return dev_err_probe(&pdev->dev, -ENODEV, "No parent device\n");
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0)
+		return irq;
+
+	max7360_keypad = devm_kzalloc(&pdev->dev, sizeof(*max7360_keypad),
+				      GFP_KERNEL);
+	if (!max7360_keypad)
+		return -ENOMEM;
+
+	ret = max7360_keypad_parse_dt(pdev, max7360_keypad);
+	if (ret)
+		return ret;
+
+	input = devm_input_allocate_device(&pdev->dev);
+	if (!input)
+		return dev_err_probe(&pdev->dev, -ENOMEM,
+				     "Failed to allocate input device\n");
+
+	max7360_keypad->input = input;
+
+	input->id.bustype = BUS_I2C;
+	input->name = pdev->name;
+	input->dev.parent = &pdev->dev;
+
+	input->open = max7360_keypad_open;
+	input->close = max7360_keypad_close;
+
+	ret = matrix_keypad_build_keymap(NULL, NULL,
+					 MAX7360_MAX_KEY_ROWS,
+					 MAX7360_MAX_KEY_COLS,
+					 max7360_keypad->keycodes, input);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret,
+				     "Failed to build keymap\n");
+
+	input_set_capability(input, EV_MSC, MSC_SCAN);
+	if (!max7360_keypad->no_autorepeat)
+		__set_bit(EV_REP, input->evbit);
+
+	input_set_drvdata(input, max7360_keypad);
+
+	flags = IRQF_TRIGGER_LOW | IRQF_ONESHOT | IRQF_SHARED;
+	ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
+					max7360_keypad_irq, flags,
+					"max7360-keypad", max7360_keypad);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret,
+				     "Failed to register interrupt: %d\n", ret);
+
+	ret = input_register_device(input);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret,
+				     "Could not register input device: %d\n",
+				     ret);
+
+	platform_set_drvdata(pdev, max7360_keypad);
+
+	device_init_wakeup(&pdev->dev, true);
+	ret = dev_pm_set_wake_irq(&pdev->dev, irq);
+	if (ret)
+		dev_warn(&pdev->dev, "Failed to set up wakeup irq: %d\n", ret);
+
+	ret = max7360_keypad_hw_init(max7360_keypad);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret,
+				     "Failed to initialize max7360 keypad\n");
+
+	return 0;
+}
+
+static void max7360_keypad_remove(struct platform_device *pdev)
+{
+	dev_pm_clear_wake_irq(&pdev->dev);
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id max7360_keypad_of_match[] = {
+	{ .compatible = "maxim,max7360-keypad", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, max7360_keypad_of_match);
+#endif
+
+static struct platform_driver max7360_keypad_driver = {
+	.driver = {
+		.name	= "max7360-keypad",
+		.of_match_table = of_match_ptr(max7360_keypad_of_match),
+	},
+	.probe		= max7360_keypad_probe,
+	.remove		= max7360_keypad_remove,
+};
+module_platform_driver(max7360_keypad_driver);
+
+MODULE_DESCRIPTION("MAX7360 Keypad driver");
+MODULE_AUTHOR("Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>");
+MODULE_ALIAS("platform:max7360-keypad");
+MODULE_LICENSE("GPL");