simons

SPI controller core

SPI protocol have 4 different clocking modes based on clock polararity (CPOL) and shifting phase (CPHA), but this spi module can only handle 2 (the two with CPOL low) + 2 other not used by SPI.

To address this while keeping the added flexibility a CPOL bit should be added to control the polarity of the outgoing clock.

While doing this it might also be worthwhile to rename RX_NEG/TX_NEG to .._PHASE2 to indicate that the action is taken on the second clock phase regardless of polarity.

Missing clock polarity control

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.

First serial bit wrong

Bit 0 of the CTRL register has a bug: when it is set by firmware, it can never ever be cleared again.  This bit is part of the CHAR_LEN field which, according to the documentation, is R/W.  But this is not the case.  This means that the amount of data to transfer can never be changed to an even value if it ever gets set to an odd value.

The problem is in spi_top.v.  My suggested fix is to change this line:

    ctrl[7:0] <= #Tp wb_dat_i[7:0] | {7'b0, ctrl[0]};

to this:

    ctrl[7:0] <= #Tp wb_dat_i[7:0];

Browse

SPI controller core