[PATCH] Input: keypad-nomadik-ske - remove the driver

[PATCH] Input: keypad-nomadik-ske - remove the driver

[Dmitry Torokhov]

The users of this driver were removed in 2013 in commit 28633c54bda6
("ARM: ux500: Rip out keypad initialisation which is no longer used").

Remove the driver as well.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/keyboard/Kconfig                |  11 -
 drivers/input/keyboard/Makefile               |   1 -
 drivers/input/keyboard/nomadik-ske-keypad.c   | 378 ------------------
 .../linux/platform_data/keypad-nomadik-ske.h  |  50 ---
 4 files changed, 440 deletions(-)

diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index 72f9552cb571..a5015d6f8bed 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -473,17 +473,6 @@ config KEYBOARD_NEWTON
 	  To compile this driver as a module, choose M here: the
 	  module will be called newtonkbd.
 
-config KEYBOARD_NOMADIK
-	tristate "ST-Ericsson Nomadik SKE keyboard"
-	depends on (ARCH_NOMADIK || ARCH_U8500 || COMPILE_TEST)
-	select INPUT_MATRIXKMAP
-	help
-	  Say Y here if you want to use a keypad provided on the SKE controller
-	  used on the Ux500 and Nomadik platforms
-
-	  To compile this driver as a module, choose M here: the
-	  module will be called nmk-ske-keypad.
-
 config KEYBOARD_NSPIRE
 	tristate "TI-NSPIRE built-in keyboard"
 	depends on ARCH_NSPIRE && OF
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index b8d12a0524e0..3631a9294c6f 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -45,7 +45,6 @@ obj-$(CONFIG_KEYBOARD_MPR121)		+= mpr121_touchkey.o
 obj-$(CONFIG_KEYBOARD_MT6779)		+= mt6779-keypad.o
 obj-$(CONFIG_KEYBOARD_MTK_PMIC) 	+= mtk-pmic-keys.o
 obj-$(CONFIG_KEYBOARD_NEWTON)		+= newtonkbd.o
-obj-$(CONFIG_KEYBOARD_NOMADIK)		+= nomadik-ske-keypad.o
 obj-$(CONFIG_KEYBOARD_NSPIRE)		+= nspire-keypad.o
 obj-$(CONFIG_KEYBOARD_OMAP)		+= omap-keypad.o
 obj-$(CONFIG_KEYBOARD_OMAP4)		+= omap4-keypad.o
diff --git a/drivers/input/keyboard/nomadik-ske-keypad.c b/drivers/input/keyboard/nomadik-ske-keypad.c
deleted file mode 100644
index b3ccc97f61e1..000000000000
--- a/drivers/input/keyboard/nomadik-ske-keypad.c
+++ /dev/null
@@ -1,378 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/*
- * Copyright (C) ST-Ericsson SA 2010
- *
- * Author: Naveen Kumar G <naveen.gaddipati@stericsson.com> for ST-Ericsson
- * Author: Sundar Iyer <sundar.iyer@stericsson.com> for ST-Ericsson
- *
- * Keypad controller driver for the SKE (Scroll Key Encoder) module used in
- * the Nomadik 8815 and Ux500 platforms.
- */
-
-#include <linux/platform_device.h>
-#include <linux/interrupt.h>
-#include <linux/spinlock.h>
-#include <linux/io.h>
-#include <linux/delay.h>
-#include <linux/input.h>
-#include <linux/slab.h>
-#include <linux/clk.h>
-#include <linux/module.h>
-
-#include <linux/platform_data/keypad-nomadik-ske.h>
-
-/* SKE_CR bits */
-#define SKE_KPMLT	(0x1 << 6)
-#define SKE_KPCN	(0x7 << 3)
-#define SKE_KPASEN	(0x1 << 2)
-#define SKE_KPASON	(0x1 << 7)
-
-/* SKE_IMSC bits */
-#define SKE_KPIMA	(0x1 << 2)
-
-/* SKE_ICR bits */
-#define SKE_KPICS	(0x1 << 3)
-#define SKE_KPICA	(0x1 << 2)
-
-/* SKE_RIS bits */
-#define SKE_KPRISA	(0x1 << 2)
-
-#define SKE_KEYPAD_ROW_SHIFT	3
-#define SKE_KPD_NUM_ROWS	8
-#define SKE_KPD_NUM_COLS	8
-
-/* keypad auto scan registers */
-#define SKE_ASR0	0x20
-#define SKE_ASR1	0x24
-#define SKE_ASR2	0x28
-#define SKE_ASR3	0x2C
-
-#define SKE_NUM_ASRX_REGISTERS	(4)
-#define	KEY_PRESSED_DELAY	10
-
-/**
- * struct ske_keypad  - data structure used by keypad driver
- * @irq:	irq no
- * @reg_base:	ske registers base address
- * @input:	pointer to input device object
- * @board:	keypad platform device
- * @keymap:	matrix scan code table for keycodes
- * @clk:	clock structure pointer
- * @pclk:	clock structure pointer
- * @ske_keypad_lock: spinlock protecting the keypad read/writes
- */
-struct ske_keypad {
-	int irq;
-	void __iomem *reg_base;
-	struct input_dev *input;
-	const struct ske_keypad_platform_data *board;
-	unsigned short keymap[SKE_KPD_NUM_ROWS * SKE_KPD_NUM_COLS];
-	struct clk *clk;
-	struct clk *pclk;
-	spinlock_t ske_keypad_lock;
-};
-
-static void ske_keypad_set_bits(struct ske_keypad *keypad, u16 addr,
-		u8 mask, u8 data)
-{
-	u32 ret;
-
-	spin_lock(&keypad->ske_keypad_lock);
-
-	ret = readl(keypad->reg_base + addr);
-	ret &= ~mask;
-	ret |= data;
-	writel(ret, keypad->reg_base + addr);
-
-	spin_unlock(&keypad->ske_keypad_lock);
-}
-
-/*
- * ske_keypad_chip_init: init keypad controller configuration
- *
- * Enable Multi key press detection, auto scan mode
- */
-static int __init ske_keypad_chip_init(struct ske_keypad *keypad)
-{
-	u32 value;
-	int timeout = keypad->board->debounce_ms;
-
-	/* check SKE_RIS to be 0 */
-	while ((readl(keypad->reg_base + SKE_RIS) != 0x00000000) && timeout--)
-		cpu_relax();
-
-	if (timeout == -1)
-		return -EINVAL;
-
-	/*
-	 * set debounce value
-	 * keypad dbounce is configured in DBCR[15:8]
-	 * dbounce value in steps of 32/32.768 ms
-	 */
-	spin_lock(&keypad->ske_keypad_lock);
-	value = readl(keypad->reg_base + SKE_DBCR);
-	value = value & 0xff;
-	value |= ((keypad->board->debounce_ms * 32000)/32768) << 8;
-	writel(value, keypad->reg_base + SKE_DBCR);
-	spin_unlock(&keypad->ske_keypad_lock);
-
-	/* enable multi key detection */
-	ske_keypad_set_bits(keypad, SKE_CR, 0x0, SKE_KPMLT);
-
-	/*
-	 * set up the number of columns
-	 * KPCN[5:3] defines no. of keypad columns to be auto scanned
-	 */
-	value = (keypad->board->kcol - 1) << 3;
-	ske_keypad_set_bits(keypad, SKE_CR, SKE_KPCN, value);
-
-	/* clear keypad interrupt for auto(and pending SW) scans */
-	ske_keypad_set_bits(keypad, SKE_ICR, 0x0, SKE_KPICA | SKE_KPICS);
-
-	/* un-mask keypad interrupts */
-	ske_keypad_set_bits(keypad, SKE_IMSC, 0x0, SKE_KPIMA);
-
-	/* enable automatic scan */
-	ske_keypad_set_bits(keypad, SKE_CR, 0x0, SKE_KPASEN);
-
-	return 0;
-}
-
-static void ske_keypad_report(struct ske_keypad *keypad, u8 status, int col)
-{
-	int row = 0, code, pos;
-	struct input_dev *input = keypad->input;
-	u32 ske_ris;
-	int key_pressed;
-	int num_of_rows;
-
-	/* find out the row */
-	num_of_rows = hweight8(status);
-	do {
-		pos = __ffs(status);
-		row = pos;
-		status &= ~(1 << pos);
-
-		code = MATRIX_SCAN_CODE(row, col, SKE_KEYPAD_ROW_SHIFT);
-		ske_ris = readl(keypad->reg_base + SKE_RIS);
-		key_pressed = ske_ris & SKE_KPRISA;
-
-		input_event(input, EV_MSC, MSC_SCAN, code);
-		input_report_key(input, keypad->keymap[code], key_pressed);
-		input_sync(input);
-		num_of_rows--;
-	} while (num_of_rows);
-}
-
-static void ske_keypad_read_data(struct ske_keypad *keypad)
-{
-	u8 status;
-	int col = 0;
-	int ske_asr, i;
-
-	/*
-	 * Read the auto scan registers
-	 *
-	 * Each SKE_ASRx (x=0 to x=3) contains two row values.
-	 * lower byte contains row value for column 2*x,
-	 * upper byte contains row value for column 2*x + 1
-	 */
-	for (i = 0; i < SKE_NUM_ASRX_REGISTERS; i++) {
-		ske_asr = readl(keypad->reg_base + SKE_ASR0 + (4 * i));
-		if (!ske_asr)
-			continue;
-
-		/* now that ASRx is zero, find out the coloumn x and row y */
-		status = ske_asr & 0xff;
-		if (status) {
-			col = i * 2;
-			ske_keypad_report(keypad, status, col);
-		}
-		status = (ske_asr & 0xff00) >> 8;
-		if (status) {
-			col = (i * 2) + 1;
-			ske_keypad_report(keypad, status, col);
-		}
-	}
-}
-
-static irqreturn_t ske_keypad_irq(int irq, void *dev_id)
-{
-	struct ske_keypad *keypad = dev_id;
-	int timeout = keypad->board->debounce_ms;
-
-	/* disable auto scan interrupt; mask the interrupt generated */
-	ske_keypad_set_bits(keypad, SKE_IMSC, ~SKE_KPIMA, 0x0);
-	ske_keypad_set_bits(keypad, SKE_ICR, 0x0, SKE_KPICA);
-
-	while ((readl(keypad->reg_base + SKE_CR) & SKE_KPASON) && --timeout)
-		cpu_relax();
-
-	/* SKEx registers are stable and can be read */
-	ske_keypad_read_data(keypad);
-
-	/* wait until raw interrupt is clear */
-	while ((readl(keypad->reg_base + SKE_RIS)) && --timeout)
-		msleep(KEY_PRESSED_DELAY);
-
-	/* enable auto scan interrupts */
-	ske_keypad_set_bits(keypad, SKE_IMSC, 0x0, SKE_KPIMA);
-
-	return IRQ_HANDLED;
-}
-
-static void ske_keypad_board_exit(void *data)
-{
-	struct ske_keypad *keypad = data;
-
-	keypad->board->exit();
-}
-
-static int __init ske_keypad_probe(struct platform_device *pdev)
-{
-	const struct ske_keypad_platform_data *plat =
-			dev_get_platdata(&pdev->dev);
-	struct device *dev = &pdev->dev;
-	struct ske_keypad *keypad;
-	struct input_dev *input;
-	int irq;
-	int error;
-
-	if (!plat) {
-		dev_err(&pdev->dev, "invalid keypad platform data\n");
-		return -EINVAL;
-	}
-
-	irq = platform_get_irq(pdev, 0);
-	if (irq < 0)
-		return irq;
-
-	keypad = devm_kzalloc(dev, sizeof(struct ske_keypad),
-			      GFP_KERNEL);
-	input = devm_input_allocate_device(dev);
-	if (!keypad || !input) {
-		dev_err(&pdev->dev, "failed to allocate keypad memory\n");
-		return -ENOMEM;
-	}
-
-	keypad->irq = irq;
-	keypad->board = plat;
-	keypad->input = input;
-	spin_lock_init(&keypad->ske_keypad_lock);
-
-	keypad->reg_base = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(keypad->reg_base))
-		return PTR_ERR(keypad->reg_base);
-
-	keypad->pclk = devm_clk_get_enabled(dev, "apb_pclk");
-	if (IS_ERR(keypad->pclk)) {
-		dev_err(&pdev->dev, "failed to get pclk\n");
-		return PTR_ERR(keypad->pclk);
-	}
-
-	keypad->clk = devm_clk_get_enabled(dev, NULL);
-	if (IS_ERR(keypad->clk)) {
-		dev_err(&pdev->dev, "failed to get clk\n");
-		return PTR_ERR(keypad->clk);
-	}
-
-	input->id.bustype = BUS_HOST;
-	input->name = "ux500-ske-keypad";
-	input->dev.parent = &pdev->dev;
-
-	error = matrix_keypad_build_keymap(plat->keymap_data, NULL,
-					   SKE_KPD_NUM_ROWS, SKE_KPD_NUM_COLS,
-					   keypad->keymap, input);
-	if (error) {
-		dev_err(&pdev->dev, "Failed to build keymap\n");
-		return error;
-	}
-
-	input_set_capability(input, EV_MSC, MSC_SCAN);
-	if (!plat->no_autorepeat)
-		__set_bit(EV_REP, input->evbit);
-
-	/* go through board initialization helpers */
-	if (keypad->board->init)
-		keypad->board->init();
-
-	if (keypad->board->exit) {
-		error = devm_add_action_or_reset(dev, ske_keypad_board_exit,
-						 keypad);
-		if (error)
-			return error;
-	}
-
-	error = ske_keypad_chip_init(keypad);
-	if (error) {
-		dev_err(&pdev->dev, "unable to init keypad hardware\n");
-		return error;
-	}
-
-	error = devm_request_threaded_irq(dev, keypad->irq,
-					  NULL, ske_keypad_irq,
-					  IRQF_ONESHOT, "ske-keypad", keypad);
-	if (error) {
-		dev_err(&pdev->dev, "allocate irq %d failed\n", keypad->irq);
-		return error;
-	}
-
-	error = input_register_device(input);
-	if (error) {
-		dev_err(&pdev->dev,
-			"unable to register input device: %d\n", error);
-		return error;
-	}
-
-	if (plat->wakeup_enable)
-		device_init_wakeup(&pdev->dev, true);
-
-	platform_set_drvdata(pdev, keypad);
-
-	return 0;
-}
-
-static int ske_keypad_suspend(struct device *dev)
-{
-	struct platform_device *pdev = to_platform_device(dev);
-	struct ske_keypad *keypad = platform_get_drvdata(pdev);
-	int irq = platform_get_irq(pdev, 0);
-
-	if (device_may_wakeup(dev))
-		enable_irq_wake(irq);
-	else
-		ske_keypad_set_bits(keypad, SKE_IMSC, ~SKE_KPIMA, 0x0);
-
-	return 0;
-}
-
-static int ske_keypad_resume(struct device *dev)
-{
-	struct platform_device *pdev = to_platform_device(dev);
-	struct ske_keypad *keypad = platform_get_drvdata(pdev);
-	int irq = platform_get_irq(pdev, 0);
-
-	if (device_may_wakeup(dev))
-		disable_irq_wake(irq);
-	else
-		ske_keypad_set_bits(keypad, SKE_IMSC, 0x0, SKE_KPIMA);
-
-	return 0;
-}
-
-static DEFINE_SIMPLE_DEV_PM_OPS(ske_keypad_dev_pm_ops,
-				ske_keypad_suspend, ske_keypad_resume);
-
-static struct platform_driver ske_keypad_driver = {
-	.driver = {
-		.name = "nmk-ske-keypad",
-		.pm = pm_sleep_ptr(&ske_keypad_dev_pm_ops),
-	},
-};
-
-module_platform_driver_probe(ske_keypad_driver, ske_keypad_probe);
-
-MODULE_LICENSE("GPL v2");
-MODULE_AUTHOR("Naveen Kumar <naveen.gaddipati@stericsson.com> / Sundar Iyer <sundar.iyer@stericsson.com>");
-MODULE_DESCRIPTION("Nomadik Scroll-Key-Encoder Keypad Driver");
-MODULE_ALIAS("platform:nomadik-ske-keypad");
diff --git a/include/linux/platform_data/keypad-nomadik-ske.h b/include/linux/platform_data/keypad-nomadik-ske.h
deleted file mode 100644
index 7efabbca1dca..000000000000
--- a/include/linux/platform_data/keypad-nomadik-ske.h
+++ /dev/null
@@ -1,50 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-/*
- * Copyright (C) ST-Ericsson SA 2010
- *
- * Author: Naveen Kumar Gaddipati <naveen.gaddipati@stericsson.com>
- *
- * ux500 Scroll key and Keypad Encoder (SKE) header
- */
-
-#ifndef __SKE_H
-#define __SKE_H
-
-#include <linux/input/matrix_keypad.h>
-
-/* register definitions for SKE peripheral */
-#define SKE_CR		0x00
-#define SKE_VAL0	0x04
-#define SKE_VAL1	0x08
-#define SKE_DBCR	0x0C
-#define SKE_IMSC	0x10
-#define SKE_RIS		0x14
-#define SKE_MIS		0x18
-#define SKE_ICR		0x1C
-
-/*
- * Keypad module
- */
-
-/**
- * struct keypad_platform_data - structure for platform specific data
- * @init:	pointer to keypad init function
- * @exit:	pointer to keypad deinitialisation function
- * @keymap_data: matrix scan code table for keycodes
- * @krow:	maximum number of rows
- * @kcol:	maximum number of columns
- * @debounce_ms: platform specific debounce time
- * @no_autorepeat: flag for auto repetition
- * @wakeup_enable: allow waking up the system
- */
-struct ske_keypad_platform_data {
-	int (*init)(void);
-	int (*exit)(void);
-	const struct matrix_keymap_data *keymap_data;
-	u8 krow;
-	u8 kcol;
-	u8 debounce_ms;
-	bool no_autorepeat;
-	bool wakeup_enable;
-};
-#endif	/*__SKE_KPD_H*/
-- 
2.46.0.184.g6999bdac58-goog


-- 
Dmitry

Re: [PATCH] Input: keypad-nomadik-ske - remove the driver

[Arnd Bergmann]

On Fri, Aug 16, 2024, at 20:54, Dmitry Torokhov wrote:
> The users of this driver were removed in 2013 in commit 28633c54bda6
> ("ARM: ux500: Rip out keypad initialisation which is no longer used").
>
> Remove the driver as well.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>  drivers/input/keyboard/Kconfig                |  11 -
>  drivers/input/keyboard/Makefile               |   1 -
>  drivers/input/keyboard/nomadik-ske-keypad.c   | 378 ------------------
>  .../linux/platform_data/keypad-nomadik-ske.h  |  50 ---
>  4 files changed, 440 deletions(-)
>

Acked-by: Arnd Bergmann <arnd@arndb.de>

I have a list of drivers that I determined to be likely
unused as well and found a few more input drivers
that were unused in 2022:

CONFIG_KEYBOARD_ADP5520/CONFIG_PMIC_ADP5520
CONFIG_KEYBOARD_ADP5589
CONFIG_INPUT_AD714X
CONFIG_TOUCHSCREEN_AD7877

As far as I can tell, these all lost their last device
definition, or they never had one and are impossible to
be used with device tree data.

     Arnd

Re: [PATCH] Input: keypad-nomadik-ske - remove the driver

[Dmitry Torokhov]

On Mon, Aug 19, 2024 at 11:29:32AM +0200, Arnd Bergmann wrote:
> On Fri, Aug 16, 2024, at 20:54, Dmitry Torokhov wrote:
> > The users of this driver were removed in 2013 in commit 28633c54bda6
> > ("ARM: ux500: Rip out keypad initialisation which is no longer used").
> >
> > Remove the driver as well.
> >
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> >  drivers/input/keyboard/Kconfig                |  11 -
> >  drivers/input/keyboard/Makefile               |   1 -
> >  drivers/input/keyboard/nomadik-ske-keypad.c   | 378 ------------------
> >  .../linux/platform_data/keypad-nomadik-ske.h  |  50 ---
> >  4 files changed, 440 deletions(-)
> >
> 
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> 
> I have a list of drivers that I determined to be likely
> unused as well and found a few more input drivers
> that were unused in 2022:
> 
> CONFIG_KEYBOARD_ADP5520/CONFIG_PMIC_ADP5520
> CONFIG_KEYBOARD_ADP5589
> CONFIG_INPUT_AD714X
> CONFIG_TOUCHSCREEN_AD7877
> 
> As far as I can tell, these all lost their last device
> definition, or they never had one and are impossible to
> be used with device tree data.

