Re: [PATCH v4 02/10] Input: cap11xx - remove unused register macros

[sashiko-bot@kernel.org]

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

New issues:
- [Medium] The patch fails to remove unused definitions in `cap11xx_volatile_reg` despite the commit message claiming otherwise.

Pre-existing issues:
- [High] The threaded interrupt handler unconditionally returns IRQ_HANDLED even when I2C operations fail to clear the hardware interrupt source, risking an IRQ storm.
--

commit 8aa29f5d8b4eefa859454cdb0496fecf07ff5aa8
Author: Jun Yan <jerrysteve1101@gmail.com>

Remove unused register address macros and unused definitions in
the cap11xx_reg_defaults array and cap11xx_volatile_reg.

This cleanup reduces code clutter and makes the driver easier to
maintain without affecting functionality.

> Remove unused register address macros and unused definitions in
> the cap11xx_reg_defaults array and cap11xx_volatile_reg.

[Severity: Medium]
Will this patch actually remove the unused definitions in
cap11xx_volatile_reg?

The commit message states they are removed, but the patch doesn't modify
that function.

> diff --git a/drivers/input/keyboard/cap11xx.c b/drivers/input/keyboard/cap11xx.c
> index 485d8ba97723..fae26f035186 100644
> --- a/drivers/input/keyboard/cap11xx.c
> +++ b/drivers/input/keyboard/cap11xx.c
> @@ -20,53 +20,24 @@
>  #define CAP11XX_REG_MAIN_CONTROL_GAIN_SHIFT	(6)
>  #define CAP11XX_REG_MAIN_CONTROL_GAIN_MASK	(0xc0)
>  #define CAP11XX_REG_MAIN_CONTROL_DLSEEP		BIT(4)
> -#define CAP11XX_REG_GENERAL_STATUS	0x02
>  #define CAP11XX_REG_SENSOR_INPUT	0x03
> -#define CAP11XX_REG_NOISE_FLAG_STATUS	0x0a
>  #define CAP11XX_REG_SENOR_DELTA(X)	(0x10 + (X))

[Severity: Medium]
Should this macro also have been removed?

Because cap11xx_volatile_reg() was not updated, this macro and its
corresponding case entries:

	case CAP11XX_REG_SENOR_DELTA(0):
	case CAP11XX_REG_SENOR_DELTA(1):

were left in the file.

[ ... ]

>  static bool cap11xx_volatile_reg(struct device *dev, unsigned int reg)

[Severity: High]
This isn't a bug introduced by this patch, but could the error handling
in cap11xx_thread_func() risk an IRQ storm?

If an I2C communication error occurs during the interrupt handler:

cap11xx_thread_func() {
	...
	if (ret < 0)
		goto out;
	...
out:
	return IRQ_HANDLED;
}

Because this uses IRQF_ONESHOT, returning IRQ_HANDLED instructs the IRQ
subsystem to unmask the interrupt. Since the device's interrupt status
was never cleared due to the I2C failure, the interrupt line will remain
asserted.

Will this cause the interrupt to immediately fire again when unmasked,
creating an infinite loop?

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