linux-kernel - Re: [PATCH V13 3/5] 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] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 6 Feb 2019 17:41:58 +0100
From:   Thierry Reding <thierry.reding@...il.com>
To:     Sowjanya Komatineni <skomatineni@...dia.com>
Cc:     Dmitry Osipenko <digetx@...il.com>,
        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 V13 3/5] i2c: tegra: Add DMA support

On Wed, Feb 06, 2019 at 04:38:55PM +0000, Sowjanya Komatineni wrote:
> > >> Looking into timestamps and transactions, DMA timeouts after start of DMA for I2C1 to touch during this transaction.
> > >> While it is waiting for I2C1 DMA transfer, lots of DVC transactions 
> > >> happened thru DMA which are successful
> > >>
> > >> What is the I2C1 speed?
> > > 
> > > 400KHz
> > > 
> > >> Also incase if device is running slow for some reason, probably timeout was not enough as this patch series changes timeout with base 100mS + msg transfer time based on transfer size.
> > >> Can you give quick try with increased timeout incase if device is running slow?
> > >>
> > > 
> > > Tried to increase the timeout to 1 second, doesn't help.
> > > 
> > > What helped again is the I2C HW resetting after each transfer. Likely that means that HW isn't programmed correctly, please carefully check every bit.
> > > 
> > > DMA-only + I2C HW reset: http://dpaste.com/26AQXFM.txt
> >
> > Seems I found what's the problem. Here is the fix, please include it to v14 if it is correct.
> >
> > diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c index a9391c3646b6..5ad54da70304 100644
> > --- a/drivers/i2c/busses/i2c-tegra.c
> > +++ b/drivers/i2c/busses/i2c-tegra.c
> > @@ -912,7 +912,7 @@ static irqreturn_t tegra_i2c_isr(int irq, void *dev_id)  static void tegra_i2c_config_fifo_trig(struct tegra_i2c_dev *i2c_dev,
> >                                        size_t len)  {
> > -       u32 val, reg;
> > +       u32 val = 0, reg;
> >         u8 dma_burst = 0;
> >         struct dma_slave_config slv_config = {0};
> >         struct dma_chan *chan;
> > @@ -922,7 +922,6 @@ static void tegra_i2c_config_fifo_trig(struct tegra_i2c_dev *i2c_dev,
> >                 reg = I2C_MST_FIFO_CONTROL;
> >         else
> >                 reg = I2C_FIFO_CONTROL;
> > -       val = i2c_readl(i2c_dev, reg);
>  >
>  >        if (i2c_dev->is_curr_dma_xfer) {
>  >                if (len & 0xF)
> 
> Thanks dmitry. Good catch. Didn't caught to my eyes. Yes FIFO_CONTROL is set based on read value without masking and oring causing back-back DMA transfers resulting in incorrect trig levels
> We can directly set trig levels.

It might be safer to explicitly clear the trigger levels before we
overwrite them. Though, admittedly, this register hasn't changed in
years, so it's unlikely we'll ever see it change in a way that might
result in breakage if we construct the register value from scratch.

Thierry

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)