Re: [PATCH] Input: adp5588-keys - cache GPIO state before registering the gpiochip

["Alvin Šipraga" <alvin.sipraga@analog.com>]

Thanks for your review sashiko-bot,

On Tue, Aug 18, 2026 at 04:11:19PM +0000, sashiko-bot@kernel.org wrote:
> [Severity: High]
> This isn't a bug introduced by this patch, but this patch actively
> exacerbates the consequences of a pre-existing issue with missing error
> handling on I2C reads. 
> 
> If adp5588_read() fails due to a bus fault, it returns a negative error
> code (e.g., -EIO). In adp5588_gpio_add(), these negative values are
> silently truncated to 8-bit unsigned values and stored in the driver's
> cached GPIO state (dat_out, dir, pull_dis). 
> 
> Because this patch moves this caching to occur before 
> devm_gpiochip_add_data(), when a GPIO hog is subsequently applied, the
> driver inadvertently clobbers the real hardware state by writing back this
> error-corrupted cache. Can this actively undermine the patch's stated goal
> of preserving bootloader GPIO configurations if an I2C read fails during
> probe?

If the reads fail, I would not expect the driver to behave properly at
all - neither before nor after this patch. It's true that this patch
might trade some broken behavior for another type of broken behavior
though. But only on systems where I2C reads are failing.

If the argument is that such a trade can be considered a regression, I
would point out that the suggestion here - to add error checking and to
fail probe on bus errors - would invariably cause probe failure on such
systems with buggy I2C reads. That's also a regression.

> Additionally, this same missing error check causes severe issues elsewhere.
> In adp5588_report_events(), the error is bitwise-ANDed with KEY_EV_MASK, 
> resulting in an invalid, large key value. This yields a calculated code
> index that exceeds the bounds of the kpad->keycode array, causing an
> out-of-bounds heap read and leaking phantom key presses to userspace. 
> 
> Finally, in adp5588_gpio_get_value(), a negative error code evaluates to
> true, incorrectly reporting the pin state as HIGH.
> 
> Would it be appropriate to add proper error checking to adp5588_read() or
> its callers to prevent state corruption and out-of-bounds memory access
> during I2C faults?

Sure the driver can be improved by adding error checking, but that would
be a much larger change. I hope that is not an impediment to getting
this particular issue fixed.

Alvin