From 49a3ce5881227f63b95dd2bef153a391161d06b8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andreas=20Dr=C3=B6scher?= <github@anticat.ch>
Date: Fri, 16 Oct 2020 14:30:54 +0200
Subject: [PATCH] i2c-mcux: take semaphore during transfer
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Neither i2c_transfer in i2c.h nor i2c_mcux_transfer in i2c_mcux.c
have any sort of locking. If e.g. an i2c eprom is updated using a
shell and simultaneously another thread access a lm75 then one
of the two transfers will fail or produce a random result.

This changes addresses this issue by that all i2c_msgs of one
i2c_transfer are completed before allowing a subsequent transfer
to start.

The code has been validated on a FRDM_K64F.

Signed-off-by: Andreas Dröscher <github@anticat.ch>
---
 drivers/i2c/i2c_mcux.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/i2c_mcux.c b/drivers/i2c/i2c_mcux.c
index cce1e66a6c1..9b445e3d3f4 100644
--- a/drivers/i2c/i2c_mcux.c
+++ b/drivers/i2c/i2c_mcux.c
@@ -34,6 +34,7 @@ struct i2c_mcux_config {
 
 struct i2c_mcux_data {
 	i2c_master_handle_t handle;
+	struct k_sem lock;
 	struct k_sem device_sync_sem;
 	status_t callback_status;
 };
@@ -42,6 +43,7 @@ static int i2c_mcux_configure(const struct device *dev,
 			      uint32_t dev_config_raw)
 {
 	I2C_Type *base = DEV_BASE(dev);
+	struct i2c_mcux_data *data = DEV_DATA(dev);
 	const struct i2c_mcux_config *config = DEV_CFG(dev);
 	uint32_t clock_freq;
 	uint32_t baudrate;
@@ -69,7 +71,9 @@ static int i2c_mcux_configure(const struct device *dev,
 	}
 
 	clock_freq = CLOCK_GetFreq(config->clock_source);
+	k_sem_take(&data->lock, K_FOREVER);
 	I2C_MasterSetBaudRate(base, baudrate, clock_freq);
+	k_sem_give(&data->lock);
 
 	return 0;
 }
@@ -109,11 +113,15 @@ static int i2c_mcux_transfer(const struct device *dev, struct i2c_msg *msgs,
 	struct i2c_mcux_data *data = DEV_DATA(dev);
 	i2c_master_transfer_t transfer;
 	status_t status;
+	int ret = 0;
+
+	k_sem_take(&data->lock, K_FOREVER);
 
 	/* Iterate over all the messages */
 	for (int i = 0; i < num_msgs; i++) {
 		if (I2C_MSG_ADDR_10_BITS & msgs->flags) {
-			return -ENOTSUP;
+			ret = -ENOTSUP;
+			break;
 		}
 
 		/* Initialize the transfer descriptor */
@@ -142,7 +150,8 @@ static int i2c_mcux_transfer(const struct device *dev, struct i2c_msg *msgs,
 		 */
 		if (status != kStatus_Success) {
 			I2C_MasterTransferAbort(base, &data->handle);
-			return -EIO;
+			ret = -EIO;
+			break;
 		}
 
 		/* Wait for the transfer to complete */
@@ -153,14 +162,17 @@ static int i2c_mcux_transfer(const struct device *dev, struct i2c_msg *msgs,
 		 */
 		if (data->callback_status != kStatus_Success) {
 			I2C_MasterTransferAbort(base, &data->handle);
-			return -EIO;
+			ret = -EIO;
+			break;
 		}
 
 		/* Move to the next message */
 		msgs++;
 	}
 
-	return 0;
+	k_sem_give(&data->lock);
+
+	return ret;
 }
 
 static void i2c_mcux_isr(const struct device *dev)
@@ -180,6 +192,7 @@ static int i2c_mcux_init(const struct device *dev)
 	i2c_master_config_t master_config;
 	int error;
 
+	k_sem_init(&data->lock, 1, 1);
 	k_sem_init(&data->device_sync_sem, 0, UINT_MAX);
 
 	clock_freq = CLOCK_GetFreq(config->clock_source);