Re: [PATCH v2 1/5] power: regulator: Only run autoset once for each regulator

[Kever Yang]

On 2023/8/22 06:30, Jonas Karlman wrote:

With the commit 4fcba5d556b4 ("regulator: implement basic reference
counter"), keeping regulator enablement in balance become more important.
Calling regulator_autoset multiple times on a fixed regulator increase
the enable count for each call, resulting in an unbalanced enable count.

Introduce a AUTOSET_DONE flag and use it to mark that autoset has run
for the regulator. Return -EALREADY on any subsequent call to autoset.

This fixes so that the enable count is only ever increased by one per
regulator for autoset.

Fixes: 4fcba5d556b4 ("regulator: implement basic reference counter")
Signed-off-by: Jonas Karlman 

Reviewed-by: Kever Yang 

Thanks,
- Kever

---
Cc: Svyatoslav Ryhel 
---
v2:
- No change

  drivers/power/regulator/regulator-uclass.c | 18 ++
  include/power/regulator.h  |  1 +
  2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/power/regulator/regulator-uclass.c 
b/drivers/power/regulator/regulator-uclass.c
index 3a6ba69f6d5f..77d101f262e2 100644
--- a/drivers/power/regulator/regulator-uclass.c
+++ b/drivers/power/regulator/regulator-uclass.c
@@ -293,6 +293,9 @@ int regulator_autoset(struct udevice *dev)
  
  	uc_pdata = dev_get_uclass_plat(dev);
  
+	if (uc_pdata->flags & REGULATOR_FLAG_AUTOSET_DONE)

+   return -EALREADY;
+
ret = regulator_set_suspend_enable(dev, uc_pdata->suspend_on);
if (ret == -ENOSYS)
ret = 0;
@@ -306,11 +309,15 @@ int regulator_autoset(struct udevice *dev)
return ret;
}
  
-	if (!uc_pdata->always_on && !uc_pdata->boot_on)

-   return -EMEDIUMTYPE;
+   if (!uc_pdata->always_on && !uc_pdata->boot_on) {
+   ret = -EMEDIUMTYPE;
+   goto out;
+   }
  
-	if (uc_pdata->type == REGULATOR_TYPE_FIXED)

-   return regulator_set_enable(dev, true);
+   if (uc_pdata->type == REGULATOR_TYPE_FIXED) {
+   ret = regulator_set_enable(dev, true);
+   goto out;
+   }
  
  	if (uc_pdata->flags & REGULATOR_FLAG_AUTOSET_UV)

ret = regulator_set_value(dev, uc_pdata->min_uV);
@@ -322,6 +329,9 @@ int regulator_autoset(struct udevice *dev)
if (!ret)
ret = regulator_set_enable(dev, true);
  
+out:

+   uc_pdata->flags |= REGULATOR_FLAG_AUTOSET_DONE;
+
return ret;
  }
  
diff --git a/include/power/regulator.h b/include/power/regulator.h

index ff1bfc2435ae..200652cb3d7a 100644
--- a/include/power/regulator.h
+++ b/include/power/regulator.h
@@ -134,6 +134,7 @@ struct dm_regulator_mode {
  enum regulator_flag {
REGULATOR_FLAG_AUTOSET_UV   = 1 << 0,
REGULATOR_FLAG_AUTOSET_UA   = 1 << 1,
+   REGULATOR_FLAG_AUTOSET_DONE = 1 << 2,
  };
  
  /**

[PATCH v2 1/5] power: regulator: Only run autoset once for each regulator

[Jonas Karlman]

With the commit 4fcba5d556b4 ("regulator: implement basic reference
counter"), keeping regulator enablement in balance become more important.
Calling regulator_autoset multiple times on a fixed regulator increase
the enable count for each call, resulting in an unbalanced enable count.

Introduce a AUTOSET_DONE flag and use it to mark that autoset has run
for the regulator. Return -EALREADY on any subsequent call to autoset.

This fixes so that the enable count is only ever increased by one per
regulator for autoset.

Fixes: 4fcba5d556b4 ("regulator: implement basic reference counter")
Signed-off-by: Jonas Karlman 
---
Cc: Svyatoslav Ryhel 
---
v2:
- No change

 drivers/power/regulator/regulator-uclass.c | 18 ++
 include/power/regulator.h  |  1 +
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/power/regulator/regulator-uclass.c 
b/drivers/power/regulator/regulator-uclass.c
index 3a6ba69f6d5f..77d101f262e2 100644
--- a/drivers/power/regulator/regulator-uclass.c
+++ b/drivers/power/regulator/regulator-uclass.c
@@ -293,6 +293,9 @@ int regulator_autoset(struct udevice *dev)
 
uc_pdata = dev_get_uclass_plat(dev);
 
+   if (uc_pdata->flags & REGULATOR_FLAG_AUTOSET_DONE)
+   return -EALREADY;
+
ret = regulator_set_suspend_enable(dev, uc_pdata->suspend_on);
if (ret == -ENOSYS)
ret = 0;
@@ -306,11 +309,15 @@ int regulator_autoset(struct udevice *dev)
return ret;
}
 
-   if (!uc_pdata->always_on && !uc_pdata->boot_on)
-   return -EMEDIUMTYPE;
+   if (!uc_pdata->always_on && !uc_pdata->boot_on) {
+   ret = -EMEDIUMTYPE;
+   goto out;
+   }
 
-   if (uc_pdata->type == REGULATOR_TYPE_FIXED)
-   return regulator_set_enable(dev, true);
+   if (uc_pdata->type == REGULATOR_TYPE_FIXED) {
+   ret = regulator_set_enable(dev, true);
+   goto out;
+   }
 
if (uc_pdata->flags & REGULATOR_FLAG_AUTOSET_UV)
ret = regulator_set_value(dev, uc_pdata->min_uV);
@@ -322,6 +329,9 @@ int regulator_autoset(struct udevice *dev)
if (!ret)
ret = regulator_set_enable(dev, true);
 
+out:
+   uc_pdata->flags |= REGULATOR_FLAG_AUTOSET_DONE;
+
return ret;
 }
 
diff --git a/include/power/regulator.h b/include/power/regulator.h
index ff1bfc2435ae..200652cb3d7a 100644
--- a/include/power/regulator.h
+++ b/include/power/regulator.h
@@ -134,6 +134,7 @@ struct dm_regulator_mode {
 enum regulator_flag {
REGULATOR_FLAG_AUTOSET_UV   = 1 << 0,
REGULATOR_FLAG_AUTOSET_UA   = 1 << 1,
+   REGULATOR_FLAG_AUTOSET_DONE = 1 << 2,
 };
 
 /**
-- 
2.41.0