Added UFS functional tests #220

jeevanandan-sandan · 2025-11-28T11:16:09Z

No description provided.

vnarapar · 2025-12-01T09:28:59Z

Please add commit messages

vnarapar · 2025-12-02T07:24:14Z

Ensure the script has execute permissions

vnarapar · 2025-12-02T11:41:49Z

Squash to single commit

vnarapar · 2025-12-02T11:43:26Z

Runner/plans/qcom-next-ci-premerge.yaml

        - $PWD/utils/send-to-lava.sh $PWD/suites/Kernel/Baseport/smmu/smmu.res || true
        - $PWD/suites/Kernel/Baseport/Storage/UFS_Validation/run.sh || true
        - $PWD/utils/send-to-lava.sh $PWD/suites/Kernel/Baseport/Storage/UFS_Validation/UFS_Validation.res || true
+        - $PWD/suites/Kernel/Baseport/Storage/ufs_gear_lane/run.sh || true


Are these tests validated on qcom-next?

smuppand

These comments apply to the other tests as well. Please also take care of the https://github.com/qualcomm-linux/qcom-linux-testkit/actions/runs/19762511972/job/56772745461.

smuppand · 2025-12-09T01:28:42Z

Runner/plans/qcom-next-ci-premerge.yaml

+        - $PWD/suites/Kernel/Baseport/Storage/ufs_runtime_suspend_resume/run.sh || true
+        - $PWD/utils/send-to-lava.sh $PWD/suites/Kernel/Baseport/Storage/ufs_runtime_suspend_resume/ufs_runtime_suspend_resume.res || true
+        - $PWD/suites/Kernel/Baseport/Storage/ufs_write_booster/run.sh || true
+        - $PWD/utils/send-to-lava.sh $PWD/suites/Kernel/Baseport/Storage/ufs_write_booster/ufs_write_booster.res || true


I suggest enabling these tests in qcom-next instead of meta-qcom.

Enabled the tests on qcom-next by adding the .yaml files for each test case.

smuppand · 2025-12-09T04:56:04Z

Runner/suites/Kernel/Baseport/Storage/ufs_gear_check/run.sh

+# Only source if not already loaded
+if [ -z "$__INIT_ENV_LOADED" ]; then
+    # shellcheck disable=SC1090
+    . "$INIT_ENV"


you never set __INIT_ENV_LOADED, so this guard is effectively useless if this script is ever sourced again.

exporting the __INIT_ENV_LOADED variable after setting it to 1 once the source is loaded.

smuppand · 2025-12-09T04:58:36Z

Runner/suites/Kernel/Baseport/Storage/ufs_gear_lane/run.sh

+log_info "--------------------------------------------------"
+log_info "------------- Starting $TESTNAME Test ------------"
+
+check_dependencies dd grep cut head tail udevadm sleep


cut, head, tail, udevadm are not used anywhere in this script. You do use findmnt and awk, and rely on readlink (though that's almost always present)

Removed unused dependencies and checking only the required dependencies.

smuppand · 2025-12-09T05:00:31Z

Runner/suites/Kernel/Baseport/Storage/ufs_gear_check/run.sh

+OPTIONAL_CONFIGS="CONFIG_SCSI_UFSHCD_PLATFORM CONFIG_SCSI_UFSHCD_PCI CONFIG_SCSI_UFS_CDNS_PLATFORM CONFIG_SCSI_UFS_HISI CONFIG_SCSI_UFS_EXYNOS CONFIG_SCSI_UFS_ROCKCHIP CONFIG_SCSI_UFS_BSG"
+
+log_info "Checking mandatory kernel configs for UFS..."
+if ! check_kernel_config "$MANDATORY_CONFIGS" 2>/dev/null; then


"$MANDATORY_CONFIGS" is one arugment with a space inside, not two config names. That'll fail.

smuppand · 2025-12-09T05:02:13Z

Runner/suites/Kernel/Baseport/Storage/ufs_gear_check/run.sh

+[ -n "$missing_optional" ] && log_info "Optional configs not present but continuing:$missing_optional"
+
+check_dt_nodes "/sys/bus/platform/devices/*ufs*" || {
+    echo "$TESTNAME SKIP" > "$res_file"


Include log_skip so it is visible to the user.

added log_skip when UFS dt_nodes are missing.

smuppand · 2025-12-09T10:57:34Z

Runner/suites/Kernel/Baseport/Storage/ufs_gear_lane/run.sh

+    log_pass "UFS Gear and Lane validation passed"
+    log_info "Gear: $CURRENT_GEAR (Expected: HS_GEAR$EXPECTED_GEAR)"
+    log_info "Lane: $CURRENT_LANE (Expected: 2)"
+    scan_dmesg_errors "ufs" "$test_path"


Called twice on the PASS path

Fixed the PASS criteria and updating to .res file only once all the test steps are successful.

smuppand · 2025-12-09T10:57:48Z

Runner/suites/Kernel/Baseport/Storage/ufs_gear_lane/run.sh

+    exit 1
+fi
+
+scan_dmesg_errors "ufs" "$test_path"


smuppand · 2025-12-09T10:58:06Z

Runner/suites/Kernel/Baseport/Storage/ufs_gear_lane/run.sh

+    log_info "Gear: $CURRENT_GEAR (Expected: HS_GEAR$EXPECTED_GEAR)"
+    log_info "Lane: $CURRENT_LANE (Expected: 2)"
+    scan_dmesg_errors "ufs" "$test_path"
+    echo "$TESTNAME PASS" > "$res_file"


.res file is written PASS twice

Fixed the PASS criteria and updating to .res file only once all the test steps are successful.

smuppand · 2025-12-09T10:58:13Z

Runner/suites/Kernel/Baseport/Storage/ufs_gear_check/run.sh

+
+scan_dmesg_errors "ufs" "$test_path"
+log_pass "$TESTNAME completed successfully"
+echo "$TESTNAME PASS" > "$res_file"


smuppand · 2025-12-09T10:59:12Z

Runner/suites/Kernel/Baseport/Storage/ufs_hibern8/run.sh

+fi
+
+# Function to get the first matching node path
+get_dt_node_path() {


Please use already available functions from functestlib.sh

smuppand · 2026-01-02T18:23:28Z

Runner/suites/Kernel/Baseport/Storage/ufs_clock_scaling/run.sh

+if [ "$UFS_CLOCK_SCALED" -ne 1 ] && [ "$UFS_CLOCK_GATED" -ne 1 ]; then
+    log_fail "UFS clock scaling & gating test failed"
+    echo "$TESTNAME FAIL" > "$res_file"
+    exit 1


In Lava: keep in mind that if one of them fails and exits with non-zero status, the whole job terminates. So update all scripts to exit 0 for PASS/ FAIL/ SKIP

Changed all the exit codes to zero irrespective of test result.

- UFS Read/Writes - Verifies the UFS read/writes in the user partition - UFS Runtime Suspend/Resume - Ensure proper link_state transitions during load and no-load - UFS Clock Scaling/Gating - Ensures UFS clock is scaled and gated based on load and requirement - UFS Hibern8 - Validate active/hibern8 entry exit on-demand - UFS Write Booster - Check whether UFS Write Booster is enabled on write - UFS Gear Validation - Check gear switching on load - Fixed Review Comments Impact: These tests improve the functional coverage for UFS Signed-off-by: Jeevanandan Sandan <sandanka@qti.qualcomm.com>

smuppand

Centralize common UFS pre-checks
Every script repeats:

mandatory/optional config checks
DT node check
detect_ufs_partition_block

This should be a reusable helper in functestlib.sh, e.g.:
ufs_common_precheck that returns SKIP reason and exports UFS_BLOCK_DEV / UFS_SYSFS_BASE.
Less duplication = fewer inconsistencies.

smuppand · 2026-01-05T17:34:51Z

Runner/utils/functestlib.sh

+
+    attempt=1
+
+    echo "Checking status of node '$node_name', expecting: '$expected_status'"


Use log_info/log_warn/log_fail from functestlib (or keep it silent and let caller log).

smuppand · 2026-01-05T17:35:29Z

Runner/utils/functestlib.sh

+    attempt=1
+
+    echo "Checking status of node '$node_name', expecting: '$expected_status'"
+    echo "Will retry up to $retries times with $delay second(s) delay between attempts"


It's the same here as well.

smuppand · 2026-01-05T17:35:35Z

Runner/utils/functestlib.sh

+    echo "Will retry up to $retries times with $delay second(s) delay between attempts"
+
+    while [ "$attempt" -le "$retries" ]; do
+        echo "Attempt $attempt of $retries..."


It's the same here as well.

smuppand · 2026-01-05T17:35:42Z

Runner/utils/functestlib.sh

+        status=$(cat "$node_name")
+
+        if [ "$status" = "$expected_status" ]; then
+            echo "Success! Node '$node_name' has status: '$status'"


It's the same here as well.

smuppand · 2026-01-05T17:35:47Z

Runner/utils/functestlib.sh

+            echo "Success! Node '$node_name' has status: '$status'"
+            return 0
+        else
+            echo "Current status: '$status', waiting for: '$expected_status'"


It's the same here as well.

smuppand · 2026-01-05T17:56:59Z

Runner/suites/Kernel/Baseport/Storage/ufs_clock_scaling/run.sh

+# wait for 60 seconds for UFS driver to go to idle state
+log_info "Waiting for 60 seconds for UFS clock to gate"
+sync
+sleep 60


Wait times: 60s/180s hardcoded, will be flaky across boards.

Provide env overrides:
LOAD_MB=${LOAD_MB:-128}
IDLE_WAIT_SEC=${IDLE_WAIT_SEC:-60}

smuppand · 2026-01-05T17:59:01Z

Runner/suites/Kernel/Baseport/Storage/ufs_clock_scaling/run.sh

+UFS_MAX_FREQ=$(cat "$UFS_MAX_FREQ_NODE" 2>/dev/null)
+if [ -z "$UFS_MAX_FREQ" ]; then
+    log_skip "Failed to read max frequency from $UFS_MAX_FREQ_NODE"
+    echo "$TESTNAME FAIL" > "$res_file"


Failing to read max_freq currently leads to FAIL (good), but elsewhere similar read failures are treated as SKIP. Rule of thumb:
“Feature/sysfs node not present” → SKIP
“Node present but unreadable / invalid value” → FAIL (because kernel/userspace is broken)

smuppand · 2026-01-05T18:04:06Z

Runner/suites/Kernel/Baseport/Storage/ufs_clock_scaling/run.sh

+
+# Check for UFS clock max freq node and assign to variable
+log_info "Checking for UFS max clock freq node..."
+if ! check_dt_nodes "/sys/devices/platform/soc@0/*ufs*/devfreq/*ufs*/max_freq"; then


is too broad. Might match unreleated nodes or multiple controllers. Detect actual UFS dir once and reuse.
e.g., locate .../ufshcd* / .../ufs base and store as UFS_SYSFS.

smuppand · 2026-01-05T18:05:39Z

Runner/suites/Kernel/Baseport/Storage/ufs_clock_scaling/run.sh

+fi
+
+# Clean up temp file
+rm -f "$tmpfile"


For every test using temp files:
tmpfile=$(mktemp "$test_path/${TESTNAME}.XXXXXX") (fallback if mktemp not available)
trap 'rm -f "$tmpfile"' EXIT INT TERM
This avoids stale files and collisions.

smuppand · 2026-01-05T18:07:56Z

Runner/suites/Kernel/Baseport/Storage/ufs_read_writes/run.sh

+    if command -v stat >/dev/null 2>&1; then
+        stat --format="[INFO] Size: %s bytes File: %n" "$tmpfile"
+    else
+        find "$tmpfile" -printf "[INFO] Size: %s bytes File: %p\n"


find -printf is not POSIX and absent in busybox builds sometimes.
If stat isn’t available, just ls -l "$tmpfile" or wc -c < "$tmpfile".

jeevanandan-sandan force-pushed the feature/storage_test_automation branch from c1f2b5b to 68e6860 Compare November 28, 2025 11:27

vnarapar requested review from smuppand and vnarapar and removed request for vnarapar December 1, 2025 09:29

jeevanandan-sandan force-pushed the feature/storage_test_automation branch from 76dc353 to ce6cfca Compare December 2, 2025 09:54

vnarapar reviewed Dec 2, 2025

View reviewed changes

smuppand requested changes Dec 9, 2025

View reviewed changes

jeevanandan-sandan force-pushed the feature/storage_test_automation branch from ce6cfca to 4650502 Compare January 2, 2026 12:32

smuppand requested changes Jan 2, 2026

View reviewed changes

jeevanandan-sandan force-pushed the feature/storage_test_automation branch from 4650502 to a499f84 Compare January 5, 2026 06:35

jeevanandan-sandan force-pushed the feature/storage_test_automation branch from a499f84 to c0e80ff Compare January 5, 2026 11:06

smuppand requested changes Jan 5, 2026

View reviewed changes


		attempt=1

		echo "Checking status of node '$node_name', expecting: '$expected_status'"

Added UFS functional tests #220

Are you sure you want to change the base?

Added UFS functional tests #220

Uh oh!

Conversation

jeevanandan-sandan commented Nov 28, 2025

Uh oh!

vnarapar commented Dec 1, 2025

Uh oh!

vnarapar commented Dec 2, 2025

Uh oh!

vnarapar commented Dec 2, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smuppand left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smuppand left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels