[Anfänger] RS232 Modul in VHDL - Kritik erwünscht

[Robert S. (razer)]

Hallo an alle,

als VHDL Anfänger hatte ich in den letzten Tagen wieder ein Wenig Zeit. 
Ziel war es ein einfaches UART Modul zu entwickeln. Der Source Code 
(UART + Testbench + TCL-Skript zum Starten der Simulation).

Die Simulation funktioniert soweit :) Als Anfänger ist es jedoch nicht 
immer so einfach zu beurteilen, wie "gut" das Design ist, ob grobe 
Fallstricken übersehen wurden oder ob das Design einfach schlecht ist 
;-)

Ich hoffe ihr könnt mir ein paar nützliche Kommentare zu meinem Design 
geben bzw. über konstruktive Kritik würde ich mich sehr freuen.

Danke im Voraus,
lg Robert

[Duke Scarring (Gast)]

Sieht gut aus.

Mir fällt nur auf, daß Dein Vektor ein Bit zu breit ist:

    WIDTH              : integer := 8;
...

 signal rx_data_in : std_logic_vector(WIDTH downto 0) := (others => '0');

Die Exponentenschreibweise hast Du nur mit dem Simulationstool getestet?

    CLK_FREQ           : integer := 100e6;

Geth das auch durch die aktuellen Synthesizer?

Wird das Signal rx_busy wirklich benötigt?

Duke

[Robert S. (razer)]

Duke Scarring schrieb:
> Mir fällt nur auf, daß Dein Vektor ein Bit zu breit ist:

Nein, dass ist gewollt so. Es wird hier das Stop-Bit mit reingetaktet 
und im letzten State nur mehr 8-bits auf den Ausgangsport.

Duke Scarring schrieb:
> Geth das auch durch die aktuellen Synthesizer?

Funktioniert mit ISE 14.7.

Duke Scarring schrieb:
> Wird das Signal rx_busy wirklich benötigt?

Ja, das stimmt. Das ist eigentlich nicht nötig. Mit rx_ready ist 
eindeutig bestimmt wann ein gültiger Lesezugriff möglich ist.

[greg (Gast)]

Gibt's einen Grund, warum rx_in drei Bits breit ist? Um eine Flanke zu 
erkennen, reichen doch zwei Bits, oder?

[Robert S. (razer)]

greg schrieb:
> Gibt's einen Grund, warum rx_in drei Bits breit ist? Um eine Flanke zu
> erkennen, reichen doch zwei Bits, oder?

Das stimmt eigentlich - Danke!

Was eigentlich noch nicht ganz stimmt ist die Auswertung des Start-Bits. 
Bei fallender Flanke verlässt die Statemachine zwar IDLE, das Start-Bit 
wird aber erst danach gesampelt. Es muss ausgewertet werden, ob das 0 
ist, dann sonst war es nur ein "Wackler" auf der RX-Leitung.

[René D. (Firma: www.dossmatik.de) (dose)]

> Die Simulation funktioniert soweit :) Als Anfänger ist es jedoch nicht
> immer so einfach zu beurteilen, wie "gut" das Design ist, ob grobe
> Fallstricken übersehen wurden oder ob das Design einfach schlecht ist
> ;-)

Es ist schon mal nicht schlecht als Anfänger.
Du will aber auch etwas Kritik.

Versuche für die Übersichtlichkeit einen Prozesse für die Statemaschine 
und einen oder mehrere Prozesse für das Datenhändling.

das Signal "TX_Start" würde ich "wr" für write nennen. (write ist ein 
reserviertes VHDL Wort).
Das ist dann einfacher auszustauschen wenn du mal den UART gegen einen 
SPI einfach austauschen musst. Dann hast du keine spezielle Signalnamen 
in Top Level Code files.

[René D. (Firma: www.dossmatik.de) (dose)]

und dann schau dir das Beispiel für die Testbench von Lothar Miller an
Beitrag "Procedure Beispiele"


Erspart dir die Zeiten, einzeln hinzufummeln.

[Lattice User (Gast)]

Robert S. schrieb:
> greg schrieb:
>> Gibt's einen Grund, warum rx_in drei Bits breit ist? Um eine Flanke zu
>> erkennen, reichen doch zwei Bits, oder?
>
> Das stimmt eigentlich - Danke!
>

Die 3 bits sind schon ok,
1 zur Synchronisation mit der System Clock, WICHTIG
2 zur Flanken Erkennung

Man darf aber nur bit 2,1 abfragen.

        -- A start bit detected
        if rx_in(rx_in'left downto 1) = "10" then

[greg (Gast)]

Robert S. schrieb:

> Was eigentlich noch nicht ganz stimmt ist die Auswertung des Start-Bits.
> Bei fallender Flanke verlässt die Statemachine zwar IDLE, das Start-Bit
> wird aber erst danach gesampelt. Es muss ausgewertet werden, ob das 0
> ist, dann sonst war es nur ein "Wackler" auf der RX-Leitung.

Und das nennt man dann Framing Error. Genau das gleiche beim Stopbit, 
wenn es nicht passt. Das musst du aber (IMHO) nicht im Idle-Fall 
anfangen. Wenn ein Framing Error erkannt wird, geht's eben einfach 
wieder zurück zu IDLE. Ein Signal für diesen Fehlerfall wäre vielleicht 
auch interessant.

[greg (Gast)]

Lattice User schrieb:
> 1 zur Synchronisation mit der System Clock, WICHTIG

Warum eigentlich? Um Glitches zu vermeiden? Spielt das eine Rolle, wenn 
rx nur ein einziges mal pro Zyklus gesampled wird?

[Robert S. (razer)]

René D. schrieb:
> Es ist schon mal nicht schlecht als Anfänger.
> Du will aber auch etwas Kritik.

Immer her damit ;-)

René D. schrieb:
> Versuche für die Übersichtlichkeit einen Prozesse für die Statemaschine
> und einen oder mehrere Prozesse für das Datenhändling.

Danke für den Tipp. Soetwas in der Art ist mir auch schon aufgefallen. 
Es mag zwar für diese "kleine" FSM noch gehen. Bei etwas größeren wird 
das wirklich kompliziert. Wie sieht das dann genauer aus? Müsssen diese 
impliziten "Substates" ausgerollt werden? Damit meine ich zB. im State 
STATE_RX die paar if.

René D. schrieb:
> das Signal "TX_Start" würde ich "wr" für write nennen. (write ist ein
> reserviertes VHDL Wort).
> Das ist dann einfacher auszustauschen wenn du mal den UART gegen einen
> SPI einfach austauschen musst. Dann hast du keine spezielle Signalnamen
> in Top Level Code files.

Hmm. Das überzeugt mich mich nicht wirklich. Warum nicht einfach 
tx_start lassen und im Top-Level ein Signal "wr" machen? Wenn zB ein 
UART Modul und ein SPI Modul instanziert wird werden doch automatisch 
solche Namen vergeben...

René D. schrieb:
> und dann schau dir das Beispiel für die Testbench von Lothar Miller an
> Beitrag "Procedure Beispiele"

Danke für den Tipp. Echt cool :)

greg schrieb:
> Und das nennt man dann Framing Error. Genau das gleiche beim Stopbit,
> wenn es nicht passt. Das musst du aber (IMHO) nicht im Idle-Fall
> anfangen. Wenn ein Framing Error erkannt wird, geht's eben einfach
> wieder zurück zu IDLE. Ein Signal für diesen Fehlerfall wäre vielleicht
> auch interessant.

Danke für die Anregungen. Das werde ich gleich mal einbauen.

greg schrieb:
> Warum eigentlich? Um Glitches zu vermeiden? Spielt das eine Rolle, wenn
> rx nur ein einziges mal pro Zyklus gesampled wird?

Daran wäre ich auch gerne interessiert.

lg Robert

[Lothar M. (Firma: Titel) (lkmiller) (Moderator)]

greg schrieb:
> Lattice User schrieb:
>> 1 zur Synchronisation mit der System Clock, WICHTIG
> Warum eigentlich? Um Glitches zu vermeiden?
Nein, um Signalinkonsistenzen im FPGA zu vermeiden
http://www.lothar-miller.de/s9y/categories/35-Einsynchronisieren:

[Robert S. (razer)]

Ich habe einmal mein Design angepasst. Es hat nun eine Framing error 
Detection, bei der Daten mit falschem Start- oder Stopbit verworfen 
werden. Dabei wird der Error Ausgang für einen Zyklus auf 1 gesetzt.

Weiters habe ich die Testbench um eine Procedure erweitert - Danke an 
René und an Lothar!

Der Source dazu ist wieder im Anhang.

lg Robert