Re: [PATCH] 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:

New issues:
- [Medium] Sysfs attributes 'startup' and 'shutdown' are unconditionally broken for the newly added AXP313A device due to out-of-bounds regmap access.

Pre-existing issues:
- [High] Unconditional manipulation of uninitialized IRQ 0 during system suspend and resume when input device registration is bypassed.
--

commit 689d64caa4b9f21a0926e40a611d5111bfe37d22
Author: Steven Feng <steven@joint-cloud.com>

Input: axp20x-pek - add support for AXP313A variant

This commit adds power button support for the AXP313A PMIC. It reuses the
existing axp20x_info structure, noting that the AXP313A shares the same
register layout and timing parameters as the AXP20X.

> diff --git a/drivers/input/misc/axp20x-pek.c b/drivers/input/misc/axp20x-pek.c
> index 24f9e9d893de3..d4e2fc9a937ff 100644
> --- a/drivers/input/misc/axp20x-pek.c
> +++ b/drivers/input/misc/axp20x-pek.c
> @@ -393,6 +393,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)&axp20x_info,

[Severity: Medium]
Will reusing axp20x_info for the AXP313A break the sysfs attributes?

The axp20x_info struct exposes the startup and shutdown sysfs attributes.
When these are accessed, the driver reads or updates AXP20X_PEK_KEY (0x36):

drivers/input/misc/axp20x-pek.c:axp20x_show_attr() {
...
	ret = regmap_read(axp20x_pek->axp20x->regmap, AXP20X_PEK_KEY, &val);
...
}

However, the AXP313A regmap configuration restricts max_register to 0x21
(AXP313A_IRQ_STATE). Won't the regmap core reject accesses to 0x36 with
-EINVAL, making these sysfs attributes non-functional for this device?

> +	},
>  	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(platform, axp_pek_id_match);

[Severity: High]
This is a pre-existing issue, but could the suspend and resume handlers crash
the system if input device registration is skipped?

In axp20x_pek_suspend() and axp20x_pek_resume(), the driver unconditionally
calls disable_irq() and enable_irq():

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

If axp20x_pek_should_register_input() returns false (for instance, on x86
systems where ACPI handles the power button), the IRQs are never initialized
and remain 0.

Does calling disable_irq(0) disable the system timer interrupt on x86,
potentially causing a system hang or panic during the suspend cycle?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/tencent_5F1FF80489E702360F352F889570656BF608@qq.com?part=1