Skip to content

Commit

Permalink
[dv/dpi] Fix CI issues
Browse files Browse the repository at this point in the history
Format and trailing whitespace.
Fix offsets for Shared SRAM.
Optimize verify to compare 4k blocks.
Fix SPI device unittest

Signed-off-by: Daniel Beitel <[email protected]>
  • Loading branch information
dbeitel-opentitan committed Feb 2, 2024
1 parent d67865b commit 05a83eb
Show file tree
Hide file tree
Showing 17 changed files with 221 additions and 211 deletions.
2 changes: 1 addition & 1 deletion hw/dv/dpi/common/tcp_server/tcp_server.c
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ struct tcp_server_ctx *tcp_server_create(const char *display_name,

if (pthread_mutex_init(&ctx->sock_mutex, NULL) != 0 ||
pthread_create(&ctx->sock_thread, NULL, server_create, (void *)ctx) !=
0) {
0) {
fprintf(stderr, "%s: Unable to create TCP socket thread\n",
ctx->display_name);
ctx_free(ctx);
Expand Down
17 changes: 9 additions & 8 deletions hw/dv/dpi/gpiodpi/gpiodpi.c
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,8 @@ void *gpiodpi_create(const char *name, int listen_port, int n_bits) {
assert(cwd != NULL);

int path_len;
path_len = snprintf(ctx->dev_to_host_path, PATH_MAX, "%s/%s-read", cwd, name);
path_len =
snprintf(ctx->dev_to_host_path, PATH_MAX, "%s/%s-read", cwd, name);
assert(path_len > 0 && path_len <= PATH_MAX);
path_len =
snprintf(ctx->host_to_dev_path, PATH_MAX, "%s/%s-write", cwd, name);
Expand Down Expand Up @@ -256,7 +257,7 @@ uint32_t gpiodpi_host_to_device_tick(void *ctx_void, svBitVecVal *gpio_oe,
read_len++;
}
} while (read_len > 0 && read_len < 256 &&
(gpio_str[read_len - 1] != '\0' &&
(gpio_str[read_len - 1] != '\0' &&
gpio_str[read_len - 1] != '\r' &&
gpio_str[read_len - 1] != '\n'));
} else {
Expand Down Expand Up @@ -288,7 +289,7 @@ uint32_t gpiodpi_host_to_device_tick(void *ctx_void, svBitVecVal *gpio_oe,
}
CLR_BIT(ctx->driven_pin_values, idx);
set_bit_val(&ctx->weak_pins, idx, weak);
} else if (idx == 255){
} else if (idx == 255) {
ctx->rst_n = 0;
} else {
fprintf(stderr,
Expand All @@ -310,7 +311,7 @@ uint32_t gpiodpi_host_to_device_tick(void *ctx_void, svBitVecVal *gpio_oe,
}
SET_BIT(ctx->driven_pin_values, idx);
set_bit_val(&ctx->weak_pins, idx, weak);
} else if (idx == 255){
} else if (idx == 255) {
ctx->rst_n = 1;
} else {
fprintf(stderr,
Expand Down Expand Up @@ -355,12 +356,12 @@ void gpiodpi_close(void *ctx_void) {
tcp_server_close(ctx->sock);
} else {
if (close(ctx->dev_to_host_fifo) != 0) {
printf("GPIO: Failed to close FIFO file at %s: %s\n", ctx->dev_to_host_path,
strerror(errno));
printf("GPIO: Failed to close FIFO file at %s: %s\n",
ctx->dev_to_host_path, strerror(errno));
}
if (close(ctx->host_to_dev_fifo) != 0) {
printf("GPIO: Failed to close FIFO file at %s: %s\n", ctx->host_to_dev_path,
strerror(errno));
printf("GPIO: Failed to close FIFO file at %s: %s\n",
ctx->host_to_dev_path, strerror(errno));
}

if (unlink(ctx->dev_to_host_path) != 0) {
Expand Down
103 changes: 55 additions & 48 deletions hw/dv/dpi/spidpi/spidpi.c
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,8 @@
#include <sys/types.h>
#include <unistd.h>

#include "verilator_sim_ctrl.h"

#include "tcp_server.h"
#include "verilator_sim_ctrl.h"

// Enable this define to stop tracing at cycle 4
// and resume at the first SPI packet
Expand Down Expand Up @@ -69,30 +68,35 @@ struct spidpi_ctx {
static const char *json_find_next_char(char value, const char *ptr) {
const char *tmp_ptr = ptr;

while (tmp_ptr != NULL &&
*tmp_ptr!= value && *tmp_ptr != ']' && *tmp_ptr != '\0') tmp_ptr++;
while (tmp_ptr != NULL && *tmp_ptr != value && *tmp_ptr != ']' &&
*tmp_ptr != '\0')
tmp_ptr++;

// Exit conditions
if (tmp_ptr == NULL || *tmp_ptr == ']' || *tmp_ptr == '\0') tmp_ptr = NULL;
if (tmp_ptr == NULL || *tmp_ptr == ']' || *tmp_ptr == '\0')
tmp_ptr = NULL;

return tmp_ptr;
}

static const char *json_parse_next_tag(char *tag, int tag_size, const char *ptr) {
static const char *json_parse_next_tag(char *tag, int tag_size,
const char *ptr) {
const char *tmp_ptr = ptr;

tmp_ptr = json_find_next_char('"', tmp_ptr);
if (tmp_ptr != NULL) tmp_ptr++;
if (tmp_ptr != NULL)
tmp_ptr++;

int index = 0;
while (index < (tag_size - 1) && tmp_ptr != NULL &&
*tmp_ptr != '"' && *tmp_ptr != ']' && *tmp_ptr != '\0') {
while (index < (tag_size - 1) && tmp_ptr != NULL && *tmp_ptr != '"' &&
*tmp_ptr != ']' && *tmp_ptr != '\0') {
tag[index++] = *tmp_ptr++;
}
tag[index] = '\0';

// Exit conditions
if (tmp_ptr == NULL || *tmp_ptr == ']' || *tmp_ptr == '\0') tmp_ptr = NULL;
if (tmp_ptr == NULL || *tmp_ptr == ']' || *tmp_ptr == '\0')
tmp_ptr = NULL;

return tmp_ptr;
}
Expand All @@ -101,7 +105,8 @@ static const char *json_parse_next_integer(int *val, const char *ptr) {
const char *tmp_ptr = ptr;
char digits[32];

while (tmp_ptr != NULL && isspace(*tmp_ptr)) tmp_ptr++;
while (tmp_ptr != NULL && isspace(*tmp_ptr))
tmp_ptr++;

int index = 0;
while (index < (sizeof(digits) - 1) && tmp_ptr != NULL &&
Expand All @@ -114,7 +119,8 @@ static const char *json_parse_next_integer(int *val, const char *ptr) {
}

// Exit conditions
if (tmp_ptr == NULL || *tmp_ptr == ']' || *tmp_ptr == '\0') tmp_ptr = NULL;
if (tmp_ptr == NULL || *tmp_ptr == ']' || *tmp_ptr == '\0')
tmp_ptr = NULL;

return tmp_ptr;
}
Expand All @@ -131,10 +137,12 @@ static const char *json_parse_next_integer_array(char *vals, int *num_vals,
}

tmp_ptr = json_find_next_char('[', tmp_ptr);
if (tmp_ptr != NULL) tmp_ptr++;
if (tmp_ptr != NULL)
tmp_ptr++;

while (tmp_ptr != NULL && *tmp_ptr != ']') {
while (isspace(*tmp_ptr)) tmp_ptr++;
while (isspace(*tmp_ptr))
tmp_ptr++;

int index = 0;
while (index < (sizeof(digits) - 1) &&
Expand All @@ -146,13 +154,18 @@ static const char *json_parse_next_integer_array(char *vals, int *num_vals,
vals[(*num_vals)++] = atoi(digits);
}

while (isspace(*tmp_ptr)) tmp_ptr++;
if (*tmp_ptr == ',') tmp_ptr++;
if (*tmp_ptr == '\0') tmp_ptr = NULL;
while (isspace(*tmp_ptr))
tmp_ptr++;
if (*tmp_ptr == ',')
tmp_ptr++;
if (*tmp_ptr == '\0')
tmp_ptr = NULL;
}
// Exit conditions
if (tmp_ptr != NULL && *tmp_ptr == ']') tmp_ptr++;
if (tmp_ptr == NULL || *tmp_ptr == ']' || *tmp_ptr == '\0') tmp_ptr = NULL;
if (tmp_ptr != NULL && *tmp_ptr == ']')
tmp_ptr++;
if (tmp_ptr == NULL || *tmp_ptr == ']' || *tmp_ptr == '\0')
tmp_ptr = NULL;

return tmp_ptr;
}
Expand Down Expand Up @@ -186,15 +199,14 @@ static const char *xfer_parse_next_trans(void *ctx_void, const char *xfer_ptr) {
if (trans_ptr != NULL) {
int data_size = sizeof(ctx->buf);
trans_ptr++;
trans_ptr = json_parse_next_integer_array(ctx->buf, &data_size, trans_ptr);
trans_ptr =
json_parse_next_integer_array(ctx->buf, &data_size, trans_ptr);
ctx->xfer_write = true;
ctx->nmax = data_size;
}
}
trans_ptr = json_find_next_char('}', trans_ptr);
}
else
if (strstr(tag, "Read") != NULL) {
} else if (strstr(tag, "Read") != NULL) {
trans_ptr = json_find_next_char('{', trans_ptr);
trans_ptr = json_parse_next_tag(tag, sizeof(tag), trans_ptr);
if (strstr(tag, "len") != NULL) {
Expand All @@ -208,13 +220,12 @@ static const char *xfer_parse_next_trans(void *ctx_void, const char *xfer_ptr) {
}
}
trans_ptr = json_find_next_char('}', trans_ptr);
}
else
if (strstr(tag, "Both") != NULL) {
} else if (strstr(tag, "Both") != NULL) {
assert(false && "Both transaction not supported.");
}

if (trans_ptr != NULL) trans_ptr++;
if (trans_ptr != NULL)
trans_ptr++;
trans_ptr = json_find_next_char('}', trans_ptr);

return trans_ptr;
Expand All @@ -231,8 +242,7 @@ static bool xfer_parse(void *ctx_void) {
// Simple parser of SPI JSON transaction header
char *ptr = ctx->xfer_buf_in;
if ((ptr = strstr(ptr, "Req")) != NULL &&
(ptr = strstr(ptr, "Spi")) != NULL &&
(ptr = strstr(ptr, "id")) != NULL &&
(ptr = strstr(ptr, "Spi")) != NULL && (ptr = strstr(ptr, "id")) != NULL &&
(ptr = strstr(ptr, "command")) != NULL &&
(ptr = strstr(ptr, "RunTransaction")) != NULL &&
(ptr = strstr(ptr, "transaction")) != NULL &&
Expand All @@ -250,7 +260,8 @@ static void xfer_respond_next_trans(void *ctx_void) {

// assemble SPI JSON response
if (!ctx->xfer_start) {
sprintf(ctx->xfer_buf_out, "{\"Res\":{\"Ok\":{"
sprintf(ctx->xfer_buf_out,
"{\"Res\":{\"Ok\":{"
"\"Spi\":{\"RunTransaction\":{\"transaction\":[");
ctx->xfer_start = true;
} else {
Expand All @@ -259,11 +270,9 @@ static void xfer_respond_next_trans(void *ctx_void) {

if (ctx->xfer_write && ctx->xfer_read) {
assert(false && "Response to Both transaction not supported.");
} else
if (ctx->xfer_write) {
} else if (ctx->xfer_write) {
strcat(ctx->xfer_buf_out, "\"Write\"");
} else
if (ctx->xfer_read) {
} else if (ctx->xfer_read) {
char value[16];

strcat(ctx->xfer_buf_out, "{\"Read\":{\"data\":[");
Expand All @@ -283,7 +292,8 @@ static void xfer_respond(void *ctx_void, bool error) {
struct spidpi_ctx *ctx = (struct spidpi_ctx *)ctx_void;

if (error) {
sprintf(ctx->xfer_buf_out, "{\"Res\":{\"Err\":{"
sprintf(ctx->xfer_buf_out,
"{\"Res\":{\"Err\":{"
"\"description\":\"Error processing SPI device transaction\","
"\"backtrace\":\"%s:%d\""
"}}}\n",
Expand Down Expand Up @@ -377,7 +387,8 @@ void *spidpi_create(const char *name, int listen_port, int mode, int loglevel) {

printf(
"\n"
"SPI: Created %s for %s. Connect to it with any terminal program, e.g.\n"
"SPI: Created %s for %s. Connect to it with any terminal program, "
"e.g.\n"
"$ screen %s\n"
"NOTE: a SPI transaction is run for every 4 characters entered.\n",
ctx->ptyname, name, ctx->ptyname);
Expand Down Expand Up @@ -432,8 +443,7 @@ char spidpi_tick(void *ctx_void, const svLogicVecVal *d2p_data) {
int result;
if ((result = read(ctx->host, &buf, 1)) == 1) {
valid = true;
} else
if (result == -1) {
} else if (result == -1) {
if (errno != EAGAIN) {
xfer_reset(ctx);
fprintf(stderr, "Read on SPI FIFO gave %s\n", strerror(errno));
Expand All @@ -442,9 +452,8 @@ char spidpi_tick(void *ctx_void, const svLogicVecVal *d2p_data) {
}
if (valid) {
if (buf == '\n') {
// Eat newlines
} else
if (ctx->xfer_start) {
// Eat newlines
} else if (ctx->xfer_start) {
ctx->xfer_buf_in[ctx->xfer_count++] = buf;
switch (buf) {
case '{':
Expand All @@ -459,8 +468,7 @@ char spidpi_tick(void *ctx_void, const svLogicVecVal *d2p_data) {
if (ctx->xfer_indent == 0) {
ctx->xfer_finish = true;
}
} else
if (buf == '{') {
} else if (buf == '{') {
ctx->xfer_buf_in[ctx->xfer_count++] = buf;
ctx->xfer_indent = 1;
ctx->xfer_start = true;
Expand All @@ -474,8 +482,7 @@ char spidpi_tick(void *ctx_void, const svLogicVecVal *d2p_data) {
fprintf(stderr, "Transaction size bigger than buffer\n");
xfer_respond(ctx, true);
xfer_reset(ctx);
} else
if (ctx->xfer_finish) {
} else if (ctx->xfer_finish) {
if (xfer_parse(ctx)) {
ctx->nout = 0;
ctx->nin = 0;
Expand Down Expand Up @@ -557,8 +564,8 @@ char spidpi_tick(void *ctx_void, const svLogicVecVal *d2p_data) {
break;
case SP_CSRISE:
xfer_respond_next_trans(ctx);
if ((ctx->xfer_next =
xfer_parse_next_trans(ctx, ctx->xfer_next)) != NULL) {
if ((ctx->xfer_next = xfer_parse_next_trans(ctx, ctx->xfer_next)) !=
NULL) {
ctx->nout = 0;
ctx->nin = 0;
ctx->bout = ctx->msbfirst ? 0x80 : 0x01;
Expand All @@ -573,7 +580,7 @@ char spidpi_tick(void *ctx_void, const svLogicVecVal *d2p_data) {
ctx->driving = P2D_CSB;
ctx->state = SP_IDLE;
// TODO: Make this configurable.
ctx->delay = 100;
ctx->delay = 1000;
xfer_respond(ctx, false);
xfer_reset(ctx);
}
Expand Down
2 changes: 1 addition & 1 deletion hw/dv/dpi/spidpi/spidpi.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
extern "C" {

#define MAX_TRANSACTION (8 * 1024)
#define MAX_TRANSFER (4 * 1024)
#define MAX_TRANSFER (4 * 1024)
struct spidpi_ctx;

// SPI Host States
Expand Down
3 changes: 2 additions & 1 deletion hw/dv/dpi/uartdpi/uartdpi.c
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ void *uartdpi_create(const char *name, int listen_port,

printf(
"\n"
"UART: Created %s for %s. Connect to it with any terminal program, e.g.\n"
"UART: Created %s for %s. Connect to it with any terminal program, "
"e.g.\n"
"$ screen %s\n",
ctx->ptyname, name, ctx->ptyname);
}
Expand Down
3 changes: 1 addition & 2 deletions hw/dv/dpi/uartdpi/uartdpi.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ extern "C" {

struct uartdpi_ctx;

void *uartdpi_create(const char *name,
int listen_port,
void *uartdpi_create(const char *name, int listen_port,
const char *log_file_path);
void uartdpi_close(void *ctx_void);
int uartdpi_can_read(void *ctx_void);
Expand Down
6 changes: 1 addition & 5 deletions hw/top_darjeeling/dv/verilator/chip_sim_tb.sv
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ module chip_sim_tb (

// GPIO DPI
gpiodpi #(
.ListenPort(44855),
.N_GPIO(32)
) u_gpiodpi (
.clk_i (clk_i),
Expand All @@ -60,7 +59,6 @@ module chip_sim_tb (
// frequency must match the settings used in the on-chip software at
// `sw/top_darjeeling/sw/device/arch/device_sim_verilator.c`.
uartdpi #(
.ListenPort(44854),
.BAUD('d7_200),
.FREQ('d500_000)
) u_uart (
Expand Down Expand Up @@ -106,9 +104,7 @@ module chip_sim_tb (
`endif

// SPI DPI
spidpi #(
.ListenPort(44856)
) u_spi (
spidpi u_spi (
.clk_i (clk_i),
.rst_ni (cio_gpio_rst_n),
.spi_device_sck_o (cio_spi_device_sck_p2d),
Expand Down
Loading

0 comments on commit 05a83eb

Please sign in to comment.