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@xxxxxxxxx>
> ---
>  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