Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ot] hw/opentitan: ot_spi_host: SPI Host V3 changes #120

Open
wants to merge 12 commits into
base: ot-earlgrey-9.1.0
Choose a base branch
from

Conversation

rivos-eblot
Copy link

@rivos-eblot rivos-eblot commented Jan 27, 2025

SFDP definitions are missing for now, and IRQ tests is still failing.

Moreover the pyot.py configuration needs to be updated (so shall the Bazel rules I guess)

    default:
    {
        rom: ${BASEDIR}/sw/device/silicon_creator/rom/mask_rom_fpga_cw310.elf
        otp: ${BASEDIR}/hw/top_earlgrey/data/otp/img_rma.24.vmem
        # ....
        machine: ot-earlgrey
        embedded-flash: 2
    }
    suffixes:
    [
        _fpga_cw310_rom_with_fake_keys
    ]
    logclass: {
        debug: [
            '^D\\d+'
        ]
        info: [
            '^I\\d+'
        ]
        warning: [
            '^W\\d+'
        ]
        error: [
            '^E\\d+'
        ]
    }
    tests:
    {
        spi_host_smoketest:
        {
            pre:
            [
                ${QEMU_BIN_DIR}/qemu-img create -f raw @{}/spiflash0.raw 16M
            ]
            opts:
            [
                -global ot-earlgrey-board.spiflash0=is25wp128
                -drive if=mtd,id=spiflash,bus=0,format=raw,file=@{}/spiflash0.raw
            ]
            timeout: 10
        }
        spi_host_macronix1Gb_flash_test:
        {
            pre:
            [
                ${QEMU_BIN_DIR}/qemu-img create -f raw @{}/spiflash1.raw 128M
            ]
            opts:
            [
                -global ot-earlgrey-board.spiflash1=mx66l1g45g
                -drive if=mtd,id=spiflash1,bus=1,format=raw,file=@{}/spiflash1.raw
            ]
            timeout: 20
        }
        spi_host_winbond_flash_test:
        {
            pre:
            [
                ${QEMU_BIN_DIR}/qemu-img create -f raw @{}/spiflash0.raw 64M
            ]
            opts:
            [
                -global ot-earlgrey-board.spiflash0=w25q512nw
                -drive if=mtd,id=spiflash0,bus=0,format=raw,file=@{}/spiflash0.raw
            ]
            timeout: 20
        }
        spi_host_irq_test:
        {
            icount: 6
            opts:
            [
                -global ot-spi_host.completion-delay=1000000
            ]
            timeout: 20
        }
   }

@rivos-eblot
Copy link
Author

Some progress:

+---------------------------------+--------+----------+--------+-------+
| Name                            | Result |     Time | Icount | Error |
+=================================+========+==========+========+=======+
| spi_host_irq_test               |   FAIL |  6116 ms |        |       |
+---------------------------------+--------+----------+--------+-------+
| spi_host_macronix1Gb_flash_test |   PASS | 12388 ms |        |       |
+---------------------------------+--------+----------+--------+-------+
| spi_host_smoketest              |   PASS |  5892 ms |        |       |
+---------------------------------+--------+----------+--------+-------+
| spi_host_winbond_flash_test     |   PASS | 11804 ms |        |       |
+---------------------------------+--------+----------+--------+-------+

@rivos-eblot rivos-eblot force-pushed the dev/ebl/spi_host_refresh branch 2 times, most recently from db325c8 to 6a84cf0 Compare January 28, 2025 13:37
@rivos-eblot rivos-eblot changed the title [Draft] SPI Host refresh [ot] hw/opentitan: ot_spi_host: SPI Host V3 changes Jan 28, 2025
@rivos-eblot
Copy link
Author

