design: working on memory sub #24
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #24 +/- ##
=======================================
Coverage 70.52% 70.52%
=======================================
Files 3 3
Lines 95 95
=======================================
Hits 67 67
Misses 28 28 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
I'm okay with the general logic, it just needs to work well with our current codebase. Like mentioned in inline comments, you need to use the interfaces and enums (defined in packages). This standardized AHB signals which reduces confusion and increases interoperability.
| parameter ADDR_WIDTH = 32, // Address width | ||
| parameter DATA_WIDTH = 32 // Data width | ||
| )( | ||
| input wire HCLK, // clock | ||
| input wire HRESETn, // reset | ||
| input wire [ADDR_WIDTH-1:0] HADDR, // address | ||
| input wire HWRITE, // write | ||
| input wire [1:0] HTRANS, // transfer | ||
| input wire [2:0] HSIZE, // tranfer size | ||
| input wire [DATA_WIDTH-1:0] HWDATA, // write data | ||
| output wire [DATA_WIDTH-1:0] HRDATA, // read data | ||
| output wire HREADY, // transfer ready | ||
| output wire [1:0] HRESP, // transfer response | ||
| // Memory Controller Interface | ||
| output wire [ADDR_WIDTH-1:0] MemAddr, // Memory address | ||
| output wire MemWrite, // Memory write enable | ||
| output wire [DATA_WIDTH-1:0] MemWData, // Memory write data | ||
| input wire [DATA_WIDTH-1:0] MemRData, // Memory read data | ||
| output wire MemReq, // Memory request signal | ||
| input wire MemReady // Memory ready signal |
There was a problem hiding this comment.
All of this has been defined in the two interfaces, AHBCommon_if and MemCommon_if. You can see SubDummy.sv to review usage.
We want them in an interface so other teams know how to interface with our code. For example, the memory team can use MemCommon_if as a blueprint to make their modules.
To get started, define your module as follows
module SubMemCtrl(
AHBCommon_if.subordinate sub,
MemCommon_if.memCtrl mem
);
endmodule| reg [DATA_WIDTH-1:0] internalRData; | ||
| reg internalReady; | ||
| reg [1:0] internalResp; |
There was a problem hiding this comment.
I don't understand why these should be "internal" to the controller subordinate.
|
|
||
| assign HRDATA = internalRData; | ||
| assign HREADY = internalReady; | ||
| assign HRESP = internalResp; |
There was a problem hiding this comment.
Response statuses are in an enum (ahb_resp_t) defined in AHBCommon_pkg.
Working on desiging a very basic memory suboordinate. Started by implementing the basic IO for now.