Welcome to EDAboard.com

Welcome to our site! EDAboard.com is an international Electronic Discussion Forum focused on EDA software, circuits, schematics, books, theory, papers, asic, pld, 8051, DSP, Network, RF, Analog Design, PCB, Service Manuals... and a whole lot more! To participate you need to register. Registration is free. Click here to register now.

Register Log in

[SOLVED] Systematic Cyclic Encoder in VHDL

Status
Not open for further replies.

TrickyDicky

Advanced Member level 5
Joined
Jun 7, 2010
Messages
6,991
Helped
2,055
Reputation
4,127
Reaction score
2,005
Trophy points
1,393
Activity points
38,349
vGoodtimes
I have another question about this sentece that you mentioned earlier.
"RTL developers prefer to have modules with registered outputs and inputs when possible."

So you mean that I have to store Utemp (which is a serial input) in A FF first?
and then just use the FF instead?
it is like causing a delay (for one clock pulse)

is that what mean?
That is exactly correct.

When designing FPGAs you usually dont care much about latency, as long as pipeline lengths are all matched. It's usually the throughput thats the most important thing.
 
  • Like
Reactions: Morell

    Morell

    points: 2
    Helpful Answer Positive Rating

std_match

Advanced Member level 4
Joined
Jul 9, 2010
Messages
1,083
Helped
408
Reputation
816
Reaction score
404
Trophy points
1,363
Location
Sweden
Activity points
8,260
vGoodtimes
I have another question about this sentece that you mentioned earlier.
"RTL developers prefer to have modules with registered outputs and inputs when possible."

So you mean that I have to store Utemp (which is a serial input) in A FF first?
and then just use the FF instead?
it is like causing a delay (for one clock pulse)

is that what mean?
Since the architecture of modern FPGAs is combinatorial logic followed by registers, it is more important to have registers on the outputs.
This is done by driving all outputs from a clocked process. Your code in post #16 doesn't drive SentU from a clocked process, so it will be driven by combinatorial logic.
Fixing that is more important than storing (delaying) Utemp in an FF.
 
  • Like
Reactions: Morell

    Morell

    points: 2
    Helpful Answer Positive Rating

vGoodtimes

Advanced Member level 4
Joined
Feb 16, 2015
Messages
1,070
Helped
304
Reputation
608
Reaction score
301
Trophy points
83
Activity points
8,520
vGoodtimes
I have another question about this sentece that you mentioned earlier.
"RTL developers prefer to have modules with registered outputs and inputs when possible."

So you mean that I have to store Utemp (which is a serial input) in A FF first?
and then just use the FF instead?
it is like causing a delay (for one clock pulse)

is that what mean?
It is part. As std_match mentions, registered outputs are more important. As TrickyDicky mentions, latency is typically not an issue as long as things are done correctly.

I'll expand on the registered in/out a bit:
Inputs are not always registered. The most common case would be a "qualified enable" that is modified. This isn't ideal, but is common. I prefer to comment known unregistered inputs.
Outputs are almost always registered. There are some cases where they cannot be registered, but this is rare. I prefer to name known unregistered outputs specially.
In some cases, a module will have an unregistered input that gets used with an unregistered output (and has non-trivial logic). These should be discouraged unless the module is intended to represent combinatorial logic only, or there is suitable commentary on why this is needed.

Now for the pipeline:
The circuit you have right now is an example of the RTL version of "tight-coupling". The source provides an input and expects an output to be valid after a specific number of cycles. This complicates the interface, and makes it less flexible. The user instantiating the module must know extra information about the implementation, and the implementor cannot make changes as easily.

For this case, it would be better if the module had a (DataIn, ValidIn) input, and generated a (DataOut, ValidOut) output. The source of the module could then specify when data is ready, and the destination of the module no longer cares about the latency. In this case "ValidIn" is the "qualified clock-enable" I mentioned earlier. It means that DataIn contains valid data on this cycle. It will likely be used as a clock-enable on the FFs. ValidOut is ValidIn delayed by the correct number of cycles, as well as being set to '1' for the shifting out of the ECC bits. You can also add other useful signals in if you want.

With an interface like this, adding registers becomes easy. External modules aren't making assumptions about the delay through your module. You should attempt to match pipeline delays for the outputs, and comment or name intentionally misaligned outputs. (also be aware of the difference between a delay of one cycle, and a delay of one ValidIn qualified cycle.)
 
  • Like
