Skip to content

GitLab

Commit cd0b202c authored by Guillem's avatar Guillem
Browse files

BUGFIX:Missing writes and bypass writes to counters 

When two writes happen consecutively the first write may not update the counter in the internal registers of the counter. Counters didn't bypass the results of the write, since counters are registered twice, one in the couynter module and once in the registers fo the wrapper, without the bypass the output was oscilating between the new value and the last value of the counter
parent bca59af2
Hide whitespace changes
Inline Side-by-side
hdl/PMU_raw.sv
View file @ cd0b202c
......@@ -289,6 +289,8 @@ end
//----------------------------------------------
//------------- Counters instance
//----------------------------------------------
//TODO: What happen if we is active but no write is done to the range of the
//counters?
PMU_counters # (
.REG_WIDTH (REG_WIDTH),
.N_COUNTERS (N_COUNTERS)
......
hdl/pmu_ahb.sv
View file @ cd0b202c
......@@ -89,7 +89,7 @@
//----------------------------------------------
// VIVADO: list of debug signals for ILA
//----------------------------------------------
//`define ILA_DEBUG_PMU_AHB
`define ILA_DEBUG_PMU_AHB
`ifdef ILA_DEBUG_PMU_AHB
(* MARK_DEBUG = "TRUE" *) wire debug_hsel_i ;
(* MARK_DEBUG = "TRUE" *) wire [HADDR_WIDTH-1:0] debug_haddr_i ;
......@@ -286,7 +286,7 @@ assign hreadyo_o = complete_transfer_status [0];
assign hresp_o = {{complete_transfer_status[1]},{complete_transfer_status[1]}};
always_comb begin
//TODO: I don't expect any of the cafe beaf values in the registers if they do
//NOTE: I don't expect any of the cafe beaf values in the registers if they do
//there is a bug
case (state)
TRANS_IDLE: begin
......@@ -341,8 +341,6 @@ end
delay1_dwrite_slave <= 0;
delay1_slv_index <= 0;
end else begin
//TODO: possible source of problems with reads and writes?
//Be sure ahb_write_req can't be 1 if hsel is low
delay1_ahb_write_req <= ahb_write_req;
delay1_dwrite_slave <= dwrite_slave;
delay1_slv_index <= slv_index;
......@@ -376,20 +374,25 @@ end
//If you add aditional logic that can change the values of the registers
//the next always block have to be modified to add the aditional
//conditions under which the slv_reg shall be updated
always_comb begin
//AHB write
//Write to slv registers if slave was selected & was a write
//Write to slv registers if slave was selected & was a write. Else
//register the values given by pmu_raw
if(address_phase.write && address_phase.select) begin
// get the values from the pmu_raw instance
slv_reg_D=pmu_regs_int;
slv_reg_D[slv_index] = dwrite_slave;
end else begin
if(delay1_ahb_write_req) begin
slv_reg_D=pmu_regs_int;
//If there is a write pending to be written and is not overwritten
//for the current write update two registers, else write only the
//most recent value
if(delay1_ahb_write_req && (delay1_slv_index!=slv_index)) begin
slv_reg_D[delay1_slv_index] = delay1_dwrite_slave;
slv_reg_D[slv_index] = dwrite_slave;
end else begin
slv_reg_D=pmu_regs_int;
slv_reg_D[slv_index] = dwrite_slave;
end
end else begin
slv_reg_D=pmu_regs_int;
end
end
......@@ -412,11 +415,40 @@ end
assume(0 == rstn_i);
end
end
//AHB assumptions
default clocking @(posedge clk_i); endclocking;
assert property (((hsel_i == 0) && f_past_valid && rstn_i==1 && $stable(rstn_i)) |=> (ahb_write_req == 0));
assert property (((hsel_i == 1) && f_past_valid && rstn_i==1 && $stable(rstn_i)) |=> (ahb_write_req == 1));
assert property (((hsel_i == 1) && f_past_valid && rstn_i==1 && $stable(rstn_i)) |-> (ahb_write_req == 0));
//If the peripheral is not selected there is no chance to issue a write
assert property (((hsel_i == 0) && f_past_valid
)
|=> (ahb_write_req == 0));
//If htrans_i is not iddle or busy and there is a write. ahb_write_req is
//one in the next cycle unless there is a reset in the next cycle
assert property (((hsel_i == 1) && f_past_valid && rstn_i==1 && $stable(rstn_i)
&& (htrans_i!=TRANS_IDLE && htrans_i!=TRANS_BUSY)
&& (hwrite_i==1)
)
|=> (ahb_write_req == 1) || rstn_i==0);
// If event 8 is low and current transaction is not a write, counter is
// stable
assert property ((events_i[8]==0 && $stable(events_i[8]) &&
ahb_write_req==0
)
|=> $stable(slv_reg[9]) ||
(slv_reg[9]==($past(slv_reg[9])+1))
|| $past(rstn_i)==1);
//*** TODO: Rewrite as a single assertion ***/
//Can a counter report something different to 0 if no write and no event
//is active? No
//assume property (slv_index!=9 && events_i[8]==0);
//assume we are not in selftest
//assume property (slv_index==0 |-> hwdata_i[31:30] == 0);
//assert property (slv_reg[9]==0);
`endif
endmodule
......
submodules/counters/PMU_counters.sv
View file @ cd0b202c
......@@ -45,11 +45,12 @@ module PMU_counters #
// Input/output wire from registers of the wrapper to PMU_raw internal
// registers
input wire [REG_WIDTH-1:0] regs_i [0:N_COUNTERS-1],
output wire [REG_WIDTH-1:0] regs_o [0:N_COUNTERS-1],
output logic [REG_WIDTH-1:0] regs_o [0:N_COUNTERS-1],
//external signals from Soc events
input wire [N_COUNTERS-1:0] events_i
);
reg [REG_WIDTH-1:0] slv_reg [0:N_COUNTERS-1] /*verilator public*/;
wire [REG_WIDTH-1:0] adder_out [0:N_COUNTERS-1] /*verilator public*/;
//-------------Adders with reset
//Inside the generate loop it creates as many counters as the parameter
//N_COUNTERS. For each of them one slv_reg is assigned. When a soft reset
......@@ -60,6 +61,7 @@ module PMU_counters #
genvar i;
generate
for (i=0; i<N_COUNTERS; i=i+1) begin
assign adder_out[i] = (events_i[i] & en_i)? slv_reg[i]+1:slv_reg[i];
always @(posedge clk_i, negedge rstn_i) begin
if(rstn_i == 1'b0 ) begin
slv_reg[i] <='{default:0};
......@@ -70,17 +72,22 @@ module PMU_counters #
if (we_i) begin
slv_reg[i] <= regs_i[i];
end else begin
if(events_i[i] & en_i) begin
slv_reg[i] <= slv_reg[i]+1;
end
slv_reg[i] <= adder_out[i];
end
end
end
end
end
endgenerate
//Map input and output registers
assign regs_o = slv_reg;
//Map input and output registers. If no write is active pass the internal
//value, otherwise bypass the write and show the most recent value
always_comb begin
if(we_i) begin
regs_o = regs_i;
end else begin
regs_o = adder_out;
end
end
//TODO: fill formal propperties
///////////////////////////////////////////////////////////////////////////////
//
......@@ -124,33 +131,9 @@ module PMU_counters #
cover property (!we_i[*5] |-> slv_reg[6]==32'hffffffff);
cover property (!we_i[*5] |-> slv_reg[7]==32'hffffffff);
cover property (!we_i[*5] |-> slv_reg[8]==32'hffffffff);
//Cover increase of the counters without external writes
always_ff @(posedge clk_i) begin
/*
cover(
(
(
($past(slv_reg,5) == '{default:0})) &&
//($past(we_i,5) === 0) &&
//($past(we_i,4) == 0) &&
//($past(we_i,3) == 0) &&
//($past(we_i,2) == 0) &&
//($past(we_i,1) == 0) &&
(slv_reg[0] == 32'h00000003)&&
(slv_reg[1] == 32'h00000003)&&
(slv_reg[2] == 32'h00000003)&&
(slv_reg[3] == 32'h00000003)&&
(slv_reg[4] == 32'h00000003)&&
(slv_reg[5] == 32'h00000003)&&
(slv_reg[6] == 32'h00000003)&&
(slv_reg[7] == 32'h00000003)&&
(slv_reg[8] == 32'h00000003)
)
);
*/
end
//TODO:Write assertions for:
// After a write register must be update and remain stable if no more
// writes or events happen
`endif
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment