From 98b38399df2b7fc84b748108ed02f7428a44cb8c Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Mon, 12 Jun 2023 21:03:17 +0200 Subject: [PATCH] cpu/qn908x/periph_i2c: enable internal pull-up on SCL Always enable the internal pull-up on the SCL line to always have a functional I2C bus. This may increase power consumption where an external pull up is present as well. But let's wait for a real world use case where this would help to extend battery life before making this configurable. This fixes https://github.com/RIOT-OS/RIOT/issues/19021 --- cpu/qn908x/include/flexcomm.h | 2 +- cpu/qn908x/periph/flexcomm.c | 4 ++- cpu/qn908x/periph/i2c.c | 68 +++++++++++++++++++++++++---------- 3 files changed, 53 insertions(+), 21 deletions(-) diff --git a/cpu/qn908x/include/flexcomm.h b/cpu/qn908x/include/flexcomm.h index 1589c34e6e..5d7f2421b2 100644 --- a/cpu/qn908x/include/flexcomm.h +++ b/cpu/qn908x/include/flexcomm.h @@ -57,7 +57,7 @@ int flexcomm_init(FLEXCOMM_Type *dev, flexcom_pselid_t mode); * For example, the flexcomm block number of FLEXCOMM2, the pointer to the * FLEXCOMM_Type block is 2. If an invalid address is passed returns -1. */ -int flexcomm_instance_from_addr(FLEXCOMM_Type *dev); +int flexcomm_instance_from_addr(const FLEXCOMM_Type *dev); #ifdef __cplusplus } diff --git a/cpu/qn908x/periph/flexcomm.c b/cpu/qn908x/periph/flexcomm.c index 80caefaddd..0659d223ee 100644 --- a/cpu/qn908x/periph/flexcomm.c +++ b/cpu/qn908x/periph/flexcomm.c @@ -27,6 +27,8 @@ #include "vendor/drivers/fsl_clock.h" +#define ENABLE_DEBUG 0 + #include "debug.h" int flexcomm_init(FLEXCOMM_Type *dev, flexcom_pselid_t mode) @@ -59,7 +61,7 @@ int flexcomm_init(FLEXCOMM_Type *dev, flexcom_pselid_t mode) return flexcomm_num; } -int flexcomm_instance_from_addr(FLEXCOMM_Type *dev) +int flexcomm_instance_from_addr(const FLEXCOMM_Type *dev) { static const FLEXCOMM_Type *flexcomm_addrs[] = FLEXCOMM_BASE_PTRS; diff --git a/cpu/qn908x/periph/i2c.c b/cpu/qn908x/periph/i2c.c index faf2f690dc..1c5fd7470d 100644 --- a/cpu/qn908x/periph/i2c.c +++ b/cpu/qn908x/periph/i2c.c @@ -32,6 +32,7 @@ #include "periph/i2c.h" #include "vendor/drivers/fsl_clock.h" +#include "vendor/drivers/fsl_iocon.h" #include "gpio_mux.h" #include "flexcomm.h" @@ -41,6 +42,22 @@ static mutex_t locks[I2C_NUMOF]; +/** + * @name Definitions for MSTSTATE bits in I2C Status register STAT + * @{ + */ +/* Controller Idle State Code */ +#define I2C_STAT_MSTSTATE_IDLE (0) +/* Controller Receive Ready State Code */ +#define I2C_STAT_MSTSTATE_RXREADY (1) +/* Controller Transmit Ready State Code */ +#define I2C_STAT_MSTSTATE_TXREADY (2) +/* Controller NACK by peripheral on address State Code */ +#define I2C_STAT_MSTSTATE_NACKADR (3) +/* Controller NACK by peripheral on data State Code */ +#define I2C_STAT_MSTSTATE_NACKDAT (4) +/** @} */ + /** * @brief Limit value I2C CLKDIV register. */ @@ -99,16 +116,41 @@ static void _i2c_init_pins(i2c_t dev) */ static const uint32_t func5_mask = (1 << 2) | (1 << 3) | (1 << 4) | (1 << 5) | (1 << 20) | (1 << 21); - /* TODO: Have a way to configure IOCON_MODE_PULLUP and IOCON_DRIVE_HIGH - * from the board. */ - gpio_init_mux(conf->pin_sda, - ((1u << GPIO_T_PIN(conf->pin_sda)) & func5_mask) ? 5 : 4); - gpio_init_mux(conf->pin_scl, - ((1u << GPIO_T_PIN(conf->pin_scl)) & func5_mask) ? 5 : 4); + + uint32_t sda_mode = IOCON_FUNC4; + if ((1u << GPIO_T_PIN(conf->pin_sda)) & func5_mask) { + sda_mode = IOCON_FUNC5; + } + + uint32_t scl_mode = IOCON_FUNC4; + if ((1u << GPIO_T_PIN(conf->pin_scl)) & func5_mask) { + sda_mode = IOCON_FUNC5; + } + + /* Using the internal pull up will make sure the I2C bus is also working + * when no external pull up is present. It can waste power by when both + * external and internal pull up are present. If a real world example + * shows up where disabling the internal pull up is helpful, we can extend + * i2c_conf_t to make it configurable */ + scl_mode |= IOCON_MODE_PULLUP; + + gpio_init_mux(conf->pin_sda, sda_mode); + gpio_init_mux(conf->pin_scl, scl_mode); mutex_unlock(&locks[dev]); } +static void _enable(I2C_Type *const i2c_dev) +{ + i2c_dev->CFG = I2C_CFG_MSTEN_MASK; + while ((i2c_dev->STAT & I2C_STAT_MSTPENDING_MASK) == 0) { + /* I2C peripheral not yet available, spin more ... */ + } + + /* I2C peripheral is online, check state is indeed idle */ + assert((i2c_dev->STAT & I2C_STAT_MSTSTATE_MASK) == I2C_STAT_MSTSTATE_IDLE); +} + void i2c_init(i2c_t dev) { assert(dev < I2C_NUMOF); @@ -126,7 +168,7 @@ void i2c_init(i2c_t dev) } /* Enable controller mode, no timeout, no monitor, no clock stretching. */ - i2c_dev->CFG = I2C_CFG_MSTEN_MASK; + _enable(i2c_dev); _i2c_controller_set_speed(i2c_dev, conf->speed); locks[dev] = (mutex_t)MUTEX_INIT_LOCKED; @@ -192,18 +234,6 @@ static uint32_t _i2c_stop(I2C_Type *i2c_dev) return status; } -/* Definitions for MSTSTATE bits in I2C Status register STAT */ - -/* Controller Idle State Code */ -#define I2C_STAT_MSTSTATE_IDLE (0) -/* Controller Receive Ready State Code */ -#define I2C_STAT_MSTSTATE_RXREADY (1) -/* Controller Transmit Ready State Code */ -#define I2C_STAT_MSTSTATE_TXREADY (2) -/* Controller NACK by peripheral on address State Code */ -#define I2C_STAT_MSTSTATE_NACKADR (3) -/* Controller NACK by peripheral on data State Code */ -#define I2C_STAT_MSTSTATE_NACKDAT (4) static int _i2c_transfer_blocking(i2c_t dev, uint32_t addr_dir, uint8_t *data, size_t len, uint8_t flags)