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

Hi Svyatoslav,

On Thu, 20 Jul 2023 at 06:38, Svyatoslav Ryhel clamor95@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@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@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