From a2acccbca6c0e9d5a0183113ec127ae2efeb50a0 Mon Sep 17 00:00:00 2001
From: Peter McGoron <code@mcgoron.com>
Date: Tue, 20 Dec 2022 06:07:54 +0000
Subject: [PATCH] misc

---
 doc/programmers_manual.md    | 22 ++++++++++++++++++++++
 firmware/rtl/raster/Makefile |  4 ++++
 firmware/rtl/raster/raster.v | 11 +++++++++--
 3 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/doc/programmers_manual.md b/doc/programmers_manual.md
index 10c70cf..8cdb831 100644
--- a/doc/programmers_manual.md
+++ b/doc/programmers_manual.md
@@ -135,6 +135,10 @@ add that case to the simulation.
 
 ## Test Synthesis
 
+**Yosys only accepts a subset of the Verilog that Verilator supports. You
+might write a bunch of code that Verilator will happily simulate but that
+will fail to go through Yosys.**
+
 Once you have simulated your design, you should use yosys to synthesize it.
 This will allow you to understand how much and what resources the module
 is taking up. To do this, you can put the follwing in a script file:
@@ -161,3 +165,21 @@ The open source toolchain that Upsilon uses is novel and unstable.
 This is really a Yosys (and really, really, an abc bug). F4PGA defaults
 to using the ABC flow, which can break, especially for block RAM. To
 fix, edit out `-abc` in the tcl script (find it before you install it...)
+
+## Yosys
+
+Yosys fails to calculate computed parameter values correctly. For instance,
+
+    parameter CTRLVAL = 5;
+    localparam VALUE = CTRLVAL + 1;
+
+Yosys will *silently* fail to compile this, setting `VALUE` to be equal
+to 0. The solution is to use preprocessor defines:
+
+    parameter CTRLVAL = 5;
+    `define VALUE (CTRLVAL + 1)
+
+In Verilog, in order to replace a macro identifier with the value of the
+macro, you must put a backtick before the name: i.e.
+
+    `VALUE
diff --git a/firmware/rtl/raster/Makefile b/firmware/rtl/raster/Makefile
index 5a42cc6..1c85bb9 100644
--- a/firmware/rtl/raster/Makefile
+++ b/firmware/rtl/raster/Makefile
@@ -27,3 +27,7 @@ obj_dir/Vram_shim: obj_dir/Vram_shim.mk ram_shim_sim.cpp
 ram_shim_cmds.h: ram_shim_cmds.vh
 	echo '#pragma once' > ram_shim_cmds.h
 	sed 's/`define/#define/g; s/`//g' ram_shim_cmds.vh >> ram_shim_cmds.h
+
+clean:
+	rm -rf obj_dir
+	rm *.vcd ram_shim_cmds.h
diff --git a/firmware/rtl/raster/raster.v b/firmware/rtl/raster/raster.v
index d951276..59694c8 100644
--- a/firmware/rtl/raster/raster.v
+++ b/firmware/rtl/raster/raster.v
@@ -1,3 +1,10 @@
+/* Raster scanner. This module sweeps two DACs (the X and Y piezos)
+ * across a box, where the X and Y axes may be at an angle. After
+ * a single step, the ADCs connected to the raster scanner are
+ * activated, with each value read into system memory (see ram_shim).
+ * The kernel then reads these values and sends them to the controller
+ * over ethernet.
+ */
 module raster #(
 	parameter SAMPLEWID = 9,
 	parameter DAC_DATA_WID = 20,
@@ -280,10 +287,10 @@ always @ (posedge clk) begin
 			end else begin
 				/* rotation of (dx,dy) by 90° -> (dy, -dx) */
 				x_val <= x_val + dy;
-				x_to_dac <= {4'b0001, x_val + dx_vert};
+				x_to_dac <= {4'b0001, x_val + dy};
 				x_arm <= 1;
 				y_val <= y_val - dx;
-				y_to_dac <= {4'b0001, y_val + dy_vert};
+				y_to_dac <= {4'b0001, y_val - dx};
 				y_arm <= 1;
 				line <= line + 1;
 			end