[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;
