Since upstream commit 8bd76b3d3f3a ("gpio: sim: lock up configfs that an instantiated device depends on"), rmdir for an active virtual devices been prohibited.
Update gpio-sim selftest to align with the change.
Reported-by: kernel test robot oliver.sang@intel.com Closes: https://lore.kernel.org/oe-lkp/202501221006.a1ca5dfa-lkp@intel.com Signed-off-by: Koichiro Den koichiro.den@canonical.com --- tools/testing/selftests/gpio/gpio-sim.sh | 31 +++++++++++++++++++----- 1 file changed, 25 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/gpio/gpio-sim.sh b/tools/testing/selftests/gpio/gpio-sim.sh index 6fb66a687f17..bbc29ed9c60a 100755 --- a/tools/testing/selftests/gpio/gpio-sim.sh +++ b/tools/testing/selftests/gpio/gpio-sim.sh @@ -46,12 +46,6 @@ remove_chip() { rmdir $CONFIGFS_DIR/$CHIP || fail "Unable to remove the chip" }
-configfs_cleanup() { - for CHIP in `ls $CONFIGFS_DIR/`; do - remove_chip $CHIP - done -} - create_chip() { local CHIP=$1
@@ -105,6 +99,13 @@ disable_chip() { echo 0 > $CONFIGFS_DIR/$CHIP/live || fail "Unable to disable the chip" }
+configfs_cleanup() { + for CHIP in `ls $CONFIGFS_DIR/`; do + disable_chip $CHIP + remove_chip $CHIP + done +} + configfs_chip_name() { local CHIP=$1 local BANK=$2 @@ -181,6 +182,7 @@ create_chip chip create_bank chip bank enable_chip chip test -n `cat $CONFIGFS_DIR/chip/bank/chip_name` || fail "chip_name doesn't work" +disable_chip chip remove_chip chip
echo "1.2. chip_name returns 'none' if the chip is still pending" @@ -195,6 +197,7 @@ create_chip chip create_bank chip bank enable_chip chip test -n `cat $CONFIGFS_DIR/chip/dev_name` || fail "dev_name doesn't work" +disable_chip chip remove_chip chip
echo "2. Creating and configuring simulated chips" @@ -204,6 +207,7 @@ create_chip chip create_bank chip bank enable_chip chip test "`get_chip_num_lines chip bank`" = "1" || fail "default number of lines is not 1" +disable_chip chip remove_chip chip
echo "2.2. Number of lines can be specified" @@ -212,6 +216,7 @@ create_bank chip bank set_num_lines chip bank 16 enable_chip chip test "`get_chip_num_lines chip bank`" = "16" || fail "number of lines is not 16" +disable_chip chip remove_chip chip
echo "2.3. Label can be set" @@ -220,6 +225,7 @@ create_bank chip bank set_label chip bank foobar enable_chip chip test "`get_chip_label chip bank`" = "foobar" || fail "label is incorrect" +disable_chip chip remove_chip chip
echo "2.4. Label can be left empty" @@ -227,6 +233,7 @@ create_chip chip create_bank chip bank enable_chip chip test -z "`cat $CONFIGFS_DIR/chip/bank/label`" || fail "label is not empty" +disable_chip chip remove_chip chip
echo "2.5. Line names can be configured" @@ -238,6 +245,7 @@ set_line_name chip bank 2 bar enable_chip chip test "`get_line_name chip bank 0`" = "foo" || fail "line name is incorrect" test "`get_line_name chip bank 2`" = "bar" || fail "line name is incorrect" +disable_chip chip remove_chip chip
echo "2.6. Line config can remain unused if offset is greater than number of lines" @@ -248,6 +256,7 @@ set_line_name chip bank 5 foobar enable_chip chip test "`get_line_name chip bank 0`" = "" || fail "line name is incorrect" test "`get_line_name chip bank 1`" = "" || fail "line name is incorrect" +disable_chip chip remove_chip chip
echo "2.7. Line configfs directory names are sanitized" @@ -267,6 +276,7 @@ for CHIP in $CHIPS; do enable_chip $CHIP done for CHIP in $CHIPS; do + disable_chip $CHIP remove_chip $CHIP done
@@ -278,6 +288,7 @@ echo foobar > $CONFIGFS_DIR/chip/bank/label 2> /dev/null && \ fail "Setting label of a live chip should fail" echo 8 > $CONFIGFS_DIR/chip/bank/num_lines 2> /dev/null && \ fail "Setting number of lines of a live chip should fail" +disable_chip chip remove_chip chip
echo "2.10. Can't create line items when chip is live" @@ -285,6 +296,7 @@ create_chip chip create_bank chip bank enable_chip chip mkdir $CONFIGFS_DIR/chip/bank/line0 2> /dev/null && fail "Creating line item should fail" +disable_chip chip remove_chip chip
echo "2.11. Probe errors are propagated to user-space" @@ -316,6 +328,7 @@ mkdir -p $CONFIGFS_DIR/chip/bank/line4/hog enable_chip chip $BASE_DIR/gpio-mockup-cdev -s 1 /dev/`configfs_chip_name chip bank` 4 2> /dev/null && \ fail "Setting the value of a hogged line shouldn't succeed" +disable_chip chip remove_chip chip
echo "3. Controlling simulated chips" @@ -331,6 +344,7 @@ test "$?" = "1" || fail "pull set incorrectly" sysfs_set_pull chip bank 0 pull-down $BASE_DIR/gpio-mockup-cdev /dev/`configfs_chip_name chip bank` 1 test "$?" = "0" || fail "pull set incorrectly" +disable_chip chip remove_chip chip
echo "3.2. Pull can be read from sysfs" @@ -344,6 +358,7 @@ SYSFS_PATH=/sys/devices/platform/$DEVNAME/$CHIPNAME/sim_gpio0/pull test `cat $SYSFS_PATH` = "pull-down" || fail "reading the pull failed" sysfs_set_pull chip bank 0 pull-up test `cat $SYSFS_PATH` = "pull-up" || fail "reading the pull failed" +disable_chip chip remove_chip chip
echo "3.3. Incorrect input in sysfs is rejected" @@ -355,6 +370,7 @@ DEVNAME=`configfs_dev_name chip` CHIPNAME=`configfs_chip_name chip bank` SYSFS_PATH="/sys/devices/platform/$DEVNAME/$CHIPNAME/sim_gpio0/pull" echo foobar > $SYSFS_PATH 2> /dev/null && fail "invalid input not detected" +disable_chip chip remove_chip chip
echo "3.4. Can't write to value" @@ -365,6 +381,7 @@ DEVNAME=`configfs_dev_name chip` CHIPNAME=`configfs_chip_name chip bank` SYSFS_PATH="/sys/devices/platform/$DEVNAME/$CHIPNAME/sim_gpio0/value" echo 1 > $SYSFS_PATH 2> /dev/null && fail "writing to 'value' succeeded unexpectedly" +disable_chip chip remove_chip chip
echo "4. Simulated GPIO chips are functional" @@ -382,6 +399,7 @@ $BASE_DIR/gpio-mockup-cdev -s 1 /dev/`configfs_chip_name chip bank` 0 & sleep 0.1 # FIXME Any better way? test `cat $SYSFS_PATH` = "1" || fail "incorrect value read from sysfs" kill $! +disable_chip chip remove_chip chip
echo "4.2. Bias settings work correctly" @@ -394,6 +412,7 @@ 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" +disable_chip chip remove_chip chip
echo "GPIO $MODULE test PASS"
On Wed, Jan 22, 2025 at 5:33 AM Koichiro Den koichiro.den@canonical.com wrote:
Since upstream commit 8bd76b3d3f3a ("gpio: sim: lock up configfs that an instantiated device depends on"), rmdir for an active virtual devices been prohibited.
Update gpio-sim selftest to align with the change.
Reported-by: kernel test robot oliver.sang@intel.com Closes: https://lore.kernel.org/oe-lkp/202501221006.a1ca5dfa-lkp@intel.com Signed-off-by: Koichiro Den koichiro.den@canonical.com
tools/testing/selftests/gpio/gpio-sim.sh | 31 +++++++++++++++++++----- 1 file changed, 25 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/gpio/gpio-sim.sh b/tools/testing/selftests/gpio/gpio-sim.sh index 6fb66a687f17..bbc29ed9c60a 100755 --- a/tools/testing/selftests/gpio/gpio-sim.sh +++ b/tools/testing/selftests/gpio/gpio-sim.sh @@ -46,12 +46,6 @@ remove_chip() { rmdir $CONFIGFS_DIR/$CHIP || fail "Unable to remove the chip" }
-configfs_cleanup() {
for CHIP in `ls $CONFIGFS_DIR/`; do
remove_chip $CHIP
done
-}
create_chip() { local CHIP=$1
@@ -105,6 +99,13 @@ disable_chip() { echo 0 > $CONFIGFS_DIR/$CHIP/live || fail "Unable to disable the chip" }
+configfs_cleanup() {
for CHIP in `ls $CONFIGFS_DIR/`; do
disable_chip $CHIP
remove_chip $CHIP
done
+}
configfs_chip_name() { local CHIP=$1 local BANK=$2 @@ -181,6 +182,7 @@ create_chip chip create_bank chip bank enable_chip chip test -n `cat $CONFIGFS_DIR/chip/bank/chip_name` || fail "chip_name doesn't work" +disable_chip chip remove_chip chip
Hi! Thanks for addressing it.
Is there any place in this file where we'd call remove_chip() without calling disable_chip() first? Maybe we can fold disable_chip() into remove_chip() and make the patch much smaller?
Bart
On Wed, Jan 22, 2025 at 10:26:27AM GMT, Bartosz Golaszewski wrote:
On Wed, Jan 22, 2025 at 5:33 AM Koichiro Den koichiro.den@canonical.com wrote:
Since upstream commit 8bd76b3d3f3a ("gpio: sim: lock up configfs that an instantiated device depends on"), rmdir for an active virtual devices been prohibited.
Update gpio-sim selftest to align with the change.
Reported-by: kernel test robot oliver.sang@intel.com Closes: https://lore.kernel.org/oe-lkp/202501221006.a1ca5dfa-lkp@intel.com Signed-off-by: Koichiro Den koichiro.den@canonical.com
tools/testing/selftests/gpio/gpio-sim.sh | 31 +++++++++++++++++++----- 1 file changed, 25 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/gpio/gpio-sim.sh b/tools/testing/selftests/gpio/gpio-sim.sh index 6fb66a687f17..bbc29ed9c60a 100755 --- a/tools/testing/selftests/gpio/gpio-sim.sh +++ b/tools/testing/selftests/gpio/gpio-sim.sh @@ -46,12 +46,6 @@ remove_chip() { rmdir $CONFIGFS_DIR/$CHIP || fail "Unable to remove the chip" }
-configfs_cleanup() {
for CHIP in `ls $CONFIGFS_DIR/`; do
remove_chip $CHIP
done
-}
create_chip() { local CHIP=$1
@@ -105,6 +99,13 @@ disable_chip() { echo 0 > $CONFIGFS_DIR/$CHIP/live || fail "Unable to disable the chip" }
+configfs_cleanup() {
for CHIP in `ls $CONFIGFS_DIR/`; do
disable_chip $CHIP
remove_chip $CHIP
done
+}
configfs_chip_name() { local CHIP=$1 local BANK=$2 @@ -181,6 +182,7 @@ create_chip chip create_bank chip bank enable_chip chip test -n `cat $CONFIGFS_DIR/chip/bank/chip_name` || fail "chip_name doesn't work" +disable_chip chip remove_chip chip
Hi! Thanks for addressing it.
Is there any place in this file where we'd call remove_chip() without calling disable_chip() first? Maybe we can fold disable_chip() into remove_chip() and make the patch much smaller?
My aplogies for being late.
Yes, there are five places where I intentionally omitted disable_chip() calls before remove_chip() because the chip wasn't enabled in thoses cases. I scattered disable_chip() calls only where truly necessary. I also think explicit enable_chip()/disable_chip() pairing look more clean and readable.
That being said, I'm fine with your suggestion.
-Koichiro Den
Bart
On Thu, Jan 23, 2025 at 2:26 PM Koichiro Den koichiro.den@canonical.com wrote:
On Wed, Jan 22, 2025 at 10:26:27AM GMT, Bartosz Golaszewski wrote:
Hi! Thanks for addressing it.
Is there any place in this file where we'd call remove_chip() without calling disable_chip() first? Maybe we can fold disable_chip() into remove_chip() and make the patch much smaller?
My aplogies for being late.
Yes, there are five places where I intentionally omitted disable_chip() calls before remove_chip() because the chip wasn't enabled in thoses cases. I scattered disable_chip() calls only where truly necessary. I also think explicit enable_chip()/disable_chip() pairing look more clean and readable.
That being said, I'm fine with your suggestion.
-Koichiro Den
Bart
No, that's fine, let me pick it up as is then.
Bartosz
From: Bartosz Golaszewski bartosz.golaszewski@linaro.org
On Wed, 22 Jan 2025 13:33:09 +0900, Koichiro Den wrote:
Since upstream commit 8bd76b3d3f3a ("gpio: sim: lock up configfs that an instantiated device depends on"), rmdir for an active virtual devices been prohibited.
Update gpio-sim selftest to align with the change.
[...]
Applied, thanks!
[1/1] selftests: gpio: gpio-sim: Fix missing chip disablements commit: f8524ac33cd452aef5384504b3264db6039a455e
Best regards,
linux-kselftest-mirror@lists.linaro.org