| Message ID | 20240927112132.3927298-1-quic_wasimn@quicinc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] selftest: remoteproc: Add basic test for start/stop sequence | expand |
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}/$instance > + rproc_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}/$instance > + rproc_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 >
On Sun, Sep 29, 2024 at 10:03:23PM -0500, Bjorn Andersson wrote: > 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... As we are tesing only the core ops based on the availabe sysfs entries, it should work accross targets where remoteproc config is enabled & instances are available. Otherwise I am skipping the tests. Please correct me if I am missing anything here. I will try to elaborate purpose of the tests in next patch. > > > 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. > Will do the correction in next patch. > > > > 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}/$instance > > + rproc_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}/$instance > > + rproc_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? This is don't care case where I only intend to stop previous instances wherever applicable. After this I am expecting that all instances will be in stop state so that I can test start/stop sequence. This is inline to the previous review comments. I will add comments in code for further clarification. Thanks & Regards, Wasim > > 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 > > >
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}/$instance + rproc_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}/$instance + rproc_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 + +# 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
This test includes: 1) Start/stop test for each rproc instance sequencially 2) Start/stop test for all rproc instances concurrently Changes in v2: - Update commit message - Addressed start/stop flow Signed-off-by: Wasim Nazir <quic_wasimn@quicinc.com> -- 2.46.1