On Wed, Apr 19, 2023 at 6:45 AM Eugen Hristev
<eugen.hris...@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.hris...@collabora.com>
> Reviewed-by: Simon Glass <s...@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@32e40000: Bus usb@32e50000: Error enabling VBUS supply (ret=-114)
probe failed, error -114
Best Regards,
Tim
