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]

Date: Mon, 12 Feb 2024 18:26:23 +0530
From: Viken Dadhaniya <quic_vdadhani@...cinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        Andi Shyti
	<andi.shyti@...nel.org>
CC: <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



On 2/8/2024 4:34 PM, Dmitry Baryshkov wrote:
> On Thu, 8 Feb 2024 at 12:02, Andi Shyti <andi.shyti@...nel.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@...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.
> 
> 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