Forum Discussion
How about try the below:
module uart_rx #(
parameter CLOCK_FREQ = 50000000, // 50 MHz clock
parameter BAUD_RATE = 9600 // 9600 baud
)(
input clk,
input rst_n,
input rx,
output reg [7:0] rx_data,
output reg rx_valid
);
localparam BAUD_TICKS = CLOCK_FREQ / BAUD_RATE;
localparam HALF_BAUD_TICKS = BAUD_TICKS / 2;
localparam COUNTER_WIDTH = $clog2(BAUD_TICKS);
reg [COUNTER_WIDTH-1:0] baud_counter;
reg [3:0] bit_counter;
reg [7:0] rx_shift_reg;
reg baud_tick;
reg rx_sync1, rx_sync2; // Synchronizer for rx input
// Input synchronizer
always @(posedge clk or negedge rst_n) begin
if (!rst_n) begin
rx_sync1 <= 1'b1;
rx_sync2 <= 1'b1;
end else begin
rx_sync1 <= rx;
rx_sync2 <= rx_sync1;
end
end
// State machine states
localparam IDLE = 2'b00;
localparam START = 2'b01;
localparam DATA = 2'b10;
localparam STOP = 2'b11;
reg [1:0] state;
// Combined baud counter and UART receiver state machine
always @(posedge clk or negedge rst_n) begin
if (!rst_n) begin
state <= IDLE;
bit_counter <= 0;
rx_shift_reg <= 0;
rx_data <= 0;
rx_valid <= 1'b0;
baud_counter <= 0;
baud_tick <= 1'b0;
end else begin
rx_valid <= 1'b0; // Default: no valid data
baud_tick <= 1'b0; // Default: no baud tick
case (state)
IDLE: begin
baud_counter <= 0;
if (!rx_sync2) begin // Start bit detected
state <= START;
baud_counter <= 0;
end
end
START: begin
if (baud_counter == HALF_BAUD_TICKS) begin
if (!rx_sync2) begin // Confirm start bit
state <= DATA;
bit_counter <= 0;
baud_counter <= 0;
end else begin
state <= IDLE; // False start bit
end
end else begin
baud_counter <= baud_counter + 1;
end
end
DATA: begin
if (baud_counter == BAUD_TICKS - 1) begin
baud_counter <= 0;
baud_tick <= 1'b1;
rx_shift_reg <= {rx_sync2, rx_shift_reg[7:1]};
if (bit_counter == 7) begin
state <= STOP;
bit_counter <= 0;
end else begin
bit_counter <= bit_counter + 1;
end
end else begin
baud_counter <= baud_counter + 1;
end
end
STOP: begin
if (baud_counter == BAUD_TICKS - 1) begin
baud_counter <= 0;
baud_tick <= 1'b1;
if (rx_sync2) begin // Valid stop bit
rx_data <= rx_shift_reg;
rx_valid <= 1'b1;
end
state <= IDLE;
end else begin
baud_counter <= baud_counter + 1;
end
end
endcase
end
end
endmodule
You're correct! The issue is that both the baud rate counter logic and the state machine are trying to drive the `baud_counter` signal, creating multiple drivers. Here's the fixed UART receiver code with the two always blocks merged:
## Fixed UART Receiver Code
```verilog
module uart_rx #(
parameter CLOCK_FREQ = 50000000, // 50 MHz clock
parameter BAUD_RATE = 9600 // 9600 baud
)(
input clk,
input rst_n,
input rx,
output reg [7:0] rx_data,
output reg rx_valid
);
localparam BAUD_TICKS = CLOCK_FREQ / BAUD_RATE;
localparam HALF_BAUD_TICKS = BAUD_TICKS / 2;
localparam COUNTER_WIDTH = $clog2(BAUD_TICKS);
reg [COUNTER_WIDTH-1:0] baud_counter;
reg [3:0] bit_counter;
reg [7:0] rx_shift_reg;
reg baud_tick;
reg rx_sync1, rx_sync2; // Synchronizer for rx input
// Input synchronizer
always @(posedge clk or negedge rst_n) begin
if (!rst_n) begin
rx_sync1 <= 1'b1;
rx_sync2 <= 1'b1;
end else begin
rx_sync1 <= rx;
rx_sync2 <= rx_sync1;
end
end
// State machine states
localparam IDLE = 2'b00;
localparam START = 2'b01;
localparam DATA = 2'b10;
localparam STOP = 2'b11;
reg [1:0] state;
// Combined baud counter and UART receiver state machine
always @(posedge clk or negedge rst_n) begin
if (!rst_n) begin
state <= IDLE;
bit_counter <= 0;
rx_shift_reg <= 0;
rx_data <= 0;
rx_valid <= 1'b0;
baud_counter <= 0;
baud_tick <= 1'b0;
end else begin
rx_valid <= 1'b0; // Default: no valid data
baud_tick <= 1'b0; // Default: no baud tick
case (state)
IDLE: begin
baud_counter <= 0;
if (!rx_sync2) begin // Start bit detected
state <= START;
baud_counter <= 0;
end
end
START: begin
if (baud_counter == HALF_BAUD_TICKS) begin
if (!rx_sync2) begin // Confirm start bit
state <= DATA;
bit_counter <= 0;
baud_counter <= 0;
end else begin
state <= IDLE; // False start bit
end
end else begin
baud_counter <= baud_counter + 1;
end
end
DATA: begin
if (baud_counter == BAUD_TICKS - 1) begin
baud_counter <= 0;
baud_tick <= 1'b1;
rx_shift_reg <= {rx_sync2, rx_shift_reg[7:1]};
if (bit_counter == 7) begin
state <= STOP;
bit_counter <= 0;
end else begin
bit_counter <= bit_counter + 1;
end
end else begin
baud_counter <= baud_counter + 1;
end
end
STOP: begin
if (baud_counter == BAUD_TICKS - 1) begin
baud_counter <= 0;
baud_tick <= 1'b1;
if (rx_sync2) begin // Valid stop bit
rx_data <= rx_shift_reg;
rx_valid <= 1'b1;
end
state <= IDLE;
end else begin
baud_counter <= baud_counter + 1;
end
end
endcase
end
end
endmodule
```
## Key Changes Made
1. **Merged Always Blocks**: Combined the separate baud counter and state machine always blocks into a single always block to eliminate multiple drivers for `baud_counter`.
2. **State-Specific Counter Logic**: The baud counter now resets and increments based on the current state:
- **IDLE**: Counter is reset to 0
- **START**: Counter increments until half baud period
- **DATA/STOP**: Counter increments until full baud period, then resets
3. **Simplified Baud Tick Generation**: The `baud_tick` signal is now generated inline with the counter logic rather than in a separate process.
## Complete Updated Top-Level Module
The UART transmitter and top-level module remain the same. Just replace the `uart_rx` module with the fixed version above.