[LTP] [PATCH v2] uart: add uart testcase in kernel device-drivers

[LTP] [PATCH v2] uart: add uart testcase in kernel device-drivers

[gengcixi]

From: Cixi Geng <gengcixi@gmail.com>

Porting UART test from ltp-ddt back to ltp.
rewrite case use ltp library functions instead of ltp-ddt's

Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
Signed-off-by: Orson Zhai <orsonzhai@gmail.com>
Signed-off-by: Cixi Geng <gengcixi@gmail.com>
---
changes v2:
    1. support more bandrate test 
    2. move test tags in kernel_misc file 
    3. use TST_NEEDS_CMDS to check lsof,dd and serialcheck command 
    4. remove arrays in bash,rewrite test flow control to adapt dash 
    5. others like License and code-style issues
explatin for v1:
    1. create_test_file should be just 2 lines of code in setup function.
    >>>the test files need have random data,So use create_test_file
    function
    2. why run 2 instances of serialcheck
    >>>run 2 instances is one test send and one test recv, sleep in
    send  for make sure recv instances can get the send data
---
 runtest/kernel_misc                           |  6 ++
 testcases/kernel/device-drivers/Makefile      |  1 +
 testcases/kernel/device-drivers/uart/Makefile | 10 +++
 .../kernel/device-drivers/uart/serialcheck.sh | 68 +++++++++++++++++++
 4 files changed, 85 insertions(+)
 create mode 100644 testcases/kernel/device-drivers/uart/Makefile
 create mode 100755 testcases/kernel/device-drivers/uart/serialcheck.sh

diff --git a/runtest/kernel_misc b/runtest/kernel_misc
index 7937c7bbf..7a077b23b 100644
--- a/runtest/kernel_misc
+++ b/runtest/kernel_misc
@@ -13,3 +13,9 @@ zram01 zram01.sh
 zram02 zram02.sh
 zram03 zram03
 umip_basic_test umip_basic_test
