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/dv] Verify CDC path when switching to/out of FW mode #15457

Closed
weicaiyang opened this issue Oct 12, 2022 · 9 comments
Closed

[spi_device/dv] Verify CDC path when switching to/out of FW mode #15457

weicaiyang opened this issue Oct 12, 2022 · 9 comments
Labels
Component:CDC Related to CDC flow Earlgrey-PROD Candidate Temporary label to triage issues into Earlgrey-PROD Milestones IP:spi_device Priority:P2 Priority: medium

Comments

@weicaiyang
Copy link
Contributor

weicaiyang commented Oct 12, 2022

Depend on how we decide in #15452

  1. We need to have CDC model in the sram, in order to verify this
  2. The sequence to switch to or out of FW mode is defined in the CSR section - sram_clk_en

estimate 12
This is from effort estimation exercise

@andreaskurth
Copy link
Contributor

Triaged for spi_device. Assigning to M2.5 with Priority:P1 Priority: high because I think verifying this CDC path is of high importance and we should work on this sooner rather than later.

@andreaskurth andreaskurth added Priority:P1 Priority: high Component:CDC Related to CDC flow Triaged and removed Component:DV DV issue: testbench, test case, etc. labels Feb 23, 2023
@johngt
Copy link

johngt commented Apr 1, 2023

@cindychip / @msfschaffner to comment or reassign

@johngt
Copy link

johngt commented Apr 19, 2023

Assigning to @msfschaffner to forward assign

@msfschaffner
Copy link
Contributor

We are not going to make any FIFO / DPRAM changes to spi_device (as proposed in #15452) before M2.5 due to the invasive nature of that change.

The course of action is to waive the associated CDC messages, and rely on proper sequencing of the mode change / clock change as described here:

// SRAM clock
// If FwMode, SRAM clock for B port uses peripheral clock (clk_i)
// If FlashMode or PassThrough, SRAM clock for B port uses SPI_CLK
// To remove glitch, CG cell is put after clock mux
// The enable signal is not synchronized to SRAM_CLK when clock is
// switched into SPI_CLK. So, change the clock only when SPI_CLK is
// not toggle.
//
// Programming sequence:
// Change to SPI_CLK
// 1. Check if SPI line is idle.
// 2. Clear sram_clk_en to 0.
// 3. Change mode to FlashMode or PassThrough
// 4. Set sram_clk_en to 1.
// Change to peripheral clk
// 1. Check if SPI_CLK is idle
// 2. Clear sram_clk_en to 0.
// 3. Change mode to FwMode
// 4. Set sram_clk_en to 1.

CC @cindychip

@johngt
Copy link

johngt commented May 4, 2023

CDC enablement was done recently.
@msfschaffner to provide a more up to date comment about this issue given the recent changes.

@msfschaffner
Copy link
Contributor

msfschaffner commented May 4, 2023

The course of action was to waive the associated messages in static CDC analysis (which was done by @cindychip), and update the docs in #18102 to note this limitation.

We can therefore move this to the M2.5 backlog so that we can track this issue and fix it properly at a later point in time.

@msfschaffner
Copy link
Contributor

Oh, one thing I should point out: we have two CDC methodologies:

  1. static CDC analysis using the RealIntent tool. This has been cleaned up by @cindychip.
  2. simulation based insertion of random delays in CDC crossings. this has been enabled on some but not all IPs, yet - see [dv/cdc] Enable CDC instrumentation in all ips #16689.
    I think the one you are referring to was 2), but this issue was referring to 1).

@johngt
Copy link

johngt commented Jun 5, 2023

@hcallahan-lowrisc - I believe that you will be working on V2.5 sign-off for SPI_Device.
Just to note this particular path.

@rnongie rnongie modified the milestone: Discrete: M2.5.2 Jun 13, 2023
@msfschaffner msfschaffner added Priority:P2 Priority: medium and removed Priority:P1 Priority: high labels Jun 19, 2023
@msfschaffner msfschaffner added the Earlgrey-PROD Candidate Temporary label to triage issues into Earlgrey-PROD Milestones label Oct 7, 2023
@a-will
Copy link
Contributor

a-will commented Jan 8, 2024

FW mode to be removed, so issue is moot.

@a-will a-will closed this as not planned Won't fix, can't repro, duplicate, stale Jan 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component:CDC Related to CDC flow Earlgrey-PROD Candidate Temporary label to triage issues into Earlgrey-PROD Milestones IP:spi_device Priority:P2 Priority: medium
Projects
None yet
Development

No branches or pull requests

9 participants