All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lars-Peter Clausen <lars@metafoo.de>
To: Shubhrajyoti Datta <shubhrajyoti.datta@gmail.com>
Cc: Wolfram Sang <wsa@the-dreams.de>,
	Shubhrajyoti Datta <shubhraj@xilinx.com>,
	linux-i2c@vger.kernel.org
Subject: Re: [PATCH 1/3] i2c: Revert "i2c: xiic: Do not reset controller before every transfer"
Date: Tue, 17 Nov 2015 08:34:30 +0100	[thread overview]
Message-ID: <564AD886.3060108@metafoo.de> (raw)
In-Reply-To: <CAKfKVtGqPPP=bnWH4dPLT1=gjrvZe=0zmix58SKBT79MsR+CXg@mail.gmail.com>

On 11/17/2015 06:17 AM, Shubhrajyoti Datta wrote:
> On Mon, Nov 16, 2015 at 7:12 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
>> Commit d701667bb331 ("i2c: xiic: Do not reset controller before every
>> transfer") removed the reinitialization of the controller before the start
>> of each transfer. Apparently this change is not safe to make and the commit
>> results in random I2C bus failures.
> 
> Which is the platform and the ip version that you  saw the issue.
> Did you see the issue with read and write as  well?

The IP version is the axi-iic v2.0 Revision 8. I've tested this on a few
platforms, custom ones and standard ones and I could reproduce it on most.
One of them was on the ZED board. The one where I couldn't reproduce it was
the ZC706. But that doesn't necessarily mean it doesn't happen there, just
that it is not triggered by the testcase.

The problem is that it is random corruption, so some I2C devices might start
to behave strangely at some point. The only good more or less reliable way
to reproduce it that I found was to run i2cdetect a couple of times and at
least one of them will produce strange behavior.

>>
>> An easy way to trigger the issue is to run i2cdetect.
>>
>> Without the patch applied:
>>      0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f
>>          00:          -- -- -- -- -- -- -- -- -- -- -- -- --
>>          10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
>>          20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
>>          30: -- -- -- -- -- -- -- -- UU UU -- UU 3c -- -- UU
>>          40: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
>>          50: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
>>          60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
>>          70: -- -- -- -- -- -- -- --
>>
>> With the patch applied every other or so invocation:
>>      0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f
>>          00:          03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f
>>          10: 10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f
>>          20: 20 21 22 23 24 25 26 27 28 29 2a 2b 2c 2d 2e 2f
>>          30: -- -- -- -- -- -- -- -- UU UU -- UU 3c -- -- UU
>>          40: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
>>          50: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
>>          60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
>>          70: -- -- -- -- -- -- -- --
>>
> I assume that you have these many peripherals.
> on the bus am I right?

Sorry, I should have been more clear. The first one is before the patch that
introduces the issue, the second one is with the patch that introduces the
issue.

- Lars

  parent reply	other threads:[~2015-11-17  7:34 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-16 13:42 [PATCH 1/3] i2c: Revert "i2c: xiic: Do not reset controller before every transfer" Lars-Peter Clausen
2015-11-16 13:42 ` [PATCH 2/3] i2c: xiic: Prevent concurrent running of the IRQ handler and __xiic_start_xfer() Lars-Peter Clausen
2015-11-17  5:32   ` Shubhrajyoti Datta
2015-11-20 15:16   ` Wolfram Sang
2015-11-16 13:42 ` [PATCH 3/3] i2c: xiic: Replace spinlock with mutex Lars-Peter Clausen
2015-11-17  5:18   ` Shubhrajyoti Datta
2015-11-17  5:20     ` Shubhrajyoti Datta
2015-11-17  5:17 ` [PATCH 1/3] i2c: Revert "i2c: xiic: Do not reset controller before every transfer" Shubhrajyoti Datta
2015-11-17  5:56   ` Shubhrajyoti Datta
2015-11-17  7:34   ` Lars-Peter Clausen [this message]
2015-11-17 14:28     ` Shubhrajyoti Datta
2015-11-18 10:01       ` Lars-Peter Clausen
2015-11-19  7:20         ` Shubhrajyoti Datta
2015-11-20 15:14           ` Wolfram Sang

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=564AD886.3060108@metafoo.de \
    --to=lars@metafoo.de \
    --cc=linux-i2c@vger.kernel.org \
    --cc=shubhraj@xilinx.com \
    --cc=shubhrajyoti.datta@gmail.com \
    --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.