[PATCH v1 1/2] selftests: gpio: gpio-sim: Fix BUG: test FAILED due to recent change

[PATCH v1 1/2] selftests: gpio: gpio-sim: Fix BUG: test FAILED due to recent change

[Andy Shevchenko]

From: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>

According to Mirsad the gpio-sim.sh test appears to FAIL in a wrong way
due to missing initialisation of shell variables:

 4.2. Bias settings work correctly
 cat: /sys/devices/platform/gpio-sim.0/gpiochip18/sim_gpio0/value: No such file or directory
 ./gpio-sim.sh: line 393: test: =: unary operator expected
 bias setting does not work
 GPIO gpio-sim test FAIL

After this change the test passed:

 4.2. Bias settings work correctly
 GPIO gpio-sim test PASS

His testing environment is AlmaLinux 8.7 on Lenovo desktop box with
the latest Linux kernel based on v6.2:

  Linux 6.2.0-mglru-kmlk-andy-09238-gd2980d8d8265 x86_64

Suggested-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 tools/testing/selftests/gpio/gpio-sim.sh | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/testing/selftests/gpio/gpio-sim.sh b/tools/testing/selftests/gpio/gpio-sim.sh
index 9f539d454ee4..fa2ce2b9dd5f 100755
--- a/tools/testing/selftests/gpio/gpio-sim.sh
+++ b/tools/testing/selftests/gpio/gpio-sim.sh
@@ -389,6 +389,9 @@ create_chip chip
 create_bank chip bank
 set_num_lines chip bank 8
 enable_chip chip
+DEVNAME=`configfs_dev_name chip`
+CHIPNAME=`configfs_chip_name chip bank`
+SYSFS_PATH="/sys/devices/platform/$DEVNAME/$CHIPNAME/sim_gpio0/value"
 $BASE_DIR/gpio-mockup-cdev -b pull-up /dev/`configfs_chip_name chip bank` 0
 test `cat $SYSFS_PATH` = "1" || fail "bias setting does not work"
 remove_chip chip
-- 
2.40.0.1.gaa8946217a0b

[PATCH v1 2/2] selftests: gpio: gpio-sim: Use same variable name for sysfs pathname

[Andy Shevchenko]

SYSFS_PATH can be used locally and globally, especially that has
the same content.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 tools/testing/selftests/gpio/gpio-sim.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/gpio/gpio-sim.sh b/tools/testing/selftests/gpio/gpio-sim.sh
index fa2ce2b9dd5f..6fb66a687f17 100755
--- a/tools/testing/selftests/gpio/gpio-sim.sh
+++ b/tools/testing/selftests/gpio/gpio-sim.sh
@@ -152,9 +152,9 @@ sysfs_set_pull() {
 	local PULL=$4
 	local DEVNAME=`configfs_dev_name $DEV`
 	local CHIPNAME=`configfs_chip_name $DEV $BANK`
-	local SYSFSPATH="/sys/devices/platform/$DEVNAME/$CHIPNAME/sim_gpio$OFFSET/pull"
+	local SYSFS_PATH="/sys/devices/platform/$DEVNAME/$CHIPNAME/sim_gpio$OFFSET/pull"
 
-	echo $PULL > $SYSFSPATH || fail "Unable to set line pull in sysfs"
+	echo $PULL > $SYSFS_PATH || fail "Unable to set line pull in sysfs"
 }
 
 # Load the gpio-sim module. This will pull in configfs if needed too.
-- 
2.40.0.1.gaa8946217a0b

Re: [PATCH v1 1/2] selftests: gpio: gpio-sim: Fix BUG: test FAILED due to recent change

[Andy Shevchenko]

On Tue, Apr 11, 2023 at 08:15:20PM +0300, Andy Shevchenko wrote:
> From: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>

Ah, Mirsad, I need your SoB, if you are okay with the change.
Otherwise I have to drop your authorship.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 2/2] selftests: gpio: gpio-sim: Use same variable name for sysfs pathname

[Andy Shevchenko]

On Tue, Apr 11, 2023 at 08:15:21PM +0300, Andy Shevchenko wrote:
> SYSFS_PATH can be used locally and globally, especially that has
> the same content.

