Hi,
i have an exercise to solve. The specifications are this:
-input serial_in that receives the input bit
-input clock 1200 HZ
-input reset
-output ready
-output parallel_out, a word of 16 bit

the input serial_in read 300 bit/s and present 16 bit on the 
parallel_out and set ready = 1. The convertitor remains in this state 
until reset = 1 and clock = 1 (rising edge), in this case starts a new 
conversion.

I write the following VHDL code. I would to know if it's a correct and a 
possible solution:

 
library ieee;
use ieee.std_logic_1164.all;

entity convertitore is
  port(serial_in, clock, reset: in std_logic;
      parallel_out: out std_logic_vector (15 downto 0);
      ready: out std_logic);
end entity convertitore;

architecture device of convertitore is
  variable output: std_logic_vector(15 downto 0);
  begin
  
    clock: process
    begin
      clock <= '0', '1' after 416.5 ms;
      wait for 833 ms;
    end process clock;
    
    main: process(clock, reset)
    begin
      if(ready /= '1') 
      then
        for i in 0 to 15 loop
          output(i) := serial_in;
          wait for 3.5 ms;
        end loop;
        parallel_out <= output;
        ready <= '1';
      end if;
      
      if(clock'event and clock ='1') then
        if(reset'event and reset = '1') then 
          ready <= '0';
        end if;
      end if;
    end process main;
  end architecture device;

Thank you

I need to generate the clock, now i separate a "clock generator" and i 
understand the error.
I also understand that in my case i don't need the sensitivity list, 
because i check inside the process the value of clock and reset.

i didn't undestand the error on loop....

Maybe i can use a internal clock for the 300 bit/s, then i use a process 
triggered by rising time of this internal clock without using a for 
loop?

Something like this:
"i" is a integer counter initialized to 0
"int_clock" is the internal clock for 300 bit/s
"clock" is the clock input

 
   main: process
    begin
      if(ready /= '1') 
      then
        if(int_clock'event and int_clock = '1') then
          output(i) := serial_in;
          i := i + 1;
        end if;
        if(i = 15) then
          parallel_out <= output;
          ready <= '1';
          i := 0;
        end if;
      end if;
      
      if(clock'event and clock ='1') then
        if(reset'event and reset = '1') then 
          ready <= '0';
        end if;
      end if;
    end process main;

Giuseppe R. wrote:
> i didn't undestand the error on loop....
Which error?

The major problem here is, that you are "programming" VHDL like a 
procedural programming language (e.g. C, Basic, Java). That will not 
work. Never!

VHDL is a hardware DESCRIPTION language. And to DESCRIBE something you 
must have a picture of it.

Start with a simple counter mapped to LEDs, simulate it and transfer it 
to hardware. Have a look at the RTL-schematic. It shows you, how the 
synthesizer did understand your DESCRIPTION.
Then change the counters code to count up while a button is pressed and 
to count down while the button is released.
Then sync in and debounce the button and use it to count up your 
counter.
Validate your solutions always with a testbench before going on 
hardware, and have a look at the RTL schmematics.


A few words to some line of your code:
Have a look how a testbench invokes a "device under test". You will need 
knowledge about components for that.

This here is nuts:

      if(clock'event and clock ='1') then
        if(reset'event and reset = '1') then 
          ready <= '0';

This will never go into real hardware, because it is impossible in two 
ways:
1. never ever you will have two rising edges of two signals at the 
very same moment.
2. there is no "double clock flipflop" with two seperate clock inputs on 
any FPGA/CPLD.

Having a second look, the whole code here is nuts:

   main: process
      begin
      ...
   end process main;

Its a process with no sensitivity list, it invokes three clocks, a latch 
for parallel out and last, but not least: an async combinatorial reset 
for ready. Each of them is bad, all of them together a valuable reason 
for "you are fired!"  ;-)


Giuseppe R. wrote:
> -input clock 1200 HZ
> the input serial_in read 300 bit/s
Is the serial data synchronous to the input clock?
Looks like with every 4 clock cycles 1 data bit comes in.

Whats the systems clock? 1200Hz? Or xxMHz (with xx in 10..100)?
Whats the target hardware? FPGA? CPLD?


BTW:
Forget loops and variables. You don't need them here in this simple 
project.

Can i simulate my code in hardware with Quartus prime? Or i need a 
physical hardware? I'll write the solution on a paper during the exam, i 
don't have the simulator....

How can i use the same clock (or a fraction of it) for two different 
purpose?

Yes clock is 1200Hz. I could count the rising edge and when i reach the 
fourth i'll read the input? In this way i need a variable. I don't 
understand why i can't use variable also on a simple project. If the 
specifications tell me that i'll present the output after reading 16 bit 
i need to save this 16 bit and THEN present it on parallel out.

Giuseppe R. wrote:
> Can i simulate my code in hardware with Quartus prime?
You get some kind of ModelSim with it.

> Or i need a physical hardware?
As I said: physical hardware i the last step.

> In this way i need a variable.
Aha, the software programmer.
I myself (as the hardware guy) would use some flipflops to store values. 
Those flipflops arranged together in a proper manner result in a 
counter. And that ist, what you need.

> In this way i need a variable.
Be sure: a variable in VHDL is not what you need. A variable in VHDL 
behaves somewhat strange to software programmers.

Meanwhile I did a little fingertwist and it resultet in the attachments. 
As expected the synthesizer says it eats up 23 flipflops: 16 for the 
parallel_out, 4 for the bit counter 0..15, 2 for the cycle counter 0..3 
and one for the ready-flag.

Don't try to serve this pieces of code to your teacher. She/he will get 
distrustful. Just use it as a "howto" to get the idea...  ;-)

Of course you can use my testbench to check your design. Thats the 
trick: when the testbench is ok, you simply must get your module to go 
through without an error.

Thank you. I had already corrected my code like sipo.vhd with the only 
difference that I used variables instead of signals. So every time I 
think of using a variable I could replace it with a signal? Will this 
serve for the next test bench? Probably we used variables because it's 
not required the test bench during the exam, but i know that is not a 
good cause.

I'm a computer science graduate, that's probably why I struggle a bit ;(

Giuseppe R. wrote:
> I had already corrected my code like sipo.vhd
Just being a little bit nosy: can you post that code? ;-)

> with the only difference that I used variables instead of signals.
> So every time I think of using a variable I could replace it with a signal?
> Will this serve for the next test bench?
Of course you can do the same with variables. This code together with 
the test bench from yesterday gives the very same output:

architecture Behavioral2 of sipo is
begin
   process (clock, reset)
   variable ccnt : integer range 0 to 3  := 0; -- cycle counter for dividing the clock by 4
   variable bcnt : integer range 0 to 15 := 0; -- bit counter for the 16 bits
   variable rdy : std_logic := '0';
   begin
      if reset='1' then
         rdy   := '0';
         ccnt  := 2;   -- in the middle of those 4 clock cycles we have a stable signal
         bcnt  := 0;
      elsif rising_edge(clock) then
         if rdy='0' then  -- not yet finished?
         
            if ccnt<3 then         -- do we have enough clocks for the next bit?
               ccnt := ccnt+1;     -- no: count on 
            else                   -- yes: 
               ccnt := 0;          -- reset cycle counter
               parallel_out(bcnt) <= serial_in; -- store the bit
               if bcnt=15 then     -- do we handle the last bit?
                  rdy := '1';      -- yes: were done!
               else 
                  bcnt := bcnt+1;  -- no: increment index
               end if;
            end if;
            
         end if;
      end if;
      ready <= rdy;        -- map internal ready signal to output
   end process;
   
end Behavioral2;

The major drawback with variables is, that they are invisible outside 
the process.

> Probably we used variables because it's not required
> the test bench during the exam, but i know that is not a good cause.
Particularly for learning VHDL the use of the simulator is absolutely 
necessary. Its the FPGA debugger.

This is my code. I use also a variable for the output because if i need 
to present the output only when i read the 16 bit, i can't store the 
single bit directly in the output, isn't it?

 
library ieee;
use ieee.std_logic_1164.all;

entity convertitore is
  port(serial_in, reset, clock: in std_logic;
      parallel_out: out std_logic_vector (15 downto 0);
      ready: out std_logic);
end entity convertitore;

architecture device of convertitore is
  variable output: std_logic_vector(15 downto 0);
  variable count: integer range 0 to 15 := 0;
  variable rising: integer range 0 to 4 := 0;
  
  begin
    main: process(clock)
    begin
      if(clock'event and clock = '1') then
        if(reset = '1') then 
          ready <= '0';
        end if;
        rising := rising + 1;
      end if;
      if(ready /= '1' and rising = 4) then
        output(i) := serial_in;
        rising := 0;
        if(i = 15) then
          parallel_out <= output;
          ready <= '1';
          i := 0;
        else
          i := i + 1;
        end if;
      end if;
    end process main;
  end architecture device;

Few words to your code (which would result in a reasonably bad grade 
when I was your teacher...;-)

Its a bad idea to combine synchronous and asynchonous parts in 1 
process:

    main: process(clock)
    begin
      if(clock'event and clock = '1') then -- clocked section
         :
      end if;
      if(ready /= '1' and rising = 4) then -- combinatorial section
         : 
      end if;
    end process main;

For sure you will not see this coding style in any book.

Beside the fact, that you cannot read back the output port "ready" you 
have a incomplete sensitivity list due to that special coding style. 
Therefore the simulation will not match the reality.

Much worse is, that you have a combinatorial loop on the variable i. And 
the simulator is not possible to show it. Because i should be in the 
sensitivity list, but variables cannot be taken in there, because 
variables must be declared in the process.

After having fixed the very obviously errors I get something to 
simulate:

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

entity sipo is
  port( serial_in, clock, reset: in std_logic;
        parallel_out: out std_logic_vector (15 downto 0);
        ready: out std_logic);
end sipo;

architecture device of sipo is
signal rdy : std_logic := '0';
begin
    main: process(clock)
    variable output: std_logic_vector(15 downto 0);
    variable i: integer range 0 to 15 := 0;
    variable rising: integer range 0 to 4 := 0;
    begin
      if(clock'event and clock = '1') then
        if(reset = '1') then 
          rdy <= '0';
        end if;
        rising := rising + 1;
      end if;
      if(rdy /= '1' and rising = 4) then
        output(i) := serial_in;
        rising := 0;
        if(i = 15) then
          parallel_out <= output;
          rdy <= '1';
          i := 0;
        else
          i := i + 1;
        end if;
      end if;
    end process main;

    ready <= rdy; -- map internal ready signal to output

end architecture device;

So having fixed most of the "problems" the simulation looks nearly fine 
at first sight, but from the synthesizer you will get a error like:
ERROR:Xst:1534 - Sequential logic for node <rdy> appears to be 
controlled by multiple clocks.

And this is due to trying to write to "rdy" from a hidden combinatorial 
loop and from a synchronous reset.

So to sum up the two days: you cannot learn VHDL without pressing the 
"simulate" or at least the "synthesize" button now and then. And without 
having a look for the resulting messages...


BTW: pls use the [vhdl ] tags instead of the [c ] tags.