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

[sashiko-bot@kernel.org]

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