[PATCH v2 1/7] power: regulator: expand basic reference counter onto all uclass

Simon Glass sjg at chromium.org
Thu Jul 20 21:42:52 CEST 2023


Hi Svyatoslav,

On Thu, 20 Jul 2023 at 06:38, Svyatoslav Ryhel <clamor95 at gmail.com> wrote:
>
> Commit is based on 4fcba5d ("regulator: implement basic reference
> counter") but expands the idea to all regulators instead of just
> fixed/gpio regulators.
>
> Signed-off-by: Svyatoslav Ryhel <clamor95 at gmail.com>
> ---
>  drivers/power/regulator/regulator-uclass.c | 41 ++++++++++++++++++++++
>  drivers/power/regulator/regulator_common.c | 22 ------------
>  drivers/power/regulator/regulator_common.h | 21 -----------
>  include/power/regulator.h                  |  2 ++
>  4 files changed, 43 insertions(+), 43 deletions(-)

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

nit below

>
> diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c
> index 3a6ba69f6d..fc7a4631b4 100644
> --- a/drivers/power/regulator/regulator-uclass.c
> +++ b/drivers/power/regulator/regulator-uclass.c
> @@ -159,6 +159,25 @@ int regulator_get_enable(struct udevice *dev)
>         return ops->get_enable(dev);
>  }
>
> +/*
> + * 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
> + * @enable: bool indicating whether to enable or disable the regulator
> + * @return:
> + * 0 on Success
> + * -EBUSY if the regulator cannot be disabled because it's requested by
> + *        another device
> + * -EALREADY if the regulator has already been enabled or has already been
> + *        disabled
> + * -EACCES if there is no possibility to enable/disable the regulator
> + * -ve on different error situation
> + */
>  int regulator_set_enable(struct udevice *dev, bool enable)
>  {
>         const struct dm_regulator_ops *ops = dev_get_driver_ops(dev);
> @@ -172,6 +191,23 @@ int regulator_set_enable(struct udevice *dev, bool enable)
>         if (!enable && uc_pdata->always_on)
>                 return -EACCES;
>
> +       /* If previously enabled, increase count */
> +       if (enable && uc_pdata->enable_count > 0) {
> +               uc_pdata->enable_count++;
> +               return -EALREADY;
> +       }
> +
> +       if (!enable) {
> +               if (uc_pdata->enable_count > 1) {
> +                       /* If enabled multiple times, decrease count */
> +                       uc_pdata->enable_count--;
> +                       return -EBUSY;
> +               } else if (!uc_pdata->enable_count) {
> +                       /* If already disabled, do nothing */
> +                       return -EALREADY;
> +               }
> +       }
> +
>         if (uc_pdata->ramp_delay)
>                 old_enable = regulator_get_enable(dev);
>
> @@ -187,6 +223,11 @@ int regulator_set_enable(struct udevice *dev, bool enable)
>                 }
>         }
>
> +       if (enable)
> +               uc_pdata->enable_count++;
> +       else
> +               uc_pdata->enable_count--;
> +
>         return ret;
>  }
>
> diff --git a/drivers/power/regulator/regulator_common.c b/drivers/power/regulator/regulator_common.c
> index e26f5ebec3..d88bc6f6de 100644
> --- a/drivers/power/regulator/regulator_common.c
> +++ b/drivers/power/regulator/regulator_common.c
> @@ -72,23 +72,6 @@ int regulator_common_set_enable(const struct udevice *dev,
>                 return 0;
>         }
>
> -       /* If previously enabled, increase count */
> -       if (enable && plat->enable_count > 0) {
> -               plat->enable_count++;
> -               return -EALREADY;
> -       }
> -
> -       if (!enable) {
> -               if (plat->enable_count > 1) {
> -                       /* If enabled multiple times, decrease count */
> -                       plat->enable_count--;
> -                       return -EBUSY;
> -               } else if (!plat->enable_count) {
> -                       /* If already disabled, do nothing */
> -                       return -EALREADY;
> -               }
> -       }
> -
>         ret = dm_gpio_set_value(&plat->gpio, enable);
>         if (ret) {
>                 pr_err("Can't set regulator : %s gpio to: %d\n", dev->name,
> @@ -103,10 +86,5 @@ int regulator_common_set_enable(const struct udevice *dev,
>         if (!enable && plat->off_on_delay_us)
>                 udelay(plat->off_on_delay_us);
>
> -       if (enable)
> -               plat->enable_count++;
> -       else
> -               plat->enable_count--;
> -
>         return 0;
>  }
> diff --git a/drivers/power/regulator/regulator_common.h b/drivers/power/regulator/regulator_common.h
> index d4962899d8..15f1fa4c93 100644
> --- a/drivers/power/regulator/regulator_common.h
> +++ b/drivers/power/regulator/regulator_common.h
> @@ -13,7 +13,6 @@ 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,
> @@ -21,26 +20,6 @@ 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 *plat);
> -/*
> - * 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
> - * @plat: Platform data
> - * @enable: bool indicating whether to enable or disable the regulator
> - * @return:
> - * 0 on Success
> - * -EBUSY if the regulator cannot be disabled because it's requested by
> - *        another device
> - * -EALREADY if the regulator has already been enabled or has already been
> - *        disabled
> - * -EACCES if there is no possibility to enable/disable the regulator
> - * -ve on different error situation
> - */
>  int regulator_common_set_enable(const struct udevice *dev,
>         struct regulator_common_plat *plat, bool enable);
>
> diff --git a/include/power/regulator.h b/include/power/regulator.h
> index ff1bfc2435..727776a8cf 100644
> --- a/include/power/regulator.h
> +++ b/include/power/regulator.h
> @@ -158,6 +158,7 @@ enum regulator_flag {
>   * @name**     - fdt regulator name - should be taken from the device tree
>   * ctrl_reg:   - Control register offset used to enable/disable regulator
>   * volt_reg:   - register offset for writing voltage vsel values
> + * enable_count - counter of enable calls for this regulator
>   *
>   * Note:
>   * *  - set automatically on device probe by the uclass's '.pre_probe' method.
> @@ -184,6 +185,7 @@ struct dm_regulator_uclass_plat {
>         u8 volt_reg;
>         bool suspend_on;
>         u32 suspend_uV;
> +       u32 enable_count;

Please add a comment for this

>  };
>
>  /* Regulator device operations */
> --
> 2.39.2
>

Regards,
Simon


More information about the U-Boot mailing list