* [PATCH v2] gpiolib: sysfs: Fix error handling on failed export
@ 2023-11-28 14:13 Boerge Struempfel
2023-11-28 16:29 ` Andy Shevchenko
0 siblings, 1 reply; 4+ messages in thread
From: Boerge Struempfel @ 2023-11-28 14:13 UTC (permalink / raw)
To: andy, Linus Walleij, Bartosz Golaszewski, linux-gpio, linux-kernel
Cc: boerge.struempfel, bstruempfel
If gpio_set_transitory() fails, we should free the gpio again. Most
notably, the flag FLAG_REQUESTED has previously been set in
gpiod_request_commit(), and should be reset on failure.
To my knowledge, this does not affect any current users, since the
gpio_set_transitory() mainly returns 0 and -ENOTSUPP, which is converted
to 0. However the gpio_set_transitory() function calles the .set_config()
function of the corresponding gpio chip and there are some gpio drivers in
which some (unlikely) branches return other values like -EPROBE_DEFER,
and EINVAL. In these cases, the above mentioned FLAG_REQUESTED would not
be reset, which results in the pin being blocked until the next reboot.
Signed-off-by: Boerge Struempfel <boerge.struempfel@gmail.com>
---
V1: https://lore.kernel.org/linux-gpio/CAEktqcuxS1sPfkGVCgSy1ki8fmUDmuUsHrdAT+zFKy5vGSoKPw@mail.gmail.com/T/#t
Changes from V1:
- Marked all functions in commit message with parenthesis
- Elaborated in commit message
drivers/gpio/gpiolib-sysfs.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
index 6f309a3b2d9a..12d853845bb8 100644
--- a/drivers/gpio/gpiolib-sysfs.c
+++ b/drivers/gpio/gpiolib-sysfs.c
@@ -474,14 +474,17 @@ static ssize_t export_store(const struct class *class,
goto done;
status = gpiod_set_transitory(desc, false);
- if (!status) {
- status = gpiod_export(desc, true);
- if (status < 0)
- gpiod_free(desc);
- else
- set_bit(FLAG_SYSFS, &desc->flags);
+ if (status) {
+ gpiod_free(desc);
+ goto done;
}
+ status = gpiod_export(desc, true);
+ if (status < 0)
+ gpiod_free(desc);
+ else
+ set_bit(FLAG_SYSFS, &desc->flags);
+
done:
if (status)
pr_debug("%s: status %d\n", __func__, status);
--
2.42.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] gpiolib: sysfs: Fix error handling on failed export
2023-11-28 14:13 [PATCH v2] gpiolib: sysfs: Fix error handling on failed export Boerge Struempfel
@ 2023-11-28 16:29 ` Andy Shevchenko
2023-11-29 15:13 ` Börge Strümpfel
0 siblings, 1 reply; 4+ messages in thread
From: Andy Shevchenko @ 2023-11-28 16:29 UTC (permalink / raw)
To: Boerge Struempfel
Cc: Linus Walleij, Bartosz Golaszewski, linux-gpio, linux-kernel,
bstruempfel
On Tue, Nov 28, 2023 at 03:13:21PM +0100, Boerge Struempfel wrote:
> If gpio_set_transitory() fails, we should free the gpio again. Most
gpio --> GPIO descriptor
(I already mentioned capitalization in v1 review)
> notably, the flag FLAG_REQUESTED has previously been set in
> gpiod_request_commit(), and should be reset on failure.
>
> To my knowledge, this does not affect any current users, since the
> gpio_set_transitory() mainly returns 0 and -ENOTSUPP, which is converted
> to 0. However the gpio_set_transitory() function calles the .set_config()
> function of the corresponding gpio chip and there are some gpio drivers in
gpio --> GPIO
> which some (unlikely) branches return other values like -EPROBE_DEFER,
> and EINVAL. In these cases, the above mentioned FLAG_REQUESTED would not
-EINVAL
> be reset, which results in the pin being blocked until the next reboot.
Fixes tag?
(`git log --no-merges --grep "Fixes:" will show you examples)
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] gpiolib: sysfs: Fix error handling on failed export
2023-11-28 16:29 ` Andy Shevchenko
@ 2023-11-29 15:13 ` Börge Strümpfel
2023-11-29 16:09 ` Andy Shevchenko
0 siblings, 1 reply; 4+ messages in thread
From: Börge Strümpfel @ 2023-11-29 15:13 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Linus Walleij, Bartosz Golaszewski, linux-gpio, linux-kernel,
bstruempfel
Hello Andy
Thanks again for your feedback.
On Tue, Nov 28, 2023 at 5:29 PM Andy Shevchenko <andy@kernel.org> wrote:
>
> On Tue, Nov 28, 2023 at 03:13:21PM +0100, Boerge Struempfel wrote:
> > If gpio_set_transitory() fails, we should free the gpio again. Most
>
> gpio --> GPIO descriptor
> (I already mentioned capitalization in v1 review)
>
I'm sorry, I misunderstood your comment "GPIO" in the v1 review. I fixed it for
the next version.
> > notably, the flag FLAG_REQUESTED has previously been set in
> > gpiod_request_commit(), and should be reset on failure.
> >
> > To my knowledge, this does not affect any current users, since the
> > gpio_set_transitory() mainly returns 0 and -ENOTSUPP, which is converted
> > to 0. However the gpio_set_transitory() function calles the .set_config()
> > function of the corresponding gpio chip and there are some gpio drivers in
>
> gpio --> GPIO
>
thanks
> > which some (unlikely) branches return other values like -EPROBE_DEFER,
> > and EINVAL. In these cases, the above mentioned FLAG_REQUESTED would not
>
> -EINVAL
>
thanks, I missed that, when I added the minus to all the other Error codes.
> > be reset, which results in the pin being blocked until the next reboot.
>
> Fixes tag?
> (`git log --no-merges --grep "Fixes:" will show you examples)
>
I thought it was optional. But I have added it for the next version.
> --
> With Best Regards,
> Andy Shevchenko
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] gpiolib: sysfs: Fix error handling on failed export
2023-11-29 15:13 ` Börge Strümpfel
@ 2023-11-29 16:09 ` Andy Shevchenko
0 siblings, 0 replies; 4+ messages in thread
From: Andy Shevchenko @ 2023-11-29 16:09 UTC (permalink / raw)
To: Börge Strümpfel
Cc: Linus Walleij, Bartosz Golaszewski, linux-gpio, linux-kernel,
bstruempfel
On Wed, Nov 29, 2023 at 04:13:44PM +0100, Börge Strümpfel wrote:
> On Tue, Nov 28, 2023 at 5:29 PM Andy Shevchenko <andy@kernel.org> wrote:
> > On Tue, Nov 28, 2023 at 03:13:21PM +0100, Boerge Struempfel wrote:
...
> > Fixes tag?
> > (`git log --no-merges --grep "Fixes:" will show you examples)
>
> I thought it was optional. But I have added it for the next version.
It's optional. We want to have this only when it's real fix at the table,
and to me this one fits. Otherwise, put into the comment area (after
the '---' cutter line) why it shouldn't be considered as a such.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-11-29 16:09 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-28 14:13 [PATCH v2] gpiolib: sysfs: Fix error handling on failed export Boerge Struempfel
2023-11-28 16:29 ` Andy Shevchenko
2023-11-29 15:13 ` Börge Strümpfel
2023-11-29 16:09 ` Andy Shevchenko
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.