Re: [PATCH v2 2/3] Input: Add driver for ESWIN EPH8621 touchscreen IC

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] input: unhandled SPI errors in IRQ handler cause IRQ storm
- [Low] input: implicit return of input_register_device() in probe
--

commit 72cf856c7b1ca07b03f9ce8e0bf75be42e60151c
Author: Luca Weiss <luca.weiss@fairphone.com>

Input: Add driver for ESWIN EPH8621 touchscreen IC

This commit adds a new SPI driver for the ESWIN EPH8621 series touchscreen
controllers.

> diff --git a/drivers/input/touchscreen/eswin_eph8621.c b/drivers/input/touchscreen/eswin_eph8621.c
> index 0000000000000..866cdc7e9334d
> --- /dev/null
> +++ b/drivers/input/touchscreen/eswin_eph8621.c
[ ... ]
> +static irqreturn_t eswin_interrupt(int irq, void *dev_id)
> +{
> +	struct eswin_touch *ts = dev_id;
> +	int payload_len;
> +
> +	payload_len = eswin_comms_two_stage_read(ts, ts->rx_buf);
> +	if (payload_len > 0)
> +		eswin_process_report(ts, ts->rx_buf, payload_len);
> +
> +	return IRQ_HANDLED;
> +}

[Severity: High]
Will this cause an interrupt storm if a communication error occurs?

If eswin_comms_two_stage_read() fails and returns a negative error code
(like -EINVAL), the device's interrupt condition isn't cleared. By
unconditionally returning IRQ_HANDLED, the kernel's spurious IRQ detector
is bypassed. Because the physical interrupt line is still asserted, won't
the interrupt immediately fire again and lead to an infinite CPU loop?

[ ... ]
> +static int eswin_probe(struct spi_device *spi)
> +{
[ ... ]
> +	error = devm_request_threaded_irq(dev, spi->irq, NULL, eswin_interrupt,
> +					  IRQF_ONESHOT, dev_name(dev), ts);
> +	if (error)
> +		return dev_err_probe(dev, error, "Failed to request IRQ\n");
> +
> +	return input_register_device(ts->input);
> +}

[Severity: Low]
This isn't a bug, but does this violate the input subsystem's style
preferences for explicit failure paths?

The subsystem guidelines recommend against using `return action(...);` where
the action returns an error code in functions with multiple failure points.
Should this be expanded to capture the return value of 
input_register_device(), check it, and explicitly return 0 on success?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260917-eswin-eph8621-v2-0-f534080a6338@fairphone.com?part=2