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] Chip Select glitch detector #12747

Closed
eunchan opened this issue May 18, 2022 · 5 comments · Fixed by #21120
Closed

[spi_device] Chip Select glitch detector #12747

eunchan opened this issue May 18, 2022 · 5 comments · Fixed by #21120
Assignees
Labels
Earlgrey-PROD Candidate Temporary label to triage issues into Earlgrey-PROD Milestones IP:spi_device Milestone:NA Issue has been triaged but no milestone assigned to this issue. Priority:P3 Priority: low Type:FutureRelease Not relevant to currently planned releases/milestones

Comments

@eunchan
Copy link
Contributor

eunchan commented May 18, 2022

Many SPI_DEVICE logics rely on a clean Chip Select signal. If Chip Select signal has glitches, the SPI_DEVICE behaves incorrectly. This issue is to add CS glitch detectors (two: one for SPI, other for TPM).

The glitch detectors catch CS assertion and deassertion without enough SPI CLK edges:

  • register is reset to 1 by CSb
  • register is cleared to 0 by csb_assertion_pulse
  • register is latched by clk_csb (not sure this is plausible as register's reset is CSb also)
  • the latched value is compared to 1 at sys_csb_deasserted_pulse. If the value is 1, then report error to the SW.

It may not be possible to use CSb as a reset, in that case the register may be reset by sys_rst_ni & ~sys_csb_deasserted_pulse.

CC: @tjaychen

Effort estimate by @hcallahan-lowrisc:

Note that there is a number of open issues about CSB behaviour and robustness to errors, so some estimated effort (if deemed necessary) may overlap between those issues. (#10058 #12747 #15517 #15721)

estimate 12
remaining 2023-03-23 12

@eunchan eunchan added Priority:P3 Priority: low IP:spi_device Type:FutureRelease Not relevant to currently planned releases/milestones Milestone:NA Issue has been triaged but no milestone assigned to this issue. labels May 18, 2022
@eunchan eunchan self-assigned this May 18, 2022
@andreaskurth
Copy link
Contributor

Triaged for spi_device. Assigning to M2.5 with Priority:P2 Priority: medium because even though this is about robustness in an environment with errors, we should check with the product team if the behavior of spi_device is acceptable. If yes, we can defer this to Type:FutureRelease Not relevant to currently planned releases/milestones .

@andreaskurth andreaskurth added this to the Discrete: M2.5 milestone Feb 24, 2023
@andreaskurth andreaskurth added Priority:P2 Priority: medium Triaged and removed Priority:P3 Priority: low labels Feb 24, 2023
@hcallahan-lowrisc
Copy link
Contributor

Note that there is a number of open issues about CSB behaviour and robustness to errors, so some estimated effort (if deemed necessary) may overlap between those issues. (#10058 #12747 #15517 #15721)

estimate 12
remaining 2023-03-23 12

@moidx
Copy link
Contributor

moidx commented Apr 5, 2023

@moidx to check if there are glitch suppression requirements that can be added to the IO pads (for integration purposes).

@zi-v
Copy link
Contributor

zi-v commented Apr 13, 2023

The IO slice has a configurable Schmitt trigger which suppresses pulses on transition if enabled. If this configuration input to the IO slice can be controlled/set by SW, the chance of catching a glitch on SPI in a well designed system is negligible.

@msfschaffner msfschaffner added the Earlgrey-PROD Candidate Temporary label to triage issues into Earlgrey-PROD Milestones label Oct 6, 2023
@a-will
Copy link
Contributor

a-will commented Jan 8, 2024

This issue is confusing, and I'm not sure what it's about. For SPI, PCB-level SI is up to the I/O buffers and the PCB design, and I think we should keep it that way.

However, logic that relies on sampling CSB to effect CDC of events may need a closer look. For flash and TPM modes, there should be no new events before at least 8 SCK cycles (smallest command), and any running state leading up to an event trigger should be thrown out on CSB release.

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 Milestone:NA Issue has been triaged but no milestone assigned to this issue. Priority:P3 Priority: low Type:FutureRelease Not relevant to currently planned releases/milestones
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants