Detect when X-axis inputs and Y-axis inputs go high

Question

I am working on a module named PinCoordinates that detects when X-axis and Y-axis inputs go high. This will be programmed onto an Altera MAX V CPLD. I am constructing a laser grid array that has a total of 50 lasers. There are 25 X-axis lasers and 25 for the y-axis. On the opposite end of the array, there are photodiodes. I hope to be able to detect objects passing through the grid and return the coordinates.

For the code, pins 1-25 represent X axis lasers, and 26-50 are for the Y-axis. The code is as follows:

module PinCoordinates(
input wire [49:0] pins, // 50 pins input
output reg [4:0] x_coordinate, // Output X coordinate pins (1-25)
output reg [4:0] y_coordinate, // Output Y coordinate pins (26-50)
output reg output_valid // Output indicating valid coordinates
);
integer i;
reg x_detected, y_detected; // Flags to indicate detection of X and Y coordinates
always @* begin
// Reset detection flags at the beginning of each evaluation
 x_detected = 1'b0;
 y_detected = 1'b0;
 output_valid = 1'b0;
 // Check for high X coordinate
 for (i = 0; i < 25; i = i + 1) begin
 if (pins[i] == 1'b1) begin
 x_coordinate = i + 1; // X coordinate pin numbering starts from 1
 x_detected = 1'b1; // Set X coordinate detection flag
 
 end
 end
 // Check for high Y coordinate
 for (i = 25; i < 50; i = i + 1) begin
 if (pins[i] == 1'b1) begin
 y_coordinate = i - 24; // Y coordinate pin numbering starts from 26
 y_detected = 1'b1; // Set Y coordinate detection flag
 
 end
 end
 // Output is valid only if both X and Y coordinates are detected
 if (x_detected && y_detected) begin
 output_valid = 1'b1;
 end
end
endmodule

I understand that one of the upgrades that I could make to this code is to include a break after either an X-pin or Y-pin is discovered to be high; however, since Verilog doesn't support break statements, I'm not sure how to implement this. Any more ideas in what I should do to the code? Any assistance or criticism would be very welcome.

Answer 1

Overview

You've done a good job with the following:

• Code layout and use of indentation
• Using compact ANSI-style module ports
• Using the compact implicit sensitivity list @*
• Using descriptive names for the module and the signals
• Using descriptive comments

Latches

Using always @* implies that you intend to describe purely combinational logic, but it simulates as a latch, which is a type of sequential logic. This code will infer latches when it is synthesized. You will likely get warning messages about that when you try to synthesize it.

The reason it behaves like a latch is because some signals are not assigned under all conditions. Specifically, x_coordinate is not assigned when i is in the range 25 to 49. In this case, x_coordinate retains its state, inferring a latch.

You should be able to easily observe this behavior in simulations by looking at waveforms of the pins and x_coordinate signals.

One way to avoid the unintended latches is to initialize the signal in the always block, as you did for x_detected. For example:

always @* begin
// Reset detection flags at the beginning of each evaluation
 x_detected = 1'b0;
 x_coordinate = 0;

y_coordinate will also infer latches.

Simpler

In Verilog, it is customary to omit explicitly comparing a 1-bit signal to 1. For example:

 if (pins[i] == 1'b1) begin

is more simply written as:

 if (pins[i]) begin

This is a matter of preference, not functionality.

Partitioning

With small designs, it is common to add all code to a single always block as you have done. However, as people start adding more logic, they sometimes fall into the trap of cramming everything into the same block.

Generally, it is easier to understand code which is split into multiple blocks based on functionality. For example, the X and Y logic are really independent of each other and can easily be partitioned into separate blocks. Here is what the X logic would look like:

always @* begin
 // Reset detection flags at the beginning of each evaluation
 x_detected = 1'b0;
 x_coordinate = 0;
 // Check for high X coordinate
 for (i = 0; i < 25; i = i + 1) begin
 if (pins[i]) begin
 x_coordinate = i + 1; // X coordinate pin numbering starts from 1
 x_detected = 1'b1; // Set X coordinate detection flag
 end
 end
end

There would be another always block for just the Y logic.

Since the valid signal depends on both X and Y, it would be outside of both blocks. It can also be simplified by using a continuous assignment instead of a procedural assignment (one inside an always block):

output output_valid // Output indicating valid coordinates
// ...
assign output_valid = (x_detected && y_detected);

Note that I removed the reg keyword from the output port declaration.

Another simplification is to factor the x_detected logic out of the block as well. You could use a "reduction-OR" operator to do a bitwise OR of pins 0 to 24. If any of the pins is set, then it is detected; otherwise, if all pins are 0, it is not detected:

wire x_detected = |pins[24:0];

The same could be done for Y.

SystemVerilog

SystemVerilog (SV) features were added to the language nearly 20 years ago, and all tool chains support these features to some degree. Annoyingly, many tools have these features disabled by default, which means you may need enable the features with some sort of tool settings. Refer to your simulation/synthesis documentation.

Using SV syntax has its advantages.

You can replace always @* with always_comb. Not only does it convey the design intent better, but it also provides built-in checking if your tool has the capability to do so.

Code can be simpler to understand using the terse int keyword and ++ operator in the for loop:

 for (int i = 0; i < 25; i++) begin

This has the advantage of declaring the i variable closer to where it is used.

The compact syntax, '0, can be used to set all bits of a signal to 0.

break

Verilog doesn't support break statements

SV does have a break keyword, but there is no need to use it here.

---

Here is the code with the suggestions above:

module PinCoordinates (
 input wire [49:0] pins,
 output reg [4:0] x_coordinate, // Output X coordinate pins (1-25)
 output reg [4:0] y_coordinate, // Output Y coordinate pins (26-50)
 output output_valid // Output indicating valid coordinates
);
// Flags to indicate detection of X and Y coordinates
wire x_detected = |pins[24:0];
wire y_detected = |pins[49:25];
assign output_valid = (x_detected && y_detected);
always_comb begin
 // Reset at the beginning of each evaluation
 x_coordinate = '0;
 // Check for high X coordinate
 for (int i = 0; i < 25; i++) begin
 if (pins[i]) begin
 x_coordinate = i + 1; // X coordinate pin numbering starts from 1
 end
 end
end
always_comb begin
 // Reset at the beginning of each evaluation
 y_coordinate = '0;
 // Check for high Y coordinate
 for (int i = 25; i < 50; i++) begin
 if (pins[i]) begin
 y_coordinate = i + 1; // Y coordinate pin numbering starts from 1
 end
 end
end
endmodule