Re: [PATCH] gpiolib: Avoid side effects in gpio_is_visible()

From: Chris Packham <Chris.Packham@alliedtelesis.co.nz>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: "brgl@bgdev.pl" <brgl@bgdev.pl>,
	"johan@kernel.org" <johan@kernel.org>,
	"andy.shevchenko@gmail.com" <andy.shevchenko@gmail.com>,
	"maz@kernel.org" <maz@kernel.org>,
	Ben Brown <Ben.Brown@alliedtelesis.co.nz>,
	"linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] gpiolib: Avoid side effects in gpio_is_visible()
Date: Tue, 16 May 2023 22:19:14 +0000	[thread overview]
Message-ID: <31a23398-9b0e-4a19-3576-84fcfd3ce4b5@alliedtelesis.co.nz> (raw)
In-Reply-To: <CACRpkdbiSAFoJP_JB1d_6gQ+Xx7Y+mLAh=C6Za+fpyWuRe6Gbw@mail.gmail.com>

On 17/05/23 01:57, Linus Walleij wrote:
> On Mon, May 15, 2023 at 12:27 AM Chris Packham
> <Chris.Packham@alliedtelesis.co.nz> wrote:
>
>> In my original case which is a kernel module that exports a GPIO for
>> userspace using gpiod_export()
> We should not add new users for that API as it increase the usage
> of the sysfs ABI but if it's an existing in-tree usecase I buy it.
>
>> The crux of the problem is that the irq_desc is created when it hasn't
>> been requested.
> The right solution to me seems to be to not use gpiod_export()
> and not use sysfs TBH.

That's not really a feasible solution. I'm dealing with application code 
that has been happily using the sysfs interface for many years.

I actually did look at getting that code updated to use libgpio earlier 
this year but found the API was in a state of flux and I wasn't going to 
recommend re-writing the application code to use libgpio if I knew the 
API was going to change and we'd have to re-write it again.

Even now with the 2.0.1 libgpio there doesn't seem to be a way of asking 
about just GPIO lines in the system, application code would still need 
to open every /dev/gpiochipN device and ask what lines are on the chip 
and keep the fds open for the chips that have lines the application 
cares about but make sure to close the fd for the ones that don't. So 
now the application code has to care about GPIO chips in addition to the 
GPIO lines.

>> In some cases we know the GPIO pin is an output so we
>> could avoid it, in others we could delay the creation until an interrupt
>> is actually requested (which is what I'm attempting to do).
> Yeah I guess. If we wanna keep papering over issues created
> by the sysfs ABI.

So that aside. Is is reasonable to expect that gpio_is_visible() should 
not have any side effects?

I still believe that this change is in the right direction although 
clearly I need to provide a better explanation of why I think that is 
the case. And there might be a better way of achieving my goal of not 
triggering the warning on kexec (certainly my initial effort was way off 
the mark).

>
> Yours,
> Linus Walleij