Иноформационно-образовательный портал для FPGA разработчиков
Главное меню
  • Страница 1 из 1
  • 1
Модератор форума: drakonof  
Форум FPGA комьюнити » HDL - вопросы по коду » Code review » UART-приемник
UART-приемник
voronovdrawtoon
Рядовой
Группа: Проверенные
Сообщений: 2
Репутация: 0
Статус: Offline
 
Обычный UART-приемник. При описании конечного автомата использована методика из книги Понг П. Чу. Все работает. Есть ли какие-нибудь замечания или предложения касательно моего кода?
Код
#VERILOG
`timescale 1ns / 100ps

module UART_receiver
#(parameter FREQ = 24_000_000,
            BAUD_RATE = 9600)
(
    input wire clk,
    input wire DATA_serial,
    
   // output reg data_bit = 0,
   // output reg data_bit2 = 0,
    //output reg [2:0] STATE_reg = 0,
   //output reg [11:0] clk_counter_reg = 0,
   //output reg [2:0] bit_index_reg = 0,
    
    output reg done_tick = 0,
    output wire [7:0] DATA_byte

    );
    
//signal declaration
reg [11:0] clk_counter_reg = 0;
reg [11:0] clk_counter_next;
reg data_bit, data_bit2;
reg [2:0] bit_index_reg = 0;
reg [2:0] bit_index_next;
reg [7:0] byte_bit = 0;
reg [2:0] STATE_reg = 0;
reg [2:0] STATE_next;

//dowble register body
always @(posedge clk)
  begin
    data_bit2 <= DATA_serial;
    data_bit <= data_bit2;
    STATE_reg <= STATE_next;
    clk_counter_reg <= clk_counter_next;
    bit_index_reg <= bit_index_next;
  end

//state declaration
localparam [2:0] IDLE = 3'b000,
                 START_BIT = 3'b001,
                 DATA_TRANSFER = 3'b010,
                 STOP_BIT = 3'b011,
                 DONE = 3'b100;
                 
//------------FSM BODY---------------
always @*
  begin
    done_tick = 0;
    clk_counter_next = clk_counter_reg;
    bit_index_next = bit_index_reg;
    STATE_next = STATE_reg;
  
  
      case (STATE_reg) IDLE: begin
                           //waiting start_bit
                    if (data_bit == 0)
                    STATE_next = START_BIT;
                    else
                    STATE_next = IDLE;
                             end
                       
           START_BIT: begin
                      //check the midle of start_bit
                        if (clk_counter_next == ((FREQ/2/BAUD_RATE)-1))
                          if (data_bit == 0) begin
                            clk_counter_next = 0; //midle of input_bit duration
                            STATE_next = DATA_TRANSFER; end
                          else
                            STATE_next = IDLE;
                        else begin
                          clk_counter_next = clk_counter_reg + 1;
                          STATE_next = START_BIT;
                        end
                      end
                      
       DATA_TRANSFER: begin
                      // receive 8 bit of data
                        if (clk_counter_next < ((FREQ/BAUD_RATE)-1)) begin
                          clk_counter_next = clk_counter_reg + 1;  
                          STATE_next = DATA_TRANSFER; end
                        else begin
                          clk_counter_next = 0;
                          byte_bit[bit_index_next] = data_bit;
                            if (bit_index_next < 7) begin
                    bit_index_next = bit_index_reg + 1;
                    STATE_next = DATA_TRANSFER; end
                            else begin
                    bit_index_next = 0;
                    STATE_next = STOP_BIT; end
                        end
                     end
                     
           STOP_BIT: begin
                     // receive stop-bit
                       if (clk_counter_next < ((FREQ/BAUD_RATE)-1)) begin
                         clk_counter_next = clk_counter_reg + 1;
                         STATE_next = STOP_BIT; end
                       else begin
                        // done_tick = 1;
                         clk_counter_next = 0;
                         STATE_next = DONE; end
                     end
                     
           DONE: begin
                     //waiting 1 clk cycle
                     done_tick = 1;
                     STATE_next = IDLE;
                     end
                     
             default: STATE_next = IDLE;
  endcase
end
   
   
assign DATA_byte = byte_bit;   
                    
    
endmodule
dsmv
Рядовой
Группа: Проверенные
Сообщений: 2
Репутация: 0
Статус: Offline
 
Для начала нет установки по сбросу для STATE_reg  и остальных сигналов.
evgeniybolnov
Рядовой
Группа: Проверенные
Сообщений: 3
Репутация: 0
Статус: Offline
 
Так же категорически желательно соблюдать заветы синхронного дизайна и избегать описания защелок(Latch) в byte_bit
andrewkushchenko
Рядовой
Группа: Проверенные
Сообщений: 2
Репутация: 2
Статус: Offline
 
1. Хорошо добавить `default_nettype none в начале файле и `default_nettype wire в конце файла. может спасти от многих ошибок.
2. очень тяжело читать из-за неравномерных отступов. если начал делать 2 отступа - стоит везде так делать. почти всегда выравнивания по каким-то блокам не имеют смысла а тут и вовсе нет отступов

Цитата
Код
#VERILOG
if (data_bit == 0) STATE_next = START_BIT; else STATE_next = IDLE;


3. Тут лучше написать localparam для каждого на мой вкус. чернила бесплатные. ну и хорошо бы явно тип указать
Цитата voronovdrawtoon ()
Код
#VERILOG
//state declarationlocalparam [2:0] IDLE = 3'b000,
START_BIT = 3'b001,
DATA_TRANSFER = 3'b010,
STOP_BIT = 3'b011,
DONE = 3'b100;

4. константа FREQ/BAUD_RATE)-1 вычисляется несколько раз в коде. было бы легче и понятнее если бы для нее завести localparam с понятным именем (аналогично для FREQ/2/BAUD_RATE)-1)

5.
Цитата voronovdrawtoon ()
Код
#VERILOG
byte_bit[bit_index_next] = data_bit;

такое ощущение, что получится дешифратор в результате и фанаут 8 на входном бите данных. лучше сделать сдвиговый регистр. все равно же ждешь 8 бит данных. сдвиговый регистр сэкономит логику и трассировочные ресурсы. сейчас не важно, но в больших и сложных проектах будет важно.

6. в то же месте. ты выполняешь блокирующее присваивание (которое без стрелочки), получается защелка (латч) надо было делать неблокирубщее (<=) и под клоком
voronovdrawtoon
Рядовой
Группа: Проверенные
Сообщений: 2
Репутация: 0
Статус: Offline
 
Спасибо большое за ревью
Цитата evgeniybolnov ()
ак же категорически желательно соблюдать заветы синхронного дизайна и избегать описания защелок(Latch) в byte_bit
Цитата andrewkushchenko ()
такое ощущение, что получится дешифратор в результате и фанаут 8 на входном бите данных. лучше сделать сдвиговый регистр. все равно же ждешь 8 бит данных. сдвиговый регистр сэкономит логику и трассировочные ресурсы. сейчас не важно, но в больших и сложных проектах будет важно.
6. в то же месте. ты выполняешь блокирующее присваивание (которое без стрелочки), получается защелка (латч) надо было делать неблокирубщее (<=) и под клоком
Да, защелку я и вправду не заметил. Решил исправить и заодно сделать сдвиговый регистр таким вот маневром:
Код
#VERILOG
always @(posedge clk)
    if (bit_index_next != bit_index_reg) byte_bit <= {data_bit, byte_bit[7:1]};

Но мне не понятно, почему при синтезе схемы сигнал done_tick то висит в воздухе, то притянут к нулю:
Прикрепления: 7225455.png (23.7 Kb)
KeisN13
Рядовой
Группа: Администраторы
Сообщений: 11
Репутация: 0
Статус: Offline
 
Цитата voronovdrawtoon ()
Но мне не понятно, почему при синтезе...

Во первых, это не синтез на картинке, а этап elaborate, там отображается просто все как есть в абстрактной форме. Всё, что подключено правильно или не правильно будет отображаться.
Flip-Fl0p
Рядовой
Группа: Проверенные
Сообщений: 2
Репутация: 0
Статус: Offline
 
1. Где фильтр дребезга по входу ?  
2. Рекомендуется вести прием на клоке х16 от бодовой частоты 
3. Лучше делать 3 выборки в середине и мажоритарным фильтром по выборкам определять значение.
4. А если не придет стоп бит, то что тогда ?
Форум FPGA комьюнити » HDL - вопросы по коду » Code review » UART-приемник
  • Страница 1 из 1
  • 1
Поиск:
ePN