fpga: Update coding guidelines

- Update recommended header
- Update module examples
- Add file/naming guidelines for modules
- Add default_nettype recommendation
- Add guidelines for generate statements
- Recommend all caps for constants
- Misc typos and adjustments

1 files changed, 107 insertions, 30 deletions

diff --git a/fpga/CODING.md b/fpga/CODING.md
index 805eaff0b..d9658ef81 100644
--- a/fpga/CODING.md
+++ b/fpga/CODING.md
@@ -23,16 +23,27 @@ helpful move for the team and future maintainability of the UHD FPGA codebase.
   worth optimizing for.
 * Try and keep line lengths to 79 characters, unless readability suffers.
 * Comment your code. Especially if your code is tricky or makes unique assumptions.
+* Provide a detailed description of each module or file in its header. Consider
+  its purpose, how it is intended to be used, assumptions made in its design,
+  limitations of the implementation, etc.
 * Use the following header at the top of each file:
 ```verilog
 //
-// Copyright <YEAR> Ettus Research, A National Instruments Company
+// Copyright <YEAR> Ettus Research, a National Instruments Brand
 //
 // SPDX-License-Identifier: LGPL-3.0-or-later
 //
 // Module: <MODULE_NAME>
+//
 // Description:
-// <DESCRIPTION>
+//   <Add a detailed description>
+//
+// Parameters:
+//   <Describe the parameters, if helpful>
+//
+// Signals:
+//   <Describe the port signals, if helpful>
+//
 ```
 
 ## Verilog Style Guidelines
@@ -58,25 +69,66 @@ end else begin
   // Do nothing
 end
 ```
-* Instantiate and declare modules as follows:
+* Declare and instantiate modules as shown below. Aligning the ports into
+  columns is not required but helps with readability and edibility. A good text
+  editor can make this easy.
 ```verilog
-dummy_module #(
-  .PARAM1(0), .PARAM2(1)
-) inst (
-  .clk(clk), .rst(rst)
+module top_level #(
+  parameter       PARAM1 = 0,
+  parameter [7:0] PARAM2 = 0
+) (
+  input  wire       clk,
+  input  wire       rst,
+  input  wire [7:0] signal1,
+  output wire [7:0] signal2
 );
+
+  example_one #(
+    .PARAM1(0), .PARAM2(1)
+  ) example_one_i (
+    .clk(clk), .rst(rst),
+    .port1(signal1), .port2(signal2)
+  );
+
+  example_two #(
+    .PARAM1(0),
+    .PARAM2(1)
+  ) example_two_i (
+    .clk(clk),
+    .rst(rst),
+    .port1(signal1),
+    .port2(signal2)
+  );
+
+  example_three #(
+    .PARAM1 (0),
+    .PARAM2 (1)
+  ) example_three_i (
+    .clk   (clk),
+    .rst   (rst),
+    .port1 (signal1),
+    .port2 (signal2)
+  );
+
+endmodule
+
 ```
 
 ### Assignments
 
-* Sequential blocks **must** only have non-blocking assignments (`<=`)
+* Sequential blocks should only have non-blocking assignments (`<=`)
 * Combinational blocks should only have blocking assignments (`=`)
 * Don't mix blocking and non-blocking assignments
 
 ### Modules
 
 * Each module should be defined in a separate file
-* Use Verilog 2001 ANSI-C style port declarations
+* The file name should be the same as the module name
+* File and module names should be unique. Do not create a new module with the
+  same name as an existing module.
+* Every module requires a description in the header that gives a synopsis of
+  its function.
+* Use Verilog 2001 ANSI-C style port declarations:
 ```verilog
 (
   ...
@@ -85,20 +137,31 @@ dummy_module #(
 );
 ```
 * Declare inputs and outputs one per line. This makes searching and commenting easier.
-* Add inline comments to input/output ports to describe their behavior
+* Add comments to the header or inline to describe the behavior of input/output ports.
 * Be explicit about whether an input or output is a wire or reg.
 * Group signals logically instead of by direction. If a single AXI-Stream bus has multiple inputs and
 outputs, keep them together.
 * Instantiate all ports for a module even if they are tied off or unconnected. Don't let the compiler
 insert values for any signals automatically.
 ```verilog
-dummy_module inst (
-  .clk(clk), .rst(1'b0), .status(/* unused */)
+dummy_module dummy_module_i (
+  .clk    (clk),
+  .rst    (1'b0),
+  .status (/* unused */)
 );
 ```
 * Don't instantiate modules using positional arguments. Use the dot form illustrated above.
-* Every module requires a header giving a synopsis of its function below the
-  copyright header.
+* Use default_nettype at the beginning and end of each module file to avoid
+  problems with undeclared signals:
+```verilog
+`default_nettype none
+
+module example #(
+  ...
+endmodule
+
+`default_nettype wire
+```
 
 ### Clocking and Resets
 
@@ -110,24 +173,43 @@ to be explicit about the domain, for example `axi_tdata_bclk`, `axi_tdata_rclk`.
 * Name resets as `rst`. If there are multiple clocks then use a prefix like `bus_rst` and `radio_rst`.
 * If a reset is asynchronous, call it `arst`.
 * Try to avoid asynchronous resets as much as possible.
-* Don't active low resets unless it is used to drive IO.
+* Don't use active low resets unless it is used to drive top-level IO or IP
+  that requires it.
+
+### Generate Statements
 
-### Parameters, defines and constants
+* Use the ``generate``/``endgenerate`` keywords for backwards compatibility
+  with earlier versions of the Verilog language.
+* Add labels to the `begin`/`end` blocks of generate statements, so that the
+  labels are used when displaying the hierarchical names in the simulator and
+  other tools, and so that logic can be referenced hierarchically by name in
+  testbenches if needed. For example:
+```verilog
+genvar i;
+generate
+  for (i = 0; i < NUM_PORTS; i++) begin : gen_loop_example
+    ...
+  end
+endgenerate
+```
 
-* Parametrize modules wherever possible, especially if they are designed for reuse. Bus widths, addresses,
+### Parameters, Defines and Constants
+
+* Parameterize modules wherever possible, especially if they are designed for reuse. Bus widths, addresses,
 buffer sizes, etc are good candidates for parametrization.
-* For modules with parameters, add inline comments to describe the behavior of each parameter
+* For modules with parameters, add comments in the header or inline to describe the behavior of each parameter.
 * Propagate parameters as far up the hierarchy as possible as long as it makes sense.
-* Place `` `define`` statements in Verilog header file (.vh) and include them in modules.
-* Avoid placing `` `define`` statements in modules
-* For local parameters, use `localparam` instead on hard-coding things like widths, etc.
+* Place `` `define`` statements in Verilog header files (.vh) and `` `include`` them in modules.
+* Avoid placing `` `define`` statements in modules.
+* For local constants, use `localparam` instead on hard-coding things like widths, etc.
+* Use all uppercase names for constants (``parameter``, ``localparam``, or
+  `` `define``) to distinguish them from signal names.
 
-### AXI Specific
+### AXI Guidelines
 
-We heavily use AXI buses in the design so here are some best practices for those:
-* Keep the components of an AXI-Stream or AXI-MM bus together in port/wire instantiations
+* Keep the components of an AXI-Stream or AXI-MM bus together in port/wire instantiations.
 * For module ports, use the master/slave naming convention as shown below. It makes connecting modules
-easier because a master always connects to a slave
+easier because a master always connects to a slave.
 ```verilog
   input  wire [63:0] s_axis_tdata,
   input  wire        s_axis_tlast,
@@ -152,8 +234,3 @@ on its function or underlying data.
   wire        samp_tvalid,
   wire        samp_tready,
 ```
-
-
-## Design Best Practices
-
-TBD