i2c: nrfx_twi: Refactor nrfx_twi to utilize common code

Code is refactored to take advantage of common codebase shared with
i2c_nrfx_twi_rtio driver.

Signed-off-by: Luis Ubieda <luisf@croxel.com>

drivers/i2c/CMakeLists.txt

@@ -36,7 +36,10 @@ if(CONFIG_I2C_RTIO)
 	i2c_nrfx_twi_common.c
 	)
 else()
-  zephyr_library_sources_ifdef(CONFIG_I2C_NRFX_TWI	i2c_nrfx_twi.c)
+  zephyr_library_sources_ifdef(CONFIG_I2C_NRFX_TWI
+	i2c_nrfx_twi.c
+	i2c_nrfx_twi_common.c
+	)
 endif()
 
 zephyr_library_sources_ifdef(CONFIG_I2C_NRFX_TWIM	i2c_nrfx_twim.c)

drivers/i2c/i2c_nrfx_twi.c

@@ -11,6 +11,7 @@
 #include <zephyr/drivers/pinctrl.h>
 #include <soc.h>
 #include <nrfx_twi.h>
+#include "i2c_nrfx_twi_common.h"
 
 #include <zephyr/logging/log.h>
 #include <zephyr/irq.h>
@@ -23,19 +24,19 @@ LOG_MODULE_REGISTER(i2c_nrfx_twi, CONFIG_I2C_LOG_LEVEL);
 #endif
 
 struct i2c_nrfx_twi_data {
+	uint32_t dev_config;
 	struct k_sem transfer_sync;
 	struct k_sem completion_sync;
 	volatile nrfx_err_t res;
-	uint32_t dev_config;
 };
 
-struct i2c_nrfx_twi_config {
+/* Enforce dev_config matches the same offset as the common structure,
-	nrfx_twi_t twi;
+ * otherwise common API won't be compatible with i2c_nrfx_twi.
-	nrfx_twi_config_t config;
+ */
-	const struct pinctrl_dev_config *pcfg;
+BUILD_ASSERT(
-};
+	offsetof(struct i2c_nrfx_twi_data, dev_config) ==
-
+	offsetof(struct i2c_nrfx_twi_common_data, dev_config)
-static int i2c_nrfx_twi_recover_bus(const struct device *dev);
+);
 
 static int i2c_nrfx_twi_transfer(const struct device *dev,
 				 struct i2c_msg *msgs,
@@ -53,60 +54,17 @@ static int i2c_nrfx_twi_transfer(const struct device *dev,
 	nrfx_twi_enable(&config->twi);
 
 	for (size_t i = 0; i < num_msgs; i++) {
-		if (I2C_MSG_ADDR_10_BITS & msgs[i].flags) {
+		bool more_msgs = ((i < (num_msgs - 1)) &&
-			ret = -ENOTSUP;
+				  !(msgs[i + 1].flags & I2C_MSG_RESTART));
+
+		ret = i2c_nrfx_twi_msg_transfer(dev, msgs[i].flags,
+						msgs[i].buf,
+						msgs[i].len, addr,
+						more_msgs);
+		if (ret) {
 			break;
 		}
 
-		nrfx_twi_xfer_desc_t cur_xfer = {
-			.p_primary_buf  = msgs[i].buf,
-			.primary_length = msgs[i].len,
-			.address	= addr,
-			.type		= (msgs[i].flags & I2C_MSG_READ) ?
-					  NRFX_TWI_XFER_RX : NRFX_TWI_XFER_TX
-		};
-		uint32_t xfer_flags = 0;
-		nrfx_err_t res;
-
-		/* In case the STOP condition is not supposed to appear after
-		 * the current message, check what is requested further:
-		 */
-		if (!(msgs[i].flags & I2C_MSG_STOP)) {
-			/* - if the transfer consists of more messages
-			 *   and the I2C repeated START is not requested
-			 *   to appear before the next message, suspend
-			 *   the transfer after the current message,
-			 *   so that it can be resumed with the next one,
-			 *   resulting in the two messages merged into
-			 *   a continuous transfer on the bus
-			 */
-			if ((i < (num_msgs - 1)) &&
-			    !(msgs[i + 1].flags & I2C_MSG_RESTART)) {
-				xfer_flags |= NRFX_TWI_FLAG_SUSPEND;
-			/* - otherwise, just finish the transfer without
-			 *   generating the STOP condition, unless the current
-			 *   message is an RX request, for which such feature
-			 *   is not supported
-			 */
-			} else if (msgs[i].flags & I2C_MSG_READ) {
-				ret = -ENOTSUP;
-				break;
-			} else {
-				xfer_flags |= NRFX_TWI_FLAG_TX_NO_STOP;
-			}
-		}
-
-		res = nrfx_twi_xfer(&config->twi, &cur_xfer, xfer_flags);
-		if (res != NRFX_SUCCESS) {
-			if (res == NRFX_ERROR_BUSY) {
-				ret = -EBUSY;
-				break;
-			} else {
-				ret = -EIO;
-				break;
-			}
-		}
-
 		ret = k_sem_take(&data->completion_sync,
 				 I2C_TRANSFER_TIMEOUT_MSEC);
 		if (ret != 0) {
@@ -132,8 +90,7 @@ static int i2c_nrfx_twi_transfer(const struct device *dev,
 			break;
 		}
 
-		res = data->res;
+		if (data->res != NRFX_SUCCESS) {
-		if (res != NRFX_SUCCESS) {
 			ret = -EIO;
 			break;
 		}
@@ -147,7 +104,8 @@ static int i2c_nrfx_twi_transfer(const struct device *dev,
 
 static void event_handler(nrfx_twi_evt_t const *p_event, void *p_context)
 {
-	struct i2c_nrfx_twi_data *dev_data = p_context;
+	const struct device *dev = p_context;
+	struct i2c_nrfx_twi_data *dev_data = (struct i2c_nrfx_twi_data *)dev->data;
 
 	switch (p_event->type) {
 	case NRFX_TWI_EVT_DONE:
@@ -167,115 +125,12 @@ static void event_handler(nrfx_twi_evt_t const *p_event, void *p_context)
 	k_sem_give(&dev_data->completion_sync);
 }
 
-static int i2c_nrfx_twi_configure(const struct device *dev,
-				  uint32_t dev_config)
-{
-	const struct i2c_nrfx_twi_config *config = dev->config;
-	struct i2c_nrfx_twi_data *data = dev->data;
-	nrfx_twi_t const *inst = &config->twi;
-
-	if (I2C_ADDR_10_BITS & dev_config) {
-		return -EINVAL;
-	}
-
-	switch (I2C_SPEED_GET(dev_config)) {
-	case I2C_SPEED_STANDARD:
-		nrf_twi_frequency_set(inst->p_twi, NRF_TWI_FREQ_100K);
-		break;
-	case I2C_SPEED_FAST:
-		nrf_twi_frequency_set(inst->p_twi, NRF_TWI_FREQ_400K);
-		break;
-	default:
-		LOG_ERR("unsupported speed");
-		return -EINVAL;
-	}
-	data->dev_config = dev_config;
-
-	return 0;
-}
-
-static int i2c_nrfx_twi_recover_bus(const struct device *dev)
-{
-	const struct i2c_nrfx_twi_config *config = dev->config;
-	uint32_t scl_pin;
-	uint32_t sda_pin;
-	nrfx_err_t err;
-
-	scl_pin = nrf_twi_scl_pin_get(config->twi.p_twi);
-	sda_pin = nrf_twi_sda_pin_get(config->twi.p_twi);
-
-	err = nrfx_twi_bus_recover(scl_pin, sda_pin);
-	return (err == NRFX_SUCCESS ? 0 : -EBUSY);
-}
-
 static const struct i2c_driver_api i2c_nrfx_twi_driver_api = {
 	.configure   = i2c_nrfx_twi_configure,
 	.transfer    = i2c_nrfx_twi_transfer,
 	.recover_bus = i2c_nrfx_twi_recover_bus,
 };
 
-static int init_twi(const struct device *dev)
-{
-	const struct i2c_nrfx_twi_config *config = dev->config;
-	struct i2c_nrfx_twi_data *dev_data = dev->data;
-	nrfx_err_t result = nrfx_twi_init(&config->twi, &config->config,
-					  event_handler, dev_data);
-	if (result != NRFX_SUCCESS) {
-		LOG_ERR("Failed to initialize device: %s",
-			    dev->name);
-		return -EBUSY;
-	}
-
-	return 0;
-}
-
-#ifdef CONFIG_PM_DEVICE
-static int twi_nrfx_pm_action(const struct device *dev,
-			      enum pm_device_action action)
-{
-	const struct i2c_nrfx_twi_config *config = dev->config;
-	struct i2c_nrfx_twi_data *data = dev->data;
-	int ret = 0;
-
-	switch (action) {
-	case PM_DEVICE_ACTION_RESUME:
-		ret = pinctrl_apply_state(config->pcfg, PINCTRL_STATE_DEFAULT);
-		if (ret < 0) {
-			return ret;
-		}
-		init_twi(dev);
-		if (data->dev_config) {
-			i2c_nrfx_twi_configure(dev, data->dev_config);
-		}
-		break;
-
-	case PM_DEVICE_ACTION_SUSPEND:
-		nrfx_twi_uninit(&config->twi);
-
-		ret = pinctrl_apply_state(config->pcfg, PINCTRL_STATE_SLEEP);
-		if (ret < 0) {
-			return ret;
-		}
-		break;
-
-	default:
-		ret = -ENOTSUP;
-	}
-
-	return ret;
-}
-#endif /* CONFIG_PM_DEVICE */
-
-#define I2C_NRFX_TWI_INVALID_FREQUENCY  ((nrf_twi_frequency_t)-1)
-#define I2C_NRFX_TWI_FREQUENCY(bitrate)					       \
-	 (bitrate == I2C_BITRATE_STANDARD ? NRF_TWI_FREQ_100K		       \
-	: bitrate == 250000               ? NRF_TWI_FREQ_250K		       \
-	: bitrate == I2C_BITRATE_FAST     ? NRF_TWI_FREQ_400K		       \
-					  : I2C_NRFX_TWI_INVALID_FREQUENCY)
-#define I2C(idx) DT_NODELABEL(i2c##idx)
-#define I2C_FREQUENCY(idx)						       \
-	I2C_NRFX_TWI_FREQUENCY(DT_PROP(I2C(idx), clock_frequency))
-
 #define I2C_NRFX_TWI_DEVICE(idx)					       \
 	NRF_DT_CHECK_NODE_HAS_PINCTRL_SLEEP(I2C(idx));			       \
 	BUILD_ASSERT(I2C_FREQUENCY(idx)	!=				       \
@@ -291,7 +146,7 @@ static int twi_nrfx_pm_action(const struct device *dev,
 		if (err < 0) {						       \
 			return err;					       \
 		}							       \
-		return init_twi(dev);					       \
+		return i2c_nrfx_twi_init(dev);				       \
 	}								       \
 	static struct i2c_nrfx_twi_data twi_##idx##_data = {		       \
 		.transfer_sync = Z_SEM_INITIALIZER(                            \
@@ -307,6 +162,7 @@ static int twi_nrfx_pm_action(const struct device *dev,
 			.skip_psel_cfg = true,				       \
 			.frequency = I2C_FREQUENCY(idx),		       \
 		},							       \
+		.event_handler = event_handler,				       \
 		.pcfg = PINCTRL_DT_DEV_CONFIG_GET(I2C(idx)),		       \
 	};								       \
 	PM_DEVICE_DT_DEFINE(I2C(idx), twi_nrfx_pm_action);		       \