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 3:12 PM, Andi Shyti wrote:
Hi Dmitry,

thanks a lot for your review!

On Tue, Jan 30, 2024 at 01:49:57AM +0200, 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.


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.

If the issue is only in the commit message Viken can propose a
proper commit message as reply to this e-mail and I can fix it
before merging the change.

Important is that no issue is seen in the code.

Please, Viken, can you either send a v3 with a proper commit
message or write it in the reply to this e-mail with the changes
that Dmitry suggested.


Submitted V3 with proper commit log

---
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

If you are going to submit again, please make also sure that the
e-mail is formatted properly.

I'm not sure that this footer will be accepted by git.

Thanks,
Andi


Submitted V3 with proper email format.





--
With best wishes
Dmitry




[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