On Tue 02 Jun 03:09 PDT 2020, Sumit Semwal wrote: > Some regulators might need to verify that they have indeed been enabled > after the enable() call is made and enable_time delay has passed. > > This is implemented by repeatedly checking is_enabled() upto > poll_enabled_time, waiting for the already calculated enable delay in > each iteration. > > Signed-off-by: Sumit Semwal <sumit.semwal@xxxxxxxxxx> > -- > v2: Address review comments, including swapping enable_time and poll_enabled_time. > --- > drivers/regulator/core.c | 58 +++++++++++++++++++++++++++++++- > include/linux/regulator/driver.h | 5 +++ > 2 files changed, 62 insertions(+), 1 deletion(-) > > diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c > index 7486f6e4e613..d9ab888da95f 100644 > --- a/drivers/regulator/core.c > +++ b/drivers/regulator/core.c > @@ -2347,6 +2347,32 @@ static void _regulator_enable_delay(unsigned int delay) > udelay(us); > } > > +/* _regulator_check_status_enabled Please make all your kerneldoc follow: https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#function-documentation > + * > + * returns: > + * 1 if status shows regulator is in enabled state > + * 0 if not enabled state > + * else, error value as received from ops->get_status() > + */ > +static inline int _regulator_check_status_enabled(struct regulator_dev *rdev) > +{ > + int ret = rdev->desc->ops->get_status(rdev); > + > + if (ret < 0) { > + rdev_info(rdev, "get_status returned error: %d\n", ret); > + return ret; > + } > + > + switch (ret) { > + case REGULATOR_STATUS_OFF: > + case REGULATOR_STATUS_ERROR: > + case REGULATOR_STATUS_UNDEFINED: > + return 0; > + default: > + return 1; > + } > +} > + > static int _regulator_do_enable(struct regulator_dev *rdev) > { > int ret, delay; > @@ -2407,7 +2433,37 @@ static int _regulator_do_enable(struct regulator_dev *rdev) > * together. */ > trace_regulator_enable_delay(rdev_get_name(rdev)); > > - _regulator_enable_delay(delay); > + /* If poll_enabled_time is set, poll upto the delay calculated > + * above, delaying poll_enabled_time uS to check if the regulator > + * actually got enabled. > + * If the regulator isn't enabled after enable_delay has > + * expired, return -ETIMEDOUT. > + */ > + if (rdev->desc->poll_enabled_time) { > + unsigned int time_remaining = delay; > + > + while (time_remaining > 0) { > + _regulator_enable_delay(rdev->desc->poll_enabled_time); > + > + if (rdev->desc->ops->get_status) { > + ret = _regulator_check_status_enabled(rdev); > + if (ret < 0) > + return ret; > + else if (ret) > + break; > + } else if (rdev->desc->ops->is_enabled(rdev)) > + break; > + > + time_remaining -= rdev->desc->poll_enabled_time; > + } > + > + if (time_remaining <= 0) { > + rdev_err(rdev, "Enabled check failed.\n"); > + return -ETIMEDOUT; > + } > + } else { > + _regulator_enable_delay(delay); > + } > > trace_regulator_enable_complete(rdev_get_name(rdev)); > > diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h > index 29d920516e0b..bb50e943010f 100644 > --- a/include/linux/regulator/driver.h > +++ b/include/linux/regulator/driver.h > @@ -322,6 +322,9 @@ enum regulator_type { > * @enable_time: Time taken for initial enable of regulator (in uS). > * @off_on_delay: guard time (in uS), before re-enabling a regulator > * > + * @poll_enabled_time: Maximum time (in uS) to poll if the regulator is > + * actually enabled, after enable() call I read this as "how long should we stay in the poll loop". I think it would be better describes as something like "The polling interval to use while checking that the regulator was actually enabled". Regards, Bjorn > + * > * @of_map_mode: Maps a hardware mode defined in a DeviceTree to a standard mode > */ > struct regulator_desc { > @@ -389,6 +392,8 @@ struct regulator_desc { > > unsigned int off_on_delay; > > + unsigned int poll_enabled_time; > + > unsigned int (*of_map_mode)(unsigned int mode); > }; > > -- > 2.26.2 >