[PATCH 2/3] [raspberrypi] Fix issues in SPI driver

Jan Sommer soja-lists at aries.uberspace.de
Sun Jun 12 18:45:02 UTC 2016


Closes PR #2733

- Add defined bitmasks for access to bits of control register
- Use the whole FIFO buffer
- Fix lsb-first mode
- 3-wire mode might work
- Fix deadlock if multiple tasks access the driver
- Add support for simultaneous read-write-operation
---
 c/src/lib/libbsp/arm/raspberrypi/spi/spi.c | 333 +++++++++++++----------------
 1 file changed, 146 insertions(+), 187 deletions(-)

diff --git a/c/src/lib/libbsp/arm/raspberrypi/spi/spi.c b/c/src/lib/libbsp/arm/raspberrypi/spi/spi.c
index c595ca1..de3c44b 100644
--- a/c/src/lib/libbsp/arm/raspberrypi/spi/spi.c
+++ b/c/src/lib/libbsp/arm/raspberrypi/spi/spi.c
@@ -34,6 +34,37 @@
     ;                                           \
   }
 
+/* Define bits of the control register */
+#define SPI_CS_LEN_LONG      (1<<25)
+#define SPI_CS_DMA_LEN       (1<<24)
+#define SPI_CS_CSPOL2        (1<<23)
+#define SPI_CS_CSPOL1        (1<<22)
+#define SPI_CS_CSPOL0        (1<<21)
+#define SPI_CS_RXF           (1<<20)
+#define SPI_CS_RXR           (1<<19)
+#define SPI_CS_TXD           (1<<18)
+#define SPI_CS_RXD           (1<<17)
+#define SPI_CS_DONE          (1<<16)
+#define SPI_CS_LEN           (1<<13)
+#define SPI_CS_REN           (1<<12)
+#define SPI_CS_ADCS          (1<<11)
+#define SPI_CS_INTR          (1<<10)
+#define SPI_CS_INTD          (1<< 9)
+#define SPI_CS_DMAEN         (1<< 8)
+#define SPI_CS_TA            (1<< 7)
+#define SPI_CS_CSPOL         (1<< 6)
+#define SPI_CS_CLEAR_ALL     (3<< 4)
+#define SPI_CS_CLEAR_RX      (1<< 5)
+#define SPI_CS_CLEAR_TX      (1<< 4)
+#define SPI_CS_CPOL          (1<< 3)
+#define SPI_CS_CPHA          (1<< 2)
+#define SPI_CS_CS            (3<< 0)
+  
+/* The RX FIFO buffer is 64 bytes (16 words * 4 bytes per word) large
+ * Thus it should be safe to write 64 bytes to TX FIFO before reading
+ * the RX FIFO without data loss */
+#define MAX_FIFO_SIZE 64
+  
 /**
  * @brief Object containing the SPI bus configuration settings.
  *
@@ -43,14 +74,9 @@ typedef struct
 {
   int initialized;
   uint8_t bytes_per_char;
-
-  /* Shift to be applied on data transfers with
-   * least significative bit first (LSB) devices. */
-  uint8_t bit_shift;
+  uint8_t lsb_first;
   uint32_t dummy_char;
-  uint32_t current_slave_addr;
   rtems_id task_id;
