[chip-test] chip_sw_spi_device_output_when_disabled_or_sleeping

andreaskurth commented 1 year ago

Note: This is a DV/simulation-only clone of a SiVal test tracked in https://github.com/lowRISC/opentitan/issues/20635.

Test point

https://github.com/lowRISC/opentitan/blob/d161dede9cfb981ce39c892072e3558053c9894e/hw/top_earlgrey/data/chip_testplan.hjson?plain=1#L218-L240

Host side component

SystemVerilog

OpenTitanTool infrastructure implemented

None

Contact person

@andreaskurth, @cfrantz, @a-will

Checklist

Please fill out this checklist as items are completed. Link to PRs and issues as appropriate.

[ ] Check if existing test covers most or all of this testpoint (see below; if so, either extend said test to cover all points, or skip the next 3 checkboxes)
[ ] Device-side (C) component developed
[ ] Bazel build rules developed
[ ] Host-side component developed
[ ] HJSON test plan updated with test name (so it shows up in the dashboard)
[ ] Test added to dvsim nightly regression (and passing at time of checking)

Existing tests relevant for this testpoint

The following tests may already cover parts of this testpoint. This has to be checked. If possible, existing tests should be extended to cover this testpoint.

https://github.com/lowRISC/opentitan/blob/f65ceb27d5661e3f9ccdb6335f1a23399039d7b5/hw/top_earlgrey/data/chip_testplan.hjson?plain=1#L472-L486

https://github.com/lowRISC/opentitan/blob/f65ceb27d5661e3f9ccdb6335f1a23399039d7b5/hw/top_earlgrey/data/chip_testplan.hjson#L435-L451

johngt commented 1 year ago

This has been tagged as a V3 test. If it is useful for testing this for the TO then this should be completed - otherwise we should push this to the M Backlog

andreaskurth commented 1 year ago

This has been tagged as a V3 test.

The V2 testplan was already defined when this testpoint got added, so we added it as a V3 test.

If it is useful for testing this for the TO then this should be completed - otherwise we should push this to the M Backlog

This originated from discussions with product teams, so please check those notes for prioritization. CC @hcallahan-lowrisc

johngt commented 1 year ago

Changed to backlog - and set as a V3 item

engdoreis commented 9 months ago

This issue is duplicated by https://github.com/lowRISC/opentitan/issues/20635 I'm closing it, if needed we can reopen.

andreaskurth commented 9 months ago

@engdoreis: #20635 is a SiVal test, whereas this issue specifies some checks to be done by DV. I'm fine with closing this issue if the SiVal test can fully do the DV checks. (See testplan linked above; one example is "DV environment to check that SPI outputs match configuration by SW."). Can you confirm that all DV checks can also be performed in the SiVal test?

engdoreis commented 9 months ago

@andreaskurth, Yes, the SiVal test can/will cover all the checks described. Where it says DV environment, it would be done by the Rust host side. However, we don't plan to port the test to work in DV. Maybe we should keep this issue opened to implement the DV environment later?

andreaskurth commented 9 months ago

Thanks @engdoreis. This sounds good to me -- unless we have a policy saying we shouldn't replicate a test in DV for which a SiVal test (with Rust code on the host side that does what the DV env would do)?

I'll reopen for now and edit the description to clarify that this is a replication of a SiVal test.

andreaskurth commented 4 months ago

@antmarzam tagged this with https://github.com/lowRISC/opentitan/labels/Triage%3A%20deprioritize%3F because he's not sure he'll have time to complete this for M5. We'll discuss it in today's triage meeting.

andreaskurth commented 4 months ago

We think this is still an important test to have. From Alex's comment here, there is already a sequence that is very similar to this: chip_sw_sleep_pin_retention_vseq. @antmarzam could you please nonetheless try to complete this for M5?

andreaskurth commented 4 months ago

@antmarzam is making progress on this and has not found faulty behavior so far, but the test is not ready for merging yet. In the meantime, a SiVal test that covers this has been added, and it passes on ES silicon and FPGA (#20635). I thus don't think we need to gate M5 on this and am moving this to M6.

vogelpi commented 3 months ago

I am closing the issue as PR #23986 which addresses this got merged.

lowRISC / opentitan