Reactions: Morell

    Morell

    points: 2
    Helpful Answer Positive Rating

Morell

Member level 1
Joined
Dec 1, 2015
Messages
35
Helped
0
Reputation
0
Reaction score
0
Trophy points
6
Activity points
614
Hi, guys
Thank you for ur advises

Here is the new code


Code VHDL - [expand]
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
library IEEE;
use IEEE.STD_LOGIC_1164.ALL;
 
entity OEncoder_3 is
    Generic (: integer range 0 to 10 :=4;-- K bits for Message
                 N  : integer range 0 to 20 :=7);-- N bits for Codeword 
             
    Port ( 
              Data_in : in STD_LOGIC;
              Data_out : out STD_LOGIC;
              
           Clk : in  STD_LOGIC;
              Reset : in STD_LOGIC;
              
              Valid_in : in STD_LOGIC;
              Valid_out : out STD_LOGIC);
              
end OEncoder_3;
 
architecture Behavioral of OEncoder_3 is
 
--Constant Declaration
Constant GP : STD_LOGIC_VECTOR ((N-K) downto 0) :="1011";   -- Generator Polynomial of (N-K) Degree
 
--Signal Declaration
Signal D,Q : STD_LOGIC_VECTOR ((N-K-1) downto 0) :=(Others => '0'); -- Flip flop's Inputs
Signal ClockCounter : integer range 0 to N;
Signal GTemp,UQX : STD_LOGIC;
Signal Input_Buffer : STD_LOGIC;
 
 
 
Type Switch is ( Parity , message );
Signal Switch2 : Switch := Message;
 
begin
 
 
--Combinatorial Part
    
--1)- taking care of FF's Input and XORs
--***CHECKED***
    Gen1:for i in 1 to N-K-1 generate 
        D(i) <= (Gtemp xor Q(i-1)) when GP(i)='1' else
                     Q(i-1);
    end generate;
    D(0) <= Gtemp;
------------------------------------------------------------------------------------------- 
 
--2) taking care of FF's Outputs
--***CHECKED***
    UQX <= (Input_Buffer xor Q(N-K-1)) When (Input_Buffer = '0' or Input_Buffer='1') else '0';
--------------------------------------------------------------------------------------------
    
--3) taking care of GATE
--***CHECKED*** 
    Gtemp <= UQX when Switch2 = Message else '0'; -- Gtemp <= UQX and Switch2
--------------------------------------------------------------------------------------------
 
-- taking care of Switch 2
--4)***CHECKED***
    Data_out <= Input_Buffer When Switch2 = Message else Q(N-K-1);
--------------------------------------------------------------------------------------------
                
-- Sequential part 
 
Process(Clk,Reset)
begin
 
    if( Reset = '0') then
 
        Q <= (Others => '0');
        ClockCounter <= 0;
        Switch2 <= Message;
        Input_Buffer <= '0';
        
   elsif (rising_edge(Clk) and Valid_in ='1' )then
        
            
            Input_Buffer <= Data_in;
            Valid_out <= '1';
            Q <= D;
            
            if (Input_Buffer = '0' or Input_Buffer = '1') then 
                if (ClockCounter < N - 1) then
                    ClockCounter <= ClockCounter + 1;
                elsif (ClockCounter = N - 1) then 
                    Valid_out <= '0';
                end if;
            end if;
            
            if (ClockCounter = K - 1) then 
                Switch2 <= Parity;
            end if;
 
  end if;
end process;
end Behavioral;

 
Last edited by a moderator:

vGoodtimes

Advanced Member level 4
Joined
Feb 16, 2015
Messages
1,070
Helped
304
Reputation
608
Reaction score
301
Trophy points
83
Activity points
8,520
Not quite there yet.

If you have time, try writing code for this:

Code:
-- This process will either drive "Data_out" or "Data_buf" where "Data_out <= Data_buf;" is outside this process.
-- Data_buf is used when bits of Data_out are used within this entity.  VHDL doesn't allow outputs to be used within the same architecture.
Process(Clk,Reset)
    -- any variables for purely combinatorial logic here.
begin
 
    if( Reset = '0') then
        -- all signals assigned in this process given constant default values here.    
    elsif (rising_edge(Clk)) then
        -- if you had any variables declared, use them here.  Ensure they do no generate registers -- never use a variable before it is assigned.

        -- default assignments here
        Valid_out <='0'; -- default to '0'.

        -- The core logic
        if (Valid_in = '1') then
            -- logic for the "input is valid case here"
        elsif (Switch2 = Parity) then
            -- logic for the parity case here
        end if;
   
    end if;
end process;
 
  • Like
Reactions: Morell

    Morell

    points: 2
    Helpful Answer Positive Rating

Morell

Member level 1
Joined
Dec 1, 2015
Messages
35
Helped
0
Reputation
0
Reaction score
0
Trophy points
6
Activity points
614
Hi,
vGoodtimes

Do you mean my code is still wrong?!!
-I simulated it and it worked.
-I didn't senthesized it.

U think my code doesn't work on (for example spartan-3) a board?
 

TrickyDicky

Advanced Member level 5
Joined
Jun 7, 2010
Messages
6,991
Helped
2,055
Reputation
4,127
Reaction score
2,005
Trophy points
1,393
Activity points
38,349
You code is not "wrong", it just doesnt quite follow the better practice techniques.
1. Your code has the enable directly anded with the clock. This, in theory, is a gated clock, which is not what you want in an FPGA (you may want it in an ASIC, but it isnt used much). You should move the enable inside the clock branch. The synthesisor will probably create a synchronous enable - but be warned it may created a gated clock.

2. vGoodtime says you cannot read outputs inside the same entity - this is true for VHDL 2002 and older, but with 2008 this is possible (as it matches verilog behaviour). In older revisions of the language, the "more acceptable" technique was to have an internal signal that was directly assigned to the output. The other method is to declare the output you want to read as a buffer:

output_to_read : buffer std_logic;

But this has connectivity implications when you come to wire up the circuit (only in VHDL land) so its not so recommended. Most compilers support 2008 now so reading outputs should be fine.

3. He also recommends not to use variables as registers. This is also something that many people "recommend" rather than it being a rule - its because variables are more prone to issues, so many engineers (and coding guidelines) dont like using variables for anything other than combinatorial logic. So as a beginner, steer clear of variables for anything other than comb logic for now.

4. There is also the issue that you have signals assigned in the clocked part of the process and not in the reset. This will mean that the async reset will be used as part of the clock enable on that signal, so when reset = '1' it will not set that register. This is possibly bad because the reset could be asserted at any point, and violate a setup time. There are two ways to fix this:
a) Assign all signals in the process in the reset.
b) use the "post clock reset" template for your code:

Code:
process(clk, reset)
begin
  if rising_edge(clk) then
    --synchronous code here
  end if;

  if reset = '1' then
    --reset only things you really want to reset here
  end if;
end process;
This means you can mix up signals with and without async reset in the same process, without generating extra clock enables. Again, you'll probably get resistance from many engineers because it doesnt conform to the "correct" template - but it works perfectly fine in all synthesisors.
 
  • Like
Reactions: FvM and Morell

    Morell

    points: 2
    Helpful Answer Positive Rating

    FvM

    points: 2
    Helpful Answer Positive Rating

vGoodtimes

Advanced Member level 4
Joined
Feb 16, 2015
Messages
1,070
Helped
304
Reputation
608
Reaction score
301
Trophy points
83
Activity points
8,520
#1 -- It is not correct either. This was my comment about cycle-delay vs qualified-delay. In this design, there are 7 output bits per 4 input bits. This means only asserting Valid_out as a result of Valid_in will not be correct.

#2 -- I actually haven't done HDL for the past year. You brighten my day by suggesting VHDL-2008 has been implemented in 2015 instead of 2051...

#3 -- My rule is that you can use variables for registers, but the intent must be clear. This is my general rule -- anything unusual that you want in code should be understood and explained.

#4 -- This is very strongly my style as well, but I don't see it often. I fear suggesting it for the reasons TD mentions -- that potential interviewers might not approve...
 
  • Like
Reactions: Morell

    Morell

    points: 2
    Helpful Answer Positive Rating

std_match

Advanced Member level 4
Joined
Jul 9, 2010
Messages
1,083
Helped
408
Reputation
816
Reaction score
404
Trophy points
1,363
Location
Sweden
Activity points
8,260
4. There is also the issue that you have signals assigned in the clocked part of the process and not in the reset. This will mean that the async reset will be used as part of the clock enable on that signal, so when reset = '1' it will not set that register. This is possibly bad because the reset could be asserted at any point, and violate a setup time. There are two ways to fix this:
a) Assign all signals in the process in the reset.
b) use the "post clock reset" template for your code:

