linux-kernel - RE: [PATCH V1] i2c: tegra: Add DMA Support

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] [day] [month] [year] [list]

Message-ID: <BYAPR12MB3398E0C5EE66FDE8F0813847C29A0@BYAPR12MB3398.namprd12.prod.outlook.com>
Date:   Thu, 24 Jan 2019 02:39:54 +0000
From:   Sowjanya Komatineni <skomatineni@...dia.com>
To:     Thierry Reding <thierry.reding@...il.com>
CC:     Jonathan Hunter <jonathanh@...dia.com>,
        Mantravadi Karthik <mkarthik@...dia.com>,
        Shardar Mohammed <smohammed@...dia.com>,
        Timo Alho <talho@...dia.com>,
        "linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-i2c@...r.kernel.org" <linux-i2c@...r.kernel.org>
Subject: RE: [PATCH V1] i2c: tegra: Add DMA Support



> > This patch adds DMA support for Tegra I2C.
>
> In the subject: "Support" -> "support". And in the commit description perhaps add some more details about why this is useful, maybe accompany with some performance numbers (if > you can come up with any) or describe why there's a threshold for PIO vs. DMA transfers.
>
> As it is, it's not immediately clear why we need or want these 400+ extra lines of code.
>
> > Signed-off-by: Sowjanya Komatineni <skomatineni@...dia.com>
> > ---
> >  drivers/i2c/busses/i2c-tegra.c | 494 
> > +++++++++++++++++++++++++++++++++++++----
> >  1 file changed, 456 insertions(+), 38 deletions(-)
> > 
>
>
> > @@ -658,7 +809,16 @@ static irqreturn_t tegra_i2c_isr(int irq, void *dev_id)
> >  	if (i2c_dev->is_dvc)
> >  		dvc_writel(i2c_dev, DVC_STATUS_I2C_DONE_INTR, DVC_STATUS);
> >  
> > -	if (status & I2C_INT_PACKET_XFER_COMPLETE) {
> > +	if (status & I2C_INT_ALL_PACKETS_XFER_COMPLETE) {
> > +		if (i2c_dev->is_curr_dma_xfer)
> > +			i2c_dev->msg_buf_remaining = 0;
>
> Why are we forcing msg_buf_remaining to 0 here and below? Doesn't the rest of the code take care of that already? Is it always guaranteed that all bytes will have been transferred in > DMA mode?

During the write (Memory to TX FIFO thru DMA), DMA completion happens prior to XFER_PACKET_COMPLETE INT from I2C
During the read (RX_FIFO to Memory thru DMA), XFER_PACKET_COMPLETE I2C INT gets generated prior to DMA Completion
Receiving XFER_PACKET_COMPLETE interrupts from I2C confirms successful I2C transfer, so In DMA Mode, msg_buf_remaining is forced to 0

DMA callback notification happens only on successful transfer of all bytes.
In case of incomplete transfer by DMA, call back notification will not be generated and DMA timeouts.
Also, DMA timeout is verified prior to msg_complete timeout.
 
Thanks
Sowjanya