linux-kernel - Re: [V3] i2c: i2c-qcom-geni: Correct I2C TRE sequence

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <uswznu3h53gcefpdc4vxozz32ecdcjvzmr7admwc4h54o27bfy@qqoevrl3dcyt>
Date: Thu, 8 Feb 2024 11:02:33 +0100
From: Andi Shyti <andi.shyti@...nel.org>
To: Viken Dadhaniya <quic_vdadhani@...cinc.com>
Cc: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>, andersson@...nel.org, 
	konrad.dybcio@...aro.org, linux-arm-msm@...r.kernel.org, linux-i2c@...r.kernel.org, 
	linux-kernel@...r.kernel.org, vkoul@...nel.org, quic_bjorande@...cinc.com, 
	manivannan.sadhasivam@...aro.org, bryan.odonoghue@...aro.org, quic_msavaliy@...cinc.com, 
	quic_vtanuku@...cinc.com
Subject: Re: [V3] i2c: i2c-qcom-geni: Correct I2C TRE sequence

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@...cinc.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.

Andi