Added CoreSight TPDM automated Scripts#363
Added CoreSight TPDM automated Scripts#363Rohan-in-Qualcomm wants to merge 1 commit intoqualcomm-linux:mainfrom
Conversation
Rohan-in-Qualcomm
force-pushed
the
debug-tpdm
branch
2 times, most recently
from
ead3bd1 to
6ca0019
Compare
|
This is 1 commit covering 4 separate new testcases. I would recommend splitting it. These are related thematically, but they are still independent runners/docs/YAMLs and will be much easier to review/fix separately. |
| exit 1 | ||
| fi | ||
|
|
||
| if [ -z "$__INIT_ENV_LOADED" ]; then |
There was a problem hiding this comment.
Please use the unset-safe/idempotent repo pattern here:
if [ -z "${__INIT_ENV_LOADED:-}" ]; then
. "$INIT_ENV"
__INIT_ENV_LOADED=1
fi
The current form expands "$__INIT_ENV_LOADED" directly and does not set the marker after sourcing.
|
|
||
| if [ "$tpdm_found" -eq 0 ]; then | ||
| log_fail "Result: $TESTNAME FAIL (No TPDM devices found)" | ||
| echo "$TESTNAME FAIL" >> "$res_file" |
There was a problem hiding this comment.
Result file write should not append here:
echo "$TESTNAME FAIL" >> "$res_file"
Please use > instead of >>, otherwise repeated or earlier writes can leave inconsistent result-file contents.
| continue | ||
| fi | ||
|
|
||
| mode_atrr "$mode" |
There was a problem hiding this comment.
This is a real bug:
mode_atrr "$mode"
if mycmd; then
mycmd is undefined, so the return status of mode_atrr is never actually used. This should directly test mode_atrr "$mode" (or $?) instead of calling a non-existent command. As written, this test cannot correctly report attribute-read success/failure.
| echo 0 > "$tpdm_path/enable_source" 2>/dev/null | ||
| done | ||
|
|
||
| if [ "$tpdm_found" -eq 0 ]; then |
There was a problem hiding this comment.
There is a failure-to-pass bug here. When no TPDM device is found, the script writes FAIL:
if [ "$tpdm_found" -eq 0 ]; then
...
echo "$TESTNAME FAIL" > "$res_file"
fi
but retval is not updated and the code immediately below can still enter the PASS branch and overwrite the result with PASS. Please set retval=1 here or exit immediately after writing FAIL.
| # shellcheck disable=SC2329 | ||
| cleanup() { | ||
| reset_devices | ||
| if [ -f "$NPU_CLK" ]; then |
There was a problem hiding this comment.
cleanup() references NPU_CLK, but this variable is never defined anywhere in this script. So this cleanup path is currently dead/misleading. Please either define NPU_CLK like the integration runner does, or remove this branch.
| sleep 1 | ||
|
|
||
| if [ -c "/dev/$sink_dev" ]; then | ||
| cat "/dev/$sink_dev" > "/tmp/$sink_dev.bin" 2>/dev/null |
There was a problem hiding this comment.
The script claims to validate trace data reaching the sink, but here it only does:
cat "/dev/$sink_dev" > "/tmp/$sink_dev.bin" 2>/dev/null
and never checks whether the read succeeded or whether the output file is non-empty. Please validate the capture result (for example, file created and non-zero size) before treating the pair as successful. Otherwise this is only a best-effort dump, not a sink validation.
| ## Script Location | ||
|
|
||
| ``` | ||
| Runner/suites/Kernel/FunctionalArea/coresight/TPDM-Trace-Sink/run.sh |
There was a problem hiding this comment.
The documented script location is wrong:
Runner/suites/Kernel/FunctionalArea/coresight/TPDM-Trace-Sink/run.sh
but this PR adds the testcase under: Runner/suites/Kernel/DEBUG/TPDM-Trace-Sink/
Please update the README so the path matches the actual repo location.
|
|
||
| - Evaluate the enabling and disabling sequence of Coresight TPDM sources against TMC sinks. | ||
| - Verify that `enable_source` sysfs nodes accurately reflect read/write state transitions. | ||
| - Ensure raw binary trace data can be successfully read from the sink's character device node. |
There was a problem hiding this comment.
The README currently says the test ensures raw binary trace data can be successfully read from the sink device node, but the implementation only dumps /dev/$sink_dev to /tmp without checking success or output size. Please either strengthen the implementation or tone down the README claim so they match.
| for node in "$CS_BASE"/*; do | ||
| [ -d "$node" ] || continue | ||
| [ -f "$node/enable_source" ] && echo 0 > "$node/enable_source" 2>/dev/null | ||
| [ -f "$node/enable_sink" ] && echo 0 > "$node/enable_sink" 2>/dev/null |
There was a problem hiding this comment.
find_path() and reset_devices() logic is duplicated across multiple new TPDM runners. There does not appear to be an existing CoreSight/TPDM helper in the common libs today, but the duplication is already visible across this PR. Please consider moving the shared CoreSight helpers (sink discovery, source/sink reset, optional NPU clock control) into functestlib.sh or a dedicated shared helper file rather than open-coding them four times.
| for node in "$CS_BASE"/*; do | ||
| [ -d "$node" ] || continue | ||
| [ -f "$node/enable_source" ] && echo 0 > "$node/enable_source" 2>/dev/null | ||
| [ -f "$node/enable_sink" ] && echo 0 > "$node/enable_sink" 2>/dev/null |
There was a problem hiding this comment.
Same shared-helper point here: the ETF discovery/reset/NPU clock scaffolding is duplicated across the new TPDM runners. Factoring the common CoreSight plumbing into shared helpers would improve orchestration consistency and reduce drift between these scripts.
Ok I will separate them |
This patch adds a suite of automated scripts to validate the functionality and stability of CoreSight TPDM hardware: * TPDM-Interface-Access: Validates sysfs read/write accessibility for various TPDM dataset modes (DSB, CMB, TC, BC, GPR) to ensure the driver exposes and handles attributes correctly. * TPDM-Enable-Disable: Stress-tests the hardware and driver state machine by repeatedly toggling the TPDM source nodes on and off. Signed-off-by: Rohan Dutta <rohadutt@qti.qualcomm.com>
Rohan-in-Qualcomm
force-pushed
the
debug-tpdm
branch
from
6ca0019 to
87723be
Compare
2 participants
This patch adds/updates a suite of automated scripts to validate the functionality and stability of CoreSight TPDM Hardware: