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

[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 <hemantk@xxxxxxxxxxxxxx>
> > ---
> >   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