This may behave differently from shell to shell, the minimum of bash, zsh,
and ash would be good to test.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/2] selftests: gpio: gpio-sim: Fix BUG: test FAILED due to recent change

[Mirsad Goran Todorovac]

On 11. 04. 2023. 19:47, Andy Shevchenko wrote:
> On Tue, Apr 11, 2023 at 08:15:20PM +0300, Andy Shevchenko wrote:
>> From: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
> 
> Ah, Mirsad, I need your SoB, if you are okay with the change.
> Otherwise I have to drop your authorship
Hi, Andy,

Sorry I needed to vent my beer before doing this test:

[root@pc-mtodorov linux_torvalds]# uname -rms
Linux 6.2.0-mglru-kmlk-andy-09238-gd2980d8d8265 x86_64
[root@pc-mtodorov linux_torvalds]# tools/testing/selftests/gpio/gpio-sim.sh
1. chip_name and dev_name attributes
1.1. Chip name is communicated to user
1.2. chip_name returns 'none' if the chip is still pending
1.3. Device name is communicated to user
2. Creating and configuring simulated chips
2.1. Default number of lines is 1
2.2. Number of lines can be specified
2.3. Label can be set
2.4. Label can be left empty
2.5. Line names can be configured
2.6. Line config can remain unused if offset is greater than number of lines
2.7. Line configfs directory names are sanitized
2.8. Multiple chips can be created
2.9. Can't modify settings when chip is live
2.10. Can't create line items when chip is live
2.11. Probe errors are propagated to user-space
2.12. Cannot enable a chip without any GPIO banks
2.13. Duplicate chip labels are not allowed
2.14. Lines can be hogged
3. Controlling simulated chips
3.1. Pull can be set over sysfs
3.2. Pull can be read from sysfs
3.3. Incorrect input in sysfs is rejected
3.4. Can't write to value
4. Simulated GPIO chips are functional
4.1. Values can be read from sysfs
4.2. Bias settings work correctly
cat: /sys/devices/platform/gpio-sim.0/gpiochip18/sim_gpio0/value: No such file or directory
tools/testing/selftests/gpio/gpio-sim.sh: line 393: test: =: unary operator expected
bias setting does not work
GPIO gpio-sim test FAIL
[root@pc-mtodorov linux_torvalds]# git apply ../gpio-sim-andy2.patch
[root@pc-mtodorov linux_torvalds]# tools/testing/selftests/gpio/gpio-sim.sh
1. chip_name and dev_name attributes
1.1. Chip name is communicated to user
1.2. chip_name returns 'none' if the chip is still pending
1.3. Device name is communicated to user
2. Creating and configuring simulated chips
2.1. Default number of lines is 1
2.2. Number of lines can be specified
2.3. Label can be set
2.4. Label can be left empty
2.5. Line names can be configured
2.6. Line config can remain unused if offset is greater than number of lines
2.7. Line configfs directory names are sanitized
2.8. Multiple chips can be created
2.9. Can't modify settings when chip is live
2.10. Can't create line items when chip is live
2.11. Probe errors are propagated to user-space
2.12. Cannot enable a chip without any GPIO banks
2.13. Duplicate chip labels are not allowed
2.14. Lines can be hogged
3. Controlling simulated chips
3.1. Pull can be set over sysfs
3.2. Pull can be read from sysfs
3.3. Incorrect input in sysfs is rejected
3.4. Can't write to value
4. Simulated GPIO chips are functional
4.1. Values can be read from sysfs
4.2. Bias settings work correctly
GPIO gpio-sim test PASS
[root@pc-mtodorov linux_torvalds]# 

So the issue was reproduced again on the above mentioned kernel, and the patch fixes it.
I will never trust an unexecuted line again, so help me the Lord Above.

Yes, you have both:

Tested-by: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
Signed-off-by: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>

Thank you for considering my bug report. It is great to assist your professional team.

This issue wasn't triggered by the 6.1.15 mglru kernel, and I am wondering what could
be the difference? (With the same gpio-sim.sh and patch.)

Best regards,
Mirsad

