Conversation
- 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
xorptr
force-pushed
the
fix_spidev_timeouts
branch
from
370d644 to
17bf6c7
Compare
| // 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 | ||
| libhoth_status busy_wait_status = spi_nor_busy_wait( |
There was a problem hiding this comment.
If the previous htool invocation was interrupted during host command execution, the timeout here may be insufficient. Currently this covers only the case if the previous invocation was interrupted after a intermediate page program operation. Do you folks think that this should handle the first case as well?
| .default_value = "10", | ||
| .desc = "Maximum duration (in milliseconds) to wait after a page program " | ||
| "command when SPI device indicates that it is busy"}, | ||
| {HTOOL_FLAG_VALUE, .name = "spidev_device_busy_wait_check_interval", |
There was a problem hiding this comment.
We have one spidev parameter here in ms and one in us. Can we just use the same function that the usb_retry logic uses so that it's clear?
|
|
||
| if (time_elapsed_ms > (timeout_us / 1000)) { | ||
| return LIBHOTH_ERR_TIMEOUT; | ||
| if (timeout_ms != 0) { |
There was a problem hiding this comment.
Do we really not want to have a timeout at all for this? I think it might make sense to have a maximum of something very high (e.g. 60s?) so that it at least eventually fails and we return the proper timeout error code to the caller.
| // - For the first iteration, wait for any previous operations that might | ||
| // have set the busy bit to finish | ||
| // - 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 |
There was a problem hiding this comment.
Can you document what this is for?
I presume what you're going for is that we can send a host command and let it process while doing other things?
2 participants
spidev_device_busy_wait_timeouttospidev_page_program_busy_wait_timeoutto indicate that the timeout is explicitly for page program operations over spidevspidev_page_program_busy_wait_timeoutto take timeout in millisecondsspidev_page_program_busy_wait_timeoutaffects only the waits during intermediate page programs. After the final page program operation,timeout_msargument fromlibhoth_spi_receive_responseis used to wait for appropriate amount of time requested for host command execution