| Message ID | 20241015120750.21217-6-quic_jseerapu@quicinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add Block event interrupt support for I2C protocol | expand |
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. > + 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? > + } > > + 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. > + gi2c->err = -ETIMEDOUT; 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 > } > > @@ -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" > + > + /* > + * 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()? > &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 (...) { ... } Andi
Hi Jyothi, kernel test robot noticed the following build errors: [auto build test ERROR on 55bcd2e0d04c1171d382badef1def1fd04ef66c5] url: https://github.com/intel-lab-lkp/linux/commits/Jyothi-Kumar-Seerapu/dt-bindings-dmaengine-qcom-gpi-Add-additional-arg-to-dma-cell-property/20241015-202637 base: 55bcd2e0d04c1171d382badef1def1fd04ef66c5 patch link: https://lore.kernel.org/r/20241015120750.21217-6-quic_jseerapu%40quicinc.com patch subject: [PATCH v1 5/5] i2c: i2c-qcom-geni: Add Block event interrupt support config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20241019/202410190549.hGAfByqg-lkp@intel.com/config) compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241019/202410190549.hGAfByqg-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202410190549.hGAfByqg-lkp@intel.com/ All errors (new ones prefixed by >>): >> drivers/i2c/busses/i2c-qcom-geni.c:562:8: error: incompatible pointer to integer conversion passing 'dma_addr_t *' (aka 'unsigned long long *') to parameter of type 'dma_addr_t' (aka 'unsigned long long'); dereference with * [-Wint-conversion] 562 | tx_multi_xfer->dma_addr[wr_idx], NULL, NULL); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | * drivers/i2c/busses/i2c-qcom-geni.c:519:36: note: passing argument to parameter 'tx_addr' here 519 | void *tx_buf, dma_addr_t tx_addr, | ^ >> drivers/i2c/busses/i2c-qcom-geni.c:562:47: error: incompatible pointer to integer conversion passing 'void *' to parameter of type 'dma_addr_t' (aka 'unsigned long long') [-Wint-conversion] 562 | tx_multi_xfer->dma_addr[wr_idx], NULL, NULL); | ^~~~ include/linux/stddef.h:8:14: note: expanded from macro 'NULL' 8 | #define NULL ((void *)0) | ^~~~~~~~~~~ drivers/i2c/busses/i2c-qcom-geni.c:520:36: note: passing argument to parameter 'rx_addr' here 520 | void *rx_buf, dma_addr_t rx_addr) | ^ >> drivers/i2c/busses/i2c-qcom-geni.c:586:7: error: incompatible pointer to integer conversion assigning to 'dma_addr_t' (aka 'unsigned long long') from 'dma_addr_t *' (aka 'unsigned long long *'); dereference with * [-Wint-conversion] 586 | addr = gi2c_gpi_xfer->dma_addr[gi2c_gpi_xfer->buf_idx]; | ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | * 3 errors generated. Kconfig warnings: (for reference only) WARNING: unmet direct dependencies detected for OMAP2PLUS_MBOX Depends on [n]: MAILBOX [=y] && (ARCH_OMAP2PLUS || ARCH_K3) Selected by [y]: - TI_K3_M4_REMOTEPROC [=y] && REMOTEPROC [=y] && (ARCH_K3 || COMPILE_TEST [=y]) vim +562 drivers/i2c/busses/i2c-qcom-geni.c 532 533 /** 534 * gpi_i2c_multi_desc_unmap() - unmaps the buffers post multi message TX transfers 535 * @dev: pointer to the corresponding dev node 536 * @gi2c: i2c dev handle 537 * @msgs: i2c messages array 538 * @peripheral: pointer to the gpi_i2c_config 539 */ 540 static void gpi_i2c_multi_desc_unmap(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], 541 struct gpi_i2c_config *peripheral) 542 { 543 u32 msg_xfer_cnt, wr_idx = 0; 544 struct gpi_multi_xfer *tx_multi_xfer = &peripheral->multi_xfer; 545 546 /* 547 * In error case, need to unmap all messages based on the msg_idx_cnt. 548 * Non-error case unmap all the processed messages. 549 */ 550 if (gi2c->err) 551 msg_xfer_cnt = tx_multi_xfer->msg_idx_cnt; 552 else 553 msg_xfer_cnt = tx_multi_xfer->irq_cnt * NUM_MSGS_PER_IRQ; 554 555 /* Unmap the processed DMA buffers based on the received interrupt count */ 556 for (; tx_multi_xfer->unmap_msg_cnt < msg_xfer_cnt; tx_multi_xfer->unmap_msg_cnt++) { 557 if (tx_multi_xfer->unmap_msg_cnt == gi2c->num_msgs) 558 break; 559 wr_idx = tx_multi_xfer->unmap_msg_cnt % QCOM_GPI_MAX_NUM_MSGS; 560 geni_i2c_gpi_unmap(gi2c, &msgs[tx_multi_xfer->unmap_msg_cnt], 561 tx_multi_xfer->dma_buf[wr_idx], > 562 tx_multi_xfer->dma_addr[wr_idx], NULL, NULL); 563 tx_multi_xfer->freed_msg_cnt++; 564 } 565 } 566 567 static int geni_i2c_gpi(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], int cur_msg_idx, 568 struct dma_slave_config *config, dma_addr_t *dma_addr_p, 569 void **buf, unsigned int op, struct dma_chan *dma_chan) 570 { 571 struct gpi_i2c_config *peripheral; 572 unsigned int flags; 573 void *dma_buf; 574 dma_addr_t addr; 575 enum dma_data_direction map_dirn; 576 enum dma_transfer_direction dma_dirn; 577 struct dma_async_tx_descriptor *desc; 578 int ret; 579 struct gpi_multi_xfer *gi2c_gpi_xfer; 580 dma_cookie_t cookie; 581 582 peripheral = config->peripheral_config; 583 gi2c_gpi_xfer = &peripheral->multi_xfer; 584 gi2c_gpi_xfer->msg_idx_cnt = cur_msg_idx; 585 dma_buf = gi2c_gpi_xfer->dma_buf[gi2c_gpi_xfer->buf_idx]; > 586 addr = gi2c_gpi_xfer->dma_addr[gi2c_gpi_xfer->buf_idx]; 587 588 dma_buf = i2c_get_dma_safe_msg_buf(&msgs[gi2c_gpi_xfer->msg_idx_cnt], 1); 589 if (!dma_buf) { 590 gi2c->err = -ENOMEM; 591 return -ENOMEM; 592 } 593 594 if (op == I2C_WRITE) 595 map_dirn = DMA_TO_DEVICE; 596 else 597 map_dirn = DMA_FROM_DEVICE; 598 599 addr = dma_map_single(gi2c->se.dev->parent, 600 dma_buf, msgs[gi2c_gpi_xfer->msg_idx_cnt].len, 601 map_dirn); 602 if (dma_mapping_error(gi2c->se.dev->parent, addr)) { 603 i2c_put_dma_safe_msg_buf(dma_buf, &msgs[gi2c_gpi_xfer->msg_idx_cnt], 604 false); 605 gi2c->err = -ENOMEM; 606 return -ENOMEM; 607 } 608 609 if (gi2c->is_tx_multi_xfer) { 610 if (((gi2c_gpi_xfer->msg_idx_cnt + 1) % NUM_MSGS_PER_IRQ)) 611 peripheral->flags |= QCOM_GPI_BLOCK_EVENT_IRQ; 612 else 613 peripheral->flags &= ~QCOM_GPI_BLOCK_EVENT_IRQ; 614 615 /* BEI bit to be cleared for last TRE */ 616 if (gi2c_gpi_xfer->msg_idx_cnt == gi2c->num_msgs - 1) 617 peripheral->flags &= ~QCOM_GPI_BLOCK_EVENT_IRQ; 618 } 619 620 /* set the length as message for rx txn */ 621 peripheral->rx_len = msgs[gi2c_gpi_xfer->msg_idx_cnt].len; 622 peripheral->op = op; 623 624 ret = dmaengine_slave_config(dma_chan, config); 625 if (ret) { 626 dev_err(gi2c->se.dev, "dma config error: %d for op:%d\n", ret, op); 627 goto err_config; 628 } 629 630 peripheral->set_config = 0; 631 peripheral->multi_msg = true; 632 flags = DMA_PREP_INTERRUPT | DMA_CTRL_ACK; 633 634 if (op == I2C_WRITE) 635 dma_dirn = DMA_MEM_TO_DEV; 636 else 637 dma_dirn = DMA_DEV_TO_MEM; 638 639 desc = dmaengine_prep_slave_single(dma_chan, addr, 640 msgs[gi2c_gpi_xfer->msg_idx_cnt].len, 641 dma_dirn, flags); 642 if (!desc) { 643 dev_err(gi2c->se.dev, "prep_slave_sg failed\n"); 644 gi2c->err = -EIO; 645 goto err_config; 646 } 647 648 desc->callback_result = i2c_gpi_cb_result; 649 desc->callback_param = gi2c; 650 651 if (!((msgs[cur_msg_idx].flags & I2C_M_RD) && op == I2C_WRITE)) { 652 gi2c_gpi_xfer->msg_idx_cnt++; 653 gi2c_gpi_xfer->buf_idx = (cur_msg_idx + 1) % QCOM_GPI_MAX_NUM_MSGS; 654 } 655 cookie = dmaengine_submit(desc); 656 if (dma_submit_error(cookie)) { 657 dev_err(gi2c->se.dev, 658 "%s: dmaengine_submit failed (%d)\n", __func__, cookie); 659 return -EINVAL; 660 } 661 662 if (gi2c->is_tx_multi_xfer) { 663 dma_async_issue_pending(gi2c->tx_c); 664 if ((cur_msg_idx == (gi2c->num_msgs - 1)) || 665 (gi2c_gpi_xfer->msg_idx_cnt >= 666 QCOM_GPI_MAX_NUM_MSGS + gi2c_gpi_xfer->freed_msg_cnt)) { 667 ret = gpi_multi_desc_process(gi2c->se.dev, gi2c_gpi_xfer, 668 gi2c->num_msgs, XFER_TIMEOUT, 669 &gi2c->done); 670 if (ret) { 671 dev_dbg(gi2c->se.dev, 672 "I2C multi write msg transfer timeout: %d\n", 673 ret); 674 gi2c->err = -ETIMEDOUT; 675 goto err_config; 676 } 677 } 678 } else { 679 /* Non multi descriptor message transfer */ 680 *buf = dma_buf; 681 *dma_addr_p = addr; 682 } 683 return 0; 684 685 err_config: 686 dma_unmap_single(gi2c->se.dev->parent, addr, 687 msgs[cur_msg_idx].len, map_dirn); 688 i2c_put_dma_safe_msg_buf(dma_buf, &msgs[cur_msg_idx], false); 689 return ret; 690 } 691
Hi Jyothi,
kernel test robot noticed the following build errors:
[auto build test ERROR on 55bcd2e0d04c1171d382badef1def1fd04ef66c5]
url: https://github.com/intel-lab-lkp/linux/commits/Jyothi-Kumar-Seerapu/dt-bindings-dmaengine-qcom-gpi-Add-additional-arg-to-dma-cell-property/20241015-202637
base: 55bcd2e0d04c1171d382badef1def1fd04ef66c5
patch link: https://lore.kernel.org/r/20241015120750.21217-6-quic_jseerapu%40quicinc.com
patch subject: [PATCH v1 5/5] i2c: i2c-qcom-geni: Add Block event interrupt support
config: i386-allmodconfig (https://download.01.org/0day-ci/archive/20241019/202410191055.bi1pWTAY-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241019/202410191055.bi1pWTAY-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410191055.bi1pWTAY-lkp@intel.com/
All errors (new ones prefixed by >>, old ones prefixed by <<):
>> ERROR: modpost: "gpi_multi_desc_process" [drivers/i2c/busses/i2c-qcom-geni.ko] undefined!
Kconfig warnings: (for reference only)
WARNING: unmet direct dependencies detected for GET_FREE_REGION
Depends on [n]: SPARSEMEM [=n]
Selected by [m]:
- RESOURCE_KUNIT_TEST [=m] && RUNTIME_TESTING_MENU [=y] && KUNIT [=m]
WARNING: unmet direct dependencies detected for OMAP2PLUS_MBOX
Depends on [n]: MAILBOX [=y] && (ARCH_OMAP2PLUS || ARCH_K3)
Selected by [m]:
- TI_K3_M4_REMOTEPROC [=m] && REMOTEPROC [=y] && (ARCH_K3 || COMPILE_TEST [=y])
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? In this case, ret is-ETIMEDOUT, so updated in V2 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, memsettx_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
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
On 10/19/2024 8:42 AM, kernel test robot wrote: > Hi Jyothi, > > kernel test robot noticed the following build errors: > > [auto build test ERROR on 55bcd2e0d04c1171d382badef1def1fd04ef66c5] > > url: https://github.com/intel-lab-lkp/linux/commits/Jyothi-Kumar-Seerapu/dt-bindings-dmaengine-qcom-gpi-Add-additional-arg-to-dma-cell-property/20241015-202637 > base: 55bcd2e0d04c1171d382badef1def1fd04ef66c5 > patch link: https://lore.kernel.org/r/20241015120750.21217-6-quic_jseerapu%40quicinc.com > patch subject: [PATCH v1 5/5] i2c: i2c-qcom-geni: Add Block event interrupt support > config: i386-allmodconfig (https://download.01.org/0day-ci/archive/20241019/202410191055.bi1pWTAY-lkp@intel.com/config) > compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241019/202410191055.bi1pWTAY-lkp@intel.com/reproduce) > > If you fix the issue in a separate patch/commit (i.e. not just a new version of > the same patch/commit), kindly add following tags > | Reported-by: kernel test robot <lkp@intel.com> > | Closes: https://lore.kernel.org/oe-kbuild-all/202410191055.bi1pWTAY-lkp@intel.com/ > > All errors (new ones prefixed by >>, old ones prefixed by <<): > >>> ERROR: modpost: "gpi_multi_desc_process" [drivers/i2c/busses/i2c-qcom-geni.ko] undefined! > > Kconfig warnings: (for reference only) > WARNING: unmet direct dependencies detected for GET_FREE_REGION > Depends on [n]: SPARSEMEM [=n] > Selected by [m]: > - RESOURCE_KUNIT_TEST [=m] && RUNTIME_TESTING_MENU [=y] && KUNIT [=m] > WARNING: unmet direct dependencies detected for OMAP2PLUS_MBOX > Depends on [n]: MAILBOX [=y] && (ARCH_OMAP2PLUS || ARCH_K3) > Selected by [m]: > - TI_K3_M4_REMOTEPROC [=m] && REMOTEPROC [=y] && (ARCH_K3 || COMPILE_TEST [=y]) > Fixed the reported issue in V2 patch. Regards, JyothiKumar
On 10/19/2024 3:41 AM, kernel test robot wrote: > Hi Jyothi, > > kernel test robot noticed the following build errors: > > [auto build test ERROR on 55bcd2e0d04c1171d382badef1def1fd04ef66c5] > > url: https://github.com/intel-lab-lkp/linux/commits/Jyothi-Kumar-Seerapu/dt-bindings-dmaengine-qcom-gpi-Add-additional-arg-to-dma-cell-property/20241015-202637 > base: 55bcd2e0d04c1171d382badef1def1fd04ef66c5 > patch link: https://lore.kernel.org/r/20241015120750.21217-6-quic_jseerapu%40quicinc.com > patch subject: [PATCH v1 5/5] i2c: i2c-qcom-geni: Add Block event interrupt support > config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20241019/202410190549.hGAfByqg-lkp@intel.com/config) > compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff) > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241019/202410190549.hGAfByqg-lkp@intel.com/reproduce) > > If you fix the issue in a separate patch/commit (i.e. not just a new version of > the same patch/commit), kindly add following tags > | Reported-by: kernel test robot <lkp@intel.com> > | Closes: https://lore.kernel.org/oe-kbuild-all/202410190549.hGAfByqg-lkp@intel.com/ > > All errors (new ones prefixed by >>): > >>> drivers/i2c/busses/i2c-qcom-geni.c:562:8: error: incompatible pointer to integer conversion passing 'dma_addr_t *' (aka 'unsigned long long *') to parameter of type 'dma_addr_t' (aka 'unsigned long long'); dereference with * [-Wint-conversion] > 562 | tx_multi_xfer->dma_addr[wr_idx], NULL, NULL); > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > | * > drivers/i2c/busses/i2c-qcom-geni.c:519:36: note: passing argument to parameter 'tx_addr' here > 519 | void *tx_buf, dma_addr_t tx_addr, > | ^ >>> drivers/i2c/busses/i2c-qcom-geni.c:562:47: error: incompatible pointer to integer conversion passing 'void *' to parameter of type 'dma_addr_t' (aka 'unsigned long long') [-Wint-conversion] > 562 | tx_multi_xfer->dma_addr[wr_idx], NULL, NULL); > | ^~~~ > include/linux/stddef.h:8:14: note: expanded from macro 'NULL' > 8 | #define NULL ((void *)0) > | ^~~~~~~~~~~ > drivers/i2c/busses/i2c-qcom-geni.c:520:36: note: passing argument to parameter 'rx_addr' here > 520 | void *rx_buf, dma_addr_t rx_addr) > | ^ >>> drivers/i2c/busses/i2c-qcom-geni.c:586:7: error: incompatible pointer to integer conversion assigning to 'dma_addr_t' (aka 'unsigned long long') from 'dma_addr_t *' (aka 'unsigned long long *'); dereference with * [-Wint-conversion] > 586 | addr = gi2c_gpi_xfer->dma_addr[gi2c_gpi_xfer->buf_idx]; > | ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > | * > 3 errors generated. Fixed the reported issues which are comiltation warnings in V2 patch. > > Kconfig warnings: (for reference only) > WARNING: unmet direct dependencies detected for OMAP2PLUS_MBOX > Depends on [n]: MAILBOX [=y] && (ARCH_OMAP2PLUS || ARCH_K3) > Selected by [y]: > - TI_K3_M4_REMOTEPROC [=y] && REMOTEPROC [=y] && (ARCH_K3 || COMPILE_TEST [=y]) > > > vim +562 drivers/i2c/busses/i2c-qcom-geni.c > > 532 > 533 /** > 534 * gpi_i2c_multi_desc_unmap() - unmaps the buffers post multi message TX transfers > 535 * @dev: pointer to the corresponding dev node > 536 * @gi2c: i2c dev handle > 537 * @msgs: i2c messages array > 538 * @peripheral: pointer to the gpi_i2c_config > 539 */ > 540 static void gpi_i2c_multi_desc_unmap(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], > 541 struct gpi_i2c_config *peripheral) > 542 { > 543 u32 msg_xfer_cnt, wr_idx = 0; > 544 struct gpi_multi_xfer *tx_multi_xfer = &peripheral->multi_xfer; > 545 > 546 /* > 547 * In error case, need to unmap all messages based on the msg_idx_cnt. > 548 * Non-error case unmap all the processed messages. > 549 */ > 550 if (gi2c->err) > 551 msg_xfer_cnt = tx_multi_xfer->msg_idx_cnt; > 552 else > 553 msg_xfer_cnt = tx_multi_xfer->irq_cnt * NUM_MSGS_PER_IRQ; > 554 > 555 /* Unmap the processed DMA buffers based on the received interrupt count */ > 556 for (; tx_multi_xfer->unmap_msg_cnt < msg_xfer_cnt; tx_multi_xfer->unmap_msg_cnt++) { > 557 if (tx_multi_xfer->unmap_msg_cnt == gi2c->num_msgs) > 558 break; > 559 wr_idx = tx_multi_xfer->unmap_msg_cnt % QCOM_GPI_MAX_NUM_MSGS; > 560 geni_i2c_gpi_unmap(gi2c, &msgs[tx_multi_xfer->unmap_msg_cnt], > 561 tx_multi_xfer->dma_buf[wr_idx], > > 562 tx_multi_xfer->dma_addr[wr_idx], NULL, NULL); > 563 tx_multi_xfer->freed_msg_cnt++; > 564 } > 565 } > 566 > 567 static int geni_i2c_gpi(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], int cur_msg_idx, > 568 struct dma_slave_config *config, dma_addr_t *dma_addr_p, > 569 void **buf, unsigned int op, struct dma_chan *dma_chan) > 570 { > 571 struct gpi_i2c_config *peripheral; > 572 unsigned int flags; > 573 void *dma_buf; > 574 dma_addr_t addr; > 575 enum dma_data_direction map_dirn; > 576 enum dma_transfer_direction dma_dirn; > 577 struct dma_async_tx_descriptor *desc; > 578 int ret; > 579 struct gpi_multi_xfer *gi2c_gpi_xfer; > 580 dma_cookie_t cookie; > 581 > 582 peripheral = config->peripheral_config; > 583 gi2c_gpi_xfer = &peripheral->multi_xfer; > 584 gi2c_gpi_xfer->msg_idx_cnt = cur_msg_idx; > 585 dma_buf = gi2c_gpi_xfer->dma_buf[gi2c_gpi_xfer->buf_idx]; > > 586 addr = gi2c_gpi_xfer->dma_addr[gi2c_gpi_xfer->buf_idx]; > 587 > 588 dma_buf = i2c_get_dma_safe_msg_buf(&msgs[gi2c_gpi_xfer->msg_idx_cnt], 1); > 589 if (!dma_buf) { > 590 gi2c->err = -ENOMEM; > 591 return -ENOMEM; > 592 } > 593 > 594 if (op == I2C_WRITE) > 595 map_dirn = DMA_TO_DEVICE; > 596 else > 597 map_dirn = DMA_FROM_DEVICE; > 598 > 599 addr = dma_map_single(gi2c->se.dev->parent, > 600 dma_buf, msgs[gi2c_gpi_xfer->msg_idx_cnt].len, > 601 map_dirn); > 602 if (dma_mapping_error(gi2c->se.dev->parent, addr)) { > 603 i2c_put_dma_safe_msg_buf(dma_buf, &msgs[gi2c_gpi_xfer->msg_idx_cnt], > 604 false); > 605 gi2c->err = -ENOMEM; > 606 return -ENOMEM; > 607 } > 608 > 609 if (gi2c->is_tx_multi_xfer) { > 610 if (((gi2c_gpi_xfer->msg_idx_cnt + 1) % NUM_MSGS_PER_IRQ)) > 611 peripheral->flags |= QCOM_GPI_BLOCK_EVENT_IRQ; > 612 else > 613 peripheral->flags &= ~QCOM_GPI_BLOCK_EVENT_IRQ; > 614 > 615 /* BEI bit to be cleared for last TRE */ > 616 if (gi2c_gpi_xfer->msg_idx_cnt == gi2c->num_msgs - 1) > 617 peripheral->flags &= ~QCOM_GPI_BLOCK_EVENT_IRQ; > 618 } > 619 > 620 /* set the length as message for rx txn */ > 621 peripheral->rx_len = msgs[gi2c_gpi_xfer->msg_idx_cnt].len; > 622 peripheral->op = op; > 623 > 624 ret = dmaengine_slave_config(dma_chan, config); > 625 if (ret) { > 626 dev_err(gi2c->se.dev, "dma config error: %d for op:%d\n", ret, op); > 627 goto err_config; > 628 } > 629 > 630 peripheral->set_config = 0; > 631 peripheral->multi_msg = true; > 632 flags = DMA_PREP_INTERRUPT | DMA_CTRL_ACK; > 633 > 634 if (op == I2C_WRITE) > 635 dma_dirn = DMA_MEM_TO_DEV; > 636 else > 637 dma_dirn = DMA_DEV_TO_MEM; > 638 > 639 desc = dmaengine_prep_slave_single(dma_chan, addr, > 640 msgs[gi2c_gpi_xfer->msg_idx_cnt].len, > 641 dma_dirn, flags); > 642 if (!desc) { > 643 dev_err(gi2c->se.dev, "prep_slave_sg failed\n"); > 644 gi2c->err = -EIO; > 645 goto err_config; > 646 } > 647 > 648 desc->callback_result = i2c_gpi_cb_result; > 649 desc->callback_param = gi2c; > 650 > 651 if (!((msgs[cur_msg_idx].flags & I2C_M_RD) && op == I2C_WRITE)) { > 652 gi2c_gpi_xfer->msg_idx_cnt++; > 653 gi2c_gpi_xfer->buf_idx = (cur_msg_idx + 1) % QCOM_GPI_MAX_NUM_MSGS; > 654 } > 655 cookie = dmaengine_submit(desc); > 656 if (dma_submit_error(cookie)) { > 657 dev_err(gi2c->se.dev, > 658 "%s: dmaengine_submit failed (%d)\n", __func__, cookie); > 659 return -EINVAL; > 660 } > 661 > 662 if (gi2c->is_tx_multi_xfer) { > 663 dma_async_issue_pending(gi2c->tx_c); > 664 if ((cur_msg_idx == (gi2c->num_msgs - 1)) || > 665 (gi2c_gpi_xfer->msg_idx_cnt >= > 666 QCOM_GPI_MAX_NUM_MSGS + gi2c_gpi_xfer->freed_msg_cnt)) { > 667 ret = gpi_multi_desc_process(gi2c->se.dev, gi2c_gpi_xfer, > 668 gi2c->num_msgs, XFER_TIMEOUT, > 669 &gi2c->done); > 670 if (ret) { > 671 dev_dbg(gi2c->se.dev, > 672 "I2C multi write msg transfer timeout: %d\n", > 673 ret); > 674 gi2c->err = -ETIMEDOUT; > 675 goto err_config; > 676 } > 677 } > 678 } else { > 679 /* Non multi descriptor message transfer */ > 680 *buf = dma_buf; > 681 *dma_addr_p = addr; > 682 } > 683 return 0; > 684 > 685 err_config: > 686 dma_unmap_single(gi2c->se.dev->parent, addr, > 687 msgs[cur_msg_idx].len, map_dirn); > 688 i2c_put_dma_safe_msg_buf(dma_buf, &msgs[cur_msg_idx], false); > 689 return ret; > 690 } > 691 >
diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c index 212336f724a6..a73dc1738a62 100644 --- a/drivers/i2c/busses/i2c-qcom-geni.c +++ b/drivers/i2c/busses/i2c-qcom-geni.c @@ -99,6 +99,10 @@ struct geni_i2c_dev { struct dma_chan *rx_c; bool gpi_mode; bool abort_done; + bool is_tx_multi_xfer; + u32 num_msgs; + u32 tx_irq_cnt; + struct gpi_i2c_config *gpi_config; }; struct geni_i2c_desc { @@ -485,6 +489,7 @@ static int geni_i2c_tx_one_msg(struct geni_i2c_dev *gi2c, struct i2c_msg *msg, static void i2c_gpi_cb_result(void *cb, const struct dmaengine_result *result) { struct geni_i2c_dev *gi2c = cb; + struct gpi_multi_xfer *tx_multi_xfer; if (result->result != DMA_TRANS_NOERROR) { dev_err(gi2c->se.dev, "DMA txn failed:%d\n", result->result); @@ -493,7 +498,21 @@ static void i2c_gpi_cb_result(void *cb, const struct dmaengine_result *result) dev_dbg(gi2c->se.dev, "DMA xfer has pending: %d\n", result->residue); } - complete(&gi2c->done); + if (gi2c->is_tx_multi_xfer) { + tx_multi_xfer = &gi2c->gpi_config->multi_xfer; + + /* + * Send Completion for last message or multiple of NUM_MSGS_PER_IRQ. + */ + if ((tx_multi_xfer->irq_msg_cnt == gi2c->num_msgs - 1) || + (!((tx_multi_xfer->irq_msg_cnt + 1) % NUM_MSGS_PER_IRQ))) { + tx_multi_xfer->irq_cnt++; + complete(&gi2c->done); + } + tx_multi_xfer->irq_msg_cnt++; + } else { + complete(&gi2c->done); + } } static void geni_i2c_gpi_unmap(struct geni_i2c_dev *gi2c, struct i2c_msg *msg, @@ -511,7 +530,41 @@ static void geni_i2c_gpi_unmap(struct geni_i2c_dev *gi2c, struct i2c_msg *msg, } } -static int geni_i2c_gpi(struct geni_i2c_dev *gi2c, struct i2c_msg *msg, +/** + * gpi_i2c_multi_desc_unmap() - unmaps the buffers post multi message TX transfers + * @dev: pointer to the corresponding dev node + * @gi2c: i2c dev handle + * @msgs: i2c messages array + * @peripheral: pointer to the gpi_i2c_config + */ +static void gpi_i2c_multi_desc_unmap(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], + struct gpi_i2c_config *peripheral) +{ + u32 msg_xfer_cnt, wr_idx = 0; + struct gpi_multi_xfer *tx_multi_xfer = &peripheral->multi_xfer; + + /* + * In error case, need to unmap all messages based on the msg_idx_cnt. + * Non-error case unmap all the processed messages. + */ + if (gi2c->err) + msg_xfer_cnt = tx_multi_xfer->msg_idx_cnt; + else + msg_xfer_cnt = tx_multi_xfer->irq_cnt * NUM_MSGS_PER_IRQ; + + /* Unmap the processed DMA buffers based on the received interrupt count */ + for (; tx_multi_xfer->unmap_msg_cnt < msg_xfer_cnt; tx_multi_xfer->unmap_msg_cnt++) { + if (tx_multi_xfer->unmap_msg_cnt == gi2c->num_msgs) + break; + wr_idx = tx_multi_xfer->unmap_msg_cnt % QCOM_GPI_MAX_NUM_MSGS; + geni_i2c_gpi_unmap(gi2c, &msgs[tx_multi_xfer->unmap_msg_cnt], + tx_multi_xfer->dma_buf[wr_idx], + tx_multi_xfer->dma_addr[wr_idx], NULL, NULL); + tx_multi_xfer->freed_msg_cnt++; + } +} + +static int geni_i2c_gpi(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], int cur_msg_idx, struct dma_slave_config *config, dma_addr_t *dma_addr_p, void **buf, unsigned int op, struct dma_chan *dma_chan) { @@ -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, + 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; + } + 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); + gi2c->err = -ETIMEDOUT; + 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; } @@ -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; + + /* + * 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, &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) { + gi2c->tx_irq_cnt = tx_multi_xfer->irq_cnt; + gpi_i2c_multi_desc_unmap(gi2c, msgs, &peripheral); + } + } } return num; @@ -648,7 +801,11 @@ static int geni_i2c_gpi_xfer(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], i dev_err(gi2c->se.dev, "GPI transfer failed: %d\n", ret); dmaengine_terminate_sync(gi2c->rx_c); dmaengine_terminate_sync(gi2c->tx_c); - geni_i2c_gpi_unmap(gi2c, &msgs[i], tx_buf, tx_addr, rx_buf, rx_addr); + if (gi2c->is_tx_multi_xfer) + gpi_i2c_multi_desc_unmap(gi2c, msgs, &peripheral); + else + geni_i2c_gpi_unmap(gi2c, &msgs[i], tx_buf, tx_addr, rx_buf, rx_addr); + return ret; }
The I2C driver gets an interrupt upon transfer completion. For multiple messages in a single transfer, N interrupts will be received for N messages, leading to significant software interrupt latency. To mitigate this latency, utilize Block Event Interrupt (BEI) only when an interrupt is necessary. This means large transfers can be split into multiple chunks of 64 messages internally, without expecting interrupts for the first 63 transfers, only the last one will trigger an interrupt indicating 64 transfers completed. By implementing BEI, multi-message transfers can be divided into chunks of 64 messages, improving overall transfer time. This optimization reduces transfer time from 168 ms to 48 ms for a series of 200 I2C write messages in a single transfer, with a clock frequency support of 100 kHz. BEI optimizations are currently implemented for I2C write transfers only, as there is no use case for multiple I2C read messages in a single transfer at this time. Signed-off-by: Jyothi Kumar Seerapu <quic_jseerapu@quicinc.com> --- drivers/i2c/busses/i2c-qcom-geni.c | 205 +++++++++++++++++++++++++---- 1 file changed, 181 insertions(+), 24 deletions(-)