[spi_device] Define a buffer address width
Previous design defined `threshold` as `[BufferAw:0]`. It is
syntatically correct but confuses contextually. It reduces readability.
In this commit, `SramBufferAw` is defined. It is the required bit width
of a Read Buffer size in bytes rather than the entries in DPSRAM.
This commit does not alter the width of the threshold. It stays 10 bits
as a Read Buffer size is 1kB.
Signed-off-by: Eunchan Kim <eunchan@opentitan.org>
diff --git a/hw/ip/spi_device/data/spi_device.hjson b/hw/ip/spi_device/data/spi_device.hjson
index 676cb66..ac1ddbb 100644
--- a/hw/ip/spi_device/data/spi_device.hjson
+++ b/hw/ip/spi_device/data/spi_device.hjson
@@ -617,6 +617,7 @@
name: "threshold"
desc: '''If 0, disable the watermark. If non-zero, when the host
access above or equal to the threshold, it reports an interrupt.
+ The value is byte-granularity not SRAM index.
'''
} // f: threshold
]
diff --git a/hw/ip/spi_device/rtl/spi_device_pkg.sv b/hw/ip/spi_device/rtl/spi_device_pkg.sv
index 477094f..c56fe82 100644
--- a/hw/ip/spi_device/rtl/spi_device_pkg.sv
+++ b/hw/ip/spi_device/rtl/spi_device_pkg.sv
@@ -372,11 +372,17 @@
} spi_cmd_e;
// Sram parameters
- parameter int unsigned SramDw = 32;
- parameter int unsigned SramStrbW = SramDw/8;
+ parameter int unsigned SramDw = 32;
+ parameter int unsigned SramStrbW = SramDw/8;
+ parameter int unsigned SramOffsetW = $clog2(SramStrbW);
// Msg region is used for read cmd in Flash and Passthrough region
parameter int unsigned SramMsgDepth = 512; // 2kB
+ parameter int unsigned SramMsgBytes = SramMsgDepth * SramStrbW;
+
+ // Address Width of a Buffer in bytes
+ parameter int unsigned SramBufferBytes = SramMsgBytes / 2; // Double Buffer
+ parameter int unsigned SramBufferAw = $clog2(SramBufferBytes);
parameter int unsigned SramMailboxDepth = 256; // 1kB for mailbox
diff --git a/hw/ip/spi_device/rtl/spi_readcmd.sv b/hw/ip/spi_device/rtl/spi_readcmd.sv
index 7004929..1f9a62c 100644
--- a/hw/ip/spi_device/rtl/spi_readcmd.sv
+++ b/hw/ip/spi_device/rtl/spi_readcmd.sv
@@ -105,9 +105,7 @@
// SFDP Base Addr: the beginning index of the SFDP region in DPSRAM
// SFDP Depth: The size of the SFDP buffer (64 fixed in the spi_device_pkg)
parameter sram_addr_t SfdpBaseAddr = spi_device_pkg::SramSfdpIdx,
- parameter int unsigned SfdpDepth = spi_device_pkg::SramSfdpDepth,
-
- localparam int unsigned BufferAw = $clog2(ReadBufferDepth)
+ parameter int unsigned SfdpDepth = spi_device_pkg::SramSfdpDepth
) (
input clk_i,
input rst_ni,
@@ -145,7 +143,7 @@
input logic [CmdInfoIdxW-1:0] cmd_info_idx_i,
// Double buffering in bytes
- input [BufferAw:0] readbuf_threshold_i,
+ input [SramBufferAw-1:0] readbuf_threshold_i,
// The command mode is 4B mode. Every read command receives 4B address
input addr_4b_en_i,
@@ -738,9 +736,7 @@
);
// Double Buffer Management logic
- spid_readbuffer #(
- .ReadBufferDepth (ReadBufferDepth)
- ) u_readbuffer (
+ spid_readbuffer u_readbuffer (
.clk_i,
.rst_ni,
diff --git a/hw/ip/spi_device/rtl/spid_readbuffer.sv b/hw/ip/spi_device/rtl/spid_readbuffer.sv
index cc4a14c..26fce71 100644
--- a/hw/ip/spi_device/rtl/spid_readbuffer.sv
+++ b/hw/ip/spi_device/rtl/spid_readbuffer.sv
@@ -37,14 +37,9 @@
`include "prim_assert.sv"
-module spid_readbuffer #(
- // Buffer size: # of indices assigned to Read Buffer.
- // This is used to calculate double buffering and threshold.
- parameter int unsigned ReadBufferDepth = spi_device_pkg::SramMsgDepth,
-
- // Derived parameters
- localparam int unsigned BufferAw = $clog2(ReadBufferDepth)
-) (
+module spid_readbuffer
+ import spi_device_pkg::SramBufferAw, spi_device_pkg::FlashMode;
+(
input clk_i,
input rst_ni,
@@ -52,8 +47,8 @@
input spi_device_pkg::spi_mode_e spi_mode_i,
- input [31:0] current_address_i,
- input [BufferAw:0] threshold_i, // A buffer size among two buffers (in bytes)
+ input [31:0] current_address_i,
+ input [SramBufferAw-1:0] threshold_i,
input sfdp_hit_i,
input mailbox_hit_i,
@@ -70,11 +65,6 @@
// Definition //
////////////////
- localparam int unsigned BufferSize = ReadBufferDepth
- * spi_device_pkg::SramDw / 8; // bytes
- localparam int unsigned OneBufferSize = BufferSize / 2;
- localparam int unsigned OneBufferAw = $clog2(OneBufferSize);
-
typedef enum logic {
StIdle,
StActive
@@ -95,7 +85,7 @@
//
// ICEBOX(#10038): If the device goes sleep, the next_buffer_addr should be
// recoverable.
- logic [31-OneBufferAw:0] next_buffer_addr;
+ logic [31-SramBufferAw:0] next_buffer_addr;
logic active;
@@ -109,14 +99,14 @@
// Flip event handling
always_ff @(posedge clk_i or negedge sys_rst_ni) begin
if (!sys_rst_ni) begin
- next_buffer_addr <= (32-OneBufferAw)'(1); // pointing to next buffer
+ next_buffer_addr <= (32-SramBufferAw)'(1); // pointing to next buffer
end else if (active && flip) begin
next_buffer_addr <= next_buffer_addr + 1'b 1;
end
end
- logic [31-OneBufferAw:0] current_buffer_idx;
- assign current_buffer_idx = current_address_i[31:OneBufferAw];
+ logic [31-SramBufferAw:0] current_buffer_idx;
+ assign current_buffer_idx = current_address_i[31:SramBufferAw];
assign flip = current_buffer_idx == next_buffer_addr;
// make flip event single cycle pulse signal
@@ -136,7 +126,7 @@
// recover `next_buffer_addr`?
// Watermark event: Threshold should not be 0 to enable the event
- assign watermark_cross = (current_address_i[BufferAw:0] >= threshold_i)
+ assign watermark_cross = (current_address_i[SramBufferAw-1:0] >= threshold_i)
&& |threshold_i;
always_ff @(posedge clk_i or negedge sys_rst_ni) begin