I asked Analog Devices folks (CCed) about 5589 and Nuno said that it is
still relevant and promised to do conversion to DT similar to adp5588.

Nuno, Michale, what about the other drivers that Arnd listed?

Thanks.

-- 
Dmitry

Re: [PATCH] Input: keypad-nomadik-ske - remove the driver

[Linus Walleij]

On Fri, Aug 16, 2024 at 8:54 PM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:

> The users of this driver were removed in 2013 in commit 28633c54bda6
> ("ARM: ux500: Rip out keypad initialisation which is no longer used").
>
> Remove the driver as well.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

This was intended for keypad phones based on ux500 and then
the smartphone/touchscreen revolution happened and it was
never used, it's there, but it's always dark silicon.

Yours,
Linus Walleij

RE: [PATCH] Input: keypad-nomadik-ske - remove the driver

[Hennerich, Michael]

> -----Original Message-----
> From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Sent: Monday, August 19, 2024 4:55 PM
> To: Arnd Bergmann <arnd@arndb.de>; Nuno Sá <noname.nuno@gmail.com>;
> Hennerich, Michael <Michael.Hennerich@analog.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>; linux-input@vger.kernel.org; linux-
> kernel@vger.kernel.org; Lee Jones <lee@kernel.org>; linux-arm-
> kernel@lists.infradead.org; Agarwal, Utsav <Utsav.Agarwal@analog.com>
> Subject: Re: [PATCH] Input: keypad-nomadik-ske - remove the driver
> 
> On Mon, Aug 19, 2024 at 11:29:32AM +0200, Arnd Bergmann wrote:
> > On Fri, Aug 16, 2024, at 20:54, Dmitry Torokhov wrote:
> > > The users of this driver were removed in 2013 in commit 28633c54bda6
> > > ("ARM: ux500: Rip out keypad initialisation which is no longer used").
> > >
> > > Remove the driver as well.
> > >
> > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > ---
> > >  drivers/input/keyboard/Kconfig                |  11 -
> > >  drivers/input/keyboard/Makefile               |   1 -
> > >  drivers/input/keyboard/nomadik-ske-keypad.c   | 378 ------------------
> > >  .../linux/platform_data/keypad-nomadik-ske.h  |  50 ---
> > >  4 files changed, 440 deletions(-)
> > >
> >
> > Acked-by: Arnd Bergmann <arnd@arndb.de>
> >
> > I have a list of drivers that I determined to be likely unused as well
> > and found a few more input drivers that were unused in 2022:
> >
> > CONFIG_KEYBOARD_ADP5520/CONFIG_PMIC_ADP5520
> > CONFIG_KEYBOARD_ADP5589
> > CONFIG_INPUT_AD714X
> > CONFIG_TOUCHSCREEN_AD7877
> >
> > As far as I can tell, these all lost their last device definition, or
> > they never had one and are impossible to be used with device tree
> > data.
> 
> I asked Analog Devices folks (CCed) about 5589 and Nuno said that it is still
> relevant and promised to do conversion to DT similar to adp5588.
> 
> Nuno, Michale, what about the other drivers that Arnd listed?

We'll provide dt conversion patches for AD7877 and AD714x as well.

Regards,
Michael

> 
> Thanks.
> 
> --
> Dmitry

Re: [PATCH] Input: keypad-nomadik-ske - remove the driver

[Nuno Sá]

On Mon, 2024-08-19 at 07:54 -0700, Dmitry Torokhov wrote:
> On Mon, Aug 19, 2024 at 11:29:32AM +0200, Arnd Bergmann wrote:
> > On Fri, Aug 16, 2024, at 20:54, Dmitry Torokhov wrote:
> > > The users of this driver were removed in 2013 in commit 28633c54bda6
> > > ("ARM: ux500: Rip out keypad initialisation which is no longer used").
> > > 
> > > Remove the driver as well.
> > > 
> > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > ---
> > >  drivers/input/keyboard/Kconfig                |  11 -
> > >  drivers/input/keyboard/Makefile               |   1 -
> > >  drivers/input/keyboard/nomadik-ske-keypad.c   | 378 ------------------
> > >  .../linux/platform_data/keypad-nomadik-ske.h  |  50 ---
> > >  4 files changed, 440 deletions(-)
> > > 
> > 
> > Acked-by: Arnd Bergmann <arnd@arndb.de>
> > 
> > I have a list of drivers that I determined to be likely
> > unused as well and found a few more input drivers
> > that were unused in 2022:
> > 
> > CONFIG_KEYBOARD_ADP5520/CONFIG_PMIC_ADP5520
> > CONFIG_KEYBOARD_ADP5589
> > CONFIG_INPUT_AD714X
> > CONFIG_TOUCHSCREEN_AD7877
> > 
> > As far as I can tell, these all lost their last device
> > definition, or they never had one and are impossible to
> > be used with device tree data.
> 
> I asked Analog Devices folks (CCed) about 5589 and Nuno said that it is
> still relevant and promised to do conversion to DT similar to adp5588.
> 
> Nuno, Michale, what about the other drivers that Arnd listed?
> 

Hi Dmitry,

This is not forgotten and I plan to start working on this early next week.

One thing I noticed and I might as well just ask before starting the work, is that
the platform data allows, in theory, for you to have holes in your keymap [1]. Like
enabling row 1 and 3 skipping 2. AFAICT, the matrix stuff does not allow this out of
the box as we just define the number of rows/cols and then without any other property
we assume (I think) that the map is contiguous. 

This is just early thinking but one way to support the current behavior would be 2
custom DT properties that would be 2 u32 arrays specifying the enabled columns and
rows. Out of it, we could build row and col masks and get the total number of cols
and rows that we could pass to matrix_keypad_build_keymap().

The question is... is it worth it? I'm aware that if we just assume a contiguous
keymap we could break some old users. But I guess it would be only out of tree ones
as we don't have any in kernel user of the platform data. On top of it, I guess it's
sane to assume that one just wants a contiguous keymap...

[1]: https://elixir.bootlin.com/linux/v6.10.8/source/drivers/input/keyboard/adp5589-keys.c#L630

Thanks!
- Nuno Sá

Re: [PATCH] Input: keypad-nomadik-ske - remove the driver

[Dmitry Torokhov]

Hi Nuno,

On Fri, Sep 06, 2024 at 10:38:35AM +0200, Nuno Sá wrote:
> 
> Hi Dmitry,
> 
> This is not forgotten and I plan to start working on this early next week.
> 
> One thing I noticed and I might as well just ask before starting the work, is that
> the platform data allows, in theory, for you to have holes in your keymap [1]. Like
> enabling row 1 and 3 skipping 2. AFAICT, the matrix stuff does not allow this out of
> the box as we just define the number of rows/cols and then without any other property
> we assume (I think) that the map is contiguous. 
> 
> This is just early thinking but one way to support the current behavior would be 2
> custom DT properties that would be 2 u32 arrays specifying the enabled columns and
> rows. Out of it, we could build row and col masks and get the total number of cols
> and rows that we could pass to matrix_keypad_build_keymap().

