drivers: i3c: Update NXP drivers transfer function

Change the transfer implementation to use interrupts
instead of a busy wait loop.
Added a Kconfig to specify a timeout period to wait
for a transfer to complete.

Signed-off-by: Mahesh Mahadevan <[email protected]>

Author: mmahadevan108

drivers/i3c/Kconfig.nxp

@@ -17,3 +17,12 @@ config I3C_MCUX
 	default y
 	help
 	  Enable mcux I3C driver.
+
+config I3C_NXP_TRANSFER_TIMEOUT
+	int "Transfer timeout [ms]"
+	default 500
+	depends on I3C_MCUX
+	help
+	  Timeout in milliseconds used for each I3C transfer.
+	  0 means that the driver should use the K_FOREVER value,
+	  i.e. it should wait as long as necessary.

drivers/i3c/i3c_mcux.c

@@ -1,6 +1,6 @@
 /*
  * Copyright (c) 2016 Freescale Semiconductor, Inc.
- * Copyright (c) 2019 NXP
+ * Copyright (c) 2019, 2025 NXP
  * Copyright (c) 2022 Intel Corporation
  *
  * SPDX-License-Identifier: Apache-2.0
@@ -65,6 +65,10 @@ LOG_MODULE_REGISTER(i3c_mcux, CONFIG_I3C_MCUX_LOG_LEVEL);
 
 #define I3C_MAX_STOP_RETRIES 5
 
+#define I3C_TRANSFER_TIMEOUT_MSEC					\
+	COND_CODE_0(CONFIG_I3C_NXP_TRANSFER_TIMEOUT, (K_FOREVER),	\
+	(K_MSEC(CONFIG_I3C_NXP_TRANSFER_TIMEOUT)))
+
 struct mcux_i3c_config {
 	/** Common I3C Driver Config */
 	struct i3c_driver_config common;
@@ -95,6 +99,9 @@ struct mcux_i3c_data {
 	/** Mutex to serialize access */
 	struct k_mutex lock;
 
+	/** Semaphore to synchronize data transfers */
+	struct k_sem device_sync_sem;
+
 	/** Condvar for waiting for bus to be in IDLE state */
 	struct k_condvar condvar;
 
@@ -886,58 +893,71 @@ static int mcux_i3c_recover_bus(const struct device *dev)
  * or time out.
  *
  * @param base Pointer to controller registers.
+ * @param data Pointer to controller device instance data.
  * @param buf Buffer to store data.
  * @param buf_sz Buffer size in bytes.
  *
  * @return Number of bytes read, or negative if error.
  */
-static int mcux_i3c_do_one_xfer_read(I3C_Type *base, uint8_t *buf, uint8_t buf_sz, bool ibi)
+static int mcux_i3c_do_one_xfer_read(I3C_Type *base, struct mcux_i3c_data *data,
+				     uint8_t *buf, uint8_t buf_sz, bool ibi)
 {
 	int ret = 0;
 	int offset = 0;
 
 	while (offset < buf_sz) {
 		/*
 		 * Transfer data from FIFO into buffer. Read
-		 * in a tight loop to reduce chance of losing
-		 * FIFO data when the i3c speed is high.
+		 * in a loop until data is unavailable in the FIFO.
 		 */
 		while (offset < buf_sz) {
 			if (mcux_i3c_fifo_rx_count_get(base) == 0) {
+				/* Enable Receive pending interrupt */
+				base->MINTSET = I3C_MSTATUS_RXPEND_MASK;
+
+				/* Wait for data to arrive or an error */
+				if (k_sem_take(&data->device_sync_sem, I3C_TRANSFER_TIMEOUT_MSEC)) {
+					ret = -ETIMEDOUT;
+				}
+				/* We break out of the loop to see if the interrupt
+				 * was due to an error.
+				 */
 				break;
+			} else {
+				buf[offset++] = (uint8_t)base->MRDATAB;
 			}
-			buf[offset++] = (uint8_t)base->MRDATAB;
 		}
-
 		/*
 		 * If controller says timed out, we abort the transaction.
 		 */
-		if (mcux_i3c_has_error(base)) {
+		if (mcux_i3c_has_error(base) || ret) {
 			if (mcux_i3c_error_is_timeout(base)) {
 				ret = -ETIMEDOUT;
 			}
-			/* clear error  */
+			/* clear error */
 			base->MERRWARN = base->MERRWARN;
 
-			/* for ibi, ignore timeout err if any bytes were
-			 * read, since the code doesn't know how many
-			 * bytes will be sent by device. for regular
-			 * application read request, return err always.
-			 */
-			if ((ret == -ETIMEDOUT) && ibi && offset) {
-				break;
-			} else {
-				if (ret == -ETIMEDOUT) {
+			if (ret == -ETIMEDOUT) {
+				/* for ibi, ignore timeout err if any bytes were
+				 * read, since the code doesn't know how many
+				 * bytes will be sent by device.
+				 */
+				if (ibi && offset) {
+					ret = offset;
+				} else {
 					LOG_ERR("Timeout error");
 				}
-				goto one_xfer_read_out;
+				break;
 			}
 		}
+
 	}
 
-	ret = offset;
+	/* If no errors, then return the number of bytes read */
+	if (ret > 0) {
+		ret = offset;
+	}
 
-one_xfer_read_out:
 	return ret;
 }
 
@@ -948,22 +968,30 @@ static int mcux_i3c_do_one_xfer_read(I3C_Type *base, uint8_t *buf, uint8_t buf_s
  * waiting for FIFO spaces.
  *
  * @param base Pointer to controller registers.
+ * @param data Pointer to controller device instance data.
  * @param buf Buffer containing data to be sent.
  * @param buf_sz Number of bytes in @p buf to send.
  * @param no_ending True if not to signal end of write message.
  *
  * @return Number of bytes written, or negative if error.
  */
-static int mcux_i3c_do_one_xfer_write(I3C_Type *base, uint8_t *buf, uint8_t buf_sz, bool no_ending)
+static int mcux_i3c_do_one_xfer_write(I3C_Type *base, struct mcux_i3c_data *data,
+				      uint8_t *buf, uint8_t buf_sz, bool no_ending)
 {
 	int offset = 0;
 	int remaining = buf_sz;
 	int ret = 0;
 
 	while (remaining > 0) {
-		ret = reg32_poll_timeout(&base->MDATACTRL, I3C_MDATACTRL_TXFULL_MASK, 0, 1000);
-		if (ret == -ETIMEDOUT) {
-			goto one_xfer_write_out;
+		if (base->MDATACTRL & I3C_MDATACTRL_TXFULL_MASK) {
+			/* Enable TX buffer ready interrupt */
+			base->MINTSET = I3C_MSTATUS_TXNOTFULL_MASK;
+
+			/* Wait for the transfer to complete */
+			ret = k_sem_take(&data->device_sync_sem, I3C_TRANSFER_TIMEOUT_MSEC);
+			if (ret) {
+				break;
+			}
 		}
 
 		if ((remaining > 1) || no_ending) {
@@ -976,9 +1004,11 @@ static int mcux_i3c_do_one_xfer_write(I3C_Type *base, uint8_t *buf, uint8_t buf_
 		remaining -= 1;
 	}
 
-	ret = offset;
+	if (!ret) {
+		/* Return the number of bytes received */
+		ret = offset;
+	}
 
-one_xfer_write_out:
 	return ret;
 }
 
@@ -1024,9 +1054,9 @@ static int mcux_i3c_do_one_xfer(I3C_Type *base, struct mcux_i3c_data *data,
 	}
 
 	if (is_read) {
-		ret = mcux_i3c_do_one_xfer_read(base, buf, buf_sz, false);
+		ret = mcux_i3c_do_one_xfer_read(base, data, buf, buf_sz, false);
 	} else {
-		ret = mcux_i3c_do_one_xfer_write(base, buf, buf_sz, no_ending);
+		ret = mcux_i3c_do_one_xfer_write(base, data, buf, buf_sz, no_ending);
 	}
 
 	if (ret < 0) {
@@ -1367,7 +1397,7 @@ static int mcux_i3c_do_ccc(const struct device *dev,
 	/* Write the CCC code */
 	mcux_i3c_status_clear_all(base);
 	mcux_i3c_errwarn_clear_all_nowait(base);
-	ret = mcux_i3c_do_one_xfer_write(base, &payload->ccc.id, 1,
+	ret = mcux_i3c_do_one_xfer_write(base, data, &payload->ccc.id, 1,
 					 payload->ccc.data_len > 0);
 	if (ret < 0) {
 		LOG_ERR("CCC[0x%02x] %s command error (%d)",
@@ -1382,7 +1412,7 @@ static int mcux_i3c_do_ccc(const struct device *dev,
 	if (payload->ccc.data_len > 0) {
 		mcux_i3c_status_clear_all(base);
 		mcux_i3c_errwarn_clear_all_nowait(base);
-		ret = mcux_i3c_do_one_xfer_write(base, payload->ccc.data,
+		ret = mcux_i3c_do_one_xfer_write(base, data, payload->ccc.data,
 						 payload->ccc.data_len, false);
 		if (ret < 0) {
 			LOG_ERR("CCC[0x%02x] %s command payload error (%d)",
@@ -1518,7 +1548,7 @@ static void mcux_i3c_ibi_work(struct k_work *work)
 	case I3C_MSTATUS_IBITYPE_IBI:
 		target = i3c_dev_list_i3c_addr_find(dev, (uint8_t)ibiaddr);
 		if (target != NULL) {
-			ret = mcux_i3c_do_one_xfer_read(base, &payload[0],
+			ret = mcux_i3c_do_one_xfer_read(base, data, &payload[0],
 							sizeof(payload), true);
 			if (ret >= 0) {
 				payload_sz = (size_t)ret;
@@ -1801,10 +1831,12 @@ int mcux_i3c_ibi_disable(const struct device *dev,
  */
 static void mcux_i3c_isr(const struct device *dev)
 {
-#ifdef CONFIG_I3C_USE_IBI
 	const struct mcux_i3c_config *config = dev->config;
 	I3C_Type *base = config->base;
+	struct mcux_i3c_data *dev_data = dev->data;
+	uint32_t interrupt_enable = base->MINTSET;
 
+#ifdef CONFIG_I3C_USE_IBI
 	/* Target initiated IBIs */
 	if (mcux_i3c_status_is_set(base, I3C_MSTATUS_SLVSTART_MASK)) {
 		int err;
@@ -1827,9 +1859,19 @@ static void mcux_i3c_isr(const struct device *dev)
 			base->MINTSET = I3C_MINTCLR_SLVSTART_MASK;
 		}
 	}
-#else
-	ARG_UNUSED(dev);
 #endif
+
+	if (interrupt_enable & I3C_MSTATUS_RXPEND_MASK) {
+		/* Disable RX buffer ready interrupt */
+		base->MINTCLR = I3C_MSTATUS_RXPEND_MASK;
+		k_sem_give(&dev_data->device_sync_sem);
+	} else if (interrupt_enable & I3C_MSTATUS_TXNOTFULL_MASK) {
+		/* Disable TX buffer ready interrupt */
+		base->MINTCLR = I3C_MSTATUS_TXNOTFULL_MASK;
+		k_sem_give(&dev_data->device_sync_sem);
+	} else {
+		/* Nothing to do right now */
+	}
 }
 
 /**
@@ -1966,6 +2008,7 @@ static int mcux_i3c_init(const struct device *dev)
 
 	k_mutex_init(&data->lock);
 	k_condvar_init(&data->condvar);
+	k_sem_init(&data->device_sync_sem, 0, K_SEM_MAX_LIMIT);
 
 	I3C_MasterGetDefaultConfig(&ctrl_config_hal);