On Tue, 30 May 2023 at 22:23, Vikram Garhwal <vikram.garhwal@amd.com> wrote:
The QTests perform three tests on the Xilinx VERSAL CANFD controller:
Tests the CANFD controllers in loopback.
Tests the CANFD controllers in normal mode with CAN frame.
Tests the CANFD controllers in normal mode with CANFD frame.
Signed-off-by: Vikram Garhwal <vikram.garhwal@amd.com>
Acked-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Francisco Iglesias <francisco.iglesias@amd.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Hi; Coverity has spotted some issues with this test code; could
you investigate and send followup patches, please ?
+static void match_rx_tx_data(const uint32_t *buf_tx, const uint32_t *buf_rx,
+ bool is_canfd_frame)
+{
+ uint16_t size = 0;
+ uint8_t len = CAN_FRAME_SIZE;
+
+ if (is_canfd_frame) {
+ len = CANFD_FRAME_SIZE;
+ }
Here len is either 4 (if !is_canfd_frame) or 18 (if is_canfd_frame)...
+
+ while (size < len) {
...and we loop with size always less than len...
+ if (R_RX0_ID_OFFSET + 4 * size == R_RX0_DLC_OFFSET) {
+ g_assert_cmpint((buf_rx[size] & DLC_FD_BIT_MASK), ==,
+ (buf_tx[size] & DLC_FD_BIT_MASK));
+ } else {
+ if (!is_canfd_frame && size == 4) {
...so here this condition can never be true: if !is_canfd_frame
then we know size is less than 4.
What was the intention here ?
(CID 1512900)
+ break;
+ }
+
+ g_assert_cmpint(buf_rx[size], ==, buf_tx[size]);
+ }
+
+ size++;
+ }
+}
+/*
+ * Xilinx CANFD supports both CAN and CANFD frames. This test will be
+ * transferring CAN frame i.e. 8 bytes of data from CANFD0 and CANFD1 through
+ * canbus. CANFD0 initiate the data transfer to can-bus, CANFD1 receives the
+ * data. Test compares the can frame data sent from CANFD0 and received on
+ * CANFD1.
+ */
+static void test_can_data_transfer(void)
+{
+ uint32_t buf_tx[CAN_FRAME_SIZE] = { 0x5a5bb9a4, 0x80000000,
+ 0x12345678, 0x87654321 };
+ uint32_t buf_rx[CAN_FRAME_SIZE] = { 0x00, 0x00, 0x00, 0x00 };
The buf_rx[] array here is only 4 bytes long...
+ uint32_t status = 0;
+
+ generate_random_data(buf_tx, false);
+
+ QTestState *qts = qtest_init("-machine xlnx-versal-virt"
+ " -object can-bus,id=canbus"
+ " -machine canbus0=canbus"
+ " -machine canbus1=canbus"
+ );
+
+ configure_canfd(qts, MSR_NORMAL_MODE);
+
+ /* Check if CANFD0 and CANFD1 are in Normal mode. */
+ status = qtest_readl(qts, CANFD0_BASE_ADDR + R_SR_OFFSET);
+ status = status & STATUS_REG_MASK;
+ g_assert_cmpint(status, ==, STATUS_NORMAL_MODE);
+
+ status = qtest_readl(qts, CANFD1_BASE_ADDR + R_SR_OFFSET);
+ status = status & STATUS_REG_MASK;
+ g_assert_cmpint(status, ==, STATUS_NORMAL_MODE);
+
+ write_data(qts, CANFD0_BASE_ADDR, buf_tx, false);
+
+ send_data(qts, CANFD0_BASE_ADDR);
+ read_data(qts, CANFD1_BASE_ADDR, buf_rx);
...but read_data() will write up to 17 bytes of data to the buffer,
if the incoming data from the device claims it to be a canfd frame.
The device shouldn't really do that, but the point of a test is
that the device might not be functioning correctly, so we should
size buf_rx[] large enough to fit whatever read_data() writes to it.
(CID 1512899)
+ match_rx_tx_data(buf_tx, buf_rx, false);
+
+ qtest_quit(qts);
+}
thanks
-- PMM