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] 2nd part of the Technical Specification #9974

Merged
merged 1 commit into from
Jan 12, 2022

Conversation

eunchan
Copy link
Contributor

@eunchan eunchan commented Jan 10, 2022

This PR adds the remained flash mode and passthrough mode on top of #9951 .

Details of the fields are explained in the {{<regref "CMD_INFO_0">}}

First 11 commands are assigned to specific submodules.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you document required encodings for the first 11 commands? As of now all reset values are 0, so it is important to have proper commands configured in the first 11 slots.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. Since I don't see any reason to have other than the fixed value for Read Status, Read JEDEC ID, and Read SFDP, I may follow the same process as EN4B / EX4B, which means, SW just configure valid and opcode.

I will create an issue and follow up.


- Support Serial Flash emulation
- HW processed Read Status, Read JEDEC ID, Read SFDP, EN4B/ EX4B, and multiple read commands
- 16 depth Command/ Address FIFOs and Payload buffer for command upload

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you mention the payload buffer size?

@@ -49,6 +69,38 @@ The SW can receive TPM commands with payload (address and data) and respond to t
The submodule provides the command, address, write, and read FIFOs for the SW to communicate with the TPM host system.
The submodule also supports the SW by managing a certain set of the FIFO registers and returning the read request by HW quickly.

In Flash mode, SPI Device HWIP behaves as a Serial Flash device by recognizing SPI Flash commands and processing those commands by HW.
The commands processed by HW are Read Status (1, 2, 3), Read JEDEC ID, Read SFDP, EN4B/ EX4B, and read commands with aids of SW.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aids -> aid


In Passthrough mode, SPI Device receives SPI transactions from a host system and forwards the transactions to a downstream flash device.
SW may filter prohibited commands by configuring 256-bit {{< regref "FILTER" >}} CSR.
The IP cancels ongoing transaction if the received opcode matches to the filter CSR by de-asserting CSb and gating SCK to the downstream flash device.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra space after SCK

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blame my keyboard. It easily adds additional space ... I probably switch the switch .


SW may program CSRs to change the address and/or the first 4 bytes of payload on-the-fly in Passthrough mode.
The address translation feature allows SW to maintain A/B binary images without aids of the host system.
The payload translation may be used to change the payload of Write Status commands to not allow certain fiels to be modified.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fiels -> fields

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another blame to my kb 😀

The address translation feature allows SW to maintain A/B binary images without aids of the host system.
The payload translation may be used to change the payload of Write Status commands to not allow certain fiels to be modified.

Flash mode and Passthrough mode may share the datapath.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think instead of saying "share the datapath", i feel like "active at the same time" might be clearer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that is clearer. However, the mode register can't be active both. Flash mode or Passthrough. In passthrough, flash mode can be active. That is intention. Let me rephrase the sentence.

Generic mode is exclusive. Flash and Passthrough modes share many parts of the datapath.
TPM mode only shares the SPI and has separate CSb port, which allows that the host sends TPM commands while other SPI mode is active.

Mod | FwMode | Status | JEDEC | SFDP | Mailbox | Read | Addr4B | Upload | Passthrough

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does the passthrough / passthru intercept box need a Y?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! @msfschaffner also pointed out. Revised.

[4] | Read SFDP
[10:5] | Read commands

When Read Status, Read JEDEC ID commands are processed in Flash mode, the other fields in the command information entry are not used.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just to make sure i understand, if we are in flash mode, OR we are in passthrough mode with intercept_en set, the other fields of these configurations are ignored right? If what i'm saying is true, it may be clearer to describe it that way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. Thanks for pointing it out.

