Merge pull request #5927 from keestux/sam21_i2c_wait_for_response

sam21_common:i2c refactor a function to wait for response

cpu/sam0_common/periph/i2c.c

@@ -44,10 +44,11 @@
 static void _i2c_poweron(SercomI2cm *sercom);
 static void _i2c_poweroff(SercomI2cm *sercom);
 
-static inline int _start(SercomI2cm *dev, uint8_t address, uint8_t rw_flag);
+static int _start(SercomI2cm *dev, uint8_t address, uint8_t rw_flag);
 static inline int _write(SercomI2cm *dev, const uint8_t *data, int length);
 static inline int _read(SercomI2cm *dev, uint8_t *data, int length);
 static inline void _stop(SercomI2cm *dev);
+static inline int _wait_for_response(SercomI2cm *dev, uint32_t max_timeout_counter);
 
 /**
  * @brief Array holding one pre-initialized mutex for each I2C device
@@ -89,7 +90,7 @@ int i2c_init_master(i2c_t dev, i2c_speed_t speed)
             mux = I2C_0_MUX;
             clock_source_speed = CLOCK_CORECLOCK;
             sercom_gclk_id = I2C_0_GCLK_ID;
-            sercom_gclk_id_slow = I2C_0_GCLK_ID_SLOW ;
+            sercom_gclk_id_slow = I2C_0_GCLK_ID_SLOW;
             break;
 #endif
         default:
@@ -102,7 +103,7 @@ int i2c_init_master(i2c_t dev, i2c_speed_t speed)
 
     /* Reset I2C */
     I2CSercom->CTRLA.reg = SERCOM_I2CS_CTRLA_SWRST;
-    while(I2CSercom->SYNCBUSY.reg & SERCOM_I2CM_SYNCBUSY_MASK) {}
+    while (I2CSercom->SYNCBUSY.reg & SERCOM_I2CM_SYNCBUSY_MASK) {}
 
     /* Turn on power manager for sercom */
     PM->APBCMASK.reg |= (PM_APBCMASK_SERCOM0 << (sercom_gclk_id - GCLK_CLKCTRL_ID_SERCOM0_CORE_Val));
@@ -135,7 +136,7 @@ int i2c_init_master(i2c_t dev, i2c_speed_t speed)
     gpio_init_mux(pin_scl, mux);
 
     /* I2C CONFIGURATION */
-    while(I2CSercom->SYNCBUSY.reg & SERCOM_I2CM_SYNCBUSY_MASK) {}
+    while (I2CSercom->SYNCBUSY.reg & SERCOM_I2CM_SYNCBUSY_MASK) {}
 
     /* Set sercom module to operate in I2C master mode. */
     I2CSercom->CTRLA.reg = SERCOM_I2CM_CTRLA_MODE_I2C_MASTER;
@@ -148,24 +149,24 @@ int i2c_init_master(i2c_t dev, i2c_speed_t speed)
      * kHz and Fast-mode (Fm) up to 400 kHz */
     switch (speed) {
         case I2C_SPEED_NORMAL:
-            tmp_baud = (int32_t)(((clock_source_speed + (2*(100000)) - 1) / (2*(100000))) - 5);
+            tmp_baud = (int32_t)(((clock_source_speed + (2 * (100000)) - 1) / (2 * (100000))) - 5);
             if (tmp_baud < 255 && tmp_baud > 0) {
                 I2CSercom->CTRLA.reg |= SERCOM_I2CM_CTRLA_SPEED(0);
                 I2CSercom->BAUD.reg = SERCOM_I2CM_BAUD_BAUD(tmp_baud);
             }
             break;
         case I2C_SPEED_FAST:
-            tmp_baud = (int32_t)(((clock_source_speed + (2*(400000)) - 1) / (2*(400000))) - 5);
+            tmp_baud = (int32_t)(((clock_source_speed + (2 * (400000)) - 1) / (2 * (400000))) - 5);
             if (tmp_baud < 255 && tmp_baud > 0) {
                 I2CSercom->CTRLA.reg |= SERCOM_I2CM_CTRLA_SPEED(0);
                 I2CSercom->BAUD.reg = SERCOM_I2CM_BAUD_BAUD(tmp_baud);
             }
             break;
         case I2C_SPEED_HIGH:
-            tmp_baud = (int32_t)(((clock_source_speed + (2*(3400000)) - 1) / (2*(3400000))) - 1);
+            tmp_baud = (int32_t)(((clock_source_speed + (2 * (3400000)) - 1) / (2 * (3400000))) - 1);
             if (tmp_baud < 255 && tmp_baud > 0) {
                 I2CSercom->CTRLA.reg |= SERCOM_I2CM_CTRLA_SPEED(2);
-                I2CSercom->BAUD.reg =SERCOM_I2CM_BAUD_HSBAUD(tmp_baud);
+                I2CSercom->BAUD.reg = SERCOM_I2CM_BAUD_HSBAUD(tmp_baud);
             }
             break;
         default:
@@ -178,7 +179,7 @@ int i2c_init_master(i2c_t dev, i2c_speed_t speed)
 
     /* Start timeout if bus state is unknown. */
     while ((I2CSercom->STATUS.reg & SERCOM_I2CM_STATUS_BUSSTATE_Msk) == BUSSTATE_UNKNOWN) {
-        if(timeout_counter++ >= SAMD21_I2C_TIMEOUT) {
+        if (timeout_counter++ >= SAMD21_I2C_TIMEOUT) {
             /* Timeout, force bus state to idle. */
             I2CSercom->STATUS.reg = BUSSTATE_IDLE;
         }
@@ -224,9 +225,13 @@ int i2c_read_bytes(i2c_t dev, uint8_t address, void *data, int length)
     }
 
     /* start transmission and send slave address */
-    if(_start(i2c, address, I2C_FLAG_READ) < 0) return 0;
+    if (_start(i2c, address, I2C_FLAG_READ) < 0) {
+        return 0;
+    }
     /* read data to register */
-    if(_read(i2c, data, length) < 0) return 0;
+    if (_read(i2c, data, length) < 0) {
+        return 0;
+    }
     _stop(i2c);
     /* return number of bytes sent */
     return length;
@@ -252,10 +257,14 @@ int i2c_read_regs(i2c_t dev, uint8_t address, uint8_t reg, void *data, int lengt
     }
 
     /* start transmission and send slave address */
-    if (_start(i2c, address, I2C_FLAG_WRITE) < 0) return 0;
+    if (_start(i2c, address, I2C_FLAG_WRITE) < 0) {
+        return 0;
+    }
     /* send register address/command and wait for complete transfer to
      * be finished */
-    if (_write(i2c, &reg, 1) < 0) return 0;
+    if (_write(i2c, &reg, 1) < 0) {
+        return 0;
+    }
     return i2c_read_bytes(dev, address, data, length);
 }
 
@@ -278,8 +287,12 @@ int i2c_write_bytes(i2c_t dev, uint8_t address, const void *data, int length)
             return -1;
     }
 
-    if(_start(I2CSercom, address, I2C_FLAG_WRITE) < 0) return 0;
+    if (_start(I2CSercom, address, I2C_FLAG_WRITE) < 0) {
-    if(_write(I2CSercom, data, length) < 0) return 0;
+        return 0;
+    }
+    if (_write(I2CSercom, data, length) < 0) {
+        return 0;
+    }
     _stop(I2CSercom);
     return length;
 }
@@ -302,14 +315,20 @@ int i2c_write_regs(i2c_t dev, uint8_t address, uint8_t reg, const void *data, in
 #endif
         default:
             return -1;
-        }
+    }
 
     /* start transmission and send slave address */
-    if (_start(i2c, address, I2C_FLAG_WRITE) < 0) return 0;
+    if (_start(i2c, address, I2C_FLAG_WRITE) < 0) {
+        return 0;
+    }
     /* send register address and wait for complete transfer to be finished */
-    if (_write(i2c, &reg, 1) < 0) return 0;
+    if (_write(i2c, &reg, 1) < 0) {
+        return 0;
+    }
     /* write data to register */
-    if (_write(i2c, data, length) < 0) return 0;
+    if (_write(i2c, data, length) < 0) {
+        return 0;
+    }
     /* finish transfer */
     _stop(i2c);
     return length;
@@ -361,11 +380,9 @@ void i2c_poweroff(i2c_t dev)
 
 static int _start(SercomI2cm *dev, uint8_t address, uint8_t rw_flag)
 {
-    uint32_t timeout_counter = 0;
-
     /* Wait for hardware module to sync */
     DEBUG("Wait for device to be ready\n");
-    while(dev->SYNCBUSY.reg & SERCOM_I2CM_SYNCBUSY_MASK) {}
+    while (dev->SYNCBUSY.reg & SERCOM_I2CM_SYNCBUSY_MASK) {}
 
     /* Set action to ACK. */
     dev->CTRLB.reg &= ~SERCOM_I2CM_CTRLB_ACKACT;
@@ -375,12 +392,14 @@ static int _start(SercomI2cm *dev, uint8_t address, uint8_t rw_flag)
     dev->ADDR.reg = (address << 1) | rw_flag | (0 << SERCOM_I2CM_ADDR_HS_Pos);
 
     /* Wait for response on bus. */
-    while (!(dev->INTFLAG.reg & SERCOM_I2CM_INTFLAG_MB)
+    if (rw_flag == I2C_FLAG_READ) {
-           && !(dev->INTFLAG.reg & SERCOM_I2CM_INTFLAG_SB)) {
+        /* Some devices (e.g. SHT2x) can hold the bus while preparing the reply */
-        if (++timeout_counter >= SAMD21_I2C_TIMEOUT) {
+        if (_wait_for_response(dev, 100 * SAMD21_I2C_TIMEOUT) < 0)
-            DEBUG("STATUS_ERR_TIMEOUT\n");
+            return -1;
+    }
+    else {
+        if (_wait_for_response(dev, SAMD21_I2C_TIMEOUT) < 0)
             return -1;
-        }
     }
 
     /* Check for address response error unless previous error is detected. */
@@ -408,7 +427,6 @@ static int _start(SercomI2cm *dev, uint8_t address, uint8_t rw_flag)
 static inline int _write(SercomI2cm *dev, const uint8_t *data, int length)
 {
     uint16_t tmp_data_length = length;
-    uint32_t timeout_counter = 0;
     uint16_t buffer_counter = 0;
 
     /* Write data buffer until the end. */
@@ -421,20 +439,14 @@ static inline int _write(SercomI2cm *dev, const uint8_t *data, int length)
         }
 
         /* Wait for hardware module to sync */
-        while(dev->SYNCBUSY.reg & SERCOM_I2CM_SYNCBUSY_MASK) {}
+        while (dev->SYNCBUSY.reg & SERCOM_I2CM_SYNCBUSY_MASK) {}
 
         DEBUG("Written byte #%i to data reg, now waiting for DR to be empty again\n", buffer_counter);
         dev->DATA.reg = data[buffer_counter++];
 
-        DEBUG("Wait for response.\n");
+        /* Wait for response on bus. */
-        timeout_counter = 0;
+        if (_wait_for_response(dev, SAMD21_I2C_TIMEOUT) < 0)
-        while (!(dev->INTFLAG.reg & SERCOM_I2CM_INTFLAG_MB)
+            return -1;
-               && !(dev->INTFLAG.reg & SERCOM_I2CM_INTFLAG_SB)) {
-            if (++timeout_counter >= SAMD21_I2C_TIMEOUT) {
-                DEBUG("STATUS_ERR_TIMEOUT\n");
-                return -1;
-            }
-        }
 
         /* Check for NACK from slave. */
         if (dev->STATUS.reg & SERCOM_I2CM_STATUS_RXNACK) {
@@ -447,7 +459,6 @@ static inline int _write(SercomI2cm *dev, const uint8_t *data, int length)
 
 static inline int _read(SercomI2cm *dev, uint8_t *data, int length)
 {
-    uint32_t timeout_counter = 0;
     uint8_t count = 0;
 
     /* Set action to ack. */
@@ -462,19 +473,14 @@ static inline int _read(SercomI2cm *dev, uint8_t *data, int length)
         }
 
         /* Wait for hardware module to sync */
-        while(dev->SYNCBUSY.reg & SERCOM_I2CM_SYNCBUSY_MASK) {}
+        while (dev->SYNCBUSY.reg & SERCOM_I2CM_SYNCBUSY_MASK) {}
         /* Save data to buffer. */
         data[count] = dev->DATA.reg;
 
-        /* Wait for response. */
+        /* Wait for response on bus. */
-        timeout_counter = 0;
+        if (_wait_for_response(dev, SAMD21_I2C_TIMEOUT) < 0)
-        while (!(dev->INTFLAG.reg & SERCOM_I2CM_INTFLAG_MB)
+            return -1;
-               && !(dev->INTFLAG.reg & SERCOM_I2CM_INTFLAG_SB)) {
+
-            if (++timeout_counter >= SAMD21_I2C_TIMEOUT) {
-                DEBUG("STATUS_ERR_TIMEOUT\n");
-                return -1;
-            }
-        }
         count++;
     }
     /* Send NACK before STOP */
@@ -485,12 +491,26 @@ static inline int _read(SercomI2cm *dev, uint8_t *data, int length)
 static inline void _stop(SercomI2cm *dev)
 {
     /* Wait for hardware module to sync */
-    while(dev->SYNCBUSY.reg & SERCOM_I2CM_SYNCBUSY_MASK) {}
+    while (dev->SYNCBUSY.reg & SERCOM_I2CM_SYNCBUSY_MASK) {}
     /* Stop command */
     dev->CTRLB.reg |= SERCOM_I2CM_CTRLB_CMD(3);
     /* Wait for bus to be idle again */
-    while((dev->STATUS.reg & SERCOM_I2CM_STATUS_BUSSTATE_Msk) != BUSSTATE_IDLE) {}
+    while ((dev->STATUS.reg & SERCOM_I2CM_STATUS_BUSSTATE_Msk) != BUSSTATE_IDLE) {}
     DEBUG("Stop sent\n");
 }
 
+static inline int _wait_for_response(SercomI2cm *dev, uint32_t max_timeout_counter)
+{
+    uint32_t timeout_counter = 0;
+    DEBUG("Wait for response.\n");
+    while (!(dev->INTFLAG.reg & SERCOM_I2CM_INTFLAG_MB)
+           && !(dev->INTFLAG.reg & SERCOM_I2CM_INTFLAG_SB)) {
+        if (++timeout_counter >= max_timeout_counter) {
+            DEBUG("STATUS_ERR_TIMEOUT\n");
+            return -1;
+        }
+    }
+    return 0;
+}
+
 #endif /* I2C_NUMOF */