Re: [V2] i2c: i2c-qcom-geni: Correct I2C TRE sequence

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 1/30/2024 5:19 AM, Dmitry Baryshkov wrote:
On Mon, 29 Jan 2024 at 08:10, Viken Dadhaniya <quic_vdadhani@xxxxxxxxxxx> wrote:

For i2c read operation, we are getting gsi mode timeout due
to malformed TRE(Transfer Ring Element). Currently we are
configuring incorrect TRE sequence in gpi driver
(drivers/dma/qcom/gpi.c) as below

- Sets up CONFIG
- Sets up DMA tre
- Sets up GO tre

As per HPG(Hardware programming guide), We should configure TREs in below
sequence for any i2c transfer

- Sets up CONFIG tre
- Sets up GO tre
- Sets up DMA tre

It is not clear how this is relevant and/or affected by swapping
I2C_WRITE and I2C_READ gpi calls.


Submitted V3 with proper commit log.


For only write operation or write followed by read operation,
existing software sequence is correct.

for only read operation, TRE sequence need to be corrected.
Hence, we have changed the sequence to submit GO tre before DMA tre.

Tested covering i2c read/write transfer on QCM6490 RB3 board.

Please read Documentation/process/submitting-patches.rst, understand
it and write a proper commit message.


Signed-off-by: Viken Dadhaniya <quic_vdadhani@xxxxxxxxxxx>
Fixes: commit d8703554f4de ("i2c: qcom-geni: Add support for GPI DMA")

As it was pointed out, this line shows ignorance of the mentioned file
and of the existing community practices.


Updated fixes tag in V3.

---
v1 -> v2:
- Remove redundant check.
- update commit log.
- add fix tag.
---
---
  drivers/i2c/busses/i2c-qcom-geni.c | 14 +++++++-------
  1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
index 0d2e7171e3a6..da94df466e83 100644
--- a/drivers/i2c/busses/i2c-qcom-geni.c
+++ b/drivers/i2c/busses/i2c-qcom-geni.c
@@ -613,20 +613,20 @@ static int geni_i2c_gpi_xfer(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], i

                 peripheral.addr = msgs[i].addr;

+               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,
                                             &rx_addr, &rx_buf, I2C_READ, gi2c->rx_c);
                         if (ret)
                                 goto err;
-               }
-
-               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)
                         dma_async_issue_pending(gi2c->rx_c);
+               }
+
                 dma_async_issue_pending(gi2c->tx_c);

                 timeout = wait_for_completion_timeout(&gi2c->done, XFER_TIMEOUT);
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation








[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux