Re: [RFC PATCH] i2c: busses: i2c-omap: Increase timeout for i2c interrupt

From: Wolfram Sang <wsa@the-dreams.de>
To: 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 11:09:09 +0200	[thread overview]
Message-ID: <20150710090909.GF1528@katana> (raw)
In-Reply-To: <559F8670.2060305@nokia.com>

[-- Attachment #1: Type: text/plain, Size: 1410 bytes --]

> 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 would propose you to throw away spinlocks. Convert threaded IRQ to
> just one hardirq handler. And continue debugging. You will reduce the
> load of the system with the above measures, maybe it will not happen
> any more, maybe you'll figure out that problem is somewhere else.

Or this.

Felipe converted it to threaded_irq, maybe he has something to add?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]