All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sebastian Andrzej Siewior <bigeasy-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
To: Mike Rapoport
	<mike.rapoport-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Mauro Carvalho Chehab
	<mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>,
	"linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Mike Galbraith
	<umgwanakikbuti-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
Subject: Re: rt-mutex usage in i2c
Date: Mon, 16 Mar 2015 21:50:56 +0100	[thread overview]
Message-ID: <55074230.20307@linutronix.de> (raw)
In-Reply-To: <CABpLfoiQg1smiebL0=nWX4Sp1H+XD9VViUqGk13gRcfdAwkFoA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On 03/15/2015 08:07 AM, Mike Rapoport wrote:
> On Sat, Mar 14, 2015 at 1:32 PM, Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> wrote:
>> On Sat, Mar 14, 2015 at 12:27:03PM +0100, Wolfram Sang wrote:
>>> Hi Sebastian,
>>>
>>>> - i2c_transfer() has this piece:
>>>>   2091                 if (in_atomic() || irqs_disabled()) {
>>>>   2092                         ret = i2c_trylock_adapter(adap);
>>>>
>>>>   is this irqs_disabled() is what bothers me and should not be there.
>>>>   pxa does a spin_lock_irq() which would enable interrupts on return /
>>>>   too early.
>>>>   mxs has a wait_for_completion() which needs irqs enabled _and_ makes
>>>>   in_atomic() problematic, too. I have't checked other drivers but the
>>>>   commit, that introduced it, does not explain why it is required.
> 
> That was some time ago, but as far as I remember, PIO in i2c_pxa was
> required to enable communication with PMIC in interrupt context.

Let me add one thing I forgot: the locking is using raw locks which are
not irq safe. It usually works. But. If the wait_lock is hold during
the unlock's slow path (that means there is no owner but the owner
field is not yet NULL) and the interrupt handler gets here with a
try_lock attempt then and it will spin forever on the wait_lock.

I will try to lookup the threads later…

Sebastian

WARNING: multiple messages have this Message-ID (diff)
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
To: Mike Rapoport <mike.rapoport@gmail.com>,
	Wolfram Sang <wsa@the-dreams.de>
Cc: linux-i2c@vger.kernel.org,
	Mauro Carvalho Chehab <mchehab@osg.samsung.com>,
	"linux-media@vger.kernel.org" <linux-media@vger.kernel.org>,
	Mike Galbraith <umgwanakikbuti@gmail.com>,
	Jean Delvare <khali@linux-fr.org>
Subject: Re: rt-mutex usage in i2c
Date: Mon, 16 Mar 2015 21:50:56 +0100	[thread overview]
Message-ID: <55074230.20307@linutronix.de> (raw)
In-Reply-To: <CABpLfoiQg1smiebL0=nWX4Sp1H+XD9VViUqGk13gRcfdAwkFoA@mail.gmail.com>

On 03/15/2015 08:07 AM, Mike Rapoport wrote:
> On Sat, Mar 14, 2015 at 1:32 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
>> On Sat, Mar 14, 2015 at 12:27:03PM +0100, Wolfram Sang wrote:
>>> Hi Sebastian,
>>>
>>>> - i2c_transfer() has this piece:
>>>>   2091                 if (in_atomic() || irqs_disabled()) {
>>>>   2092                         ret = i2c_trylock_adapter(adap);
>>>>
>>>>   is this irqs_disabled() is what bothers me and should not be there.
>>>>   pxa does a spin_lock_irq() which would enable interrupts on return /
>>>>   too early.
>>>>   mxs has a wait_for_completion() which needs irqs enabled _and_ makes
>>>>   in_atomic() problematic, too. I have't checked other drivers but the
>>>>   commit, that introduced it, does not explain why it is required.
> 
> That was some time ago, but as far as I remember, PIO in i2c_pxa was
> required to enable communication with PMIC in interrupt context.

Let me add one thing I forgot: the locking is using raw locks which are
not irq safe. It usually works. But. If the wait_lock is hold during
the unlock's slow path (that means there is no owner but the owner
field is not yet NULL) and the interrupt handler gets here with a
try_lock attempt then and it will spin forever on the wait_lock.

I will try to lookup the threads later…

Sebastian

  parent reply	other threads:[~2015-03-16 20:50 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-09 13:43 rt-mutex usage in i2c Sebastian Andrzej Siewior
2015-03-14 11:27 ` Wolfram Sang
2015-03-14 11:32   ` Wolfram Sang
2015-03-15  7:07     ` Mike Rapoport
2015-03-15  7:07       ` Mike Rapoport
     [not found]       ` <CABpLfoiQg1smiebL0=nWX4Sp1H+XD9VViUqGk13gRcfdAwkFoA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-03-16 20:50         ` Sebastian Andrzej Siewior [this message]
2015-03-16 20:50           ` Sebastian Andrzej Siewior

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=55074230.20307@linutronix.de \
    --to=bigeasy-hfztesqfncyowbw4kg4ksq@public.gmane.org \
    --cc=khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org \
    --cc=linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org \
    --cc=mike.rapoport-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=umgwanakikbuti-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org \
    /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.