All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kent Gibson <warthog618@gmail.com>
To: Adrian Ambrozewicz <aambrozewicz@nvidia.com>
Cc: Bartosz Golaszewski <brgl@kernel.org>,
	Linus Walleij <linusw@kernel.org>,
	Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>,
	"Paul E. McKenney" <paulmck@kernel.org>,
	Herve Codina <herve.codina@bootlin.com>,
	Jia Jia <physicalmtea@gmail.com>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org,
	stable@vger.kernel.org, Sashiko <sashiko-bot@kernel.org>
Subject: Re: [PATCH v4 6/7] gpiolib: cdev: don't report a failed line read as an edge
Date: Fri, 30 Oct 2026 21:46:39 +0800	[thread overview]
Message-ID: <20261030134639.GA94733@rigel> (raw)
In-Reply-To: <20261030123109.1935475-2-aambrozewicz@nvidia.com>

On Fri, Oct 30, 2026 at 02:31:10PM +0200, Adrian Ambrozewicz wrote:
> On Fri, Oct 30, 2026 at 07:14:35PM +0800, Kent Gibson wrote:
> > > 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.
> 
> From the hot-plug case in the message: a driver's ->get() error has
> reached these checks since each was added. Removal is newer, as you
> say.
> 
> > 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?
> 
> Confirmed: with gpio-sim and delayed IRQ threads, a falling edge
> during removal reads as falling on the parent of commit d83cee3d2bb1
> ("gpio: protect the pointer to gpio_chip in gpio_device with SRCU")
> and as rising on it, 10 of 10 each.
> 
> A test chip whose ->get() returns -EIO gives a rising edge on that
> parent and on gpio/for-current, none with this patch. pcf857x and dln2
> already returned such errors at commit 61f922db7221
> ("gpio: userspace ABI for reading GPIO line events").
> 

Ok, so the driver ->get() itself may return an error if the physical chip
has gone away.  Hadn't considered that path.

Could you update the commit message to better describe that case?

> I would use Fixes: 61f922db7221 for ->get() errors and d83cee3d2bb1
> for removal. Is that OK?

Not sure - I'll defer to Bart on that.

Cheers,
Kent.

  reply	other threads:[~2026-10-30 13:46 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-29 11:29 [PATCH v4 0/7] gpiolib: fix chip removal with lines still requested from user space Adrian Ambrozewicz
2026-10-29 11:29 ` [PATCH v4 1/7] gpiolib: wait for pending label frees before cleaning up desc_srcu Adrian Ambrozewicz
2026-10-29 11:29 ` [PATCH v4 2/7] gpiolib: free the label of lines released after their chip was removed Adrian Ambrozewicz
2026-10-29 11:29 ` [PATCH v4 3/7] gpiolib: cdev: do not free line IRQs again after the chip is removed Adrian Ambrozewicz
2026-10-29 11:29 ` [PATCH v4 4/7] gpiolib: cdev: don't sleep in the SRCU read section in read() Adrian Ambrozewicz
2026-10-29 11:29 ` [PATCH v4 5/7] gpiolib: cdev: don't leak the IRQ name when the chip is removed Adrian Ambrozewicz
2026-10-29 11:29 ` [PATCH v4 6/7] gpiolib: cdev: don't report a failed line read as an edge Adrian Ambrozewicz
2026-10-30 11:14   ` Kent Gibson
2026-10-30 12:31     ` Adrian Ambrozewicz
2026-10-30 13:46       ` Kent Gibson [this message]
2026-10-29 11:29 ` [PATCH v4 7/7] gpiolib: let the character device free its IRQs first on chip removal Adrian Ambrozewicz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261030134639.GA94733@rigel \
    --to=warthog618@gmail.com \
    --cc=aambrozewicz@nvidia.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=bartosz.golaszewski@oss.qualcomm.com \
    --cc=brgl@kernel.org \
    --cc=herve.codina@bootlin.com \
    --cc=linusw@kernel.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=paulmck@kernel.org \
    --cc=physicalmtea@gmail.com \
    --cc=sashiko-bot@kernel.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.