From 6bdedf4f8673568f9da4a511ce7cc4049df9f7ba Mon Sep 17 00:00:00 2001 From: Daniel Jobson Date: Fri, 22 Nov 2024 13:03:45 +0100 Subject: [PATCH] Fix bug in timer core, where it misses clock cycles Remove redundant timer state. This fixes a bug where the timer misses a clock cycle every time the prescaler counter reaches 1. This means if one uses a large prescaler, like 18E6, it is barely noticeable, but if one have a low prescaler and a high timer value it becomes significant. This also yields the running_* registers redundant, which are removed. Add clarity to the readme. Update the timer to default to values of one, for prescaler and timer count. --- .../application_fpga.bin.sha256 | 2 +- hw/application_fpga/core/timer/README.md | 32 +++++-- .../core/timer/rtl/timer_core.v | 83 ++++--------------- .../core/timer/tb/tb_timer_core.v | 3 + 4 files changed, 45 insertions(+), 75 deletions(-) diff --git a/hw/application_fpga/application_fpga.bin.sha256 b/hw/application_fpga/application_fpga.bin.sha256 index cb83b36..6b3aa34 100644 --- a/hw/application_fpga/application_fpga.bin.sha256 +++ b/hw/application_fpga/application_fpga.bin.sha256 @@ -1 +1 @@ -d5e4153187d1808b77535c9233a28073de7eeed4ead373efa9d7dd835833b03a application_fpga.bin +2a3928e8587c8d5d7eca6048c6448dc1d16d52a9a25d3ca5026bec3e7fb14ef3 application_fpga.bin diff --git a/hw/application_fpga/core/timer/README.md b/hw/application_fpga/core/timer/README.md index 826e75d..76299e6 100644 --- a/hw/application_fpga/core/timer/README.md +++ b/hw/application_fpga/core/timer/README.md @@ -3,9 +3,11 @@ A simple timer with prescaler. ## Introduction This core implements a simple timer with a prescaler. The prescaler -allows measurement of time durations rather than cycles. If for -example setting the prescaler to the clock frequency in Hertz, the -timer will count seconds. +can be used to divide the system clock frequency to yield a specific +amount of clock frequency ticks for each timer tick. This allows +measurement of time rather than cycles. If for example setting the +prescaler to the clock frequency in Hertz, the timer will count +seconds. ## API @@ -27,11 +29,23 @@ The following addresses define the API for the timer: ## Details The core consists of the timer_core module (in timer_core.v) and a top level wrapper, timer (in timer.v). The top level wrapper implements -the API, while the timer_core implements the actual timer -functionality. +the API, while the timer_core implements the timer functionality. + +The timer counter and the prescaler counter are both 32 bits. Both the +`prescaler` and the `timer` register can be set through the API above, +and should be a non-zero value, the defaults are one. These values +cannot be changed when the timer is running. + +Bit zero of the `status` register shows if the timer is running, one +indicates a running timer. + +Start the timer by writing 1 to bit zero of the `ctrl` register, write +1 to bit one to stop it. + +Time elapsed can be calculated using: + +``` +time = clock_freq / (prescaler * timer) +``` -The timer counter and the prescaler counter are both 32 bits. -When enabled the counter counts down one integer value per cycle. -The timer will stop when reaching final zero (given by prescaler times the initial value of the timer) -and the running flag will be lowered. diff --git a/hw/application_fpga/core/timer/rtl/timer_core.v b/hw/application_fpga/core/timer/rtl/timer_core.v index 468bf02..e4dc4c2 100644 --- a/hw/application_fpga/core/timer/rtl/timer_core.v +++ b/hw/application_fpga/core/timer/rtl/timer_core.v @@ -30,18 +30,13 @@ module timer_core ( //---------------------------------------------------------------- // Internal constant and parameter definitions. //---------------------------------------------------------------- - localparam CTRL_IDLE = 2'h0; - localparam CTRL_PRESCALER = 2'h1; - localparam CTRL_TIMER = 2'h2; + localparam CTRL_IDLE = 1'h0; + localparam CTRL_RUNNING = 1'h1; //---------------------------------------------------------------- // Registers including update variables and write enable. //---------------------------------------------------------------- - reg running_reg; - reg running_new; - reg running_we; - reg [31 : 0] prescaler_reg; reg [31 : 0] prescaler_new; reg prescaler_we; @@ -54,8 +49,8 @@ module timer_core ( reg timer_set; reg timer_dec; - reg [ 1 : 0] core_ctrl_reg; - reg [ 1 : 0] core_ctrl_new; + reg core_ctrl_reg; + reg core_ctrl_new; reg core_ctrl_we; @@ -63,7 +58,7 @@ module timer_core ( // Concurrent connectivity for ports etc. //---------------------------------------------------------------- assign curr_timer = timer_reg; - assign running = running_reg; + assign running = core_ctrl_reg; //---------------------------------------------------------------- @@ -71,16 +66,11 @@ module timer_core ( //---------------------------------------------------------------- always @(posedge clk) begin : reg_update if (!reset_n) begin - running_reg <= 1'h0; prescaler_reg <= 32'h0; timer_reg <= 32'h0; core_ctrl_reg <= CTRL_IDLE; end else begin - if (running_we) begin - running_reg <= running_new; - end - if (prescaler_we) begin prescaler_reg <= prescaler_new; end @@ -136,8 +126,6 @@ module timer_core ( // Core control FSM. //---------------------------------------------------------------- always @* begin : core_ctrl - running_new = 1'h0; - running_we = 1'h0; prescaler_set = 1'h0; prescaler_dec = 1'h0; timer_set = 1'h0; @@ -148,69 +136,34 @@ module timer_core ( case (core_ctrl_reg) CTRL_IDLE: begin if (start) begin - running_new = 1'h1; - running_we = 1'h1; prescaler_set = 1'h1; timer_set = 1'h1; - if (prescaler_init == 0) begin - core_ctrl_new = CTRL_TIMER; - core_ctrl_we = 1'h1; - end - - else begin - core_ctrl_new = CTRL_PRESCALER; - core_ctrl_we = 1'h1; - end + core_ctrl_new = CTRL_RUNNING; + core_ctrl_we = 1'h1; end end - CTRL_PRESCALER: begin + CTRL_RUNNING: begin if (stop) begin - running_new = 1'h0; - running_we = 1'h1; core_ctrl_new = CTRL_IDLE; core_ctrl_we = 1'h1; end else begin - if (prescaler_reg == 1) begin - core_ctrl_new = CTRL_TIMER; - core_ctrl_we = 1'h1; - end - - else begin - prescaler_dec = 1'h1; - end - end - end - - - CTRL_TIMER: begin - if (stop) begin - running_new = 1'h0; - running_we = 1'h1; - core_ctrl_new = CTRL_IDLE; - core_ctrl_we = 1'h1; - end - - else begin - if (timer_reg == 1) begin - running_new = 1'h0; - running_we = 1'h1; - core_ctrl_new = CTRL_IDLE; - core_ctrl_we = 1'h1; - end - - else begin - timer_dec = 1'h1; - - if (prescaler_init > 0) begin - prescaler_set = 1'h1; - core_ctrl_new = CTRL_PRESCALER; + if (prescaler_reg[31 : 1] == 0) begin // Check prescaler_reg <= 1 + if (timer_reg[31 : 1] == 0) begin // Check timer_reg <= 1 + core_ctrl_new = CTRL_IDLE; core_ctrl_we = 1'h1; end + else begin + prescaler_set = 1'h1; + timer_dec = 1'h1; + end + end + else begin + prescaler_dec = 1'h1; end end end diff --git a/hw/application_fpga/core/timer/tb/tb_timer_core.v b/hw/application_fpga/core/timer/tb/tb_timer_core.v index cdb65c8..7d0203e 100644 --- a/hw/application_fpga/core/timer/tb/tb_timer_core.v +++ b/hw/application_fpga/core/timer/tb/tb_timer_core.v @@ -182,6 +182,9 @@ module tb_timer_core (); //---------------------------------------------------------------- // test1() + // + // Set prescaler and timer and count until the timer returns expired. + // Check so the clock cycles passed adds up to timer * prescaler. //---------------------------------------------------------------- task test1; begin