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

[Jonas Karlman]

Hi Eugen,

On 2023-03-31 11:15, Eugen Hristev 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>
> Reviewed-by: Simon Glass <sjg at chromium.org>
> ---
> Changes in v4:
>  - add documentation for error codes
> 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 | 21 +++++++++++++++++++++
>  2 files changed, 43 insertions(+)
> 
> 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..0faab447d099 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,26 @@ 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 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

With this new return value added this has potential to break IO voltage
switching for mmc UHS support. Main pattern for IO voltage switch seem
to be: disable regulator, set voltage, enable regulator.

regulator_set_enable_if_allowed should probably be updated to return
success on -EALREADY, but that also has the potential to hide regulators
enabled at boot not being disabled unless initial enable_count reflects
the initial regulator state.

Guessing initial enable_count will match for regulator-boot-on regulators
in Rockchip and few other platforms/boards because they the call
regulators_enable_boot_on, does not look like all platform will have
initial enable_count match regulator initial state.

Regards,
Jonas

> + * -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 *dev_pdata, bool enable);
>