From: Thierry Reding <thierry.reding@gmail.com>
To: Sowjanya Komatineni <skomatineni@nvidia.com>
Cc: Dmitry Osipenko <digetx@gmail.com>,
Jonathan Hunter <jonathanh@nvidia.com>,
Mantravadi Karthik <mkarthik@nvidia.com>,
Shardar Mohammed <smohammed@nvidia.com>,
Timo Alho <talho@nvidia.com>,
"linux-tegra@vger.kernel.org" <linux-tegra@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-i2c@vger.kernel.org" <linux-i2c@vger.kernel.org>
Subject: Re: [PATCH V12 3/5] i2c: tegra: Add DMA support
Date: Wed, 6 Feb 2019 15:01:41 +0100 [thread overview]
Message-ID: <20190206140141.GO21676@ulmo> (raw)
In-Reply-To: <BYAPR12MB33989329D0E0C0049F1DEA6EC26F0@BYAPR12MB3398.namprd12.prod.outlook.com>
[-- Attachment #1: Type: text/plain, Size: 6932 bytes --]
On Wed, Feb 06, 2019 at 01:51:12PM +0000, Sowjanya Komatineni wrote:
>
> > >>>> That's odd because it suggests that DMA actually completed, but
> > >>>> the message didn't.
> > >>>>
> > >>>> I'm not sure I understand how that could happen.
> > >>>>
> > >>>> What's also weird above is that there doesn't seem to be a DMA
> > >>>> that is started for that particular message. Or is the timeout
> > >>>> message a response to the prior transfer (length 10)? Seems like
> > >>>> that should not be possible because we get the "transfer complete" message.
> > >>>
> > >>> Wait, those are actually different instances of the I2C
> > >>> controller, so the relevant log entries are these:
> > >>>
> > >>> [ 0.945445] tegra-i2c 7000d000.i2c: starting DMA for length: 16
> > >>> [ 0.945456] tegra-i2c 7000d000.i2c: unmasked irq: 0c
> > >>> ...
> > >>> [ 1.049224] tegra-i2c 7000d000.i2c: i2c transfer timed out
> > >>>
> > >>> And these don't happen if you use higher burst sizes or before the
> > >>> DMA series?
> > >>
> > >> I tried to enforce DMA without changing bursts:
> > >>
> > >> ----------
> > >> diff --git a/drivers/i2c/busses/i2c-tegra.c
> > >> b/drivers/i2c/busses/i2c-tegra.c index c538ed5f8e2c..5d1c54ce7800
> > >> 100644
> > >> --- a/drivers/i2c/busses/i2c-tegra.c
> > >> +++ b/drivers/i2c/busses/i2c-tegra.c
> > >> @@ -6,6 +6,8 @@
> > >> * Author: Colin Cross <ccross@android.com>
> > >> */
> > >>
> > >> +#define DEBUG
> > >> +
> > >> #include <linux/clk.h>
> > >> #include <linux/delay.h>
> > >> #include <linux/dmaengine.h>
> > >> @@ -1046,8 +1048,7 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev,
> > >> xfer_size = msg->len + I2C_PACKET_HEADER_SIZE;
> > >>
> > >> xfer_size = ALIGN(xfer_size, BYTES_PER_FIFO_WORD);
> > >> - i2c_dev->is_curr_dma_xfer = (xfer_size > I2C_PIO_MODE_MAX_LEN) &&
> > >> - i2c_dev->dma_buf;
> > >> + i2c_dev->is_curr_dma_xfer = !!i2c_dev->dma_buf;
> > >> tegra_i2c_config_fifo_trig(i2c_dev, xfer_size);
> > >> dma = i2c_dev->is_curr_dma_xfer;
> > >> ----------
> > >>
> > >> Here is the log with this change:
> > >>
> > >> [ 0.760796] tegra_rtc 7000e000.rtc: registered as rtc1
> > >> [ 0.760850] tegra_rtc 7000e000.rtc: Tegra internal Real Time Clock
> > >> [ 0.761050] i2c /dev entries driver
> > >> [ 0.918928] tegra-i2c 7000c000.i2c: starting DMA for length: 16
> > >> [ 0.918940] tegra-i2c 7000c000.i2c: unmasked irq: 0c
> > >> [ 0.919040] tegra-i2c 7000c000.i2c: transfer complete: 10 0 0
> > >> [ 0.919050] tegra-i2c 7000c000.i2c: starting DMA for length: 8
> > >> [ 0.919059] tegra-i2c 7000c000.i2c: unmasked irq: 0c
> > >> [ 0.919322] tegra-i2c 7000c000.i2c: transfer complete: 10 0 0
> > >> [ 0.919335] tegra-i2c 7000c000.i2c: starting DMA for length: 16
> > >> [ 0.919343] tegra-i2c 7000c000.i2c: unmasked irq: 0c
> > >> [ 0.919440] tegra-i2c 7000c000.i2c: transfer complete: 10 0 0
> > >> [ 0.919448] tegra-i2c 7000c000.i2c: starting DMA for length: 112
> > >> [ 0.919456] tegra-i2c 7000c000.i2c: unmasked irq: 0c
> > >> [ 0.922818] tegra-i2c 7000c000.i2c: transfer complete: 11 0 0
> > >> [ 0.922829] atmel_mxt_ts 0-004c: Family: 160 Variant: 0 Firmware V1.0.AA Objects: 18
> > >> [ 0.922886] tegra-i2c 7000c000.i2c: starting DMA for length: 16
> > >> [ 0.922895] tegra-i2c 7000c000.i2c: unmasked irq: 0c
> > >> [ 0.922993] tegra-i2c 7000c000.i2c: transfer complete: 10 0 0
> > >> [ 0.923002] tegra-i2c 7000c000.i2c: starting DMA for length: 224
> > >> [ 0.923011] tegra-i2c 7000c000.i2c: unmasked irq: 0c
> > >> [ 0.933253] tegra-i2c 7000c000.i2c: transfer complete: 11 0 0
> > >> [ 0.933287] tegra-i2c 7000c000.i2c: starting DMA for length: 16
> > >> [ 0.933297] tegra-i2c 7000c000.i2c: unmasked irq: 0c
> > >> [ 0.933478] tegra-i2c 7000c000.i2c: transfer complete: 10 0 0
> > >> [ 0.933487] tegra-i2c 7000c000.i2c: starting DMA for length: 12
> > >> [ 0.933496] tegra-i2c 7000c000.i2c: unmasked irq: 0c
> > >> [ 0.945120] tegra-i2c 7000d000.i2c: starting DMA for length: 16
> > >> [ 0.945130] tegra-i2c 7000d000.i2c: unmasked irq: 0c
> > >> [ 1.038917] tegra-i2c 7000c000.i2c: DMA transfer timeout
> > >> [ 1.038982] atmel_mxt_ts 0-004c: __mxt_read_reg: i2c transfer failed (-110)
> > >> [ 1.039000] tegra-i2c 7000c000.i2c: starting DMA for length: 16
> > >> [ 1.039006] atmel_mxt_ts 0-004c: Failed to read T44 and T5 (-110)
>
>
>
> > >> [ 1.039009] tegra-i2c 7000c000.i2c: unmasked irq: 0c
> > >> [ 1.039148] tegra-i2c 7000c000.i2c: transfer complete: 10 0 0
> > >> [ 1.039157] tegra-i2c 7000c000.i2c: starting DMA for length: 4
> > >> [ 1.039166] tegra-i2c 7000c000.i2c: unmasked irq: 0c
> > >> [ 1.039304] tegra-i2c 7000c000.i2c: transfer complete: 10 0 0
> > >> [ 1.039340] tegra-i2c 7000c000.i2c: starting DMA for length: 16
> > >> [ 1.039349] tegra-i2c 7000c000.i2c: unmasked irq: 0c
> > >> [ 1.039535] tegra-i2c 7000c000.i2c: transfer complete: 10 0 0
> > >> [ 1.039544] tegra-i2c 7000c000.i2c: starting DMA for length: 12
> > >> [ 1.039552] tegra-i2c 7000c000.i2c: unmasked irq: 0c
> > >> [ 1.040055] tegra-i2c 7000c000.i2c: transfer complete: 10 0 0
> > >> [ 1.040083] tegra-i2c 7000c000.i2c: starting DMA for length: 16
> > >> [ 1.040092] tegra-i2c 7000c000.i2c: unmasked irq: 0c
> > >> [ 1.040301] tegra-i2c 7000c000.i2c: transfer complete: 10 0 0
> > >> [ 1.048934] tegra-i2c 7000d000.i2c: i2c transfer timed out
> > >
> > > This log shows DMA transfer timeout for atmel read.
> > > Do you see issue if you don’t enforce dma all time and let it choose PIO Vs DMA?
> > >
> >
> > No, there are no timeout errors in PIO mode. I could post full log with the PIO-only mode if you want.
> >
> Yes please post full log of PIO only and DMA only
> Also, in above log, lots of DMA transfer went thru fine except this
> one transfer where DMA timed out.
> DMA completion doesnt happen on DMA submit during this particular
> transaction timeframe for some reason.
Dmitry, can you provide some background information on the setup that
you're using? I vaguely remember something about i2c@7000d000 being
special on older Tegra devices. Perhaps there's something additional
that we need to take into account?
Aha! Look at tegra_i2c_reg_addr() which special-cases I2C_TX_FIFO on
the DVC variant of the controller, which is exactly the one that you
are seeing the timeouts on and it's exactly the register that we use
as destination for the DMA transfer.
I think we need to do something like this:
slv_config.dst_addr = i2c_dev->base_phys + tegra_i2c_reg_addr(i2c_dev, I2C_TX_FIFO);
to make sure we pass the right FIFO register address to DMA.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2019-02-06 14:01 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-05 22:46 [PATCH V12 1/5] i2c: tegra: sort all the include headers alphabetically Sowjanya Komatineni
2019-02-05 22:46 ` [PATCH V12 2/5] i2c: tegra: add bus clear Master Support Sowjanya Komatineni
2019-02-05 22:46 ` [PATCH V12 3/5] i2c: tegra: Add DMA support Sowjanya Komatineni
2019-02-06 11:55 ` Dmitry Osipenko
2019-02-06 12:49 ` Sowjanya Komatineni
2019-02-06 13:06 ` Dmitry Osipenko
2019-02-06 12:59 ` Dmitry Osipenko
2019-02-06 13:03 ` Sowjanya Komatineni
2019-02-06 13:05 ` Dmitry Osipenko
2019-02-06 13:17 ` Sowjanya Komatineni
2019-02-06 13:23 ` Dmitry Osipenko
2019-02-06 13:29 ` Sowjanya Komatineni
2019-02-06 13:30 ` Sowjanya Komatineni
2019-02-06 13:34 ` Dmitry Osipenko
2019-02-06 13:40 ` Dmitry Osipenko
2019-02-06 13:09 ` Thierry Reding
2019-02-06 13:13 ` Thierry Reding
2019-02-06 13:18 ` Dmitry Osipenko
2019-02-06 13:25 ` Sowjanya Komatineni
2019-02-06 13:42 ` Dmitry Osipenko
2019-02-06 13:45 ` Sowjanya Komatineni
2019-02-06 13:51 ` Sowjanya Komatineni
2019-02-06 14:01 ` Thierry Reding [this message]
2019-02-06 14:10 ` Sowjanya Komatineni
2019-02-06 14:13 ` Dmitry Osipenko
2019-02-06 14:19 ` Sowjanya Komatineni
2019-02-06 14:24 ` Dmitry Osipenko
2019-02-06 14:29 ` Sowjanya Komatineni
2019-02-06 14:35 ` Dmitry Osipenko
2019-02-06 14:48 ` Sowjanya Komatineni
2019-02-06 12:40 ` Dmitry Osipenko
2019-02-06 12:48 ` Thierry Reding
2019-02-06 13:01 ` Dmitry Osipenko
2019-02-05 22:46 ` [PATCH V12 4/5] i2c: tegra: update transfer timeout Sowjanya Komatineni
2019-02-05 22:46 ` [PATCH V12 5/5] i2c: tegra: add i2c interface timing support Sowjanya Komatineni
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190206140141.GO21676@ulmo \
--to=thierry.reding@gmail.com \
--cc=digetx@gmail.com \
--cc=jonathanh@nvidia.com \
--cc=linux-i2c@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-tegra@vger.kernel.org \
--cc=mkarthik@nvidia.com \
--cc=skomatineni@nvidia.com \
--cc=smohammed@nvidia.com \
--cc=talho@nvidia.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).