Messages in this thread | | | Date | Mon, 12 Feb 2024 18:26:23 +0530 | Subject | Re: [V3] i2c: i2c-qcom-geni: Correct I2C TRE sequence | From | Viken Dadhaniya <> |
| |
On 2/8/2024 4:34 PM, Dmitry Baryshkov wrote: > On Thu, 8 Feb 2024 at 12:02, Andi Shyti <andi.shyti@kernel.org> wrote: >> >> Hi Viken, Dmitry, >> >> On Fri, Feb 02, 2024 at 04:13:06PM +0530, Viken Dadhaniya wrote: >>> >>> On 2/1/2024 5:24 PM, Dmitry Baryshkov wrote: >>>> On Thu, 1 Feb 2024 at 12:13, 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. >>>>> >>>>> 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. >>>>> >>>>> Driver calls GPI driver API to config each TRE depending on the protocol. >>>>> If we see GPI driver, for RX operation we are configuring DMA tre and >>>>> for TX operation we are configuring GO tre. >>>>> >>>>> 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 >>>>> >>>>> In summary, for RX only transfers, we are reordering DMA and GO TREs. >>>>> Tested covering i2c read/write transfer on QCM6490 RB3 board. >>>> >>>> This hasn't improved. You must describe what is the connection between >>>> TRE types and the geni_i2c_gpi calls. >>>> It is not obvious until somebody looks into the GPI DMA driver. >>>> >>>> Another point, for some reason you are still using just the patch >>>> version in email subject. Please fix your setup so that the email >>>> subject also includes the `[PATCH` part in the subject, which is there >>>> by default. >>>> Hint: git format-patch -1 -v4 will do that for you without a need to >>>> correct anything afterwards. >>>> >>> >>> At high level, let me explain the I2C to GPI driver flow in general. >>> >>> I2C driver calls GPI driver exposed functions which will prepare all the >>> TREs as per programming guide and >>> queues to the GPI DMA engine for execution. Upon completion of the Transfer, >>> GPI DMA engine will generate an >>> interrupt which will be handled inside the GPIO driver. Then GPI driver will >>> call DMA framework registered callback by i2c. >>> Upon receiving this callback, i2c driver marks the transfer completion. >> >> Any news about this? Dmitry do you still have concerns? We can >> add this last description in the commit log, as well, if needed. > > I was looking for pretty simple addition to the commit message, that > links existing commit message to the actual source code change: that > geni_i2c_gpi(I2C_WRITE) results in the GO TRE and > geni_i2c_gpi(I2C_READ) generates DMA TRE. But I haven't seen anything > sensible up to now. So far we have a nice description of required > programming sequence in terms of CONFIG, GO, DMA TREs and then source > code change that seems completely unrelated to the commit message, > unless one actually goes deep into the corresponding GPI DMA driver. >
Updated commit log in V4
| |