Skip to content

Commit

Permalink
[spi_device] Harmonize address mode syncs to byte beats
Browse files Browse the repository at this point in the history
Simplify address mode synchronization by using a common commit point at
the end of the command phase. Add a sync pulse for the address mode to
commit at the 8th posedge of the SPI clock. This allows enough time for
software-originated changes to cross into the SPI domain and be picked
up for the next command. In addition, there is no need to reset the
entire spi_device IP when changing address modes after a period of SPI
activity.

Harmonize address calculations to align with this commit point. The
passthrough and upload modules have slightly different mechanisms, but
they now use the address mode from this point. The passthrough module
uses the registered value, but the upload module uses the D side of that
flop instead. The upload module may need this information for some
future enhancements related to metadata committed to the command FIFO.

In addition, inform software when a software-originated address mode
change is still pending. This may occur if no transactions occurred
since the last software write. When a write is pending, the value shown
is the pending value, not the current committed one in hardware.
However, that pending value will be picked up in time for the next
command. If the desired value is different from the prior write,
software may overwrite, as long as the SPI host is known to be inactive.

Signed-off-by: Alexander Williams <[email protected]>
  • Loading branch information
a-will committed Feb 1, 2024
1 parent 40a6ace commit 81cfbe0
Show file tree
Hide file tree
Showing 17 changed files with 850 additions and 793 deletions.
66 changes: 47 additions & 19 deletions hw/ip/spi_device/data/spi_device.hjson
Original file line number Diff line number Diff line change
Expand Up @@ -513,23 +513,6 @@
desc: "RX bit order on SDI. Module stores bitstream from MSB to LSB if value is 0.",
resval: "0",
},
{ bits: "16"
name: "addr_4b_en"
swaccess: "rw"
hwaccess: "hrw" // Updated by EN4B/EX4B
desc: '''4B Address Mode enable.

This field configures the internal module to receive 32 bits of the
SPI commands. The affected commands are the SPI read commands
except QPI, and program commands. It is expected for SW to
configure this field at the configuration stage and leave the
updation to HW until next reset.

Even though Read SFDP command has address fields, the SFDP command
is not affected by this field. The command always parse 24 bits on
the SPI line 0 following the SPI command as the address field.
'''
}
{ bits: "24"
name: "mailbox_en"
desc: '''Mailbox enable.
Expand Down Expand Up @@ -588,6 +571,52 @@
} // f: mailbox
]
} // R: INTERCEPT_EN
{ name: "ADDR_MODE"
desc: '''Flash address mode configuration

This register shows the current address mode and pending changes.
It is updated by the HW when the command phase completes.
'''
swaccess: "ro"
hwaccess: "hwo"
hwext: true
tags: [
// SW is not able to read back the ADDR_MODE register right after write.
// The pending field changes whenever the CSR is written, regardless of the value.
// So, the CSR is excluded from most automated CSR tests.
"excl:CsrNonInitTests:CsrExclWrite"
]
fields: [
{ bits: "0"
name: "addr_4b_en"
swaccess: "rw"
hwaccess: "hrw" // Updated by EN4B/EX4B
hwqe: true
desc: '''4B Address Mode enable.

This field configures the internal module to receive 32 bits of the SPI commands.
The affected commands are the SPI read commands except QPI, and program commands.
It is expected for SW to configure this field at the configuration stage and release control to HW until the next reset.

Even though Read SFDP command has address fields, the SFDP command is not affected by this field.
The command always parse 24 bits on the SPI line 0 following the SPI command as the address field.

This field has noteworthy read behavior.
If a software-initiated change is still `pending` the sync to the SPI domain, this bit will reflect the value to be sent.
Otherwise, this field will reflect the current value observed in the SPI domain.
'''
},
{ bits: "31"
name: "pending"
desc: '''SW-originated change is pending.

This bit is 1 whenever the current value of addr_4b_en has yet to sync with the SPI domain.
If an EN4B or EX4B command arrives next, the current value in `addr_4b_en` will be ignored,
and the SPI flash command will take priority, with an update to `addr_4b_en` to match the command's result.
'''
}
]
} // R: ADDR_MODE
{ name: "LAST_READ_ADDR"
desc: '''Last Read Address

Expand Down Expand Up @@ -622,8 +651,7 @@
hwqe: true
tags: [
// SW is not able to read back the STATUS register right after write.
// The read back value is committed value, which needs at least a SPI
// transaction.
// The read back value is the committed value, which needs at least a SPI transaction.
// So excluded from CSR automation test.
"excl:CsrNonInitTests:CsrExclWrite"
]
Expand Down
300 changes: 154 additions & 146 deletions hw/ip/spi_device/doc/registers.md

Large diffs are not rendered by default.

14 changes: 11 additions & 3 deletions hw/ip/spi_device/doc/theory_of_operation.md
Original file line number Diff line number Diff line change
Expand Up @@ -183,13 +183,21 @@ The HW logic creates the default **CMD_INFO** structures for the two commands.
The command parser module uses the generated structures to process and trigger the 4B management module.

When the HW receives one of the commands, the HW changes the broadcast signal *cfg_addr_4b_en*.
Also the HW updates [`CFG.addr_4b_en`](registers.md#cfg) 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).
Also the HW updates [`ADDR_MODE.addr_4b_en`](registers.md#addr_mode) after passing through CDC.
It takes at most three SYS_CLK cycles to update the value in the *ADDR_MODE* register after the completion of the SPI transaction (CSb de-assertion).

_Note: The HW changes the broadcasting signal and the CSR even though the 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 information entries.
The broadcasted `cfg_addr_4b_en` signal affects the read commands for which `addr_mode` is *AddrCfg* in their command information entries.

SW may also configure the initial address mode by writing to [`ADDR_MODE.addr_4b_en`](registers.md#addr_mode).
SW must not write to this CSR unless it knows the upstream host is inactive and expects initial values for the next transaction.
After initialization (from the upstream SPI host's perspective), it is a protocol violation for the address mode to change outside the host's explicit commands.
A SW request will set the [`ADDR_MODE.pending`](registers.md#addr_mode) bit, which will remain set until HW consumes the update.
SW will see the intended mode in [`ADDR_MODE.addr_4b_en`](registers.md#addr_mode) until HW completes the request.
However, SW may overwrite the requested value, so long as the SPI host is held inactive.
The request does not complete until the SPI host completes the opcode phase of the transaction, and it will take effect for that same transaction's address phase (if any).

### Command Upload

Expand Down
13 changes: 7 additions & 6 deletions hw/ip/spi_device/dv/env/seq_lib/spi_device_pass_base_vseq.sv
Original file line number Diff line number Diff line change
Expand Up @@ -342,9 +342,10 @@ class spi_device_pass_base_vseq extends spi_device_base_vseq;
ral.cfg.rx_order.set(cfg.spi_host_agent_cfg.device_bit_dir);

if (cfg.do_addr_4b_cfg) begin
`DV_CHECK_RANDOMIZE_FATAL(ral.cfg.addr_4b_en)
cfg.spi_host_agent_cfg.flash_addr_4b_en = ral.cfg.addr_4b_en.get();
cfg.spi_device_agent_cfg.flash_addr_4b_en = ral.cfg.addr_4b_en.get();
`DV_CHECK_RANDOMIZE_FATAL(ral.addr_mode.addr_4b_en)
cfg.spi_host_agent_cfg.flash_addr_4b_en = ral.addr_mode.addr_4b_en.get();
cfg.spi_device_agent_cfg.flash_addr_4b_en = ral.addr_mode.addr_4b_en.get();
csr_update(.csr(ral.addr_mode));
cfg.do_addr_4b_cfg = 0;
end

Expand Down Expand Up @@ -523,7 +524,7 @@ class spi_device_pass_base_vseq extends spi_device_base_vseq;
@(posedge cfg.spi_host_agent_cfg.vif.csb[FW_FLASH_CSB_ID]);
`uvm_info(`gfn, $sformatf("set addr_4b_en = %0b", value), UVM_MEDIUM)
cfg.clk_rst_vif.wait_clks(3);
void'(ral.cfg.addr_4b_en.predict(.value(value)));
void'(ral.addr_mode.addr_4b_en.predict(.value(value)));
end join_none
endtask

Expand Down Expand Up @@ -744,14 +745,14 @@ class spi_device_pass_base_vseq extends spi_device_base_vseq;
// Wait for register to be free
`DV_SPINWAIT(
while(1) begin
if (ral.cfg.is_busy()) begin
if (ral.addr_mode.is_busy()) begin
cfg.clk_rst_vif.wait_clks(1);
end else begin
break;
end
end)
// Check the contents of addr4b_en
csr_rd_check(.ptr(ral.cfg.addr_4b_en),
csr_rd_check(.ptr(ral.addr_mode.addr_4b_en),
.compare_value(cfg.spi_device_agent_cfg.flash_addr_4b_en));
cfg.spi_cfg_sema.put();
endtask
Expand Down
14 changes: 7 additions & 7 deletions hw/ip/spi_device/dv/env/spi_device_scoreboard.sv
Original file line number Diff line number Diff line change
Expand Up @@ -159,16 +159,16 @@ class spi_device_scoreboard extends cip_base_scoreboard #(.CFG_T (spi_device_env
if (`GET_OPCODE_VALID_AND_MATCH(cmd_info_en4b, item.opcode)) begin
if (cfg.en_cov) begin
cov.spi_device_addr_4b_enter_exit_command_cg.sample(
.addr_4b_en(1), .prev_addr_4b_en(`gmv(ral.cfg.addr_4b_en)));
.addr_4b_en(1), .prev_addr_4b_en(`gmv(ral.addr_mode.addr_4b_en)));
end
void'(ral.cfg.addr_4b_en.predict(.value(1), .kind(UVM_PREDICT_WRITE)));
void'(ral.addr_mode.addr_4b_en.predict(.value(1), .kind(UVM_PREDICT_WRITE)));
`uvm_info(`gfn, "Enable 4b addr due to cmd EN4B", UVM_MEDIUM)
end else if (`GET_OPCODE_VALID_AND_MATCH(cmd_info_ex4b, item.opcode)) begin
if (cfg.en_cov) begin
cov.spi_device_addr_4b_enter_exit_command_cg.sample(
.addr_4b_en(0), .prev_addr_4b_en(`gmv(ral.cfg.addr_4b_en)));
.addr_4b_en(0), .prev_addr_4b_en(`gmv(ral.addr_mode.addr_4b_en)));
end
void'(ral.cfg.addr_4b_en.predict(.value(0), .kind(UVM_PREDICT_WRITE)));
void'(ral.addr_mode.addr_4b_en.predict(.value(0), .kind(UVM_PREDICT_WRITE)));
`uvm_info(`gfn, "Disable 4b addr due to cmd EX4B", UVM_MEDIUM)
end else if (`GET_OPCODE_VALID_AND_MATCH(cmd_info_wren, item.opcode)) begin
update_wel = 1;
Expand Down Expand Up @@ -987,9 +987,9 @@ class spi_device_scoreboard extends cip_base_scoreboard #(.CFG_T (spi_device_env
tpm_hw_reg_pre_val_aa[csr.get_name] = `gmv(csr);
end
end
if (cfg.en_cov && csr.get_name == "cfg") begin
bit pre_addr4b = `gmv(ral.cfg.addr_4b_en);
bit cur_addr4b = get_field_val(ral.cfg.addr_4b_en, item.a_data);
if (cfg.en_cov && csr.get_name == "addr_mode") begin
bit pre_addr4b = `gmv(ral.addr_mode.addr_4b_en);
bit cur_addr4b = get_field_val(ral.addr_mode.addr_4b_en, item.a_data);
// cover that the addr4b is updated by SW
if (pre_addr4b != cur_addr4b) cov.sw_update_addr4b_cg.sample();
end
Expand Down
7 changes: 7 additions & 0 deletions hw/ip/spi_device/rtl/spi_cmdparse.sv
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ module spi_cmdparse
output cmd_info_t cmd_only_info_o,
output logic [CmdInfoIdxW-1:0] cmd_only_info_idx_o,

// Synchronization pulse that indicates the 8th bit of the command is
// arriving. Originates from the SCK domain.
output logic cmd_sync_pulse_o,

// CFG: Intercept
input cfg_intercept_en_status_i,
input cfg_intercept_en_jedec_i,
Expand Down Expand Up @@ -308,6 +312,7 @@ module spi_cmdparse
sel_dp = DpNone;
cmd_only_sel_dp = DpNone;

cmd_sync_pulse_o = 1'b0;
latch_cmdinfo = 1'b 0;

intercept_d = 1'b 0;
Expand All @@ -317,6 +322,7 @@ module spi_cmdparse
if (module_active && data_valid_i && cmd_info_d.valid) begin
// 8th bit is valid here
latch_cmdinfo = 1'b 1;
cmd_sync_pulse_o = 1'b1;

priority case (1'b 1)
opcode_readstatus: begin
Expand Down Expand Up @@ -390,6 +396,7 @@ module spi_cmdparse
else if (module_active && data_valid_i) begin
// Could not find valid command information entry.
st_d = StWait;
cmd_sync_pulse_o = 1'b1;
end // if (module_active && data_valid_i)
end

Expand Down
50 changes: 16 additions & 34 deletions hw/ip/spi_device/rtl/spi_device.sv
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ module spi_device

spi_mode_e spi_mode;
logic cfg_addr_4b_en;
logic cmd_sync_addr_4b_en;

// Address 3B/ 4B tracker related signals
//
Expand Down Expand Up @@ -268,11 +269,15 @@ module spi_device
cmd_info_t cmd_only_info_broadcast;
logic [CmdInfoIdxW-1:0] cmd_only_info_idx_broadcast;

// Synchronization pulse indicating that the 8th bit of the command is
// arriving. This is used to time the transfer of some data to/from the sys
// domain.
logic cmd_sync_pulse;

// CSb edge detector in the system clock and SPI input clock
// SYS clock assertion can be detected but no usage for the event yet.
// SPI clock de-assertion cannot be detected as no SCK at the time is given.
logic sys_csb_deasserted_pulse;
logic sck_csb_asserted_pulse ;

// Read Status input and broadcast
logic sck_status_busy_set; // set by HW (upload)
Expand Down Expand Up @@ -358,8 +363,6 @@ module spi_device
assign txorder = reg2hw.cfg.tx_order.q;
assign rxorder = reg2hw.cfg.rx_order.q;

//assign cfg_addr_4b_en = reg2hw.cfg.addr_4b_en.q;

// CSb : after 2stage synchronizer
assign hw2reg.status.csb.d = sys_csb_syncd;
assign hw2reg.status.tpm_csb.d = sys_tpm_csb_syncd;
Expand Down Expand Up @@ -709,23 +712,6 @@ module spi_device
.q_o (sys_csb_syncd)
);

// CSb edge on the SPI input clock
prim_edge_detector #(
.Width (1 ),
.ResetValue (1'b 1),
.EnSync (1'b 1)
) u_csb_edge_spiclk (
.clk_i (clk_spi_in_buf),
.rst_ni (rst_spi_n),

.d_i (sck_csb),
.q_sync_o ( ),

// posedge(deassertion) cannot be detected as clock could be absent.
.q_posedge_pulse_o ( ),
.q_negedge_pulse_o (sck_csb_asserted_pulse)
);

// TPM CSb 2FF sync to SYS_CLK
prim_flop_2sync #(
.Width (1 ),
Expand Down Expand Up @@ -1058,6 +1044,7 @@ module spi_device
.cmd_info_idx_o (cmd_info_idx_broadcast),
.cmd_only_info_o (cmd_only_info_broadcast),
.cmd_only_info_idx_o (cmd_only_info_idx_broadcast),
.cmd_sync_pulse_o (cmd_sync_pulse),

.cfg_intercept_en_status_i (cfg_intercept_en.status),
.cfg_intercept_en_jedec_i (cfg_intercept_en.jedec),
Expand Down Expand Up @@ -1185,7 +1172,7 @@ module spi_device

.clk_out_i (clk_spi_out_buf),

.inclk_csb_asserted_pulse_i (sck_csb_asserted_pulse),
.cmd_sync_pulse_i (cmd_sync_pulse),

.sys_jedec_i (jedec_cfg),

Expand Down Expand Up @@ -1226,8 +1213,6 @@ module spi_device

.clk_csb_i (clk_csb),

.sck_csb_asserted_pulse_i (sck_csb_asserted_pulse),

.sel_dp_i (cmd_dp_sel),
.cmd_only_sel_dp_i (cmd_only_dp_sel),

Expand Down Expand Up @@ -1262,7 +1247,7 @@ module spi_device

.spi_mode_i (spi_mode),

.cfg_addr_4b_en_i (cfg_addr_4b_en),
.cmd_sync_cfg_addr_4b_en_i (cmd_sync_addr_4b_en),

.cmd_only_info_i (cmd_only_info_broadcast),
.cmd_only_info_idx_i (cmd_only_info_idx_broadcast),
Expand Down Expand Up @@ -1326,18 +1311,15 @@ module spi_device

.spi_clk_i (clk_spi_in_buf),

.spi_csb_asserted_pulse_i (sck_csb_asserted_pulse ),
.sys_csb_deasserted_pulse_i(sys_csb_deasserted_pulse),
.cmd_sync_pulse_i (cmd_sync_pulse),

// Assume CFG.addr_4b_en is not external register.
// And has the permissions as below:
// swaccess: "rw"
// hwaccess: "hrw"
.reg2hw_cfg_addr_4b_en_q_i (reg2hw.cfg.addr_4b_en.q ), // registered input
.hw2reg_cfg_addr_4b_en_de_o (hw2reg.cfg.addr_4b_en.de),
.hw2reg_cfg_addr_4b_en_d_o (hw2reg.cfg.addr_4b_en.d ),
.reg2hw_addr_mode_addr_4b_en_q_i (reg2hw.addr_mode.addr_4b_en.q),
.reg2hw_addr_mode_addr_4b_en_qe_i (reg2hw.addr_mode.addr_4b_en.qe),
.hw2reg_addr_mode_pending_d_o (hw2reg.addr_mode.pending.d),
.hw2reg_addr_mode_addr_4b_en_d_o (hw2reg.addr_mode.addr_4b_en.d),

.spi_cfg_addr_4b_en_o (cfg_addr_4b_en), // broadcast
.spi_cfg_addr_4b_en_o (cfg_addr_4b_en), // broadcast
.cmd_sync_cfg_addr_4b_en_o (cmd_sync_addr_4b_en), // early output for upload

.spi_addr_4b_set_i (cmd_en4b_pulse), // EN4B command
.spi_addr_4b_clr_i (cmd_ex4b_pulse) // EX4B command
Expand Down
Loading

0 comments on commit 81cfbe0

Please sign in to comment.