On Fri, Sep 27, 2024 at 04:51:32PM GMT, Wasim Nazir wrote:
This test includes: 1) Start/stop test for each rproc instance sequencially 2) Start/stop test for all rproc instances concurrently
This fails to describe the purpose of the patch. Provide a proper commit mesasge.
In particular, I expect an argumentation for your test scheme. Will this work across all remoteproc instances? Does it have any dependencies, etc...
Changes in v2:
- Update commit message
- Addressed start/stop flow
The changelog goes below the '---' line, adjacent to your diffstat - which is missing from your patch. I don't know how you're sending these patches, but your system is either configured weirdly or you're not following my instructions on go/upstream.
Signed-off-by: Wasim Nazir quic_wasimn@quicinc.com
diff --git a/MAINTAINERS b/MAINTAINERS index a77770cd96b8..02ebad5ae790 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -19596,6 +19596,7 @@ F: Documentation/staging/remoteproc.rst F: drivers/remoteproc/ F: include/linux/remoteproc.h F: include/linux/remoteproc/ +F: tools/testing/selftests/remoteproc/
REMOTE PROCESSOR MESSAGING (RPMSG) SUBSYSTEM M: Bjorn Andersson andersson@kernel.org diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile index b38199965f99..0c8a0f427d01 100644 --- a/tools/testing/selftests/Makefile +++ b/tools/testing/selftests/Makefile @@ -82,6 +82,7 @@ TARGETS += proc TARGETS += pstore TARGETS += ptrace TARGETS += openat2 +TARGETS += remoteproc TARGETS += resctrl TARGETS += riscv TARGETS += rlimits diff --git a/tools/testing/selftests/remoteproc/Makefile b/tools/testing/selftests/remoteproc/Makefile new file mode 100644 index 000000000000..a84b3934fd36 --- /dev/null +++ b/tools/testing/selftests/remoteproc/Makefile @@ -0,0 +1,4 @@ +# SPDX-License-Identifier: GPL-2.0 +TEST_PROGS := remoteproc_test.sh
+include ../lib.mk diff --git a/tools/testing/selftests/remoteproc/config b/tools/testing/selftests/remoteproc/config new file mode 100644 index 000000000000..a5c237d2f3b4 --- /dev/null +++ b/tools/testing/selftests/remoteproc/config @@ -0,0 +1 @@ +CONFIG_REMOTEPROC=y diff --git a/tools/testing/selftests/remoteproc/remoteproc_test.sh b/tools/testing/selftests/remoteproc/remoteproc_test.sh new file mode 100644 index 000000000000..589368285307 --- /dev/null +++ b/tools/testing/selftests/remoteproc/remoteproc_test.sh @@ -0,0 +1,134 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0 +# +# Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved. +#
+DIR="$(dirname $(readlink -f "$0"))"
+KTAP_HELPERS="${DIR}/../kselftest/ktap_helpers.sh" +if [ -e "$KTAP_HELPERS" ]; then
- . "$KTAP_HELPERS"
+else
- echo -n "1..0 # SKIP $KTAP_HELPERS file not found"
- exit 4
+fi
+RPROC_SYS=/sys/class/remoteproc +RPROC_SEQ_SLEEP=5 +rproc_instances= +# Declare an array to save initial states of each instance +org_instance_to_state="" +num_tests=0 +test_err=0
+check_error() {
- if [ $? -ne 0 ]; then
test_err=$((test_err+1))ktap_print_msg "$@"- fi
+}
+rproc_stop_instances() {
- for instance in ${rproc_instances}; do
rproc=${RPROC_SYS}/$instancerproc_name=$(cat $rproc/name)rproc_state=$(cat $rproc/state)echo stop > "$rproc/state"check_error "$rproc_name state-stop failed at state $rproc_state"- done
- sleep ${RPROC_SEQ_SLEEP}
+}
+rproc_start_instances() {
- for instance in ${rproc_instances}; do
rproc=${RPROC_SYS}/$instancerproc_name=$(cat $rproc/name)rproc_state=$(cat $rproc/state)echo start > "$rproc/state"check_error "$rproc_name state-start failed at state $rproc_state"- done
- sleep ${RPROC_SEQ_SLEEP}
+}
+rproc_seq_test_instance_one() {
- instance=$1
- rproc=${RPROC_SYS}/$instance
- rproc_name=$(cat $rproc/name)
- rproc_state=$(cat $rproc/state)
- ktap_print_msg "Testing rproc sequence for $rproc_name"
- # Reset test_err value
- test_err=0
- # Begin start/stop sequence
- echo start > "$rproc/state"
- check_error "$rproc_name state-start failed at state $rproc_state"
- sleep ${RPROC_SEQ_SLEEP}
- echo stop > "$rproc/state"
- check_error "$rproc_name state-stop failed at state $rproc_state"
- if [ $test_err -ne 0 ]; then
ktap_test_fail "$rproc_name"- else
ktap_test_pass "$rproc_name"- fi
+}
+rproc_seq_test_instances_concurrently() {
- # Reset test_err value
- test_err=0
- rproc_start_instances
- rproc_stop_instances
- if [ $test_err -ne 0 ]; then
ktap_test_fail "for any of $rproc_instances"- else
ktap_test_pass "for all $rproc_instances"- fi
+}
+ktap_print_header
+if [ ! -d "${RPROC_SYS}" ]; then
- ktap_skip_all "${RPROC_SYS} doesn't exist."
- exit "${KSFT_SKIP}"
+fi
+rproc_instances=$(find ${RPROC_SYS}/remoteproc* -maxdepth 1 -exec basename {} ;) +num_tests=$(echo ${rproc_instances} | wc -w) +if [ "${num_tests}" -eq 0 ]; then
- ktap_skip_all "${RPROC_SYS}/remoteproc* doesn't exist."
- exit "${KSFT_SKIP}"
+fi
+# Total tests will be: +# 1) Seq tests for each instance sequencially +# 2) Seq tests for all instances concurrently +num_tests=$((num_tests+1))
+ktap_set_plan "${num_tests}"
+# Stop all instances +rproc_stop_instances
Will this not fail for remoteproc instances that aren't started automatically?
Regards, Bjorn
+# Test 1 +ktap_print_msg "Testing rproc start/stop sequence for each instance sequencially" +for instance in ${rproc_instances}; do
- rproc_seq_test_instance_one $instance
+done
+# Test 2 +ktap_print_msg "Testing rproc start/stop sequence for all instances concurrently" +rproc_seq_test_instances_concurrently
+# Restore all instances +rproc_start_instances
+ktap_finished
2.46.1