Erster ernster Versuch- kann mal jemand drüberschauen?

[Heiko L. (der_caveman)]

Hallo!
Das ist mein erster, ernster Versuch mit VHDL ;)

Ich habe versucht den TWI- Prescaler der Mega AVRs (48/88/168- bestimmt 
auch viele andere) nachzubauen.

Ich finde, dass das schon mal ganz toll aussieht, aber ist es das auch?

library IEEE;
use IEEE.STD_LOGIC_1164.ALL;
use IEEE.NUMERIC_STD.ALL;

entity i2c_ce_gen is
    Port ( CLK_I : in  STD_LOGIC;
           RST_I : in STD_LOGIC;
           CE_I : in  STD_LOGIC;
           TWPS_I : in  STD_LOGIC_VECTOR (1 downto 0);
           TWBR_I : in  STD_LOGIC_VECTOR (7 downto 0);
           CE_O : out  STD_LOGIC);
end i2c_ce_gen;

architecture Behavioral of i2c_ce_gen is

signal prescale : integer range 0 to 32767;
signal prescale_cmp : integer range 0 to 32767;
signal sub_prescale : integer range 1 to 64;
signal twbr_temp : integer range 0 to 255;

begin

process (CLK_I)
begin
  --[CLKSYNC] sync process
  if rising_edge(CLK_I) then
    --[RESET]sync reset
    if RST_I = '1' then
      CE_O <= '0';
      prescale <= 0;
      prescale_cmp <= 0;
      sub_prescale <= 1;
    else
      --avr style prescaler selection
      case TWPS_I is
        when "00" => sub_prescale <= 1;
        when "01" => sub_prescale <= 4;
        when "10" => sub_prescale <= 16;
        when "11" => sub_prescale <= 64;
        when others => null;
      end case;
      --avr style prescale count register
      twbr_temp <= to_integer( unsigned(TWBR_I));
      --calculation of compare value (avr style)
      prescale_cmp <= 15 + 2 * twbr_temp * sub_prescale;
      
      --[CE] to cascade prescalers
      if CE_I = '1' then
        prescale <= prescale + 1;
        --[COMP] comparator
        if prescale = prescale_cmp then
          CE_O <= '1';
          prescale <= 0;
        else
          CE_O <= '0';
        end if; --/[COMP]
      end if; --/[CE]
    end if; --/[RESET]
  end if; --/[CLKSYNC]
end process;
end Behavioral;

[Jan M. (mueschel)]

Ich würde sagen, da gibts erstmal nichts zu meckern.
Du solltest aber bei der Synthese mal nachschauen, was hieraus gemacht 
wird:
> prescale_cmp <= 15 + 2  twbr_temp  sub_prescale;
Da sub_prescale ja nur Zweierpotenzen als Wert annehmen kann, braucht es 
hier ja keinen Multiplizierer - ich weiß nur nicht, ob das 
Synthese-Programm das auch erkennt. Falls nicht, kannst du den Code 
etwas umstellen um ihm auf die Sprünge zu helfen.

[Duke Scarring (Gast)]

@Heiko Lechner:

Für den ersten Versuch sieht es echt gut aus. Hast Du auch eine 
Testbench dazu, um den Block funktional verifizieren zu können?

Duke

[Heiko L. (der_caveman)]

Jan M. wrote:

>> prescale_cmp <= 15 + 2  twbr_temp  sub_prescale;
> Da sub_prescale ja nur Zweierpotenzen als Wert annehmen kann, braucht es
> hier ja keinen Multiplizierer - ich weiß nur nicht, ob das
> Synthese-Programm das auch erkennt. Falls nicht, kannst du den Code
> etwas umstellen um ihm auf die Sprünge zu helfen.

Ok, da habe ich noch nicht dran gedacht.
Wie wird denn ein "left shift" synthetisiert (Mux, Schieberegister oder 
ganz anders)?

> Für den ersten Versuch sieht es echt gut aus.

Erster ernster Versuch ;)
Davor gab es auch schon so manchen Müll :)

> Hast Du auch eine
> Testbench dazu, um den Block funktional verifizieren zu können?

Bin ich gerade dran, die ersten Versuche sehen schon brauchbar aus.

Danke,
Heiko.

[Jan M. (mueschel)]

