Re: [PATCH v4 6/7] gpiolib: cdev: don't report a failed line read as an edge

[Kent Gibson <warthog618@gmail.com>]

On Thu, Oct 29, 2026 at 01:29:21PM +0200, Adrian Ambrozewicz wrote:
> With both edges requested, the IRQ threads read the line to tell a
> rising edge from a falling one, and report any non-zero result as
> rising, an error included. A read fails when a hot-pluggable chip is
> going away, or when gpiochip_remove() has cleared gdev->chip before the
> IRQ is freed.
> 
> Drop the event when the read fails, as debounce_work_func() does.
> 
> Reproduced with gpio-sim and a test delay in the IRQ threads: a falling
> edge during chip removal is read as rising; with this change, it is
> dropped and read() returns -ENODEV. The HTE path was compile-tested.
> 
> Fixes: 61f922db7221 ("gpio: userspace ABI for reading GPIO line events")
> Fixes: 73e0341992b6 ("gpiolib: cdev: support edge detection for uAPI v2")
> Fixes: 2068339a6c35 ("gpiolib: cdev: Add hardware timestamp clock type")

How did you determine that?  Those commits are where the associated blocks
of code were first added - not where the problem first exhibited.

The failure mode you are addressing was added by
commit d83cee3d ("gpio: protect the pointer to gpio_chip in gpio_device with SRCU").
Prior to that gpiod_get_raw_value_commit(), and gpiod_get_value_cansleep()
and gpiod_get_raw_value_cansleep() that wrap it, wouldn't return the error
that you are handling.

Could you bisect to confirm?

The code change itself looks good.

Cheers,
Kent.

> Cc: stable@vger.kernel.org
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20261020-gpiolib-chip-removal-v3-0-aambrozewicz@nvidia.com?part=6
> Assisted-by: LLM
> Signed-off-by: Adrian Ambrozewicz <aambrozewicz@nvidia.com>
> ---
>  drivers/gpio/gpiolib-cdev.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
> index cf23b5296fd1..0d6853751e6e 100644
> --- a/drivers/gpio/gpiolib-cdev.c
> +++ b/drivers/gpio/gpiolib-cdev.c
> @@ -643,6 +643,8 @@ static enum hte_return process_hw_ts_thread(void *p)
>  		level = (line->raw_level >= 0) ?
>  				line->raw_level :
>  				gpiod_get_raw_value_cansleep(line->desc);
> +		if (level < 0)
> +			return HTE_CB_HANDLED;
>  
>  		if (edflags & GPIO_V2_LINE_FLAG_ACTIVE_LOW)
>  			level = !level;
> @@ -743,6 +745,7 @@ static irqreturn_t edge_irq_thread(int irq, void *p)
>  	struct line *line = p;
>  	struct linereq *lr = line->req;
>  	struct gpio_v2_line_event le;
> +	int level;
>  
>  	/* Do not leak kernel stack to userspace */
>  	memset(&le, 0, sizeof(le));
> @@ -763,7 +766,10 @@ static irqreturn_t edge_irq_thread(int irq, void *p)
>  
>  	switch (READ_ONCE(line->edflags) & GPIO_V2_LINE_EDGE_FLAGS) {
>  	case GPIO_V2_LINE_FLAG_EDGE_BOTH:
> -		le.id = line_event_id(gpiod_get_value_cansleep(line->desc));
> +		level = gpiod_get_value_cansleep(line->desc);
> +		if (level < 0)
> +			return IRQ_HANDLED;
> +		le.id = line_event_id(level);
>  		break;
>  	case GPIO_V2_LINE_FLAG_EDGE_RISING:
>  		le.id = GPIO_V2_LINE_EVENT_RISING_EDGE;
> @@ -2006,6 +2012,8 @@ static irqreturn_t lineevent_irq_thread(int irq, void *p)
>  	    && le->eflags & GPIOEVENT_REQUEST_FALLING_EDGE) {
>  		int level = gpiod_get_value_cansleep(le->desc);
>  
> +		if (level < 0)
> +			return IRQ_HANDLED;
>  		if (level)
>  			/* Emit low-to-high event */
>  			ge.id = GPIOEVENT_EVENT_RISING_EDGE;
> -- 
> 2.43.0
>