Re: [RFC] Improving udelay/ndelay on platforms where that is possible - Russell King

From: Russell King - ARM Linux <linux@armlinux.org.uk>
To: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
Cc: Nicolas Pitre <nicolas.pitre@linaro.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Alan Cox <gnomes@lxorguk.ukuu.org.uk>,
	LKML <linux-kernel@vger.kernel.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Ingo Molnar <mingo@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Peter Zijlstra <peterz@infradead.org>,
	John Stultz <john.stultz@linaro.org>,
	Douglas Anderson <dianders@chromium.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	Jonathan Austin <jonathan.austin@arm.com>,
	Arnd Bergmann <arnd@arndb.de>, Kevin Hilman <khilman@kernel.org>,
	Michael Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@codeaurora.org>,
	Boris Brezillon <boris.brezillon@free-electrons.com>,
	Thibaud Cornic <thibaud_cornic@sigmadesigns.com>,
	Mason <slash.tmp@free.fr>
Subject: Re: [RFC] Improving udelay/ndelay on platforms where that is possible
Date: Thu, 16 Nov 2017 16:32:54 +0000	[thread overview]
Message-ID: <20171116163254.GK31757@n2100.armlinux.org.uk> (raw)
In-Reply-To: <48c38055-20f7-e565-aa56-74f360e6e3d9@sigmadesigns.com>

On Thu, Nov 16, 2017 at 05:26:32PM +0100, Marc Gonzalez wrote:
> On 16/11/2017 17:08, Nicolas Pitre wrote:
> 
> > On Thu, 16 Nov 2017, Marc Gonzalez wrote:
> > 
> >> On 16/11/2017 16:36, Russell King - ARM Linux wrote:
> >>> On Thu, Nov 16, 2017 at 04:26:51PM +0100, Marc Gonzalez wrote:
> >>>> On 15/11/2017 14:13, Russell King - ARM Linux wrote:
> >>>>
> >>>>> udelay() needs to offer a consistent interface so that drivers know
> >>>>> what to expect no matter what the implementation is.  Making one
> >>>>> implementation conform to your ideas while leaving the other
> >>>>> implementations with other expectations is a recipe for bugs.
> >>>>>
> >>>>> If you really want to do this, fix the loops_per_jiffy implementation
> >>>>> as well so that the consistency is maintained.
> >>>>
> >>>> Hello Russell,
> >>>>
> >>>> It seems to me that, when using DFS, there's a serious issue with loop-based
> >>>> delays. (IIRC, it was you who pointed this out a few years ago.)
> >>>>
> >>>> If I'm reading arch/arm/kernel/smp.c correctly, loops_per_jiffy is scaled
> >>>> when the frequency changes.
> >>>>
> >>>> But arch/arm/lib/delay-loop.S starts by loading the current value of
> >>>> loops_per_jiffy, computes the number of times to loop, and then loops.
> >>>> If the frequency increases when the core is in __loop_delay, the
> >>>> delay will be much shorter than requested.
> >>>>
> >>>> Is this a correct assessment of the situation?
> >>>
> >>> Absolutely correct, and it's something that people are aware of, and
> >>> have already catered for while writing their drivers.
> >>
> >> In their cpufreq driver?
> >> In "real" device drivers that happen to use delays?
> >>
> >> On my system, the CPU frequency may ramp up from 120 MHz to 1.2 GHz.
> >> If the frequency increases at the beginning of __loop_delay, udelay(100)
> >> would spin only 10 microseconds. This is likely to cause issues in
> >> any driver using udelay.
> >>
> >> How does one cater for that?
> > 
> > You make sure your delays are based on a stable hardware timer.
> > Most platforms nowadays should have a suitable timer source.
> 
> So you propose fixing loop-based delays by using clock-based delays,
> is that correct? (That is indeed what I did on my platform.)
> 
> Russell stated that there are platforms using loop-based delays with
> cpufreq enabled. I'm asking how they manage the brokenness.

Quite simply, they don't have such a wide range of frequencies that can
be selected.  They're on the order of 4x.  For example, the original
platform that cpufreq was developed on, a StrongARM-1110 board, can
practically range from 221MHz down to 59MHz.

BTW, your example above is incorrect.  If I'm not mistaken "120 MHz to
1.2 GHz." is only a 10x range, not a 100x range.  That would mean if
udelay(100) is initially executed while at 1.2GHz, and the frequency
drops to 120MHz during that delay, the delay could be anything between
just short of 100us to just short of 1ms.

For 10ms to come into it, you'd need to range from 1.2GHz down to
0.012GHz, iow, 12MHz.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up