firmware_loader: few selftest fixes

From: Luis Chamberlain <[email protected]>

Greg,

I've found that Dan's patches really broke firmware testing. I've identified a proper fix for the issue he found, its the last patch. This series reverts his two patches which break testing, and fixes the issue he was running into. I leave it to him as exercise to add a busybox bash quirk for the bash issue he found with diff.

His patches are merged on v5.0-rc1 as such these should go to Linus tree as well. They are regressions on v5.0-rc1. Please let me know if there are any questions.

Luis

Luis Chamberlain (3):
Revert "selftests: firmware: add CONFIG_FW_LOADER_USER_HELPER_FALLBACK
to config"
Revert "selftests: firmware: remove use of non-standard diff -Z
option"
selftests: firmware: fix verify_reqs() return value

 tools/testing/selftests/firmware/config           | 1 -
 tools/testing/selftests/firmware/fw_filesystem.sh | 9 ++++++---
 tools/testing/selftests/firmware/fw_lib.sh        | 2 +-
 3 files changed, 7 insertions(+), 5 deletions(-)

2.18.0

From: Luis Chamberlain <[email protected]>

This reverts commit 7492902e8d22b568463897fa967c0886764cf034.

The commit tried to address an issue discovered by Dan where he got a message saying:

'usermode helper disabled so ignoring test'.

Dans's commit is forcing CONFIG_FW_LOADER_USER_HELPER_FALLBACK but just having CONFIG_FW_LOADER_USER_HELPER suffices to emulate the_FALLBACK functionality.

Dan's commit is trying to fix an issue which is hidden from a previous commit. That issue will be addressed properly next.

---
 tools/testing/selftests/firmware/config | 1 -
 1 file changed, 1 deletion(-)

index 913a25a4a32b..bf634dda0720 100644
@@ -1,6 +1,5 @@
 CONFIG_TEST_FIRMWARE=y
 CONFIG_FW_LOADER=y
 CONFIG_FW_LOADER_USER_HELPER=y
-CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y
 CONFIG_IKCONFIG=y
 CONFIG_IKCONFIG_PROC=y

2.18.0

From: Luis Chamberlain <[email protected]>

This reverts commit f70b472e937bb659a7b7a14e64f07308e230888c.

This breaks testing on Debian, and this patch was NACKed anyway. The proper way to address this is a quirk for busybox as that is where the issue is present.

---
 tools/testing/selftests/firmware/fw_filesystem.sh | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

index 466cf2f91ba0..a4320c4b44dc 100755
@@ -155,8 +155,11 @@ read_firmwares()
 {
 	for i in $(seq 0 3); do
 		config_set_read_fw_idx $i
-		# Verify the contents match
-		if ! diff -q "$FW" $DIR/read_firmware 2>/dev/null ; then
+		# Verify the contents are what we expect.
+		# -Z required for now -- check for yourself, md5sum
+		# on $FW and DIR/read_firmware will yield the same. Even
+		# cmp agrees, so something is off.
+		if ! diff -q -Z "$FW" $DIR/read_firmware 2>/dev/null ; then
 			echo "request #$i: firmware was not loaded" >&2
 			exit 1
 		fi

@@ -168,7 +171,7 @@ read_firmwares_expect_nofile()
 	for i in $(seq 0 3); do
 		config_set_read_fw_idx $i
 		# Ensures contents differ
-		if diff -q "$FW" $DIR/read_firmware 2>/dev/null ; then
+		if diff -q -Z "$FW" $DIR/read_firmware 2>/dev/null ; then
 			echo "request $i: file was not expected to match" >&2
 			exit 1
 		fi

2.18.0

From: Luis Chamberlain <[email protected]>

commit a6a9be9270c87 ("selftests: firmware: return Kselftest Skip code for skipped tests") by Shuah modified failures to return the special error code of $ksft_skip (4). We have a corner case issue where we *do* want to verify_reqs().

---
 tools/testing/selftests/firmware/fw_lib.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 6c5f1b2ffb74..1cbb12e284a6 100755
@@ -91,7 +91,7 @@ verify_reqs()
 	if [ "$TEST_REQS_FW_SYSFS_FALLBACK" = "yes" ]; then
 		if [ ! "$HAS_FW_LOADER_USER_HELPER" = "yes" ]; then
 			echo "usermode helper disabled so ignoring test"
-			exit $ksft_skip
+			exit 0
 		fi
 	fi
 }

2.18.0

Luis: Thanks for finding and fixing the problems. My apologies for pulling the problem patch in by mistake in the first place. :(

Greg! Do you plan to take these through firmware or do you want me to get these in through kselftest tree. Either way is fine.

thanks,
-- Shuah

I already queued these up in my tree yesterday, so no need to add them to yours.

thanks,

greg k-h