Re: [PATCH 1/2] spi/pl022: timeout on polled transfer

From: Wolfram Sang <w.sang@pengutronix.de>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: Linus Walleij <linus.walleij@stericsson.com>,
	Magnus Templing <magnus.templing@stericsson.com>,
	Grant Likely <grant.likely@secretlab.ca>,
	spi-devel-general@lists.sourceforge.net,
	Lee Jones <lee.jones@linaro.org>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 1/2] spi/pl022: timeout on polled transfer
Date: Thu, 19 May 2011 16:23:23 +0200	[thread overview]
Message-ID: <20110519142323.GD2219@pengutronix.de> (raw)
In-Reply-To: <BANLkTinrk-FjMy1trDnsxwT_NTgQ5x_8Rg@mail.gmail.com>

[-- Attachment #1.1: Type: text/plain, Size: 1396 bytes --]

> >> +             while (pl022->tx < pl022->tx_end || pl022->rx < pl022->rx_end) {
> >> +                     if (time_after(jiffies, timeout)) {
> >> +                             dev_warn(&pl022->adev->dev,
> >> +                             "%s: timeout!\n", __func__);
> >> +                             message->state = STATE_ERROR;
> >> +                             goto out;
> >> +                     }
> >>                       readwriter(pl022);
> >> +             }
> 
> What about we move readerwriter() above if (time_after...) then it atleast
> gets one chance to run even if we're preempted for 10 seconds.

This will fix the initial problem, but still exit early if e.g. the
preemption came on a subsequent iteration of the while loop, say during
readwriter().

> 
> Any other design patterns that'd be better?

A bit better might be:

	while (...) {
		time = jiffies;
		readwriter();
		if (time_after(time, timeout)...)
			...
	}

So you continue triggering transfers as long as the last one was in the
valid timespan. In case of an preemption, you made sure you tried the
whole timespan (+ the jitter) which is probably better than stop trying
too early.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel