linux-i2c.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Oliver Neukum <oneukum@suse.com>
To: Hans de Goede <hdegoede@redhat.com>, Max Staudt <mstaudt@suse.de>
Cc: linux-i2c@vger.kernel.org
Subject: Re: entering the error case of i2c-designware with a timeout at probe
Date: Tue, 21 Mar 2017 15:05:11 +0100	[thread overview]
Message-ID: <1490105111.8154.22.camel@suse.com> (raw)
In-Reply-To: <57e0b3a8-be5a-7b73-f47b-34d02847d3b7@redhat.com>

Am Dienstag, den 21.03.2017, 14:55 +0100 schrieb Hans de Goede:
> Hi,
> 
> On 21-03-17 14:36, Oliver Neukum wrote:
> > 
> > Am Dienstag, den 21.03.2017, 14:01 +0100 schrieb Hans de Goede:
> > > 
> > > Hi,
> > > 
> > > On 21-03-17 13:57, Max Staudt wrote:

Hello,

> > > > In other words, whether we should rather wait for lock acquisition,
> > > > unconditionally. No timeout, just wait. That's what our hardware
> > > > seems to need.
> > > > 
> > > > It feels like once the lock has been requested by the Linux driver,
> > > > we can't cancel that request and have to actually follow through
> > > > with accepting the lock and only giving it back after that.
> > > > Resetting the "request" bit to 0 as it is done now doesn't work as
> > > > it leads to the hung system sometime soon after that.
> > > 
> > > Hmm, interesting theory. I would say give it a test and if it
> > > helps then maybe increase the timeouts to say 10 seconds or
> > > such, so that e.g. on poweroff we at least report an error
> > > rather then just sitting there ?
> > 
> > I did some testing. Unconditionally taking the error path without waiting
> > for the semaphore crashes or freezes the machine in about 3/4 of all
> > tests.
> > As I said, with a timeout of 500ms, this issue is not seen.
> 
> Ah ok, so that patch is enough to fix this ? Then as I already

Yes, it is enough.

> said I'm fine with that patch, needing long timeouts unfortunately
> seems normal when dealing with embedded micro-controllers, I've
> seen the same with e.g. ACPI embedded-controllers.

I am quite uncomfortable with code in the kernel that will crash
the machine if it ever runs. Yet I am also uncomfortable with code
that would run forever.

> > Do you have reliable information that the error handling works
> > on BayTrail?
> 
> No, Bay Trail actually is known to not always be stable with Linux.

So this code is best effort just in case?

	Regards
		Oliver

  reply	other threads:[~2017-03-21 14:43 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-21 12:52 entering the error case of i2c-designware with a timeout at probe Oliver Neukum
2017-03-21 12:57 ` Max Staudt
2017-03-21 13:01   ` Hans de Goede
2017-03-21 13:36     ` Oliver Neukum
2017-03-21 13:55       ` Hans de Goede
2017-03-21 14:05         ` Oliver Neukum [this message]
2017-03-21 14:48           ` Hans de Goede
2017-03-21 15:37             ` Oliver Neukum
2017-03-21 15:40               ` Hans de Goede
2017-03-21 13:00 ` Hans de Goede

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=1490105111.8154.22.camel@suse.com \
    --to=oneukum@suse.com \
    --cc=hdegoede@redhat.com \
    --cc=linux-i2c@vger.kernel.org \
    --cc=mstaudt@suse.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 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).