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

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

[Rosen Penev]

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

Re: [PATCH] Input: mtk-pmic-keys: Count available keys during probe instead of pre-counting

[sashiko-bot]

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

Re: [PATCH] Input: mtk-pmic-keys: Count available keys during probe instead of pre-counting

[Dmitry Torokhov]

Hi Rosen,

On Thu, May 28, 2026 at 04:56:00PM -0700, Rosen Penev wrote:
> 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.

Please split this out into a separate patch.

> 
> 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) {

Let's keep using for_each_child_of_node_scoped() and check
of_device_is_available() inside the loop. This will allow marking a key
as disabled without shifting it's meaning (power key vs home key).

In the rest of the driver we should be able to determine if key is set
up checking for key->irq > 0.

Thanks.

-- 
Dmitry