All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH] uart: add uart testcase in kernel device-driver
@ 2020-03-18  5:31 Cixi Geng
  2020-03-19 10:28 ` Petr Vorel
  0 siblings, 1 reply; 5+ messages in thread
From: Cixi Geng @ 2020-03-18  5:31 UTC (permalink / raw)
  To: ltp

Porting UART test from ltp-ddt back to ltp. only test 115200 UART_RATE.

[TODO] support more rate, and add test HWFLOW function test.

Signed-off-by: Orson Zhai <orson@gmail.com>
Signed-off-by: Cixi Geng <gengcixi@gmail.com>
---
 runtest/kernel_ddt                            |   1 +
 testcases/kernel/device-drivers/Makefile      |   1 +
 testcases/kernel/device-drivers/uart/Makefile |  22 ++++
 .../kernel/device-drivers/uart/serialcheck.sh | 111 ++++++++++++++++++
 4 files changed, 135 insertions(+)
 create mode 100644 runtest/kernel_ddt
 create mode 100644 testcases/kernel/device-drivers/uart/Makefile
 create mode 100755 testcases/kernel/device-drivers/uart/serialcheck.sh

diff --git a/runtest/kernel_ddt b/runtest/kernel_ddt
new file mode 100644
index 000000000..30e9a0269
--- /dev/null
+++ b/runtest/kernel_ddt
@@ -0,0 +1 @@
+uart serialcheck.sh
diff --git a/testcases/kernel/device-drivers/Makefile
b/testcases/kernel/device-drivers/Makefile
index 55e0d25a0..a214f211b 100644
--- a/testcases/kernel/device-drivers/Makefile
+++ b/testcases/kernel/device-drivers/Makefile
@@ -27,6 +27,7 @@ SUBDIRS := acpi \
    rtc \
    tbio \
    uaccess \
+   uart \
    zram

 include $(top_srcdir)/include/mk/generic_trunk_target.mk
diff --git a/testcases/kernel/device-drivers/uart/Makefile
b/testcases/kernel/device-drivers/uart/Makefile
new file mode 100644
index 000000000..0d73f6635
--- /dev/null
+++ b/testcases/kernel/device-drivers/uart/Makefile
@@ -0,0 +1,22 @@
+# Copyright (c) 2015 Oracle and/or its affiliates. All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation; either version 2 of
+# the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+
+top_srcdir ?= ../../../..
+include $(top_srcdir)/include/mk/testcases.mk
+
+INSTALL_TARGETS := *.sh
+
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/device-drivers/uart/serialcheck.sh
b/testcases/kernel/device-drivers/uart/serialcheck.sh
new file mode 100755
index 000000000..f4cf13e02
--- /dev/null
+++ b/testcases/kernel/device-drivers/uart/serialcheck.sh
@@ -0,0 +1,111 @@
+#!/bin/sh
+###############################################################################
+#
+# Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com/
+# Copyright (C) 2019, Unisoc Communications Inc.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation version 2.
+#
+# This program is distributed "as is" WITHOUT ANY WARRANTY of any
+# kind, whether express or implied; without even the implied warranty
+# of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+###############################################################################
+
+# @desc Test UART ports using git://
git.breakpoint.cc/bigeasy/serialcheck.git
+
+#### Functions definitions ####
+usage()
+{
+    echo "usage: ./${0##*/} [-r UART_RATE] [-l LOOPS] [-x to enable HW
flow control]"
+    exit 1
+}
+
+# Default values
+: ${UART_RATE:=115200}
+: ${UART_LOOPS:=5}
+: ${UART_HWFLOW:=0}
+
+PORTS_TO_TEST=();
+UART_PORTS=();
+ARRAY=(`find /sys/class/tty/*/uartclk`);
+
+check_requirements()
+{
+ which serialcheck
+ ret=$?
+ if [ $ret -eq 0 ];then
+ tst_res TINFO "serialcheck command is in system,continue to test"
+ else
+ tst_brk TCONF "test failed for lack of requirement,returned is $ret"
+ fi
+
+}
+
+create_test_file()
+{
+    temp_test_file=`mktemp`
+    dd if=/dev/urandom of=$temp_test_file count=1 bs=$((UART_RATE / 2))
+}
+
+get_uart_ports()
+{
+for i in ${ARRAY[@]}; do
+    PORT=/dev/`echo $i | cut -d'/' -f 5`
+    # Activate port in case it will be initialized only when startup
+    echo "DDT TESTING" > $PORT 2>/dev/null
+    if [ `cat $i` -ne 0 ]; then
+        UART_PORTS=("${UART_PORTS[@]}" "$PORT")
+    fi
+done
+}
+
+filter_out_used_ports()
+{
+ which lsof
+ ret=$?
+ if [ $ret -eq 0 ];then
+ tst_res TINFO "lsof command exist, filter out used ports";
+ else
+ tst_brk TCONF "test failed for lack of requirement,returned is $ret"
+ fi
+
+    for i in ${UART_PORTS[@]}; do
+        lsof | grep $i &> /dev/null ||
PORTS_TO_TEST=("${PORTS_TO_TEST[@]}" $i)
+    done
+}
+
+run_serial_test()
+{
+ create_test_file
+    for i in ${PORTS_TO_TEST[@]}; do
+        if [ $UART_HWFLOW -eq 0 ]; then
+            { sleep 1; serialcheck -b $UART_RATE -d $i -f $temp_test_file
-l $UART_LOOPS -m t -k; }&
+            PID=$!
+            serialcheck -b $UART_RATE -d $i -f $temp_test_file -l
$UART_LOOPS -m r -k || { kill -- -$PID 2>/dev/null; tst_res TFAIL "TEST
FAILED"; }
+        else
+            { sleep 1; serialcheck -b $UART_RATE -d $i -f $temp_test_file
-l $UART_LOOPS -m t -h; } &
+            PID=$!
+            serialcheck -b $UART_RATE -d $i -f $temp_test_file -l
$UART_LOOPS -m r -h || { kill -- -$PID 2>/dev/null; tst_res TFAIL "TEST
FAILED"; }
+        fi
+    done
+    rm $temp_test_file
+ tst_res TPASS "uart test passed"
+}
+
+
+TST_TESTFUNC=do_test
+. tst_test.sh
+
+do_test()
+{
+ check_requirements
+ get_uart_ports
+ filter_out_used_ports
+ run_serial_test
+}
+
+tst_run
-- 
2.17.1
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20200318/cce05c12/attachment-0001.htm>

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [LTP] [PATCH] uart: add uart testcase in kernel device-driver
  2020-03-18  5:31 [LTP] [PATCH] uart: add uart testcase in kernel device-driver Cixi Geng
@ 2020-03-19 10:28 ` Petr Vorel
  2020-03-19 15:07   ` Cixi Geng
  0 siblings, 1 reply; 5+ messages in thread
From: Petr Vorel @ 2020-03-19 10:28 UTC (permalink / raw)
  To: ltp

Hi Cixi,

> Porting UART test from ltp-ddt back to ltp. only test 115200 UART_RATE.
Thanks for your effort. There are several problems with this test, I'll try to
address them all.

> [TODO] support more rate, and add test HWFLOW function test.
Could this be in v2?

> diff --git a/runtest/kernel_ddt b/runtest/kernel_ddt
> new file mode 100644
> index 000000000..30e9a0269
> --- /dev/null
> +++ b/runtest/kernel_ddt
> @@ -0,0 +1 @@
> +uart serialcheck.sh
I wonder if there needs to be in it's own runtest file.
Maybe yes, but I'd propose more meaningful name (serial, uart).
What is ddt anyway?

> diff --git a/testcases/kernel/device-drivers/Makefile
> b/testcases/kernel/device-drivers/Makefile
> index 55e0d25a0..a214f211b 100644
> --- a/testcases/kernel/device-drivers/Makefile
> +++ b/testcases/kernel/device-drivers/Makefile
> @@ -27,6 +27,7 @@ SUBDIRS := acpi \
>     rtc \
>     tbio \
>     uaccess \
> +   uart \
>     zram

>  include $(top_srcdir)/include/mk/generic_trunk_target.mk
> diff --git a/testcases/kernel/device-drivers/uart/Makefile
> b/testcases/kernel/device-drivers/uart/Makefile
> new file mode 100644
> index 000000000..0d73f6635
> --- /dev/null
> +++ b/testcases/kernel/device-drivers/uart/Makefile
> @@ -0,0 +1,22 @@
> +# Copyright (c) 2015 Oracle and/or its affiliates. All Rights Reserved.
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License as
> +# published by the Free Software Foundation; either version 2 of
> +# the License, or (at your option) any later version.
> +#
> +# This program is distributed in the hope that it would be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write the Free Software Foundation,
> +# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
Please next time instead of verbose GPL text ^ use just:
# SPDX-License-Identifier: GPL-2.0-or-later

> +
> +top_srcdir ?= ../../../..
> +include $(top_srcdir)/include/mk/testcases.mk
> +
> +INSTALL_TARGETS := *.sh
> +
> +include $(top_srcdir)/include/mk/generic_leaf_target.mk
> diff --git a/testcases/kernel/device-drivers/uart/serialcheck.sh
> b/testcases/kernel/device-drivers/uart/serialcheck.sh
> new file mode 100755
> index 000000000..f4cf13e02
> --- /dev/null
> +++ b/testcases/kernel/device-drivers/uart/serialcheck.sh
> @@ -0,0 +1,111 @@
> +#!/bin/sh
> +###############################################################################
> +#
> +# Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com/
> +# Copyright (C) 2019, Unisoc Communications Inc.
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License as
> +# published by the Free Software Foundation version 2.
> +#
> +# This program is distributed "as is" WITHOUT ANY WARRANTY of any
> +# kind, whether express or implied; without even the implied warranty
> +# of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +###############################################################################
And here ^:
# SPDX-License-Identifier: GPL-2.0-or-later
> +
> +# @desc Test UART ports using git://
> git.breakpoint.cc/bigeasy/serialcheck.git
It looks your mailer wraps lines, that's unusable for applying.
Could you use git format-patch and git send-email for generating patches and
sending them?

> +
> +#### Functions definitions ####
Please avoid these useless comments.

> +usage()
> +{
> +    echo "usage: ./${0##*/} [-r UART_RATE] [-l LOOPS] [-x to enable HW
> flow control]"
Also here is wrapped.

