refactor control loop interface

README.md

@@ -17,5 +17,8 @@ Read `doc/copying/docker.md` to set up the Docker build environment.
 * `doc/`: Documentation.
 * `doc/copying`: Licenses.
 * `gateware/`: FPGA source.
+* `gateware/rtl`: Verilog sources.
+* `gateware/rtl/control_loop`: Control loop code.
+* `gateware/rtl/spi`: SPI code.
 * `linux/`: Software that runs on the controller.
 * `opensbi/`: OpenSBI configuration files and source fragments.

doc/controller_manual.md

@@ -7,9 +7,7 @@ source distribution.
 __________________________________________________________________________
 
 This manual describes the controller software programming. This guide does not
-describe client programming (programs that run on the client and interface with
+describe Verilog: see `verilog_manual.md` for that.
-the controller). It does not describe Verilog: see `verilog_manual.md` for
-that.
 
 # Preqreuisites
 

gateware/mmio_descr.py

@@ -0,0 +1,220 @@
+import textwrap
+
+class Descr:
+    def __init__(self, name, blen, rwperm, num, descr):
+        """
+        :param name: Name of the pin without numerical suffix.
+        :param blen: Bit length of the pin.
+        :param doc: Restructured text documentation of the register.
+        :param num: The amount of registers of the same type.
+        :param read_only: A string that must be either "read-only" or "write-write".
+        """
+        self.name = name
+        self.blen = blen
+        self.doc = textwrap.deindent(descr)
+        self.num =num
+        self.read_only = read_only == "read-only"
+
+    @classmethod
+    def from_dict(cls, jsdict, name):
+        return cls(name, jsdict[name]["len"], jsdict[name]["ro"], jsdict[name]["num"], jsdict[name]["doc"])
+    def store_to_dict(self, d):
+        d[self.name = {
+                "len": self.blen,
+                "doc": self.doc,
+                "num": self.num,
+                "ro": ro
+                }
+
+registers = [
+        Descr("adc_sel", 3, "read-write", """\
+                Select which on-FPGA SPI master controls the ADC.
+
+                Valid settings:
+
+                * ``0``: ADC is controlled by MMIO registers.
+                * ``0b10``: ADC is controlled by MMIO registers, but conversion is
+                   disabled. This is used to flush output from an out-of-sync ADC.
+                * ``0b100``: ADC 0 only. ADC is controlled by control loop."""),
+        Descr("adc_finished", "read-only", """\
+                Signals that an ADC master has finished an SPI cycle.
+
+                Values:
+
+                * ``0``: MMIO master is either not armed or currently in a
+                  SPI transfer.
+                * ``1``: MMIO master has finished.
+
+                This flag is on only when ``adc_arm`` is high. The flag does not
+                mean that data has been received successfully, only that the master
+                has finished it's SPI transfer.
+                """),
+        Descr("adc_arm", "read-write", """\
+                Start a DAC master SPI transfer.
+
+                If ``adc_arm`` is raised from and the master is currently not in a SPI
+                transfer, the SPI master will start an SPI transfer and write data
+                into ``adc_recv_buf``.
+
+                If ``adc_arm`` is raised while the master is in an SPI transfer,
+                nothing changes.
+
+                If ``adc_arm`` is lowered while the master is in an SPI transfer,
+                nothing changes. The SPI cycle will continue to execute and it will
+                continue to write data to ``adc_recv_buf``.
+
+                If the SPI transfer finishes and ``adc_arm`` is still set to
+                1, then ``adc_finished`` is raised to 1. If ``adc_arm`` is lowered
+                in this state, then ``adc_finished`` is lowered.
+
+                Linear Technologies ADCs must not have their SPI transfers
+                interrupted. The transfer can be interrupted by
+
+                1. Interrupt the signal physically (i.e. pulling out cable connecting
+                   the FPGA to the ADC)
+                2. Reset of the ADC master
+                3. Reset of the FPGA
+                4. Switching ``adc_sel`` to the control loop
+
+                If the ADC is interrupted then it will be in an unknown transfer
+                state. To recover from an unknown transfer state, set ``adc_sel``
+                to ``0b10`` and run a SPI transfer cycle. This will run the SPI
+                clock and flush the ADC buffer. The only other way is to power-cycle
+                the ADC.
+
+                If ``adc_sel`` is not set to 0 then the transfer will proceed
+                as normal, but no data will be received from the ADC."""),
+        Descr("adc_recv_buf", "read-only", """\
+                ADC Master receive buffer.
+
+                This buffer is stable if there is no ADC transfer caused by ``adc_arm``
+                is in process.
+
+                This register only changes if an SPI transfer is triggered by the MMIO
+                registers. SPI transfers by other masters will not affect this register.
+                buffer."""),
+
+        Descr("dac_sel", 2, "read-write", """\
+                Select which on-FPGA SPI master controls the DAC.
+
+                Valid settings:
+
+                * ``0``: DAC is controlled by MMIO registers.
+                * ``0b10``: DAC 0 only. DAC is controlled by control loop."""),
+        Descr("dac_finished", 1, "read-only", """\
+                Signals that the DAC master has finished transmitting data.
+
+                Values:
+
+                * ``0``: MMIO master is either not armed or currently in a
+                  SPI transfer.
+                * ``1``: MMIO master has finished transmitting.
+
+                This flag is on only when ``dac_arm`` is high. The flag does not
+                mean that data has been received or transmitted successfully, only that
+                the master has finished it's SPI transfer."""),
+        Descr("dac_arm", 1, "read-write", """\
+                Start a DAC master SPI transfer.
+
+                If ``dac_arm`` is raised from and the master is currently not in a SPI
+                transfer, the SPI master reads from the ``dac_send_buf`` register and sends
+                it over the wire to the DAC, while reading data from the DAC into
+                ``dac_recv_buf``.
+
+                If ``dac_arm`` is raised while the master is in an SPI transfer,
+                nothing changes.
+
+                If ``dac_arm`` is lowered while the master is in an SPI transfer,
+                nothing changes. The SPI cycle will continue to execute and it will
+                continue to write data to ``dac_recv_buf``.
+
+                If the SPI transfer finishes and ``dac_arm`` is still set to
+                1, then ``dac_finished`` is raised to 1. If ``dac_arm`` is lowered
+                in this state, then ``dac_finished`` is lowered.
+
+                Analog Devices DACs can have their SPI transfers interrupted without
+                issue. However it is currently not possible to interrupt SPI transfers
+                in software without resetting the entire device.
+
+                If ``dac_sel`` is set to another master then the transfer will proceed
+                as normal, but no data will be sent to or received from the DAC."""),
+        Descr("dac_recv_buf", 24, "read-only", """\
+                DAC master receive buffer.
+
+                This buffer is stable if there is no DAC transfer caused by ``dac_arm``
+                is in process.
+
+                This register only changes if an SPI transfer is triggered by the MMIO
+                registers. SPI transfers by other masters will not affect this register.
+                buffer."""),
+        Descr("dac_send_buf, 24, "read-write", """\
+                DAC master send buffer.
+
+                Fill this buffer with a 24 bit Analog Devices DAC command. Updating
+                this buffer does not start an SPI transfer. To send data to the DAC,
+                fill this buffer and raise ``dac_arm``.
+
+                The DAC copies this buffer into an internal register when writing data.
+                Modifying this buffer during a transfer does not disrupt an in-process
+                transfer."""),
+
+        Descr("cl_assert_change", 1, "read-write", """\
+                Flush parameter changes to control loop.
+
+                When this bit is raised from low to high, this signals the control
+                loop that it should read in new values from the MMIO registers.
+                While the bit is raised high, the control loop will read the constants
+                at most once.
+
+                When this bit is raised from high to low before ``cl_change_made``
+                is asserted by the control loop, nothing happens."""),
+        Descr("cl_change_made", 1, "read-only", """\
+                Signal from the control loop that the parameters have been applied.
+
+                This signal goes high only while ``cl_assert_change`` is high. No
+                change will be applied afterwards while both are high."""),
+
+        Descr("cl_in_loop_in", 1, "read-only", """\
+                This bit is high if the control loop is running."""),
+        Descr("cl_setpt_in", 18, "read-write", """\
+                Setpoint of the control loop.
+
+                This is a twos-complement number in ADC units.
+
+                This is a parameter: see ``cl_assert_change``."""),
+        Descr("cl_P_in", 64, "read-write", """\
+                Proportional parameter of the control loop.
+
+                This is a twos-complement fixed point number with 21 whole
+                bits and 43 fractional bits. This is applied to the error
+                in DAC units.
+
+                This is a parameter: see ``cl_assert_change``."""),
+        Descr("cl_I_in", 64, "read-write", """\
+                Integral parameter of the control loop.
+
+                This is a twos-complement fixed point number with 21 whole
+                bits and 43 fractional bits. This is applied to the error
+                in DAC units.
+
+                This is a parameter: see ``cl_assert_change``."""),
+        Descr("cl_delay_in", 16, "read-write", """\
+                Delay parameter of the loop.
+
+                This is an unsigned number denoting the number of cycles
+                the loop should wait between loop executions.
+
+                This is a parameter: see ``cl_assert_change``."""),
+
+        Descr("cl_cycle_count", 18, "read-only", """\
+                Delay parameter of the loop.
+
+                This is an unsigned number denoting the number of cycles
+                the loop should wait between loop executions."""),
+        Descr("cl_z_pos", 20, "read-only", """\
+                Control loop DAC Z position.
+                """),
+        Descr("cl_z_measured", 18, "read-only", """\
+                Control loop ADC Z position.
+                """),
+        ]

gateware/rtl/base/base.v.m4

@@ -1,7 +1,6 @@
 m4_changequote(`⟨', `⟩')
 m4_changecom(⟨/*⟩, ⟨*/⟩)
 m4_define(generate_macro, ⟨m4_define(M4_$1, $2)⟩)
-m4_include(../control_loop/control_loop_cmds.m4)
 /*
 Copyright 2023 (C) Peter McGoron
 
@@ -291,14 +290,18 @@ m4_define(CL_DATA_WID, CL_CONSTS_WID)
 	m4_adc_wires(ADC_TYPE3_WID, 6, ADC_PORTS),
 	m4_adc_wires(ADC_TYPE3_WID, 7, ADC_PORTS),
 
-	output cl_in_loop,
+	input cl_assert_change,
-	input [M4_CONTROL_LOOP_CMD_WIDTH-1:0] cl_cmd,
+	output cl_change_made,
-	input [CL_DATA_WID-1:0] cl_word_in,
-	output reg [CL_DATA_WID-1:0] cl_word_out,
-	input cl_start_cmd,
-	output cl_finish_cmd,
 
-	output [DAC_DATA_WID-1:0] cl_z_report
+	input cl_run_loop_in,
+	input [ADC_TYPE1_WID-1:0] cl_setpt_in,
+	input [CL_DATA_WID-1:0] cl_P_in,
+	input [CL_DATA_WID-1:0] cl_I_in,
+	input [CL_DELAY_WID-1:0] cl_delay_in,
+
+	output [CYCLE_COUNT_WID-1:0] cl_cycle_count,
+	output [DAC_DATA_WID-1:0] cl_z_pos,
+	output [ADC_WID-1:0] cl_z_measured
 );
 
 assign set_low = 0;
@@ -377,12 +380,16 @@ control_loop #(
 	.adc_miso(adc_sdo_port_0[2]),
 	.adc_conv_L(adc_conv_L_port_0[2]),
 	.adc_sck(adc_sck_port_0[2]),
-	.cmd(cl_cmd),
+	.assert_change(cl_assert_change),
-	.word_in(cl_word_in),
+	.change_made(cl_change_made),
-	.word_out(cl_word_out),
+	.run_loop_in(cl_run_loop_in),
-	.start_cmd(cl_start_cmd),
+	.setpt_in(cl_setpt_in),
-	.finish_cmd(cl_finish_cmd),
+	.P_in(cl_P_in),
-	.z_report(cl_z_report)
+	.I_in(cl_I_in),
+	.delay_in(cl_delay_in),
+	.cycle_count(cl_cycle_count),
+	.z_pos(cl_z_pos),
+	.z_measured(cl_z_measured)
 );
 
 endmodule

gateware/rtl/control_loop/Makefile

@@ -38,7 +38,7 @@ obj_dir/Vcontrol_loop_sim_top.mk: control_loop_sim.cpp ${COMMON} \
                           ../spi/spi_master_ss.v \
                           ../spi/spi_slave_no_write.v \
                           control_loop_sim_top.v control_loop_sim_top.v \
-                          control_loop_cmds.vh control_loop.v \
+                          control_loop.v \
                           ${control_loop_math_verilog}
 	verilator --cc --exe -Wall --trace --trace-fst \
 		--top-module control_loop_sim_top \
@@ -48,19 +48,13 @@ obj_dir/Vcontrol_loop_sim_top.mk: control_loop_sim.cpp ${COMMON} \
 		control_loop_sim_top.v control_loop.v control_loop_sim.cpp \
 		${COMMON_CPP} adc_sim.v dac_sim.v ../spi/spi_master_ss.v \
 		../spi/spi_slave_no_read.v ../spi/spi_slave.v
-obj_dir/Vcontrol_loop_sim_top: obj_dir/Vcontrol_loop_sim_top.mk control_loop_cmds.h
+obj_dir/Vcontrol_loop_sim_top: obj_dir/Vcontrol_loop_sim_top.mk
 	cd obj_dir && make -f Vcontrol_loop_sim_top.mk
 
 ####### Codegen ########
 
 include ../common.makefile
-CODEGEN_FILES=control_loop_cmds.h boothmul_preprocessed.v control_loop_math.v control_loop.v control_loop_cmds.vh
+CODEGEN_FILES=boothmul_preprocessed.v control_loop_math.v control_loop.v
 codegen: ${CODEGEN_FILES}
-control_loop_cmds.vh: control_loop_cmds.m4
-	m4 -P control_loop_cmds.vh.m4  > control_loop_cmds.vh
-control_loop_cmds.h: control_loop_cmds.m4
-	echo '#pragma once' > control_loop_cmds.h
-	m4 -P control_loop_cmds.h.m4 >> control_loop_cmds.h
-
 clean:
 	rm -rf obj_dir *.fst ${CODEGEN_FILES}

gateware/rtl/control_loop/control_loop.v.m4

@@ -27,7 +27,6 @@ module control_loop
 	parameter CONSTS_SIZ = 7,
 m4_define(M4_CONSTS_WID, (CONSTS_WHOLE + CONSTS_FRAC))
 	parameter DELAY_WID = 16,
-m4_define(M4_DATA_WID, M4_CONSTS_WID)
 	parameter READ_DAC_DELAY = 5,
 	parameter CYCLE_COUNT_WID = 18,
 	parameter DAC_WID = 24,
@@ -47,7 +46,6 @@ m4_define(M4_E_WID, (DAC_DATA_WID + 1))
 ) (
 	input clk,
 	input rst_L,
-	output in_loop,
 
 	output dac_mosi,
 	input dac_miso,
@@ -58,14 +56,20 @@ m4_define(M4_E_WID, (DAC_DATA_WID + 1))
 	output adc_conv_L,
 	output adc_sck,
 
-	/* Hacky ad-hoc read-write interface. */
+	output in_loop,
-	input [M4_CONTROL_LOOP_CMD_WIDTH-1:0] cmd,
-	input [M4_DATA_WID-1:0] word_in,
-	output reg [M4_DATA_WID-1:0] word_out,
-	input start_cmd,
-	output reg finish_cmd,
 
-	output [DAC_DATA_WID-1:0] z_report
+	input assert_change,
+	output reg change_made,
+
+	input run_loop_in,
+	input [ADC_WID-1:0] setpt_in,
+	input [M4_CONSTS_WID-1:0] P_in,
+	input [M4_CONSTS_WID-1:0] I_in,
+	input [DELAY_WID-1:0] delay_in,
+
+	output [CYCLE_COUNT_WID-1:0] cycle_count,
+	output [DAC_DATA_WID-1:0] z_pos,
+	output [ADC_WID-1:0] z_measured
 );
 
 /************ ADC and DAC modules ***************/
@@ -104,6 +108,7 @@ spi_master_ss #(
 reg adc_arm;
 reg adc_finished;
 wire [ADC_WID-1:0] measured_value;
+assign z_measured = measured_value;
 wire adc_ready_to_arm_unused;
 
 localparam [3-1:0] DAC_REGISTER = 3'b001;
@@ -133,33 +138,30 @@ spi_master_ss_no_write #(
  * Parameters can be adjusted on the fly by the user. The modifications
  * cannot happen during a calculation, but calculations occur in a matter
  * of milliseconds. Instead, modifications are checked and applied at the
- * start of each iteration (CYCLE_START). Before this, the new values
+ * start of each iteration (CYCLE_START).
- * have to be buffered.
  */
 
 /* Setpoint: what should the ADC read */
 reg signed [ADC_WID-1:0] setpt = 0;
-reg signed [ADC_WID-1:0] setpt_buffer = 0;
 
 /* Integral parameter */
 reg signed [M4_CONSTS_WID-1:0] cl_I_reg = 0;
-reg signed [M4_CONSTS_WID-1:0] cl_I_reg_buffer = 0;
 
 /* Proportional parameter */
 reg signed [M4_CONSTS_WID-1:0] cl_p_reg = 0;
-reg signed [M4_CONSTS_WID-1:0] cl_p_reg_buffer = 0;
 
 /* Delay parameter (to make the loop run slower) */
 reg [DELAY_WID-1:0] dely = 0;
-reg [DELAY_WID-1:0] dely_buffer = 0;
 
 /************ Loop Control and Internal Parameters *************/
 
 reg running = 0;
 
 reg signed [DAC_DATA_WID-1:0] stored_dac_val = 0;
-assign z_report = stored_dac_val;
+assign z_pos = stored_dac_val;
+
 reg [CYCLE_COUNT_WID-1:0] last_timer = 0;
+assign cycle_count = last_timer;
 reg [CYCLE_COUNT_WID-1:0] counting_timer = 0;
 reg [M4_CONSTS_WID-1:0] adjval_prev = 0;
 
@@ -214,12 +216,6 @@ control_loop_math #(
  * ┃       ↓
  * ┗━━━━━━━┛
  ****** Outline
- * There are two systems: the read-write interface and the loop.
- * The read-write interface allows another module (i.e. the CPU)
- * to access and change constants. When a constant is changed the
- * loop must reset the values that are preserved between loops
- * (previous adjustment and previous delay).
- *
  * When the loop starts it must find the current value from the
  * DAC and write to it. The value from the DAC is then adjusted
  * with the output of the control loop. Afterwards it does not
@@ -253,106 +249,21 @@ always @ (posedge clk) begin
 	end
 end
 
-/**** Read-Write control interface.
- * `write_control` ensures that writes to the dirty bit do not happen when
- * the main loop is clearing the dirty bit.
- */
-
-wire write_control = state == WAIT_ON_DAC || !running;
-reg dirty_bit = 0;
-
-always @ (posedge clk) begin
-	if (!rst_L) begin
-		dirty_bit <= 0;
-		finish_cmd <= 0;
-		word_out <= 0;
-	end else if (start_cmd && !finish_cmd) begin
-		case (cmd)
-		M4_CONTROL_LOOP_NOOP:
-			finish_cmd <= 1;
-		M4_CONTROL_LOOP_NOOP | M4_CONTROL_LOOP_WRITE_BIT:
-			finish_cmd <= 1;
-		M4_CONTROL_LOOP_STATUS: begin
-			word_out[M4_DATA_WID-1:1] <= 0;
-			word_out[0] <= running;
-			finish_cmd <= 1;
-		end
-		M4_CONTROL_LOOP_STATUS | M4_CONTROL_LOOP_WRITE_BIT:
-			if (write_control) begin
-				running <= word_in[0];
-				finish_cmd <= 1;
-				dirty_bit <= 1;
-			end
-		M4_CONTROL_LOOP_SETPT: begin
-			word_out[M4_DATA_WID-1:ADC_WID] <= 0;
-			word_out[ADC_WID-1:0] <= setpt;
-			finish_cmd <= 1;
-		end
-		M4_CONTROL_LOOP_SETPT | M4_CONTROL_LOOP_WRITE_BIT:
-			if (write_control) begin
-				setpt_buffer <= word_in[ADC_WID-1:0];
-				finish_cmd <= 1;
-				dirty_bit <= 1;
-			end
-		M4_CONTROL_LOOP_P: begin
-			word_out <= cl_p_reg;
-			finish_cmd <= 1;
-		end
-		M4_CONTROL_LOOP_P | M4_CONTROL_LOOP_WRITE_BIT: begin
-			if (write_control) begin
-				cl_p_reg_buffer <= word_in;
-				finish_cmd <= 1;
-				dirty_bit <= 1;
-			end
-		end
-		M4_CONTROL_LOOP_I: begin
-			word_out <= cl_I_reg;
-			finish_cmd <= 1;
-		end
-		M4_CONTROL_LOOP_I | M4_CONTROL_LOOP_WRITE_BIT: begin
-			if (write_control) begin
-				cl_I_reg_buffer <= word_in;
-				finish_cmd <= 1;
-				dirty_bit <= 1;
-			end
-		end
-		M4_CONTROL_LOOP_DELAY: begin
-			word_out[M4_DATA_WID-1:DELAY_WID] <= 0;
-			word_out[DELAY_WID-1:0] <= dely;
-			finish_cmd <= 1;
-		end
-		M4_CONTROL_LOOP_DELAY | M4_CONTROL_LOOP_WRITE_BIT: begin
-			if (write_control) begin
-				dely_buffer <= word_in[DELAY_WID-1:0];
-				finish_cmd <= 1;
-				dirty_bit <= 1;
-			end
-		end
-		M4_CONTROL_LOOP_ERR: begin
-			word_out[M4_DATA_WID-1:M4_E_WID] <= 0;
-			word_out[M4_E_WID-1:0] <= err_prev;
-			finish_cmd <= 1;
-		end
-		M4_CONTROL_LOOP_Z: begin
-			word_out[M4_DATA_WID-1:DAC_DATA_WID] <= 0;
-			word_out[DAC_DATA_WID-1:0] <= stored_dac_val;
-			finish_cmd <= 1;
-		end
-		M4_CONTROL_LOOP_CYCLES: begin
-			word_out[M4_DATA_WID-1:CYCLE_COUNT_WID] <= 0;
-			word_out[CYCLE_COUNT_WID-1:0] <= last_timer;
-			finish_cmd <= 0;
-		end
-		endcase
-	end else if (!start_cmd) begin
-		finish_cmd <= 0;
-	end
-end
-
 assign in_loop = state != INIT_READ_FROM_DAC || running;
 
+/* Reset the change acknowledge interface after the master
+ * stops its transfer.
+ *
+ * The module only writes to change_made in the main always block
+ * when state == CYCLE_START, so make sure that this does not
+ * compete with CYCLE_START.
+ */
 always @ (posedge clk) begin
-	if (!rst_L) begin
+	if (state != CYCLE_START && !assert_change && change_made)
+		change_made <= 0;
+end
+
+task reset_loop();
 	to_dac <= 0;
 	dac_arm <= 0;
 	state <= INIT_READ_FROM_DAC;
@@ -366,12 +277,20 @@ always @ (posedge clk) begin
 
 	adc_arm <= 0;
 	arm_math <= 0;
+endtask
+
+always @ (posedge clk) begin
+	if (!rst_L) begin
+		reset_loop();
 	end else case (state)
 	INIT_READ_FROM_DAC: begin
-		if (running) begin
+		if (run_loop_in) begin
+			running <= 1;
 			to_dac <= {1'b1, DAC_REGISTER, 20'b0};
 			dac_arm <= 1;
 			state <= WAIT_FOR_DAC_READ;
+		end else begin
+			reset_loop();
 		end
 	end
 	WAIT_FOR_DAC_READ: begin
@@ -396,21 +315,21 @@ always @ (posedge clk) begin
 		end
 	end
 	CYCLE_START: begin
-		if (!running) begin
+		if (!run_loop_in) begin
-			state <= INIT_READ_FROM_DAC;
+			reset_loop();
 		end else if (timer < dely) begin
 			timer <= timer + 1;
 		end else begin
 			/* On change of constants, previous values are invalidated. */
-			if (dirty_bit) begin
+			if (assert_change && !change_made) begin
-				setpt <= setpt_buffer;
+				change_made <= 1;
-				dely <= dely_buffer;
+
-				cl_I_reg <= cl_I_reg_buffer;
+				setpt <= setpt_in;
-				cl_p_reg <= cl_p_reg_buffer;
+				dely <= delay_in;
+				cl_I_reg <= I_in;
+				cl_p_reg <= P_in;
 				adjval_prev <= 0;
 				err_prev <= 0;
-
-				dirty_bit <= 0;
 			end
 
 			state <= WAIT_ON_ADC;

gateware/rtl/control_loop/control_loop_cmds.h.m4

@@ -1,7 +0,0 @@
-m4_changequote(`⟨', `⟩')m4_dnl
-m4_changecom(⟨/*⟩, ⟨*/⟩)m4_dnl
-m4_define(generate_macro, ⟨m4_dnl
-#define $1 $2 m4_dnl
-m4_define(M4_$1, $2)m4_dnl
-⟩)m4_dnl
-m4_include(control_loop_cmds.m4)

gateware/rtl/control_loop/control_loop_cmds.m4

@@ -1,11 +0,0 @@
-generate_macro(CONTROL_LOOP_NOOP, 0)
-generate_macro(CONTROL_LOOP_STATUS, 1)
-generate_macro(CONTROL_LOOP_SETPT, 2)
-generate_macro(CONTROL_LOOP_P, 3)
-generate_macro(CONTROL_LOOP_I, 4)
-generate_macro(CONTROL_LOOP_ERR, 5)
-generate_macro(CONTROL_LOOP_Z, 6)
-generate_macro(CONTROL_LOOP_CYCLES, 7)
-generate_macro(CONTROL_LOOP_DELAY, 8)
-generate_macro(CONTROL_LOOP_CMD_WIDTH, 8)
-generate_macro(CONTROL_LOOP_WRITE_BIT, (1 << (M4_CONTROL_LOOP_CMD_WIDTH-1)))

gateware/rtl/control_loop/control_loop_cmds.vh.m4

@@ -1,7 +0,0 @@
-m4_changequote(`⟨', `⟩')m4_dnl
-m4_changecom(⟨/*⟩, ⟨*/⟩)m4_dnl
-m4_define(generate_macro, ⟨m4_dnl
-`define $1 $2 m4_dnl
-m4_define(M4_$1, $2)m4_dnl
-⟩)m4_dnl
-m4_include(control_loop_cmds.m4)

gateware/rtl/control_loop/control_loop_sim.cpp

@@ -14,7 +14,6 @@
 
 #include <verilated.h>
 #include "control_loop_math_implementation.h"
-#include "control_loop_cmds.h"
 #include "Vcontrol_loop_sim_top.h"
 using ModType = Vcontrol_loop_sim_top;
 
@@ -41,32 +40,27 @@ static void init(int argc, char **argv) {
 	mod->rst_L = 1;
 }
 
-static void set_value(V val, unsigned name) {
-	mod->cmd = CONTROL_LOOP_WRITE_BIT | name;
-	mod->word_into_loop = val;
-	mod->start_cmd = 1;
-
-	do { run_clock(); } while (!mod->finish_cmd);
-	mod->start_cmd = 0;
-	run_clock();
-}
-
 int main(int argc, char **argv) {
 	printf("sim top\n");
 	init(argc, argv);
 	Transfer func = Transfer{150, 0, 2, 1.1, 10, -1};
 
-	set_value(0b11010111000010100011110101110000101000111, CONTROL_LOOP_P);
 	/* Constant values must be sized to 64 bits, or else the compiler
 	 * will think they are 32 bit and silently mess things up
 	 */
-	set_value((V)6 << CONSTS_FRAC, CONTROL_LOOP_I);
+	mod->P_in = 0b11010111000010100011110101110000101000111;
-	set_value(20, CONTROL_LOOP_DELAY);
+	mod->I_in = (V)6 << CONSTS_FRAC;
-	set_value(10000, CONTROL_LOOP_SETPT);
+	mod->delay_in = 20;
-	set_value(1, CONTROL_LOOP_STATUS);
+	mod->setpt_in = 10000;
+	mod->assert_change = 1;
+	mod->run_loop_in = 1;
 	mod->curset = 0;
 
 	for (int tick = 0; tick < 1000;) {
+		if (mod->change_made) {
+			mod->assert_change = 0;
+		}
+
 		run_clock();
 		if (mod->request && !mod->fulfilled) {
 			/* Verilator values are not sign-extended to the
@@ -81,10 +75,6 @@ int main(int argc, char **argv) {
 			mod->fulfilled = 0;
 			tick++;
 		}
-
-		if (mod->finish_cmd) {
-			mod->start_cmd = 0;
-		}
 	}
 
 	mod->final();

gateware/rtl/control_loop/control_loop_sim_top.v

@@ -3,7 +3,6 @@
  * For license terms, refer to the files in `doc/copying` in the Upsilon
  * source distribution.
  */
-`include "control_loop_cmds.vh"
 
 module control_loop_sim_top #(
 	parameter ADC_WID = 18,
@@ -16,6 +15,7 @@ module control_loop_sim_top #(
 	parameter DAC_DATA_WID = 20,
 	parameter DAC_WID = 24,
 	parameter DAC_WID_SIZ = 5,
+	parameter CYCLE_COUNT_WID = 18,
 
 	parameter CONSTS_WHOLE = 21,
 	parameter CONSTS_FRAC = 43,
@@ -35,11 +35,18 @@ module control_loop_sim_top #(
 	input fulfilled,
 	output adc_err,
 
-	input [`CONSTS_WID-1:0] word_into_loop,
+	input assert_change,
-	output [`CONSTS_WID-1:0] word_outof_loop,
+	output change_made,
-	input start_cmd,
+
-	output finish_cmd,
+	input run_loop_in,
-	input [`CONTROL_LOOP_CMD_WIDTH-1:0] cmd
+	input [ADC_WID-1:0] setpt_in,
+	input [`CONSTS_WID-1:0] P_in,
+	input [`CONSTS_WID-1:0] I_in,
+	input [DELAY_WID-1:0] delay_in,
+
+	output [CYCLE_COUNT_WID-1:0] cycle_count,
+	output [DAC_DATA_WID-1:0] z_pos,
+	output [ADC_WID-1:0] z_measured
 );
 
 /* Emulate a control loop environment with simulator controlled
@@ -101,6 +108,7 @@ control_loop #(
 	/* Keeping cycle half wait and conv wait the same
 	 * since it doesn't matter for this simulation */
 
+	.CYCLE_COUNT_WID(CYCLE_COUNT_WID),
 	.CONSTS_WHOLE(CONSTS_WHOLE),
 	.CONSTS_FRAC(CONSTS_FRAC),
 	.CONSTS_SIZ(CONSTS_SIZ),
@@ -124,11 +132,18 @@ control_loop #(
 	.adc_conv_L(adc_ss_L),
 	.adc_sck(adc_sck),
 
-	.word_in(word_into_loop),
+	.assert_change(assert_change),
-	.word_out(word_outof_loop),
+	.change_made(change_made),
-	.start_cmd(start_cmd),
+
-	.finish_cmd(finish_cmd),
+	.run_loop_in(run_loop_in),
-	.cmd(cmd)
+	.setpt_in(setpt_in),
+	.P_in(P_in),
+	.I_in(I_in),
+	.delay_in(delay_in),
+
+	.cycle_count(cycle_count),
+	.z_pos(z_pos),
+	.z_measured(z_measured)
 );
 
 `ifdef VERILATOR

gateware/soc.py

@@ -56,6 +56,8 @@ from litedram.modules import MT41K128M16
 from litedram.frontend.dma import LiteDRAMDMAReader
 from liteeth.phy.mii import LiteEthPHYMII
 
+import mmio_descr
+
 """
 Keep this diagram up to date! This is the wiring diagram from the ADC to
 the named Verilog pins.
@@ -128,28 +130,26 @@ class Base(Module, AutoCSR):
 	keyword arguments to pass all the arguments.
 	"""
 
-	def _make_csr(self, name, csrclass, csrlen, description, num=None):
+	def _make_csr(self, reg, num=None):
-		""" Add a CSR for a pin `f"{name}_{num}"` with CSR type
+		""" Add a CSR for a pin `f"{name}_{num}".`
-		`csrclass`. This will automatically handle the `i_` and
-		`o_` prefix in the keyword arguments.
 
-        This function is used to automate the creation of memory mapped
+        :param name: Name of the MMIO register without prefixes or numerical
-        IO pins for all the converters on the device.
+          suffix.
-
+        :param num: Numerical suffix of this MMIO register. This is the only
-        `csrclass` must be CSRStorage (Read-Write) or CSRStatus (Read only).
+          parameter that should change when adding multiple CSRs of the same
-        `csrlen` is the length in bits of the MMIO register. LiteX automatically
+          name.
-        takes care of byte alignment, etc. so the length can be any positive
-        number.
-
-        Description is optional but recommended for debugging.
 		"""
 
-		if name not in self.csrdict.keys():
+        name = reg.name
-			self.csrdict[name] = csrlen
 		if num is not None:
 			name = f"{name}_{num}"
 
-		csr = csrclass(csrlen, name=name, description=description)
+        if self.ro == "read-only":
+            csrclass = CSRStatus
+        else:
+            csrclass = CSRStorage
+
+		csr = csrclass(reg.blen, name=name, description=None)
 		setattr(self, name, csr)
 
 		if csrclass is CSRStorage:
@@ -161,44 +161,13 @@ class Base(Module, AutoCSR):
 
 	def __init__(self, clk, sdram, platform):
 		self.kwargs = {}
-		self.csrdict = {}
 
-		for i in range(0,8):
+        for reg in mmio_descr.registers:
-			self._make_csr("adc_sel", CSRStorage, 3, f"Select ADC {i} Output", num=i)
+            if reg.num > 1:
-			self._make_csr("dac_sel", CSRStorage, 3, f"Select DAC {i} Output", num=i)
+                for i in range(0,reg.num):
-			self._make_csr("dac_finished", CSRStatus, 1, f"DAC {i} Transmission Finished Flag", num=i)
+                    self._make_csr(reg,i)
-			self._make_csr("dac_arm", CSRStorage, 1, f"DAC {i} Arm Flag", num=i)
+            else:
-			self._make_csr("dac_recv_buf", CSRStatus, 24, f"DAC {i} Received Data", num=i)
+                self._make_csr(reg)
-			self._make_csr("dac_send_buf", CSRStorage, 24, f"DAC {i} Data to Send", num=i)
-#			self._make_csr("wf_arm", CSRStorage, 1, f"Waveform {i} Arm Flag", num=i)
-#			self._make_csr("wf_halt_on_finish", CSRStorage, 1, f"Waveform {i} Halt on Finish Flag", num=i)
-#			self._make_csr("wf_finished", CSRStatus, 1, f"Waveform {i} Finished Flag", num=i)
-#			self._make_csr("wf_running", CSRStatus, 1, f"Waveform {i} Running Flag", num=i)
-#			self._make_csr("wf_time_to_wait", CSRStorage, 16, f"Waveform {i} Wait Time", num=i)
-#			self._make_csr("wf_refresh_start", CSRStorage, 1, f"Waveform {i} Data Refresh Start Flag", num=i)
-#			self._make_csr("wf_refresh_finished", CSRStatus, 1, f"Waveform {i} Data Refresh Finished Flag", num=i)
-#			self._make_csr("wf_start_addr", CSRStorage, 32, f"Waveform {i} Data Addr", num=i)
-#
-#			port = sdram.crossbar.get_port()
-#			setattr(self, f"wf_sdram_{i}", LiteDRAMDMAReader(port))
-#			cur_sdram = getattr(self, f"wf_sdram_{i}")
-#
-#			self.kwargs[f"o_wf_ram_dma_addr_{i}"] = cur_sdram.sink.address
-#			self.kwargs[f"i_wf_ram_word_{i}"] = cur_sdram.source.data
-#			self.kwargs[f"o_wf_ram_read_{i}"] = cur_sdram.sink.valid
-#			self.kwargs[f"i_wf_ram_valid_{i}"] = cur_sdram.source.valid
-
-			self._make_csr("adc_finished", CSRStatus, 1, f"ADC {i} Finished Flag", num=i)
-			self._make_csr("adc_arm", CSRStorage, 1, f"ADC {i} Arm Flag", num=i)
-			self._make_csr("adc_recv_buf", CSRStatus, 32, f"ADC {i} Received Data", num=i)
-
-		self._make_csr("cl_in_loop", CSRStatus, 1, "Control Loop Loop Enabled Flag")
-		self._make_csr("cl_cmd", CSRStorage, 8, "Control Loop Command Input")
-		self._make_csr("cl_word_in", CSRStorage, 64, "Control Loop Data Input")
-		self._make_csr("cl_word_out", CSRStatus, 64, "Control Loop Data Output")
-		self._make_csr("cl_start_cmd", CSRStorage, 1, "Control Loop Command Start Flag")
-		self._make_csr("cl_finish_cmd", CSRStatus, 1, "Control Loop Command Finished Flag")
-		self._make_csr("cl_z_report", CSRStatus, 1, "Control Loop Z Setting")
 
 		self.kwargs["i_clk"] = clk
 		self.kwargs["i_rst_L"] = ~platform.request("module_reset")
@@ -209,14 +178,8 @@ class Base(Module, AutoCSR):
 		self.kwargs["o_adc_conv"] = platform.request("adc_conv")
 		self.kwargs["i_adc_sdo"] = platform.request("adc_sdo")
 		self.kwargs["o_adc_sck"] = platform.request("adc_sck")
-#		self.kwargs["o_test_clock"] = platform.request("test_clock")
 		self.kwargs["o_set_low"] = platform.request("differntial_output_low")
 
-		""" Dump all MMIO pins to a JSON file with their exact bit widths. """
-		with open("csr_bitwidth.json", mode='w') as f:
-			import json
-			json.dump(self.csrdict, f)
-
 		self.specials += Instance("base", **self.kwargs)
 
 # Clock and Reset Generator

opensbi/litex/vexriscv/objects.mk

@@ -11,7 +11,6 @@ platform-runcmd = echo LiteX/VexRiscv
 
 PLATFORM_RISCV_XLEN = 32
 PLATFORM_RISCV_ABI = ilp32
-#PLATFORM_RISCV_ISA = rv32ima ## XXX: Broken on new binutils
 PLATFORM_RISCV_ISA = rv32ima_zicsr_zifencei
 PLATFORM_RISCV_CODE_MODEL = medany
 platform-objs-y += platform.o