Re: [PATCH v4 2/2] Input: axp20x-pek - add support for AXP313A variant

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Suspend and resume callbacks unconditionally disable/enable IRQs even when the input device is not registered, potentially disabling the system timer (IRQ 0).
- [Low] Manual assignment of the input device's parent is redundant when using devm_input_allocate_device().
--

commit 54f63f8b19bf8de6d0676d801f797fd4b2f755ae
Author: Steven Feng <steven@joint-cloud.com>

Input: axp20x-pek - add support for AXP313A variant

Add power button support for the AXP313A PMIC. Creates a dedicated
axp313a_info configuration with NULL time tables to support the
device since it lacks the PEK_KEY register.

> diff --git a/drivers/input/misc/axp20x-pek.c b/drivers/input/misc/axp20x-pek.c
> index c1a4e5ea1e001..964b39817af9f 100644
> --- a/drivers/input/misc/axp20x-pek.c
> +++ b/drivers/input/misc/axp20x-pek.c

[ ... ]

> @@ -414,6 +421,10 @@ static const struct platform_device_id axp_pek_id_match[] = {
>  		.name = "axp221-pek",
>  		.driver_data = (kernel_ulong_t)&axp221_info,
>  	},
> +	{
> +		.name = "axp313a-pek",
> +		.driver_data = (kernel_ulong_t)&axp313a_info,
> +	},
>  	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(platform, axp_pek_id_match);

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the driver
I noticed an issue in the suspend and resume callbacks:

drivers/input/misc/axp20x-pek.c:axp20x_pek_suspend() {
        ...
        disable_irq(axp20x_pek->irq_dbf);
        disable_irq(axp20x_pek->irq_dbr);
        ...
}

If axp20x_pek_should_register_input() returns false, the input device
is not registered and the IRQ numbers remain initialized to 0.

During suspend and resume, these callbacks unconditionally disable and
enable the IRQs. Since device_may_wakeup() evaluates to false in this
state, the callbacks execute disable_irq(0) and enable_irq(0).

On systems where IRQ 0 is the system timer, manipulating its state
during power transitions can lead to system hangs or kernel panics.

Should these callbacks verify that the input device is actually registered
before interacting with the IRQs?

[Severity: Low]
This is also a pre-existing issue, but I noticed a redundant assignment
in axp20x_pek_probe_input_device():

drivers/input/misc/axp20x-pek.c:axp20x_pek_probe_input_device() {
        ...
        idev->dev.parent = &pdev->dev;
        ...
}

Since devm_input_allocate_device() automatically sets the input device's
parent, is it necessary to manually assign it here?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260603074727.613319-1-steven@joint-cloud.com?part=2