Skip to contentSkip to conversation

regression from commit b0ce9ce408b6 ("gpiolib: Do not unexport GPIO on freeing")

DISCUSSION Marek Behún 2 participants 7 messages Aug 8, 2023 – Aug 14, 2023 GPIO

lore
Share & tools
lore

Marek Behún

author
#0 lore raw

Hi,

the commit b0ce9ce408b6 ("gpiolib: Do not unexport GPIO on freeing")

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b0ce9ce408b6

causes a regression on my mvebu arm board (haven't tested on other systems), wherein if I export a GPIO to sysfs and then unexport it, it does not disasppear from the /sys/class/gpio directory, and subsequent writes to the export and unexport files for the gpio fail.

  $ cd /sys/class/gpio
  $ ls
  export       gpiochip0    gpiochip32   gpiochip512  unexport
  $ echo 43 >export
  $ ls
  export       gpio43       gpiochip0    gpiochip32   gpiochip512
  unexport
  $ cat gpio43/value
  1
  $ echo 43 >unexport
  $ ls
  export       gpio43       gpiochip0    gpiochip32   gpiochip512
  unexport
  $ echo 43 >unexport
  ash: write error: Invalid argument
  $ echo 43 >export
  ash: write error: Operation not permitted

Marek

#1 lore raw
Quoted 10 lines · Marek Behún

Hi,

the commit b0ce9ce408b6 ("gpiolib: Do not unexport GPIO on freeing")

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b0ce9ce408b6

causes a regression on my mvebu arm board (haven't tested on other systems), wherein if I export a GPIO to sysfs and then unexport it, it does not disasppear from the /sys/class/gpio directory, and subsequent writes to the export and unexport files for the gpio fail.

Thank you for the report. I'll look into this ASAP.

Signature · 2 lines
With Best Regards,
Andy Shevchenko
#2 lore raw
In reply to Marek Behún
Quoted 20 lines · Marek Behún

Hi,

the commit b0ce9ce408b6 ("gpiolib: Do not unexport GPIO on freeing")

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b0ce9ce408b6

causes a regression on my mvebu arm board (haven't tested on other systems), wherein if I export a GPIO to sysfs and then unexport it, it does not disasppear from the /sys/class/gpio directory, and subsequent writes to the export and unexport files for the gpio fail.

  $ cd /sys/class/gpio
  $ ls
  export       gpiochip0    gpiochip32   gpiochip512  unexport
  $ echo 43 >export
  $ ls
  export       gpio43       gpiochip0    gpiochip32   gpiochip512
  unexport
  $ cat gpio43/value
  1
Quoted 8 lines
  $ echo 43 >unexport
  $ ls
  export       gpio43       gpiochip0    gpiochip32   gpiochip512
  unexport
  $ echo 43 >unexport
  ash: write error: Invalid argument
  $ echo 43 >export
  ash: write error: Operation not permitted

Can you test the following change (I'll submit a formal patch if it works)?

drivers/gpio/gpiolib-sysfs.c +2 -1
index 530dfd19d7b5..b10a9b5598b2 100644
@@ -515,8 +515,9 @@ static ssize_t unexport_store(const struct class *class,
 	 * they may be undone on its behalf too.
 	 */
 	if (test_and_clear_bit(FLAG_SYSFS, &desc->flags)) {
+		gpiod_unexport(desc);
+		gpiod_free(desc);
 		status = 0;
-		gpiod_free(desc);
 	}
 done:
 	if (status)
Signature · 2 lines
With Best Regards,
Andy Shevchenko
author
#3 lore raw
Quoted 38 lines · On Fri, 11 Aug 2023 13:16:46 +0300 Andy Shevchenko…
Quoted 20 lines · Marek Behún

Hi,

the commit b0ce9ce408b6 ("gpiolib: Do not unexport GPIO on freeing")

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b0ce9ce408b6

causes a regression on my mvebu arm board (haven't tested on other systems), wherein if I export a GPIO to sysfs and then unexport it, it does not disasppear from the /sys/class/gpio directory, and subsequent writes to the export and unexport files for the gpio fail.

  $ cd /sys/class/gpio
  $ ls
  export       gpiochip0    gpiochip32   gpiochip512  unexport
  $ echo 43 >export
  $ ls
  export       gpio43       gpiochip0    gpiochip32   gpiochip512
  unexport
  $ cat gpio43/value
  1
Quoted 8 lines
  $ echo 43 >unexport
  $ ls
  export       gpio43       gpiochip0    gpiochip32   gpiochip512
  unexport
  $ echo 43 >unexport
  ash: write error: Invalid argument
  $ echo 43 >export
  ash: write error: Operation not permitted

Can you test the following change (I'll submit a formal patch if it works)?

drivers/gpio/gpiolib-sysfs.c +0 -0
index 530dfd19d7b5..b10a9b5598b2 100644

drivers/gpio/gpiolib-sysfs.c static ssize_t unexport_store(const struct class *class, · line 523

Quoted 10 lines
drivers/gpio/gpiolib-sysfs.c +2 -1
@@ -515,8 +515,9 @@ static ssize_t unexport_store(const struct class *class,
 	 * they may be undone on its behalf too.
 	 */
 	if (test_and_clear_bit(FLAG_SYSFS, &desc->flags)) {
+		gpiod_unexport(desc);
+		gpiod_free(desc);
 		status = 0;
-		gpiod_free(desc);
 	}
 done:
 	if (status)

tested, works.

Please add Fixes tag when sending the patch

#4 lore raw
Quoted 2 lines · Marek Behún

On Fri, 11 Aug 2023 13:16:46 +0300
Andy Shevchenko <[email protected]> wrote:

...

Quoted 1 line

tested, works.

I convert this to the Tested-by tag, I hope you won't object. But tell me if it's the case.

Quoted 1 line

Please add Fixes tag when sending the patch

Sure, it's done locally that day already :-)

Signature · 2 lines
With Best Regards,
Andy Shevchenko
author
#5 lore raw
Quoted 10 lines · On Mon, 14 Aug 2023 14:18:25 +0300 Andy Shevchenko…
Quoted 2 lines · Marek Behún

On Fri, 11 Aug 2023 13:16:46 +0300
Andy Shevchenko <[email protected]> wrote:

...

Quoted 1 line

tested, works.

I convert this to the Tested-by tag, I hope you won't object. But tell me if it's the case.

Yes, Tested-by, maybe even Reported-by.

Marek

#6 lore raw
Quoted 6 lines · Marek Behún
Quoted 3 lines · On Mon, 14 Aug 2023 14:18:25 +0300 Andy Shevchenko…
Quoted 2 lines · Marek Behún

On Fri, 11 Aug 2023 13:16:46 +0300
Andy Shevchenko <[email protected]> wrote:

...

Quoted 6 lines
Quoted 4 lines
Quoted 1 line

tested, works.

I convert this to the Tested-by tag, I hope you won't object. But tell me if it's the case.

Yes, Tested-by, maybe even Reported-by.

Both. As they are about different phases of the flow.

Signature · 2 lines
With Best Regards,
Andy Shevchenko