2
\$\begingroup\$

I created this project on EDA Playground that builds a variable-width 3-way multiplexer out of 2 variable-width 2-way multiplexers. It will be part of homework that I assign, so I want to follow all best practices. I followed the lowRISC Verilog Coding Style Guide.

Design Modules

module mux2 #(
  parameter Width = 4
) (
  input              control,
  input [Width-1:0]  a,
  input [Width-1:0]  b,
  output [Width-1:0] y
);
  
  assign y = control ? b : a;
  
endmodule

module mux3 #(
  parameter Width = 4
) (
  input [1:0]        control,
  input [Width-1:0]  a,
  input [Width-1:0]  b,
  input [Width-1:0]  c,
  output [Width-1:0] y
);
  
  wire [Width-1:0] mux_a_out;
  
  mux2 #(
    .Width   (Width)
  ) mux_a (
    .control (control[0]), 
    .a       (a), 
    .b       (b), 
    .y       (mux_a_out)
  );
  
  mux2 #(
    .Width   (Width)
  ) mux_b (
    .control (control[1]), 
    .a       (mux_a_out), 
    .b       (c), 
    .y       (y)
  );
  
endmodule

Test Module

module test;
  reg[1:0] control;
  reg[7:0] a;
  reg[7:0] b;
  reg[7:0] c;
  reg[7:0] y;
  
  mux3 #(
    .Width(8)
  ) mux3(
    .control (control),
    .a       (a),
    .b       (b),
    .c       (c),
    .y       (y)
  );
  
  initial begin
    a = "A";
    b = "B";
    c = "C";
    
    for (int i = 0; i < 3; i++)
      begin
        control = i;
        #1
        $display("When Control = %d, Y = %c", control, y);
      end
  end
  
endmodule
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

I don't see any problems with the code functionally, and the layout follows good practices for the most part. That style guide seems mostly reasonable, and it is a good idea to use a set of guidelines.

In my many years of Verilog coding, I have most frequently seen parameters use all capital letters (this departs from the guide you posted):

Use WIDTH instead of Width.

The guide recommends 2-space indents in some places and 4 in others; I recommend 4-space indent everywhere because I think it is much easier to read.

I don't think your for loop follows the guide. I would lay it out like this:

for (int i = 0; i < 3; i++) begin
    control = i;
    #1
    $display("When Control = %d, Y = %c", control, y);
end

I would also add a space between reg and [:

  reg [1:0] control;
  reg [7:0] a;
  reg [7:0] b;
  reg [7:0] c;
  reg [7:0] y;

I usually find it helpful for debugging to also display the time (copied from my Answer to your previous Verilog Question):

$display($time, " When Control = %d, Y = %c", control, y);

The for loop exercises 3 of the 4 possible values of Control. It is a good practice to simulate all possibilities (Control = 3). Perhaps make it an extra-credit question to explain what happens and why.


I assume one of the goals of this assignment is to demonstrate module hierarchy using an extremely simple design. Otherwise, your design modules normally would be replaced with a single assign statement.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Thank you! Your feedback is helpful, and your assumption is correct. \$\endgroup\$ Nov 9, 2020 at 23:46

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.