From eff46a7c2a3992ff35008ba7c0320bfdd212108b Mon Sep 17 00:00:00 2001 From: Tom Burdick Date: Fri, 18 Oct 2024 09:55:25 -0500 Subject: [PATCH] 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 --- drivers/i2c/i2c_rtio_default.c | 104 +++++++++++++++++++-------------- 1 file changed, 61 insertions(+), 43 deletions(-) diff --git a/drivers/i2c/i2c_rtio_default.c b/drivers/i2c/i2c_rtio_default.c index 048b5b42d79a59f..c484cc584467c54 100644 --- a/drivers/i2c/i2c_rtio_default.c +++ b/drivers/i2c/i2c_rtio_default.c @@ -12,97 +12,115 @@ #include 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; + /* Allocate the i2c_msg's on the stack, to do so + * the count of messages needs to be determined. + */ 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 */ + struct i2c_msg msgs[num_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); } }