Re: [PATCH 1/2] iio: ltc2983: add support for optional reset gpio

From: Andy Shevchenko <andy.shevchenko@gmail.com>
To: "Nuno Sá" <noname.nuno@gmail.com>
Cc: "Nuno Sá" <nuno.sa@analog.com>,
	linux-iio@vger.kernel.org, "Jonathan Cameron" <jic23@kernel.org>,
	"Lars-Peter Clausen" <lars@metafoo.de>,
	"Drew Fustini" <drew@pdp7.com>
Subject: Re: [PATCH 1/2] iio: ltc2983: add support for optional reset gpio
Date: Mon, 23 Aug 2021 17:27:20 +0300	[thread overview]
Message-ID: <CAHp75Vcu47BUKPVVtJK0U4G5_6ey-U=o5yDjCH43Nz7TsH+NMA@mail.gmail.com> (raw)
In-Reply-To: <e81553af88194c89f6d31bf83e6e486fe75bd4ea.camel@gmail.com>

On Mon, Aug 23, 2021 at 3:51 PM Nuno Sá <noname.nuno@gmail.com> wrote:
> On Mon, 2021-08-23 at 14:14 +0300, Andy Shevchenko wrote:
> > On Fri, Aug 20, 2021 at 9:53 AM Nuno Sá <nuno.sa@analog.com> wrote:
> > > Check if an optional reset gpio is present and if so, make sure to
> > > reset
> > > the device.
> >
> > ...
> >
> > > +               usleep_range(1000, 1005);
> >
> > The delta should be at least 20%, otherwise I'm not sure why such a
> > strict range?
> >
>
> No special reason... I just had no hard requirement for delta so I just
> gave something small. Is 20% documented anywhere?

Quick search shows nothing, but I remember I saw it somewhere.
So, the explanation is empirical, because the idea behind is to allow
less HRT interrupts. When you do a tough margin, you may generate too
many interrupts from the timer. So, 20% seems like a good balance for
most of the values.

The parameters to take into account are:
 - minimum (or maybe rather median?) CPU frequency the code will be run on
 - minimal sleep (for small sleeps even better to have udelay() as I
believe documented in timers.rst, for bigger sleeps, like 10ms the
margin can be 10% or so)
 - NO_HZ kernel configuration
 - etc (if anything I forgot)

> I did a quick look on
> the API's and I could not find nothing related. Anyways, if that is a
> best practise for being more power friendly I'm happy to change it...
> (well, we might end up just having 100ms here which means 'msleep()').

-- 
With Best Regards,
Andy Shevchenko