On Fri, Sep 20, 2024 at 11:28:42PM GMT, Wasim Nazir wrote: > Add new basic remoteproc test that check start/stop > sequence of all subsystems available. > Please describe your test scenario more than just "check start/stop sequence". Signed-off-by... > diff --git a/MAINTAINERS b/MAINTAINERS > index e062b5328341..aff76edc4242 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -18225,6 +18225,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@xxxxxxxxxx> > diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile > index 697f13bbbc32..31db0311efdc 100644 > --- a/tools/testing/selftests/Makefile > +++ b/tools/testing/selftests/Makefile > @@ -68,6 +68,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..88c8f15d8406 > --- /dev/null > +++ b/tools/testing/selftests/remoteproc/remoteproc_test.sh > @@ -0,0 +1,165 @@ > +#!/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 > + source "$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_ss_files= > +num_tests=0 > +test_err=0 > + > +check_error() { > + if [ $? -ne 0 ]; then > + test_err=$((test_err+1)) > + ktap_print_msg "$@" > + fi > +} > + > +rproc_seq_test_ss_one() { > + ss=$1 "ss" or "subsystem" are Qualcomm terms, please use "remoteproc instance" instead. > + rproc=${RPROC_SYS}/$ss > + rproc_name=$(cat $rproc/name) > + rproc_state=$(cat $rproc/state) > + rproc_ssr=$(cat $rproc/recovery) > + ktap_print_msg "Testing rproc sequence for $rproc_name" > + > + # Reset test_err value > + test_err=0 > + if [ "$rproc_ssr" != "enabled" ]; then > + echo enabled > $rproc/recovery > + check_error "$rproc_name SSR-enabled failed" Same with "SSR", you can express this with standard terms. Why do we need "recovery" enabled in order to perform start/stop or stop/start testing? Doesn't recovery only affect the crash code path? > + fi > + > + if [ "$rproc_state" != "running" ]; then I'd like to see your arguments in the commit message, or a comment here, of why you do either start/stop or stop/start - instead of e.g. make sure they are all stopped and then start/stop them in the test. PS. Please use check go/upstream and adopt b4. Regards, Bjorn > + echo start > "$rproc/state" > + check_error "$rproc_name state-start failed" > + > + sleep ${RPROC_SEQ_SLEEP} > + > + echo stop > "$rproc/state" > + check_error "$rproc_name state-stop failed" > + else > + echo stop > "$rproc/state" > + check_error "$rproc_name state-stop failed" > + > + sleep ${RPROC_SEQ_SLEEP} > + > + echo start > "$rproc/state" > + check_error "$rproc_name state-start failed" > + fi > + > + if [ $test_err -ne 0 ]; then > + ktap_test_fail "$rproc_name" > + else > + ktap_test_pass "$rproc_name" > + fi > +}