As explained in the [previous section](#command-information-list), the command parser checks the index to activate Read Status / Read JEDEC ID/ Read Command / Address 4B modules.
Other than the first 11 slots and last two slots (the last two slots are not visible to SW), the cmdparse checks the *upload* field and activates the upload module if the field is set.

SW may configure the submodules if the modules process the command in the passthrough mode or not by changing {{<regref "INTERCEPT_EN">}} CSR.
Copy link

@tjaychen tjaychen Jan 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just to double check, you are saying that the submodules will process the command in passthrough mode only if the intercept_en is set right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Revised the sentence.


### Status Control

If the received command is one of the three read status commands, STATUS control module reigns SPI interface after the opcode.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reigns? just to be sure, you mean the status control module will take over the spi interface right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. "take over" is way better 👍

When the module receives `addr[0]`, at the positive edge of SCK, the module moves to appropriate command state based on the given CMD_INFO data.

If the received address falls into mailbox address ragne and mailbox feature is enabled, the module turns on the mailbox selection bit.
Then all out-going requests to the DPSRAM are forwarded to the mailbox section, not the read buffer section.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just wanted to clarify one thing here. If the mailbox space is for example located ahead of other space, we DO NOT dynamically switch right? Ie, once you start in mailbox, even if you read into non-mailbox space, will we still remain in mailbox space?

I'm not suggesting we change anything, just that it would be good to clarify the behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hum... good point. Let me check the design. I am honestly not sure about the behavior.

Also the HW updates {{<regref "CFG.addr_4b_en">}} after passing through CDC.
It takes at most three SYS_CLK cycles to update the value in the *CFG* register after the completion of the SPI transaction (CSb de-assertion).

_Note: The HW changes the broadcasting signal and the CSR even thought eh SPI host system sends more than 8 beats of the SPI S[0].

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit, extra underscore in front of Note? or maybe that was intended.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

even thought -> even though

Copy link

@tjaychen tjaychen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some nitpicks, but overall this looks great to me.

Copy link
Contributor

@msfschaffner msfschaffner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with a few nits

As explained in the [previous section](#command-information-list), the command parser checks the index to activate Read Status / Read JEDEC ID/ Read Command / Address 4B modules.
Other than the first 11 slots and last two slots (the last two slots are not visible to SW), the cmdparse checks the *upload* field and activates the upload module if the field is set.

SW can configure whether a submodule shoudl process the command while in the passthrough mode by setting the {{<regref "INTERCEPT_EN">}} CSR.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: shoudl -> should

Also the HW updates {{<regref "CFG.addr_4b_en">}} after passing through CDC.
It takes at most three SYS_CLK cycles to update the value in the *CFG* register after the completion of the SPI transaction (CSb de-assertion).

_Note: The HW changes the broadcasting signal and the CSR even though eh SPI host system sends more than 8 beats of the SPI S[0].
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: eh -> the

_Note: The HW changes the broadcasting signal and the CSR even though eh SPI host system sends more than 8 beats of the SPI S[0].
After the logic matches the received command byte with EN4B/ EX4B, the logic ignores the rest of the SPI data._

The broadcasted `cfg_addr_4b_en` signal affects the read commands which `addr_mode` is *AddrCfg* in their command informatio entries.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: informatio -> information

- The command matches to any of the rest command information entries.
- The matched entry has the `upload` field set.

The upload module checks the command information entry to determine whether the address/ payload fields to be uploaded or not.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: two spaces

The information of the pad enable and direction is given by SW.
SW configures the address size, payload lanes, dummy size in **CMD_INFO** slots.

If passthrough logic does not find valid commandinformation entry based on the received opcode, it assumes the command is **PayloadIn&** Single IO command.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: commandinformation -> command information

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, is the & intended in **PayloadIn&**?

@eunchan
Copy link
Contributor Author

eunchan commented Jan 12, 2022

I don't think this PR affects ROM binary and FPGA tests. Let me merge this.
image

This commit adds the flash and passthrough modes in the technical
specification.

Signed-off-by: Eunchan Kim <[email protected]>
@eunchan eunchan merged commit be2aec9 into lowRISC:master Jan 12, 2022
@eunchan eunchan deleted the spi_doc2 branch January 12, 2022 18:39
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.

4 participants