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: Fri, 2 Feb 2024 16:14:34 +0530
From: Viken Dadhaniya <quic_vdadhani@...cinc.com>
To: 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>,
        Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Subject: Re: [V3] i2c: i2c-qcom-geni: Correct I2C TRE sequence

Hi Andi,

Thanks for review and taking care of patch.
We have responded to dmitry to describe more about GPI operation and 
tried to explain flow in general.
We shall keep updating over email for commit log.

Please help amend below tags in commit log since we are taking it over 
email:

Fixes: d8703554f4de ("i2c: qcom-geni: Add support for GPI DMA")
Reviewed-by: Andi Shyti <andi.shyti@...nel.org>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
Tested-by: Bryan O'Donoghue <bryan.odonoghue@...aro.org> # qrb5165-rb5
Co-developed-by: Mukesh Kumar Savaliya <quic_msavaliy@...cinc.com>
Signed-off-by: Viken Dadhaniya <quic_vdadhani@...cinc.com>

On 2/1/2024 7:52 PM, Andi Shyti wrote:
> Hi Viken,
> 
> On Thu, Feb 01, 2024 at 03:43:23PM +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.
>>
>> 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.
>>
>> Fixes: d8703554f4de ("i2c: qcom-geni: Add support for GPI DMA")
>> Signed-off-by: Viken Dadhaniya <quic_vdadhani@...cinc.com>
> 
> as Dmitry has written, please, next time don't forget the tags:
> 
> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
> Tested-by: Bryan O'Donoghue <bryan.odonoghue@...aro.org> # qrb5165-rb5
> 
> You can also add mine:
> 
> Reviewed-by: Andi Shyti <andi.shyti@...nel.org>
> 
> Please make sure to Cc Dmitry who is raising his concerns and
> check on his comments.
> 
> Andi
> 
> PS just as a reminder, if Dmitry's concerns remain related only
> to the commit log, I gave you the option to agree with him in the
> e-mail thread without necessarily sending a v4. I can then update
> the commit log before pushing.