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

[spi_device] TPM interrupt for Write FIFO #15785

Closed
eunchan opened this issue Oct 27, 2022 · 6 comments · Fixed by #21322
Closed

[spi_device] TPM interrupt for Write FIFO #15785

eunchan opened this issue Oct 27, 2022 · 6 comments · Fixed by #21322
Assignees
Labels
Earlgrey-PROD Candidate Temporary label to triage issues into Earlgrey-PROD Milestones IP:spi_device Priority:P1 Priority: high

Comments

@eunchan
Copy link
Contributor

eunchan commented Oct 27, 2022

Related Issue: #15773

Current TPM IP does not provide an interrupt about the write transaction completion. Absent of the interrupt results the SW to wait until the transaction completed by reading the WrFIFO depth or polling the TPM CS# de-assertion.

A TPM transaction may took up to 55 us (64B + 1 Wait + address + cmd @ 10MHz). By introducing the completion interrupt for the Write FIFO, SW can switch the context.

Idea suggested by @tjaychen

CC: @weicaiyang @tjaychen @a-will

estimate 16
remaining 2023-03-21 16

@eunchan eunchan added this to the Backlog milestone Oct 27, 2022
@eunchan eunchan self-assigned this Oct 27, 2022
@andreaskurth
Copy link
Contributor

Triaged for spi_device. Assigning to M2.5 with Priority:P2 Priority: medium , though I'm not sure (1) whether this would free clock cycles on Ibex or on the TPM and (2) how this relates to requirements and capabilities of the product. Maybe @a-will knows? If not, I'll bring this up for discussion with the product team.

@andreaskurth andreaskurth modified the milestones: Backlog, Discrete: M2.5 Feb 23, 2023
@andreaskurth andreaskurth added Priority:P2 Priority: medium Triaged labels Feb 23, 2023
@a-will
Copy link
Contributor

a-will commented Feb 24, 2023

Triaged for spi_device. Assigning to M2.5 with Priority:P2 Priority: medium , though I'm not sure (1) whether this would free clock cycles on Ibex or on the TPM and (2) how this relates to requirements and capabilities of the product. Maybe @a-will knows? If not, I'll bring this up for discussion with the product team.

  1. This frees up cycles on Ibex. The current design only has an interrupt to inform software when the command and address phase completes (the "header"), but the transaction continues afterwards, potentially for a substantial amount of time (for the 64-byte payload case, especially).

  2. For a SPI TPM that only does 4-byte payloads, the thinking (in the IP doc) was that the transmission time is too short for an interrupt to be useful. However, as payloads grow larger, the time duration grows larger, potentially making busy-waiting expensive. For the sake of prioritization, the product team may be helpful for determining which payload sizes are important.

@hcallahan-lowrisc
Copy link
Contributor

Conclusion from product team feedback:

  • add interrupt on CSb deassertion after write command (write completion)
  • add interrupt after read command + header (read completion)

Estimate to cover RTL changes + DV

estimate 16
remaining 2023-03-21 16

@moidx moidx added Priority:P1 Priority: high and removed Priority:P2 Priority: medium labels Apr 5, 2023
@johngt johngt assigned johngt, GregAC and hcallahan-lowrisc and unassigned johngt May 4, 2023
@jesultra
Copy link
Contributor

jesultra commented May 4, 2023

As far as I know the only TPM register which is wider than 4 bytes is the FIFO register. That is used all the time, though. If we do not get interrupt at the end of a transaction involving the GSC receiving data through the FIFO register, that would be a problem.
We could busy-wait, but doing so would adversely affect our ability to verify performance of other aspects of our code, such as whether forwarding of characters from UART to USB encounters data loss during other system activity

@johngt
Copy link

johngt commented May 5, 2023

Based on prioritisation discussion - this RTL change will likely need to go into the next cycle and use the SW workaround.
De-prioritising accordingly. If this does need to go in then it should be re-raised as a P0 @jesultra / @moidx / @msfschaffner / @GregAC

@moidx
Copy link
Contributor

moidx commented Jun 15, 2023

We missed the window for RTL changes, we'll move this to the backlog so that it can be considered for the next RTL freeze.

@msfschaffner msfschaffner added the Earlgrey-PROD Candidate Temporary label to triage issues into Earlgrey-PROD Milestones label Oct 7, 2023
@msfschaffner msfschaffner removed this from the Earlgrey ES M2.5.Backlog milestone Nov 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Earlgrey-PROD Candidate Temporary label to triage issues into Earlgrey-PROD Milestones IP:spi_device Priority:P1 Priority: high
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants