[PATCH 00/14] Remove support for platform data from samsung keypad

[PATCH 00/14] Remove support for platform data from samsung keypad

[Dmitry Torokhov]

Hi,

This series attempts to rework samsumg=keypad driver to stop using
platform data and instead rely on generic device properties only.

The first 8 patches are general cleanup/facelift patches.

The 9th patch introduces alternative binding that is more compact that
the original one, which makes it more suitable for use in legacy (non
DT) boards with static device properties. Note that the "new" binding is
the standard binding for matrix keypads.

Patch #10 implements the new binding in the driver, #11 converts the
only user of platform data in the mainline tree to the static device
properties, and #12 drops support for platform data from the driver.

Patches #13 and #14 are "bonus" converting the rest of crag6410 to use
software nodes/properties to describe GPIO keys, LEDs and other
peripherals. Note that I believe they fix and issue with recent
conversion to GPIO lookup tables - the names of gpiochip structures I
think are "GP<N>" ("GPK", "GPL", etc) and not "GPIO<N>".

Mark, you said you are using this board, it would be great if you could
give this a spin.

Thanks!

Dmitry Torokhov (14):
  Input: samsung-keypad - switch to using devm_clk_get_prepared()
  Input: samsung-keypad - do not set input device's parent explicitly
  Input: samsung-keypad - do not combine memory allocation checks
  Input: samsung-keypad - use struct_size() helper
  Input: samsung-keypad - use devm to disable runtime PM
  Input: samsung-keypad - use guard notation to acquire mutex
  Input: samsung-keypad - use per-chip parameters
  Input: samsung-keypad - use BIT() and GENMASK() where appropriate
  dt-bindings: input: samsung,s3c6410-keypad: introduce compact binding
  Input: samsung-keypad - handle compact binding
  ARM: s3c: crag6410: switch keypad device to software properties
  Input: samsung-keypad - remove support for platform data
  ARM: s3c: crag6410: use software nodes/properties to set up GPIO keys
  ARM: s3c: crag6410 - convert GPIO lookup tables to property entries

 .../input/samsung,s3c6410-keypad.yaml         |  57 +++-
 arch/arm/mach-s3c/Kconfig                     |   5 -
 arch/arm/mach-s3c/Kconfig.s3c64xx             |   1 -
 arch/arm/mach-s3c/Makefile.s3c64xx            |   1 -
 arch/arm/mach-s3c/devs.c                      |  62 ----
 arch/arm/mach-s3c/devs.h                      |   2 -
 arch/arm/mach-s3c/gpio-samsung-s3c64xx.h      |   5 +
 arch/arm/mach-s3c/gpio-samsung.c              |  35 ++
 arch/arm/mach-s3c/keypad.h                    |  27 --
 arch/arm/mach-s3c/mach-crag6410.c             | 242 ++++++++-----
 arch/arm/mach-s3c/setup-keypad-s3c64xx.c      |  20 --
 drivers/input/keyboard/samsung-keypad.c       | 318 ++++++++----------
 include/linux/input/samsung-keypad.h          |  39 ---
 13 files changed, 398 insertions(+), 416 deletions(-)
 delete mode 100644 arch/arm/mach-s3c/keypad.h
 delete mode 100644 arch/arm/mach-s3c/setup-keypad-s3c64xx.c
 delete mode 100644 include/linux/input/samsung-keypad.h

-- 
Dmitry

[PATCH 01/14] Input: samsung-keypad - switch to using devm_clk_get_prepared()

[Dmitry Torokhov]

Switch to using devm_clk_get_prepared() instead of combining
devm_clk_get() with clk_prepare(), which simplifies the code and
ensures that the clock is unprepared at the right time relative to
releasing other managed resources.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/keyboard/samsung-keypad.c | 22 ++++------------------
 1 file changed, 4 insertions(+), 18 deletions(-)

diff --git a/drivers/input/keyboard/samsung-keypad.c b/drivers/input/keyboard/samsung-keypad.c
index e212eff7687c..a5fac7de8b5d 100644
--- a/drivers/input/keyboard/samsung-keypad.c
+++ b/drivers/input/keyboard/samsung-keypad.c
@@ -361,18 +361,12 @@ static int samsung_keypad_probe(struct platform_device *pdev)
 	if (!keypad->base)
 		return -EBUSY;
 
-	keypad->clk = devm_clk_get(&pdev->dev, "keypad");
+	keypad->clk = devm_clk_get_prepared(&pdev->dev, "keypad");
 	if (IS_ERR(keypad->clk)) {
 		dev_err(&pdev->dev, "failed to get keypad clk\n");
 		return PTR_ERR(keypad->clk);
 	}
 
-	error = clk_prepare(keypad->clk);
-	if (error) {
-		dev_err(&pdev->dev, "keypad clock prepare failed\n");
-		return error;
-	}
-
 	keypad->input_dev = input_dev;
 	keypad->pdev = pdev;
 	keypad->row_shift = row_shift;
@@ -399,7 +393,7 @@ static int samsung_keypad_probe(struct platform_device *pdev)
 					   keypad->keycodes, input_dev);
 	if (error) {
 		dev_err(&pdev->dev, "failed to build keymap\n");
-		goto err_unprepare_clk;
+		return error;
 	}
 
 	input_set_capability(input_dev, EV_MSC, MSC_SCAN);
@@ -411,7 +405,7 @@ static int samsung_keypad_probe(struct platform_device *pdev)
 	keypad->irq = platform_get_irq(pdev, 0);
 	if (keypad->irq < 0) {
 		error = keypad->irq;
-		goto err_unprepare_clk;
+		return error;
 	}
 
 	error = devm_request_threaded_irq(&pdev->dev, keypad->irq, NULL,
@@ -419,7 +413,7 @@ static int samsung_keypad_probe(struct platform_device *pdev)
 					  dev_name(&pdev->dev), keypad);
 	if (error) {
 		dev_err(&pdev->dev, "failed to register keypad interrupt\n");
-		goto err_unprepare_clk;
+		return error;
 	}
 
 	device_init_wakeup(&pdev->dev, pdata->wakeup);
@@ -439,20 +433,12 @@ static int samsung_keypad_probe(struct platform_device *pdev)
 
 err_disable_runtime_pm:
 	pm_runtime_disable(&pdev->dev);
-err_unprepare_clk:
-	clk_unprepare(keypad->clk);
 	return error;
 }
 
 static void samsung_keypad_remove(struct platform_device *pdev)
 {
-	struct samsung_keypad *keypad = platform_get_drvdata(pdev);
-
 	pm_runtime_disable(&pdev->dev);
-
-	input_unregister_device(keypad->input_dev);
-
-	clk_unprepare(keypad->clk);
 }
 
 static int samsung_keypad_runtime_suspend(struct device *dev)
-- 
2.46.0.184.g6999bdac58-goog

[PATCH 02/14] Input: samsung-keypad - do not set input device's parent explicitly

[Dmitry Torokhov]

The driver uses devm_input_allocate_device() to allocate instances of
input device, which sets the parent appropriately. Remove extraneous
assignment.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/keyboard/samsung-keypad.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/input/keyboard/samsung-keypad.c b/drivers/input/keyboard/samsung-keypad.c
index a5fac7de8b5d..46c0d0c900f6 100644
--- a/drivers/input/keyboard/samsung-keypad.c
+++ b/drivers/input/keyboard/samsung-keypad.c
@@ -383,7 +383,6 @@ static int samsung_keypad_probe(struct platform_device *pdev)
 
 	input_dev->name = pdev->name;
 	input_dev->id.bustype = BUS_HOST;
-	input_dev->dev.parent = &pdev->dev;
 
 	input_dev->open = samsung_keypad_open;
 	input_dev->close = samsung_keypad_close;
-- 
2.46.0.184.g6999bdac58-goog

[PATCH 03/14] Input: samsung-keypad - do not combine memory allocation checks

[Dmitry Torokhov]

The driver uses devm API to allocate resources so there's no reason
to do allocations first and then check and release everything at once.

Check results immediately after doing allocation of each resource.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/keyboard/samsung-keypad.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/input/keyboard/samsung-keypad.c b/drivers/input/keyboard/samsung-keypad.c
index 46c0d0c900f6..2be3bfa87ce1 100644
--- a/drivers/input/keyboard/samsung-keypad.c
+++ b/drivers/input/keyboard/samsung-keypad.c
@@ -349,8 +349,11 @@ static int samsung_keypad_probe(struct platform_device *pdev)
 
 	keypad = devm_kzalloc(&pdev->dev, sizeof(*keypad) + keymap_size,
 			      GFP_KERNEL);
+	if (!keypad)
+		return -ENOMEM;
+
 	input_dev = devm_input_allocate_device(&pdev->dev);
-	if (!keypad || !input_dev)
+	if (!input_dev)
 		return -ENOMEM;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-- 
2.46.0.184.g6999bdac58-goog

[PATCH 04/14] Input: samsung-keypad - use struct_size() helper

[Dmitry Torokhov]

When allocating memory for the keypad use struct_size() helper to be
protected from overflows.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/keyboard/samsung-keypad.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/input/keyboard/samsung-keypad.c b/drivers/input/keyboard/samsung-keypad.c
index 2be3bfa87ce1..7adcd2657bca 100644
--- a/drivers/input/keyboard/samsung-keypad.c
+++ b/drivers/input/keyboard/samsung-keypad.c
@@ -318,7 +318,6 @@ static int samsung_keypad_probe(struct platform_device *pdev)
 	struct resource *res;
 	struct input_dev *input_dev;
 	unsigned int row_shift;
-	unsigned int keymap_size;
 	int error;
 
 	pdata = dev_get_platdata(&pdev->dev);
@@ -345,9 +344,10 @@ static int samsung_keypad_probe(struct platform_device *pdev)
 		pdata->cfg_gpio(pdata->rows, pdata->cols);
 
 	row_shift = get_count_order(pdata->cols);
-	keymap_size = (pdata->rows << row_shift) * sizeof(keypad->keycodes[0]);
 
-	keypad = devm_kzalloc(&pdev->dev, sizeof(*keypad) + keymap_size,
+	keypad = devm_kzalloc(&pdev->dev,
+			      struct_size(keypad, keycodes,
+					  pdata->rows << row_shift),
 			      GFP_KERNEL);
 	if (!keypad)
 		return -ENOMEM;
-- 
2.46.0.184.g6999bdac58-goog

[PATCH 05/14] Input: samsung-keypad - use devm to disable runtime PM

[Dmitry Torokhov]

To make sure that runtime PM is disabled at the right time relative
to all other devm-managed resources use devm_add_action_or_reset()
to register a handler that will disable it.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/keyboard/samsung-keypad.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/input/keyboard/samsung-keypad.c b/drivers/input/keyboard/samsung-keypad.c
index 7adcd2657bca..924327de4d8f 100644
--- a/drivers/input/keyboard/samsung-keypad.c
+++ b/drivers/input/keyboard/samsung-keypad.c
@@ -310,6 +310,13 @@ samsung_keypad_parse_dt(struct device *dev)
 }
 #endif
 
+static void samsung_disable_runtime_pm(void *data)
+{
+	struct samsung_keypad *keypad = data;
+
+	pm_runtime_disable(&keypad->pdev->dev);
+}
+
 static int samsung_keypad_probe(struct platform_device *pdev)
 {
 	const struct samsung_keypad_platdata *pdata;
@@ -420,11 +427,16 @@ static int samsung_keypad_probe(struct platform_device *pdev)
 
 	device_init_wakeup(&pdev->dev, pdata->wakeup);
 	platform_set_drvdata(pdev, keypad);
+
 	pm_runtime_enable(&pdev->dev);
+	error = devm_add_action_or_reset(&pdev->dev, samsung_disable_runtime_pm,
+					 keypad);
+	if (error)
+		return error;
 
 	error = input_register_device(keypad->input_dev);
 	if (error)
-		goto err_disable_runtime_pm;
+		return error;
 
 	if (pdev->dev.of_node) {
 		devm_kfree(&pdev->dev, (void *)pdata->keymap_data->keymap);
@@ -432,15 +444,6 @@ static int samsung_keypad_probe(struct platform_device *pdev)
 		devm_kfree(&pdev->dev, (void *)pdata);
 	}
 	return 0;
-
-err_disable_runtime_pm:
-	pm_runtime_disable(&pdev->dev);
-	return error;
-}
-
-static void samsung_keypad_remove(struct platform_device *pdev)
-{
-	pm_runtime_disable(&pdev->dev);
 }
 
 static int samsung_keypad_runtime_suspend(struct device *dev)
@@ -575,7 +578,6 @@ MODULE_DEVICE_TABLE(platform, samsung_keypad_driver_ids);
 
 static struct platform_driver samsung_keypad_driver = {
 	.probe		= samsung_keypad_probe,
-	.remove_new	= samsung_keypad_remove,
 	.driver		= {
 		.name	= "samsung-keypad",
 		.of_match_table = of_match_ptr(samsung_keypad_dt_match),
-- 
2.46.0.184.g6999bdac58-goog

[PATCH 06/14] Input: samsung-keypad - use guard notation to acquire mutex

[Dmitry Torokhov]

Guard notation is more compact and ensures that the mutex will be
released when control leaves the function.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/keyboard/samsung-keypad.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/input/keyboard/samsung-keypad.c b/drivers/input/keyboard/samsung-keypad.c
index 924327de4d8f..71f5b85b02bd 100644
--- a/drivers/input/keyboard/samsung-keypad.c
+++ b/drivers/input/keyboard/samsung-keypad.c
@@ -519,15 +519,13 @@ static int samsung_keypad_suspend(struct device *dev)
 	struct samsung_keypad *keypad = platform_get_drvdata(pdev);
 	struct input_dev *input_dev = keypad->input_dev;
 
-	mutex_lock(&input_dev->mutex);
+	guard(mutex)(&input_dev->mutex);
 
 	if (input_device_enabled(input_dev))
 		samsung_keypad_stop(keypad);
 
 	samsung_keypad_toggle_wakeup(keypad, true);
 
-	mutex_unlock(&input_dev->mutex);
-
 	return 0;
 }
 
@@ -537,15 +535,13 @@ static int samsung_keypad_resume(struct device *dev)
 	struct samsung_keypad *keypad = platform_get_drvdata(pdev);
 	struct input_dev *input_dev = keypad->input_dev;
 
-	mutex_lock(&input_dev->mutex);
+	guard(mutex)(&input_dev->mutex);
 
 	samsung_keypad_toggle_wakeup(keypad, false);
 
 	if (input_device_enabled(input_dev))
 		samsung_keypad_start(keypad);
 
-	mutex_unlock(&input_dev->mutex);
-
 	return 0;
 }
 
-- 
2.46.0.184.g6999bdac58-goog

[PATCH 07/14] Input: samsung-keypad - use per-chip parameters

[Dmitry Torokhov]

Instead of doing conditional logic based on the chip type, define
per-chip parameter structure, and reference it in the match data
(either of_device_id or platform_device_id).

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/keyboard/samsung-keypad.c | 61 +++++++++++++++----------
 1 file changed, 37 insertions(+), 24 deletions(-)

diff --git a/drivers/input/keyboard/samsung-keypad.c b/drivers/input/keyboard/samsung-keypad.c
index 71f5b85b02bd..ccc1186e43b6 100644
--- a/drivers/input/keyboard/samsung-keypad.c
+++ b/drivers/input/keyboard/samsung-keypad.c
@@ -44,8 +44,7 @@
 #define S5PV210_KEYIFSTSCLR_R_INT_OFFSET	16
 
 /* SAMSUNG_KEYIFCOL */
-#define SAMSUNG_KEYIFCOL_MASK			(0xff << 0)
-#define S5PV210_KEYIFCOLEN_MASK			(0xff << 8)
+#define SAMSUNG_KEYIFCOL_MASK			0xff
 
 /* SAMSUNG_KEYIFROW */
 #define SAMSUNG_KEYIFROW_MASK			(0xff << 0)
@@ -54,12 +53,12 @@
 /* SAMSUNG_KEYIFFC */
 #define SAMSUNG_KEYIFFC_MASK			(0x3ff << 0)
 
-enum samsung_keypad_type {
-	KEYPAD_TYPE_SAMSUNG,
-	KEYPAD_TYPE_S5PV210,
+struct samsung_chip_info {
+	unsigned int column_shift;
 };
 
 struct samsung_keypad {
+	const struct samsung_chip_info *chip;
 	struct input_dev *input_dev;
 	struct platform_device *pdev;
 	struct clk *clk;
@@ -68,7 +67,6 @@ struct samsung_keypad {
 	bool stopped;
 	bool wake_enabled;
 	int irq;
-	enum samsung_keypad_type type;
 	unsigned int row_shift;
 	unsigned int rows;
 	unsigned int cols;
@@ -83,13 +81,8 @@ static void samsung_keypad_scan(struct samsung_keypad *keypad,
 	unsigned int val;
 
 	for (col = 0; col < keypad->cols; col++) {
-		if (keypad->type == KEYPAD_TYPE_S5PV210) {
-			val = S5PV210_KEYIFCOLEN_MASK;
-			val &= ~(1 << col) << 8;
-		} else {
-			val = SAMSUNG_KEYIFCOL_MASK;
-			val &= ~(1 << col);
-		}
+		val = SAMSUNG_KEYIFCOL_MASK & ~(1 << col);
+		val <<= keypad->chip->column_shift;
 
 		writel(val, keypad->base + SAMSUNG_KEYIFCOL);
 		mdelay(1);
@@ -321,6 +314,7 @@ static int samsung_keypad_probe(struct platform_device *pdev)
 {
 	const struct samsung_keypad_platdata *pdata;
 	const struct matrix_keymap_data *keymap_data;
+	const struct platform_device_id *id;
 	struct samsung_keypad *keypad;
 	struct resource *res;
 	struct input_dev *input_dev;
@@ -385,11 +379,17 @@ static int samsung_keypad_probe(struct platform_device *pdev)
 	keypad->stopped = true;
 	init_waitqueue_head(&keypad->wait);
 
-	if (pdev->dev.of_node)
-		keypad->type = of_device_is_compatible(pdev->dev.of_node,
-					"samsung,s5pv210-keypad");
-	else
-		keypad->type = platform_get_device_id(pdev)->driver_data;
+	keypad->chip = device_get_match_data(&pdev->dev);
+	if (!keypad->chip) {
+		id = platform_get_device_id(pdev);
+		if (id)
+			keypad->chip = (const void *)id->driver_data;
+	}
+
+	if (!keypad->chip) {
+		dev_err(&pdev->dev, "Unable to determine chip type");
+		return -EINVAL;
+	}
 
 	input_dev->name = pdev->name;
 	input_dev->id.bustype = BUS_HOST;
@@ -551,11 +551,24 @@ static const struct dev_pm_ops samsung_keypad_pm_ops = {
 		       samsung_keypad_runtime_resume, NULL)
 };
 
+static const struct samsung_chip_info samsung_s3c6410_chip_info = {
+	.column_shift = 0,
+};
+
+static const struct samsung_chip_info samsung_s5pv210_chip_info = {
+	.column_shift = 8,
+};
+
 #ifdef CONFIG_OF
 static const struct of_device_id samsung_keypad_dt_match[] = {
-	{ .compatible = "samsung,s3c6410-keypad" },
-	{ .compatible = "samsung,s5pv210-keypad" },
-	{},
+	{
+		.compatible = "samsung,s3c6410-keypad",
+		.data = &samsung_s3c6410_chip_info,
+	}, {
+		.compatible = "samsung,s5pv210-keypad",
+		.data = &samsung_s5pv210_chip_info,
+	},
+	{ }
 };
 MODULE_DEVICE_TABLE(of, samsung_keypad_dt_match);
 #endif
@@ -563,12 +576,12 @@ MODULE_DEVICE_TABLE(of, samsung_keypad_dt_match);
 static const struct platform_device_id samsung_keypad_driver_ids[] = {
 	{
 		.name		= "samsung-keypad",
-		.driver_data	= KEYPAD_TYPE_SAMSUNG,
+		.driver_data	= (kernel_ulong_t)&samsung_s3c6410_chip_info,
 	}, {
 		.name		= "s5pv210-keypad",
-		.driver_data	= KEYPAD_TYPE_S5PV210,
+		.driver_data	= (kernel_ulong_t)&samsung_s5pv210_chip_info,
 	},
-	{ },
+	{ }
 };
 MODULE_DEVICE_TABLE(platform, samsung_keypad_driver_ids);
 
-- 
2.46.0.184.g6999bdac58-goog

[PATCH 08/14] Input: samsung-keypad - use BIT() and GENMASK() where appropriate

[Dmitry Torokhov]

Instead of using (1 << <shift>) construct use BIT() helper. Convert (1
<< <shift>) - 1 to GENMASK().

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/keyboard/samsung-keypad.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/input/keyboard/samsung-keypad.c b/drivers/input/keyboard/samsung-keypad.c
index ccc1186e43b6..e262137b6838 100644
--- a/drivers/input/keyboard/samsung-keypad.c
+++ b/drivers/input/keyboard/samsung-keypad.c
@@ -7,6 +7,7 @@
  * Author: Donghwa Lee <dh09.lee@samsung.com>
  */
 
+#include <linux/bits.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/err.h>
@@ -29,11 +30,11 @@
 #define SAMSUNG_KEYIFFC				0x10
 
 /* SAMSUNG_KEYIFCON */
-#define SAMSUNG_KEYIFCON_INT_F_EN		(1 << 0)
-#define SAMSUNG_KEYIFCON_INT_R_EN		(1 << 1)
-#define SAMSUNG_KEYIFCON_DF_EN			(1 << 2)
-#define SAMSUNG_KEYIFCON_FC_EN			(1 << 3)
-#define SAMSUNG_KEYIFCON_WAKEUPEN		(1 << 4)
+#define SAMSUNG_KEYIFCON_INT_F_EN		BIT(0)
+#define SAMSUNG_KEYIFCON_INT_R_EN		BIT(1)
+#define SAMSUNG_KEYIFCON_DF_EN			BIT(2)
+#define SAMSUNG_KEYIFCON_FC_EN			BIT(3)
+#define SAMSUNG_KEYIFCON_WAKEUPEN		BIT(4)
 
 /* SAMSUNG_KEYIFSTSCLR */
 #define SAMSUNG_KEYIFSTSCLR_P_INT_MASK		(0xff << 0)
@@ -81,14 +82,14 @@ static void samsung_keypad_scan(struct samsung_keypad *keypad,
 	unsigned int val;
 
 	for (col = 0; col < keypad->cols; col++) {
-		val = SAMSUNG_KEYIFCOL_MASK & ~(1 << col);
+		val = SAMSUNG_KEYIFCOL_MASK & ~BIT(col);
 		val <<= keypad->chip->column_shift;
 
 		writel(val, keypad->base + SAMSUNG_KEYIFCOL);
 		mdelay(1);
 
 		val = readl(keypad->base + SAMSUNG_KEYIFROW);
-		row_state[col] = ~val & ((1 << keypad->rows) - 1);
+		row_state[col] = ~val & GENMASK(keypad->rows - 1, 0);
 	}
 
 	/* KEYIFCOL reg clear */
@@ -112,10 +113,10 @@ static bool samsung_keypad_report(struct samsung_keypad *keypad,
 			continue;
 
 		for (row = 0; row < keypad->rows; row++) {
-			if (!(changed & (1 << row)))
+			if (!(changed & BIT(row)))
 				continue;
 
-			pressed = row_state[col] & (1 << row);
+			pressed = row_state[col] & BIT(row);
 
 			dev_dbg(&keypad->input_dev->dev,
 				"key %s, row: %d, col: %d\n",
-- 
2.46.0.184.g6999bdac58-goog

[PATCH 09/14] dt-bindings: input: samsung,s3c6410-keypad: introduce compact binding

[Dmitry Torokhov]

The binding with a sub-node per each key is very verbose and is hard to
use with static device properties. Allow standard matrix keymap binding
in addition to the verbose one.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 .../input/samsung,s3c6410-keypad.yaml         | 57 ++++++++++++++++++-
 1 file changed, 54 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/input/samsung,s3c6410-keypad.yaml b/Documentation/devicetree/bindings/input/samsung,s3c6410-keypad.yaml
index a53569aa0ee7..28a318a0ff7e 100644
--- a/Documentation/devicetree/bindings/input/samsung,s3c6410-keypad.yaml
+++ b/Documentation/devicetree/bindings/input/samsung,s3c6410-keypad.yaml
@@ -16,6 +16,10 @@ description:
 maintainers:
   - Krzysztof Kozlowski <krzk@kernel.org>
 
+allOf:
+  - $ref: input.yaml#
+  - $ref: matrix-keymap.yaml#
+
 properties:
   compatible:
     enum:
@@ -37,6 +41,10 @@ properties:
 
   wakeup-source: true
 
+  keypad,num-columns: true
+  keypad,num-rows: true
+  linux,keymap: true
+
   linux,input-no-autorepeat:
     type: boolean
     description:
@@ -81,12 +89,33 @@ patternProperties:
       - keypad,row
       - linux,code
 
+dependencies:
+  linux,keymap:
+    required:
+      - keypad,num-columns
+      - keypad,num-rows
+
 required:
   - compatible
   - reg
   - interrupts
-  - samsung,keypad-num-columns
-  - samsung,keypad-num-rows
+
+if:
+  required:
+    - linux,keymap
+then:
+  properties:
+    samsung,keypad-num-columns: false
+    samsung,keypad-num-rows: false
+  patternProperties:
+    '^key-[0-9a-z]+$': false
+else:
+  properties:
+    keypad,num-columns: false
+    keypad,num-rows: false
+  required:
+    - samsung,keypad-num-columns
+    - samsung,keypad-num-rows
 
 additionalProperties: false
 
@@ -94,8 +123,9 @@ examples:
   - |
     #include <dt-bindings/clock/exynos4.h>
     #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/input/input.h>
 
-    keypad@100a0000 {
+    keypad1@100a0000 {
         compatible = "samsung,s5pv210-keypad";
         reg = <0x100a0000 0x100>;
         interrupts = <GIC_SPI 109 IRQ_TYPE_LEVEL_HIGH>;
@@ -119,3 +149,24 @@ examples:
             linux,code = <3>;
         };
     };
+  - |
+    #include <dt-bindings/clock/exynos4.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/input/input.h>
+
+    keypad2@100a0000 {
+        compatible = "samsung,s5pv210-keypad";
+        reg = <0x100a0000 0x100>;
+        interrupts = <GIC_SPI 109 IRQ_TYPE_LEVEL_HIGH>;
+        clocks = <&clock CLK_KEYIF>;
+        clock-names = "keypad";
+
+        keypad,num-rows = <2>;
+        keypad,num-columns = <8>;
+        linux,keymap = <
+          MATRIX_KEY(0, 3, 2)
+          MATRIX_KEY(0, 4, 3)
+        >;
+        linux,input-no-autorepeat;
+        wakeup-source;
+    };
-- 
2.46.0.184.g6999bdac58-goog

[PATCH 10/14] Input: samsung-keypad - handle compact binding

[Dmitry Torokhov]

Add support for standard matrix keymap binding (in addition to the
existing verbose binding with a sub-node for each key). This will
allow easier conversions from platform data to device properties when
using static device properties.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/keyboard/samsung-keypad.c | 122 +++++++++++++-----------
 1 file changed, 64 insertions(+), 58 deletions(-)

diff --git a/drivers/input/keyboard/samsung-keypad.c b/drivers/input/keyboard/samsung-keypad.c
index e262137b6838..df2427ac63d7 100644
--- a/drivers/input/keyboard/samsung-keypad.c
+++ b/drivers/input/keyboard/samsung-keypad.c
@@ -231,78 +231,85 @@ static void samsung_keypad_close(struct input_dev *input_dev)
 	samsung_keypad_stop(keypad);
 }
 
-#ifdef CONFIG_OF
-static struct samsung_keypad_platdata *
-samsung_keypad_parse_dt(struct device *dev)
+static const struct matrix_keymap_data *
+samsung_parse_verbose_keymap(struct device *dev)
 {
-	struct samsung_keypad_platdata *pdata;
 	struct matrix_keymap_data *keymap_data;
-	uint32_t *keymap, num_rows = 0, num_cols = 0;
-	struct device_node *np = dev->of_node, *key_np;
+	struct fwnode_handle *child;
+	u32 *keymap;
 	unsigned int key_count;
 
-	if (!np) {
-		dev_err(dev, "missing device tree data\n");
-		return ERR_PTR(-EINVAL);
-	}
-
-	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
-	if (!pdata) {
-		dev_err(dev, "could not allocate memory for platform data\n");
-		return ERR_PTR(-ENOMEM);
-	}
-
-	of_property_read_u32(np, "samsung,keypad-num-rows", &num_rows);
-	of_property_read_u32(np, "samsung,keypad-num-columns", &num_cols);
-	if (!num_rows || !num_cols) {
-		dev_err(dev, "number of keypad rows/columns not specified\n");
-		return ERR_PTR(-EINVAL);
-	}
-	pdata->rows = num_rows;
-	pdata->cols = num_cols;
-
 	keymap_data = devm_kzalloc(dev, sizeof(*keymap_data), GFP_KERNEL);
 	if (!keymap_data) {
 		dev_err(dev, "could not allocate memory for keymap data\n");
 		return ERR_PTR(-ENOMEM);
 	}
-	pdata->keymap_data = keymap_data;
 
-	key_count = of_get_child_count(np);
-	keymap_data->keymap_size = key_count;
-	keymap = devm_kcalloc(dev, key_count, sizeof(uint32_t), GFP_KERNEL);
+	key_count = device_get_child_node_count(dev);
+	keymap = devm_kcalloc(dev, key_count, sizeof(*keymap), GFP_KERNEL);
 	if (!keymap) {
 		dev_err(dev, "could not allocate memory for keymap\n");
 		return ERR_PTR(-ENOMEM);
 	}
+
+	keymap_data->keymap_size = key_count;
 	keymap_data->keymap = keymap;
 
-	for_each_child_of_node(np, key_np) {
+	device_for_each_child_node(dev, child) {
 		u32 row, col, key_code;
-		of_property_read_u32(key_np, "keypad,row", &row);
-		of_property_read_u32(key_np, "keypad,column", &col);
-		of_property_read_u32(key_np, "linux,code", &key_code);
+
+		fwnode_property_read_u32(child, "keypad,row", &row);
+		fwnode_property_read_u32(child, "keypad,column", &col);
+		fwnode_property_read_u32(child, "linux,code", &key_code);
+
 		*keymap++ = KEY(row, col, key_code);
 	}
 
-	pdata->no_autorepeat = of_property_read_bool(np, "linux,input-no-autorepeat");
+	return keymap_data;
+}
 
-	pdata->wakeup = of_property_read_bool(np, "wakeup-source") ||
-			/* legacy name */
-			of_property_read_bool(np, "linux,input-wakeup");
+static const struct samsung_keypad_platdata *
+samsung_keypad_parse_properties(struct device *dev)
+{
+	const struct matrix_keymap_data *keymap_data;
+	struct samsung_keypad_platdata *pdata;
+	u32 num_rows = 0, num_cols = 0;
+	int error;
+
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata) {
+		dev_err(dev, "could not allocate memory for platform data\n");
+		return ERR_PTR(-ENOMEM);
+	}
 
+	device_property_read_u32(dev, "samsung,keypad-num-rows", &num_rows);
+	device_property_read_u32(dev, "samsung,keypad-num-columns", &num_cols);
+
+	error = matrix_keypad_parse_properties(dev, &num_rows, &num_cols);
+	if (error)
+		return ERR_PTR(error);
+
+	pdata->rows = num_rows;
+	pdata->cols = num_cols;
+
+	if (!device_property_present(dev, "linux,keymap")) {
+		keymap_data = samsung_parse_verbose_keymap(dev);
+		if (IS_ERR(keymap_data))
+			return ERR_CAST(keymap_data);
+
+		pdata->keymap_data = keymap_data;
+	}
 
-	return pdata;
-}
-#else
-static struct samsung_keypad_platdata *
-samsung_keypad_parse_dt(struct device *dev)
-{
-	dev_err(dev, "no platform data defined\n");
 
-	return ERR_PTR(-EINVAL);
+	pdata->no_autorepeat =
+		device_property_read_bool(dev, "linux,input-no-autorepeat");
+
+	pdata->wakeup = device_property_read_bool(dev, "wakeup-source") ||
+			/* legacy name */
+			device_property_read_bool(dev, "linux,input-wakeup");
+
+	return pdata;
 }
-#endif
 
 static void samsung_disable_runtime_pm(void *data)
 {
@@ -314,7 +321,6 @@ static void samsung_disable_runtime_pm(void *data)
 static int samsung_keypad_probe(struct platform_device *pdev)
 {
 	const struct samsung_keypad_platdata *pdata;
-	const struct matrix_keymap_data *keymap_data;
 	const struct platform_device_id *id;
 	struct samsung_keypad *keypad;
 	struct resource *res;
@@ -323,18 +329,17 @@ static int samsung_keypad_probe(struct platform_device *pdev)
 	int error;
 
 	pdata = dev_get_platdata(&pdev->dev);
-	if (!pdata) {
-		pdata = samsung_keypad_parse_dt(&pdev->dev);
+	if (pdata) {
+		if (!pdata->keymap_data) {
+			dev_err(&pdev->dev, "no keymap data defined\n");
+			return -EINVAL;
+		}
+	} else {
+		pdata = samsung_keypad_parse_properties(&pdev->dev);
 		if (IS_ERR(pdata))
 			return PTR_ERR(pdata);
 	}
 
-	keymap_data = pdata->keymap_data;
-	if (!keymap_data) {
-		dev_err(&pdev->dev, "no keymap data defined\n");
-		return -EINVAL;
-	}
-
 	if (!pdata->rows || pdata->rows > SAMSUNG_MAX_ROWS)
 		return -EINVAL;
 
@@ -398,7 +403,7 @@ static int samsung_keypad_probe(struct platform_device *pdev)
 	input_dev->open = samsung_keypad_open;
 	input_dev->close = samsung_keypad_close;
 
-	error = matrix_keypad_build_keymap(keymap_data, NULL,
+	error = matrix_keypad_build_keymap(pdata->keymap_data, NULL,
 					   pdata->rows, pdata->cols,
 					   keypad->keycodes, input_dev);
 	if (error) {
@@ -439,11 +444,12 @@ static int samsung_keypad_probe(struct platform_device *pdev)
 	if (error)
 		return error;
 
-	if (pdev->dev.of_node) {
+	if (!dev_get_platdata(&pdev->dev)) {
 		devm_kfree(&pdev->dev, (void *)pdata->keymap_data->keymap);
 		devm_kfree(&pdev->dev, (void *)pdata->keymap_data);
 		devm_kfree(&pdev->dev, (void *)pdata);
 	}
+
 	return 0;
 }
 
-- 
2.46.0.184.g6999bdac58-goog

[PATCH 11/14] ARM: s3c: crag6410: switch keypad device to software properties

[Dmitry Torokhov]

Switch the keypad device to use software properties to describe the
keypad. This will allow dropping support for platform data from the
samsung-keypad driver.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 arch/arm/mach-s3c/Kconfig                |  5 ---
 arch/arm/mach-s3c/Kconfig.s3c64xx        |  1 -
 arch/arm/mach-s3c/Makefile.s3c64xx       |  1 -
 arch/arm/mach-s3c/devs.c                 | 27 ------------
 arch/arm/mach-s3c/devs.h                 |  1 -
 arch/arm/mach-s3c/keypad.h               | 27 ------------
 arch/arm/mach-s3c/mach-crag6410.c        | 56 ++++++++++++++++++------
 arch/arm/mach-s3c/setup-keypad-s3c64xx.c | 20 ---------
 8 files changed, 42 insertions(+), 96 deletions(-)
 delete mode 100644 arch/arm/mach-s3c/keypad.h
 delete mode 100644 arch/arm/mach-s3c/setup-keypad-s3c64xx.c

diff --git a/arch/arm/mach-s3c/Kconfig b/arch/arm/mach-s3c/Kconfig
index b3656109f1f7..2e77bb4f3a8c 100644
--- a/arch/arm/mach-s3c/Kconfig
+++ b/arch/arm/mach-s3c/Kconfig
@@ -111,11 +111,6 @@ config S3C64XX_DEV_SPI0
 	  Compile in platform device definitions for S3C64XX's type
 	  SPI controller 0
 
-config SAMSUNG_DEV_KEYPAD
-	bool
-	help
-	  Compile in platform device definitions for keypad
-
 config SAMSUNG_DEV_PWM
 	bool
 	help
diff --git a/arch/arm/mach-s3c/Kconfig.s3c64xx b/arch/arm/mach-s3c/Kconfig.s3c64xx
index 8f40af063ad6..cc1257fbf642 100644
--- a/arch/arm/mach-s3c/Kconfig.s3c64xx
+++ b/arch/arm/mach-s3c/Kconfig.s3c64xx
@@ -116,7 +116,6 @@ config MACH_WLF_CRAGG_6410
 	select S3C_DEV_I2C1
 	select S3C_DEV_USB_HOST
 	select S3C_DEV_USB_HSOTG
-	select SAMSUNG_DEV_KEYPAD
 	select SAMSUNG_DEV_PWM
 	help
 	  Machine support for the Wolfson Cragganmore S3C6410 variant.
diff --git a/arch/arm/mach-s3c/Makefile.s3c64xx b/arch/arm/mach-s3c/Makefile.s3c64xx
index 61287ad2ea42..1686b1f344f8 100644
--- a/arch/arm/mach-s3c/Makefile.s3c64xx
+++ b/arch/arm/mach-s3c/Makefile.s3c64xx
@@ -32,7 +32,6 @@ obj-y				+= dev-audio-s3c64xx.o
 obj-$(CONFIG_S3C64XX_SETUP_FB_24BPP)	+= setup-fb-24bpp-s3c64xx.o
 obj-$(CONFIG_S3C64XX_SETUP_I2C0)	+= setup-i2c0-s3c64xx.o
 obj-$(CONFIG_S3C64XX_SETUP_I2C1)	+= setup-i2c1-s3c64xx.o
-obj-$(CONFIG_S3C64XX_SETUP_KEYPAD)	+= setup-keypad-s3c64xx.o
 obj-$(CONFIG_S3C64XX_SETUP_SDHCI_GPIO)	+= setup-sdhci-gpio-s3c64xx.o
 obj-$(CONFIG_S3C64XX_SETUP_SPI)		+= setup-spi-s3c64xx.o
 obj-$(CONFIG_S3C64XX_SETUP_USB_PHY) += setup-usb-phy-s3c64xx.o
diff --git a/arch/arm/mach-s3c/devs.c b/arch/arm/mach-s3c/devs.c
index 8c26d592d2a3..31827cfc5700 100644
--- a/arch/arm/mach-s3c/devs.c
+++ b/arch/arm/mach-s3c/devs.c
@@ -40,7 +40,6 @@
 #include "devs.h"
 #include "fb.h"
 #include <linux/platform_data/i2c-s3c2410.h>
-#include "keypad.h"
 #include "pwm-core.h"
 #include "sdhci.h"
 #include "usb-phy.h"
@@ -266,32 +265,6 @@ void __init s3c_i2c1_set_platdata(struct s3c2410_platform_i2c *pd)
 }
 #endif /* CONFIG_S3C_DEV_I2C1 */
 
-/* KEYPAD */
-
-#ifdef CONFIG_SAMSUNG_DEV_KEYPAD
-static struct resource samsung_keypad_resources[] = {
-	[0] = DEFINE_RES_MEM(SAMSUNG_PA_KEYPAD, SZ_32),
-	[1] = DEFINE_RES_IRQ(IRQ_KEYPAD),
-};
-
-struct platform_device samsung_device_keypad = {
-	.name		= "samsung-keypad",
-	.id		= -1,
-	.num_resources	= ARRAY_SIZE(samsung_keypad_resources),
-	.resource	= samsung_keypad_resources,
-};
-
-void __init samsung_keypad_set_platdata(struct samsung_keypad_platdata *pd)
-{
-	struct samsung_keypad_platdata *npd;
-
-	npd = s3c_set_platdata(pd, sizeof(*npd), &samsung_device_keypad);
-
-	if (!npd->cfg_gpio)
-		npd->cfg_gpio = samsung_keypad_cfg_gpio;
-}
-#endif /* CONFIG_SAMSUNG_DEV_KEYPAD */
-
 /* PWM Timer */
 
 #ifdef CONFIG_SAMSUNG_DEV_PWM
diff --git a/arch/arm/mach-s3c/devs.h b/arch/arm/mach-s3c/devs.h
index 21c00786c264..2737990063b1 100644
--- a/arch/arm/mach-s3c/devs.h
+++ b/arch/arm/mach-s3c/devs.h
@@ -39,7 +39,6 @@ extern struct platform_device s3c_device_i2c1;
 extern struct platform_device s3c_device_ohci;
 extern struct platform_device s3c_device_usb_hsotg;
 
-extern struct platform_device samsung_device_keypad;
 extern struct platform_device samsung_device_pwm;
 
 /**
diff --git a/arch/arm/mach-s3c/keypad.h b/arch/arm/mach-s3c/keypad.h
deleted file mode 100644
index 9754b9a29945..000000000000
--- a/arch/arm/mach-s3c/keypad.h
+++ /dev/null
@@ -1,27 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0+ */
-/*
- * Samsung Platform - Keypad platform data definitions
- *
- * Copyright (C) 2010 Samsung Electronics Co.Ltd
- * Author: Joonyoung Shim <jy0922.shim@samsung.com>
- */
-
-#ifndef __PLAT_SAMSUNG_KEYPAD_H
-#define __PLAT_SAMSUNG_KEYPAD_H
-
-#include <linux/input/samsung-keypad.h>
-
-/**
- * samsung_keypad_set_platdata - Set platform data for Samsung Keypad device.
- * @pd: Platform data to register to device.
- *
- * Register the given platform data for use with Samsung Keypad device.
- * The call will copy the platform data, so the board definitions can
- * make the structure itself __initdata.
- */
-extern void samsung_keypad_set_platdata(struct samsung_keypad_platdata *pd);
-
-/* defined by architecture to configure gpio. */
-extern void samsung_keypad_cfg_gpio(unsigned int rows, unsigned int cols);
-
-#endif /* __PLAT_SAMSUNG_KEYPAD_H */
diff --git a/arch/arm/mach-s3c/mach-crag6410.c b/arch/arm/mach-s3c/mach-crag6410.c
index e5df2cb51ab2..16b6ef312aaf 100644
--- a/arch/arm/mach-s3c/mach-crag6410.c
+++ b/arch/arm/mach-s3c/mach-crag6410.c
@@ -15,6 +15,7 @@
 #include <linux/io.h>
 #include <linux/init.h>
 #include <linux/input-event-codes.h>
+#include <linux/input/matrix_keypad.h>
 #include <linux/gpio.h>
 #include <linux/gpio/machine.h>
 #include <linux/leds.h>
@@ -53,7 +54,6 @@
 #include "gpio-cfg.h"
 #include <linux/platform_data/spi-s3c64xx.h>
 
-#include "keypad.h"
 #include "devs.h"
 #include "cpu.h"
 #include <linux/platform_data/i2c-s3c2410.h>
@@ -176,7 +176,7 @@ static struct s3c_fb_platdata crag6410_lcd_pdata = {
 
 /* 2x6 keypad */
 
-static uint32_t crag6410_keymap[] = {
+static const uint32_t crag6410_keymap[] __initconst = {
 	/* KEY(row, col, keycode) */
 	KEY(0, 0, KEY_VOLUMEUP),
 	KEY(0, 1, KEY_HOME),
@@ -192,17 +192,41 @@ static uint32_t crag6410_keymap[] = {
 	KEY(1, 5, KEY_CAMERA),
 };
 
-static struct matrix_keymap_data crag6410_keymap_data = {
-	.keymap		= crag6410_keymap,
-	.keymap_size	= ARRAY_SIZE(crag6410_keymap),
+static const struct property_entry crag6410_keypad_props[] __initconst = {
+	PROPERTY_ENTRY_U32("keypad,num-columns", 6),
+	PROPERTY_ENTRY_U32("keypad,num-rows", 2),
+	PROPERTY_ENTRY_U32_ARRAY("linux,keymap", crag6410_keymap),
+	{ }
 };
 
-static struct samsung_keypad_platdata crag6410_keypad_data = {
-	.keymap_data	= &crag6410_keymap_data,
-	.rows		= 2,
-	.cols		= 6,
+static const struct resource crag6410_keypad_resources[] __initconst = {
+	[0] = DEFINE_RES_MEM(SAMSUNG_PA_KEYPAD, SZ_32),
+	[1] = DEFINE_RES_IRQ(IRQ_KEYPAD),
 };
 
+static const struct platform_device_info crag6410_keypad_info __initconst = {
+	.name		= "samsung-keypad",
+	.id		= PLATFORM_DEVID_NONE,
+	.res		= crag6410_keypad_resources,
+	.num_res	= ARRAY_SIZE(crag6410_keypad_resources),
+	.properties	= crag6410_keypad_props,
+};
+
+static void __init crag6410_setup_keypad(void)
+{
+	struct platform_device *pd;
+
+	/* Set all the necessary GPK pins to special-function 3: KP_ROW[x] */
+	s3c_gpio_cfgrange_nopull(S3C64XX_GPK(8), 2, S3C_GPIO_SFN(3));
+
+	/* Set all the necessary GPL pins to special-function 3: KP_COL[x] */
+	s3c_gpio_cfgrange_nopull(S3C64XX_GPL(0), 6, S3C_GPIO_SFN(3));
+
+	pd = platform_device_register_full(&crag6410_keypad_info);
+	if (IS_ERR(pd))
+		pr_err("failed to instantiate keypad device");
+}
+
 static struct gpio_keys_button crag6410_gpio_keys[] = {
 	[0] = {
 		.code	= KEY_SUSPEND,
@@ -358,7 +382,7 @@ static struct platform_device wallvdd_device = {
 	},
 };
 
-static struct platform_device *crag6410_devices[] __initdata = {
+static struct platform_device *crag6410_devs0[] __initdata = {
 	&s3c_device_hsmmc0,
 	&s3c_device_hsmmc2,
 	&s3c_device_i2c0,
@@ -369,8 +393,10 @@ static struct platform_device *crag6410_devices[] __initdata = {
 	&samsung_device_pwm,
 	&s3c64xx_device_iis0,
 	&s3c64xx_device_iis1,
-	&samsung_device_keypad,
 	&crag6410_gpio_keydev,
+};
+
+static struct platform_device *crag6410_devs1[] __initdata = {
 	&crag6410_dm9k_device,
 	&s3c64xx_device_spi0,
 	&crag6410_mmgpio,
@@ -864,13 +890,15 @@ static void __init crag6410_machine_init(void)
 	gpiod_add_lookup_table(&crag_wm1250_ev1_gpiod_table);
 	i2c_register_board_info(1, i2c_devs1, ARRAY_SIZE(i2c_devs1));
 
-	samsung_keypad_set_platdata(&crag6410_keypad_data);
-
 	gpiod_add_lookup_table(&crag_spi0_gpiod_table);
 	s3c64xx_spi0_set_platdata(0, 2);
 
 	pwm_add_table(crag6410_pwm_lookup, ARRAY_SIZE(crag6410_pwm_lookup));
-	platform_add_devices(crag6410_devices, ARRAY_SIZE(crag6410_devices));
+	platform_add_devices(crag6410_devs0, ARRAY_SIZE(crag6410_devs0));
+
+	crag6410_setup_keypad();
+
+	platform_add_devices(crag6410_devs1, ARRAY_SIZE(crag6410_devs1));
 
 	gpio_led_register_device(-1, &gpio_leds_pdata);
 
diff --git a/arch/arm/mach-s3c/setup-keypad-s3c64xx.c b/arch/arm/mach-s3c/setup-keypad-s3c64xx.c
deleted file mode 100644
index 8463ad37c6ab..000000000000
--- a/arch/arm/mach-s3c/setup-keypad-s3c64xx.c
+++ /dev/null
@@ -1,20 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-//
-// Copyright (c) 2010 Samsung Electronics Co., Ltd.
-//		http://www.samsung.com/
-//
-// GPIO configuration for S3C64XX KeyPad device
-
-#include <linux/gpio.h>
-#include "gpio-cfg.h"
-#include "keypad.h"
-#include "gpio-samsung.h"
-
-void samsung_keypad_cfg_gpio(unsigned int rows, unsigned int cols)
-{
-	/* Set all the necessary GPK pins to special-function 3: KP_ROW[x] */
-	s3c_gpio_cfgrange_nopull(S3C64XX_GPK(8), rows, S3C_GPIO_SFN(3));
-
-	/* Set all the necessary GPL pins to special-function 3: KP_COL[x] */
-	s3c_gpio_cfgrange_nopull(S3C64XX_GPL(0), cols, S3C_GPIO_SFN(3));
-}
-- 
2.46.0.184.g6999bdac58-goog

[PATCH 12/14] Input: samsung-keypad - remove support for platform data

[Dmitry Torokhov]

Because there are no more users of samsung_keypad_platdata left in
the kernel remove support for it from the driver.

The driver supports generic device properties so all configuration
should be done using them instead of a custom platform data.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/keyboard/samsung-keypad.c | 190 +++++++++---------------
 include/linux/input/samsung-keypad.h    |  39 -----
 2 files changed, 70 insertions(+), 159 deletions(-)
 delete mode 100644 include/linux/input/samsung-keypad.h

diff --git a/drivers/input/keyboard/samsung-keypad.c b/drivers/input/keyboard/samsung-keypad.c
index df2427ac63d7..78fa55498fc3 100644
--- a/drivers/input/keyboard/samsung-keypad.c
+++ b/drivers/input/keyboard/samsung-keypad.c
@@ -12,6 +12,7 @@
 #include <linux/delay.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/module.h>
@@ -21,7 +22,9 @@
 #include <linux/slab.h>
 #include <linux/of.h>
 #include <linux/sched.h>
-#include <linux/input/samsung-keypad.h>
+
+#define SAMSUNG_MAX_ROWS			8
+#define SAMSUNG_MAX_COLS			8
 
 #define SAMSUNG_KEYIFCON			0x00
 #define SAMSUNG_KEYIFSTSCLR			0x04
@@ -231,84 +234,43 @@ static void samsung_keypad_close(struct input_dev *input_dev)
 	samsung_keypad_stop(keypad);
 }
 
-static const struct matrix_keymap_data *
-samsung_parse_verbose_keymap(struct device *dev)
+static int samsung_keypad_parse_keymap(struct samsung_keypad *keypad)
 {
-	struct matrix_keymap_data *keymap_data;
+	struct matrix_keymap_data keymap_data = { 0 };
+	struct device *dev = &keypad->pdev->dev;
 	struct fwnode_handle *child;
 	u32 *keymap;
 	unsigned int key_count;
-
-	keymap_data = devm_kzalloc(dev, sizeof(*keymap_data), GFP_KERNEL);
-	if (!keymap_data) {
-		dev_err(dev, "could not allocate memory for keymap data\n");
-		return ERR_PTR(-ENOMEM);
-	}
+	int retval;
 
 	key_count = device_get_child_node_count(dev);
-	keymap = devm_kcalloc(dev, key_count, sizeof(*keymap), GFP_KERNEL);
-	if (!keymap) {
-		dev_err(dev, "could not allocate memory for keymap\n");
-		return ERR_PTR(-ENOMEM);
-	}
-
-	keymap_data->keymap_size = key_count;
-	keymap_data->keymap = keymap;
-
-	device_for_each_child_node(dev, child) {
-		u32 row, col, key_code;
-
-		fwnode_property_read_u32(child, "keypad,row", &row);
-		fwnode_property_read_u32(child, "keypad,column", &col);
-		fwnode_property_read_u32(child, "linux,code", &key_code);
-
-		*keymap++ = KEY(row, col, key_code);
-	}
-
-	return keymap_data;
-}
-
-static const struct samsung_keypad_platdata *
-samsung_keypad_parse_properties(struct device *dev)
-{
-	const struct matrix_keymap_data *keymap_data;
-	struct samsung_keypad_platdata *pdata;
-	u32 num_rows = 0, num_cols = 0;
-	int error;
+	if (key_count) {
+		keymap = kcalloc(key_count, sizeof(*keymap), GFP_KERNEL);
+		if (!keymap) {
+			dev_err(dev, "could not allocate memory for keymap\n");
+			return -ENOMEM;
+		}
 
-	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
-	if (!pdata) {
-		dev_err(dev, "could not allocate memory for platform data\n");
-		return ERR_PTR(-ENOMEM);
-	}
+		keymap_data.keymap = keymap;
+		keymap_data.keymap_size = key_count;
 
-	device_property_read_u32(dev, "samsung,keypad-num-rows", &num_rows);
-	device_property_read_u32(dev, "samsung,keypad-num-columns", &num_cols);
+		device_for_each_child_node(dev, child) {
+			u32 row, col, key_code;
 
-	error = matrix_keypad_parse_properties(dev, &num_rows, &num_cols);
-	if (error)
-		return ERR_PTR(error);
+			fwnode_property_read_u32(child, "keypad,row", &row);
+			fwnode_property_read_u32(child, "keypad,column", &col);
+			fwnode_property_read_u32(child, "linux,code", &key_code);
 
-	pdata->rows = num_rows;
-	pdata->cols = num_cols;
-
-	if (!device_property_present(dev, "linux,keymap")) {
-		keymap_data = samsung_parse_verbose_keymap(dev);
-		if (IS_ERR(keymap_data))
-			return ERR_CAST(keymap_data);
-
-		pdata->keymap_data = keymap_data;
+			*keymap++ = KEY(row, col, key_code);
+		}
 	}
 
-
-	pdata->no_autorepeat =
-		device_property_read_bool(dev, "linux,input-no-autorepeat");
-
-	pdata->wakeup = device_property_read_bool(dev, "wakeup-source") ||
-			/* legacy name */
-			device_property_read_bool(dev, "linux,input-wakeup");
-
-	return pdata;
+	retval = matrix_keypad_build_keymap(key_count ? &keymap_data : NULL,
+					    NULL, keypad->rows, keypad->cols,
+					    keypad->keycodes,
+					    keypad->input_dev);
+	kfree(keymap_data.keymap);
+	return retval;
 }
 
 static void samsung_disable_runtime_pm(void *data)
@@ -320,68 +282,39 @@ static void samsung_disable_runtime_pm(void *data)
 
 static int samsung_keypad_probe(struct platform_device *pdev)
 {
-	const struct samsung_keypad_platdata *pdata;
 	const struct platform_device_id *id;
+	struct device *dev = &pdev->dev;
 	struct samsung_keypad *keypad;
 	struct resource *res;
 	struct input_dev *input_dev;
 	unsigned int row_shift;
+	u32 num_rows = 0, num_cols = 0;
+	bool wakeup;
 	int error;
 
-	pdata = dev_get_platdata(&pdev->dev);
-	if (pdata) {
-		if (!pdata->keymap_data) {
-			dev_err(&pdev->dev, "no keymap data defined\n");
-			return -EINVAL;
-		}
-	} else {
-		pdata = samsung_keypad_parse_properties(&pdev->dev);
-		if (IS_ERR(pdata))
-			return PTR_ERR(pdata);
-	}
+	device_property_read_u32(dev, "samsung,keypad-num-rows", &num_rows);
+	device_property_read_u32(dev, "samsung,keypad-num-columns", &num_cols);
 
-	if (!pdata->rows || pdata->rows > SAMSUNG_MAX_ROWS)
-		return -EINVAL;
+	error = matrix_keypad_parse_properties(dev, &num_rows, &num_cols);
+	if (error)
+		return error;
 
-	if (!pdata->cols || pdata->cols > SAMSUNG_MAX_COLS)
+	if (num_rows > SAMSUNG_MAX_ROWS || num_cols > SAMSUNG_MAX_COLS)
 		return -EINVAL;
 
-	/* initialize the gpio */
-	if (pdata->cfg_gpio)
-		pdata->cfg_gpio(pdata->rows, pdata->cols);
-
-	row_shift = get_count_order(pdata->cols);
+	row_shift = get_count_order(num_cols);
 
 	keypad = devm_kzalloc(&pdev->dev,
 			      struct_size(keypad, keycodes,
-					  pdata->rows << row_shift),
+					  num_rows << row_shift),
 			      GFP_KERNEL);
 	if (!keypad)
 		return -ENOMEM;
 
-	input_dev = devm_input_allocate_device(&pdev->dev);
-	if (!input_dev)
-		return -ENOMEM;
-
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res)
-		return -ENODEV;
-
-	keypad->base = devm_ioremap(&pdev->dev, res->start, resource_size(res));
-	if (!keypad->base)
-		return -EBUSY;
-
-	keypad->clk = devm_clk_get_prepared(&pdev->dev, "keypad");
-	if (IS_ERR(keypad->clk)) {
-		dev_err(&pdev->dev, "failed to get keypad clk\n");
-		return PTR_ERR(keypad->clk);
-	}
-
-	keypad->input_dev = input_dev;
 	keypad->pdev = pdev;
 	keypad->row_shift = row_shift;
-	keypad->rows = pdata->rows;
-	keypad->cols = pdata->cols;
+	keypad->rows = num_rows;
+	keypad->cols = num_cols;
 	keypad->stopped = true;
 	init_waitqueue_head(&keypad->wait);
 
@@ -397,26 +330,45 @@ static int samsung_keypad_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
+	input_dev = devm_input_allocate_device(&pdev->dev);
+	if (!input_dev)
+		return -ENOMEM;
+
+	keypad->input_dev = input_dev;
+
 	input_dev->name = pdev->name;
 	input_dev->id.bustype = BUS_HOST;
 
 	input_dev->open = samsung_keypad_open;
 	input_dev->close = samsung_keypad_close;
 
-	error = matrix_keypad_build_keymap(pdata->keymap_data, NULL,
-					   pdata->rows, pdata->cols,
-					   keypad->keycodes, input_dev);
+	error = samsung_keypad_parse_keymap(keypad);
 	if (error) {
 		dev_err(&pdev->dev, "failed to build keymap\n");
 		return error;
 	}
 
 	input_set_capability(input_dev, EV_MSC, MSC_SCAN);
-	if (!pdata->no_autorepeat)
+
+	if (!device_property_read_bool(&pdev->dev, "linux,input-no-autorepeat"))
 		__set_bit(EV_REP, input_dev->evbit);
 
 	input_set_drvdata(input_dev, keypad);
 
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -ENODEV;
+
+	keypad->base = devm_ioremap(&pdev->dev, res->start, resource_size(res));
+	if (!keypad->base)
+		return -EBUSY;
+
+	keypad->clk = devm_clk_get_prepared(&pdev->dev, "keypad");
+	if (IS_ERR(keypad->clk)) {
+		dev_err(&pdev->dev, "failed to get keypad clk\n");
+		return PTR_ERR(keypad->clk);
+	}
+
 	keypad->irq = platform_get_irq(pdev, 0);
 	if (keypad->irq < 0) {
 		error = keypad->irq;
@@ -431,7 +383,11 @@ static int samsung_keypad_probe(struct platform_device *pdev)
 		return error;
 	}
 
-	device_init_wakeup(&pdev->dev, pdata->wakeup);
+	wakeup = device_property_read_bool(dev, "wakeup-source") ||
+		 /* legacy name */
+		 device_property_read_bool(dev, "linux,input-wakeup");
+	device_init_wakeup(&pdev->dev, wakeup);
+
 	platform_set_drvdata(pdev, keypad);
 
 	pm_runtime_enable(&pdev->dev);
@@ -444,12 +400,6 @@ static int samsung_keypad_probe(struct platform_device *pdev)
 	if (error)
 		return error;
 
-	if (!dev_get_platdata(&pdev->dev)) {
-		devm_kfree(&pdev->dev, (void *)pdata->keymap_data->keymap);
-		devm_kfree(&pdev->dev, (void *)pdata->keymap_data);
-		devm_kfree(&pdev->dev, (void *)pdata);
-	}
-
 	return 0;
 }
 
diff --git a/include/linux/input/samsung-keypad.h b/include/linux/input/samsung-keypad.h
deleted file mode 100644
index ab6b97114c08..000000000000
--- a/include/linux/input/samsung-keypad.h
+++ /dev/null
@@ -1,39 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-or-later */
-/*
- * Samsung Keypad platform data definitions
- *
- * Copyright (C) 2010 Samsung Electronics Co.Ltd
- * Author: Joonyoung Shim <jy0922.shim@samsung.com>
- */
-
-#ifndef __SAMSUNG_KEYPAD_H
-#define __SAMSUNG_KEYPAD_H
-
-#include <linux/input/matrix_keypad.h>
-
-#define SAMSUNG_MAX_ROWS	8
-#define SAMSUNG_MAX_COLS	8
-
-/**
- * struct samsung_keypad_platdata - Platform device data for Samsung Keypad.
- * @keymap_data: pointer to &matrix_keymap_data.
- * @rows: number of keypad row supported.
- * @cols: number of keypad col supported.
- * @no_autorepeat: disable key autorepeat.
- * @wakeup: controls whether the device should be set up as wakeup source.
- * @cfg_gpio: configure the GPIO.
- *
- * Initialisation data specific to either the machine or the platform
- * for the device driver to use or call-back when configuring gpio.
- */
-struct samsung_keypad_platdata {
-	const struct matrix_keymap_data	*keymap_data;
-	unsigned int rows;
-	unsigned int cols;
-	bool no_autorepeat;
-	bool wakeup;
-
-	void (*cfg_gpio)(unsigned int rows, unsigned int cols);
-};
-
-#endif /* __SAMSUNG_KEYPAD_H */
-- 
2.46.0.184.g6999bdac58-goog

[PATCH 13/14] ARM: s3c: crag6410: use software nodes/properties to set up GPIO keys

[Dmitry Torokhov]

Switch the gpio-keys device to use software inodes/properties to
describe the buttons and switches. This will allow dropping support
for platform data from the gpio-keys driver in the future.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 arch/arm/mach-s3c/gpio-samsung-s3c64xx.h |  5 ++
 arch/arm/mach-s3c/gpio-samsung.c         | 35 +++++++++++
 arch/arm/mach-s3c/mach-crag6410.c        | 80 +++++++++++++++++-------
 3 files changed, 99 insertions(+), 21 deletions(-)

diff --git a/arch/arm/mach-s3c/gpio-samsung-s3c64xx.h b/arch/arm/mach-s3c/gpio-samsung-s3c64xx.h
index 8ed144a0d474..7cf16cc085ea 100644
--- a/arch/arm/mach-s3c/gpio-samsung-s3c64xx.h
+++ b/arch/arm/mach-s3c/gpio-samsung-s3c64xx.h
@@ -13,6 +13,8 @@
 
 #ifdef CONFIG_GPIO_SAMSUNG
 
+#include <linux/property.h>
+
 /* GPIO bank sizes */
 #define S3C64XX_GPIO_A_NR	(8)
 #define S3C64XX_GPIO_B_NR	(7)
@@ -89,6 +91,9 @@ enum s3c_gpio_number {
 /* define the number of gpios we need to the one after the GPQ() range */
 #define GPIO_BOARD_START (S3C64XX_GPQ(S3C64XX_GPIO_Q_NR) + 1)
 
+extern const struct software_node samsung_gpiochip_nodes[];
+#define SAMSUNG_GPIO_NODE(node)	&samsung_gpiochip_nodes[(node) - 'A']
+
 #endif /* GPIO_SAMSUNG */
 #endif /* GPIO_SAMSUNG_S3C64XX_H */
 
diff --git a/arch/arm/mach-s3c/gpio-samsung.c b/arch/arm/mach-s3c/gpio-samsung.c
index 87daaa09e2c3..82f325112d30 100644
--- a/arch/arm/mach-s3c/gpio-samsung.c
+++ b/arch/arm/mach-s3c/gpio-samsung.c
@@ -21,6 +21,7 @@
 #include <linux/device.h>
 #include <linux/ioport.h>
 #include <linux/of.h>
+#include <linux/property.h>
 #include <linux/slab.h>
 #include <linux/of_address.h>
 
@@ -788,6 +789,38 @@ static struct samsung_gpio_chip s3c64xx_gpios_2bit[] = {
 	},
 };
 
+const struct software_node samsung_gpiochip_nodes[] = {
+	SOFTWARE_NODE("GPA", NULL, NULL),
+	SOFTWARE_NODE("GPB", NULL, NULL),
+	SOFTWARE_NODE("GPC", NULL, NULL),
+	SOFTWARE_NODE("GPD", NULL, NULL),
+	SOFTWARE_NODE("GPE", NULL, NULL),
+	SOFTWARE_NODE("GPF", NULL, NULL),
+	SOFTWARE_NODE("GPG", NULL, NULL),
+	SOFTWARE_NODE("GPH", NULL, NULL),
+	SOFTWARE_NODE("GPI", NULL, NULL),
+	SOFTWARE_NODE("GPJ", NULL, NULL),
+	SOFTWARE_NODE("GPK", NULL, NULL),
+	SOFTWARE_NODE("GPL", NULL, NULL),
+	SOFTWARE_NODE("GPM", NULL, NULL),
+	SOFTWARE_NODE("GPN", NULL, NULL),
+	SOFTWARE_NODE("GPO", NULL, NULL),
+	SOFTWARE_NODE("GPP", NULL, NULL),
+	SOFTWARE_NODE("GPQ", NULL, NULL),
+};
+#define NUM_SAMSUNG_GPIOCHIPS ARRAY_SIZE(samsung_gpiochip_nodes)
+
+static void __init samsung_setup_gpiochip_nodes(void)
+{
+	const struct software_node *group[NUM_SAMSUNG_GPIOCHIPS + 1] = { 0 };
+	int i;
+
+	for (i = 0; i < NUM_SAMSUNG_GPIOCHIPS; i++)
+		group[i] = &samsung_gpiochip_nodes[i];
+
+	software_node_register_node_group(group);
+}
+
 /* TODO: cleanup soc_is_* */
 static __init int samsung_gpiolib_init(void)
 {
@@ -811,6 +844,8 @@ static __init int samsung_gpiolib_init(void)
 				S3C64XX_VA_GPIO);
 		samsung_gpiolib_add_4bit2_chips(s3c64xx_gpios_4bit2,
 				ARRAY_SIZE(s3c64xx_gpios_4bit2));
+
+		samsung_setup_gpiochip_nodes();
 	}
 
 	return 0;
diff --git a/arch/arm/mach-s3c/mach-crag6410.c b/arch/arm/mach-s3c/mach-crag6410.c
index 16b6ef312aaf..6aa74db08af9 100644
--- a/arch/arm/mach-s3c/mach-crag6410.c
+++ b/arch/arm/mach-s3c/mach-crag6410.c
@@ -18,11 +18,13 @@
 #include <linux/input/matrix_keypad.h>
 #include <linux/gpio.h>
 #include <linux/gpio/machine.h>
+#include <linux/gpio/property.h>
 #include <linux/leds.h>
 #include <linux/delay.h>
 #include <linux/mmc/host.h>
 #include <linux/regulator/machine.h>
 #include <linux/regulator/fixed.h>
+#include <linux/property.h>
 #include <linux/pwm.h>
 #include <linux/pwm_backlight.h>
 #include <linux/dm9000.h>
@@ -227,32 +229,68 @@ static void __init crag6410_setup_keypad(void)
 		pr_err("failed to instantiate keypad device");
 }
 
-static struct gpio_keys_button crag6410_gpio_keys[] = {
-	[0] = {
-		.code	= KEY_SUSPEND,
-		.gpio	= S3C64XX_GPL(10),	/* EINT 18 */
-		.type	= EV_KEY,
-		.wakeup	= 1,
-		.active_low = 1,
-	},
-	[1] = {
-		.code	= SW_FRONT_PROXIMITY,
-		.gpio	= S3C64XX_GPN(11),	/* EINT 11 */
-		.type	= EV_SW,
-	},
+static const struct software_node crag6410_gpio_keys_node = {
+	.name = "crag6410-gpio-keys",
 };
 
-static struct gpio_keys_platform_data crag6410_gpio_keydata = {
-	.buttons	= crag6410_gpio_keys,
-	.nbuttons	= ARRAY_SIZE(crag6410_gpio_keys),
+static const struct property_entry crag6410_suspend_key_props[] = {
+	PROPERTY_ENTRY_U32("linux,code", KEY_SUSPEND),
+	PROPERTY_ENTRY_GPIO("gpios",
+			    SAMSUNG_GPIO_NODE('L'), 10,	/* EINT 18 */
+			    GPIO_ACTIVE_LOW),
+	PROPERTY_ENTRY_BOOL("wakeup-source"),
+	{ }
 };
 
-static struct platform_device crag6410_gpio_keydev = {
-	.name		= "gpio-keys",
-	.id		= 0,
-	.dev.platform_data = &crag6410_gpio_keydata,
+static const struct software_node crag6410_suspend_key_node = {
+	.parent = &crag6410_gpio_keys_node,
+	.properties = crag6410_suspend_key_props,
+};
+
+static const struct property_entry crag6410_prox_sw_props[] = {
+	PROPERTY_ENTRY_U32("linux,type", EV_SW),
+	PROPERTY_ENTRY_U32("linux,code", SW_FRONT_PROXIMITY),
+	PROPERTY_ENTRY_GPIO("gpios",
+			    SAMSUNG_GPIO_NODE('N'), 11,	/* EINT 11 */
+			    GPIO_ACTIVE_HIGH),
+	{ }
+};
+
+static const struct software_node crag6410_prox_sw_node = {
+	.parent = &crag6410_gpio_keys_node,
+	.properties = crag6410_prox_sw_props,
 };
 
+static const struct software_node *crag6410_gpio_keys_swnodes[] = {
+	&crag6410_gpio_keys_node,
+	&crag6410_suspend_key_node,
+	&crag6410_prox_sw_node,
+	NULL
+};
+
+static void __init crag6410_setup_gpio_keys(void)
+{
+	struct platform_device_info keys_info = {
+		.name	= "gpio-keys",
+		.id	= 0,
+	};
+	struct platform_device *pd;
+	int err;
+
+	err = software_node_register_node_group(crag6410_gpio_keys_swnodes);
+	if (err) {
+		pr_err("failed to register gpio-keys software nodes: %d\n", err);
+		return;
+	}
+
+	keys_info.fwnode = software_node_fwnode(&crag6410_gpio_keys_node);
+
+	pd = platform_device_register_full(&keys_info);
+	err = PTR_ERR_OR_ZERO(pd);
+	if (err)
+		pr_err("failed to create gpio-keys device: %d\n", err);
+}
+
 static struct resource crag6410_dm9k_resource[] = {
 	[0] = DEFINE_RES_MEM(S3C64XX_PA_XM0CSN5, 2),
 	[1] = DEFINE_RES_MEM(S3C64XX_PA_XM0CSN5 + (1 << 8), 2),
@@ -393,7 +431,6 @@ static struct platform_device *crag6410_devs0[] __initdata = {
 	&samsung_device_pwm,
 	&s3c64xx_device_iis0,
 	&s3c64xx_device_iis1,
-	&crag6410_gpio_keydev,
 };
 
 static struct platform_device *crag6410_devs1[] __initdata = {
@@ -897,6 +934,7 @@ static void __init crag6410_machine_init(void)
 	platform_add_devices(crag6410_devs0, ARRAY_SIZE(crag6410_devs0));
 
 	crag6410_setup_keypad();
+	crag6410_setup_gpio_keys();
 
 	platform_add_devices(crag6410_devs1, ARRAY_SIZE(crag6410_devs1));
 
-- 
2.46.0.184.g6999bdac58-goog

[PATCH 14/14] ARM: s3c: crag6410 - convert GPIO lookup tables to property entries

[Dmitry Torokhov]

Static property entries support defining GPIOs and are more similar to
device tree properties and are not prone to losing link between device
and a lookup table because of changes in device name. Convert the board
to use them.

This also fixes issue with recent conversion to GPIO descriptors
where GPIO lookup tables were specifying incorrect GPIO chip name
("GPIO<N>" vs "GP<N>").

Fixes: 10a366f36e2a ("ASoC: wm1250-ev1: Convert to GPIO descriptors")
Fixes: a45cf3cc72dd ("spi: s3c64xx: Convert to use GPIO descriptors")
Fixes: 9a5ed0bac86e ("regulator: wm831x: Convert to use GPIO descriptors")
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 arch/arm/mach-s3c/devs.c          |  35 ----------
 arch/arm/mach-s3c/devs.h          |   1 -
 arch/arm/mach-s3c/mach-crag6410.c | 108 ++++++++++++++++++------------
 3 files changed, 67 insertions(+), 77 deletions(-)

diff --git a/arch/arm/mach-s3c/devs.c b/arch/arm/mach-s3c/devs.c
index 31827cfc5700..e24967cc648b 100644
--- a/arch/arm/mach-s3c/devs.c
+++ b/arch/arm/mach-s3c/devs.c
@@ -336,38 +336,3 @@ void __init dwc2_hsotg_set_platdata(struct dwc2_hsotg_plat *pd)
 		npd->phy_exit = s3c_usb_phy_exit;
 }
 #endif /* CONFIG_S3C_DEV_USB_HSOTG */
-
-#ifdef CONFIG_S3C64XX_DEV_SPI0
-static struct resource s3c64xx_spi0_resource[] = {
-	[0] = DEFINE_RES_MEM(S3C_PA_SPI0, SZ_256),
-	[1] = DEFINE_RES_IRQ(IRQ_SPI0),
-};
-
-struct platform_device s3c64xx_device_spi0 = {
-	.name		= "s3c6410-spi",
-	.id		= 0,
-	.num_resources	= ARRAY_SIZE(s3c64xx_spi0_resource),
-	.resource	= s3c64xx_spi0_resource,
-	.dev = {
-		.dma_mask		= &samsung_device_dma_mask,
-		.coherent_dma_mask	= DMA_BIT_MASK(32),
-	},
-};
-
-void __init s3c64xx_spi0_set_platdata(int src_clk_nr, int num_cs)
-{
-	struct s3c64xx_spi_info pd;
-
-	/* Reject invalid configuration */
-	if (!num_cs || src_clk_nr < 0) {
-		pr_err("%s: Invalid SPI configuration\n", __func__);
-		return;
-	}
-
-	pd.num_cs = num_cs;
-	pd.src_clk_nr = src_clk_nr;
-	pd.cfg_gpio = s3c64xx_spi0_cfg_gpio;
-
-	s3c_set_platdata(&pd, sizeof(pd), &s3c64xx_device_spi0);
-}
-#endif /* CONFIG_S3C64XX_DEV_SPI0 */
diff --git a/arch/arm/mach-s3c/devs.h b/arch/arm/mach-s3c/devs.h
index 2737990063b1..90a86ade3570 100644
--- a/arch/arm/mach-s3c/devs.h
+++ b/arch/arm/mach-s3c/devs.h
@@ -27,7 +27,6 @@ extern struct platform_device *s3c24xx_uart_src[];
 
 extern struct platform_device s3c64xx_device_iis0;
 extern struct platform_device s3c64xx_device_iis1;
-extern struct platform_device s3c64xx_device_spi0;
 
 extern struct platform_device s3c_device_fb;
 extern struct platform_device s3c_device_hsmmc0;
diff --git a/arch/arm/mach-s3c/mach-crag6410.c b/arch/arm/mach-s3c/mach-crag6410.c
index 6aa74db08af9..a7a25239793e 100644
--- a/arch/arm/mach-s3c/mach-crag6410.c
+++ b/arch/arm/mach-s3c/mach-crag6410.c
@@ -435,7 +435,6 @@ static struct platform_device *crag6410_devs0[] __initdata = {
 
 static struct platform_device *crag6410_devs1[] __initdata = {
 	&crag6410_dm9k_device,
-	&s3c64xx_device_spi0,
 	&crag6410_mmgpio,
 	&crag6410_lcd_powerdev,
 	&crag6410_backlight_device,
@@ -654,22 +653,13 @@ static struct wm831x_pdata crag_pmic_pdata = {
 	.touch = &touch_pdata,
 };
 
-/*
- * VDDARM is eventually ending up as a regulator hanging on the MFD cell device
- * "wm831x-buckv.1" spawn from drivers/mfd/wm831x-core.c.
- *
- * From the note on the platform data we can see that this is clearly DVS1
- * and assigned as dcdc1 resource to the MFD core which sets .id of the cell
- * spawning the DVS1 platform device to 1, then the cell platform device
- * name is calculated from 10*instance + id resulting in the device name
- * "wm831x-buckv.11"
- */
-static struct gpiod_lookup_table crag_pmic_gpiod_table = {
-	.dev_id = "wm831x-buckv.11",
-	.table = {
-		GPIO_LOOKUP("GPIOK", 0, "dvs", GPIO_ACTIVE_HIGH),
-		{ },
-	},
+static const struct property_entry crag_pmic_properties[] = {
+	PROPERTY_ENTRY_GPIO("dvs-gpios",
+			    SAMSUNG_GPIO_NODE('K'), 0, GPIO_ACTIVE_HIGH),
+	{ }
+};
+static const struct software_node crag_pmic_swnode = {
+	.properties = crag_pmic_properties,
 };
 
 static struct i2c_board_info i2c_devs0[] = {
@@ -680,6 +670,7 @@ static struct i2c_board_info i2c_devs0[] = {
 	{ I2C_BOARD_INFO("wm8312", 0x34),
 	  .platform_data = &crag_pmic_pdata,
 	  .irq = S3C_EINT(23),
+	  .swnode = &crag_pmic_swnode,
 	},
 };
 
@@ -774,17 +765,22 @@ static struct wm831x_pdata glenfarclas_pmic_pdata = {
 	.disable_touch = true,
 };
 
-static struct gpiod_lookup_table crag_wm1250_ev1_gpiod_table = {
-	/* The WM1250-EV1 is device 0027 on I2C bus 1 */
-	.dev_id = "1-0027",
-	.table = {
-		GPIO_LOOKUP("GPION", 12, "clk-ena", GPIO_ACTIVE_HIGH),
-		GPIO_LOOKUP("GPIOL", 12, "clk-sel0", GPIO_ACTIVE_HIGH),
-		GPIO_LOOKUP("GPIOL", 13, "clk-sel1", GPIO_ACTIVE_HIGH),
-		GPIO_LOOKUP("GPIOL", 14, "osr", GPIO_ACTIVE_HIGH),
-		GPIO_LOOKUP("GPIOL", 8, "master", GPIO_ACTIVE_HIGH),
-		{ },
-	},
+static const struct property_entry crag_wm1250_ev1_properties[] = {
+	PROPERTY_ENTRY_GPIO("clk-ena-gpios",
+			    SAMSUNG_GPIO_NODE('N'), 12, GPIO_ACTIVE_HIGH),
+	PROPERTY_ENTRY_GPIO("clk-sel0-gpios",
+			    SAMSUNG_GPIO_NODE('L'), 12, GPIO_ACTIVE_HIGH),
+	PROPERTY_ENTRY_GPIO("clk-sel1-gpios",
+			    SAMSUNG_GPIO_NODE('L'), 13, GPIO_ACTIVE_HIGH),
+	PROPERTY_ENTRY_GPIO("osr-gpios",
+			    SAMSUNG_GPIO_NODE('L'), 14, GPIO_ACTIVE_HIGH),
+	PROPERTY_ENTRY_GPIO("master-gpios",
+			    SAMSUNG_GPIO_NODE('L'), 8, GPIO_ACTIVE_HIGH),
+	{ }
+};
+
+static const struct software_node crag_wm1250_ev1_swnode = {
+	.properties = crag_wm1250_ev1_properties,
 };
 
 static struct i2c_board_info i2c_devs1[] = {
@@ -797,7 +793,8 @@ static struct i2c_board_info i2c_devs1[] = {
 	{ I2C_BOARD_INFO("wlf-gf-module", 0x24) },
 	{ I2C_BOARD_INFO("wlf-gf-module", 0x25) },
 	{ I2C_BOARD_INFO("wlf-gf-module", 0x26) },
-	{ I2C_BOARD_INFO("wm1250-ev1", 0x27), },
+	{ I2C_BOARD_INFO("wm1250-ev1", 0x27),
+          .swnode = &crag_wm1250_ev1_swnode, },
 };
 
 static struct s3c2410_platform_i2c i2c1_pdata = {
@@ -887,15 +884,48 @@ static const struct gpio_led_platform_data gpio_leds_pdata = {
 
 static struct dwc2_hsotg_plat crag6410_hsotg_pdata;
 
-static struct gpiod_lookup_table crag_spi0_gpiod_table = {
-	.dev_id = "s3c6410-spi.0",
-	.table = {
-		GPIO_LOOKUP_IDX("GPIOC", 3, "cs", 0, GPIO_ACTIVE_LOW),
-		GPIO_LOOKUP_IDX("GPION", 5, "cs", 1, GPIO_ACTIVE_LOW),
-		{ },
-	},
+static const struct software_node_ref_args crag6410_spi0_gpio_refs[] = {
+	SOFTWARE_NODE_REFERENCE(SAMSUNG_GPIO_NODE('C'), 3, GPIO_ACTIVE_LOW),
+	SOFTWARE_NODE_REFERENCE(SAMSUNG_GPIO_NODE('N'), 5, GPIO_ACTIVE_LOW),
+};
+
+static const struct property_entry crag6410_spi0_properties[] __initconst = {
+	PROPERTY_ENTRY_REF_ARRAY("cs-gpios", crag6410_spi0_gpio_refs),
+	{ }
 };
 
+static const struct resource crag6410_spi0_resource[] __initconst = {
+	[0] = DEFINE_RES_MEM(S3C_PA_SPI0, SZ_256),
+	[1] = DEFINE_RES_IRQ(IRQ_SPI0),
+};
+
+static const struct s3c64xx_spi_info crag6410_spi0_platform_data __initconst = {
+	.num_cs = 2,
+	.cfg_gpio = s3c64xx_spi0_cfg_gpio,
+};
+
+static const struct platform_device_info crag6410_spi0_info __initconst = {
+	.name		= "s3c6410-spi",
+	.id		= 0,
+	.res		= crag6410_spi0_resource,
+	.num_res	= ARRAY_SIZE(crag6410_spi0_resource),
+	.data		= &crag6410_spi0_platform_data,
+	.size_data	= sizeof(crag6410_spi0_platform_data),
+	.dma_mask	= DMA_BIT_MASK(32),
+	.properties	= crag6410_spi0_properties,
+};
+
+static void __init crag6410_setup_spi0(void)
+{
+	struct platform_device *pd;
+	int err;
+
+	pd = platform_device_register_full(&crag6410_spi0_info);
+	err = PTR_ERR_OR_ZERO(pd);
+	if (err)
+		pr_err("failed to create spi0 device: %d\n", err);
+}
+
 static void __init crag6410_machine_init(void)
 {
 	/* Open drain IRQs need pullups */
@@ -922,19 +952,15 @@ static void __init crag6410_machine_init(void)
 	s3c_fb_set_platdata(&crag6410_lcd_pdata);
 	dwc2_hsotg_set_platdata(&crag6410_hsotg_pdata);
 
-	gpiod_add_lookup_table(&crag_pmic_gpiod_table);
 	i2c_register_board_info(0, i2c_devs0, ARRAY_SIZE(i2c_devs0));
-	gpiod_add_lookup_table(&crag_wm1250_ev1_gpiod_table);
 	i2c_register_board_info(1, i2c_devs1, ARRAY_SIZE(i2c_devs1));
 
-	gpiod_add_lookup_table(&crag_spi0_gpiod_table);
-	s3c64xx_spi0_set_platdata(0, 2);
-
 	pwm_add_table(crag6410_pwm_lookup, ARRAY_SIZE(crag6410_pwm_lookup));
 	platform_add_devices(crag6410_devs0, ARRAY_SIZE(crag6410_devs0));
 
 	crag6410_setup_keypad();
 	crag6410_setup_gpio_keys();
+	crag6410_setup_spi0();
 
 	platform_add_devices(crag6410_devs1, ARRAY_SIZE(crag6410_devs1));
 
-- 
2.46.0.184.g6999bdac58-goog

Re: [PATCH 00/14] Remove support for platform data from samsung keypad

[Arnd Bergmann]

On Mon, Aug 19, 2024, at 06:57, Dmitry Torokhov wrote:
>
> This series attempts to rework samsumg=keypad driver to stop using
> platform data and instead rely on generic device properties only.
>
> The first 8 patches are general cleanup/facelift patches.
>
> The 9th patch introduces alternative binding that is more compact that
> the original one, which makes it more suitable for use in legacy (non
> DT) boards with static device properties. Note that the "new" binding is
> the standard binding for matrix keypads.
>
> Patch #10 implements the new binding in the driver, #11 converts the
> only user of platform data in the mainline tree to the static device
> properties, and #12 drops support for platform data from the driver.
>
> Patches #13 and #14 are "bonus" converting the rest of crag6410 to use
> software nodes/properties to describe GPIO keys, LEDs and other
> peripherals. Note that I believe they fix and issue with recent
> conversion to GPIO lookup tables - the names of gpiochip structures I
> think are "GP<N>" ("GPK", "GPL", etc) and not "GPIO<N>".

I had a (brief) look at the patches, everything looks fine to
me, thanks for working on this! Let's see what Mark and 
Krzysztof think.

      Arnd

Re: [PATCH 01/14] Input: samsung-keypad - switch to using devm_clk_get_prepared()

[Krzysztof Kozlowski]

On Sun, Aug 18, 2024 at 09:57:58PM -0700, Dmitry Torokhov wrote:
> Switch to using devm_clk_get_prepared() instead of combining
> devm_clk_get() with clk_prepare(), which simplifies the code and
> ensures that the clock is unprepared at the right time relative to
> releasing other managed resources.

...

>  	device_init_wakeup(&pdev->dev, pdata->wakeup);
> @@ -439,20 +433,12 @@ static int samsung_keypad_probe(struct platform_device *pdev)
>  
>  err_disable_runtime_pm:
>  	pm_runtime_disable(&pdev->dev);
> -err_unprepare_clk:
> -	clk_unprepare(keypad->clk);
>  	return error;
>  }
>  
>  static void samsung_keypad_remove(struct platform_device *pdev)
>  {
> -	struct samsung_keypad *keypad = platform_get_drvdata(pdev);
> -
>  	pm_runtime_disable(&pdev->dev);
> -
> -	input_unregister_device(keypad->input_dev);

This looks unrelated.

Best regards,
Krzysztof

Re: [PATCH 02/14] Input: samsung-keypad - do not set input device's parent explicitly

[Krzysztof Kozlowski]

On Sun, Aug 18, 2024 at 09:57:59PM -0700, Dmitry Torokhov wrote:
> The driver uses devm_input_allocate_device() to allocate instances of
> input device, which sets the parent appropriately. Remove extraneous
> assignment.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH 03/14] Input: samsung-keypad - do not combine memory allocation checks

[Krzysztof Kozlowski]

On Sun, Aug 18, 2024 at 09:58:00PM -0700, Dmitry Torokhov wrote:
> The driver uses devm API to allocate resources so there's no reason
> to do allocations first and then check and release everything at once.
> 
> Check results immediately after doing allocation of each resource.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>  drivers/input/keyboard/samsung-keypad.c | 5 ++++-

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH 04/14] Input: samsung-keypad - use struct_size() helper

[Krzysztof Kozlowski]

On Sun, Aug 18, 2024 at 09:58:01PM -0700, Dmitry Torokhov wrote:
> When allocating memory for the keypad use struct_size() helper to be
> protected from overflows.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>  drivers/input/keyboard/samsung-keypad.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH 05/14] Input: samsung-keypad - use devm to disable runtime PM

[Krzysztof Kozlowski]

On Sun, Aug 18, 2024 at 09:58:02PM -0700, Dmitry Torokhov wrote:
> To make sure that runtime PM is disabled at the right time relative
> to all other devm-managed resources use devm_add_action_or_reset()
> to register a handler that will disable it.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>  drivers/input/keyboard/samsung-keypad.c | 24 +++++++++++++-----------
>  1 file changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/input/keyboard/samsung-keypad.c b/drivers/input/keyboard/samsung-keypad.c
> index 7adcd2657bca..924327de4d8f 100644
> --- a/drivers/input/keyboard/samsung-keypad.c
> +++ b/drivers/input/keyboard/samsung-keypad.c
> @@ -310,6 +310,13 @@ samsung_keypad_parse_dt(struct device *dev)
>  }
>  #endif
>  
> +static void samsung_disable_runtime_pm(void *data)
> +{
> +	struct samsung_keypad *keypad = data;
> +
> +	pm_runtime_disable(&keypad->pdev->dev);
> +}
> +
>  static int samsung_keypad_probe(struct platform_device *pdev)
>  {
>  	const struct samsung_keypad_platdata *pdata;
> @@ -420,11 +427,16 @@ static int samsung_keypad_probe(struct platform_device *pdev)
>  
>  	device_init_wakeup(&pdev->dev, pdata->wakeup);
>  	platform_set_drvdata(pdev, keypad);
> +
>  	pm_runtime_enable(&pdev->dev);
> +	error = devm_add_action_or_reset(&pdev->dev, samsung_disable_runtime_pm,
> +					 keypad);
> +	if (error)
> +		return error;

I think you are open-coding devm_pm_runtime_enable().

Re: [PATCH 07/14] Input: samsung-keypad - use per-chip parameters

[Krzysztof Kozlowski]

On Sun, Aug 18, 2024 at 09:58:04PM -0700, Dmitry Torokhov wrote:
> Instead of doing conditional logic based on the chip type, define
> per-chip parameter structure, and reference it in the match data
> (either of_device_id or platform_device_id).
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>  drivers/input/keyboard/samsung-keypad.c | 61 +++++++++++++++----------

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH 09/14] dt-bindings: input: samsung,s3c6410-keypad: introduce compact binding

[Krzysztof Kozlowski]

On Sun, Aug 18, 2024 at 09:58:06PM -0700, Dmitry Torokhov wrote:
> The binding with a sub-node per each key is very verbose and is hard to
> use with static device properties. Allow standard matrix keymap binding
> in addition to the verbose one.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>  .../input/samsung,s3c6410-keypad.yaml         | 57 ++++++++++++++++++-
>  1 file changed, 54 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/input/samsung,s3c6410-keypad.yaml b/Documentation/devicetree/bindings/input/samsung,s3c6410-keypad.yaml
> index a53569aa0ee7..28a318a0ff7e 100644
> --- a/Documentation/devicetree/bindings/input/samsung,s3c6410-keypad.yaml
> +++ b/Documentation/devicetree/bindings/input/samsung,s3c6410-keypad.yaml
> @@ -16,6 +16,10 @@ description:
>  maintainers:
>    - Krzysztof Kozlowski <krzk@kernel.org>
>  
> +allOf:
> +  - $ref: input.yaml#
> +  - $ref: matrix-keymap.yaml#
> +
>  properties:
>    compatible:
>      enum:
> @@ -37,6 +41,10 @@ properties:
>  
>    wakeup-source: true
>  
> +  keypad,num-columns: true
> +  keypad,num-rows: true
> +  linux,keymap: true
> +
>    linux,input-no-autorepeat:
>      type: boolean
>      description:
> @@ -81,12 +89,33 @@ patternProperties:
>        - keypad,row
>        - linux,code
>  
> +dependencies:
> +  linux,keymap:
> +    required:

Why "required" keyword? The dependencies should have just list of
properties. See example-schema.

> +      - keypad,num-columns
> +      - keypad,num-rows
> +
>  required:
>    - compatible
>    - reg
>    - interrupts
> -  - samsung,keypad-num-columns
> -  - samsung,keypad-num-rows
> +
> +if:

put allOf: here and this within allOf, so you the "if" could grow in the
future.

Best regards,
Krzysztof

Re: [PATCH 01/14] Input: samsung-keypad - switch to using devm_clk_get_prepared()

[Dmitry Torokhov]

On Mon, Aug 19, 2024 at 02:51:09PM +0200, Krzysztof Kozlowski wrote:
> On Sun, Aug 18, 2024 at 09:57:58PM -0700, Dmitry Torokhov wrote:
> > Switch to using devm_clk_get_prepared() instead of combining
> > devm_clk_get() with clk_prepare(), which simplifies the code and
> > ensures that the clock is unprepared at the right time relative to
> > releasing other managed resources.
> 
> ...
> 
> >  	device_init_wakeup(&pdev->dev, pdata->wakeup);
> > @@ -439,20 +433,12 @@ static int samsung_keypad_probe(struct platform_device *pdev)
> >  
> >  err_disable_runtime_pm:
> >  	pm_runtime_disable(&pdev->dev);
> > -err_unprepare_clk:
> > -	clk_unprepare(keypad->clk);
> >  	return error;
> >  }
> >  
> >  static void samsung_keypad_remove(struct platform_device *pdev)
> >  {
> > -	struct samsung_keypad *keypad = platform_get_drvdata(pdev);
> > -
> >  	pm_runtime_disable(&pdev->dev);
> > -
> > -	input_unregister_device(keypad->input_dev);
> 
> This looks unrelated.

It actually is related: with clk moved to devm we no longer need to
unregister input device by hand to keep the right ordering and we can
rely on devm to clean the input device as well (it already is being
allocated with devm_input_allocate_device()).

Thanks.

-- 
Dmitry

Re: [PATCH 05/14] Input: samsung-keypad - use devm to disable runtime PM

[Dmitry Torokhov]

On Mon, Aug 19, 2024 at 02:54:39PM +0200, Krzysztof Kozlowski wrote:
> On Sun, Aug 18, 2024 at 09:58:02PM -0700, Dmitry Torokhov wrote:
> > To make sure that runtime PM is disabled at the right time relative
> > to all other devm-managed resources use devm_add_action_or_reset()
> > to register a handler that will disable it.
> > 
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> >  drivers/input/keyboard/samsung-keypad.c | 24 +++++++++++++-----------
> >  1 file changed, 13 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/input/keyboard/samsung-keypad.c b/drivers/input/keyboard/samsung-keypad.c
> > index 7adcd2657bca..924327de4d8f 100644
> > --- a/drivers/input/keyboard/samsung-keypad.c
> > +++ b/drivers/input/keyboard/samsung-keypad.c
> > @@ -310,6 +310,13 @@ samsung_keypad_parse_dt(struct device *dev)
> >  }
> >  #endif
> >  
> > +static void samsung_disable_runtime_pm(void *data)
> > +{
> > +	struct samsung_keypad *keypad = data;
> > +
> > +	pm_runtime_disable(&keypad->pdev->dev);
> > +}
> > +
> >  static int samsung_keypad_probe(struct platform_device *pdev)
> >  {
> >  	const struct samsung_keypad_platdata *pdata;
> > @@ -420,11 +427,16 @@ static int samsung_keypad_probe(struct platform_device *pdev)
> >  
> >  	device_init_wakeup(&pdev->dev, pdata->wakeup);
> >  	platform_set_drvdata(pdev, keypad);
> > +
> >  	pm_runtime_enable(&pdev->dev);
> > +	error = devm_add_action_or_reset(&pdev->dev, samsung_disable_runtime_pm,
> > +					 keypad);
> > +	if (error)
> > +		return error;
> 
> I think you are open-coding devm_pm_runtime_enable().

Ah, I didn't realize we had it. Thanks!

-- 
Dmitry

Re: [PATCH 09/14] dt-bindings: input: samsung,s3c6410-keypad: introduce compact binding

[Dmitry Torokhov]

On Mon, Aug 19, 2024 at 03:02:07PM +0200, Krzysztof Kozlowski wrote:
> On Sun, Aug 18, 2024 at 09:58:06PM -0700, Dmitry Torokhov wrote:
> > The binding with a sub-node per each key is very verbose and is hard to
> > use with static device properties. Allow standard matrix keymap binding
> > in addition to the verbose one.
> > 
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> >  .../input/samsung,s3c6410-keypad.yaml         | 57 ++++++++++++++++++-
> >  1 file changed, 54 insertions(+), 3 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/input/samsung,s3c6410-keypad.yaml b/Documentation/devicetree/bindings/input/samsung,s3c6410-keypad.yaml
> > index a53569aa0ee7..28a318a0ff7e 100644
> > --- a/Documentation/devicetree/bindings/input/samsung,s3c6410-keypad.yaml
> > +++ b/Documentation/devicetree/bindings/input/samsung,s3c6410-keypad.yaml
> > @@ -16,6 +16,10 @@ description:
> >  maintainers:
> >    - Krzysztof Kozlowski <krzk@kernel.org>
> >  
> > +allOf:
> > +  - $ref: input.yaml#
> > +  - $ref: matrix-keymap.yaml#
> > +
> >  properties:
> >    compatible:
> >      enum:
> > @@ -37,6 +41,10 @@ properties:
> >  
> >    wakeup-source: true
> >  
> > +  keypad,num-columns: true
> > +  keypad,num-rows: true
> > +  linux,keymap: true
> > +
> >    linux,input-no-autorepeat:
> >      type: boolean
> >      description:
> > @@ -81,12 +89,33 @@ patternProperties:
> >        - keypad,row
> >        - linux,code
> >  
> > +dependencies:
> > +  linux,keymap:
> > +    required:
> 
> Why "required" keyword? The dependencies should have just list of
> properties. See example-schema.

OK, changed this to

dependencies:
  linux,keymap: [ "keypad,num-columns", "keypad,num-rows" ]
  
> 
> > +      - keypad,num-columns
> > +      - keypad,num-rows
> > +
> >  required:
> >    - compatible
> >    - reg
> >    - interrupts
> > -  - samsung,keypad-num-columns
> > -  - samsung,keypad-num-rows
> > +
> > +if:
> 
> put allOf: here and this within allOf, so you the "if" could grow in the
> future.

Hmm, there is already "allOf" at the beginning of the file, so adding
another one results in complaints about duplicate "allOf". I can move it
all to the top, like this:

allOf:
  - $ref: input.yaml#
  - $ref: matrix-keymap.yaml#
  - if:
      required:
        - linux,keymap
    then:
      properties:
        samsung,keypad-num-columns: false
        samsung,keypad-num-rows: false
      patternProperties:
        '^key-[0-9a-z]+$': false
    else:
      properties:
        keypad,num-columns: false
        keypad,num-rows: false
      required:
        - samsung,keypad-num-columns
        - samsung,keypad-num-rows

Is this OK? I don't quite like that "tweaks" are listed before main
body of properties.

Thanks.

-- 
Dmitry

Re: [PATCH 09/14] dt-bindings: input: samsung,s3c6410-keypad: introduce compact binding

[Conor Dooley]

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

On Mon, Aug 19, 2024 at 08:49:10AM -0700, Dmitry Torokhov wrote:
> On Mon, Aug 19, 2024 at 03:02:07PM +0200, Krzysztof Kozlowski wrote:
> > On Sun, Aug 18, 2024 at 09:58:06PM -0700, Dmitry Torokhov wrote:

> > 
> > > +      - keypad,num-columns
> > > +      - keypad,num-rows
> > > +
> > >  required:
> > >    - compatible
> > >    - reg
> > >    - interrupts
> > > -  - samsung,keypad-num-columns
> > > -  - samsung,keypad-num-rows
> > > +
> > > +if:
> > 
> > put allOf: here and this within allOf, so you the "if" could grow in the
> > future.
> 
> Hmm, there is already "allOf" at the beginning of the file, so adding
> another one results in complaints about duplicate "allOf". I can move it
> all to the top, like this:
> 
> allOf:
>   - $ref: input.yaml#
>   - $ref: matrix-keymap.yaml#
>   - if:
>       required:
>         - linux,keymap
>     then:
>       properties:
>         samsung,keypad-num-columns: false
>         samsung,keypad-num-rows: false
>       patternProperties:
>         '^key-[0-9a-z]+$': false
>     else:
>       properties:
>         keypad,num-columns: false
>         keypad,num-rows: false
>       required:
>         - samsung,keypad-num-columns
>         - samsung,keypad-num-rows
> 
> Is this OK? I don't quite like that "tweaks" are listed before main
> body of properties.

The normal thing to do is to put the allOf at the end, not the start, in
cases like this, for the reason you mention.

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

Re: [PATCH 09/14] dt-bindings: input: samsung,s3c6410-keypad: introduce compact binding

[Dmitry Torokhov]

On Mon, Aug 19, 2024 at 05:48:06PM +0100, Conor Dooley wrote:
> On Mon, Aug 19, 2024 at 08:49:10AM -0700, Dmitry Torokhov wrote:
> > On Mon, Aug 19, 2024 at 03:02:07PM +0200, Krzysztof Kozlowski wrote:
> > > On Sun, Aug 18, 2024 at 09:58:06PM -0700, Dmitry Torokhov wrote:
> 
> > > 
> > > > +      - keypad,num-columns
> > > > +      - keypad,num-rows
> > > > +
> > > >  required:
> > > >    - compatible
> > > >    - reg
> > > >    - interrupts
> > > > -  - samsung,keypad-num-columns
> > > > -  - samsung,keypad-num-rows
> > > > +
> > > > +if:
> > > 
> > > put allOf: here and this within allOf, so you the "if" could grow in the
> > > future.
> > 
> > Hmm, there is already "allOf" at the beginning of the file, so adding
> > another one results in complaints about duplicate "allOf". I can move it
> > all to the top, like this:
> > 
> > allOf:
> >   - $ref: input.yaml#
> >   - $ref: matrix-keymap.yaml#
> >   - if:
> >       required:
> >         - linux,keymap
> >     then:
> >       properties:
> >         samsung,keypad-num-columns: false
> >         samsung,keypad-num-rows: false
> >       patternProperties:
> >         '^key-[0-9a-z]+$': false
> >     else:
> >       properties:
> >         keypad,num-columns: false
> >         keypad,num-rows: false
> >       required:
> >         - samsung,keypad-num-columns
> >         - samsung,keypad-num-rows
> > 
> > Is this OK? I don't quite like that "tweaks" are listed before main
> > body of properties.
> 
> The normal thing to do is to put the allOf at the end, not the start, in
> cases like this, for the reason you mention.

I see, thanks. It would be nice if it could combine several "allOf"s
into one internally.

Thanks.

-- 
Dmitry

Re: [PATCH 11/14] ARM: s3c: crag6410: switch keypad device to software properties

[Krzysztof Kozlowski]

On Sun, Aug 18, 2024 at 09:58:08PM -0700, Dmitry Torokhov wrote:
> Switch the keypad device to use software properties to describe the
> keypad. This will allow dropping support for platform data from the
> samsung-keypad driver.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>  arch/arm/mach-s3c/Kconfig                |  5 ---
>  arch/arm/mach-s3c/Kconfig.s3c64xx        |  1 -
>  arch/arm/mach-s3c/Makefile.s3c64xx       |  1 -
>  arch/arm/mach-s3c/devs.c                 | 27 ------------
>  arch/arm/mach-s3c/devs.h                 |  1 -
>  arch/arm/mach-s3c/keypad.h               | 27 ------------
>  arch/arm/mach-s3c/mach-crag6410.c        | 56 ++++++++++++++++++------
>  arch/arm/mach-s3c/setup-keypad-s3c64xx.c | 20 ---------
>  8 files changed, 42 insertions(+), 96 deletions(-)
>  delete mode 100644 arch/arm/mach-s3c/keypad.h
>  delete mode 100644 arch/arm/mach-s3c/setup-keypad-s3c64xx.c

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH 06/14] Input: samsung-keypad - use guard notation to acquire mutex

[Krzysztof Kozlowski]

On Sun, Aug 18, 2024 at 09:58:03PM -0700, Dmitry Torokhov wrote:
> Guard notation is more compact and ensures that the mutex will be
> released when control leaves the function.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>  drivers/input/keyboard/samsung-keypad.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 

You need to include cleanup.h (unless some other patch already did it
and I missed it?)

Best regards,
Krzysztof

Re: [PATCH 14/14] ARM: s3c: crag6410 - convert GPIO lookup tables to property entries

[Krzysztof Kozlowski]

On Sun, Aug 18, 2024 at 09:58:11PM -0700, Dmitry Torokhov wrote:
> Static property entries support defining GPIOs and are more similar to
> device tree properties and are not prone to losing link between device
> and a lookup table because of changes in device name. Convert the board
> to use them.
> 
> This also fixes issue with recent conversion to GPIO descriptors
> where GPIO lookup tables were specifying incorrect GPIO chip name
> ("GPIO<N>" vs "GP<N>").
> 
> Fixes: 10a366f36e2a ("ASoC: wm1250-ev1: Convert to GPIO descriptors")
> Fixes: a45cf3cc72dd ("spi: s3c64xx: Convert to use GPIO descriptors")
> Fixes: 9a5ed0bac86e ("regulator: wm831x: Convert to use GPIO descriptors")
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>  arch/arm/mach-s3c/devs.c          |  35 ----------
>  arch/arm/mach-s3c/devs.h          |   1 -
>  arch/arm/mach-s3c/mach-crag6410.c | 108 ++++++++++++++++++------------
>  3 files changed, 67 insertions(+), 77 deletions(-)

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH 13/14] ARM: s3c: crag6410: use software nodes/properties to set up GPIO keys

[Krzysztof Kozlowski]

On Sun, Aug 18, 2024 at 09:58:10PM -0700, Dmitry Torokhov wrote:
> Switch the gpio-keys device to use software inodes/properties to
> describe the buttons and switches. This will allow dropping support
> for platform data from the gpio-keys driver in the future.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>  arch/arm/mach-s3c/gpio-samsung-s3c64xx.h |  5 ++
>  arch/arm/mach-s3c/gpio-samsung.c         | 35 +++++++++++
>  arch/arm/mach-s3c/mach-crag6410.c        | 80 +++++++++++++++++-------
>  3 files changed, 99 insertions(+), 21 deletions(-)

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH 06/14] Input: samsung-keypad - use guard notation to acquire mutex

[Dmitry Torokhov]

On Thu, Aug 22, 2024 at 05:48:33PM +0200, Krzysztof Kozlowski wrote:
> On Sun, Aug 18, 2024 at 09:58:03PM -0700, Dmitry Torokhov wrote:
> > Guard notation is more compact and ensures that the mutex will be
> > released when control leaves the function.
> > 
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> >  drivers/input/keyboard/samsung-keypad.c | 8 ++------
> >  1 file changed, 2 insertions(+), 6 deletions(-)
> > 
> 
> You need to include cleanup.h (unless some other patch already did it
> and I missed it?)

Guard for mutexes defined in mutex.h which is pulled in indirectly, and
cleanup.h is included there.

If we want to list all the headers that we need instead of relying on
indirect inclusions I think we need a separate patch. But even then I
wonder if things like cleanup.h should be included by drivers
directly...

Thanks.

-- 
Dmitry

Re: [PATCH 06/14] Input: samsung-keypad - use guard notation to acquire mutex

[Krzysztof Kozlowski]

On 22/08/2024 20:07, Dmitry Torokhov wrote:
> On Thu, Aug 22, 2024 at 05:48:33PM +0200, Krzysztof Kozlowski wrote:
>> On Sun, Aug 18, 2024 at 09:58:03PM -0700, Dmitry Torokhov wrote:
>>> Guard notation is more compact and ensures that the mutex will be
>>> released when control leaves the function.
>>>
>>> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>>> ---
>>>  drivers/input/keyboard/samsung-keypad.c | 8 ++------
>>>  1 file changed, 2 insertions(+), 6 deletions(-)
>>>
>>
>> You need to include cleanup.h (unless some other patch already did it
>> and I missed it?)
> 
> Guard for mutexes defined in mutex.h which is pulled in indirectly, and

guard() is not in mutex.h and in general we are including headers for
the things directly used in the unit.

> cleanup.h is included there.
> 
> If we want to list all the headers that we need instead of relying on

Fixing existing code is another thing. I only propose to add new code
with the header.

I don't have strong opinion anyway, so:

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH 06/14] Input: samsung-keypad - use guard notation to acquire mutex

[Dmitry Torokhov]

On Fri, Aug 23, 2024 at 08:06:17AM +0200, Krzysztof Kozlowski wrote:
> On 22/08/2024 20:07, Dmitry Torokhov wrote:
> > On Thu, Aug 22, 2024 at 05:48:33PM +0200, Krzysztof Kozlowski wrote:
> >> On Sun, Aug 18, 2024 at 09:58:03PM -0700, Dmitry Torokhov wrote:
> >>> Guard notation is more compact and ensures that the mutex will be
> >>> released when control leaves the function.
> >>>
> >>> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> >>> ---
> >>>  drivers/input/keyboard/samsung-keypad.c | 8 ++------
> >>>  1 file changed, 2 insertions(+), 6 deletions(-)
> >>>
> >>
> >> You need to include cleanup.h (unless some other patch already did it
> >> and I missed it?)
> > 
> > Guard for mutexes defined in mutex.h which is pulled in indirectly, and
> 
> guard() is not in mutex.h and in general we are including headers for
> the things directly used in the unit.

Oh, but it is:

https://elixir.bootlin.com/linux/v6.10/source/include/linux/mutex.h#L196

DEFINE_GUARD(mutex, struct mutex *, mutex_lock(_T), mutex_unlock(_T))

> 
> > cleanup.h is included there.
> > 
> > If we want to list all the headers that we need instead of relying on
> 
> Fixing existing code is another thing. I only propose to add new code
> with the header.
> 
> I don't have strong opinion anyway, so:
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Thanks!

-- 
Dmitry

Re: [PATCH 06/14] Input: samsung-keypad - use guard notation to acquire mutex

[Krzysztof Kozlowski]

On 23/08/2024 10:32, Dmitry Torokhov wrote:
> On Fri, Aug 23, 2024 at 08:06:17AM +0200, Krzysztof Kozlowski wrote:
>> On 22/08/2024 20:07, Dmitry Torokhov wrote:
>>> On Thu, Aug 22, 2024 at 05:48:33PM +0200, Krzysztof Kozlowski wrote:
>>>> On Sun, Aug 18, 2024 at 09:58:03PM -0700, Dmitry Torokhov wrote:
>>>>> Guard notation is more compact and ensures that the mutex will be
>>>>> released when control leaves the function.
>>>>>
>>>>> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>>>>> ---
>>>>>  drivers/input/keyboard/samsung-keypad.c | 8 ++------
>>>>>  1 file changed, 2 insertions(+), 6 deletions(-)
>>>>>
>>>>
>>>> You need to include cleanup.h (unless some other patch already did it
>>>> and I missed it?)
>>>
>>> Guard for mutexes defined in mutex.h which is pulled in indirectly, and
>>
>> guard() is not in mutex.h and in general we are including headers for
>> the things directly used in the unit.
> 
> Oh, but it is:
> 
> https://elixir.bootlin.com/linux/v6.10/source/include/linux/mutex.h#L196
> 
> DEFINE_GUARD(mutex, struct mutex *, mutex_lock(_T), mutex_unlock(_T))

That's DEFINE_GUARD, not guard().

Best regards,
Krzysztof

Re: [PATCH 06/14] Input: samsung-keypad - use guard notation to acquire mutex

[Dmitry Torokhov]

On Fri, Aug 23, 2024 at 10:52:54AM +0200, Krzysztof Kozlowski wrote:
> On 23/08/2024 10:32, Dmitry Torokhov wrote:
> > On Fri, Aug 23, 2024 at 08:06:17AM +0200, Krzysztof Kozlowski wrote:
> >> On 22/08/2024 20:07, Dmitry Torokhov wrote:
> >>> On Thu, Aug 22, 2024 at 05:48:33PM +0200, Krzysztof Kozlowski wrote:
> >>>> On Sun, Aug 18, 2024 at 09:58:03PM -0700, Dmitry Torokhov wrote:
> >>>>> Guard notation is more compact and ensures that the mutex will be
> >>>>> released when control leaves the function.
> >>>>>
> >>>>> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> >>>>> ---
> >>>>>  drivers/input/keyboard/samsung-keypad.c | 8 ++------
> >>>>>  1 file changed, 2 insertions(+), 6 deletions(-)
> >>>>>
> >>>>
> >>>> You need to include cleanup.h (unless some other patch already did it
> >>>> and I missed it?)
> >>>
> >>> Guard for mutexes defined in mutex.h which is pulled in indirectly, and
> >>
> >> guard() is not in mutex.h and in general we are including headers for
> >> the things directly used in the unit.
> > 
> > Oh, but it is:
> > 
> > https://elixir.bootlin.com/linux/v6.10/source/include/linux/mutex.h#L196
> > 
> > DEFINE_GUARD(mutex, struct mutex *, mutex_lock(_T), mutex_unlock(_T))
> 
> That's DEFINE_GUARD, not guard().

OK, I see what you mean. I believe that cleanup.h is too low level to
include directly by anyone who does not actually define a new guard or
destructor routine. The concrete implementations of needed guards and
all necessary support is pulled together with the headers defining the
primary objects (mutex, spinlock, etc).

Thanks.

-- 
Dmitry

Re: [PATCH 00/14] Remove support for platform data from samsung keypad

[Dmitry Torokhov]

On Mon, Aug 19, 2024 at 11:40:56AM +0200, Arnd Bergmann wrote:
> On Mon, Aug 19, 2024, at 06:57, Dmitry Torokhov wrote:
> >
> > This series attempts to rework samsumg=keypad driver to stop using
> > platform data and instead rely on generic device properties only.
> >
> > The first 8 patches are general cleanup/facelift patches.
> >
> > The 9th patch introduces alternative binding that is more compact that
> > the original one, which makes it more suitable for use in legacy (non
> > DT) boards with static device properties. Note that the "new" binding is
> > the standard binding for matrix keypads.
> >
> > Patch #10 implements the new binding in the driver, #11 converts the
> > only user of platform data in the mainline tree to the static device
> > properties, and #12 drops support for platform data from the driver.
> >
> > Patches #13 and #14 are "bonus" converting the rest of crag6410 to use
> > software nodes/properties to describe GPIO keys, LEDs and other
> > peripherals. Note that I believe they fix and issue with recent
> > conversion to GPIO lookup tables - the names of gpiochip structures I
> > think are "GP<N>" ("GPK", "GPL", etc) and not "GPIO<N>".
> 
> I had a (brief) look at the patches, everything looks fine to
> me, thanks for working on this! Let's see what Mark and 
> Krzysztof think.

Mark, were you able to give this a spin? Or should I address the DT
binding comments from Krzysztof and Connor and resent for merge?

Thanks.

-- 
Dmitry

Re: [PATCH 00/14] Remove support for platform data from samsung keypad

[Mark Brown]

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

On Fri, Oct 04, 2024 at 06:38:01AM -0700, Dmitry Torokhov wrote:
> On Mon, Aug 19, 2024 at 11:40:56AM +0200, Arnd Bergmann wrote:

> > I had a (brief) look at the patches, everything looks fine to
> > me, thanks for working on this! Let's see what Mark and 
> > Krzysztof think.

> Mark, were you able to give this a spin? Or should I address the DT
> binding comments from Krzysztof and Connor and resent for merge?

Sorry, no - I've been some combination of ill, travelling and super busy
since you posted it (sadly the bootloader for the board only supports
booting from SD card so until I port u-boot it's not in my board farm
and I need to specifically set it up on my desk whenver I want to do
anything with it, especially for this where I need to poke at the
keypad).  I've got one more week of travel next week and one of the
major sources of super busy just got better so I'm reasonably hopeful
I'll get to it week of the 14th but probably worth respinning.

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