From: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>
To: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>,
Alexander Sverdlin
<alexander.sverdlin-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>,
Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
Cc: ext Vignesh R <vigneshr-l0cyMroinI0@public.gmane.org>,
Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>,
linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [RFC PATCH] i2c: busses: i2c-omap: Increase timeout for i2c interrupt
Date: Fri, 10 Jul 2015 17:02:39 +0300 [thread overview]
Message-ID: <559FD07F.7040402@ti.com> (raw)
In-Reply-To: <20150710090909.GF1528@katana>
Hi Wolfram,
On 07/10/2015 12:09 PM, Wolfram Sang wrote:
>
>> 60 s sounds way too much and actually I simply don't believe this is
>> the root cause. If I take a look into the driver, then I see, that
>
> I agree, this is just a workaround.
>
>> the design is not really the best. The whole IRQ handling could be
>> actually performed in hard IRQ handler, without threading overhead.
>> Putting even 2 bytes in the controller FIFO should not be too heavy
>> for the hard IRQ handler. Then these ridiculous spin_lock()s. What
>> is the reason behind? The IRQ is flagged with ONESHOT, so thread and
>> hardirq handler are anyway mutually excluded. But if this thread
>> ever runs longer than it's allowed in IRQ context, then it anyway
>> produces this IRQ latency because it locks spin_lock_irqsave() for
>> the whole time! So the whole point of threaded interrupt is missing.
>
> Furthermore, this combination of threaded_irq and struct completion seems
> bogus to me. If you just want to ensure the irq happened before timeout,
> you just complete when the irq happened and do the "bottom half" after the
> completion returned?
>
I'd very appreciated if You would be able to clarify your point a bit, pls?
completion is used to indicate end of one message transfer (+check for msg timeout),
so .master_xfer()->omap_i2c_xfer could switch to next msg.
And there could be more than on IRQ triggered depending on msg length
while one message is being transfered.
--
regards,
-grygorii
WARNING: multiple messages have this Message-ID (diff)
From: Grygorii Strashko <grygorii.strashko@ti.com>
To: Wolfram Sang <wsa@the-dreams.de>,
Alexander Sverdlin <alexander.sverdlin@nokia.com>,
Felipe Balbi <balbi@ti.com>
Cc: ext Vignesh R <vigneshr@ti.com>, Tony Lindgren <tony@atomide.com>,
<linux-omap@vger.kernel.org>, <linux-i2c@vger.kernel.org>,
<linux-kernel@vger.kernel.org>
Subject: Re: [RFC PATCH] i2c: busses: i2c-omap: Increase timeout for i2c interrupt
Date: Fri, 10 Jul 2015 17:02:39 +0300 [thread overview]
Message-ID: <559FD07F.7040402@ti.com> (raw)
In-Reply-To: <20150710090909.GF1528@katana>
Hi Wolfram,
On 07/10/2015 12:09 PM, Wolfram Sang wrote:
>
>> 60 s sounds way too much and actually I simply don't believe this is
>> the root cause. If I take a look into the driver, then I see, that
>
> I agree, this is just a workaround.
>
>> the design is not really the best. The whole IRQ handling could be
>> actually performed in hard IRQ handler, without threading overhead.
>> Putting even 2 bytes in the controller FIFO should not be too heavy
>> for the hard IRQ handler. Then these ridiculous spin_lock()s. What
>> is the reason behind? The IRQ is flagged with ONESHOT, so thread and
>> hardirq handler are anyway mutually excluded. But if this thread
>> ever runs longer than it's allowed in IRQ context, then it anyway
>> produces this IRQ latency because it locks spin_lock_irqsave() for
>> the whole time! So the whole point of threaded interrupt is missing.
>
> Furthermore, this combination of threaded_irq and struct completion seems
> bogus to me. If you just want to ensure the irq happened before timeout,
> you just complete when the irq happened and do the "bottom half" after the
> completion returned?
>
I'd very appreciated if You would be able to clarify your point a bit, pls?
completion is used to indicate end of one message transfer (+check for msg timeout),
so .master_xfer()->omap_i2c_xfer could switch to next msg.
And there could be more than on IRQ triggered depending on msg length
while one message is being transfered.
--
regards,
-grygorii
next prev parent reply other threads:[~2015-07-10 14:02 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-10 5:09 [RFC PATCH] i2c: busses: i2c-omap: Increase timeout for i2c interrupt Vignesh R
2015-07-10 5:09 ` Vignesh R
[not found] ` <1436504994-31137-1-git-send-email-vigneshr-l0cyMroinI0@public.gmane.org>
2015-07-10 8:09 ` Roger Quadros
2015-07-10 8:09 ` Roger Quadros
2015-07-10 8:46 ` Alexander Sverdlin
2015-07-10 9:09 ` Wolfram Sang
2015-07-10 13:17 ` Vignesh R
2015-07-10 13:17 ` Vignesh R
[not found] ` <559FC5D7.3000108-l0cyMroinI0@public.gmane.org>
2015-07-10 13:26 ` Alexander Sverdlin
2015-07-10 13:26 ` Alexander Sverdlin
2015-07-10 13:44 ` Vignesh R
2015-07-10 13:44 ` Vignesh R
[not found] ` <559FC7E9.1060003-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
2015-07-10 13:48 ` Grygorii Strashko
2015-07-10 13:48 ` Grygorii Strashko
2015-07-10 14:02 ` Grygorii Strashko [this message]
2015-07-10 14:02 ` Grygorii Strashko
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=559FD07F.7040402@ti.com \
--to=grygorii.strashko-l0cymroini0@public.gmane.org \
--cc=alexander.sverdlin-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org \
--cc=balbi-l0cyMroinI0@public.gmane.org \
--cc=linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org \
--cc=vigneshr-l0cyMroinI0@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.