I'd ask DT maintainers but in my opinion we could add 2 u32 scalar
properties to specify row and col masks (either enabled or disabled,
whatever is more convenient) and then indeed we could figure out the
resulting size of key matrix and use matrix_keypad_build_keymap() to
load it.

> 
> The question is... is it worth it? I'm aware that if we just assume a contiguous
> keymap we could break some old users. But I guess it would be only out of tree ones
> as we don't have any in kernel user of the platform data. On top of it, I guess it's
> sane to assume that one just wants a contiguous keymap...
> 
> [1]: https://elixir.bootlin.com/linux/v6.10.8/source/drivers/input/keyboard/adp5589-keys.c#L630

I think in practice it's just a few extra lines of code, so shoudl be
fairly easy to keep supporting this.

But we can actually split the binding and the driver implementation,
with binding defining all capabilities of the hardware and driver
implementing just a subset of it (i.e. complain if row and column mask
properties are specified and abort probe).

Thanks.

-- 
Dmitry

Re: [PATCH] Input: keypad-nomadik-ske - remove the driver

[Nuno Sá]

On Fri, 2024-09-06 at 09:51 -0700, Dmitry Torokhov wrote:
> Hi Nuno,
> 
> On Fri, Sep 06, 2024 at 10:38:35AM +0200, Nuno Sá wrote:
> > 
> > Hi Dmitry,
> > 
> > This is not forgotten and I plan to start working on this early next week.
> > 
> > One thing I noticed and I might as well just ask before starting the work,
> > is that
> > the platform data allows, in theory, for you to have holes in your keymap
> > [1]. Like
> > enabling row 1 and 3 skipping 2. AFAICT, the matrix stuff does not allow
> > this out of
> > the box as we just define the number of rows/cols and then without any other
> > property
> > we assume (I think) that the map is contiguous. 
> > 
> > This is just early thinking but one way to support the current behavior
> > would be 2
> > custom DT properties that would be 2 u32 arrays specifying the enabled
> > columns and
> > rows. Out of it, we could build row and col masks and get the total number
> > of cols
> > and rows that we could pass to matrix_keypad_build_keymap().
> 
> I'd ask DT maintainers but in my opinion we could add 2 u32 scalar
> properties to specify row and col masks (either enabled or disabled,
> whatever is more convenient) and then indeed we could figure out the
> resulting size of key matrix and use matrix_keypad_build_keymap() to
> load it.
> 

Alright, I'll see how it looks like and DT maintainers can then comment on it.
I'm afraid they can complain about the masks (for not being super user friendly)
but I think key arrays should be acceptable. I'll try the masks anyways...

- Nuno Sá

>