Re: [PATCH v5 3/3] media: i2c: imx274: Add IMX274 power on and off sequence

From: Sowjanya Komatineni
Date: Tue Sep 08 2020 - 14:13:50 EST

On 9/7/20 12:48 AM, Jacopo Mondi wrote:

On Fri, Sep 04, 2020 at 10:04:10AM -0700, Sowjanya Komatineni wrote:
On 9/4/20 1:55 AM, Jacopo Mondi wrote:
usleep_range() allows you to provide an interval in which your timeout
can be coalesced with others. Giving a [1usec, 2usec] range kind of
defeat the purpose. And most than everything, does sleeping for 2usec
serve any real purpose ?
Following delay recommendation from DS for power on sequence.

2 useconds ? Seems very short:)

As per IMX274 datasheet for power on sequence, 100ns is the min wait time
after the last power supply of 1v8/1v2/2v8 is ON before releasing RESET
ook.. well, it's actually reasonable, it's just the time for the
regulators to ramp up, I initially thought it was the time for the
chip to exit reset.

Let me be a bit more picky and ask if you have considered busy waiting
on such a small sleep interval by using udelay. Again, as this happens
at chip power on only, the impact on the system of mis-using
usleep_range() is negligible, but according to documentation:

SLEEPING FOR "A FEW" USECS ( < ~10us? ):
* Use udelay

- Why not usleep?
On slower systems, (embedded, OR perhaps a speed-
stepped PC!) the overhead of setting up the hrtimers
for usleep *may* not be worth it. Such an evaluation
will obviously depend on your specific situation, but
it is something to be aware of.

Up to you, really!

Thanks Jacopo. Will update in v6 to use udelay.