RE: Re: [PATCH] IIO: Change msleep to usleep_range for small msecs

From: "Aniroop Mathur" <a.mathur@samsung.com>
To: "Lars-Peter Clausen" <lars@metafoo.de>,
	"Jonathan Cameron" <jic23@kernel.org>,
	"knaack.h@gmx.de" <knaack.h@gmx.de>,
	"pmeerw@pmeerw.net" <pmeerw@pmeerw.net>,
	"linux-iio@vger.kernel.org" <linux-iio@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Cc: "SAMUEL SEQUEIRA" <s.samuel@samsung.com>,
	"Rahul Mahale" <r.mahale@samsung.com>,
	"aniroop.mathur@gmail.com" <aniroop.mathur@gmail.com>,
	"Linus Walleij" <linus.walleij@stericsson.com>,
	"Vlad Dogaru" <vlad.dogaru@intel.com>
Subject: RE: Re: [PATCH] IIO: Change msleep to usleep_range for small msecs
Date: Wed, 30 Nov 2016 14:43:45 +0000	[thread overview]
Message-ID: <20161130144345epcms5p24751680d9fec0f33d5283db9e09410ca@epcms5p2> (raw)
In-Reply-To: CGME20161130133557epcas2p2caca5d58770bf31614812d14b809fe8f@epcas2p2.samsung.com

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

On 30 Nov 2016 19:05, "Lars-Peter Clausen" <lars@metafoo.de> wrote:
 >
 > On 11/27/2016 11:51 AM, Jonathan Cameron wrote:
 > > On 26/11/16 03:47, Aniroop Mathur wrote:
 > >> msleep(1~20) may not do what the caller intends, and will often sleep longer.
 > >> (~20 ms actual sleep for any value given in the 1~20ms range)
 > >> This is not the desired behaviour for many cases like device resume time,
 > >> device suspend time, device enable time, data reading time, etc.
 > >> Thus, change msleep to usleep_range for precise wakeups.
 > >>
 > >> Signed-off-by: Aniroop Mathur <a.mathur@samsung.com>
 > > As these need individual review by the various original authors and driver maintainers to
 > > establish the intent of the sleep, it would have been better to have done a series of
 > > patches (one per driver) with the relevant maintainers cc'd on the ones that they care about.
 > >
 > > Most of these are ADI parts looked after by Lars though so perhaps wait for his comments
 > > before respining.
 >
 > I agree with what Jonathan said. For most of these extending the maximum
 > sleep time a bit further is ok.
 >

Well, its right that sleep a bit further is okay but this patch is not trying
to solve any major bug. This patch is only trying to make the wake up more
precise than before. So like with msleep(1), process could sleep for 20 ms
so this patch only making the making the process to sleep for 1 ms as
mentioned in the parameter. So I think, changing to usleep_range is only
advantageous and does not cause any harm here.
We have also applied the same change in enable/disable/suspend/resume
functions in accel, gyro, mag, etc drivers and found decent results like the
first sesor data is generated much faster than before so response time
increased. This is critical as  sensor can run at a rate of 200Hz / 5ms or
even more now a days in new devices. We even applied in probe as doing the
same in many drivers contribute to a little saving overall in kernel boot up.
Also, it is recommended and mentioned in kernel documentation to use
usleep_range for 1-10 ms.
Sources -
https://www.kernel.org/doc/Documentation/timers/timers-howto.txt
https://lkml.org/lkml/2007/8/3/250

Thanks.

BR,
Aniroop Mathur