SPI controller core

File spi_shift.vhd: The first bit to transmit is always stored inside s_out while tip is zero. Writing new data will effect this s_out signal while no transfer is in progress (minor effect). The major problem is that writing new data and changing the transfer size afterwards (sequence is TX3, TX2, TX1, TX0, CTRL) will transmit the highest bit position from the last transfer /which was transmitted with a different size) and NOT the highest bit of the new data first. Way to simulate this: a) Set CS to slave 1 b) Wtite TX0 with 0x00000006 c) Write CTRL with 0x00001708 d) Set CS to no slave e) Set CS to slave 1 f) Write TX3 with 0x00020040 g) Write TX2 with 0x685FB693 h) Write TX1 with 0x96489326 i) Write TX0 with 0xC362FDC3 (Note at this time the error happens, the s_out line is changed from low to high) j) Write CTRL with 0x00001778 k) Set CS to no slave

In this sequence the first transmitted byte is not the 0x02, instaed a 0x82 is transmitted). In combination with an SPI Prom the write command will fail and the data is missing.

Suggested change: OLD: spi_shift.v, line 122: // Sending bits to the line always @(posedge clk or posedge rst) begin if (rst) s_out <= #Tp 1'b0; else s_out <= #Tp (tx_clk || !tip) ? data[tx_bit_pos`SPI_CHAR_LEN_BITS-1:0] : s_out; end

NEW: spi_shift.v, line 122: // Sending bits to the line always @(posedge clk or posedge rst) begin if (rst) s_out <= #Tp 1'b0; else if (!tip) s_out <= #Tp 1'b0; else if (tx_clk || (!tip_registered && tip)) s_out <= #Tp data[tx_bit_pos`SPI_CHAR_LEN_BITS-1:0]; else s_out <= #Tp s_out; end

The bugfix solve the random set of the line during no tip and solves the wrong first bit after size change.

The example code is incomplete (there's no tip_registered).

The problem isn't s_out.

The problem is that we're setting the go bit at the same time we're changing the transmit length.

The first bit is clocked out of the shifter on the go bit, which is before the length has changed.

I've fixed this for our system with the code below. It basically delays updates to s_out for one clock cycle. I've added a "reg do_sout" in the declaration section. This replaces the s_out output block mentioned above:

// Sending bits to the line always @(posedge clk or posedge rst) begin if (rst) do_sout <= 1'b0; else if (tx_clk || !tip) do_sout <= 1'b1; else do_sout <= 1'b0; end

// Sending bits to the line always @(posedge wb_clk or posedge rst) begin if (rst) s_out <= 1'b0; else // s_out <= (tx_clk || !tip) ? data[tx_bit_pos`SPI_CHAR_LEN_BITS-1:0] : s_out; if (do_sout) s_out <= data[tx_bit_pos`SPI_CHAR_LEN_BITS-1:0]; else s_out <= s_out; end

Sorry... here's another attempt at properly formed VHDL. The system ate text around the "<" and ">" characters.

If you set the go bit to start a transmit and change the bit count at the same time, then the first bit out will be incorrect. You need to either set the bit count, then set the go bit in a separate transaction... or make a change to the core.

I found it simpler to alter the core than our firmware. I added a new process that sets a flag "do_sout" with the same control flow as the "s_out" process, then I changed the "s_out" process to use this flag. This effectively delays the setting of the output bit until the next clock cycle... which allows the internal length value to change before it's used in indexing the first bit.

Here's the change (I'm passing in the faster clock "wb_clk", this is probably unnecessary but helps a little):

reg do_sout;

always @(posedge clk or posedge rst) begin if (rst) do_sout <= 1'b0; else if (tx_clk || !tip) do_sout <= 1'b1; else do_sout <= 1'b0; end

// Sending bits to the line // this replaces the s_out process containing the following line: // s_out <= (tx_clk || !tip) ? data[tx_bit_pos`SPI_CHAR_LEN_BITS-1:0] : s_out; always @(posedge wb_clk or posedge rst) begin if (rst) s_out <= 1'b0; else if (do_sout) s_out <= data[tx_bit_pos`SPI_CHAR_LEN_BITS-1:0]; else s_out <= s_out; end

The suggested solution solves the issue, but it introduces a delay for one clock cycle(faster), e.g. if we want to transmit the MISO_data_o at the negative edge of TX_CLK, the data is not transmitted until the next rising edge of the faster clock.

The suggested solution solves the issue, but it introduces a delay for one clock cycle(faster), e.g. if we want to transmit the MISO_data_o at the negative edge of TX_CLK, the data is not transmitted until the next rising edge of the faster clock.

The documentation is very clear that you MUST set the GO_BSY bit in a separate transaction after all other bits of the controller have been configured including the rest of the CTRL register.

But having this fixed is nice I think, makes life easier and less unexpected suprices. But a the same time worried about the added delay and it's effect on timings. It this is done then maybe the clock output needs to be delayed by the same amount, maybe chip select output as wel.

My bugfix:

Minor effect:

file: spi_shift.v

-: s_out <= #Tp (tx_clk || !tip) ? data[tx_bit_pos`SPI_CHAR_LEN_BITS-1:0] : s_out; +: s_out <= #Tp (tx_clk || (!tip && go)) ? data[tx_bit_pos`SPI_CHAR_LEN_BITS-1:0] : s_out;

Major problem:

file spi_top.v

-: wire go; // go +: reg go; // go

-: assign go = ctrl`SPI_CTRL_GO;

always @(posedge wb_clk_i or posedge wb_rst_i) begin if (wb_rst_i) +: begin ctrl <= #Tp {`SPI_CTRL_BIT_NB{1'b0}}; +: go <= #Tp 1'b0; +: end

...

else if(tip && last_bit && pos_edge)

+: begin ctrl`SPI_CTRL_GO <= #Tp 1'b0; +: go <= #Tp 1'b0; +: end +: else +: go <= ctrl`SPI_CTRL_GO; end

Sory :(

Minor effect bugfix:

file: spi_shift.v

-: s_out <= #Tp (tx_clk || !tip) ? data[tx_bit_pos`SPI_CHAR_LEN_BITS-1:0] : s_out; +: s_out <= #Tp (tx_clk || (!tip && go)) ? data[tx_bit_pos`SPI_CHAR_LEN_BITS-1:0] : s_out;

Major problem:

file spi_top.v

-: wire go; // go +: reg go; // go

-: assign go = ctrl`SPI_CTRL_GO;

+: always @(posedge wb_clk_i or posedge wb_rst_i) begin if (wb_rst_i) +: begin ctrl <= #Tp {`SPI_CTRL_BIT_NB{1'b0}}; +: go <= #Tp 1'b0; +: end else if(spi_ctrl_sel && wb_we_i && !tip)

...

else if(tip && last_bit && pos_edge)

+: begin ctrl`SPI_CTRL_GO <= #Tp 1'b0; +: go <= #Tp 1'b0; +: end +: else +: go <= ctrl`SPI_CTRL_GO; end

Major problem:

file spi_top.v

-: wire go; // go +: reg go; // go

-: assign go = ctrl`SPI_CTRL_GO;

+: always @(posedge wb_clk_i or posedge wb_rst_i) begin if (wb_rst_i) +: begin ctrl <= #Tp {`SPI_CTRL_BIT_NB{1'b0}}; +: go <= #Tp 1'b0; +: end else if(spi_ctrl_sel && wb_we_i && !tip)

...

else if(tip && last_bit && pos_edge)

+: begin ctrl`SPI_CTRL_GO <= #Tp 1'b0; +: go <= #Tp 1'b0; +: end +: else +: go <= ctrl`SPI_CTRL_GO; end