Re: [PATCH v12 5/5] selftest: mhi: Add support to test MHI LOOPBACK channel

2020-11-20 Thread Hemant Kumar

Hi Shuah,

On 11/20/20 7:26 AM, Shuah Khan wrote:

On 11/16/20 3:46 PM, Hemant Kumar wrote:

Loopback test opens the MHI device file node and writes
a data buffer to it. MHI UCI kernel space driver copies
the data and sends it to MHI uplink (Tx) LOOPBACK channel.
MHI device loops back the same data to MHI downlink (Rx)
LOOPBACK channel. This data is read by test application
and compared against the data sent. Test passes if data
buffer matches between Tx and Rx. Test application performs
open(), poll(), write(), read() and close() file operations.

Signed-off-by: Hemant Kumar 
---
  Documentation/mhi/uci.rst  |  32 +
  tools/testing/selftests/Makefile   |   1 +
  tools/testing/selftests/drivers/.gitignore |   1 +
  tools/testing/selftests/drivers/mhi/Makefile   |   8 +
  tools/testing/selftests/drivers/mhi/config |   2 +
  .../testing/selftests/drivers/mhi/loopback_test.c  | 802 
+

  6 files changed, 846 insertions(+)
  create mode 100644 tools/testing/selftests/drivers/mhi/Makefile
  create mode 100644 tools/testing/selftests/drivers/mhi/config
  create mode 100644 tools/testing/selftests/drivers/mhi/loopback_test.c

diff --git a/Documentation/mhi/uci.rst b/Documentation/mhi/uci.rst
index ce8740e..0a04afe 100644
--- a/Documentation/mhi/uci.rst
+++ b/Documentation/mhi/uci.rst
@@ -79,6 +79,38 @@ MHI client driver performs read operation, same 
data gets looped back to MHI
  host using LOOPBACK channel 1. LOOPBACK channel is used to verify 
data path

  and data integrity between MHI Host and MHI device.


Nice.

[..]

+
+enum debug_level {
+    DBG_LVL_VERBOSE,
+    DBG_LVL_INFO,
+    DBG_LVL_ERROR,
+};
+
+enum test_status {
+    TEST_STATUS_SUCCESS,
+    TEST_STATUS_ERROR,
+    TEST_STATUS_NO_DEV,
+};
+


Since you are running this test as part of kselftest, please use
ksft errors nd returns.

Are you suggesting to use following macros instead of test_status enum ?
#define KSFT_PASS  0
#define KSFT_FAIL  1




+struct lb_test_ctx {
+    char dev_node[256];
+    unsigned char *tx_buff;
+    unsigned char *rx_buff;
+    unsigned int rx_pkt_count;
+    unsigned int tx_pkt_count;
+    int iterations;
+    bool iter_complete;
+    bool comp_complete;
+    bool test_complete;
+    bool all_complete;
+    unsigned long buff_size;
+    long byte_recvd;
+    long byte_sent;
+};
+
+bool force_exit;
+char write_data = 'a';
+int completed_iterations;
+
+struct lb_test_ctx test_ctxt;
+enum debug_level msg_lvl;
+struct pollfd read_fd;
+struct pollfd write_fd;
+enum test_status mhi_test_return_value;
+enum test_status tx_status;
+enum test_status rx_status;
+enum test_status cmp_rxtx_status;
+
+#define test_log(test_msg_lvl, format, ...) do { \
+    if (test_msg_lvl >= msg_lvl) \
+    fprintf(stderr, format, ##__VA_ARGS__); \
+} while (0)
+
+static void loopback_test_sleep_ms(int ms)
+{
+    usleep(1000 * ms);
+}
+


Have you run this as part of "make kselftest" run. How does this
sleep work in that env.?
Looks like kselftest runs this test application by directly executing 
the binary, but this test application requires a valid mhi device file 
node as a required parameter. So considering that requirement, is this 
test application qualifies to run using kselftest ? Without a valid 
device file node test would fail. Is there an option to run this test as 
standalone test which can only be run when a mhi device file node is 
present ? Having said that i tested this driver by

directly executing it using the test binary which is compiled using
make loopback_test under mhi dir.


Are there any cases where this test can't run and have to - those
cases need to be skips.
Yes, as this test application can not run by itself it needs a valid mhi 
device file node to write and test reads the same device node to get the 
data back.
So test can not be run without having a MHI device connected over a 
transport (in my testing MHI device is connected over PCIe). Could you 
please suggest an option to use this test application as a standalone 
test instead of being part of kselftest?


thanks,
-- Shuah


Thanks,
Hemant
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


Re: [PATCH v12 5/5] selftest: mhi: Add support to test MHI LOOPBACK channel

2020-11-20 Thread Hemant Kumar



Hi Mani,
On 11/19/20 10:10 PM, Manivannan Sadhasivam wrote:

On Mon, Nov 16, 2020 at 02:46:22PM -0800, Hemant Kumar wrote:

Loopback test opens the MHI device file node and writes
a data buffer to it. MHI UCI kernel space driver copies
the data and sends it to MHI uplink (Tx) LOOPBACK channel.
MHI device loops back the same data to MHI downlink (Rx)
LOOPBACK channel. This data is read by test application
and compared against the data sent. Test passes if data
buffer matches between Tx and Rx. Test application performs
open(), poll(), write(), read() and close() file operations.

Signed-off-by: Hemant Kumar 


One nitpick below, with that addressed:

Reviewed-by: Manivannan Sadhasivam 

[..]


Effectively this functions does parse and run, so this should be called
as, "loopback_test_parse_run" or pthread creation should be moved here.

Done.

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


Re: [PATCH v12 5/5] selftest: mhi: Add support to test MHI LOOPBACK channel

2020-11-20 Thread Shuah Khan

On 11/16/20 3:46 PM, Hemant Kumar wrote:

Loopback test opens the MHI device file node and writes
a data buffer to it. MHI UCI kernel space driver copies
the data and sends it to MHI uplink (Tx) LOOPBACK channel.
MHI device loops back the same data to MHI downlink (Rx)
LOOPBACK channel. This data is read by test application
and compared against the data sent. Test passes if data
buffer matches between Tx and Rx. Test application performs
open(), poll(), write(), read() and close() file operations.

Signed-off-by: Hemant Kumar 
---
  Documentation/mhi/uci.rst  |  32 +
  tools/testing/selftests/Makefile   |   1 +
  tools/testing/selftests/drivers/.gitignore |   1 +
  tools/testing/selftests/drivers/mhi/Makefile   |   8 +
  tools/testing/selftests/drivers/mhi/config |   2 +
  .../testing/selftests/drivers/mhi/loopback_test.c  | 802 +
  6 files changed, 846 insertions(+)
  create mode 100644 tools/testing/selftests/drivers/mhi/Makefile
  create mode 100644 tools/testing/selftests/drivers/mhi/config
  create mode 100644 tools/testing/selftests/drivers/mhi/loopback_test.c

diff --git a/Documentation/mhi/uci.rst b/Documentation/mhi/uci.rst
index ce8740e..0a04afe 100644
--- a/Documentation/mhi/uci.rst
+++ b/Documentation/mhi/uci.rst
@@ -79,6 +79,38 @@ MHI client driver performs read operation, same data gets 
looped back to MHI
  host using LOOPBACK channel 1. LOOPBACK channel is used to verify data path
  and data integrity between MHI Host and MHI device.
  


Nice.


+Loopback Test
+~
+
+Loopback test application is used to verify data integrity between MHI host and
+MHI device over LOOPBACK channel. This also confirms that basic MHI data path
+is working properly. Test performs write() to send tx buffer to MHI device file
+node for LOOPBACK uplink channel. MHI LOOPBACK downlink channel loops back
+transmit data to MHI Host. Test application receives data in receive buffer as
+part of read(). It verifies if tx buffer matches rx buffer. Test application
+performs poll() before making write() and read() system calls. Test passes if
+match is found.
+
+Test is present under tools/testing/selftests/drivers/mhi. It is compiled using
+following command in same dir:-
+
+make loopback_test
+
+Test is run using following command arguments:-
+
+loopback_test -c  -b  -l  -i
+
+
+Required argument:
+-c : loopback chardev node
+
+Optional argument:
+-b : transmit buffer size. If not present 1024 bytes size transmit buffer
+ is sent.
+-i : Number of iterations to perform, If not present only one transmit buffer
+ is sent.
+-l : Log level. If not present defaults to DBG_LVL_INFO.
+
  Other Use Cases
  ---
  
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile

index d9c2835..084bc1e 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -10,6 +10,7 @@ TARGETS += core
  TARGETS += cpufreq
  TARGETS += cpu-hotplug
  TARGETS += drivers/dma-buf
+TARGETS += drivers/mhi
  TARGETS += efivarfs
  TARGETS += exec
  TARGETS += filesystems
diff --git a/tools/testing/selftests/drivers/.gitignore 
b/tools/testing/selftests/drivers/.gitignore
index ca74f2e..e4806d5 100644
--- a/tools/testing/selftests/drivers/.gitignore
+++ b/tools/testing/selftests/drivers/.gitignore
@@ -1,2 +1,3 @@
  # SPDX-License-Identifier: GPL-2.0-only
  /dma-buf/udmabuf
+/mhi/loopback_test
diff --git a/tools/testing/selftests/drivers/mhi/Makefile 
b/tools/testing/selftests/drivers/mhi/Makefile
new file mode 100644
index 000..c06c925
--- /dev/null
+++ b/tools/testing/selftests/drivers/mhi/Makefile
@@ -0,0 +1,8 @@
+# SPDX-License-Identifier: GPL-2.0-only
+CFLAGS += -I../../../../../usr/include/ -g -Wall
+
+LDLIBS = -lpthread
+TEST_GEN_PROGS := loopback_test
+
+include ../../lib.mk
+
diff --git a/tools/testing/selftests/drivers/mhi/config 
b/tools/testing/selftests/drivers/mhi/config
new file mode 100644
index 000..471dc92
--- /dev/null
+++ b/tools/testing/selftests/drivers/mhi/config
@@ -0,0 +1,2 @@
+CONFIG_MHI_BUS=y
+CONFIG_MHi_UCI=y
diff --git a/tools/testing/selftests/drivers/mhi/loopback_test.c 
b/tools/testing/selftests/drivers/mhi/loopback_test.c
new file mode 100644
index 000..99b7712
--- /dev/null
+++ b/tools/testing/selftests/drivers/mhi/loopback_test.c
@@ -0,0 +1,802 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Loopback test application performs write() to send tx buffer to MHI device
+ * file node for LOOPBACK uplink channel. MHI LOOPBACK downlink channel loops
+ * back transmit data to MHI Host. Test application receives data in receive
+ * buffer as part of read(). It verifies if tx buffer matches rx buffer. Test
+ * application performs poll() before making write() and read() system
+ * calls. Test passes if match is found.
+ *
+ * Test is compiled using following command:-
+ *
+ * make loopback_test
+ *
+ * Test is run using following command arguments:-
+ *
+ * loopb

Re: [PATCH v12 5/5] selftest: mhi: Add support to test MHI LOOPBACK channel

2020-11-19 Thread Manivannan Sadhasivam
On Mon, Nov 16, 2020 at 02:46:22PM -0800, Hemant Kumar wrote:
> Loopback test opens the MHI device file node and writes
> a data buffer to it. MHI UCI kernel space driver copies
> the data and sends it to MHI uplink (Tx) LOOPBACK channel.
> MHI device loops back the same data to MHI downlink (Rx)
> LOOPBACK channel. This data is read by test application
> and compared against the data sent. Test passes if data
> buffer matches between Tx and Rx. Test application performs
> open(), poll(), write(), read() and close() file operations.
> 
> Signed-off-by: Hemant Kumar 

One nitpick below, with that addressed:

Reviewed-by: Manivannan Sadhasivam 

