Re: [PATCH v2 03/14] mfd: pm8008: deassert reset on probe

From: Johan Hovold
Date: Thu May 30 2024 - 04:51:41 EST


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