i2c: Fix default RTIO handler transactions

Transactions from RTIO should result in single calls to i2c_transfer.
This corrects the default handler to first count the number of
submissions in the transaction, allocate on the stack, and then copy
over each submission to an equivalent i2c_msg.

It also cleans up the helper functions to be infallible, taking only the
submission and msg to copy to.

Signed-off-by: Tom Burdick <thomas.burdick@intel.com>

drivers/i2c/Kconfig

@@ -97,6 +97,20 @@ config I2C_RTIO_CQ_SIZE
 	  is going to be 4 given the device address, register address, and a value
 	  to be read or written.
 
+config I2C_RTIO_FALLBACK_MSGS
+	int "Number of available i2c_msg structs for the default handler to use"
+	default 4
+	help
+		When RTIO is used with a driver that does not yet implement the submit API
+		natively the submissions are converted back to struct i2c_msg values that
+		are given to i2c_transfer. This requires some number of msgs be available to convert
+		the submissions into on the stack. MISRA rules dictate we must know this in
+		advance.
+
+		In all likelihood 4 is going to work for everyone, but in case you do end up with
+		an issue where you are using RTIO, your driver does not implement submit natively,
+		and get an error relating to not enough i2c msgs this is the Kconfig to manipulate.
+
 endif # I2C_RTIO
 
 

drivers/i2c/i2c_rtio_default.c

@@ -12,97 +12,126 @@
 #include <zephyr/logging/log.h>
 LOG_MODULE_DECLARE(i2c_rtio, CONFIG_I2C_LOG_LEVEL);
 
-static int i2c_iodev_submit_rx(struct rtio_iodev_sqe *iodev_sqe, struct i2c_msg msgs[2],
-			       uint8_t *num_msgs)
+static inline void i2c_msg_from_rx(const struct rtio_iodev_sqe *iodev_sqe, struct i2c_msg *msg)
 {
 	__ASSERT_NO_MSG(iodev_sqe->sqe.op == RTIO_OP_RX);
 
-	msgs[0].buf = iodev_sqe->sqe.rx.buf;
-	msgs[0].len = iodev_sqe->sqe.rx.buf_len;
-	msgs[0].flags =
+	msg->buf = iodev_sqe->sqe.rx.buf;
+	msg->len = iodev_sqe->sqe.rx.buf_len;
+	msg->flags =
 		((iodev_sqe->sqe.iodev_flags & RTIO_IODEV_I2C_STOP) ? I2C_MSG_STOP : 0) |
 		((iodev_sqe->sqe.iodev_flags & RTIO_IODEV_I2C_RESTART) ? I2C_MSG_RESTART : 0) |
 		((iodev_sqe->sqe.iodev_flags & RTIO_IODEV_I2C_10_BITS) ? I2C_MSG_ADDR_10_BITS : 0) |
 		I2C_MSG_READ;
-	*num_msgs = 1;
-	return 0;
 }
 
-static int i2c_iodev_submit_tx(struct rtio_iodev_sqe *iodev_sqe, struct i2c_msg msgs[2],
-			       uint8_t *num_msgs)
+static inline void i2c_msg_from_tx(const struct rtio_iodev_sqe *iodev_sqe, struct i2c_msg *msg)
 {
 	__ASSERT_NO_MSG(iodev_sqe->sqe.op == RTIO_OP_TX);
 
-	msgs[0].buf = (uint8_t *)iodev_sqe->sqe.tx.buf;
-	msgs[0].len = iodev_sqe->sqe.tx.buf_len;
-	msgs[0].flags =
+	msg->buf = (uint8_t *)iodev_sqe->sqe.tx.buf;
+	msg->len = iodev_sqe->sqe.tx.buf_len;
+	msg->flags =
 		((iodev_sqe->sqe.iodev_flags & RTIO_IODEV_I2C_STOP) ? I2C_MSG_STOP : 0) |
 		((iodev_sqe->sqe.iodev_flags & RTIO_IODEV_I2C_RESTART) ? I2C_MSG_RESTART : 0) |
 		((iodev_sqe->sqe.iodev_flags & RTIO_IODEV_I2C_10_BITS) ? I2C_MSG_ADDR_10_BITS : 0) |
 		I2C_MSG_WRITE;
-	*num_msgs = 1;
-	return 0;
 }
 
-static int i2c_iodev_submit_tiny_tx(struct rtio_iodev_sqe *iodev_sqe, struct i2c_msg msgs[2],
-				    uint8_t *num_msgs)
+static inline void i2c_msg_from_tiny_tx(const struct rtio_iodev_sqe *iodev_sqe, struct i2c_msg *msg)
 {
 	__ASSERT_NO_MSG(iodev_sqe->sqe.op == RTIO_OP_TINY_TX);
 
-	msgs[0].buf = (uint8_t *)iodev_sqe->sqe.tiny_tx.buf;
-	msgs[0].len = iodev_sqe->sqe.tiny_tx.buf_len;
-	msgs[0].flags =
+	msg->buf = (uint8_t *)iodev_sqe->sqe.tiny_tx.buf;
+	msg->len = iodev_sqe->sqe.tiny_tx.buf_len;
+	msg->flags =
 		((iodev_sqe->sqe.iodev_flags & RTIO_IODEV_I2C_STOP) ? I2C_MSG_STOP : 0) |
 		((iodev_sqe->sqe.iodev_flags & RTIO_IODEV_I2C_RESTART) ? I2C_MSG_RESTART : 0) |
 		((iodev_sqe->sqe.iodev_flags & RTIO_IODEV_I2C_10_BITS) ? I2C_MSG_ADDR_10_BITS : 0) |
 		I2C_MSG_WRITE;
-	*num_msgs = 1;
-	return 0;
 }
 
-void i2c_iodev_submit_work_handler(struct rtio_iodev_sqe *iodev_sqe)
+void i2c_iodev_submit_work_handler(struct rtio_iodev_sqe *txn_first)
 {
-	const struct i2c_dt_spec *dt_spec = (const struct i2c_dt_spec *)iodev_sqe->sqe.iodev->data;
+	const struct i2c_dt_spec *dt_spec = (const struct i2c_dt_spec *)txn_first->sqe.iodev->data;
 	const struct device *dev = dt_spec->bus;
 
-	LOG_DBG("Sync RTIO work item for: %p", (void *)iodev_sqe);
-
-	struct rtio_iodev_sqe *transaction_current = iodev_sqe;
-	struct i2c_msg msgs[2];
-	uint8_t num_msgs;
+	LOG_DBG("Sync RTIO work item for: %p", (void *)txn_first);
+	uint32_t num_msgs = 0;
 	int rc = 0;
+	struct rtio_iodev_sqe *txn_last = txn_first;
 
+	/* We allocate the i2c_msg's on the stack, to do so
+	 * the count of messages needs to be determined to
+	 * ensure we don't go over the statically sized array.
+	 */
 	do {
-		/* Convert the iodev_sqe back to an i2c_msg */
-		switch (transaction_current->sqe.op) {
+		switch (txn_last->sqe.op) {
+		case RTIO_OP_RX:
+		case RTIO_OP_TX:
+		case RTIO_OP_TINY_TX:
+			num_msgs++;
+			break;
+		default:
+			LOG_ERR("Invalid op code %d for submission %p", txn_last->sqe.op,
+				(void *)&txn_last->sqe);
+			rc = -EIO;
+			break;
+		}
+		txn_last = rtio_txn_next(txn_last);
+	} while (rc == 0 && txn_last != NULL);
+
+	if (rc != 0) {
+		rtio_iodev_sqe_err(txn_first, rc);
+		return;
+	}
+
+	/* Allocate msgs on the stack, MISRA doesn't like VLAs so we need a statically
+	* sized array here. It's pretty unlikely we have more than 4 i2c messages
+	* in a transaction as we typically would only have 2, one to write a
+	* register address, and another to read/write the register into an array
+	*/
+	if (num_msgs > CONFIG_I2C_RTIO_FALLBACK_MSGS) {
+		LOG_ERR("At most " CONFIG_I2C_RTIO_FALLBACK_MSGS " submissions in a transaction are "
+		        "allowed in the default handler");
+		rtio_iodev_sqe_err(txn_first, -ENOMEM);
+		return;
+	}
+	struct i2c_msg msgs[CONFIG_I2C_RTIO_FALLBACK_MSGS];
+
+	rc = 0;
+	txn_last = txn_first;
+
+	/* Copy the transaction into the stack allocated msgs */
+	for (int i = 0; i < num_msgs; i++) {
+		switch (txn_last->sqe.op) {
 		case RTIO_OP_RX:
-			rc = i2c_iodev_submit_rx(transaction_current, msgs, &num_msgs);
+			i2c_msg_from_rx(txn_last, &msgs[i]);
 			break;
 		case RTIO_OP_TX:
-			rc = i2c_iodev_submit_tx(transaction_current, msgs, &num_msgs);
+			i2c_msg_from_tx(txn_last, &msgs[i]);
 			break;
 		case RTIO_OP_TINY_TX:
-			rc = i2c_iodev_submit_tiny_tx(transaction_current, msgs, &num_msgs);
+			i2c_msg_from_tiny_tx(txn_last, &msgs[i]);
 			break;
 		default:
-			LOG_ERR("Invalid op code %d for submission %p", transaction_current->sqe.op,
-				(void *)&transaction_current->sqe);
 			rc = -EIO;
 			break;
 		}
 
-		if (rc == 0) {
-			__ASSERT_NO_MSG(num_msgs > 0);
+		txn_last = rtio_txn_next(txn_last);
+	}
 
-			rc = i2c_transfer(dev, msgs, num_msgs, dt_spec->addr);
-			transaction_current = rtio_txn_next(transaction_current);
-		}
-	} while (rc == 0 && transaction_current != NULL);
+	if (rc == 0) {
+		__ASSERT_NO_MSG(num_msgs > 0);
+
+		rc = i2c_transfer(dev, msgs, num_msgs, dt_spec->addr);
+	}
 
 	if (rc != 0) {
-		rtio_iodev_sqe_err(iodev_sqe, rc);
+		rtio_iodev_sqe_err(txn_first, rc);
 	} else {
-		rtio_iodev_sqe_ok(iodev_sqe, 0);
+		rtio_iodev_sqe_ok(txn_first, 0);
 	}
 }