-  int irq_write;
 } rpi_spi_softc_t;
 
 /**
@@ -155,26 +181,18 @@ static rtems_status_code rpi_spi_set_tfr_mode(
     default:
       return RTEMS_INVALID_NUMBER;
   }
-
-  /* Check the data mode (most or least significant bit first) and calculate
-   * the correcting bit shift value to apply on the data before sending. */
-  if ( tfr_mode->lsb_first ) {
-    softc_ptr->bit_shift = 32 - tfr_mode->bits_per_char;
-  }
-  /* If MSB first. */
-  else {
-    softc_ptr->bit_shift = 0;
-  }
+  
+  softc_ptr->lsb_first = tfr_mode->lsb_first;
 
   /* Set SPI clock polarity.
    * If clock_inv is TRUE, the clock is active high.*/
   if ( tfr_mode->clock_inv ) {
     /* Rest state of clock is low. */
-    BCM2835_REG(BCM2835_SPI_CS) &= ~(1 << 3);
+    BCM2835_REG(BCM2835_SPI_CS) &= ~SPI_CS_CPOL;
   }
   else {
     /* Rest state of clock is high. */
-    BCM2835_REG(BCM2835_SPI_CS) |= (1 << 3);
+    BCM2835_REG(BCM2835_SPI_CS) |= SPI_CS_CPOL;
   }
 
   /* Set SPI clock phase.
@@ -182,11 +200,11 @@ static rtems_status_code rpi_spi_set_tfr_mode(
    * at the start of the data transfer. */
   if ( tfr_mode->clock_phs ) {
     /* First SCLK transition at beginning of data bit. */
-    BCM2835_REG(BCM2835_SPI_CS) |= (1 << 2);
+    BCM2835_REG(BCM2835_SPI_CS) |= SPI_CS_CPHA;
   }
   else {
     /* First SCLK transition at middle of data bit. */
-    BCM2835_REG(BCM2835_SPI_CS) &= ~(1 << 2);
+    BCM2835_REG(BCM2835_SPI_CS) &= ~SPI_CS_CPHA;
   }
 
   return sc;
