From be3279f9bc996c910935e74c3bdba5164196a402 Mon Sep 17 00:00:00 2001 From: Victor Arino Date: Wed, 3 Feb 2016 16:58:09 +0100 Subject: [PATCH 1/5] stm32f1/i2c: fix multi byte reading --- cpu/stm32f1/periph/i2c.c | 1 + 1 file changed, 1 insertion(+) diff --git a/cpu/stm32f1/periph/i2c.c b/cpu/stm32f1/periph/i2c.c index a43dfa49f2..d09d82230e 100644 --- a/cpu/stm32f1/periph/i2c.c +++ b/cpu/stm32f1/periph/i2c.c @@ -252,6 +252,7 @@ int i2c_read_bytes(i2c_t dev, uint8_t address, char *data, int length) DEBUG("Send Slave address and wait for ADDR == 1\n"); _start(i2c, address, I2C_FLAG_READ); _clear_addr(i2c); + i2c->CR1 |= (I2C_CR1_ACK); while (i < (length - 3)) { DEBUG("Wait until byte was received\n"); From da5b03df5d2026035fba5db8c8c97d47b01dd5c6 Mon Sep 17 00:00:00 2001 From: Victor Arino Date: Wed, 3 Feb 2016 13:55:32 +0100 Subject: [PATCH 2/5] stm32f1/i2c: remove duplicated code --- cpu/stm32f1/periph/i2c.c | 108 +++++++++++++-------------------------- 1 file changed, 36 insertions(+), 72 deletions(-) diff --git a/cpu/stm32f1/periph/i2c.c b/cpu/stm32f1/periph/i2c.c index d09d82230e..7d22aaaba8 100644 --- a/cpu/stm32f1/periph/i2c.c +++ b/cpu/stm32f1/periph/i2c.c @@ -180,7 +180,6 @@ int i2c_read_byte(i2c_t dev, uint8_t address, char *data) int i2c_read_bytes(i2c_t dev, uint8_t address, char *data, int length) { - unsigned int state; int i = 0; I2C_TypeDef *i2c; @@ -194,99 +193,64 @@ int i2c_read_bytes(i2c_t dev, uint8_t address, char *data, int length) return -1; } + DEBUG("Send Slave address and wait for ADDR == 1\n"); + _start(i2c, address, I2C_FLAG_READ); + + DEBUG("Clear ADDR\n"); + _clear_addr(i2c); + switch (length) { case 1: - DEBUG("Send Slave address and wait for ADDR == 1\n"); - _start(i2c, address, I2C_FLAG_READ); - - DEBUG("Set ACK = 0\n"); - i2c->CR1 &= ~(I2C_CR1_ACK); - - DEBUG("Clear ADDR and set STOP = 1\n"); - state = disableIRQ(); - _clear_addr(i2c); - i2c->CR1 |= (I2C_CR1_STOP); - restoreIRQ(state); - - DEBUG("Wait for RXNE == 1\n"); - while (!(i2c->SR1 & I2C_SR1_RXNE)); - - DEBUG("Read received data\n"); - *data = (char)i2c->DR; - /* wait until STOP is cleared by hardware */ - while (i2c->CR1 & I2C_CR1_STOP); - /* reset ACK to be able to receive new data */ - i2c->CR1 |= (I2C_CR1_ACK); break; + case 2: - DEBUG("Send Slave address and wait for ADDR == 1\n"); - _start(i2c, address, I2C_FLAG_READ); - DEBUG("Set POS bit\n"); + DEBUG("Set POS and ACK bit\n"); i2c->CR1 |= (I2C_CR1_POS | I2C_CR1_ACK); - DEBUG("Crit block: Clear ADDR bit and clear ACK flag\n"); - state = disableIRQ(); - _clear_addr(i2c); + DEBUG("Crit block: clear ACK flag\n"); i2c->CR1 &= ~(I2C_CR1_ACK); - restoreIRQ(state); - DEBUG("Wait for transfer to be completed\n"); - while (!(i2c->SR1 & I2C_SR1_BTF)); + while (!(i2c->SR1 & I2C_SR1_BTF)) ; - DEBUG("Crit block: set STOP and read first byte\n"); - state = disableIRQ(); - i2c->CR1 |= (I2C_CR1_STOP); - data[0] = (char)i2c->DR; - restoreIRQ(state); - - DEBUG("read second byte\n"); - data[1] = (char)i2c->DR; - - DEBUG("wait for STOP bit to be cleared again\n"); - while (i2c->CR1 & I2C_CR1_STOP); - - DEBUG("reset POS = 0 and ACK = 1\n"); - i2c->CR1 &= ~(I2C_CR1_POS); - i2c->CR1 |= (I2C_CR1_ACK); break; + default: - DEBUG("Send Slave address and wait for ADDR == 1\n"); - _start(i2c, address, I2C_FLAG_READ); - _clear_addr(i2c); i2c->CR1 |= (I2C_CR1_ACK); while (i < (length - 3)) { DEBUG("Wait until byte was received\n"); - while (!(i2c->SR1 & I2C_SR1_RXNE)); + while (!(i2c->SR1 & I2C_SR1_RXNE)) ; DEBUG("Copy byte from DR\n"); data[i++] = (char)i2c->DR; } DEBUG("Reading the last 3 bytes, waiting for BTF flag\n"); - while (!(i2c->SR1 & I2C_SR1_BTF)); + while (!(i2c->SR1 & I2C_SR1_BTF)) ; - DEBUG("Disable ACK\n"); - i2c->CR1 &= ~(I2C_CR1_ACK); - - DEBUG("Crit block: set STOP and read N-2 byte\n"); - state = disableIRQ(); + DEBUG("Read N-3 byte\n"); data[i++] = (char)i2c->DR; - i2c->CR1 |= (I2C_CR1_STOP); - restoreIRQ(state); - - DEBUG("Read N-1 byte\n"); - data[i++] = (char)i2c->DR; - - while (!(i2c->SR1 & I2C_SR1_RXNE)); - DEBUG("Read last byte\n"); - data[i++] = (char)i2c->DR; - - DEBUG("wait for STOP bit to be cleared again\n"); - while (i2c->CR1 & I2C_CR1_STOP); - - DEBUG("reset POS = 0 and ACK = 1\n"); - i2c->CR1 &= ~(I2C_CR1_POS); - i2c->CR1 |= (I2C_CR1_ACK); } + + DEBUG("Clear ACK\n"); + i2c->CR1 &= ~(I2C_CR1_ACK); + + DEBUG("Setting STOP=1\n"); + i2c->CR1 |= (I2C_CR1_STOP); + + while (i < length) { + DEBUG("Wait for RXNE == 1\n"); + while (!(i2c->SR1 & I2C_SR1_RXNE)) ; + + DEBUG("Read byte\n"); + data[i++] = (char)i2c->DR; + } + + DEBUG("wait for STOP bit to be cleared again\n"); + while (i2c->CR1 & I2C_CR1_STOP) ; + + DEBUG("reset POS = 0 and ACK = 1\n"); + i2c->CR1 &= ~(I2C_CR1_POS); + i2c->CR1 |= (I2C_CR1_ACK); + return length; } From a477d6f81ddb60adb90c7b86de2b1c1a6be9b9f2 Mon Sep 17 00:00:00 2001 From: Victor Arino Date: Wed, 3 Feb 2016 14:04:48 +0100 Subject: [PATCH 3/5] stm32f1/i2c: do not block in case of error Due to the errata of some of the stm32f1xx family, the i2c lines need to be toggled when setting up the peripheral. This however seems to hang some i2c slaves which do not ack the first message sent after initialization. This caused the code to be stucked waiting for the never coming ACK. The same situation could occur when a byte was not acked due to whatever reason. The previous implementation of the i2c driver didn't allow recovery on these situations. Now the driver does not block forever but rather returns a <0 code to indicate that the transaction was not succesful. --- cpu/stm32f1/periph/i2c.c | 96 +++++++++++++++++++++++++++++----------- 1 file changed, 69 insertions(+), 27 deletions(-) diff --git a/cpu/stm32f1/periph/i2c.c b/cpu/stm32f1/periph/i2c.c index 7d22aaaba8..7e677931f1 100644 --- a/cpu/stm32f1/periph/i2c.c +++ b/cpu/stm32f1/periph/i2c.c @@ -19,6 +19,7 @@ * * @author Hauke Petersen * @author Thomas Eichinger + * @author Víctor Ariño * * @} */ @@ -41,10 +42,10 @@ /* static function definitions */ static void _i2c_init(I2C_TypeDef *i2c, int ccr); static void _pin_config(gpio_t pin_scl, gpio_t pin_sda); -static void _start(I2C_TypeDef *dev, uint8_t address, uint8_t rw_flag); +static void _start(I2C_TypeDef *dev, uint8_t address, uint8_t rw_flag, char *err); static inline void _clear_addr(I2C_TypeDef *dev); -static inline void _write(I2C_TypeDef *dev, char *data, int length); -static inline void _stop(I2C_TypeDef *dev); +static inline void _write(I2C_TypeDef *dev, char *data, int length, char *err); +static inline void _stop(I2C_TypeDef *dev, char *err); /** * @brief Array holding one pre-initialized mutex for each I2C device @@ -64,6 +65,21 @@ static mutex_t locks[] = { #endif }; +static char err_flag[] = { +#if I2C_0_EN + [I2C_0] = 0x00, +#endif +#if I2C_1_EN + [I2C_1] = 0x00, +#endif +#if I2C_2_EN + [I2C_2] = 0x00 +#endif +#if I2C_3_EN + [I2C_3] = 0x00 +#endif +}; + int i2c_init_master(i2c_t dev, i2c_speed_t speed) { I2C_TypeDef *i2c; @@ -194,7 +210,11 @@ int i2c_read_bytes(i2c_t dev, uint8_t address, char *data, int length) } DEBUG("Send Slave address and wait for ADDR == 1\n"); - _start(i2c, address, I2C_FLAG_READ); + _start(i2c, address, I2C_FLAG_READ, &err_flag[dev]); + + if (err_flag[dev]) { + return -2; + } DEBUG("Clear ADDR\n"); _clear_addr(i2c); @@ -276,11 +296,16 @@ int i2c_read_regs(i2c_t dev, uint8_t address, uint8_t reg, char *data, int lengt /* send start condition and slave address */ DEBUG("Send slave address and clear ADDR flag\n"); - _start(i2c, address, I2C_FLAG_WRITE); + _start(i2c, address, I2C_FLAG_WRITE, &err_flag[dev]); _clear_addr(i2c); DEBUG("Write reg into DR\n"); i2c->DR = reg; - _stop(i2c); + _stop(i2c, &err_flag[dev]); + + if (err_flag[dev]) { + return -2; + } + DEBUG("Now start a read transaction\n"); return i2c_read_bytes(dev, address, data, length); } @@ -306,15 +331,21 @@ int i2c_write_bytes(i2c_t dev, uint8_t address, char *data, int length) /* start transmission and send slave address */ DEBUG("sending start sequence\n"); - _start(i2c, address, I2C_FLAG_WRITE); + _start(i2c, address, I2C_FLAG_WRITE, &err_flag[dev]); _clear_addr(i2c); /* send out data bytes */ - _write(i2c, data, length); + _write(i2c, data, length, &err_flag[dev]); /* end transmission */ DEBUG("Ending transmission\n"); - _stop(i2c); + _stop(i2c, &err_flag[dev]); DEBUG("STOP condition was send out\n"); - return length; + + if (err_flag[dev]) { + return -2; + } + else { + return length; + } } int i2c_write_reg(i2c_t dev, uint8_t address, uint8_t reg, char data) @@ -337,16 +368,22 @@ int i2c_write_regs(i2c_t dev, uint8_t address, uint8_t reg, char *data, int leng } /* start transmission and send slave address */ - _start(i2c, address, I2C_FLAG_WRITE); + _start(i2c, address, I2C_FLAG_WRITE, &err_flag[dev]); _clear_addr(i2c); /* send register address and wait for complete transfer to be finished*/ - _write(i2c, (char *)(®), 1); + _write(i2c, (char *)(®), 1, &err_flag[dev]); /* write data to register */ - _write(i2c, data, length); + _write(i2c, data, length, &err_flag[dev]); /* finish transfer */ - _stop(i2c); - /* return number of bytes send */ - return length; + _stop(i2c, &err_flag[dev]); + + if (err_flag[dev]) { + return -2; + } + else { + /* return number of bytes send */ + return length; + } } void i2c_poweron(i2c_t dev) @@ -372,22 +409,24 @@ void i2c_poweroff(i2c_t dev) } } -static void _start(I2C_TypeDef *dev, uint8_t address, uint8_t rw_flag) +static void _start(I2C_TypeDef *dev, uint8_t address, uint8_t rw_flag, char *err) { + /* flag that there's no error (yet) */ + *err = 0x00; /* wait for device to be ready */ DEBUG("Wait for device to be ready\n"); - while (dev->SR2 & I2C_SR2_BUSY); + while (dev->SR2 & I2C_SR2_BUSY) ; /* generate start condition */ DEBUG("Generate start condition\n"); dev->CR1 |= I2C_CR1_START; DEBUG("Wait for SB flag to be set\n"); - while (!(dev->SR1 & I2C_SR1_SB)); + while (!(dev->SR1 & I2C_SR1_SB)) ; /* send address and read/write flag */ DEBUG("Send address\n"); dev->DR = (address << 1) | rw_flag; /* clear ADDR flag by reading first SR1 and then SR2 */ DEBUG("Wait for ADDR flag to be set\n"); - while (!(dev->SR1 & I2C_SR1_ADDR)); + while (!(dev->SR1 & I2C_SR1_ADDR) && !(*err)) ; } static inline void _clear_addr(I2C_TypeDef *dev) @@ -396,28 +435,28 @@ static inline void _clear_addr(I2C_TypeDef *dev) dev->SR2; } -static inline void _write(I2C_TypeDef *dev, char *data, int length) +static inline void _write(I2C_TypeDef *dev, char *data, int length, char *err) { DEBUG("Looping through bytes\n"); - for (int i = 0; i < length; i++) { + for (int i = 0; i < length && !(*err); i++) { /* write data to data register */ dev->DR = (uint8_t)data[i]; DEBUG("Written %i byte to data reg, now waiting for DR to be empty again\n", i); /* wait for transfer to finish */ - while (!(dev->SR1 & I2C_SR1_TXE)); + while (!(dev->SR1 & I2C_SR1_TXE) && !(*err)) ; DEBUG("DR is now empty again\n"); } } -static inline void _stop(I2C_TypeDef *dev) +static inline void _stop(I2C_TypeDef *dev, char *err) { /* make sure last byte was send */ - while (!(dev->SR1 & I2C_SR1_BTF)); + while (!(dev->SR1 & I2C_SR1_BTF) && !(*err)) ; /* send STOP condition */ dev->CR1 |= I2C_CR1_STOP; /* wait until transmission is complete */ - while (dev->SR2 & I2C_SR2_BUSY); + while (dev->SR2 & I2C_SR2_BUSY) ; } #if I2C_0_EN @@ -447,7 +486,10 @@ void I2C_0_ERR_ISR(void) if (state & I2C_SR1_SMBALERT) { DEBUG("SMBALERT\n"); } - while (1); + + /* record and clear errors */ + err_flag[I2C_0] = (state >> 8); + I2C_0_DEV->SR1 &= 0x00ff; } #endif From 57e20941f584d83a71c172c43534844fdedf048e Mon Sep 17 00:00:00 2001 From: Victor Arino Date: Wed, 3 Feb 2016 14:18:33 +0100 Subject: [PATCH 4/5] stm32f1/i2c: add support for secondary i2c --- cpu/stm32f1/periph/i2c.c | 72 +++++++++++++++++++++++++++++++++++----- 1 file changed, 63 insertions(+), 9 deletions(-) diff --git a/cpu/stm32f1/periph/i2c.c b/cpu/stm32f1/periph/i2c.c index 7e677931f1..0f6469840a 100644 --- a/cpu/stm32f1/periph/i2c.c +++ b/cpu/stm32f1/periph/i2c.c @@ -37,7 +37,7 @@ #include "debug.h" /* guard file in case no I2C device is defined */ -#if I2C_0_EN +#if I2C_0_EN || I2C_1_EN /* static function definitions */ static void _i2c_init(I2C_TypeDef *i2c, int ccr); @@ -109,6 +109,16 @@ int i2c_init_master(i2c_t dev, i2c_speed_t speed) NVIC_SetPriority(I2C_0_ERR_IRQ, I2C_IRQ_PRIO); NVIC_EnableIRQ(I2C_0_ERR_IRQ); break; +#endif +#if I2C_1_EN + case I2C_1: + i2c = I2C_1_DEV; + pin_scl = I2C_1_SCL_PIN; + pin_sda = I2C_1_SDA_PIN; + I2C_1_CLKEN(); + NVIC_SetPriority(I2C_1_ERR_IRQ, I2C_IRQ_PRIO); + NVIC_EnableIRQ(I2C_1_ERR_IRQ); + break; #endif default: return -1; @@ -204,6 +214,11 @@ int i2c_read_bytes(i2c_t dev, uint8_t address, char *data, int length) case I2C_0: i2c = I2C_0_DEV; break; +#endif +#if I2C_1_EN + case I2C_1: + i2c = I2C_1_DEV; + break; #endif default: return -1; @@ -289,6 +304,11 @@ int i2c_read_regs(i2c_t dev, uint8_t address, uint8_t reg, char *data, int lengt case I2C_0: i2c = I2C_0_DEV; break; +#endif +#if I2C_1_EN + case I2C_1: + i2c = I2C_1_DEV; + break; #endif default: return -1; @@ -324,6 +344,11 @@ int i2c_write_bytes(i2c_t dev, uint8_t address, char *data, int length) case I2C_0: i2c = I2C_0_DEV; break; +#endif +#if I2C_1_EN + case I2C_1: + i2c = I2C_1_DEV; + break; #endif default: return -1; @@ -362,6 +387,11 @@ int i2c_write_regs(i2c_t dev, uint8_t address, uint8_t reg, char *data, int leng case I2C_0: i2c = I2C_0_DEV; break; +#endif +#if I2C_1_EN + case I2C_1: + i2c = I2C_1_DEV; + break; #endif default: return -1; @@ -393,6 +423,11 @@ void i2c_poweron(i2c_t dev) case I2C_0: I2C_0_CLKEN(); break; +#endif +#if I2C_1_EN + case I2C_1: + I2C_1_CLKEN(); + break; #endif } } @@ -405,6 +440,12 @@ void i2c_poweroff(i2c_t dev) while (I2C_0_DEV->SR2 & I2C_SR2_BUSY); I2C_0_CLKDIS(); break; +#endif +#if I2C_1_EN + case I2C_1: + while (I2C_1_DEV->SR2 & I2C_SR2_BUSY); + I2C_1_CLKDIS(); + break; #endif } } @@ -459,11 +500,15 @@ static inline void _stop(I2C_TypeDef *dev, char *err) while (dev->SR2 & I2C_SR2_BUSY) ; } -#if I2C_0_EN -void I2C_0_ERR_ISR(void) +static inline void i2c_irq_handler(i2c_t i2c_dev, I2C_TypeDef *dev) { - unsigned state = I2C_0_DEV->SR1; - DEBUG("\n\n### I2C ERROR OCCURED ###\n"); + unsigned volatile state = dev->SR1; + + /* record and clear errors */ + err_flag[i2c_dev] = (state >> 8); + dev->SR1 &= 0x00ff; + + DEBUG("\n\n### I2C %d ERROR OCCURED ###\n", i2c_dev); DEBUG("status: %08x\n", state); if (state & I2C_SR1_OVR) { DEBUG("OVR\n"); @@ -486,11 +531,20 @@ void I2C_0_ERR_ISR(void) if (state & I2C_SR1_SMBALERT) { DEBUG("SMBALERT\n"); } +} - /* record and clear errors */ - err_flag[I2C_0] = (state >> 8); - I2C_0_DEV->SR1 &= 0x00ff; +#if I2C_0_EN +void I2C_0_ERR_ISR(void) +{ + i2c_irq_handler(I2C_0, I2C_0_DEV); } #endif -#endif /* I2C_0_EN */ +#if I2C_1_EN +void I2C_1_ERR_ISR(void) +{ + i2c_irq_handler(I2C_1, I2C_1_DEV); +} +#endif + +#endif /* I2C_0_EN || I2C_1_EN */ From b40cb6bebb6964908a85ad41a934126d3624303c Mon Sep 17 00:00:00 2001 From: Victor Arino Date: Wed, 3 Feb 2016 17:41:44 +0100 Subject: [PATCH 5/5] stm32f1/i2c: uncrustify untouched code --- cpu/stm32f1/periph/i2c.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpu/stm32f1/periph/i2c.c b/cpu/stm32f1/periph/i2c.c index 0f6469840a..f2701500ff 100644 --- a/cpu/stm32f1/periph/i2c.c +++ b/cpu/stm32f1/periph/i2c.c @@ -437,13 +437,13 @@ void i2c_poweroff(i2c_t dev) switch (dev) { #if I2C_0_EN case I2C_0: - while (I2C_0_DEV->SR2 & I2C_SR2_BUSY); + while (I2C_0_DEV->SR2 & I2C_SR2_BUSY) ; I2C_0_CLKDIS(); break; #endif #if I2C_1_EN case I2C_1: - while (I2C_1_DEV->SR2 & I2C_SR2_BUSY); + while (I2C_1_DEV->SR2 & I2C_SR2_BUSY) ; I2C_1_CLKDIS(); break; #endif