[PATCH v4 0/2] Input: axp20x-pek - add AXP313A support

[PATCH v4 0/2] Input: axp20x-pek - add AXP313A support

[Steven Feng]

This series adds power button support for the AXP313A PMIC.

The AXP313A lacks the PEK_KEY register used for configuring startup
and shutdown timings. To handle this, the first patch adds an
is_visible callback to hide unsupported sysfs attributes, and the
second patch adds the AXP313A device itself.

Changes in v4:
- Split into two patches 
- First patch adds is_visible infrastructure
- Second patch adds AXP313A device support
- Fixed subject line to show version number

Changes in v3:
- Use is_visible to hide attributes instead of returning -EOPNOTSUPP
- Corrected author name from "steven" to "Steven Feng"

Changes in v2:
- Fixed regmap out-of-bounds access issue reported by Sashiko AI
- Created dedicated axp313a_info with NULL time tables
- AXP313A lacks PEK_KEY (0x36) per datasheet verification

v3: https://lore.kernel.org/all/tencent_81FCC0443B81A3078C17F44AB4B22DFB2E07@qq.com/
v2: https://lore.kernel.org/all/tencent_48A497E0CA81323CFB6C7CB84428019A8707@qq.com/
v1: https://lore.kernel.org/all/tencent_5F1FF80489E702360F352F889570656BF608@qq.com/

Steven Feng (2):
  Input: axp20x-pek - add is_visible callback for sysfs attributes
  Input: axp20x-pek - add support for AXP313A variant

 drivers/input/misc/axp20x-pek.c | 34 ++++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

-- 
2.43.0

[PATCH v4 1/2] Input: axp20x-pek - add is_visible callback for sysfs attributes

[Steven Feng]

Add an is_visible callback to control the visibility of sysfs attributes
based on hardware capabilities. This is needed to support PMICs that lack
the PEK_KEY register for configuring startup and shutdown timings.

Devices without startup_time configuration will have these attributes
hidden from sysfs, providing a cleaner interface than returning errors
when the attributes are accessed.

This prepares the driver for adding support for the AXP313A variant.

Signed-off-by: Steven Feng <steven@joint-cloud.com>
---
 drivers/input/misc/axp20x-pek.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/input/misc/axp20x-pek.c b/drivers/input/misc/axp20x-pek.c
index 24f9e9d893de..c1a4e5ea1e00 100644
--- a/drivers/input/misc/axp20x-pek.c
+++ b/drivers/input/misc/axp20x-pek.c
@@ -193,7 +193,28 @@ static struct attribute *axp20x_attrs[] = {
 	&dev_attr_shutdown.attr,
 	NULL,
 };
-ATTRIBUTE_GROUPS(axp20x);
+
+static umode_t axp20x_attr_is_visible(struct kobject *kobj,
+				      struct attribute *attr, int n)
+{
+	struct device *dev = kobj_to_dev(kobj);
+	struct axp20x_pek *axp20x_pek = dev_get_drvdata(dev);
+
+	if (!axp20x_pek->info->startup_time)
+		return 0;
+
+	return attr->mode;
+}
+
+static const struct attribute_group axp20x_group = {
+	.attrs = axp20x_attrs,
+	.is_visible = axp20x_attr_is_visible,
+};
+
+static const struct attribute_group *axp20x_groups[] = {
+	&axp20x_group,
+	NULL,
+};
 
 static irqreturn_t axp20x_pek_irq(int irq, void *pwr)
 {
-- 
2.43.0

Re: [PATCH v4 1/2] Input: axp20x-pek - add is_visible callback for sysfs attributes

[sashiko-bot]

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

Pre-existing issues:
- [High] The driver unconditionally operates on uninitialized IRQs (0) during suspend and resume when the input device is not registered, which can crash or hang the system.
--

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

Input: axp20x-pek - add is_visible callback for sysfs attributes

This commit adds an is_visible callback to control the visibility of sysfs
attributes based on hardware capabilities. It hides the attributes for
devices without a startup_time configuration.

> diff --git a/drivers/input/misc/axp20x-pek.c b/drivers/input/misc/axp20x-pek.c
> index 24f9e9d893de3..c1a4e5ea1e001 100644
> --- a/drivers/input/misc/axp20x-pek.c
> +++ b/drivers/input/misc/axp20x-pek.c
> @@ -193,7 +193,28 @@ static struct attribute *axp20x_attrs[] = {
>  	&dev_attr_shutdown.attr,
>  	NULL,
>  };
> -ATTRIBUTE_GROUPS(axp20x);
> +
> +static umode_t axp20x_attr_is_visible(struct kobject *kobj,
> +				      struct attribute *attr, int n)
> +{
> +	struct device *dev = kobj_to_dev(kobj);
> +	struct axp20x_pek *axp20x_pek = dev_get_drvdata(dev);
> +
> +	if (!axp20x_pek->info->startup_time)
> +		return 0;
> +
> +	return attr->mode;
> +}

[Severity: High]
This is a pre-existing issue, but I noticed a potential problem in how
suspend and resume are handled when the input device is not registered.

If axp20x_pek_should_register_input() returns false (for example, on Cherry
Trail platforms with certain ACPI devices), axp20x_pek_probe_input_device()
is skipped. Since axp20x_pek is allocated with devm_kzalloc(), the irq_dbf
and irq_dbr fields remain 0.

During system suspend and resume, if device_may_wakeup(dev) is false, the
callbacks in axp20x_pek_suspend() and axp20x_pek_resume() unconditionally
execute:

        disable_irq(axp20x_pek->irq_dbf);
        disable_irq(axp20x_pek->irq_dbr);

and

        enable_irq(axp20x_pek->irq_dbf);
        enable_irq(axp20x_pek->irq_dbr);

Could this cause the driver to unconditionally call disable_irq(0) and
enable_irq(0)? On x86 architectures, tampering with IRQ 0 (typically the
legacy system timer) could cause severe system instability or kernel hangs
during the suspend and resume cycle.

> +
> +static const struct attribute_group axp20x_group = {
> +	.attrs = axp20x_attrs,
> +	.is_visible = axp20x_attr_is_visible,
> +};

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

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

[Steven Feng]

Add power button support for the AXP313A PMIC. The AXP313A does not
have the PEK_KEY register (0x36) used to configure startup and shutdown
timings, unlike AXP20X and AXP221.

Create a dedicated axp313a_info configuration with NULL time tables.
The sysfs attributes for startup/shutdown timing are automatically
hidden by the is_visible callback added in the previous patch.

Signed-off-by: Steven Feng <steven@joint-cloud.com>
---
 drivers/input/misc/axp20x-pek.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/input/misc/axp20x-pek.c b/drivers/input/misc/axp20x-pek.c
index c1a4e5ea1e00..964b39817af9 100644
--- a/drivers/input/misc/axp20x-pek.c
+++ b/drivers/input/misc/axp20x-pek.c
@@ -85,6 +85,13 @@ static const struct axp20x_info axp221_info = {
 	.shutdown_mask = AXP20X_PEK_SHUTDOWN_MASK,
 };
 
+static const struct axp20x_info axp313a_info = {
+	.startup_time = NULL,
+	.startup_mask = 0,
+	.shutdown_time = NULL,
+	.shutdown_mask = 0,
+};
+
 static ssize_t axp20x_show_attr(struct device *dev,
 				const struct axp20x_time *time,
 				unsigned int mask, char *buf)
@@ -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);
-- 
2.43.0

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

[sashiko-bot]

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