Code:
process(clk, reset)
begin
  if rising_edge(clk) then
    --synchronous code here
  end if;

  if reset = '1' then
    --reset only things you really want to reset here
  end if;
end process;
This means you can mix up signals with and without async reset in the same process, without generating extra clock enables. Again, you'll probably get resistance from many engineers because it doesnt conform to the "correct" template - but it works perfectly fine in all synthesisors.
The same style is also good for processes with synchronous reset.
Standard template:
Code:
process(clk, reset)
begin
  if rising_edge(clk) then
    if reset = '1' then
      -- Synchronous reset
      -- This style is only good if all signals are resetted here
    else
      -- Synchronous code here
    end if;
  end if;
end process;
The problem is that extra logic can be required to keep non-resetted signals unchanged when reset is applied.

Better template:
Code:
process(clk, reset)
begin
  if rising_edge(clk) then
    -- Synchronous code here

    if reset = '1' then
      -- Synchronous reset
      -- Skip signals that don't need to be resetted
    end if;
  end if;
end process;
 
  • Like
Reactions: Morell

    Morell

    points: 2
    Helpful Answer Positive Rating

FvM

Super Moderator
Staff member
Joined
Jan 22, 2008
Messages
46,758
Helped
13,880
Reputation
28,008
Reaction score
12,519
Trophy points
1,393
Location
Bochum, Germany
Activity points
272,893
4. There is also the issue that you have signals assigned in the clocked part of the process and not in the reset. This will mean that the async reset will be used as part of the clock enable on that signal, so when reset = '1' it will not set that register. This is possibly bad because the reset could be asserted at any point, and violate a setup time.
I don't realize at first sight which problem you are referring to. Where do you see an unintended clock enable?

Possible violation of setup times by an asynchronous reset is a general problem which can be only avoided by synchronizing the reset. Without an explicit reset, the problem is shifted to possible POR to clock setup violations.
 
  • Like
Reactions: Morell

    Morell

    points: 2
    Helpful Answer Positive Rating

ads-ee

Super Moderator
Staff member
Joined
Sep 10, 2013
Messages
7,531
Helped
1,763
Reputation
3,532
Reaction score
1,707
Trophy points
113
Location
USA
Activity points
55,484
I don't realize at first sight which problem you are referring to. Where do you see an unintended clock enable?

Possible violation of setup times by an asynchronous reset is a general problem which can be only avoided by synchronizing the reset. Without an explicit reset, the problem is shifted to possible POR to clock setup violations.
Interpretation ambiguity like this is why I don't code using a single if with a bunch of unrelated signals all munged together. I separate reset and non-reset code in separate process/always code and keep signals in those process/always separated in their own if statements. Mixing stuff just makes things more complicated IMO, and decreases readability (and hence maintainability).

So far in all the years I've been working, nobody has ever come back to me after I've left and said that they couldn't figure out my code. Those that have picked up my code have commented that it's some of the easiest to read and modify they've come across.
 
  • Like
Reactions: Morell

    Morell

    points: 2
    Helpful Answer Positive Rating

std_match

Advanced Member level 4
Joined
Jul 9, 2010
Messages
1,083
Helped
408
Reputation
816
Reaction score
404
Trophy points
1,363
Location
Sweden
Activity points
8,260
I don't realize at first sight which problem you are referring to. Where do you see an unintended clock enable?
Look at the "standard" template for a clocked process with asynchronous reset:
Code:
process(clk, reset)
begin
    if reset = '1' then
      -- Asynchronous reset
      -- This style is only good if all signals are resetted here
    elsif rising_edge(clk) then
      -- Synchronous code here
    end if;
  end if;
end process;
The problem is that the signals missing in the reset part must keep their values when reset is active. This means having a clock enable or putting extra logic in the data path to feed the output value back to the input for each non-resetted signal. It is therefore better to do what Tricky Dicky suggests:

Code:
process(clk, reset)
begin
  if rising_edge(clk) then
    --synchronous code here
  end if;

  if reset = '1' then
    --reset only things you really want to reset here
  end if;
end process;
This code gives the same result as the "standard" template when all signals are resetted, so it is a much better template.
 
  • Like
Reactions: Morell and FvM

    FvM

    points: 2
    Helpful Answer Positive Rating

    Morell

    points: 2
    Helpful Answer Positive Rating

Tetik

Member level 4
Joined
May 29, 2014
Messages
74
Helped
20
Reputation
40
Reaction score
20
Trophy points
8
Activity points
437
Just one question. Does the signal reset needs to be released synchronously (managed in another process)?

Code:
Code:
process(clk, reset)
begin
  if rising_edge(clk) then
    --synchronous code here
  end if;

  if reset = '1' then
    --reset only things you really want to reset here
  end if;
end process;
Thanks
 
  • Like
Reactions: Morell

    Morell

    points: 2
    Helpful Answer Positive Rating

std_match

Advanced Member level 4
Joined
Jul 9, 2010
Messages
1,083
Helped
408
Reputation
816
Reaction score
404
Trophy points
1,363
Location
Sweden
Activity points
8,260
Just one question. Does the signal reset need to be released synchronously (managed in another process)?
Normally, the release should be synchronous. There are cases where it doesn't matter, but the analysis will probably cost more than going safe with synchronous release (which will only cost 2 registers).
 
  • Like
Reactions: Tetik

    Tetik

    points: 2
    Helpful Answer Positive Rating

Morell

Member level 1
Joined
Dec 1, 2015
Messages
35
Helped
0
Reputation
0
Reaction score
0
Trophy points
6
Activity points
614
What about this one?


Code VHDL - [expand]
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
entity OEncoder_3v2 is
    Generic (: integer range 0 to 10 :=4;-- K bits for Message
                 N  : integer range 0 to 20 :=7);-- N bits for Codeword 
             
    Port ( 
              Data_in : in STD_LOGIC;
              Data_out : out STD_LOGIC;
              
           Clk : in  STD_LOGIC;
              Reset : in STD_LOGIC;
              
              Valid_in : in STD_LOGIC;
              Valid_out : out STD_LOGIC);
end OEncoder_3v2;
 
architecture Behavioral of OEncoder_3v2 is
 
--Constant Declaration
Constant GP : STD_LOGIC_VECTOR ((N-K) downto 0) :="1011";   -- Generator Polynomial of (N-K) Degree
 
--Signal Declaration
Signal D,Q : STD_LOGIC_VECTOR ((N-K-1) downto 0) :=(Others => '0'); -- Flip flop's Inputs
Signal ClockCounter : integer range 0 to N;
Signal GTemp,UQX : STD_LOGIC;
Signal Input_Buffer,Vin_Buffer,Vout_Buffer : STD_LOGIC;
 
 
 
Type Switch is ( Parity , message );
Signal Switch2 : Switch := Message;
 
begin
 
--Combinatorial Part
    
--1)- taking care of FF's Input and XORs
--***CHECKED***
    Gen1:for i in 1 to N-K-1 generate 
        D(i) <= (Gtemp xor Q(i-1)) when GP(i)='1' else
                     Q(i-1);
    end generate;
    D(0) <= Gtemp;
------------------------------------------------------------------------------------------- 
 
--2) taking care of FF's Outputs
--***CHECKED***
    UQX <= (Input_Buffer xor Q(N-K-1)) When (Vin_Buffer = '1') else '0';
--------------------------------------------------------------------------------------------
    
--3) taking care of GATE
--***CHECKED*** 
    Gtemp <= UQX when Switch2 = Message else '0'; -- Gtemp <= UQX and Switch2
--------------------------------------------------------------------------------------------
 
-- taking care of Switch 2
--4)***CHECKED***
    Data_out <= Input_Buffer When Switch2 = Message else Q(N-K-1);
--------------------------------------------------------------------------------------------
-- taking care of Valid Out
--5)***CHECKED***
    Valid_out <= Vout_Buffer;
-- Sequential part Va
 
Process(Clk,Reset)
begin
    if rising_edge(clk) then 
        if reset='1' then 
            -- reset all assigned signals here
            Q <= (Others => '0');
            ClockCounter <= 0;
            Input_Buffer <= '0';
            Vout_Buffer <= '0';
            Vin_Buffer <= '0';
            Switch2 <= Message;
        else
            -- Default assignments first
            Q <= D;
            Input_buffer <= Data_in;
            Vin_Buffer <= Valid_in;
            
            if (Vin_Buffer = '1') then 
                if (ClockCounter < N - 1) then
                    ClockCounter <= ClockCounter + 1;
                    Vout_Buffer <= '1';
                elsif (ClockCounter = N - 1) then 
                    Vout_Buffer <= '0';
                end if;
            end if;
            
            if (ClockCounter = K-1) then 
                Switch2 <= Parity;
            end if;
 
        end if;--Reset
    end if;--Clock