+# uart test in loopback mode
+uart_9600_k serialcheck.sh 9600 5 k
+uart_19200_k serialcheck.sh 19200 5 k
+uart_38400_k serialcheck.sh 38400 5 k
+uart_57600_k  serialcheck.sh 57600 5 k
+uart_115200_k serialcheck.sh 115200 5 k
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..2e4781a6b
--- /dev/null
+++ b/testcases/kernel/device-drivers/uart/Makefile
@@ -0,0 +1,10 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) 2014-2015 Oracle and/or its affiliates. All Rights Reserved.
+
+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..afdc9212f
--- /dev/null
+++ b/testcases/kernel/device-drivers/uart/serialcheck.sh
@@ -0,0 +1,68 @@
+#!/bin/sh
+###############################################################################
+#
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com/
+# Copyright (C) 2019, Unisoc Communications Inc.
+#
+# Test UART ports using git://git.breakpoint.cc/bigeasy/serialcheck.git
+#
+###############################################################################
+
+TST_TESTFUNC=do_test
+TST_POS_ARGS=3
+TST_USAGE=usage
+TST_NEEDS_ROOT=1
+TST_NEEDS_CMDS="serialcheck"
+TST_NEEDS_CMDS="lsof"
+TST_NEEDS_CMDS="dd"
+TST_NEEDS_TMPDIR=1
+
+. tst_test.sh
+
+usage()
+{
+    echo "usage: ./${0} [-r UART_RATE] [-l LOOPS] [-h|k to enable HW flow control or loopback]"
+    exit 1
+}
+
+UART_RATE=$1
+UART_LOOPS=$2
+UART_HWFLOW=$3
+
+create_test_file()
+{
+    temp_test_file=`mktemp`
+    dd if=/dev/urandom of=$temp_test_file count=1 bs=$((UART_RATE / 2))
+}
+
+test_serial()
+{
+    create_test_file
+    { sleep 1; serialcheck -b $UART_RATE -d $1 -f $temp_test_file -l $UART_LOOPS -m t -${UART_HWFLOW}; }&
+    PID=$!
+    serialcheck -b $UART_RATE -d $1 -f $temp_test_file -l $UART_LOOPS -m r -${UART_HWFLOW}
+    if [ $? ];  then
+        kill -- -$PID 2>/dev/null
+        tst_res TFAIL "uart test failed"
+    else
+        tst_res TPASS "uart test passed"
+    fi
+    rm $temp_test_file
+}
+
+do_test()
+{
+    for i in /sys/class/tty/*/uartclk ;do
+        PORT=`echo $i |cut -d '/' -f 5`
+        # Activate port in case it will be initialized only when startup
+        echo "UART TESTING">${PORT} 2>/dev/null
+        if [ `cat /sys/class/tty/${PORT}/uartclk` -ne 0 ]; then
+            lsof | grep "/dev/${PORT}" &> /dev/null || PORT_TO_TEST="/dev/${PORT}"
+            tst_res TINFO "start test on ${PORT_TO_TEST} ${UART_RATE}"
+            test_serial ${PORT_TO_TEST}
+        fi
+    done
+}
+
+tst_run
-- 
2.17.1

[LTP] [PATCH v2] uart: add uart testcase in kernel device-drivers

[Cyril Hrubis]

Hi!
>  runtest/kernel_misc                           |  6 ++
>  testcases/kernel/device-drivers/Makefile      |  1 +
>  testcases/kernel/device-drivers/uart/Makefile | 10 +++
>  .../kernel/device-drivers/uart/serialcheck.sh | 68 +++++++++++++++++++
>  4 files changed, 85 insertions(+)
>  create mode 100644 testcases/kernel/device-drivers/uart/Makefile
>  create mode 100755 testcases/kernel/device-drivers/uart/serialcheck.sh
> 
> diff --git a/runtest/kernel_misc b/runtest/kernel_misc
> index 7937c7bbf..7a077b23b 100644
> --- a/runtest/kernel_misc
> +++ b/runtest/kernel_misc
> @@ -13,3 +13,9 @@ zram01 zram01.sh
>  zram02 zram02.sh
>  zram03 zram03
>  umip_basic_test umip_basic_test
> +# uart test in loopback mode
> +uart_9600_k serialcheck.sh 9600 5 k
> +uart_19200_k serialcheck.sh 19200 5 k
> +uart_38400_k serialcheck.sh 38400 5 k
> +uart_57600_k  serialcheck.sh 57600 5 k
> +uart_115200_k serialcheck.sh 115200 5 k
> 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..2e4781a6b
> --- /dev/null
> +++ b/testcases/kernel/device-drivers/uart/Makefile
> @@ -0,0 +1,10 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# Copyright (c) 2014-2015 Oracle and/or its affiliates. All Rights Reserved.
> +
> +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..afdc9212f
> --- /dev/null
> +++ b/testcases/kernel/device-drivers/uart/serialcheck.sh
> @@ -0,0 +1,68 @@
> +#!/bin/sh
> +###############################################################################
> +#
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com/
> +# Copyright (C) 2019, Unisoc Communications Inc.
> +#
> +# Test UART ports using git://git.breakpoint.cc/bigeasy/serialcheck.git
> +#
> +###############################################################################
> +
> +TST_TESTFUNC=do_test
> +TST_POS_ARGS=3
> +TST_USAGE=usage
> +TST_NEEDS_ROOT=1
> +TST_NEEDS_CMDS="serialcheck"
> +TST_NEEDS_CMDS="lsof"
> +TST_NEEDS_CMDS="dd"
> +TST_NEEDS_TMPDIR=1
> +
> +. tst_test.sh
> +
> +usage()
> +{
> +    echo "usage: ./${0} [-r UART_RATE] [-l LOOPS] [-h|k to enable HW flow control or loopback]"
> +    exit 1
> +}
> +
> +UART_RATE=$1
> +UART_LOOPS=$2
> +UART_HWFLOW=$3
> +
> +create_test_file()
> +{
> +    temp_test_file=`mktemp`
> +    dd if=/dev/urandom of=$temp_test_file count=1 bs=$((UART_RATE / 2))

Once the test creates temporary directory by TST_NEEDS_TMPDIR there is
no need for a mktemp anymore. The test runs with PWD pointing to a
unique temporary directory at that point.

> +}
> +
> +test_serial()
> +{
> +    create_test_file
> +    { sleep 1; serialcheck -b $UART_RATE -d $1 -f $temp_test_file -l $UART_LOOPS -m t -${UART_HWFLOW}; }&
> +    PID=$!
> +    serialcheck -b $UART_RATE -d $1 -f $temp_test_file -l $UART_LOOPS -m r -${UART_HWFLOW}

What do you need the sleep 1 for?

Tests should synchronize processes properly with locks instead of doing
random sleep and hoping for the best.

> +    if [ $? ];  then
> +        kill -- -$PID 2>/dev/null
> +        tst_res TFAIL "uart test failed"
> +    else
> +        tst_res TPASS "uart test passed"
> +    fi
> +    rm $temp_test_file
> +}
> +
> +do_test()
> +{
> +    for i in /sys/class/tty/*/uartclk ;do
> +        PORT=`echo $i |cut -d '/' -f 5`
> +        # Activate port in case it will be initialized only when startup
> +        echo "UART TESTING">${PORT} 2>/dev/null
> +        if [ `cat /sys/class/tty/${PORT}/uartclk` -ne 0 ]; then
> +            lsof | grep "/dev/${PORT}" &> /dev/null || PORT_TO_TEST="/dev/${PORT}"
> +            tst_res TINFO "start test on ${PORT_TO_TEST} ${UART_RATE}"
> +            test_serial ${PORT_TO_TEST}
> +        fi
> +    done

This is problematic as well, it expects that all ports that are not
in-use are loopback connected. This is not true in general case, which
means that we cannot add the test to the kernel_misc runtest file as it
is because it will fail on most of the systems out there.

We will have to figure out how to pass which ports are interconnected to
the test somehow, because that is something that only the user who set
up the machine knows.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2] uart: add uart testcase in kernel device-drivers

[Cyril Hrubis]

Hi!
Also looking at the serialtest.c source code, it's just a few lines that
haven't been changed since 2014, so it may be just easier to write a C
testcase based on that code and would avoid dependency on third party
git repository that way as well.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2] uart: add uart testcase in kernel device-drivers

[Petr Vorel]

Hi,

> This is problematic as well, it expects that all ports that are not
> in-use are loopback connected. This is not true in general case, which
> means that we cannot add the test to the kernel_misc runtest file as it
> is because it will fail on most of the systems out there.
Oh, I didn't realize this obvious thing.

> We will have to figure out how to pass which ports are interconnected to
> the test somehow, because that is something that only the user who set
> up the machine knows.
Maybe expect user sets it in some variable before running the test? e.g.:

if [ -n "$UART_INTERCONNECTED_PORTS" ]; then
    tst_brk TCONF "set space separated interconnected ports in \$UART_INTERCONNECTED_PORTS"
fi

Kind regards,
Petr

[LTP] [PATCH v2] uart: add uart testcase in kernel device-drivers

[Cixi Geng]

Hi ALL:
Thank you very much for your guidance,
I will follow your some advises, and try to add serialcheck in next version
but I also have some questions

>We both Cyril and me mentioned that you don't need to use mktemp (+ it'd be
>unnecessary dependency).
Now I knew  TST_NEEDS_TMPDIR  is make a temp direct and cd in it ,
but I need a file contains random data, this file is to used to test the
PORT,
Do you meaning I should creat a normal file use dd ,and named by myself?

>> This is problematic as well, it expects that all ports that are not
>> in-use are loopback connected. This is not true in general case, which
>> means that we cannot add the test to the kernel_misc runtest file as it
>> is because it will fail on most of the systems out there.
>Oh, I didn't realize this obvious thing.
>> We will have to figure out how to pass which ports are interconnected to
>> the test somehow, because that is something that only the user who set
>> up the machine knows.
>Maybe expect user sets it in some variable before running the test? e.g.:
we can decide  which PORT can be test,  But just like Cyril said, we don't
know the machines is run int loopback mode or  HW flow control  mode,
So can I put the testcase tags into two file in the runtest, one is used to
test
loopback mode, and the other is test for   HW flow control  ?
runtest/uart-loopback
runtest/uart-HWflow

Petr Vorel <pvorel@suse.cz> ?2020?3?25??? ??11:28???

> Hi,
>
> > This is problematic as well, it expects that all ports that are not
> > in-use are loopback connected. This is not true in general case, which
> > means that we cannot add the test to the kernel_misc runtest file as it
> > is because it will fail on most of the systems out there.
> Oh, I didn't realize this obvious thing.
>
> > We will have to figure out how to pass which ports are interconnected to
> > the test somehow, because that is something that only the user who set
> > up the machine knows.
> Maybe expect user sets it in some variable before running the test? e.g.:
>
> if [ -n "$UART_INTERCONNECTED_PORTS" ]; then
>     tst_brk TCONF "set space separated interconnected ports in
> \$UART_INTERCONNECTED_PORTS"
> fi
>
> Kind regards,
> Petr
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20200326/c89669dc/attachment.htm>

[LTP] [PATCH v2] uart: add uart testcase in kernel device-drivers

[Petr Vorel]

Hi Cixi,

> >We both Cyril and me mentioned that you don't need to use mktemp (+ it'd be
> >unnecessary dependency).
> Now I knew  TST_NEEDS_TMPDIR  is make a temp direct and cd in it ,
> but I need a file contains random data, this file is to used to test the
> PORT,
> Do you meaning I should creat a normal file use dd ,and named by myself?
Yes. We do not care about test concurrency of the same test (i.e. the same test
run more times simultaneously). And even if we care it could be solved by adding $$ -
PID, i.e.: foo.$$ (but you don't need to).
BTW I wrote it in https://lists.linux.it/pipermail/ltp/2020-March/016107.html

> >> This is problematic as well, it expects that all ports that are not
> >> in-use are loopback connected. This is not true in general case, which
> >> means that we cannot add the test to the kernel_misc runtest file as it
> >> is because it will fail on most of the systems out there.
> >Oh, I didn't realize this obvious thing.
> >> We will have to figure out how to pass which ports are interconnected to
> >> the test somehow, because that is something that only the user who set
> >> up the machine knows.
> >Maybe expect user sets it in some variable before running the test? e.g.:
> we can decide  which PORT can be test,  But just like Cyril said, we don't
> know the machines is run int loopback mode or  HW flow control  mode,
> So can I put the testcase tags into two file in the runtest, one is used to
> test
> loopback mode, and the other is test for   HW flow control  ?
> runtest/uart-loopback
> runtest/uart-HWflow

Wouldn't it help this below?
> > Maybe expect user sets it in some variable before running the test? e.g.:

> > if [ -n "$UART_INTERCONNECTED_PORTS" ]; then
> >     tst_brk TCONF "set space separated interconnected ports in
> > \$UART_INTERCONNECTED_PORTS"
> > fi

Kind regards,
Petr

[LTP] [PATCH v2] uart: add uart testcase in kernel device-drivers

[Cixi Geng]

Hi Petr:
>> +    { sleep 1; serialcheck -b $UART_RATE -d $1 -f $temp_test_file -l
$UART_LOOPS -m t -${UART_HWFLOW}; }&
>Cyril already mentioned the need to use proper locks instead of sleep
I have realized the sleep is not a good way , but I don't have any idea
about the "proper locks"
Can you give me some help?

Petr Vorel <pvorel@suse.cz> ?2020?3?26??? ??3:05???

> Hi Cixi,
>
> > >We both Cyril and me mentioned that you don't need to use mktemp (+
> it'd be
> > >unnecessary dependency).
> > Now I knew  TST_NEEDS_TMPDIR  is make a temp direct and cd in it ,
> > but I need a file contains random data, this file is to used to test the
> > PORT,
> > Do you meaning I should creat a normal file use dd ,and named by myself?
> Yes. We do not care about test concurrency of the same test (i.e. the same
> test
> run more times simultaneously). And even if we care it could be solved by
> adding $$ -
> PID, i.e.: foo.$$ (but you don't need to).
> BTW I wrote it in
> https://lists.linux.it/pipermail/ltp/2020-March/016107.html
>
> > >> This is problematic as well, it expects that all ports that are not
> > >> in-use are loopback connected. This is not true in general case, which
> > >> means that we cannot add the test to the kernel_misc runtest file as
> it
> > >> is because it will fail on most of the systems out there.
> > >Oh, I didn't realize this obvious thing.
> > >> We will have to figure out how to pass which ports are interconnected
> to
> > >> the test somehow, because that is something that only the user who set
> > >> up the machine knows.
> > >Maybe expect user sets it in some variable before running the test?
> e.g.:
> > we can decide  which PORT can be test,  But just like Cyril said, we
> don't
> > know the machines is run int loopback mode or  HW flow control  mode,
> > So can I put the testcase tags into two file in the runtest, one is used
> to
> > test
> > loopback mode, and the other is test for   HW flow control  ?
> > runtest/uart-loopback
> > runtest/uart-HWflow
>
> Wouldn't it help this below?
> > > Maybe expect user sets it in some variable before running the test?
> e.g.:
>
> > > if [ -n "$UART_INTERCONNECTED_PORTS" ]; then
> > >     tst_brk TCONF "set space separated interconnected ports in
> > > \$UART_INTERCONNECTED_PORTS"
> > > fi
>
> Kind regards,
> Petr
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20200327/b9353681/attachment.htm>

[LTP] [PATCH v2] uart: add uart testcase in kernel device-drivers

[Cyril Hrubis]

Hi!
> >Cyril already mentioned the need to use proper locks instead of sleep
> I have realized the sleep is not a good way , but I don't have any idea
> about the "proper locks"
> Can you give me some help?

I've just send a patchset that implements the serial test based on the
serialcheck C source code, please have a look at that patchset.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2] uart: add uart testcase in kernel device-drivers

[Petr Vorel]

Hi Cixi,

...
> +++ b/testcases/kernel/device-drivers/uart/serialcheck.sh
> @@ -0,0 +1,68 @@
> +#!/bin/sh
> +###############################################################################
> +#
Please avoid these '####...'

> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com/
> +# Copyright (C) 2019, Unisoc Communications Inc.
> +#
> +# Test UART ports using git://git.breakpoint.cc/bigeasy/serialcheck.git
> +#
> +###############################################################################
> +
> +TST_TESTFUNC=do_test
> +TST_POS_ARGS=3
> +TST_USAGE=usage
> +TST_NEEDS_ROOT=1
> +TST_NEEDS_CMDS="serialcheck"
> +TST_NEEDS_CMDS="lsof"
> +TST_NEEDS_CMDS="dd"
Note, this is a normal shell variable, so you'd request only dd. You need to
use:
TST_NEEDS_CMDS="serialcheck lsof dd"

> +TST_NEEDS_TMPDIR=1
> +
> +. tst_test.sh
> +
> +usage()
> +{
> +    echo "usage: ./${0} [-r UART_RATE] [-l LOOPS] [-h|k to enable HW flow control or loopback]"
> +    exit 1
> +}
BTW, you use positional arguments, so this help is wrong.
IMHO it's better to use getopts parameters (positional parameters are rarely
used + use "k" as a parameter looks strange), but of course it's not a
mistake. If you want to keep them, it should be:

usage()
{
    echo "usage: $0 {UART_RATE} {LOOPS} {h|k to enable HW flow control or loopback}"
}

The convention in unix/linux program is:
[ foo ] => foo is optional argument
{ bar } => bar is mandatory argument

> +
> +UART_RATE=$1
> +UART_LOOPS=$2
> +UART_HWFLOW=$3
> +
> +create_test_file()
> +{
> +    temp_test_file=`mktemp`
We both Cyril and me mentioned that you don't need to use mktemp (+ it'd be
unnecessary dependency).

> +    dd if=/dev/urandom of=$temp_test_file count=1 bs=$((UART_RATE / 2))
> +}
> +
> +test_serial()
> +{
> +    create_test_file
> +    { sleep 1; serialcheck -b $UART_RATE -d $1 -f $temp_test_file -l $UART_LOOPS -m t -${UART_HWFLOW}; }&
Cyril already mentioned the need to use proper locks instead of sleep.
> +    PID=$!
> +    serialcheck -b $UART_RATE -d $1 -f $temp_test_file -l $UART_LOOPS -m r -${UART_HWFLOW}
> +    if [ $? ];  then
This is always true. Use either:
serialcheck -b $UART_RATE -d $1 -f $temp_test_file -l $UART_LOOPS -m r -${UART_HWFLOW}
if [ $? -eq 0 ];  then
...

or put the command itself into if:

if serialcheck -b $UART_RATE -d $1 -f $temp_test_file -l $UART_LOOPS -m r -${UART_HWFLOW}; then
...

> +        kill -- -$PID 2>/dev/null
> +        tst_res TFAIL "uart test failed"
> +    else
> +        tst_res TPASS "uart test passed"
> +    fi
> +    rm $temp_test_file
> +}
> +
> +do_test()
> +{
You re supposed to declare non-global variables:
local i

> +    for i in /sys/class/tty/*/uartclk ;do
> +        PORT=`echo $i |cut -d '/' -f 5`
> +        # Activate port in case it will be initialized only when startup
> +        echo "UART TESTING">${PORT} 2>/dev/null
> +        if [ `cat /sys/class/tty/${PORT}/uartclk` -ne 0 ]; then
> +            lsof | grep "/dev/${PORT}" &> /dev/null || PORT_TO_TEST="/dev/${PORT}"
> +            tst_res TINFO "start test on ${PORT_TO_TEST} ${UART_RATE}"
> +            test_serial ${PORT_TO_TEST}
> +        fi
> +    done
> +}
> +
> +tst_run

Kind regards,
Petr