| Message ID | 20240212125239.7764-1-quic_vdadhani@quicinc.com |
|---|---|
| State | Accepted |
| Delegated to: | Andi Shyti |
| Headers | show |
| Series | [v4] i2c: i2c-qcom-geni: Correct I2C TRE sequence | expand |
On Mon, 12 Feb 2024 at 14:52, Viken Dadhaniya <quic_vdadhani@quicinc.com> wrote: > > For i2c read operation in GSI mode, we are getting timeout > due to malformed TRE basically incorrect TRE sequence > in gpi(drivers/dma/qcom/gpi.c) driver. > > I2C driver has geni_i2c_gpi(I2C_WRITE) function which generates GO TRE and > geni_i2c_gpi(I2C_READ)generates DMA TRE. Hence to generate GO TRE before > DMA TRE, we should move geni_i2c_gpi(I2C_WRITE) before > geni_i2c_gpi(I2C_READ) inside the I2C GSI mode transfer function > i.e. geni_i2c_gpi_xfer(). > > TRE stands for Transfer Ring Element - which is basically an element with > size of 4 words. It contains all information like slave address, > clk divider, dma address value data size etc). > > Mainly we have 3 TREs(Config, GO and DMA tre). > - CONFIG TRE : consists of internal register configuration which is > required before start of the transfer. > - DMA TRE : contains DDR/Memory address, called as DMA descriptor. > - GO TRE : contains Transfer directions, slave ID, Delay flags, Length > of the transfer. > > I2c driver calls GPI driver API to config each TRE depending on the > protocol. > > For read operation tre sequence will be as below which is not aligned > to hardware programming guide. > > - CONFIG tre > - DMA tre > - GO tre > > As per Qualcomm's internal Hardware Programming Guide, we should configure > TREs in below sequence for any RX only transfer. > > - CONFIG tre > - GO tre > - DMA tre > > Fixes: d8703554f4de ("i2c: qcom-geni: Add support for GPI DMA") > Reviewed-by: Andi Shyti <andi.shyti@kernel.org> > Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # qrb5165-rb5 > Co-developed-by: Mukesh Kumar Savaliya <quic_msavaliy@quicinc.com> > Signed-off-by: Mukesh Kumar Savaliya <quic_msavaliy@quicinc.com> > Signed-off-by: Viken Dadhaniya <quic_vdadhani@quicinc.com> Thank you, Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Hi On Mon, 12 Feb 2024 18:22:39 +0530, Viken Dadhaniya wrote: > For i2c read operation in GSI mode, we are getting timeout > due to malformed TRE basically incorrect TRE sequence > in gpi(drivers/dma/qcom/gpi.c) driver. > > I2C driver has geni_i2c_gpi(I2C_WRITE) function which generates GO TRE and > geni_i2c_gpi(I2C_READ)generates DMA TRE. Hence to generate GO TRE before > DMA TRE, we should move geni_i2c_gpi(I2C_WRITE) before > geni_i2c_gpi(I2C_READ) inside the I2C GSI mode transfer function > i.e. geni_i2c_gpi_xfer(). > > [...] Applied to i2c/i2c-host-fixes on git://git.kernel.org/pub/scm/linux/kernel/git/andi.shyti/linux.git Thank you, Andi Patches applied =============== [1/1] i2c: i2c-qcom-geni: Correct I2C TRE sequence commit: 32e9b680de4b46cfe835cbc7ee3dc721f7cc65fb
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);
For i2c read operation in GSI mode, we are getting timeout due to malformed TRE basically incorrect TRE sequence in gpi(drivers/dma/qcom/gpi.c) driver. I2C driver has geni_i2c_gpi(I2C_WRITE) function which generates GO TRE and geni_i2c_gpi(I2C_READ)generates DMA TRE. Hence to generate GO TRE before DMA TRE, we should move geni_i2c_gpi(I2C_WRITE) before geni_i2c_gpi(I2C_READ) inside the I2C GSI mode transfer function i.e. geni_i2c_gpi_xfer(). TRE stands for Transfer Ring Element - which is basically an element with size of 4 words. It contains all information like slave address, clk divider, dma address value data size etc). Mainly we have 3 TREs(Config, GO and DMA tre). - CONFIG TRE : consists of internal register configuration which is required before start of the transfer. - DMA TRE : contains DDR/Memory address, called as DMA descriptor. - GO TRE : contains Transfer directions, slave ID, Delay flags, Length of the transfer. I2c driver calls GPI driver API to config each TRE depending on the protocol. For read operation tre sequence will be as below which is not aligned to hardware programming guide. - CONFIG tre - DMA tre - GO tre As per Qualcomm's internal Hardware Programming Guide, we should configure TREs in below sequence for any RX only transfer. - CONFIG tre - GO tre - DMA tre Fixes: d8703554f4de ("i2c: qcom-geni: Add support for GPI DMA") Reviewed-by: Andi Shyti <andi.shyti@kernel.org> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # qrb5165-rb5 Co-developed-by: Mukesh Kumar Savaliya <quic_msavaliy@quicinc.com> Signed-off-by: Mukesh Kumar Savaliya <quic_msavaliy@quicinc.com> Signed-off-by: Viken Dadhaniya <quic_vdadhani@quicinc.com> --- v3 -> v4: - Update commit log. - Add review/tested/co-develop tag. - Fix patch title by adding "PATCH" string. v2 -> v3: - Update commit log to explain change in simple way. - Correct fix tag format. 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(-)