end Process;
end Behavioral;

 
Last edited by a moderator:

ads-ee

Super Moderator
Staff member
Joined
Sep 10, 2013
Messages
7,531
Helped
1,763
Reputation
3,532
Reaction score
1,707
Trophy points
113
Location
USA
Activity points
55,484
My take on how this should have been formatted and some embedded comments on what you wrote.

Code VHDL - [expand]
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
Process(Clk,Reset)
begin
    if rising_edge(clk) then 
 
        -- data WHY would you reset the data!?
        -- You have a valid it doesn't need a reset.
        if reset = '1' then
            Q <= (others => '0');
            Input_Buffer <= '0';
            Vin_Buffer <= '0';
        else
            Q <= D;
            Input_buffer <= Data_in;
            Vin_Buffer <= Valid_in;
        end if;
 
        -- your counter doesn't count after it reaches N-1
        -- it saturates at N-1 and stays there forever.
        if reset = '1' then
            ClockCounter <= 0;
        elsif (Vin_Buffer = '1') then
            if (ClockCounter < N - 1) then
                ClockCounter <= ClockCounter + 1;
            end if;
        end if;
 
        -- Generate an output valid signal for next stage
        if reset = '1' then
            Vout_Buffer <= '0';
        elsif (Vin_Buffer = '1') then 
            if (ClockCounter < N - 1) then
                Vout_Buffer <= '1';
            else
                Vout_Buffer <= '0';
            end if;
        end if;
 
        -- So you hammer on the reset for everything to load a message?
        -- you shouldn't call this a reset it's really a load_msg signal
        -- you also need to learn how to name things that make sense to
        -- someone else that will have to read the code in the future.
        -- like Vout_Buffer implies to me it's a buffered Vout not a registered
        -- Valid output...Valid_reg makes more sense to me if I was glancing
        -- at the code.
        if reset = '1' then
            Switch2 <= Message;
        elsif (ClockCounter = K-1) then 
            Switch2 <= Parity;
        end if;
 
    end if;--Clock
end Process;



- - - Updated - - -

If you look at the way I rewrote your code it's easy to see how each signal is generated there is no ambiguity of how the code should be interpreted and if you decide to remove the reset from the data there is no subtle problems to worry about as the control logic is not embedded in a single if statement.

I'm sure some of the VHDL experts on this forum will have issues with my method (too long), but from my experience I'd rather get code like this to work on than the dense, layered code that takes me 5-10 minutes of study to figure out all the relationships between the various layers of if statements and conditional code based on the position of the resets and default statements.

IMO, coders who do all that are just trying to show off and aren't interested in making something simple and easy to follow (for both co-workers left picking up your code and the synthesis tools, which won't have to do anything but translate the simple boiler plate template like code.

YMMV
 

Morell

Member level 1
Joined
Dec 1, 2015
Messages
35
Helped
0
Reputation
0
Reaction score
0
Trophy points
6
Activity points
614
Hi,
thanks alot ads_ee, for your tips and advices

I will try to do this coding thing your way, but right now
all I care is to get some answers.
Because it's my final project and my Professor doesn't read the code,
he just checks the inputs and outputs.
 

TrickyDicky

Advanced Member level 5
Joined
Jun 7, 2010
Messages
6,991
Helped
2,055
Reputation
4,127
Reaction score
2,005
Trophy points
1,393
Activity points
38,349
Hi,
thanks alot ads_ee, for your tips and advices

I will try to do this coding thing your way, but right now
all I care is to get some answers.
Because it's my final project and my Professor doesn't read the code,
he just checks the inputs and outputs.
The problem is most professors are terrible at HDL. If they do read the code, or have a model answer, it was usually written many years ago using styles that are far out of date. Most beginners start using these out of date examples and end up on here.
 

Morell

Member level 1
Joined
Dec 1, 2015
Messages
35
Helped
0
Reputation
0
Reaction score
0
Trophy points
6
Activity points
614
Yessssssssssssssssssssssssssssss
that's 10000000000% right!!!!!
:lol:
 