But you're supposed to use TST_OPTS TST_PARSE_ARGS and TST_USAGE from The API
https://github.com/linux-test-project/ltp/wiki/Test-Writing-Guidelines#233-optional-command-line-parameters

> +    exit 1
> +}
> +
> +# Default values
> +: ${UART_RATE:=115200}
> +: ${UART_LOOPS:=5}
> +: ${UART_HWFLOW:=0}
Even this is a valid syntax, please use more convinient:
UART_RATE="${UART_RATE:=115200}"

> +
> +PORTS_TO_TEST=();
> +UART_PORTS=();
> +ARRAY=(`find /sys/class/tty/*/uartclk`);
Arrays are bashisms (bash specific), we don't allow them in LTP, as we require
POSIX shell syntax, which is portable (some systems doesn't have bash, but dash
or other shell e.g. toybox or busybox on Android. I guess you target embedded
system with this test):
https://github.com/linux-test-project/ltp/wiki/Test-Writing-Guidelines#132-shell-coding-style

Most of the time arrays can be replaced by string separated by space.
If you need these devices, in /sys/class/tty/*/uartclk, it could be done like:
ports=$(for i in /sys/class/tty/*/uartclk ; do echo $i | cut -d '/' -f 5; done)

> +
> +check_requirements()
> +{
> + which serialcheck


> + ret=$?
> + if [ $ret -eq 0 ];then
> + tst_res TINFO "serialcheck command is in system,continue to test"
> + else
> + tst_brk TCONF "test failed for lack of requirement,returned is $ret"
> + fi
> +
Useless blank line here.
> +}
TINFO is not much useful, I'd avoid that. And check_requirements should be a setup function, not called directly in do_test:
TST_SETUP=check_requirements

But given that whole function just check serialcheck, whole function should be
replaced just by:

TST_NEEDS_CMDS="serialcheck"
https://github.com/linux-test-project/ltp/wiki/Test-Writing-Guidelines#232-library-environment-variables-for-shell

BTW serialcheck source isn't probably packaged in distros
https://git.breakpoint.cc/cgit/bigeasy/serialcheck.git/tree/serialcheck.c
Maybe we could just adapt it to LTP and include it as well (if we consider whole
testing useful).

> +
> +create_test_file()
> +{
> +    temp_test_file=`mktemp`
I guess you need to use TST_NEEDS_TMPDIR=1
and then just any file in it. Or use $$ (e.g. file.$$) if you want to have
concurrency, but we usually don't care).
> +    dd if=/dev/urandom of=$temp_test_file count=1 bs=$((UART_RATE / 2))
You also need to add dd to TST_NEEDS_CMDS.

> +}
> +
> +get_uart_ports()
> +{
> +for i in ${ARRAY[@]}; do
> +    PORT=/dev/`echo $i | cut -d'/' -f 5`
Well, you use cut yourself, so why that complicated code with arrays?

> +    # Activate port in case it will be initialized only when startup
> +    echo "DDT TESTING" > $PORT 2>/dev/null
> +    if [ `cat $i` -ne 0 ]; then
> +        UART_PORTS=("${UART_PORTS[@]}" "$PORT")
> +    fi
> +done
> +}
> +
> +filter_out_used_ports()
> +{
> + which lsof
> + ret=$?
> + if [ $ret -eq 0 ];then
> + tst_res TINFO "lsof command exist, filter out used ports";
> + else
> + tst_brk TCONF "test failed for lack of requirement,returned is $ret"
> + fi
Again whole block is useless, just add lsof into TST_NEEDS_CMDS.

> +
> +    for i in ${UART_PORTS[@]}; do
> +        lsof | grep $i &> /dev/null ||
> PORTS_TO_TEST=("${PORTS_TO_TEST[@]}" $i)
> +    done
> +}
> +
> +run_serial_test()
> +{
> + create_test_file
create_test_file should be just 2 lines of code in setup function.

> +    for i in ${PORTS_TO_TEST[@]}; do
Array => bashism :(.

> +        if [ $UART_HWFLOW -eq 0 ]; then
> +            { sleep 1; serialcheck -b $UART_RATE -d $i -f $temp_test_file
> -l $UART_LOOPS -m t -k; }&
Again line wrapped :(

> +            PID=$!
> +            serialcheck -b $UART_RATE -d $i -f $temp_test_file -l
Hm, why do you run 2 instances?

> $UART_LOOPS -m r -k || { kill -- -$PID 2>/dev/null; tst_res TFAIL "TEST
> FAILED"; }
Using complicated code in { } isn't much readable. I'd put it into:
if ! $UART_LOOPS -m r -k; then
	kill ...
fi

> +        else
> +            { sleep 1; serialcheck -b $UART_RATE -d $i -f $temp_test_file
> -l $UART_LOOPS -m t -h; } &
> +            PID=$!
> +            serialcheck -b $UART_RATE -d $i -f $temp_test_file -l
> $UART_LOOPS -m r -h || { kill -- -$PID 2>/dev/null; tst_res TFAIL "TEST
> FAILED"; }

This can be written better to not repeat much yourself.
Whole if and else block is the same exept -h and -k parameter are different.
Why not put this extra parameter into variable?
Why sleep added into {} block?

> +        fi
> +    done
> +    rm $temp_test_file
> + tst_res TPASS "uart test passed"
> +}
> +
> +
> +TST_TESTFUNC=do_test
> +. tst_test.sh
We usually put this at the beginning of the test.
Please see some tests as examples (testcases/commands/df/df01.sh,
testcases/commands/mkfs/mkfs01.sh, testcases/commands/lsmod/lsmod01.sh, ...)

> +
> +do_test()
> +{
> + check_requirements

> + get_uart_ports
> + filter_out_used_ports
> + run_serial_test
> +}
> +
> +tst_run

Kind regards,
Petr

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [LTP] [PATCH] uart: add uart testcase in kernel device-driver
  2020-03-19 10:28 ` Petr Vorel
@ 2020-03-19 15:07   ` Cixi Geng
  2020-03-19 18:43     ` Petr Vorel
  0 siblings, 1 reply; 5+ messages in thread
From: Cixi Geng @ 2020-03-19 15:07 UTC (permalink / raw)
  To: ltp

Hi Petr:
Thank you for taking the trouble to help me. I do appreciate it.
Sorry about the so many mistakes,
I will fix the code in next vesion.

Thanks again for you help!

Petr Vorel <pvorel@suse.cz> ?2020?3?19??? ??6:28???

> Hi Cixi,
>
> > Porting UART test from ltp-ddt back to ltp. only test 115200 UART_RATE.
> Thanks for your effort. There are several problems with this test, I'll
> try to
> address them all.
>
> > [TODO] support more rate, and add test HWFLOW function test.
> Could this be in v2?
>
> > diff --git a/runtest/kernel_ddt b/runtest/kernel_ddt
> > new file mode 100644
> > index 000000000..30e9a0269
> > --- /dev/null
> > +++ b/runtest/kernel_ddt
> > @@ -0,0 +1 @@
> > +uart serialcheck.sh
> I wonder if there needs to be in it's own runtest file.
> Maybe yes, but I'd propose more meaningful name (serial, uart).
> What is ddt anyway?
>
> > diff --git a/testcases/kernel/device-drivers/Makefile
> > b/testcases/kernel/device-drivers/Makefile
> > index 55e0d25a0..a214f211b 100644
> > --- a/testcases/kernel/device-drivers/Makefile
> > +++ b/testcases/kernel/device-drivers/Makefile
> > @@ -27,6 +27,7 @@ SUBDIRS := acpi \
> >     rtc \
> >     tbio \
> >     uaccess \
> > +   uart \
> >     zram
>
> >  include $(top_srcdir)/include/mk/generic_trunk_target.mk
> > diff --git a/testcases/kernel/device-drivers/uart/Makefile
> > b/testcases/kernel/device-drivers/uart/Makefile
> > new file mode 100644
> > index 000000000..0d73f6635
> > --- /dev/null
> > +++ b/testcases/kernel/device-drivers/uart/Makefile
> > @@ -0,0 +1,22 @@
> > +# Copyright (c) 2015 Oracle and/or its affiliates. All Rights Reserved.
> > +#
> > +# This program is free software; you can redistribute it and/or
> > +# modify it under the terms of the GNU General Public License as
> > +# published by the Free Software Foundation; either version 2 of
> > +# the License, or (at your option) any later version.
> > +#
> > +# This program is distributed in the hope that it would be useful,
> > +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > +# GNU General Public License for more details.
> > +#
> > +# You should have received a copy of the GNU General Public License
> > +# along with this program; if not, write the Free Software Foundation,
> > +# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> Please next time instead of verbose GPL text ^ use just:
> # SPDX-License-Identifier: GPL-2.0-or-later
>
> > +
> > +top_srcdir ?= ../../../..
> > +include $(top_srcdir)/include/mk/testcases.mk
> > +
> > +INSTALL_TARGETS := *.sh
> > +
> > +include $(top_srcdir)/include/mk/generic_leaf_target.mk
> > diff --git a/testcases/kernel/device-drivers/uart/serialcheck.sh
> > b/testcases/kernel/device-drivers/uart/serialcheck.sh
> > new file mode 100755
> > index 000000000..f4cf13e02
> > --- /dev/null
> > +++ b/testcases/kernel/device-drivers/uart/serialcheck.sh
> > @@ -0,0 +1,111 @@
> > +#!/bin/sh
> >
> +###############################################################################
> > +#
> > +# Copyright (C) 2015 Texas Instruments Incorporated -
> http://www.ti.com/
> > +# Copyright (C) 2019, Unisoc Communications Inc.
> > +#
> > +# This program is free software; you can redistribute it and/or
> > +# modify it under the terms of the GNU General Public License as
> > +# published by the Free Software Foundation version 2.
> > +#
> > +# This program is distributed "as is" WITHOUT ANY WARRANTY of any
> > +# kind, whether express or implied; without even the implied warranty
> > +# of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > +# GNU General Public License for more details.
> > +#
> >
> +###############################################################################
> And here ^:
> # SPDX-License-Identifier: GPL-2.0-or-later
> > +
> > +# @desc Test UART ports using git://
> > git.breakpoint.cc/bigeasy/serialcheck.git
> It looks your mailer wraps lines, that's unusable for applying.
> Could you use git format-patch and git send-email for generating patches
> and
> sending them?
>
> > +
> > +#### Functions definitions ####
> Please avoid these useless comments.
>
> > +usage()
> > +{
> > +    echo "usage: ./${0##*/} [-r UART_RATE] [-l LOOPS] [-x to enable HW
> > flow control]"
> Also here is wrapped.
>
> But you're supposed to use TST_OPTS TST_PARSE_ARGS and TST_USAGE from The
> API
>
> https://github.com/linux-test-project/ltp/wiki/Test-Writing-Guidelines#233-optional-command-line-parameters
>
> > +    exit 1
> > +}
> > +
> > +# Default values
> > +: ${UART_RATE:=115200}
> > +: ${UART_LOOPS:=5}
> > +: ${UART_HWFLOW:=0}
> Even this is a valid syntax, please use more convinient:
> UART_RATE="${UART_RATE:=115200}"
>
> > +
> > +PORTS_TO_TEST=();
> > +UART_PORTS=();
> > +ARRAY=(`find /sys/class/tty/*/uartclk`);
> Arrays are bashisms (bash specific), we don't allow them in LTP, as we
> require
> POSIX shell syntax, which is portable (some systems doesn't have bash, but
> dash
> or other shell e.g. toybox or busybox on Android. I guess you target
> embedded
> system with this test):
>
> https://github.com/linux-test-project/ltp/wiki/Test-Writing-Guidelines#132-shell-coding-style
>
> Most of the time arrays can be replaced by string separated by space.
> If you need these devices, in /sys/class/tty/*/uartclk, it could be done
> like:
> ports=$(for i in /sys/class/tty/*/uartclk ; do echo $i | cut -d '/' -f 5;
> done)
>
> > +
> > +check_requirements()
> > +{
> > + which serialcheck
>
>
> > + ret=$?
> > + if [ $ret -eq 0 ];then
> > + tst_res TINFO "serialcheck command is in system,continue to test"
> > + else
> > + tst_brk TCONF "test failed for lack of requirement,returned is $ret"
> > + fi
> > +
> Useless blank line here.
> > +}
> TINFO is not much useful, I'd avoid that. And check_requirements should be
> a setup function, not called directly in do_test:
> TST_SETUP=check_requirements
>
> But given that whole function just check serialcheck, whole function
> should be
> replaced just by:
>
> TST_NEEDS_CMDS="serialcheck"
>
> https://github.com/linux-test-project/ltp/wiki/Test-Writing-Guidelines#232-library-environment-variables-for-shell
>
> BTW serialcheck source isn't probably packaged in distros
> https://git.breakpoint.cc/cgit/bigeasy/serialcheck.git/tree/serialcheck.c
> Maybe we could just adapt it to LTP and include it as well (if we consider
> whole
> testing useful).
>
> > +
> > +create_test_file()
> > +{
> > +    temp_test_file=`mktemp`
> I guess you need to use TST_NEEDS_TMPDIR=1
> and then just any file in it. Or use $$ (e.g. file.$$) if you want to have
> concurrency, but we usually don't care).
> > +    dd if=/dev/urandom of=$temp_test_file count=1 bs=$((UART_RATE / 2))
> You also need to add dd to TST_NEEDS_CMDS.
>
> > +}
> > +
> > +get_uart_ports()
> > +{
> > +for i in ${ARRAY[@]}; do
> > +    PORT=/dev/`echo $i | cut -d'/' -f 5`
> Well, you use cut yourself, so why that complicated code with arrays?
>
> > +    # Activate port in case it will be initialized only when startup
> > +    echo "DDT TESTING" > $PORT 2>/dev/null
> > +    if [ `cat $i` -ne 0 ]; then
> > +        UART_PORTS=("${UART_PORTS[@]}" "$PORT")
> > +    fi
> > +done
> > +}
> > +
> > +filter_out_used_ports()
> > +{
> > + which lsof
> > + ret=$?
> > + if [ $ret -eq 0 ];then
> > + tst_res TINFO "lsof command exist, filter out used ports";
> > + else
> > + tst_brk TCONF "test failed for lack of requirement,returned is $ret"
> > + fi
> Again whole block is useless, just add lsof into TST_NEEDS_CMDS.
>
> > +
> > +    for i in ${UART_PORTS[@]}; do
> > +        lsof | grep $i &> /dev/null ||
> > PORTS_TO_TEST=("${PORTS_TO_TEST[@]}" $i)
> > +    done
> > +}
> > +
> > +run_serial_test()
> > +{
> > + create_test_file
> create_test_file should be just 2 lines of code in setup function.
>
> > +    for i in ${PORTS_TO_TEST[@]}; do
> Array => bashism :(.
>
> > +        if [ $UART_HWFLOW -eq 0 ]; then
> > +            { sleep 1; serialcheck -b $UART_RATE -d $i -f
> $temp_test_file
> > -l $UART_LOOPS -m t -k; }&
> Again line wrapped :(
>
> > +            PID=$!
> > +            serialcheck -b $UART_RATE -d $i -f $temp_test_file -l
> Hm, why do you run 2 instances?
>
> > $UART_LOOPS -m r -k || { kill -- -$PID 2>/dev/null; tst_res TFAIL "TEST
> > FAILED"; }
> Using complicated code in { } isn't much readable. I'd put it into:
> if ! $UART_LOOPS -m r -k; then
>         kill ...
> fi
>
> > +        else
> > +            { sleep 1; serialcheck -b $UART_RATE -d $i -f
> $temp_test_file
> > -l $UART_LOOPS -m t -h; } &
> > +            PID=$!
> > +            serialcheck -b $UART_RATE -d $i -f $temp_test_file -l
> > $UART_LOOPS -m r -h || { kill -- -$PID 2>/dev/null; tst_res TFAIL "TEST
> > FAILED"; }
>
> This can be written better to not repeat much yourself.
> Whole if and else block is the same exept -h and -k parameter are
> different.
> Why not put this extra parameter into variable?
> Why sleep added into {} block?
>
> > +        fi
> > +    done
> > +    rm $temp_test_file
> > + tst_res TPASS "uart test passed"
> > +}
> > +
> > +
> > +TST_TESTFUNC=do_test
> > +. tst_test.sh
> We usually put this at the beginning of the test.
> Please see some tests as examples (testcases/commands/df/df01.sh,
> testcases/commands/mkfs/mkfs01.sh, testcases/commands/lsmod/lsmod01.sh,
> ...)
>
> > +
> > +do_test()
> > +{
> > + check_requirements
>
> > + get_uart_ports
> > + filter_out_used_ports
> > + run_serial_test
> > +}
> > +
> > +tst_run
>
> Kind regards,
> Petr
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20200319/d532914e/attachment.htm>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [LTP] [PATCH] uart: add uart testcase in kernel device-driver
  2020-03-19 15:07   ` Cixi Geng
@ 2020-03-19 18:43     ` Petr Vorel
  0 siblings, 0 replies; 5+ messages in thread
From: Petr Vorel @ 2020-03-19 18:43 UTC (permalink / raw)
  To: ltp

Hi Cixi,

> Thank you for taking the trouble to help me. I do appreciate it.
> Sorry about the so many mistakes,
> I will fix the code in next vesion.

> Thanks again for you help!

Not a big deal :). We appreciate contributors.

Kind regards,
Petr

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [LTP] [PATCH] uart: add uart testcase in kernel device-driver
@ 2020-03-18  5:25 Cixi Geng
  0 siblings, 0 replies; 5+ messages in thread
From: Cixi Geng @ 2020-03-18  5:25 UTC (permalink / raw)
  To: ltp

Porting UART test from ltp-ddt back to ltp. only test 115200 UART_RATE.

[TODO] support more rate, and add test HWFLOW function test.

Signed-off-by: Orson Zhai <orson@gmail.com>
Signed-off-by: Cixi Geng <gengcixi@gmail.com>
Signed-off-by: Carlos Hernandez <ceh@ti.com>
---
 runtest/kernel_ddt                            |   1 +
 testcases/kernel/device-drivers/Makefile      |   1 +
 testcases/kernel/device-drivers/uart/Makefile |  22 ++++
 .../kernel/device-drivers/uart/serialcheck.sh | 111 ++++++++++++++++++
 4 files changed, 135 insertions(+)
 create mode 100644 runtest/kernel_ddt
 create mode 100644 testcases/kernel/device-drivers/uart/Makefile
 create mode 100755 testcases/kernel/device-drivers/uart/serialcheck.sh

diff --git a/runtest/kernel_ddt b/runtest/kernel_ddt
new file mode 100644
index 000000000..30e9a0269
--- /dev/null
+++ b/runtest/kernel_ddt
@@ -0,0 +1 @@
+uart serialcheck.sh
diff --git a/testcases/kernel/device-drivers/Makefile
b/testcases/kernel/device-drivers/Makefile
index 55e0d25a0..a214f211b 100644
--- a/testcases/kernel/device-drivers/Makefile
+++ b/testcases/kernel/device-drivers/Makefile
@@ -27,6 +27,7 @@ SUBDIRS := acpi \
    rtc \
    tbio \
    uaccess \
+   uart \
    zram

 include $(top_srcdir)/include/mk/generic_trunk_target.mk
diff --git a/testcases/kernel/device-drivers/uart/Makefile
b/testcases/kernel/device-drivers/uart/Makefile
new file mode 100644
index 000000000..0d73f6635
--- /dev/null
+++ b/testcases/kernel/device-drivers/uart/Makefile
@@ -0,0 +1,22 @@
+# Copyright (c) 2015 Oracle and/or its affiliates. All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation; either version 2 of
+# the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+
+top_srcdir ?= ../../../..
+include $(top_srcdir)/include/mk/testcases.mk
+
+INSTALL_TARGETS := *.sh
+
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/device-drivers/uart/serialcheck.sh
b/testcases/kernel/device-drivers/uart/serialcheck.sh
new file mode 100755
index 000000000..f4cf13e02
--- /dev/null
+++ b/testcases/kernel/device-drivers/uart/serialcheck.sh
@@ -0,0 +1,111 @@
+#!/bin/sh
+###############################################################################
+#
+# Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com/
+# Copyright (C) 2019, Unisoc Communications Inc.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation version 2.
+#
+# This program is distributed "as is" WITHOUT ANY WARRANTY of any
+# kind, whether express or implied; without even the implied warranty
+# of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+###############################################################################
+
+# @desc Test UART ports using git://
git.breakpoint.cc/bigeasy/serialcheck.git
+
+#### Functions definitions ####
+usage()
+{
+    echo "usage: ./${0##*/} [-r UART_RATE] [-l LOOPS] [-x to enable HW
flow control]"
+    exit 1
+}
+
+# Default values
+: ${UART_RATE:=115200}
+: ${UART_LOOPS:=5}
+: ${UART_HWFLOW:=0}
+
+PORTS_TO_TEST=();
+UART_PORTS=();
+ARRAY=(`find /sys/class/tty/*/uartclk`);
+
+check_requirements()
+{
+ which serialcheck
+ ret=$?
+ if [ $ret -eq 0 ];then
+ tst_res TINFO "serialcheck command is in system,continue to test"
+ else
+ tst_brk TCONF "test failed for lack of requirement,returned is $ret"
+ fi
+
+}
+
+create_test_file()
+{
+    temp_test_file=`mktemp`
+    dd if=/dev/urandom of=$temp_test_file count=1 bs=$((UART_RATE / 2))
+}
+
+get_uart_ports()
+{
+for i in ${ARRAY[@]}; do
+    PORT=/dev/`echo $i | cut -d'/' -f 5`
+    # Activate port in case it will be initialized only when startup
+    echo "DDT TESTING" > $PORT 2>/dev/null
+    if [ `cat $i` -ne 0 ]; then
+        UART_PORTS=("${UART_PORTS[@]}" "$PORT")
+    fi
+done
+}
+
+filter_out_used_ports()
+{
+ which lsof
+ ret=$?
+ if [ $ret -eq 0 ];then
+ tst_res TINFO "lsof command exist, filter out used ports";
+ else
+ tst_brk TCONF "test failed for lack of requirement,returned is $ret"
+ fi
+
+    for i in ${UART_PORTS[@]}; do
+        lsof | grep $i &> /dev/null ||
PORTS_TO_TEST=("${PORTS_TO_TEST[@]}" $i)
+    done
+}
+
+run_serial_test()
+{
+ create_test_file
+    for i in ${PORTS_TO_TEST[@]}; do
+        if [ $UART_HWFLOW -eq 0 ]; then
+            { sleep 1; serialcheck -b $UART_RATE -d $i -f $temp_test_file
-l $UART_LOOPS -m t -k; }&
+            PID=$!
+            serialcheck -b $UART_RATE -d $i -f $temp_test_file -l
$UART_LOOPS -m r -k || { kill -- -$PID 2>/dev/null; tst_res TFAIL "TEST
FAILED"; }
+        else
+            { sleep 1; serialcheck -b $UART_RATE -d $i -f $temp_test_file
-l $UART_LOOPS -m t -h; } &
+            PID=$!
+            serialcheck -b $UART_RATE -d $i -f $temp_test_file -l
$UART_LOOPS -m r -h || { kill -- -$PID 2>/dev/null; tst_res TFAIL "TEST
FAILED"; }
+        fi
+    done
+    rm $temp_test_file
+ tst_res TPASS "uart test passed"
+}
+
+
+TST_TESTFUNC=do_test
+. tst_test.sh
+
+do_test()
+{
+ check_requirements
+ get_uart_ports
+ filter_out_used_ports
+ run_serial_test
+}
+
+tst_run
-- 
2.17.1
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20200318/eaf9b6c6/attachment.htm>

^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-03-19 18:43 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-18  5:31 [LTP] [PATCH] uart: add uart testcase in kernel device-driver Cixi Geng
2020-03-19 10:28 ` Petr Vorel
2020-03-19 15:07   ` Cixi Geng
2020-03-19 18:43     ` Petr Vorel
  -- strict thread matches above, loose matches on Subject: below --
2020-03-18  5:25 Cixi Geng

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.