fix: update shmbridge testcase#446
Conversation
smuppand
left a comment
smuppand
left a comment
There was a problem hiding this comment.
The direction is good: this expands shmbridge from a simple qcom_scm presence/log check into stronger runtime validation for sysfs, DT, platform driver registration, binding, attributes, and uevent information.
A few changes are needed
This test validates the presence, registration, and correct runtime state of the qcom_scm driver on the target device Signed-off-by: Vamsee Narapareddi <vnarapar@qti.qualcomm.com>
| fi | ||
|
|
||
| # shellcheck disable=SC1090 | ||
| . "$INIT_ENV" |
There was a problem hiding this comment.
init_env is sourced unconditionally.
Not a functional blocker, but it is inconsistent with the current repo convention and can re-source init_env unnecessarily if launched through a wrapper that already loaded it.
Recommended fix:
if [ -z "${__INIT_ENV_LOADED:-}" ]; then
# shellcheck disable=SC1090
. "$INIT_ENV"
fi
| echo "$TESTNAME SKIP" >"$res_file" | ||
| log_info "Checking if required tools are available..." | ||
| if ! check_dependencies grep find basename cat readlink head; then | ||
| log_skip "$TESTNAME SKIP - missing required grep/find utility" |
There was a problem hiding this comment.
The dependency list is now fixed, but the log message still only says grep/find.
If basename, cat, readlink, or head is missing, the skip message will be misleading.
| log_pass "qcom_scm platform driver is registered on the platform bus." | ||
| else | ||
| log_fail "qcom_scm platform driver is NOT registered on the platform bus." | ||
| fail_flag=1 |
There was a problem hiding this comment.
The hard check is fine, but if it fails, it still does not print available SCM-like platform drivers.
If the driver name/path changes across kernel versions, the failure will not show enough context.
| log_info "--- TEE/TrustZone device node presence ---" | ||
| tee_found=0 | ||
| for tee_node in /dev/tee0 /dev/teepriv0; do | ||
| if [ -c "$tee_node" ]; then |
There was a problem hiding this comment.
README still says TEE node presence is mandatory.
Impact: Script behavior and documentation disagree.
Recommended fix: Update README table and note. See README comment below.
| | 4 | **Driver-to-device binding** | Symlinks under `/sys/bus/platform/drivers/qcom_scm/` | Yes | | ||
| | 5 | **sysfs attribute readability** | `/sys/module/qcom_scm/parameters/*` | Yes | | ||
| | 6 | **Device uevent / modalias** | `<bound_device>/uevent` | Informational | | ||
| | 7 | **TEE / TrustZone device node** | `/dev/tee0`, `/dev/teepriv0` | Yes | |
There was a problem hiding this comment.
Current script treats missing TEE nodes as WARN, not FAIL.
Impact: Documentation gives the wrong test expectation.
Recommended fix:
| 7 | TEE / TrustZone device node | /dev/tee0, /dev/teepriv0 | Informational |
or:
| 7 | TEE / TrustZone device node | /dev/tee0, /dev/teepriv0 | Warning only |
|
|
||
| > **Informational checks** log their findings but do not cause the test to WARN | ||
| > when the path is absent. This handles platforms where `CONFIG_TEE` or runtime | ||
| > PM exposure is optional. |
There was a problem hiding this comment.
This sentence is confusing. Informational checks do not cause the test to fail, but they may log a warning. The current script logs log_warn for missing TEE nodes.
Impact: The README wording contradicts the script and is hard to understand.
Recommended fix:
Informational checks may log warnings, but they do not cause the test to
FAIL when the path is absent. This handles platforms whereCONFIG_TEEor
related runtime exposure is optional.
| 2. **Verify Transfer**: Ensure that the repo has been successfully copied to the target device. | ||
| 3. **Run Scripts**: Navigate to the directory where these files are copied on the target device and execute the scripts as needed. | ||
| | Requirement | Notes | | ||
| |-------------|-------| |
There was a problem hiding this comment.
Issue: This describes the check but does not state that absence is warning-only.
Impact: Users may still assume missing TEE nodes fail the test.
Recommended fix:
- TEE / TrustZone device node — Checks for the character devices
/dev/tee0
and/dev/teepriv0. These are created by the TEE subsystem whenCONFIG_TEE
is enabled. Absence is logged as a warning only and does not fail the test.
2 participants
This test validates the presence, registration, and correct runtime state of the qcom_scm driver on the target device