> ---
>  Documentation/mhi/uci.rst  |  32 +
>  tools/testing/selftests/Makefile   |   1 +
>  tools/testing/selftests/drivers/.gitignore |   1 +
>  tools/testing/selftests/drivers/mhi/Makefile   |   8 +
>  tools/testing/selftests/drivers/mhi/config |   2 +
>  .../testing/selftests/drivers/mhi/loopback_test.c  | 802 
> +
>  6 files changed, 846 insertions(+)
>  create mode 100644 tools/testing/selftests/drivers/mhi/Makefile
>  create mode 100644 tools/testing/selftests/drivers/mhi/config
>  create mode 100644 tools/testing/selftests/drivers/mhi/loopback_test.c
> 
> diff --git a/Documentation/mhi/uci.rst b/Documentation/mhi/uci.rst
> index ce8740e..0a04afe 100644
> --- a/Documentation/mhi/uci.rst
> +++ b/Documentation/mhi/uci.rst
> @@ -79,6 +79,38 @@ MHI client driver performs read operation, same data gets 
> looped back to MHI
>  host using LOOPBACK channel 1. LOOPBACK channel is used to verify data path
>  and data integrity between MHI Host and MHI device.
>  
> +Loopback Test
> +~
> +
> +Loopback test application is used to verify data integrity between MHI host 
> and
> +MHI device over LOOPBACK channel. This also confirms that basic MHI data path
> +is working properly. Test performs write() to send tx buffer to MHI device 
> file
> +node for LOOPBACK uplink channel. MHI LOOPBACK downlink channel loops back
> +transmit data to MHI Host. Test application receives data in receive buffer 
> as
> +part of read(). It verifies if tx buffer matches rx buffer. Test application
> +performs poll() before making write() and read() system calls. Test passes if
> +match is found.
> +
> +Test is present under tools/testing/selftests/drivers/mhi. It is compiled 
> using
> +following command in same dir:-
> +
> +make loopback_test
> +
> +Test is run using following command arguments:-
> +
> +loopback_test -c  -b  -l  -i
> +
> +
> +Required argument:
> +-c : loopback chardev node
> +
> +Optional argument:
> +-b : transmit buffer size. If not present 1024 bytes size transmit buffer
> + is sent.
> +-i : Number of iterations to perform, If not present only one transmit buffer
> + is sent.
> +-l : Log level. If not present defaults to DBG_LVL_INFO.
> +
>  Other Use Cases
>  ---
>  
> diff --git a/tools/testing/selftests/Makefile 
> b/tools/testing/selftests/Makefile
> index d9c2835..084bc1e 100644
> --- a/tools/testing/selftests/Makefile
> +++ b/tools/testing/selftests/Makefile
> @@ -10,6 +10,7 @@ TARGETS += core
>  TARGETS += cpufreq
>  TARGETS += cpu-hotplug
>  TARGETS += drivers/dma-buf
> +TARGETS += drivers/mhi
>  TARGETS += efivarfs
>  TARGETS += exec
>  TARGETS += filesystems
> diff --git a/tools/testing/selftests/drivers/.gitignore 
> b/tools/testing/selftests/drivers/.gitignore
> index ca74f2e..e4806d5 100644
> --- a/tools/testing/selftests/drivers/.gitignore
> +++ b/tools/testing/selftests/drivers/.gitignore
> @@ -1,2 +1,3 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  /dma-buf/udmabuf
> +/mhi/loopback_test
> diff --git a/tools/testing/selftests/drivers/mhi/Makefile 
> b/tools/testing/selftests/drivers/mhi/Makefile
> new file mode 100644
> index 000..c06c925
> --- /dev/null
> +++ b/tools/testing/selftests/drivers/mhi/Makefile
> @@ -0,0 +1,8 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +CFLAGS += -I../../../../../usr/include/ -g -Wall
> +
> +LDLIBS = -lpthread
> +TEST_GEN_PROGS := loopback_test
> +
> +include ../../lib.mk
> +
> diff --git a/tools/testing/selftests/drivers/mhi/config 
> b/tools/testing/selftests/drivers/mhi/config
> new file mode 100644
> index 000..471dc92
> --- /dev/null
> +++ b/tools/testing/selftests/drivers/mhi/config
> @@ -0,0 +1,2 @@
> +CONFIG_MHI_BUS=y
> +CONFIG_MHi_UCI=y
> diff --git a/tools/testing/selftests/drivers/mhi/loopback_test.c 
> b/tools/testing/selftests/drivers/mhi/loopback_test.c
> new file mode 100644
> index 000..99b7712
> --- /dev/null
> +++ b/tools/testing/selftests/drivers/mhi/loopback_test.c
> @@ -0,0 +1,802 @@
> +

[...]

> +int main(int argc, char *argv[])
> +{
> + int ret = 0;
> +
> + loopback_test_set_defaults();
> + test_log(DBG_LVL_INFO, "MHI Loopback test App\n");
> +
> + if (argc > 1)
> + ret = loopback_test_parse(argc, argv);

Effectively this functions does parse and r

Re: [PATCH v12 5/5] selftest: mhi: Add support to test MHI LOOPBACK channel

2020-11-17 Thread Loic Poulain
On Mon, 16 Nov 2020 at 23:46, Hemant Kumar  wrote:
>
> Loopback test opens the MHI device file node and writes
> a data buffer to it. MHI UCI kernel space driver copies
> the data and sends it to MHI uplink (Tx) LOOPBACK channel.
> MHI device loops back the same data to MHI downlink (Rx)
> LOOPBACK channel. This data is read by test application
> and compared against the data sent. Test passes if data
> buffer matches between Tx and Rx. Test application performs
> open(), poll(), write(), read() and close() file operations.
>
> Signed-off-by: Hemant Kumar 
> ---
>  Documentation/mhi/uci.rst  |  32 +
>  tools/testing/selftests/Makefile   |   1 +
>  tools/testing/selftests/drivers/.gitignore |   1 +
>  tools/testing/selftests/drivers/mhi/Makefile   |   8 +
>  tools/testing/selftests/drivers/mhi/config |   2 +
>  .../testing/selftests/drivers/mhi/loopback_test.c  | 802 
> +
>  6 files changed, 846 insertions(+)
>  create mode 100644 tools/testing/selftests/drivers/mhi/Makefile
>  create mode 100644 tools/testing/selftests/drivers/mhi/config
>  create mode 100644 tools/testing/selftests/drivers/mhi/loopback_test.c
>
> diff --git a/Documentation/mhi/uci.rst b/Documentation/mhi/uci.rst
> index ce8740e..0a04afe 100644
> --- a/Documentation/mhi/uci.rst
> +++ b/Documentation/mhi/uci.rst
> @@ -79,6 +79,38 @@ MHI client driver performs read operation, same data gets 
> looped back to MHI
>  host using LOOPBACK channel 1. LOOPBACK channel is used to verify data path
>  and data integrity between MHI Host and MHI device.
>
> +Loopback Test
> +~
> +
> +Loopback test application is used to verify data integrity between MHI host 
> and
> +MHI device over LOOPBACK channel. This also confirms that basic MHI data path
> +is working properly. Test performs write() to send tx buffer to MHI device 
> file
> +node for LOOPBACK uplink channel. MHI LOOPBACK downlink channel loops back
> +transmit data to MHI Host. Test application receives data in receive buffer 
> as
> +part of read(). It verifies if tx buffer matches rx buffer. Test application
> +performs poll() before making write() and read() system calls. Test passes if
> +match is found.
> +
> +Test is present under tools/testing/selftests/drivers/mhi. It is compiled 
> using
> +following command in same dir:-
> +
> +make loopback_test
> +
> +Test is run using following command arguments:-
> +
> +loopback_test -c  -b  -l  -i
> +
> +
> +Required argument:
> +-c : loopback chardev node
> +
> +Optional argument:
> +-b : transmit buffer size. If not present 1024 bytes size transmit buffer
> + is sent.
> +-i : Number of iterations to perform, If not present only one transmit buffer
> + is sent.
> +-l : Log level. If not present defaults to DBG_LVL_INFO.
> +
>  Other Use Cases
>  ---
>
> diff --git a/tools/testing/selftests/Makefile 
> b/tools/testing/selftests/Makefile
> index d9c2835..084bc1e 100644
> --- a/tools/testing/selftests/Makefile
> +++ b/tools/testing/selftests/Makefile
> @@ -10,6 +10,7 @@ TARGETS += core
>  TARGETS += cpufreq
>  TARGETS += cpu-hotplug
>  TARGETS += drivers/dma-buf
> +TARGETS += drivers/mhi
>  TARGETS += efivarfs
>  TARGETS += exec
>  TARGETS += filesystems
> diff --git a/tools/testing/selftests/drivers/.gitignore 
> b/tools/testing/selftests/drivers/.gitignore
> index ca74f2e..e4806d5 100644
> --- a/tools/testing/selftests/drivers/.gitignore
> +++ b/tools/testing/selftests/drivers/.gitignore
> @@ -1,2 +1,3 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  /dma-buf/udmabuf
> +/mhi/loopback_test
> diff --git a/tools/testing/selftests/drivers/mhi/Makefile 
> b/tools/testing/selftests/drivers/mhi/Makefile
> new file mode 100644
> index 000..c06c925
> --- /dev/null
> +++ b/tools/testing/selftests/drivers/mhi/Makefile
> @@ -0,0 +1,8 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +CFLAGS += -I../../../../../usr/include/ -g -Wall
> +
> +LDLIBS = -lpthread
> +TEST_GEN_PROGS := loopback_test
> +
> +include ../../lib.mk
> +
> diff --git a/tools/testing/selftests/drivers/mhi/config 
> b/tools/testing/selftests/drivers/mhi/config
> new file mode 100644
> index 000..471dc92
> --- /dev/null
> +++ b/tools/testing/selftests/drivers/mhi/config
> @@ -0,0 +1,2 @@
> +CONFIG_MHI_BUS=y
> +CONFIG_MHi_UCI=y

CONFIG_MHI_UCI

> diff --git a/tools/testing/selftests/drivers/mhi/loopback_test.c 
> b/tools/testing/selftests/drivers/mhi/loopback_test.c
> new file mode 100644
> index 000..99b7712
> --- /dev/null
> +++ b/tools/testing/selftests/drivers/mhi/loopback_test.c
> @@ -0,0 +1,802 @@
> +// SPDX-License-Identifier: GPL-2.0

[...]