Heiko Lechner wrote:
> Jan M. wrote:
>
>>> prescale_cmp <= 15 + 2  twbr_temp  sub_prescale;
>> Da sub_prescale ja nur Zweierpotenzen als Wert annehmen kann, braucht es
>> hier ja keinen Multiplizierer - ich weiß nur nicht, ob das
>> Synthese-Programm das auch erkennt. Falls nicht, kannst du den Code
>> etwas umstellen um ihm auf die Sprünge zu helfen.
>
> Ok, da habe ich noch nicht dran gedacht.
> Wie wird denn ein "left shift" synthetisiert (Mux, Schieberegister oder
> ganz anders)?

Es muss nichts wirklich geschoben werden, nur die richtigen Bits 
ausgewaehlt - es gibt also einen 4-zu-1-mux, der je nach Prescaler 
entweder Bit0, 2,4 oder 6 von twbr_temp auf Bit0 von prescale_cmp legt 
(zuzueglich Addition natuerlich).

[Heiko L. (der_caveman)]

Jan M. wrote:

> Es muss nichts wirklich geschoben werden, nur die richtigen Bits
> ausgewaehlt - es gibt also einen 4-zu-1-mux, der je nach Prescaler
> entweder Bit0, 2,4 oder 6 von twbr_temp auf Bit0 von prescale_cmp legt
> (zuzueglich Addition natuerlich).

Es wird tatsächlich ein Multiplizierer generiert.

Ich habe es nun geändert zu:

--avr style prescaler selection
case TWPS_I is
  when "00" => sub_prescale <= 0; -- x<<0 == x*1
  when "01" => sub_prescale <= 2; -- x<<2 == x*4
  when "10" => sub_prescale <= 4; -- x<<4 == x*16
  when "11" => sub_prescale <= 6; -- x<<6 == x*64
  when others => null;
end case;  
--avr style prescale count register
twbr_temp <= to_integer( unsigned(TWBR_I));
--calculation of compare value (avr style)
prescale_cmp <= 15 + to_integer(to_unsigned(twbr_temp,8) sll (sub_prescale+1));

Allerdings ist die "Multiplizierer- Variante" bis auf den Multiplizierer 
ressourcensparender.

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

Das:

--avr style prescaler selection
case TWPS_I is
  when "00" => sub_prescale <= 0; -- x<<0 == x*1
  when "01" => sub_prescale <= 2; -- x<<2 == x*4
  when "10" => sub_prescale <= 4; -- x<<4 == x*16
  when "11" => sub_prescale <= 6; -- x<<6 == x*64
  when others => null;
end case;

ginge aber auch so:

  sub_prescale <=  to_integer(unsigned(TWPS_I & '0'));

[Heiko L. (der_caveman)]

Lothar Miller wrote:

> ginge aber auch so:

sub_prescale <=  to_integer(unsigned(TWPS_I & '0'));

Fällt mir jetzt erst auf :)

Kann die case Anweisung denn zu weniger "optimierten" Synthetisierungen 
führen (etwas übersichtlicher finde ich es schon)?

[Duke Scarring (Gast)]

@Heiko:
> Kann die case Anweisung denn zu weniger "optimierten" Synthetisierungen
> führen (etwas übersichtlicher finde ich es schon)?

Ja, kann. Die Wege der Synthesetools sind manchmal unergründlich :-(
Ich würde nur auf Platz optimieren, wenn es triftige Gründe gibt. Sicher 
muß man die Ressourcen nicht verschwenden, aber bei 
Entwicklungsprojekten, wo es nicht auf Masse oder Platz, sondern auf 
Zeit ankommt, hat man einfach keine Zeit ewig rumzuoptimieren.

Lieber lesebarer hinschreiben, da hat man später (manchmal) mehr davon.

Duke

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

> (etwas übersichtlicher finde ich es schon)?
Weil TWPS_I nur 4 Zustände hat und alle im case auscodiert wurden, 
solltest du aber das

when others => null;

 aus dem case herauslassen. Sonst könnte man ja meinen, da gäbe es noch 
andere (nicht abgedeckte) Zustände. Und das wäre dann doch etwas 
unübersichtlich ...  ;-)

[Duke Scarring (Gast)]

@Lothar:
Die anderen Zustände gibt es doch, oder wolltest Du alles hinschreiben:

when "ZZ" => ...;
when "U1" => ...;
when "LH" => ...;
...

:-)

Duke

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

>  when "ZZ" => ...;
>  when "U1" => ...;
>  when "XU" => ...;
>  when "LZ" => ...;
>  when "LH" => ...;
Auf die Signalzustände wollte ich schon immer mal abfragen  ;-)