[PATCH] Input: mtk-pmic-keys - use of_device_get_match_data()

[PATCH] Input: mtk-pmic-keys - use of_device_get_match_data()

[Rosen Penev]

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: Codex:GPT-5.5
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/input/keyboard/mtk-pmic-keys.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/input/keyboard/mtk-pmic-keys.c b/drivers/input/keyboard/mtk-pmic-keys.c
index c78d9f6d97c4..fd684ac16938 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>
@@ -333,8 +332,6 @@ static int mtk_pmic_keys_probe(struct platform_device *pdev)
 	struct mtk_pmic_keys *keys;
 	const struct mtk_pmic_regs *mtk_pmic_regs;
 	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)
@@ -342,7 +339,7 @@ static int mtk_pmic_keys_probe(struct platform_device *pdev)
 
 	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);
 
 	keys->input_dev = input_dev = devm_input_allocate_device(keys->dev);
 	if (!input_dev) {
-- 
2.54.0

Re: [PATCH] Input: mtk-pmic-keys - use of_device_get_match_data()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing NULL check for of_device_get_match_data() result leads to NULL pointer dereference.
- [High] Mismatch between child counting and iteration logic can cause out-of-bounds array accesses.
--

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

Input: mtk-pmic-keys - use of_device_get_match_data()

This commit simplifies the driver to fetch PMIC key register data directly
using of_device_get_match_data(). It also removes the unneeded of_device.h
include.

> diff --git a/drivers/input/keyboard/mtk-pmic-keys.c b/drivers/input/keyboard/mtk-pmic-keys.c
> index c78d9f6d97c4f..fd684ac169386 100644
> --- a/drivers/input/keyboard/mtk-pmic-keys.c
> +++ b/drivers/input/keyboard/mtk-pmic-keys.c
[ ... ]
> @@ -342,7 +339,7 @@ static int mtk_pmic_keys_probe(struct platform_device *pdev)
>  
>  	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);

While this isn't a new issue introduced by this patch, can mtk_pmic_regs be
NULL here if the device is bound manually via sysfs without a matching
device tree node?

If mtk_pmic_regs is NULL, it appears it is unconditionally dereferenced
later when calling mtk_pmic_keys_lp_reset_setup().

Would it make sense to add a check for !mtk_pmic_regs right after fetching
the match data?


Also, while reviewing this probe function, I noticed another pre-existing
issue further down:

	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_child_of_node_scoped(node, child) {
		keys->keys[index].regs = &mtk_pmic_regs->keys_regs[index];

Is there a mismatch between the child counting and iteration logic?

The validation check uses of_get_available_child_count(), which ignores
disabled nodes. But the loop uses for_each_child_of_node_scoped(), which
iterates over all children, including disabled ones.

If a disabled child node exists in the device tree alongside available ones,
could the loop execute more times than the initial validation accounted for?
This might increment the index variable past the allocated array bounds,
leading to out-of-bounds accesses on irqnames[] and keys->keys[].

Would updating the loop to use for_each_available_child_of_node_scoped()
prevent this?

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

Re: [PATCH] Input: mtk-pmic-keys - use of_device_get_match_data()

[Rosen Penev]

On Sun May 17, 2026 at 9:34 PM PDT, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Missing NULL check for of_device_get_match_data() result leads to NULL pointer dereference.
> - [High] Mismatch between child counting and iteration logic can cause out-of-bounds array accesses.
> --
>
> commit 0ee3fee49ccf02b7e629f90533e67fc9b40d2b9e
> Author: Rosen Penev <rosenp@gmail.com>
>
> Input: mtk-pmic-keys - use of_device_get_match_data()
>
> This commit simplifies the driver to fetch PMIC key register data directly
> using of_device_get_match_data(). It also removes the unneeded of_device.h
> include.
>
>> diff --git a/drivers/input/keyboard/mtk-pmic-keys.c b/drivers/input/keyboard/mtk-pmic-keys.c
>> index c78d9f6d97c4f..fd684ac169386 100644
>> --- a/drivers/input/keyboard/mtk-pmic-keys.c
>> +++ b/drivers/input/keyboard/mtk-pmic-keys.c
> [ ... ]
>> @@ -342,7 +339,7 @@ static int mtk_pmic_keys_probe(struct platform_device *pdev)
>>
>>  	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);
>
> While this isn't a new issue introduced by this patch, can mtk_pmic_regs be
> NULL here if the device is bound manually via sysfs without a matching
> device tree node?
>
> If mtk_pmic_regs is NULL, it appears it is unconditionally dereferenced
> later when calling mtk_pmic_keys_lp_reset_setup().
>
> Would it make sense to add a check for !mtk_pmic_regs right after fetching
> the match data?
No because every compatible has a non NULL data component.
>
>
> Also, while reviewing this probe function, I noticed another pre-existing
> issue further down:
>
> 	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_child_of_node_scoped(node, child) {
> 		keys->keys[index].regs = &mtk_pmic_regs->keys_regs[index];
>
> Is there a mismatch between the child counting and iteration logic?
>
> The validation check uses of_get_available_child_count(), which ignores
> disabled nodes. But the loop uses for_each_child_of_node_scoped(), which
> iterates over all children, including disabled ones.
>
> If a disabled child node exists in the device tree alongside available ones,
> could the loop execute more times than the initial validation accounted for?
> This might increment the index variable past the allocated array bounds,
> leading to out-of-bounds accesses on irqnames[] and keys->keys[].
>
> Would updating the loop to use for_each_available_child_of_node_scoped()
> prevent this?
Yes. Interesting that pre-existing issues are flagged...