All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mika Westerberg <mika.westerberg@linux.intel.com>
To: Russell King - ARM Linux <linux@armlinux.org.uk>
Cc: Andrew Jackson <Andrew.Jackson@arm.com>,
	Liviu Dudau <Liviu.Dudau@arm.com>,
	Wolfram Sang <wsa@the-dreams.de>,
	Jarkko Nikula <jarkko.nikula@linux.intel.com>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	linux-i2c@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [BUG] i2c-designware silently fails on long transfers
Date: Mon, 21 Nov 2016 13:09:47 +0200	[thread overview]
Message-ID: <20161121110947.GI1446@lahna.fi.intel.com> (raw)
In-Reply-To: <20161121104329.GB1041@n2100.armlinux.org.uk>

On Mon, Nov 21, 2016 at 10:43:29AM +0000, Russell King - ARM Linux wrote:
> On Mon, Nov 21, 2016 at 12:29:01PM +0200, Mika Westerberg wrote:
> > On Fri, Nov 18, 2016 at 07:35:42PM +0000, Russell King - ARM Linux wrote:
> > > Another mitigation would be to lower the I2C bus frequency on Juno from
> > > 400kHz to 100kHz, so that there's 4x longer IRQ latency possible.
> > > However, even that isn't going to be reliable - even going to 100kHz
> > > isn't going to allow the above case to be solved - the interrupt is
> > > delayed by around 2ms, and it takes about 1.4ms to send/receive 16 bytes
> > > at 100kHz.  (9 * 16 / (100*10^3)).
> > > 
> > > So, I think all hope is lost for i2c-designware on Juno to cope with
> > > reading the EDID from TDA998x reliably.
> > 
> > :-(
> > 
> > I wonder if we can get it work more reliably by using DMA (provided that
> > there are DMA channels available for I2C in Juno)? That would allow the
> > hardware to perform longer reads without relying on how fast the
> > interrupt handler is able to empty the Rx FIFO.
> 
> It would need to DMA to the Tx FIFO to keep it filled - it triggers the
> stop condition when the Tx FIFO empties.  From what I can see in the
> driver, the Tx FIFO not only takes the data but also a "command" to tell
> the hardware what to do.

Yes, the command is either "read" or "write" (meaning even for Rx a
write to the Tx FIFO is needed).

> The Rx FIFO would also need DMA to avoid it overflowing due to high
> interrupt latency.

I guess for Rx we would need to supply a dummy buffer of "read" commands
for the Tx FIFO in addition to reading bytes from Rx FIFO. But still
that might help to improve reliability in case of Juno.

> I don't know what state DMA is in on the Juno, or even whether it has
> DMA - it has a PL330 DMA controller, but I see nothing in the DT files
> making use of it.

OK

WARNING: multiple messages have this Message-ID (diff)
From: mika.westerberg@linux.intel.com (Mika Westerberg)
To: linux-arm-kernel@lists.infradead.org
Subject: [BUG] i2c-designware silently fails on long transfers
Date: Mon, 21 Nov 2016 13:09:47 +0200	[thread overview]
Message-ID: <20161121110947.GI1446@lahna.fi.intel.com> (raw)
In-Reply-To: <20161121104329.GB1041@n2100.armlinux.org.uk>

On Mon, Nov 21, 2016 at 10:43:29AM +0000, Russell King - ARM Linux wrote:
> On Mon, Nov 21, 2016 at 12:29:01PM +0200, Mika Westerberg wrote:
> > On Fri, Nov 18, 2016 at 07:35:42PM +0000, Russell King - ARM Linux wrote:
> > > Another mitigation would be to lower the I2C bus frequency on Juno from
> > > 400kHz to 100kHz, so that there's 4x longer IRQ latency possible.
> > > However, even that isn't going to be reliable - even going to 100kHz
> > > isn't going to allow the above case to be solved - the interrupt is
> > > delayed by around 2ms, and it takes about 1.4ms to send/receive 16 bytes
> > > at 100kHz.  (9 * 16 / (100*10^3)).
> > > 
> > > So, I think all hope is lost for i2c-designware on Juno to cope with
> > > reading the EDID from TDA998x reliably.
> > 
> > :-(
> > 
> > I wonder if we can get it work more reliably by using DMA (provided that
> > there are DMA channels available for I2C in Juno)? That would allow the
> > hardware to perform longer reads without relying on how fast the
> > interrupt handler is able to empty the Rx FIFO.
> 
> It would need to DMA to the Tx FIFO to keep it filled - it triggers the
> stop condition when the Tx FIFO empties.  From what I can see in the
> driver, the Tx FIFO not only takes the data but also a "command" to tell
> the hardware what to do.

Yes, the command is either "read" or "write" (meaning even for Rx a
write to the Tx FIFO is needed).

> The Rx FIFO would also need DMA to avoid it overflowing due to high
> interrupt latency.

I guess for Rx we would need to supply a dummy buffer of "read" commands
for the Tx FIFO in addition to reading bytes from Rx FIFO. But still
that might help to improve reliability in case of Juno.

> I don't know what state DMA is in on the Juno, or even whether it has
> DMA - it has a PL330 DMA controller, but I see nothing in the DT files
> making use of it.

OK

  reply	other threads:[~2016-11-21 11:09 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-18 19:35 [BUG] i2c-designware silently fails on long transfers Russell King - ARM Linux
2016-11-18 19:35 ` Russell King - ARM Linux
2016-11-18 19:40 ` [PATCH 1/2] i2c: designware: report short transfers Russell King
2016-11-18 19:40   ` Russell King
2016-11-21 10:32   ` Mika Westerberg
2016-11-21 10:32     ` Mika Westerberg
2016-11-23 14:13     ` Jarkko Nikula
2016-11-23 14:13       ` Jarkko Nikula
2016-11-24 15:18   ` Wolfram Sang
2016-11-24 15:18     ` Wolfram Sang
2016-11-18 19:40 ` [PATCH 2/2] i2c: designware: fix rx fifo depth tracking Russell King
2016-11-18 19:40   ` Russell King
2016-11-21 10:40   ` Mika Westerberg
2016-11-21 10:40     ` Mika Westerberg
2016-11-23 14:13     ` Jarkko Nikula
2016-11-23 14:13       ` Jarkko Nikula
2016-11-24 15:18   ` Wolfram Sang
2016-11-24 15:18     ` Wolfram Sang
2016-11-21 10:29 ` [BUG] i2c-designware silently fails on long transfers Mika Westerberg
2016-11-21 10:29   ` Mika Westerberg
2016-11-21 10:43   ` Russell King - ARM Linux
2016-11-21 10:43     ` Russell King - ARM Linux
2016-11-21 11:09     ` Mika Westerberg [this message]
2016-11-21 11:09       ` Mika Westerberg
2016-11-21 11:21     ` Liviu Dudau
2016-11-21 11:21       ` Liviu Dudau
2016-11-21 11:36       ` Russell King - ARM Linux
2016-11-21 11:36         ` Russell King - ARM Linux
2016-11-21 12:11     ` Robin Murphy
2016-11-21 12:11       ` Robin Murphy

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=20161121110947.GI1446@lahna.fi.intel.com \
    --to=mika.westerberg@linux.intel.com \
    --cc=Andrew.Jackson@arm.com \
    --cc=Liviu.Dudau@arm.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=jarkko.nikula@linux.intel.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=wsa@the-dreams.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.