-- 
Mirsad Goran Todorovac
Sistem inženjer
Grafički fakultet | Akademija likovnih umjetnosti
Sveučilište u Zagrebu
 
System engineer
Faculty of Graphic Arts | Academy of Fine Arts
University of Zagreb, Republic of Croatia
The European Union

"I see something approaching fast ... Will it be friends with me?"

Re: [PATCH v1 1/2] selftests: gpio: gpio-sim: Fix BUG: test FAILED due to recent change

[Bartosz Golaszewski]

On Tue, Apr 11, 2023 at 7:15 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> From: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
>
> According to Mirsad the gpio-sim.sh test appears to FAIL in a wrong way
> due to missing initialisation of shell variables:
>
>  4.2. Bias settings work correctly
>  cat: /sys/devices/platform/gpio-sim.0/gpiochip18/sim_gpio0/value: No such file or directory
>  ./gpio-sim.sh: line 393: test: =: unary operator expected
>  bias setting does not work
>  GPIO gpio-sim test FAIL
>
> After this change the test passed:
>
>  4.2. Bias settings work correctly
>  GPIO gpio-sim test PASS
>
> His testing environment is AlmaLinux 8.7 on Lenovo desktop box with
> the latest Linux kernel based on v6.2:
>
>   Linux 6.2.0-mglru-kmlk-andy-09238-gd2980d8d8265 x86_64
>
> Suggested-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  tools/testing/selftests/gpio/gpio-sim.sh | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/tools/testing/selftests/gpio/gpio-sim.sh b/tools/testing/selftests/gpio/gpio-sim.sh
> index 9f539d454ee4..fa2ce2b9dd5f 100755
> --- a/tools/testing/selftests/gpio/gpio-sim.sh
> +++ b/tools/testing/selftests/gpio/gpio-sim.sh
> @@ -389,6 +389,9 @@ create_chip chip
>  create_bank chip bank
>  set_num_lines chip bank 8
>  enable_chip chip
> +DEVNAME=`configfs_dev_name chip`
> +CHIPNAME=`configfs_chip_name chip bank`
> +SYSFS_PATH="/sys/devices/platform/$DEVNAME/$CHIPNAME/sim_gpio0/value"
>  $BASE_DIR/gpio-mockup-cdev -b pull-up /dev/`configfs_chip_name chip bank` 0
>  test `cat $SYSFS_PATH` = "1" || fail "bias setting does not work"
>  remove_chip chip
> --
> 2.40.0.1.gaa8946217a0b
>

Queued for fixes. Thanks!

Bart

Re: [PATCH v1 2/2] selftests: gpio: gpio-sim: Use same variable name for sysfs pathname

[Bartosz Golaszewski]

On Tue, Apr 11, 2023 at 7:15 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> SYSFS_PATH can be used locally and globally, especially that has
> the same content.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  tools/testing/selftests/gpio/gpio-sim.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/gpio/gpio-sim.sh b/tools/testing/selftests/gpio/gpio-sim.sh
> index fa2ce2b9dd5f..6fb66a687f17 100755
> --- a/tools/testing/selftests/gpio/gpio-sim.sh
> +++ b/tools/testing/selftests/gpio/gpio-sim.sh
> @@ -152,9 +152,9 @@ sysfs_set_pull() {
>         local PULL=$4
>         local DEVNAME=`configfs_dev_name $DEV`
>         local CHIPNAME=`configfs_chip_name $DEV $BANK`
> -       local SYSFSPATH="/sys/devices/platform/$DEVNAME/$CHIPNAME/sim_gpio$OFFSET/pull"
> +       local SYSFS_PATH="/sys/devices/platform/$DEVNAME/$CHIPNAME/sim_gpio$OFFSET/pull"
>
> -       echo $PULL > $SYSFSPATH || fail "Unable to set line pull in sysfs"
> +       echo $PULL > $SYSFS_PATH || fail "Unable to set line pull in sysfs"
>  }
>
>  # Load the gpio-sim module. This will pull in configfs if needed too.
> --
> 2.40.0.1.gaa8946217a0b
>

Applied, thanks!

Bart