Skip to content

Commit 370d644

Browse files
committed
Fix timeouts for operations over spidev
- Rename `spidev_device_busy_wait_timeout` to `spidev_page_program_busy_wait_timeout` to indicate that the timeout is explicitly for page program operations over spidev - Change `spidev_page_program_busy_wait_timeout` to take timeout in milliseconds - Change timeout behavior for spidev so that timeout value of 0 indicates indefinite wait - Change timeout behavior for spidev so that `spidev_page_program_busy_wait_timeout` affects only the waits during intermediate page programs. After the final page program operation, `timeout_ms` argument from `libhoth_spi_receive_response` is used to wait for appropriate amount of time requested for host command execution
1 parent 2d6aac5 commit 370d644

5 files changed

Lines changed: 45 additions & 36 deletions

File tree

examples/htool.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1777,10 +1777,10 @@ static const struct htool_param GLOBAL_FLAGS[] = {
17771777
{HTOOL_FLAG_VALUE, .name = "spidev_speed_hz", .default_value = "0",
17781778
.desc = "Clock speed (in Hz) to use when using spidev transport. Default "
17791779
"behavior (with input 0) is to not change the clock speed"},
1780-
{HTOOL_FLAG_VALUE, .name = "spidev_device_busy_wait_timeout",
1781-
.default_value = "180000000",
1782-
.desc = "Maximum duration (in microseconds) to wait when SPI device "
1783-
"indicates that it is busy"},
1780+
{HTOOL_FLAG_VALUE, .name = "spidev_page_program_busy_wait_timeout",
1781+
.default_value = "10",
1782+
.desc = "Maximum duration (in milliseconds) to wait after a page program "
1783+
"command when SPI device indicates that it is busy"},
17841784
{HTOOL_FLAG_VALUE, .name = "spidev_device_busy_wait_check_interval",
17851785
.default_value = "100",
17861786
.desc = "Interval duration (in microseconds) to wait before checking SPI "

examples/htool_spi.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ struct libhoth_device* htool_libhoth_spi_device(void) {
3535
uint32_t mailbox_location;
3636
bool atomic;
3737
uint32_t spidev_speed_hz;
38-
uint32_t spidev_device_busy_wait_timeout;
38+
uint32_t spidev_page_program_busy_wait_timeout;
3939
uint32_t spidev_device_busy_wait_check_interval;
4040
rv = htool_get_param_string(htool_global_flags(), "spidev_path",
4141
&spidev_path_str) ||
@@ -45,8 +45,8 @@ struct libhoth_device* htool_libhoth_spi_device(void) {
4545
htool_get_param_u32(htool_global_flags(), "spidev_speed_hz",
4646
&spidev_speed_hz) ||
4747
htool_get_param_u32(htool_global_flags(),
48-
"spidev_device_busy_wait_timeout",
49-
&spidev_device_busy_wait_timeout) ||
48+
"spidev_page_program_busy_wait_timeout",
49+
&spidev_page_program_busy_wait_timeout) ||
5050
htool_get_param_u32(htool_global_flags(),
5151
"spidev_device_busy_wait_check_interval",
5252
&spidev_device_busy_wait_check_interval);
@@ -64,7 +64,7 @@ struct libhoth_device* htool_libhoth_spi_device(void) {
6464
.mailbox = mailbox_location,
6565
.atomic = atomic,
6666
.speed = spidev_speed_hz,
67-
.device_busy_wait_timeout = spidev_device_busy_wait_timeout,
67+
.page_program_busy_wait_timeout = spidev_page_program_busy_wait_timeout,
6868
.device_busy_wait_check_interval = spidev_device_busy_wait_check_interval,
6969
};
7070
rv = libhoth_spi_open(&opts, &result);

transports/libhoth_device.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ int libhoth_send_request(struct libhoth_device* dev, const void* request,
6464
// be written. Errors if libhoth_send_request() wasn't called previously.
6565
// Returns LIBHOTH_ERR_TIMEOUT if the response is not ready by the
6666
// specified timeout, and the user can call again later. If timeout_ms is zero,
67-
// returns immediately.
67+
// the waiting behavior is implementation defined
6868
// This function is not thread-safe. In multi-threaded contexts, callers must
6969
// ensure libhoth_send_request() and libhoth_receive_response() occur
7070
// atomically (with respect to other calls to those functions).

transports/libhoth_spi.c

Lines changed: 35 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ struct libhoth_spi_device {
4242

4343
void* buffered_request;
4444
size_t buffered_request_size;
45-
uint32_t device_busy_wait_timeout;
45+
uint32_t page_program_busy_wait_timeout;
4646
uint32_t device_busy_wait_check_interval;
4747
};
4848

@@ -99,7 +99,7 @@ static int get_monotonic_ms(uint64_t* time_ms) {
9999
return 0;
100100
}
101101

102-
static libhoth_status spi_nor_busy_wait(const int fd, uint32_t timeout_us,
102+
static libhoth_status spi_nor_busy_wait(const int fd, uint32_t timeout_ms,
103103
uint32_t check_interval_us) {
104104
uint8_t tx_buf[2];
105105
uint8_t rx_buf[2];
@@ -128,20 +128,22 @@ static libhoth_status spi_nor_busy_wait(const int fd, uint32_t timeout_us,
128128
return LIBHOTH_OK;
129129
}
130130

131-
uint64_t current_time_ms;
132-
if (get_monotonic_ms(&current_time_ms) != 0) {
133-
return LIBHOTH_ERR_FAIL;
134-
}
135-
uint64_t time_elapsed_ms = 0;
136-
if (current_time_ms < start_time_ms) {
137-
// Wrap around
138-
time_elapsed_ms = (UINT64_MAX - start_time_ms) + current_time_ms;
139-
} else {
140-
time_elapsed_ms = current_time_ms - start_time_ms;
141-
}
142-
143-
if (time_elapsed_ms > (timeout_us / 1000)) {
144-
return LIBHOTH_ERR_TIMEOUT;
131+
if (timeout_ms != 0) {
132+
uint64_t current_time_ms;
133+
if (get_monotonic_ms(&current_time_ms) != 0) {
134+
return LIBHOTH_ERR_FAIL;
135+
}
136+
uint64_t time_elapsed_ms = 0;
137+
if (current_time_ms < start_time_ms) {
138+
// Wrap around
139+
time_elapsed_ms = (UINT64_MAX - start_time_ms) + current_time_ms;
140+
} else {
141+
time_elapsed_ms = current_time_ms - start_time_ms;
142+
}
143+
144+
if (time_elapsed_ms > timeout_ms) {
145+
return LIBHOTH_ERR_TIMEOUT;
146+
}
145147
}
146148
usleep(check_interval_us);
147149
}
@@ -167,13 +169,20 @@ static int spi_nor_write_enable(const int fd) {
167169

168170
static int spi_nor_write(int fd, bool address_mode_4b, unsigned int address,
169171
const void* data, size_t data_len,
170-
uint32_t device_busy_wait_timeout,
172+
uint32_t page_program_busy_wait_timeout,
171173
uint32_t device_busy_wait_check_interval) {
172174
if (fd < 0 || !data || !data_len) return LIBHOTH_ERR_INVALID_PARAMETER;
173175

174176
// Page program operations
175177
size_t bytes_sent = 0;
176178
while (bytes_sent < data_len) {
179+
// - For the first iteration, wait for any previous operations that might have set the busy bit to finish
180+
// - For rest of the iterations, wait for each page program operation to be handled except the last one. Waiting for the last page program will be done in `libhoth_spi_receive_response` since RoT will take some time to execute the host command
181+
libhoth_status busy_wait_status = spi_nor_busy_wait(
182+
fd, page_program_busy_wait_timeout, device_busy_wait_check_interval);
183+
if (busy_wait_status != LIBHOTH_OK) {
184+
return busy_wait_status;
185+
}
177186
// Send write enable before each Page program operation
178187
int status = spi_nor_write_enable(fd);
179188
if (status != LIBHOTH_OK) {
@@ -206,13 +215,6 @@ static int spi_nor_write(int fd, bool address_mode_4b, unsigned int address,
206215
}
207216
bytes_sent += chunk_send_size;
208217
address += chunk_send_size;
209-
210-
// Wait for each page program operation to be handled
211-
libhoth_status busy_wait_status = spi_nor_busy_wait(
212-
fd, device_busy_wait_timeout, device_busy_wait_check_interval);
213-
if (busy_wait_status != LIBHOTH_OK) {
214-
return busy_wait_status;
215-
}
216218
}
217219
return LIBHOTH_OK;
218220
}
@@ -363,7 +365,8 @@ int libhoth_spi_open(const struct libhoth_spi_device_init_options* options,
363365
spi_dev->fd = fd;
364366
spi_dev->mailbox_address = options->mailbox;
365367
spi_dev->address_mode_4b = true;
366-
spi_dev->device_busy_wait_timeout = options->device_busy_wait_timeout;
368+
spi_dev->page_program_busy_wait_timeout =
369+
options->page_program_busy_wait_timeout;
367370
spi_dev->device_busy_wait_check_interval =
368371
options->device_busy_wait_check_interval;
369372

@@ -408,7 +411,7 @@ int libhoth_spi_send_request(struct libhoth_device* dev, const void* request,
408411

409412
return spi_nor_write(spi_dev->fd, spi_dev->address_mode_4b,
410413
spi_dev->mailbox_address, request, request_size,
411-
spi_dev->device_busy_wait_timeout,
414+
spi_dev->page_program_busy_wait_timeout,
412415
spi_dev->device_busy_wait_check_interval);
413416
}
414417

@@ -429,6 +432,12 @@ int libhoth_spi_receive_response(struct libhoth_device* dev, void* response,
429432
struct libhoth_spi_device* spi_dev =
430433
(struct libhoth_spi_device*)dev->user_ctx;
431434

435+
libhoth_status busy_wait_status = spi_nor_busy_wait(
436+
spi_dev->fd, timeout_ms, spi_dev->device_busy_wait_check_interval);
437+
if (busy_wait_status != LIBHOTH_OK) {
438+
return busy_wait_status;
439+
}
440+
432441
// Read Header From Mailbox
433442
status = spi_nor_read(spi_dev->fd, spi_dev->address_mode_4b,
434443
spi_dev->mailbox_address, response,

transports/libhoth_spi.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ struct libhoth_spi_device_init_options {
3333
int mode;
3434
int speed;
3535
int atomic;
36-
uint32_t device_busy_wait_timeout;
36+
uint32_t page_program_busy_wait_timeout;
3737
uint32_t device_busy_wait_check_interval;
3838
};
3939

0 commit comments

Comments
 (0)