[PATCH v5 1/3] regulator: implement basic reference counter

[Tim Harvey]

On Wed, Apr 19, 2023 at 6:45 AM 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>
> Reviewed-by: Simon Glass <sjg at chromium.org>
> ---
> Changes in v5:
>  - none
> 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;
> +               }
> +       }
> +

Eugen,

Thank you for working on this series!

I wonder if instead of returning a failure you should print an error
here but return 0 in order to not break unbalanced regulator calls
(like what is done in clk_disable()). I see that you have another
patch in this series which handles htis for
regulator_set_enable_if_allowed() but that doesn't cover drivers that
call regulator_common_set_enable() directly such as
drivers/power/regulator/fixed.c and
drivers/power/regulator/gpio-regulator.c.

I know there is an unbalanced call currently on imx8mm that this patch
causes a failure where it previously did not:
u-boot=> usb start && usb tree
starting USB...
Bus usb at 32e40000: Bus usb at 32e50000: Error enabling VBUS supply (ret=-114)
probe failed, error -114

Best Regards,

Tim