Implementing a simple SPI based protocol in VHDL

[Endymion]

Hello,

I am attempting to use VHDL to implement a SPI based shift register. However, instead of using SPI for loading or reading the shift register I would like to use a command based approach. The shift register will never be required to have more than one '1' loaded into it (it will always have a walking one vector). So I've come up with the following commands that I'd like to implement:

CMD: 0x01 -- Load shift register with '1' at the least significant position.
CMD: 0x0F -- Set all bits to 0.
CMD: 0xAA -- Shift the contents of the shift register by one bit.

I am a newbie in VHDL and digital design so I might have made some really basic mistakes in the following program and thats why I'm submitting it. Please note, I am not done yet. Notably, I don't have the shift register output being set anywhere. I am hoping someone here could take a look and let me know if there is something I'm doing wrong (i.e. how does my design look?). The simulation works well. All 3 commands that I mentioned above seem to work well.

The "sync" component is a component I wrote to sync. external signals like Chip Select and SCK.

library ieee;
use ieee.std_logic_1164.all;

entity RotatingOne is
  port(nRESET, CLK, SCK, SI, nCS : in  std_logic;
     POut                      : out std_logic_vector(7 downto 0));
end RotatingOne;

architecture shift of RotatingOne is
  constant WIDTH : integer := 8;
  signal cmd     : std_logic_vector(7 downto 0);
  signal temp    : std_logic_vector(WIDTH - 1 downto 0);

  signal rise_SCK, fall_SCK : std_logic;
  signal CS_rising, CS_falling : std_logic;

  component sync
    port(
      SCK        : in  std_logic;
      CLK        : in  std_logic;
      rise, fall : out std_logic
    );
  end component;

begin
  sync1 : sync port map(SCK, CLK, rise_SCK, fall_SCK);
  sync2 : sync port map(nCS, CLK, CS_rising, CS_falling);

  process(CLK, nRESET)
  begin
    if (nRESET = '0') then
      cmd <= (others => '0');
    elsif rising_edge(CLK) then
      if nCS = '0' then
        if rise_SCK = '1' then
          cmd <= cmd(WIDTH - 2 downto 0) & SI;
        end if;
      end if;
    end if;
  end process;

  process(CLK, nRESET)
  begin
    if (nRESET = '0') then
      temp <= (others => '0');
    elsif rising_edge(CLK) then
      if CS_rising = '1' then
      case cmd is
        when "00000001" =>
          temp <= "00000001";
        when "00001111" =>
          temp <= "00000000";
        when "10101010" =>
          temp <= temp(WIDTH - 2 downto 0) & '0';
        when others =>
          null;
      end case;
    end if;
    end if;
  end process;
  
 
end shift;

The first process handles an internal 8-bit shift register which holds the incoming command (cmd). This command is then compared with in a Case statement in the 2nd process. If it matches a known command, the appropriate action is executed. I am wondering weather I should set the contents of the cmd register to 0 whenever an action is executed and done.

 

[mrflibble]

If I understand your plan correctly, then you can just make it 2 interacting FSM's.

One FSM does the SPI stuff, and the other FSM handles the command execution. If you just draw out those 2 FSMs on paper then you will probably answer your own question.

 

[TrickyDicky]

This setup is fine, only if you know for sure that CS will rise at the end of every DWord sent. Often, the CS is held low so that large amounts of data can be sent in one go. it may be easier just to use a counter to count the number of bits coming in, and then act on that dward every 8 bits in your case.

 

[Endymion]

Thanks guys. Looking at the simulation a little closely, I was a little confused about something. My CS signal is synced with CLK and it's rising edge is detected. When there is a rising edge and the CLK has a rising edge too, I execute a command (CMD 0x01 etc.). You can see this in the 2nd process. But looking at the simulation, I note that when CLK is rising CS_rising is falling. It is at this moment that the command is executed. Can this cause problems?

**broken link removed**

You can see this where the yellow cursor is. For reference, my clock syncing code is as follows:

library ieee;
use ieee.std_logic_1164.all;

entity sync is
	port(
		d : in std_logic;
		CLK : in std_logic;
		rise_d, fall_d: out std_logic
	);
end sync;

architecture sync_arch of sync is
	signal sig_rise, sig_fall: std_logic;
	
	begin
		sync1: process(CLK)
			variable resync: std_logic_vector(1 to 3);
				begin
					if rising_edge(CLK) then
						sig_rise	<= resync(2) and not resync(3);
						sig_fall	<=	resync(3) and not resync(2);
						resync := d & resync(1 to 2);
					end if;
		end process;
		rise_d <= sig_rise;
		fall_d <= sig_fall;
end sync_arch

 

[mrflibble]

I get a "Invalid Attachment specified" for the attachment you link to...

 

[Endymion]

I get a "Invalid Attachment specified" for the attachment you link to...

Sorry about that. Here it is

 

[Endymion]

Tried out my code on actual hardware today and it worked well! I'm now trying to send information back to the Master of the SPI bus. Again, I haven't fully implemented this but was hoping someone could take a look and let me know if my design is OK. My major concern is the DO output (data out). As I'm trying to implement SPI mode 0, DO needs to hold the most significant bit of the register as soon as CS goes low. But as CS isn't synchronous I cannot use that - so I instead use a synced version called cs_sync. DO also needs to toggle when SCK falls, so I utilize sck_falling for that.

I'm not sure if I have the right approach though. The following is my SPI code:

process(CLK, nRESET)
  begin
    if (nRESET = '0') then
      cmd <= (others => '0');
      SO <= '0';
    elsif rising_edge(CLK) then
      if cs_falling = '1' then
        SO <= cmd(cmd'high);
      end if;
      if CS_sync = '0' then
        if SCK_rising = '1' then
          cmd <= cmd(8 - 2 downto 0) & SI;
        elsif sck_falling = '1' then
          SO <= cmd(cmd'high);
        end if;
      end if;
    end if;
  end process;
DO <= SO when cs_sync = '0' else 'Z';

Another concern mine of mine is this: cs_sync is a slightly delayed version of nCS. When the master takes nCS high it means it has deselected the device. However, because cs_sync is delayed, the device will not release the MISO line for a couple of clock cycles. Is this OK? I do have other devices on the bus that I will need to talk to immediately after this device. However, I can easily also put a delay after deselecting the device. This will allow the device ample time to release MISO. I would really appreciate if someone could give me pointers on this.

 

[FvM]

However, because cs_sync is delayed, the device will not release the MISO line for a couple of clock cycles. Is this OK?

If it's a problem, MISO output enable can be easily controlled asynchronously without affecting the synchronous SPI design operation.

As more general point, the synchronous slave operation causes a considerable MISO delay, you have to check if it's tolerated by the master controller.

 

[Endymion]

If it's a problem, MISO output enable can be easily controlled asynchronously without affecting the synchronous SPI design operation.

As more general point, the synchronous slave operation causes a considerable MISO delay, you have to check if it's tolerated by the master controller.

Thanks, FvM. I assume by your first point you meant this is considered OK:

DO <= SO when nCS = '0' else 'Z';

If I understand you correctly, this is considered OK because this isn't part of our sequential design but rather combinational.

 

[FvM]

If I understand you correctly, this is considered OK because this isn't part of our sequential design but rather combinational.

Yes, exactly.

 

[Endymion]

The code I posted yesterday seemed to work well today, on actual hardware. However, there was a strange bug that I wasn't able to solve. My system clock is 8MHz. If I increase my SPI clock, SCK, to about 1 MHz the master would not read the data correctly sometimes. But if I decreased the system clock to 500KHz, it worked perfectly. Not once did the system read data back in correctly.

It cannot be a signal integrity issue as it's just a 1 MHz signal with 5 or so ns rise times. This leads me to think that I may to increase my system clock frequency if I want to increase SCK any further as the data on MISO/MOSI may not be setting up in time for the MCU to read correctly due to the added delays of synchronization. Am I guess correctly?

I also further changed my code to now incorporate a small variable counter, from range 0 to 72. The counter increment whenever the command 0xAA is executed (so it's purpose is to count the number of times I have shifted my walking one test vector). The information is relayed back to the MCU during the next command, so I'm using SPI in full-duplex mode. But again, due to my background as a C programmer, I am not sure if I got my VHDL right. The sim seems to work OK, but bad designs can work too! The MCU will read the count from it's MISO line and if it equals what it expects, then the shift register shifted correctly. In other words, it's just a mechanism to 'know' where the '1' is located in the shift register.

process(CLK, nRESET)
	begin
		if (nRESET = '0') then
			cmd <= (others => '0');
			SO  <= '0';
		elsif rising_edge(CLK) then
			if CS_sync = '0' then
				if SCK_rising = '1' then
					cmd <= cmd(8 - 2 downto 0) & SI;
				end if;
			end if;
		end if;
	end process;

	process(CLK, nRESET)
	begin
		if nRESET = '0' then
			out_reg <= (others => '0');
		elsif rising_edge(CLK) then
			if cs_sync = '0' then
				if sck_falling = '1' then
					out_reg <= out_reg(out_reg'high - 1 downto out_reg'low) & '0';
				end if;
			elsif cs_sync = '1' then
				if ACK = '1' then
					out_reg <= shift_count_vector;
				else
					out_reg <= (others => '1');
				end if;
			end if;
		end if;
	end process;

	process(CLK, nRESET)
		variable shift_count : integer range 0 to 72;
	begin
		if (nRESET = '0') then
			temp        <= (others => '0');
			shift_count := 0;
		elsif rising_edge(CLK) then
			if CS_rising = '1' then
				case cmd is
					when "00000001" =>
						temp        <= (others => '0');
						temp(0)     <= '1';
						shift_count := 1;
						ACK         <= '1';
					when "00001111" =>
						temp        <= (others => '0');
						shift_count := 0;
						ACK         <= '1';
					when "10101010" =>
						temp        <= temp(WIDTH - 2 downto 0) & '0';
						shift_count := shift_count + 1;
						ACK         <= '1';
					when others =>
						ACK <= '0';
				end case;
				shift_count_vector <= std_logic_vector(to_unsigned(shift_count,shift_count_vector'length));
			end if;
		end if;
	end process;

I am aware of a very obvious bug in the program. If I execute 0xAA without first sending 0x01, I'll have a false count. I suppose I should have this as a state machine where 0xAA cannot be executed unless 0x01 already has. But what would you folks think of the 2nd process - the one which sends data back to the MCU. The ACK bit is a way to load the out_reg shift register. If the MCU sends a bad command, ACK is 0 and 0xFF is sent back. So if 0xFF is encountered is the MCU will know the previous command sent was bad and it must send it again.

 

[FvM]

The accidental failure may be related to the MISO delay problem I mentioned in post #8. If so, changing SPI master operation to late data sampling (if provided with your uP) can help. With Microchip, it's option SMP = 1 (Input data sampled at end of data output time). It increases the MISO delay margin by 1/2 SPI clock cycle.

 

[Endymion]

FvM, does that basically set CPHA to 1?

EDIT: Looking at this a little more closely, I think AVR offers any equivalent way of sampling the data at the end of a cycle. So would my only course of action be increasing my system clock?

 

[FvM]

So would my only course of action be increasing my system clock?

Yes, or change the synchronization scheme.