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.

hw/application_fpga/application_fpga.bin.sha256

@@ -1 +1 @@
-d5e4153187d1808b77535c9233a28073de7eeed4ead373efa9d7dd835833b03a  application_fpga.bin
+2a3928e8587c8d5d7eca6048c6448dc1d16d52a9a25d3ca5026bec3e7fb14ef3  application_fpga.bin

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
+can be used to divide the system clock frequency to yield a specific
-example setting the prescaler to the clock frequency in Hertz, the
+amount of clock frequency ticks for each timer tick. This allows
-timer will count seconds.
+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
+the API, while the timer_core implements the timer functionality.
-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.

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_IDLE = 1'h0;
-  localparam CTRL_PRESCALER = 2'h1;
+  localparam CTRL_RUNNING = 1'h1;
-  localparam CTRL_TIMER = 2'h2;
 
 
   //----------------------------------------------------------------
   // 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          core_ctrl_reg;
-  reg [ 1 : 0] core_ctrl_new;
+  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,73 +136,38 @@ 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_RUNNING;
-            core_ctrl_new = CTRL_TIMER;
-            core_ctrl_we  = 1'h1;
-          end
-
-          else begin
-            core_ctrl_new = CTRL_PRESCALER;
           core_ctrl_we  = 1'h1;
         end
       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
+          if (prescaler_reg[31 : 1] == 0) begin  // Check prescaler_reg <= 1
-            core_ctrl_new = CTRL_TIMER;
+            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
 
-
-      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;
-              core_ctrl_we  = 1'h1;
-            end
-          end
-        end
-      end
-
       default: begin
       end
     endcase  // case (core_ctrl_reg)

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