@@ -214,162 +232,110 @@ static int rpi_spi_read_write(
   rpi_spi_softc_t *softc_ptr = &(((rpi_spi_desc_t *)(bushdl))->softc);
 
   uint8_t bytes_per_char = softc_ptr->bytes_per_char;
-  uint8_t bit_shift =  softc_ptr->bit_shift;
-  uint32_t dummy_char = softc_ptr->dummy_char;
-
-  uint32_t bytes_sent = buffer_size;
-  uint32_t fifo_data;
-
-  /* Clear SPI bus FIFOs. */
-  BCM2835_REG(BCM2835_SPI_CS) |= (3 << 4);
-
-  /* Set SPI transfer active. */
-  BCM2835_REG(BCM2835_SPI_CS) |= (1 << 7);
-
-  /* If using the SPI bus in interrupt-driven mode. */
-#if SPI_IO_MODE == 1
-  softc_ptr->irq_write = 1;
-
-  BCM2835_REG(BCM2835_SPI_CS) |= (1 << 9);
-
-  if ( rtems_event_transient_receive(RTEMS_WAIT, 0) != RTEMS_SUCCESSFUL ) {
-    rtems_event_transient_clear();
-
-    return -1;
+  
+  uint32_t  dummy_char = softc_ptr->dummy_char;
+  uint32_t  bytes_sent = buffer_size;
+  
+  uint32_t  num_transfers = (buffer_size + MAX_FIFO_SIZE-1) / MAX_FIFO_SIZE;
+  uint32_t  bytes_transfer;
+  
+  if ( bytes_per_char < 1 || bytes_per_char > 4) {
+      return -1;
   }
-
-  /* If using the bus in polling mode. */
-#else
-  /* Poll TXD bit until there is space to write at least one byte
-   * on the TX FIFO. */
-  SPI_POLLING((BCM2835_REG(BCM2835_SPI_CS) & (1 << 18)) == 0);
-#endif
-
-  /* While there is data to be transferred. */
-  while ( buffer_size >= bytes_per_char ) {
-    /* If reading from the bus, send a dummy character to the device. */
-    if ( rd_buf != NULL ) {
-      BCM2835_REG(BCM2835_SPI_FIFO) = dummy_char;
-    }
-    /* If writing to the bus, move the buffer data to the TX FIFO. */
+   
+  /* Clear the FIFOs and set SPI transfer active */
+  BCM2835_REG(BCM2835_SPI_CS) |= SPI_CS_TA | SPI_CS_CLEAR_ALL;
+  
+  if ( wr_buf == NULL && bidirectional) {
+    /* If using bi-directional SPI. */
+    /* Change bus direction to read from the slave device. */
+    BCM2835_REG(BCM2835_SPI_CS) |= SPI_CS_REN;
+  }
+    
+  for (size_t i=0; i<num_transfers; ++i) {
+    /* Determine how many bytes to write in this transfer */
+    bytes_transfer = buffer_size <= MAX_FIFO_SIZE ? buffer_size : MAX_FIFO_SIZE;
+    
+    /* Write new (dummy) data to the fifo until either the TX FIFO is full (TXD cleared)
+     * or bytes_transfer bytes have been transmitted */
+    if ( wr_buf == 0 ) {
+      while ( (BCM2835_REG(BCM2835_SPI_CS) & SPI_CS_TXD) && bytes_transfer) {
+        BCM2835_REG(BCM2835_SPI_FIFO) = dummy_char & 0xFF;
+        bytes_transfer--;
+        buffer_size--;
+      }
+    } 
     else {
-      switch ( bytes_per_char ) {
-        case 1:
-          BCM2835_REG(BCM2835_SPI_FIFO) = (((*wr_buf) & 0xFF) << bit_shift);
-          break;
-
-        case 2:
-          BCM2835_REG(BCM2835_SPI_FIFO) = (((*wr_buf) & 0xFFFF) << bit_shift);
-          break;
-
-        case 3:
-          BCM2835_REG(BCM2835_SPI_FIFO) = (((*wr_buf) & 0xFFFFFF) << bit_shift);
-          break;
-
-        case 4:
-          BCM2835_REG(BCM2835_SPI_FIFO) = ((*wr_buf) << bit_shift);
-          break;
-
-        default:
-          return -1;
+      while ( (BCM2835_REG(BCM2835_SPI_CS) & SPI_CS_TXD) && bytes_transfer) {
+        if ( softc_ptr->lsb_first ) {
+          /* Bitflip the byte before writing it */
+          BCM2835_REG(BCM2835_SPI_FIFO) = 
+                    ((*wr_buf * 0x0802LU & 0x22110LU) | (*wr_buf * 0x8020LU & 0x88440LU)) * 0x10101LU >> 16; 
+        } else {
+          BCM2835_REG(BCM2835_SPI_FIFO) = (*wr_buf) & 0xFF;
+        }
+        bytes_transfer--;
+        buffer_size--;
+        wr_buf++;
       }
-
-      wr_buf += bytes_per_char;
-
-      buffer_size -= bytes_per_char;
     }
 
-    /* If using bi-directional SPI. */
-    if ( bidirectional ) {
-      /* Change bus direction to read from the slave device. */
-      BCM2835_REG(BCM2835_SPI_CS) |= (1 << 12);
-    }
-
-    /* If using the SPI bus in interrupt-driven mode. */
 #if SPI_IO_MODE == 1
-    softc_ptr->irq_write = 0;
-
-    BCM2835_REG(BCM2835_SPI_CS) |= (1 << 9);
-
-    if ( rtems_event_transient_receive(RTEMS_WAIT, 0) != RTEMS_SUCCESSFUL ) {
-      rtems_event_transient_clear();
-
-      return -1;
+    /* If using the SPI bus in interrupt-driven mode. */
+    /* Only wait if the transfer is not already DONE */
+    if ( (BCM2835_REG(BCM2835_SPI_CS) & SPI_CS_DONE) == 0 ) {
+      softc_ptr->task_id = rtems_task_self();
+     
+      BCM2835_REG(BCM2835_SPI_CS) |= SPI_CS_INTD;
+    
+      if ( rtems_event_transient_receive(RTEMS_WAIT, 0) != RTEMS_SUCCESSFUL ) {
+        rtems_event_transient_clear();
+    
+        return -1;
+      }
     }
-
-    /* If using the bus in polling mode. */
 #else
+    /* If using the bus in polling mode. */
     /* Poll the Done bit until the data transfer is complete. */
-    SPI_POLLING((BCM2835_REG(BCM2835_SPI_CS) & (1 << 16)) == 0);
+    SPI_POLLING((BCM2835_REG(BCM2835_SPI_CS) & SPI_CS_DONE) == 0);
 
     /* Poll the RXD bit until there is at least one byte
-     * on the RX FIFO to be read. */
-    SPI_POLLING((BCM2835_REG(BCM2835_SPI_CS) & (1 << 17)) == 0);
+    * on the RX FIFO to be read. */
+    SPI_POLLING((BCM2835_REG(BCM2835_SPI_CS) & SPI_CS_RXD) == 0);
 #endif
-
-    /* If writing to the bus, read the dummy char sent by the slave device. */
+    
+    /* If only writing to the bus, discard the dummy chars sent by the slave device. */
     if ( rd_buf == NULL ) {
-      fifo_data = BCM2835_REG(BCM2835_SPI_FIFO) & 0xFF;
+      BCM2835_REG(BCM2835_SPI_CS) |= SPI_CS_CLEAR_RX;
     }
-
-    /* If reading from the bus, retrieve data from the RX FIFO and
-     * store it on the buffer. */
-    if ( rd_buf != NULL ) {
-      switch ( bytes_per_char ) {
-        case 1:
-          fifo_data = BCM2835_REG(BCM2835_SPI_FIFO) & 0xFF;
-          (*rd_buf) = (fifo_data >> bit_shift);
-          break;
-
-        case 2:
-          fifo_data = BCM2835_REG(BCM2835_SPI_FIFO) & 0xFFFF;
-          (*rd_buf) = (fifo_data >> bit_shift);
-          break;
-
-        case 3:
-          fifo_data = BCM2835_REG(BCM2835_SPI_FIFO) & 0xFFFFFF;
-          (*rd_buf) = (fifo_data >> bit_shift);
-          break;
-
-        case 4:
-          fifo_data = BCM2835_REG(BCM2835_SPI_FIFO);
-          (*rd_buf) = (fifo_data >> bit_shift);
-          break;
-
-        default:
-          return -1;
+    else
+    {
+      uint8_t* ptr = rd_buf;
+      while ( BCM2835_REG(BCM2835_SPI_CS) & SPI_CS_RXD ) {
+        /* If reading from the bus, retrieve data from the RX FIFO and
+        * store it in the buffer. */
+        *ptr = BCM2835_REG(BCM2835_SPI_FIFO) & 0xFF;
+        ptr++;
       }
-
-      rd_buf += bytes_per_char;
-
-      buffer_size -= bytes_per_char;
-    }
-
-    /* If using bi-directional SPI. */
-    if ( bidirectional ) {
-      /* Restore bus direction to write to the slave. */
-      BCM2835_REG(BCM2835_SPI_CS) &= ~(1 << 12);
     }
   }
-
-  /* If using the SPI bus in interrupt-driven mode. */
-#if SPI_IO_MODE == 1
-  softc_ptr->irq_write = 1;
-
-  BCM2835_REG(BCM2835_SPI_CS) |= (1 << 9);
-
-  if ( rtems_event_transient_receive(RTEMS_WAIT, 0) != RTEMS_SUCCESSFUL ) {
-    rtems_event_transient_clear();
-
-    return -1;
+  
+  /* If using bi-directional SPI. */
+  if ( bidirectional ) {
+    /* Restore bus direction to write to the slave. */
+    BCM2835_REG(BCM2835_SPI_CS) &= ~SPI_CS_REN;
   }
-
-  /* If using the bus in polling mode. */
-#else
-  /* Poll the Done bit until the data transfer is complete. */
-  SPI_POLLING((BCM2835_REG(BCM2835_SPI_CS) & (1 << 16)) == 0);
-#endif
-
+  
+  if ( softc_ptr->lsb_first && rd_buf != 0) {
+    /* Bit flip the complete read buffer */
+    /* see graphics.stanford.edu/~seander/bithacks.html#BitReverseObvious */
+    uint8_t* ptr = rd_buf;
+    for (int i = 0; i < buffer_size; ++i) {
+      *ptr = ((*ptr * 0x0802LU & 0x22110LU) | (*ptr * 0x8020LU & 0x88440LU)) * 0x10101LU >> 16; 
+      ptr++;
+    }
+  }
+  
   bytes_sent -= buffer_size;
 
   return bytes_sent;
@@ -404,19 +370,10 @@ static void spi_handler(void* arg)
 {
   rpi_spi_softc_t *softc_ptr = (rpi_spi_softc_t *) arg;
 
-  /* If waiting to write to the bus, expect the TXD bit to be set, or
-   * if waiting to read from the bus, expect the RXD bit to be set
-   * before sending a waking event to the transfer task. */
-  if (
-      ( softc_ptr->irq_write == 1 &&
-        (BCM2835_REG(BCM2835_SPI_CS) & (1 << 18)) != 0
-      ) ||
-      ( softc_ptr->irq_write == 0 &&
-        (BCM2835_REG(BCM2835_SPI_CS) & (1 << 17)) != 0
-      )
-  ) {
+  /* Check if the current transfer is finished */
+  if ( BCM2835_REG(BCM2835_SPI_CS) & SPI_CS_DONE ) {
     /* Disable the SPI interrupt generation when a transfer is complete. */
-    BCM2835_REG(BCM2835_SPI_CS) &= ~(1 << 9);
+    BCM2835_REG(BCM2835_SPI_CS) &= ~SPI_CS_INTD;
 
     /* Allow the transfer process to continue. */
     rtems_event_transient_send(softc_ptr->task_id);
@@ -486,16 +443,11 @@ static rtems_status_code rpi_libi2c_spi_send_start(rtems_libi2c_bus_t * bushdl)
  */
 static rtems_status_code rpi_libi2c_spi_stop(rtems_libi2c_bus_t * bushdl)
 {
-  rpi_spi_softc_t *softc_ptr = &(((rpi_spi_desc_t *)(bushdl))->softc);
-
-  uint32_t addr = softc_ptr->current_slave_addr;
-  uint32_t chip_select_bit = 21 + addr;
-
   /* Set SPI transfer as not active. */
-  BCM2835_REG(BCM2835_SPI_CS) &= ~(1 << 7);
+  BCM2835_REG(BCM2835_SPI_CS) &= ~SPI_CS_TA;
   
   /* Deselect CS line */
-  BCM2835_REG(BCM2835_SPI_CS) &= 3;
+  BCM2835_REG(BCM2835_SPI_CS) |= SPI_CS_CS;
 
   return RTEMS_SUCCESSFUL;
 }
@@ -517,18 +469,13 @@ static rtems_status_code rpi_libi2c_spi_send_addr(
   uint32_t addr,
   int rw
 ) {
-  rpi_spi_softc_t *softc_ptr = &(((rpi_spi_desc_t *)(bushdl))->softc);
-
-  /* Save which slave will be currently addressed,
-   * so it can be unselected later. */
-  softc_ptr->current_slave_addr = addr;
 
   /* Select one of the two available SPI slave address lines. */
   switch ( addr ) {
     case 0:
     case 1:
-        /* Clear the last 2 bits (CS) and set them to addr */
-        BCM2835_REG(BCM2835_SPI_CS) = (BCM2835_REG(BCM2835_SPI_CS) & (~3) ) | addr;
+        /* Clear the last 2 bits (CS) then set them to addr */
+        BCM2835_REG(BCM2835_SPI_CS) = (BCM2835_REG(BCM2835_SPI_CS) & ~SPI_CS_CS ) | addr;
       break;
 
     default:
@@ -592,16 +539,27 @@ static int rpi_libi2c_spi_write_bytes(
  */
 static int rpi_libi2c_spi_ioctl(rtems_libi2c_bus_t * bushdl, int cmd, void *arg)
 {
+  const rtems_libi2c_read_write_t* rw;
+  const rtems_libi2c_tfm_read_write_t* rwt;
   switch ( cmd ) {
     case RTEMS_LIBI2C_IOCTL_SET_TFRMODE:
       return rpi_spi_set_tfr_mode(
                bushdl,
                (const rtems_libi2c_tfr_mode_t *)arg
              );
+      
+    case RTEMS_LIBI2C_IOCTL_READ_WRITE:
+      rw = (const rtems_libi2c_read_write_t*) arg;
+      return rpi_spi_read_write(
+                bushdl, 
+                rw->rd_buf, 
+                rw->wr_buf, 
+                rw->byte_cnt);
+      
     default:
       return -1;
   }
-
+  
   return 0;
 }
 
@@ -638,8 +596,9 @@ int rpi_spi_init(bool bidirectional_mode)
   bidirectional = bidirectional_mode;
 
   /* Clear SPI control register and clear SPI FIFOs. */
-  BCM2835_REG(BCM2835_SPI_CS) = (3 << 4);
-
+  BCM2835_REG(BCM2835_SPI_CS) = SPI_CS_CLEAR_ALL;
+  /* Deselect CS line */
+  BCM2835_REG(BCM2835_SPI_CS) |= SPI_CS_CS;
   /* Register the SPI bus. */
   return rtems_libi2c_register_bus("/dev/spi", &(rpi_spi_bus_desc.bus_desc));
 }
-- 
2.7.4




More information about the devel mailing list