(stealing PR #119 title)

@rivos-eblot
Copy link
Author

spi_host_irq_test will require more work:

  1. test results depend on execution on previous tests
  2. several tests heavily depend on expected time synchronisation between the host CPU and the HW, which is something that is very hard, if not impossible, to handle with QEMU

spi_host_irq_test test results:

  1. spi_host_irq_test.active_event_irq: OK
  2. spi_host_irq_test.ready_event_irq OK
  3. spi_host_irq_test.tx_empty_event_irq: OK IIF ready_event_irq is not run
  4. spi_host_irq_test.tx_wm_event_irq: OK IIF ready_event_irq not run
  5. spi_host_irq_test.rx_full_event_irq: FAIL
  6. spi_host_irq_test.rx_wm_event_irq: FAIL
  7. spi_host_irq_test.cmd_busy_error_irq: OK IIF SPI clock is slow enough

Should be address with another PR; it might be impossible to fix all issues without tweaking the unit tests so that they do not expect a fixed clock ratio between CPU and SPI host.

@rivos-eblot
Copy link
Author

rivos-eblot commented Jan 28, 2025

@jwnrt you may need to update the Bazel CI files once this PR is merged, as the embedded flash is now using MTD bus # 2 (used to be # 1)

@rivos-eblot rivos-eblot force-pushed the dev/ebl/spi_host_refresh branch 2 times, most recently from b8812a0 to cdb471f Compare January 28, 2025 16:36
…read out

Refill the RX FIFO only when it is about to be empty or when its level reaches
the specified RX watermark level. This reduces the overhead required for filling
up the RX FIFO.

Signed-off-by: Emmanuel Blot <[email protected]>
(cherry picked from commit 04792225206608b2e0deb7cd26590cca3542edaf)
Should be closer to real HW.

Signed-off-by: Emmanuel Blot <[email protected]>
(cherry picked from commit 938395d0deb06df0f73f9062abd2c94d5817b844)
Replace previous boolean-based state with an enumeration, as
non-executing command can be either about to execute or in finalization
stage.

This enables differentiation of zero length command which can either be
a 1-byte command to be executed, or a command whose all bytes have been
processed.

Signed-off-by: Emmanuel Blot <[email protected]>
(cherry picked from commit 98a555a1449856df66c331de05d9338aac5fc096)
…ble property.

A better solution would be to compute the delay based on the input clock and the
SPI clock, but it requires basic clock management, which is not available yet.

Signed-off-by: Emmanuel Blot <[email protected]>
It is no longer edge-driven, but level-driven. The SPI event bit cannot
be explicitly cleared, it only tracks the current statuses of the IP.

Signed-off-by: Emmanuel Blot <[email protected]>
… qemu_log_mask

This is more coherent with other OT IP blocks.

Signed-off-by: Emmanuel Blot <[email protected]>
This command is supported on Macronix flash devices, among other.
It is similar to the existing QPP command, with a different opcode.

Signed-off-by: Emmanuel Blot <[email protected]>
- enables multiple SPI data flash configuration
- fixes bus assignment (collisions between SPI buses and internal flash)
- flash device type should now be defined as board properties
- only explicitly defined flash devices are instantiated, as the OT FPGA
  HW enables swapping/selecting flash types

Signed-off-by: Emmanuel Blot <[email protected]>
@rivos-eblot rivos-eblot force-pushed the dev/ebl/spi_host_refresh branch from cdb471f to 9995d51 Compare January 28, 2025 18:04
@rivos-eblot
Copy link
Author

Note: need to check if the ACTIVE bit implementation has been reworked in the RTL. If used to flicker between each command, rendering it useless from a SW perspective. Maybe this has been fixed, in which case QEMU needs also be fixed.

Update:
6. spi_host_irq_test.rx_full_event_irq: OK
7. spi_host_irq_test.rx_wm_event_irq: OK

@rivos-eblot
Copy link
Author

rivos-eblot commented Jan 29, 2025

I do not think it is possible to fully pass the spi_host_irq_test for now. If I get it right:

  1. The STATUS.active bit goes down between each command. When multiple commands are pushed to the command FIFO, polling this bit alone does not give the information that all commands/transfers have been handled, as the Ibex may poll this bit between each command. To ensure all the commands have completed, the STATUS.cmdqd should be polled for 0 to ensure that all commands, except maybe the last one, have been handled. Only then the STATUS.active bit should be polled. Please let me know if I get it wrong.

  2. Tests such cmd_busy_error_irq expect that the Ibex pushes commands faster than the HW can handle them. This may be right for the selected SPI clock and the tight synchronization between the Ibex clock and the peripheral clock on the real HW, but this may not be true on QEMU. It is possible to slow down the SPI execution command on QEMU - so this test passes; however this breaks other tests that have a short timeout, such as tx_wm_event_irq

On my machine (timings may differ from one host to another), it is possible to pass all the spi_host_irq_test tests but the last one ( cmd_busy_error_irq) with this SPI host option: -global ot-spi_host.completion-delay=10000

This PR is ready for review.

@rivos-eblot rivos-eblot requested a review from jwnrt January 29, 2025 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant