Merge pull request RIOT-OS#11585 from fedepell/i2c_reg_endianess_2

periph/i2c: handle i2c register write/read helper function correctly in big endian
mgielda · May 29, 2019 · 4922321 · 4922321
2 parents 1216c5f + 6171247
commit 4922321
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 5 deletions.
diff --git a/cpu/efm32/periph/i2c.c b/cpu/efm32/periph/i2c.c
@@ -22,6 +22,7 @@
 
 #include "cpu.h"
 #include "mutex.h"
+#include "byteorder.h"
 
 #include "periph_conf.h"
 #include "periph/i2c.h"
@@ -183,16 +184,23 @@ int i2c_read_bytes(i2c_t dev, uint16_t address, void *data, size_t length, uint8
 int i2c_read_regs(i2c_t dev, uint16_t address, uint16_t reg,
                   void *data, size_t length, uint8_t flags)
 {
+    uint16_t reg_end = reg;
+
     if (flags & (I2C_NOSTART | I2C_NOSTOP)) {
         return -EOPNOTSUPP;
     }
 
+    /* Handle endianess of register if 16 bit */
+    if (flags & I2C_REG16) {
+        reg_end = htons(reg); /* Make sure register is in big-endian on I2C bus */
+    }
+
     /* prepare transfer */
     I2C_TransferSeq_TypeDef transfer;
 
     transfer.addr = (address << 1);
     transfer.flags = I2C_FLAG_WRITE_READ | ((flags & I2C_ADDR10) ? I2C_FLAG_10BIT_ADDR : 0);
-    transfer.buf[0].data = (uint8_t *) &reg;
+    transfer.buf[0].data = (uint8_t *) &reg_end;
     transfer.buf[0].len = (flags & I2C_REG16) ? 2 : 1;
     transfer.buf[1].data = (uint8_t *) data;
     transfer.buf[1].len = length;
@@ -222,16 +230,23 @@ int i2c_write_bytes(i2c_t dev, uint16_t address, const void *data, size_t length
 int i2c_write_regs(i2c_t dev, uint16_t address, uint16_t reg,
                    const void *data, size_t length, uint8_t flags)
 {
+    uint16_t reg_end = reg;
+
     if (flags & (I2C_NOSTART | I2C_NOSTOP)) {
         return -EOPNOTSUPP;
     }
 
+    /* Handle endianess of register if 16 bit */
+    if (flags & I2C_REG16) {
+        reg_end = htons(reg); /* Make sure register is in big-endian on I2C bus */
+    }
+
     /* prepare transfer */
     I2C_TransferSeq_TypeDef transfer;
 
     transfer.addr = (address << 1);
     transfer.flags = I2C_FLAG_WRITE_WRITE | ((flags & I2C_ADDR10) ? I2C_FLAG_10BIT_ADDR : 0);
-    transfer.buf[0].data = (uint8_t *) &reg;
+    transfer.buf[0].data = (uint8_t *) &reg_end;
     transfer.buf[0].len = (flags & I2C_REG16) ? 2 : 1;
     transfer.buf[1].data = (uint8_t *) data;
     transfer.buf[1].len = length;

diff --git a/drivers/include/periph/i2c.h b/drivers/include/periph/i2c.h
@@ -195,7 +195,7 @@ typedef enum {
 #ifndef HAVE_I2C_FLAGS_T
 typedef enum {
     I2C_ADDR10  = 0x01,     /**< use 10-bit device addressing */
-    I2C_REG16   = 0x02,     /**< use 16-bit register addressing */
+    I2C_REG16   = 0x02,     /**< use 16-bit register addressing, big-endian */
     I2C_NOSTOP  = 0x04,     /**< do not issue a STOP condition after transfer */
     I2C_NOSTART = 0x08,     /**< skip START sequence, ignores address field */
 } i2c_flags_t;

diff --git a/drivers/periph_common/i2c.c b/drivers/periph_common/i2c.c
@@ -22,6 +22,7 @@
 #include "board.h"
 #include "cpu.h"
 #include "periph/i2c.h"
+#include "byteorder.h"
 
 #ifdef I2C_NUMOF
 
@@ -37,11 +38,19 @@ int i2c_read_reg(i2c_t dev, uint16_t addr, uint16_t reg,
 int i2c_read_regs(i2c_t dev, uint16_t addr, uint16_t reg,
                   void *data, size_t len, uint8_t flags)
 {
+    uint16_t reg_end = reg;
+
     if (flags & (I2C_NOSTOP | I2C_NOSTART)) {
         return -EOPNOTSUPP;
     }
+
+    /* Handle endianess of register if 16 bit */
+    if (flags & I2C_REG16) {
+        reg_end = htons(reg); /* Make sure register is in big-endian on I2C bus */
+    }
+
     /* First set ADDR and register with no stop */
-    int ret = i2c_write_bytes(dev, addr, &reg, (flags & I2C_REG16) ? 2 : 1,
+    int ret = i2c_write_bytes(dev, addr, &reg_end, (flags & I2C_REG16) ? 2 : 1,
                               flags | I2C_NOSTOP);
     if (ret < 0) {
         return ret;
@@ -73,11 +82,19 @@ int i2c_write_reg(i2c_t dev, uint16_t addr, uint16_t reg,
 int i2c_write_regs(i2c_t dev, uint16_t addr, uint16_t reg,
                    const void *data, size_t len, uint8_t flags)
 {
+    uint16_t reg_end = reg;
+
     if (flags & (I2C_NOSTOP | I2C_NOSTART)) {
         return -EOPNOTSUPP;
     }
+
+    /* Handle endianess of register if 16 bit */
+    if (flags & I2C_REG16) {
+        reg_end = htons(reg); /* Make sure register is in big-endian on I2C bus */
+    }
+
     /* First set ADDR and register with no stop */
-    int ret = i2c_write_bytes(dev, addr, &reg, (flags & I2C_REG16) ? 2 : 1,
+    int ret = i2c_write_bytes(dev, addr, &reg_end, (flags & I2C_REG16) ? 2 : 1,
                               flags | I2C_NOSTOP);
     if (ret < 0) {
         return ret;