Re: [PATCH v2] selftest: remoteproc: Add basic test for start/stop sequence

2024-10-03 Thread Wasim Nazir 
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 
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index a0cd96b8..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 
> > 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 ..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 ..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 ..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 sta

Re: [PATCH v2] selftest: remoteproc: Add basic test for start/stop sequence

2024-09-29 Thread Bjorn Andersson 
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 
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a0cd96b8..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 
> 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 ..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 ..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 ..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 -n

[PATCH v2] selftest: remoteproc: Add basic test for start/stop sequence

2024-09-27 Thread Wasim Nazir 
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 

diff --git a/MAINTAINERS b/MAINTAINERS
index a0cd96b8..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 
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 ..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 ..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 ..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