Morell

Member level 1
Joined
Dec 1, 2015
Messages
35
Helped
0
Reputation
0
Reaction score
0
Trophy points
6
Activity points
614
Hi

This is the latest version of my encoder module.

1-This is my questions :

a)Does this code works on a board (Xilinx Spartan-3)?

b)If it doesn't, What do you think the problem is?

c)How can I solve the problem and make it work (Implement on a board)?

2- I simulate it and it worked (as expected!!!).

3- Attached file is going to say what the code should do.

4- Others things (Readability, Modularity and so on) are not important right now.
Specially if the code works on a board.

5- This module (Encoder_3v2) is a component in another module (Encode).
And right now Encode is not the major part because I didn't finished Encoder_3v2.
But I would like to know your suggestions and tips about the design of the Top module (Encode)


Code VHDL - [expand]
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
entity OEncoder_3v2 is
    Generic (: integer range 0 to 10 :=4;-- K bits for Message
                 N  : integer range 0 to 20 :=7);-- N bits for Codeword 
             
    Port ( 
              Data_in : in STD_LOGIC;
              Data_out : out STD_LOGIC;
              
           Clk : in  STD_LOGIC;
              Reset : in STD_LOGIC;
              
              Valid_in : in STD_LOGIC;
              Valid_out : out STD_LOGIC);
end OEncoder_3v2;
 
architecture Behavioral of OEncoder_3v2 is
 
--Constant Declaration
Constant GP : STD_LOGIC_VECTOR ((N-K) downto 0) :="1011";   -- Generator Polynomial of (N-K) Degree
 
--Signal Declaration
Signal D,Q : STD_LOGIC_VECTOR ((N-K-1) downto 0) :=(Others => '0'); -- Flip flop's Inputs
Signal ClockCounter : integer range 0 to N;
Signal GTemp,UQX : STD_LOGIC;
Signal Input_Buffer,Vin_Buffer : STD_LOGIC;
 
 
 
Type Switch is ( Parity , message );
Signal Switch2 : Switch := Message;
 
begin
 
--Combinatorial Part
    
--1)- taking care of FF's Input and XORs
--***CHECKED***
    Gen1:for i in 1 to N-K-1 generate 
        D(i) <= (Gtemp xor Q(i-1)) when GP(i)='1' else
                     Q(i-1);
    end generate;
    D(0) <= Gtemp;
------------------------------------------------------------------------------------------- 
 
--2) taking care of FF's Outputs
--***CHECKED***
    UQX <= (Input_Buffer xor Q(N-K-1)) When (Vin_Buffer = '1') else '0';
--------------------------------------------------------------------------------------------
    
--3) taking care of GATE
--***CHECKED*** 
    Gtemp <= UQX when Switch2 = Message else '0'; -- Gtemp <= UQX and Switch2
--------------------------------------------------------------------------------------------
 
-- taking care of Switch 2
--4)***CHECKED***
    Data_out <= Input_Buffer When Switch2 = Message else Q(N-K-1);
--------------------------------------------------------------------------------------------
    Valid_out <= '0' When ClockCounter = N else Vin_Buffer;
-- Sequential part Va
 
Process(Clk)
begin
    if rising_edge(clk) then 
    
        if reset='1' then 
    
            -- reset all assigned signals here
            Q <= (Others => '0');
            ClockCounter <= 0;
            Input_Buffer <= '0';
            --Output_Buffer <= '0';
            --Vout_Buffer <= '0';
            Vin_Buffer <= '0';
            Switch2 <= Message;
            
        else
            -- Default assignments first
            Q <= D;
            Vin_Buffer <= Valid_in;
            Input_Buffer <= Data_in;
            
            if Vin_Buffer ='1' then 
                if ClockCounter < N then
                    ClockCounter <= ClockCounter + 1;
                end if;
            end if; 
            
            if ClockCounter = K-1 then 
                Switch2 <= Parity;
            end if;
                
        end if;--Reset
    end if;--Clock
end Process;
end Behavioral;



- - - Updated - - -

This is the exact schematic for Encoder_3v2
 

Attachments


Status
Not open for further replies.
Toggle Sidebar

Part and Inventory Search


Welcome to EDABoard.com

Sponsor

Sponsor

Design Fast


×
Top