Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH] Input: mtk-pmic-keys: Count available keys during probe instead of pre-counting
@ 2026-05-28 23:56 Rosen Penev
  2026-05-29  0:29 ` sashiko-bot
  0 siblings, 1 reply; 2+ messages in thread
From: Rosen Penev @ 2026-05-28 23:56 UTC (permalink / raw)
  To: linux-input
  Cc: Dmitry Torokhov, Matthias Brugger, AngeloGioacchino Del Regno,
	open list:ARM/Mediatek SoC support,
	moderated list:ARM/Mediatek SoC support,
	moderated list:ARM/Mediatek SoC support

Replace the separate of_get_available_child_count() pre-count and
validation step with a single pass through for_each_child_of_node_scoped().
Skip unavailable child nodes and bail out if more than
MTK_PMIC_MAX_KEY_COUNT available keys are found. Set nkeys after the
loop so suspend/resume iterate only over initialized entries.

Also use a key variable in the loop for clarity.

Use of_device_get_match_data() to fetch the PMIC key register data directly
instead of open-coding an of_match_device() lookup.

This also lets the driver drop the of_device.h include.

Assisted-by: OpenCode:BigPickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/input/keyboard/mtk-pmic-keys.c | 53 ++++++++++++--------------
 1 file changed, 24 insertions(+), 29 deletions(-)

diff --git a/drivers/input/keyboard/mtk-pmic-keys.c b/drivers/input/keyboard/mtk-pmic-keys.c
index c78d9f6d97c4..e34856693ee2 100644
--- a/drivers/input/keyboard/mtk-pmic-keys.c
+++ b/drivers/input/keyboard/mtk-pmic-keys.c
@@ -16,7 +16,6 @@
 #include <linux/mfd/mt6397/core.h>
 #include <linux/mfd/mt6397/registers.h>
 #include <linux/module.h>
-#include <linux/of_device.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
@@ -147,6 +146,7 @@ struct mtk_pmic_keys {
 	struct input_dev *input_dev;
 	struct device *dev;
 	struct regmap *regmap;
+	unsigned int nkeys;
 	struct mtk_pmic_keys_info keys[MTK_PMIC_MAX_KEY_COUNT];
 };
 
@@ -267,7 +267,7 @@ static int mtk_pmic_keys_suspend(struct device *dev)
 	struct mtk_pmic_keys *keys = dev_get_drvdata(dev);
 	int index;
 
-	for (index = 0; index < MTK_PMIC_MAX_KEY_COUNT; index++) {
+	for (index = 0; index < keys->nkeys; index++) {
 		if (keys->keys[index].wakeup) {
 			enable_irq_wake(keys->keys[index].irq);
 			if (keys->keys[index].irq_r > 0)
@@ -283,7 +283,7 @@ static int mtk_pmic_keys_resume(struct device *dev)
 	struct mtk_pmic_keys *keys = dev_get_drvdata(dev);
 	int index;
 
-	for (index = 0; index < MTK_PMIC_MAX_KEY_COUNT; index++) {
+	for (index = 0; index < keys->nkeys; index++) {
 		if (keys->keys[index].wakeup) {
 			disable_irq_wake(keys->keys[index].irq);
 			if (keys->keys[index].irq_r > 0)
@@ -325,24 +325,23 @@ MODULE_DEVICE_TABLE(of, of_mtk_pmic_keys_match_tbl);
 static int mtk_pmic_keys_probe(struct platform_device *pdev)
 {
 	int error, index = 0;
-	unsigned int keycount;
 	struct mt6397_chip *pmic_chip = dev_get_drvdata(pdev->dev.parent);
 	struct device_node *node = pdev->dev.of_node;
 	static const char *const irqnames[] = { "powerkey", "homekey" };
 	static const char *const irqnames_r[] = { "powerkey_r", "homekey_r" };
 	struct mtk_pmic_keys *keys;
 	const struct mtk_pmic_regs *mtk_pmic_regs;
+	struct mtk_pmic_keys_info *key;
 	struct input_dev *input_dev;
-	const struct of_device_id *of_id =
-		of_match_device(of_mtk_pmic_keys_match_tbl, &pdev->dev);
 
 	keys = devm_kzalloc(&pdev->dev, sizeof(*keys), GFP_KERNEL);
 	if (!keys)
 		return -ENOMEM;
-
 	keys->dev = &pdev->dev;
 	keys->regmap = pmic_chip->regmap;
-	mtk_pmic_regs = of_id->data;
+	mtk_pmic_regs = of_device_get_match_data(&pdev->dev);
+	if (!mtk_pmic_regs)
+		return -EINVAL;
 
 	keys->input_dev = input_dev = devm_input_allocate_device(keys->dev);
 	if (!input_dev) {
@@ -356,31 +355,26 @@ static int mtk_pmic_keys_probe(struct platform_device *pdev)
 	input_dev->id.product = 0x0001;
 	input_dev->id.version = 0x0001;
 
-	keycount = of_get_available_child_count(node);
-	if (keycount > MTK_PMIC_MAX_KEY_COUNT ||
-	    keycount > ARRAY_SIZE(irqnames)) {
-		dev_err(keys->dev, "too many keys defined (%d)\n", keycount);
-		return -EINVAL;
-	}
+	for_each_available_child_of_node_scoped(node, child) {
+		if (index >= MTK_PMIC_MAX_KEY_COUNT) {
+			dev_err(&pdev->dev, "too many keys defined\n");
+			return -EINVAL;
+		}
 
-	for_each_child_of_node_scoped(node, child) {
-		keys->keys[index].regs = &mtk_pmic_regs->keys_regs[index];
+		key = &keys->keys[index];
+		key->regs = &mtk_pmic_regs->keys_regs[index];
 
-		keys->keys[index].irq =
-			platform_get_irq_byname(pdev, irqnames[index]);
-		if (keys->keys[index].irq < 0)
-			return keys->keys[index].irq;
+		key->irq = platform_get_irq_byname(pdev, irqnames[index]);
+		if (key->irq < 0)
+			return key->irq;
 
 		if (mtk_pmic_regs->key_release_irq) {
-			keys->keys[index].irq_r = platform_get_irq_byname(pdev,
-									  irqnames_r[index]);
-
-			if (keys->keys[index].irq_r < 0)
-				return keys->keys[index].irq_r;
+			key->irq_r = platform_get_irq_byname(pdev, irqnames_r[index]);
+			if (key->irq_r < 0)
+				return key->irq_r;
 		}
 
-		error = of_property_read_u32(child,
-			"linux,keycodes", &keys->keys[index].keycode);
+		error = of_property_read_u32(child, "linux,keycodes", &key->keycode);
 		if (error) {
 			dev_err(keys->dev,
 				"failed to read key:%d linux,keycode property: %d\n",
@@ -389,14 +383,15 @@ static int mtk_pmic_keys_probe(struct platform_device *pdev)
 		}
 
 		if (of_property_read_bool(child, "wakeup-source"))
-			keys->keys[index].wakeup = true;
+			key->wakeup = true;
 
-		error = mtk_pmic_key_setup(keys, &keys->keys[index]);
+		error = mtk_pmic_key_setup(keys, key);
 		if (error)
 			return error;
 
 		index++;
 	}
+	keys->nkeys = index;
 
 	error = input_register_device(input_dev);
 	if (error) {
-- 
2.54.0


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

* Re: [PATCH] Input: mtk-pmic-keys: Count available keys during probe instead of pre-counting
  2026-05-28 23:56 [PATCH] Input: mtk-pmic-keys: Count available keys during probe instead of pre-counting Rosen Penev
@ 2026-05-29  0:29 ` sashiko-bot
  0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-05-29  0:29 UTC (permalink / raw)
  To: Rosen Penev; +Cc: dmitry.torokhov, linux-input

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Skipping unavailable device tree child nodes shifts the loop index, breaking the implicit hardware mapping and incorrectly assigning registers and IRQs.
--

commit ddd98727b5f1967e1232c34c0d42b4d0d314c947
Author: Rosen Penev <rosenp@gmail.com>

Input: mtk-pmic-keys: Count available keys during probe instead of pre-counting

This commit replaces the separate pre-count of available keys with a
single pass using for_each_available_child_of_node_scoped(). It skips
unavailable nodes and bails out if the key count exceeds the maximum.
The match data lookup is also updated to use of_device_get_match_data().

> diff --git a/drivers/input/keyboard/mtk-pmic-keys.c b/drivers/input/keyboard/mtk-pmic-keys.c
> index c78d9f6d97c4f..e34856693ee2c 100644
> --- a/drivers/input/keyboard/mtk-pmic-keys.c
> +++ b/drivers/input/keyboard/mtk-pmic-keys.c
[ ... ]
> @@ -356,31 +355,26 @@ static int mtk_pmic_keys_probe(struct platform_device *pdev)
>  	input_dev->id.product = 0x0001;
>  	input_dev->id.version = 0x0001;
>  
> -	keycount = of_get_available_child_count(node);
> -	if (keycount > MTK_PMIC_MAX_KEY_COUNT ||
> -	    keycount > ARRAY_SIZE(irqnames)) {
> -		dev_err(keys->dev, "too many keys defined (%d)\n", keycount);
> -		return -EINVAL;
> -	}
> +	for_each_available_child_of_node_scoped(node, child) {
> +		if (index >= MTK_PMIC_MAX_KEY_COUNT) {
> +			dev_err(&pdev->dev, "too many keys defined\n");
> +			return -EINVAL;
> +		}
>  
> -	for_each_child_of_node_scoped(node, child) {
> -		keys->keys[index].regs = &mtk_pmic_regs->keys_regs[index];
> +		key = &keys->keys[index];
> +		key->regs = &mtk_pmic_regs->keys_regs[index];
>  
> -		keys->keys[index].irq =
> -			platform_get_irq_byname(pdev, irqnames[index]);
> -		if (keys->keys[index].irq < 0)
> -			return keys->keys[index].irq;
> +		key->irq = platform_get_irq_byname(pdev, irqnames[index]);

[Severity: High]
Does switching to for_each_available_child_of_node_scoped() break the
strict index mapping for hardware registers and IRQs?

The original code iterated over all child nodes, preserving the device tree
order index (e.g., index 0 for powerkey, index 1 for homekey).

If a device tree has a disabled node first, skipping it will shift the index
for subsequent available nodes. For example, if the powerkey node is
disabled, the homekey node would be processed with index 0 instead of 1,
causing it to be incorrectly assigned the hardware IRQ and registers of the
powerkey.

[ ... ]

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

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

end of thread, other threads:[~2026-05-29  0:29 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-28 23:56 [PATCH] Input: mtk-pmic-keys: Count available keys during probe instead of pre-counting Rosen Penev
2026-05-29  0:29 ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox