[usbdpi] Data type corrections and comments Drop duplicated hexdump functionality Style and linting alterations Signed-off-by: Adrian Lees <a.lees@lowrisc.org>
diff --git a/hw/dv/dpi/usbdpi/usb_monitor.c b/hw/dv/dpi/usbdpi/usb_monitor.c index f368afb..aea1c77 100644 --- a/hw/dv/dpi/usbdpi/usb_monitor.c +++ b/hw/dv/dpi/usbdpi/usb_monitor.c
@@ -42,7 +42,7 @@ * Current bus driver */ usbmon_driver_t driver; - int pu; + uint32_t pu; int line; int rawbits; int bits;
diff --git a/hw/dv/dpi/usbdpi/usb_utils.c b/hw/dv/dpi/usbdpi/usb_utils.c index 792104f..0b69de1 100644 --- a/hw/dv/dpi/usbdpi/usb_utils.c +++ b/hw/dv/dpi/usbdpi/usb_utils.c
@@ -64,7 +64,7 @@ // Printable ASCII characters for (unsigned idx = 0u; idx < chunk; idx++) { char ch = row[idx]; - *dp++ = (ch < ' ' || ch >= 0x80u) ? '.' : ch; + *dp++ = (ch < ' ' || ch >= 0x7f) ? '.' : ch; } *dp = '\0'; fprintf(out, "%s%s\n", prefix, buf);
diff --git a/hw/dv/dpi/usbdpi/usbdpi.c b/hw/dv/dpi/usbdpi/usbdpi.c index d9ce188..8c3c9c5 100644 --- a/hw/dv/dpi/usbdpi/usbdpi.c +++ b/hw/dv/dpi/usbdpi/usbdpi.c
@@ -66,7 +66,7 @@ // Try to send OUT transfer. Optionally expect Status packet (eg. ACK|NAK) in // response -static void tryTX(usbdpi_ctx_t *ctx, uint8_t endpoint, bool bExpectStatus); +static void tryTX(usbdpi_ctx_t *ctx, uint8_t endpoint, bool expect_status); // Test the ischronous transfers (without ACKs) // static void testIso(usbdpi_ctx_t *ctx); @@ -879,7 +879,7 @@ // Try an OUT transfer to the device, optionally expecting a Status // packet (eg. ACK|NAK) in response; this is not expected for // Isochronous transfers -void tryTX(usbdpi_ctx_t *ctx, uint8_t endpoint, bool bExpectStatus) { +void tryTX(usbdpi_ctx_t *ctx, uint8_t endpoint, bool expect_status) { const uint8_t pattern[] = { "AbCdEfGhIjKlMnOpQrStUvWxYz+0123456789-aBcDeFgHiJkLmNoPqRsTuVwXyZ"}; usbdpi_transfer_t *tr = ctx->sending; @@ -904,7 +904,7 @@ if (ctx->tick_bits >= ctx->wait) { // Note: Isochronous transfers are not acknowledged and do not employ // Data Toggle Synchronization - if (bExpectStatus && ctx->lastrxpid == USB_PID_ACK) { + if (expect_status && ctx->lastrxpid == USB_PID_ACK) { ctx->ep_out[endpoint].next_data = DATA_TOGGLE_ADVANCE(ctx->ep_out[endpoint].next_data); } @@ -1107,6 +1107,7 @@ return ctx->driving; } + // Are allowed to start transmitting yet; device recovery time elapsed? if (ctx->tick < ctx->recovery_time) { ctx->frame_start = ctx->tick_bits; return ctx->driving;
diff --git a/hw/dv/dpi/usbdpi/usbdpi.h b/hw/dv/dpi/usbdpi/usbdpi.h index cad73ba..eb2be17 100644 --- a/hw/dv/dpi/usbdpi/usbdpi.h +++ b/hw/dv/dpi/usbdpi/usbdpi.h
@@ -348,7 +348,7 @@ */ usbdpi_transfer_t *sending; - int last_pu; + uint32_t last_pu; uint8_t lastrxpid; /** @@ -359,8 +359,10 @@ * Current time in USB bit intervals */ uint32_t tick_bits; - - int recovery_time; + /** + * End time of recovery interval (following device attachment) + */ + uint32_t recovery_time; /** * Test step number
diff --git a/hw/dv/dpi/usbdpi/usbdpi_stream.c b/hw/dv/dpi/usbdpi/usbdpi_stream.c index a6d4139..b4ac7d5 100644 --- a/hw/dv/dpi/usbdpi/usbdpi_stream.c +++ b/hw/dv/dpi/usbdpi/usbdpi_stream.c
@@ -79,6 +79,8 @@ // Initialize streaming state for the given number of streams bool streams_init(usbdpi_ctx_t *ctx, unsigned nstreams, bool retrieve, bool checking, bool retrying, bool send) { + assert(ctx); + // Can we support the requested number of streams? if (!nstreams || nstreams > USBDPI_MAX_STREAMS) { return false; @@ -301,7 +303,7 @@ if (get_le32(&sig[0]) == STREAM_SIGNATURE_HEAD && get_le32(&sig[12]) == STREAM_SIGNATURE_TAIL && // sanity check on transfer length, though we rely upon the CPU - // oftware to send, receive and count the number of bytes + // software to send, receive and count the number of bytes num_bytes > 0U && num_bytes < 0x10000000U && !sig[6] && !sig[7]) { // Signature includes the initial value of the device-side LFSR s->tst_lfsr = sig[4];
diff --git a/hw/dv/dpi/usbdpi/usbdpi_test.c b/hw/dv/dpi/usbdpi/usbdpi_test.c index 8c8236e..845a463 100644 --- a/hw/dv/dpi/usbdpi/usbdpi_test.c +++ b/hw/dv/dpi/usbdpi/usbdpi_test.c
@@ -11,12 +11,12 @@ // Test-specific initialization void usbdpi_test_init(usbdpi_ctx_t *ctx) { // Test-specific initialization code - bool bOK = false; + bool ok = false; switch (ctx->test_number) { case 0: // The simple smoke test (usbdev_test) does not require any parameters // or special initialization at present - bOK = true; + ok = true; break; // Initialize streaming test @@ -33,7 +33,7 @@ bool send = (ctx->test_arg[0] & 0x80U) != 0U; if (nstreams <= USBDPI_MAX_STREAMS) { - bOK = streams_init(ctx, nstreams, retrieve, checking, retrying, send); + ok = streams_init(ctx, nstreams, retrieve, checking, retrying, send); } } break; @@ -43,7 +43,7 @@ } // TODO - commute this to a proper test failure at run time - assert(bOK); + assert(ok); } // Return the next step in the test sequence
diff --git a/sw/device/tests/usbdev_stream_test.c b/sw/device/tests/usbdev_stream_test.c index 427582a..a119324 100644 --- a/sw/device/tests/usbdev_stream_test.c +++ b/sw/device/tests/usbdev_stream_test.c
@@ -8,14 +8,14 @@ // on the USB host. The test initializes the USB device and configures a set of // endpoints for data streaming using bulk transfers. // -// The DPI model mimicks an USB host. After device initialization, it detects +// The DPI model mimicks a USB host. After device initialization, it detects // the assertion of the pullup and first assigns an address to the device. // For this test it will then repeatedly fetch data via IN requests to // each stream and propagate that data to the corresponding OUT endpoints. // // The data itself is pseudo-randomly generated by the sender and, // independently, by the receiving code to check that the data has been -// propapgated unmodified and without data loss, corruption, replication etc. +// propagated unmodified and without data loss, corruption, replication etc. #include "sw/device/lib/dif/dif_pinmux.h" #include "sw/device/lib/runtime/log.h" @@ -54,7 +54,7 @@ #define MAX_TX_BUFFERS 24U #endif -// This takes about 256s presently with 10MHz CPU in CW-310 FPGA and physical +// This takes about 256s presently with 10MHz CPU in CW310 FPGA and physical // USB with randomized packet sizes and the default memcpy implementation; // The _MEM_FASTER switch drops the run time to 187s #define TRANSFER_BYTES_FPGA (0x10U << 20) @@ -118,6 +118,10 @@ uint32_t tail_sig; } usbdev_stream_sig_t; +// Sanity check because the host-side code relies upon the same structure +static_assert(sizeof(usbdev_stream_sig_t) == 0x10U, + "Host-side code relies upon signature structure"); + /** * Context state for a single stream */ @@ -212,7 +216,7 @@ /** * Configuration values for USB. - * TODO - dymamically construct a config descriptor appropriate to the test; + * TODO - dynamically construct a config descriptor appropriate to the test; * this would avoid creating unusable ports on the host and also provide * a little more testing */ @@ -278,39 +282,13 @@ // Dump a sequence of bytes as hexadecimal and ASCII for diagnostic purposes static void buffer_dump(const uint8_t *data, size_t n) { - static const char hex_digits[] = "0123456789abcdef"; - const unsigned ncols = 0x20u; - char buf[ncols * 4u + 2u]; + base_hexdump_fmt_t fmt = { + .bytes_per_word = 1, + .words_per_line = 0x20u, + .alphabet = &kBaseHexdumpDefaultFmtAlphabet, + }; - // Note: we have no generic (s(n))printf functionality and must use LOG_INFO() - while (n > 0u) { - const unsigned chunk = (n > ncols) ? ncols : (unsigned)n; - const uint8_t *row = data; - unsigned idx = 0u; - char *dp = buf; - - // Columns of hexadecimal bytes - while (idx < chunk) { - dp[0] = hex_digits[row[idx] >> 4]; - dp[1] = hex_digits[row[idx++] & 0xfu]; - dp[2] = ' '; - dp += 3; - } - while (idx++ < ncols) { - dp[2] = dp[1] = dp[0] = ' '; - dp += 3; - } - - // Printable ASCII characters - for (unsigned idx = 0u; idx < chunk; idx++) { - char ch = row[idx]; - *dp++ = (ch < ' ' || ch >= 0x80u) ? '.' : ch; - } - *dp = '\0'; - LOG_INFO("%s", buf); - data += chunk; - n -= chunk; - } + base_hexdump_with(fmt, (char *)data, n); } // Create a stream signature buffer @@ -326,9 +304,6 @@ sig.num_bytes = s->transfer_bytes; sig.tail_sig = STREAM_SIGNATURE_TAIL; - // Sanity check because the host-side code relies upon the same structure - CHECK(sizeof(sig) == 0x10U); - size_t bytes_written; CHECK_DIF_OK(dif_usbdev_buffer_write(usbdev.dev, buf, (uint8_t *)&sig, sizeof(sig), &bytes_written)); @@ -363,7 +338,8 @@ // Update the LFSR for the next packet s->tx_lfsr = lfsr; } else { - // Undefined buffer contents + // Undefined buffer contents; useful for profiling IN throughput on + // CW310, because the CPU load at 10MHz can be an appreciable slowdown } if (verbose && log_traffic) { @@ -717,7 +693,7 @@ LOG_INFO("Commencing data transfer..."); } - bool bDone = false; + bool done = false; do { for (unsigned s = 0U; s < nstreams; s++) { stream_service(ctx, &ctx->streams[s]); @@ -732,8 +708,8 @@ while (s < nstreams && stream_completed(&ctx->streams[s])) { s++; } - bDone = (s >= nstreams); - } while (!bDone); + done = (s >= nstreams); + } while (!done); // Determine the total counts of bytes sent and received uint32_t tx_bytes = 0U;