On Thu, May 30, 2024 at 11:34:55AM +0300, Andy Shevchenko wrote: > On Thu, May 30, 2024 at 11:08 AM Johan Hovold <johan@xxxxxxxxxx> wrote: > > On Wed, May 29, 2024 at 10:45:40PM +0300, Andy Shevchenko wrote: > > > On Wed, May 29, 2024 at 7:30 PM Johan Hovold <johan+linaro@xxxxxxxxxx> wrote: > > > > > > > > Request and deassert any (optional) reset gpio during probe in case it > > > > has been left asserted by the boot firmware. > > > > > > > > Note the reset line is not asserted to avoid reverting to the default > > > > I2C address in case the firmware has configured an alternate address. > > ... > > > > > + /* > > > > + * The PMIC does not appear to require a post-reset delay, but wait > > > > + * for a millisecond for now anyway. > > > > + */ > > > > > > > + usleep_range(1000, 2000); > > > > > > fsleep() ? > > > > No, I'd only use fsleep() when the argument is variable. > > Okay, this is basically the same issue as with use of dev_err_probe() > with known errors. fsleep() hides the choice between let's say > msleep() / usleep_range() / udelay() from the caller. This, in > particular, might allow shifting constraints if the timer core is > changed or becomes more granular. It's independent to the variable or > constant parameter(s). Whatever, I'm not going to insist. I prefer that developers are aware of what they are doing and understand the difference between, say, usleep_range() and udelay(), instead of hiding things away in obscure helper functions. Johan