VHLD Serial Communication Pointers

[rmercury]

I am new to VHDL, but have been programing multiple other languages for years. I've been reading a few textbooks and websites. I've also read through alot of the newbie posts here. I'm looking for any feedback on my code.

The point of this code is to communicate with a TLC5940 a PWM 16 LED driver. It requires a serial connection of 192 bits to control the LED intensity within 4096 clock pulses that the PWM of the chip.

The proper clocks are generated and data is output at the neg edge (so it will be ready at the positive clock edge.

I will be adding more code to generate the LED sequence and intensity.

LIBRARY ieee;
use ieee.std_logic_1164.all;
use ieee.std_logic_arith.all;
use ieee.std_logic_unsigned.all;


ENTITY light_tester is
	GENERIC(
		pwm_pulses  : integer := 10;  --4096--number of spi slaves
		transmit_pulses : integer := 5); --192--data bus width
	PORT(
		clock :	in			std_logic;										--SYS Clock
		gsclk	:	buffer	std_logic := '0';								--GS Clock
		dcprg	:	out		std_logic := '0';								--DC Program (keep low) for future development
		sclk	:	buffer	std_logic := '0';								--Shift load clock
		blank	:	out		std_logic := '0';								--Blank outputs/zero PWM counter
		vprg	:	out		std_logic := '0';								--EEPROM Program (keep low) for future development
		xlat	:	buffer	std_logic := '0';								--latch new data use when blank is high
		data	:	out		std_logic := '0');							--data sent out
end light_tester;

ARCHITECTURE logic OF light_tester IS
	
begin
	process (clock)						--clocks pwm of TLC5940 using 1/2 FPGA clock freq (25 MHz => 12.5 MHz)
		variable gs_counter : integer range 0 to pwm_pulses*2+3 := 0;
		variable senddata : boolean := true;
		variable data_to_send : std_logic_vector(0 to 7) := ('1', '1', '0', '1', '0', '0', '1', '0');
	begin
	
		if(clock'EVENT and clock = '1') then
			case gs_counter IS
				when pwm_pulses*2 =>		-- done with clocking in data
					gs_counter := gs_counter+1;
					blank <= '1';				--set blank high to reset the TLC5940 PWM counter
				when pwm_pulses*2+1 =>
					xlat <= '1';				--set xlat high to latch new data
					gs_counter := gs_counter+1;
				when pwm_pulses*2+2 =>		--toggle xlat and blank and reset to start over
					xlat <= '0';
					blank<= '0';
					gs_counter :=0;
					senddata :=true;        -- send new data next round

				when transmit_pulses*2 =>	--last transmitted serial data so dont transmit any more
					senddata :=false;
					gs_counter := gs_counter+1;
					gsclk <= not gsclk;		--toggle pwm clock
						
				when others => 				--clock 4096 pwm pulses
					gs_counter := gs_counter+1;
					gsclk <= not gsclk;		--toggle pwm clock
					if (senddata) then		--only send data when you haven't transmitted all the bits
						sclk <= not sclk;		--toggle serial data clock
					end if;
			end case;
		end if;
		
		if (clock'event and clock = '0' and senddata) then --clock is 0 => load new data
			data<=data_to_send(gs_counter);				--load data to be transmitted
																	--right now this is just dummy data
		end if;
	end process; 
	
end logic;

Because I've never done VHDL I want to make sure that I'm updating the signals correctly and that real delays won't hinder my approach. (I have not simulated with real time delays or on the actual FPGA which is my next step). I also want to make sure that I'm not breaking any golden rules or doing it in a non efficient way.

Thanks for the feedback!

 

[TrickyDicky]

There are a few bad practices in this code:

1. You should only have a single clocked if statement inside a process, and you chould never "and" anything with the clock. You will probably escape, the synthesisor will probably see your mistake and create a synchronous enable, but you're effectivly asking the synthesisor to and the clock with sendddata before connecting it to the clock pin of the data registers.

prefered enable method:

process(clk)
begin
  if rising_edge(clk) then
    if sendata then
      data<=data_to_send(gs_counter)
    end if;
  end if;
end process;

2. Use of variables. As a beginner, variables are usually not recomended. Although you should be ok, you're using their output imediately, rather than the registered version of gs_counter and senddata, which may lead to poorer timing performance (the more logic between registers, the slower the max clock frequency).

3. Generated clocks. Generally a big no no. You are usually ok if they are slow clocks and only go off chip, but clocking internal logic with them is very poor practice. Logic generated clocks are prone to poor timing performance as they can have high fan-out and skew. But better to generate clock enables and clock the system with the same system clock.

Have you created a testbench? most designers avoid doing timing simulation as much as possible, as effective RTL testbenches along with good practice will usually eliminate 99% of bugs and problems (99% of the rest in FPGA design are usually fixed with good timing specs, timing analysis and then re-pipelinig the RTL).

 

[ads-ee]

I've also seen a lot of comments about using the following non-standard synonpsys libraries
use ieee.std_logic_arith.all;
use ieee.std_logic_unsigned.all;

which have been deprecated in favor of using the standard IEEE numeric_std library.

Regards

 

[TrickyDicky]

I've also seen a lot of comments about using the following non-standard synonpsys libraries
use ieee.std_logic_arith.all;
use ieee.std_logic_unsigned.all;

which have been deprecated in favor of using the standard IEEE numeric_std library.

Regards

They were never depricated, they were just never part of the standard. Synopsys made them for their tools and other vendors made their own versions. They became a defacto standard, hence why all the older textbooks (and less well informed engineers, and Xilinx) still use them.

 

[ads-ee]

According to the definition of deprecate - to criticize or express disapproval of (someone or something)
Hmmm, this is exactly what every VHDL guru on this board has done at one time or another...(I'm too lazy to add all those links from this board)

and according to the software definition: https://en.wikipedia.org/wiki/Deprecation

I stand by my original statement.


Something doesn't need to be part of a standard to be deprecated. I just has to be in use until something else superseds it.


rmercury, What you should get from all this is don't use the synopsys libraries they aren't part of the standard and they never were. Use the IEEE standard libraries.

In my opinion IEEE dropped the ball on this originally (Hardware engineers don't need to use numbers?...)