[PATCH v3 1/2] regulator: implement basic reference counter

[Simon Glass]

Hi Eugen,

On Fri, 31 Mar 2023 at 05:21, Eugen Hristev <eugen.hristev at collabora.com> wrote:
>
> Some devices share a regulator supply, when the first one will request
> regulator disable, the second device will have it's supply cut off before
> graciously shutting down. Hence there will be timeouts and other failed
> operations.
> Implement a reference counter mechanism similar with what is done in
> Linux, to keep track of enable and disable requests, and only disable the
> regulator when the last of the consumers has requested shutdown.
>
> Signed-off-by: Eugen Hristev <eugen.hristev at collabora.com>
> ---
> Changes in v3:
>  - add error return codes
> Changes in v2:
>  - add info in header regarding the function
>
>  drivers/power/regulator/regulator_common.c | 22 ++++++++++++++++++++++
>  drivers/power/regulator/regulator_common.h | 14 ++++++++++++++
>  2 files changed, 36 insertions(+)
>

Reviewed-by: Simon Glass <sjg at chromium.org>

with changes below

> diff --git a/drivers/power/regulator/regulator_common.c b/drivers/power/regulator/regulator_common.c
> index 93d8196b381e..484a4fc31ef7 100644
> --- a/drivers/power/regulator/regulator_common.c
> +++ b/drivers/power/regulator/regulator_common.c
> @@ -73,6 +73,23 @@ int regulator_common_set_enable(const struct udevice *dev,
>                 return 0;
>         }
>
> +       /* If previously enabled, increase count */
> +       if (enable && dev_pdata->enable_count > 0) {
> +               dev_pdata->enable_count++;
> +               return -EALREADY;
> +       }
> +
> +       if (!enable) {
> +               if (dev_pdata->enable_count > 1) {
> +                       /* If enabled multiple times, decrease count */
> +                       dev_pdata->enable_count--;
> +                       return -EBUSY;
> +               } else if (!dev_pdata->enable_count) {
> +                       /* If already disabled, do nothing */
> +                       return -EALREADY;
> +               }
> +       }
> +
>         ret = dm_gpio_set_value(&dev_pdata->gpio, enable);
>         if (ret) {
>                 pr_err("Can't set regulator : %s gpio to: %d\n", dev->name,
> @@ -87,5 +104,10 @@ int regulator_common_set_enable(const struct udevice *dev,
>         if (!enable && dev_pdata->off_on_delay_us)
>                 udelay(dev_pdata->off_on_delay_us);
>
> +       if (enable)
> +               dev_pdata->enable_count++;
> +       else
> +               dev_pdata->enable_count--;
> +
>         return 0;
>  }
> diff --git a/drivers/power/regulator/regulator_common.h b/drivers/power/regulator/regulator_common.h
> index c10492f01675..7a4df703ea1a 100644
> --- a/drivers/power/regulator/regulator_common.h
> +++ b/drivers/power/regulator/regulator_common.h
> @@ -13,6 +13,7 @@ struct regulator_common_plat {
>         struct gpio_desc gpio; /* GPIO for regulator enable control */
>         unsigned int startup_delay_us;
>         unsigned int off_on_delay_us;
> +       unsigned int enable_count;
>  };
>
>  int regulator_common_of_to_plat(struct udevice *dev,
> @@ -20,6 +21,19 @@ int regulator_common_of_to_plat(struct udevice *dev,
>                                 char *enable_gpio_name);
>  int regulator_common_get_enable(const struct udevice *dev,
>         struct regulator_common_plat *dev_pdata);
> +/*
> + * Enable or Disable a regulator
> + *
> + * This is a reentrant function and subsequent calls that enable will
> + * increase an internal counter, and disable calls will decrease the counter.
> + * The actual resource will be enabled when the counter gets to 1 coming from 0,
> + * and disabled when it reaches 0 coming from 1.
> + *
> + * @dev: regulator device
> + * @dev_pdata: Platform data
> + * @enable: bool indicating whether to enable or disable the regulator
> + * @return 0 if OK, -ve on error

Returns: -EBUSY if ..., -EALREADY if .......

You do need to clearly document this, since this is how people know
what is going on.

> + */
>  int regulator_common_set_enable(const struct udevice *dev,
>         struct regulator_common_plat *dev_pdata, bool enable);
>
> --
> 2.34.1
>

Regards,
Simon