[dv,ottf_spi_console] OTTF spi console component dv improvements #28493
Conversation
…ange This accomodates bootstrapping a multi-slot binary, which may be much larger than a single-slot image. Signed-off-by: Harry Callahan <[email protected]>
These logs wrapped a task which contained it's own internal logging, so was unnecessary. Signed-off-by: Harry Callahan <[email protected]>
…ing loop Remove the task host_spi_console_read_wait_for() in favour of a new two-pronged approach to capturing reads. 1) host_spi_console_poll_reads() is forked into a background process, which automatically reads the spi_console when the DEVICE indicates new data is available. 2) Sequences can call host_spi_console_read_wait_for_polled_str() to block awaiting a certain string to be polled by the background task, at which point the read buffer is flushed. This allows for a much more tolerant testbench which will capture and ignore messages from the DEVICE it is not expecting, while still allowing for a stimulus vseq to block awaiting a particular message for synchronization purposes. Note. A control bit 'enable_read_polling' is added to allow this background polling to be paused temporarily while the existing task host_spi_console_read_payload() takes over the task of issuing reads. This allows fixed payloads to be captured using the synchronization messages and known test structrue as delimiters, instead of needing to implement a message parser. See the code comment above this bit for more details. Signed-off-by: Harry Callahan <[email protected]>
hcallahan-lowrisc
requested review from
antmarzam,
phani-lowrisc and
rswarbrick
| uint wait_on_busy_timeout_ns = 200_000_000; // 200ms | ||
| uint write_completion_timeout_ns = 200_000_000; // 200ms | ||
| uint min_interval_ns = 3_000; // 3us | ||
| uint await_flow_ctrl_timeout_ns = 1_000_000_000; // 1s |
There was a problem hiding this comment.
We just chatted about this in person (and you taught me something about how this works!)
I wonder whether a better approach (or follow-up) would be to move these out of the package and into the classes in question. For example, I think that await_flow_ctrl_timeout_ns is only used in ottf_spi_console.sv, where it's used as a default value (never overridden) for the timeout_ns argument to await_flow_ctrl_signal. I think it might be better to drop this variable from the package, move it to the class, and remove the optional argument from await_flow_ctrl_signal (since it isn't used anyway).
Then the "multiply by 5" can be done in just the multi-slot tests, I think.
| extern task host_spi_console_write_when_ready(input bit [7:0] bytes[][], | ||
| uint timeout_ns = write_completion_timeout_ns); | ||
| extern task host_spi_console_write_when_ready( | ||
| input bit [7:0] bytes[][], |
There was a problem hiding this comment.
I realise this isn't new, but I'd probably suggest dropping the input on this argument (since it's inconsistent with the next argument)
| // Wait until the DEVICE signals it is awaiting a write, then perform one or more console write | ||
| // operations. This task tries to write all given payloads successively before waiting for the | ||
| // device to clear the sideband flow_control signal, and cannot short-circuit or early return. | ||
| extern task host_spi_console_write_when_ready(input bit [7:0] bytes[][], |
There was a problem hiding this comment.
Minor nitty comment: is this task actually used anywhere?
2 participants
This PR improves on the initial implementation for capturing reads in #28194 to make it more robust to logging changes on the DEVICE / software side that are not of interest to the test. Due to the way the spi_console works, the HOST-side needs to be constantly attentive to the DEVICE indicating it wishes to write to the console, and device software will block if the spi_device buffer is not emptied by the HOST reading from it.
Review commit by commit. These routines are not yet used in any tests / sequences, so there are no users to break (yet). However, future users will rely on this new behaviour.