On 10/16/2024 8:36 PM, Andi Shyti wrote:
Hi Jyothi,
...
@@ -523,26 +576,49 @@ static int geni_i2c_gpi(struct geni_i2c_dev *gi2c, struct
i2c_msg *msg,
enum dma_transfer_direction dma_dirn;
struct dma_async_tx_descriptor *desc;
int ret;
+ struct gpi_multi_xfer *gi2c_gpi_xfer;
+ dma_cookie_t cookie;
peripheral = config->peripheral_config;
-
- dma_buf = i2c_get_dma_safe_msg_buf(msg, 1);
- if (!dma_buf)
+ gi2c_gpi_xfer = &peripheral->multi_xfer;
+ gi2c_gpi_xfer->msg_idx_cnt = cur_msg_idx;
+ dma_buf = gi2c_gpi_xfer->dma_buf[gi2c_gpi_xfer->buf_idx];
+ addr = gi2c_gpi_xfer->dma_addr[gi2c_gpi_xfer->buf_idx];
+
+ dma_buf = i2c_get_dma_safe_msg_buf(&msgs[gi2c_gpi_xfer->msg_idx_cnt],
1);
+ if (!dma_buf) {
+ gi2c->err = -ENOMEM;
return -ENOMEM;
+ }
if (op == I2C_WRITE)
map_dirn = DMA_TO_DEVICE;
else
map_dirn = DMA_FROM_DEVICE;
- addr = dma_map_single(gi2c->se.dev->parent, dma_buf, msg->len, map_dirn);
+ addr = dma_map_single(gi2c->se.dev->parent,
+ dma_buf, msgs[gi2c_gpi_xfer->msg_idx_cnt].len,
You could save msgs[gi2c_gpi_xfer->msg_idx_cnt] in a separate
variable to avoid this extra indexing.
Thanks Andi, moved gi2c_gpi_xfer->msg_idx_cnt to separate local variable.
+ map_dirn);
if (dma_mapping_error(gi2c->se.dev->parent, addr)) {
- i2c_put_dma_safe_msg_buf(dma_buf, msg, false);
+ i2c_put_dma_safe_msg_buf(dma_buf,
&msgs[gi2c_gpi_xfer->msg_idx_cnt],
+ false);
+ gi2c->err = -ENOMEM;
return -ENOMEM;
}
+ if (gi2c->is_tx_multi_xfer) {
+ if (((gi2c_gpi_xfer->msg_idx_cnt + 1) % NUM_MSGS_PER_IRQ))
+ peripheral->flags |= QCOM_GPI_BLOCK_EVENT_IRQ;
+ else
+ peripheral->flags &= ~QCOM_GPI_BLOCK_EVENT_IRQ;
+
+ /* BEI bit to be cleared for last TRE */
+ if (gi2c_gpi_xfer->msg_idx_cnt == gi2c->num_msgs - 1)
+ peripheral->flags &= ~QCOM_GPI_BLOCK_EVENT_IRQ;
+ }
+
/* set the length as message for rx txn */
- peripheral->rx_len = msg->len;
+ peripheral->rx_len = msgs[gi2c_gpi_xfer->msg_idx_cnt].len;
peripheral->op = op;
ret = dmaengine_slave_config(dma_chan, config);
@@ -560,25 +636,56 @@ static int geni_i2c_gpi(struct geni_i2c_dev *gi2c, struct
i2c_msg *msg,
else
dma_dirn = DMA_DEV_TO_MEM;
- desc = dmaengine_prep_slave_single(dma_chan, addr, msg->len, dma_dirn, flags);
+ desc = dmaengine_prep_slave_single(dma_chan, addr,
+ msgs[gi2c_gpi_xfer->msg_idx_cnt].len,
+ dma_dirn, flags);
if (!desc) {
dev_err(gi2c->se.dev, "prep_slave_sg failed\n");
- ret = -EIO;
+ gi2c->err = -EIO;
goto err_config;
}
desc->callback_result = i2c_gpi_cb_result;
desc->callback_param = gi2c;
- dmaengine_submit(desc);
- *buf = dma_buf;
- *dma_addr_p = addr;
+ if (!((msgs[cur_msg_idx].flags & I2C_M_RD) && op == I2C_WRITE)) {
+ gi2c_gpi_xfer->msg_idx_cnt++;
+ gi2c_gpi_xfer->buf_idx = (cur_msg_idx + 1) %
QCOM_GPI_MAX_NUM_MSGS;
+ }
+ cookie = dmaengine_submit(desc);
+ if (dma_submit_error(cookie)) {
+ dev_err(gi2c->se.dev,
+ "%s: dmaengine_submit failed (%d)\n", __func__, cookie);
+ return -EINVAL;
goto err_config?
yes, updated it.
+ }
+ if (gi2c->is_tx_multi_xfer) {
+ dma_async_issue_pending(gi2c->tx_c);
+ if ((cur_msg_idx == (gi2c->num_msgs - 1)) ||
+ (gi2c_gpi_xfer->msg_idx_cnt >=
+ QCOM_GPI_MAX_NUM_MSGS + gi2c_gpi_xfer->freed_msg_cnt)) {
+ ret = gpi_multi_desc_process(gi2c->se.dev,
gi2c_gpi_xfer,
+ gi2c->num_msgs,
XFER_TIMEOUT,
+ &gi2c->done);
+ if (ret) {
+ dev_dbg(gi2c->se.dev,
+ "I2C multi write msg transfer timeout:
%d\n",
+ ret);
if you are returning an error, then print an error.
sure, updated it to error in V2 patch.
+ gi2c->err = -ETIMEDOUT;
gi2c->err = ret?
Yes in this case, ret is -ETIMEDOUT, so updated in V2 patch as
gi2c->err= ret.
+ goto err_config;
+ }
+ }
+ } else {
+ /* Non multi descriptor message transfer */
+ *buf = dma_buf;
+ *dma_addr_p = addr;
+ }
return 0;
err_config:
- dma_unmap_single(gi2c->se.dev->parent, addr, msg->len, map_dirn);
- i2c_put_dma_safe_msg_buf(dma_buf, msg, false);
+ dma_unmap_single(gi2c->se.dev->parent, addr,
+ msgs[cur_msg_idx].len, map_dirn);
+ i2c_put_dma_safe_msg_buf(dma_buf, &msgs[cur_msg_idx], false);
return ret;
I would have one more label here:
out:
gi2c->err = ret;
return ret;
in order to avoid always assigning twice
Thanks, added new label as out and handled it.
}
@@ -590,6 +697,7 @@ static int geni_i2c_gpi_xfer(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], i
unsigned long time_left;
dma_addr_t tx_addr, rx_addr;
void *tx_buf = NULL, *rx_buf = NULL;
+ struct gpi_multi_xfer *tx_multi_xfer;
const struct geni_i2c_clk_fld *itr = gi2c->clk_fld;
config.peripheral_config = &peripheral;
@@ -603,6 +711,39 @@ static int geni_i2c_gpi_xfer(struct geni_i2c_dev *gi2c,
struct i2c_msg msgs[], i
peripheral.set_config = 1;
peripheral.multi_msg = false;
+ gi2c->gpi_config = &peripheral;
+ gi2c->num_msgs = num;
+ gi2c->is_tx_multi_xfer = false;
+ gi2c->tx_irq_cnt = 0;
+
+ tx_multi_xfer = &peripheral.multi_xfer;
+ tx_multi_xfer->msg_idx_cnt = 0;
+ tx_multi_xfer->buf_idx = 0;
+ tx_multi_xfer->unmap_msg_cnt = 0;
+ tx_multi_xfer->freed_msg_cnt = 0;
+ tx_multi_xfer->irq_msg_cnt = 0;
+ tx_multi_xfer->irq_cnt = 0;
you can initialize tx_multi_xfer to "{ };" to avoid all these
" = 0"
Sure, done memset tx_multi_xfer to 0 in V2 patch.
+
+ /*
+ * If number of write messages are four and higher then
+ * configure hardware for multi descriptor transfers with BEI.
+ */
+ if (num >= MIN_NUM_OF_MSGS_MULTI_DESC) {
+ gi2c->is_tx_multi_xfer = true;
+ for (i = 0; i < num; i++) {
+ if (msgs[i].flags & I2C_M_RD) {
+ /*
+ * Multi descriptor transfer with BEI
+ * support is enabled for write transfers.
+ * Add BEI optimization support for read
+ * transfers later.
+ */
+ gi2c->is_tx_multi_xfer = false;
+ break;
+ }
+ }
+ }
+
for (i = 0; i < num; i++) {
gi2c->cur = &msgs[i];
gi2c->err = 0;
@@ -613,14 +754,16 @@ static int geni_i2c_gpi_xfer(struct geni_i2c_dev *gi2c,
struct i2c_msg msgs[], i
peripheral.stretch = 1;
peripheral.addr = msgs[i].addr;
+ if (i > 0 && (!(msgs[i].flags & I2C_M_RD)))
+ peripheral.multi_msg = false;
- ret = geni_i2c_gpi(gi2c, &msgs[i], &config,
+ ret = geni_i2c_gpi(gi2c, msgs, i, &config,
what is the point of passing 'i' if you always refer to msgs[i]
in geni_i2c_gpi()?
Handled with new variable in "geni_i2c_gpi "and so no need to pass
current i2c msg index, removed it in V2 patch.
&tx_addr, &tx_buf, I2C_WRITE, gi2c->tx_c);
if (ret)
goto err;
if (msgs[i].flags & I2C_M_RD) {
- ret = geni_i2c_gpi(gi2c, &msgs[i], &config,
+ ret = geni_i2c_gpi(gi2c, msgs, i, &config,
&rx_addr, &rx_buf, I2C_READ,
gi2c->rx_c);
if (ret)
goto err;
@@ -628,18 +771,28 @@ static int geni_i2c_gpi_xfer(struct geni_i2c_dev *gi2c,
struct i2c_msg msgs[], i
dma_async_issue_pending(gi2c->rx_c);
}
- dma_async_issue_pending(gi2c->tx_c);
-
- time_left = wait_for_completion_timeout(&gi2c->done,
XFER_TIMEOUT);
- if (!time_left)
- gi2c->err = -ETIMEDOUT;
+ if (!gi2c->is_tx_multi_xfer) {
+ dma_async_issue_pending(gi2c->tx_c);
+ time_left = wait_for_completion_timeout(&gi2c->done,
XFER_TIMEOUT);
+ if (!time_left) {
+ dev_err(gi2c->se.dev, "%s:I2C timeout\n",
__func__);
+ gi2c->err = -ETIMEDOUT;
+ }
+ }
if (gi2c->err) {
ret = gi2c->err;
goto err;
}
- geni_i2c_gpi_unmap(gi2c, &msgs[i], tx_buf, tx_addr, rx_buf, rx_addr);
+ if (!gi2c->is_tx_multi_xfer) {
+ geni_i2c_gpi_unmap(gi2c, &msgs[i], tx_buf, tx_addr,
rx_buf, rx_addr);
+ } else {
+ if (gi2c->tx_irq_cnt != tx_multi_xfer->irq_cnt) {
else if (...) {
...
}
Sure, else if used here in V2 patch.
Andi
Regards,
JyothiKumar
