[U-Boot] [PATCH] regulator: Allow autosetting fixed regulators
Fixed regulators don't have a set_value method. Therefore, regulator_set_value will return -ENOSYS when called from regulator_autoset.
Accepting this return value allows autosetting fixed regulators.
Signed-off-by: Sven Schwermer sven@svenschwermer.de Cc: Jaehoon Chung jh80.chung@samsung.com Cc: Peng Fan peng.fan@nxp.com --- drivers/power/regulator/regulator-uclass.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c index 9118b8eb39..0b99c262ac 100644 --- a/drivers/power/regulator/regulator-uclass.c +++ b/drivers/power/regulator/regulator-uclass.c @@ -243,7 +243,7 @@ int regulator_autoset(struct udevice *dev) if (!ret && (uc_pdata->flags & REGULATOR_FLAG_AUTOSET_UA)) ret = regulator_set_current(dev, uc_pdata->min_uA);
- if (!ret) + if (!ret || ret == -ENOSYS) ret = regulator_set_enable(dev, true);
return ret;
Hi Sven,
Subject: [PATCH] regulator: Allow autosetting fixed regulators
Fixed regulators don't have a set_value method. Therefore, regulator_set_value will return -ENOSYS when called from regulator_autoset.
Accepting this return value allows autosetting fixed regulators.
Signed-off-by: Sven Schwermer sven@svenschwermer.de Cc: Jaehoon Chung jh80.chung@samsung.com Cc: Peng Fan peng.fan@nxp.com
drivers/power/regulator/regulator-uclass.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c index 9118b8eb39..0b99c262ac 100644 --- a/drivers/power/regulator/regulator-uclass.c +++ b/drivers/power/regulator/regulator-uclass.c @@ -243,7 +243,7 @@ int regulator_autoset(struct udevice *dev) if (!ret && (uc_pdata->flags & REGULATOR_FLAG_AUTOSET_UA)) ret = regulator_set_current(dev, uc_pdata->min_uA);
- if (!ret)
- if (!ret || ret == -ENOSYS)
According to code: if (uc_pdata->flags & REGULATOR_FLAG_AUTOSET_UV) ret = regulator_set_value(dev, uc_pdata->min_uV); if (!ret && (uc_pdata->flags & REGULATOR_FLAG_AUTOSET_UA)) ret = regulator_set_current(dev, uc_pdata->min_uA);
So you get -ENOSYS from the upper code?
Regards, Peng.
ret = regulator_set_enable(dev, true);return ret;
2.17.1
Hi Peng,
According to code: if (uc_pdata->flags & REGULATOR_FLAG_AUTOSET_UV) ret = regulator_set_value(dev, uc_pdata->min_uV); if (!ret && (uc_pdata->flags & REGULATOR_FLAG_AUTOSET_UA)) ret = regulator_set_current(dev, uc_pdata->min_uA);
So you get -ENOSYS from the upper code?
Yes, for fixed regulators, the following if clause evaluates to true in regulator_pre_probe:
/* Those values are optional (-ENODATA if unset) */ if ((uc_pdata->min_uV != -ENODATA) && (uc_pdata->max_uV != -ENODATA) && (uc_pdata->min_uV == uc_pdata->max_uV)) uc_pdata->flags |= REGULATOR_FLAG_AUTOSET_UV;
However, in regulator_set_value, there is this section:
if (!ops || !ops->set_value) return -ENOSYS;
So for fixed regulators, which don’t have a set_value, we’ll always get a -ENOSYS.
Sven
Subject: Re: [PATCH] regulator: Allow autosetting fixed regulators
Hi Peng,
According to code: if (uc_pdata->flags & REGULATOR_FLAG_AUTOSET_UV) ret = regulator_set_value(dev, uc_pdata->min_uV); if (!ret && (uc_pdata->flags & REGULATOR_FLAG_AUTOSET_UA)) ret = regulator_set_current(dev, uc_pdata->min_uA);
So you get -ENOSYS from the upper code?
Yes, for fixed regulators, the following if clause evaluates to true in regulator_pre_probe:
/* Those values are optional (-ENODATA if unset) */ if ((uc_pdata->min_uV != -ENODATA) && (uc_pdata->max_uV != -ENODATA) && (uc_pdata->min_uV == uc_pdata->max_uV)) uc_pdata->flags |= REGULATOR_FLAG_AUTOSET_UV;
For fixed regulator, will min_uV/max_uV be set to value other than -ENODATA? I think no.
Regards, Peng.
However, in regulator_set_value, there is this section:
if (!ops || !ops->set_value) return -ENOSYS;
So for fixed regulators, which don’t have a set_value, we’ll always get a -ENOSYS.
Sven
For fixed regulator, will min_uV/max_uV be set to value other than -ENODATA? I think no.
I see this commonly done (see Linux device trees as well as the u-boot device trees) and the kernel documentation seems to encourage it: https://www.kernel.org/doc/Documentation/devicetree/bindings/regulator/fixed... https://www.kernel.org/doc/Documentation/devicetree/bindings/regulator/fixed-regulator.txt
Sven
Subject: [PATCH] regulator: Allow autosetting fixed regulators
Fixed regulators don't have a set_value method. Therefore, regulator_set_value will return -ENOSYS when called from regulator_autoset.
Accepting this return value allows autosetting fixed regulators.
Signed-off-by: Sven Schwermer sven@svenschwermer.de Cc: Jaehoon Chung jh80.chung@samsung.com Cc: Peng Fan peng.fan@nxp.com
drivers/power/regulator/regulator-uclass.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c index 9118b8eb39..0b99c262ac 100644 --- a/drivers/power/regulator/regulator-uclass.c +++ b/drivers/power/regulator/regulator-uclass.c @@ -243,7 +243,7 @@ int regulator_autoset(struct udevice *dev) if (!ret && (uc_pdata->flags & REGULATOR_FLAG_AUTOSET_UA)) ret = regulator_set_current(dev, uc_pdata->min_uA);
- if (!ret)
- if (!ret || ret == -ENOSYS) ret = regulator_set_enable(dev, true);
How about the following patch? not tested
diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c index 9118b8eb39..76be95bcd1 100644 --- a/drivers/power/regulator/regulator-uclass.c +++ b/drivers/power/regulator/regulator-uclass.c @@ -238,6 +238,9 @@ int regulator_autoset(struct udevice *dev) if (!uc_pdata->always_on && !uc_pdata->boot_on) return -EMEDIUMTYPE;
+ if (uc_pdata->type == REGULATOR_TYPE_FIXED) + return regulator_set_enable(dev, true); + if (uc_pdata->flags & REGULATOR_FLAG_AUTOSET_UV) ret = regulator_set_value(dev, uc_pdata->min_uV); if (!ret && (uc_pdata->flags & REGULATOR_FLAG_AUTOSET_UA))
Regards, Peng.
return ret;
2.17.1
How about the following patch? not tested
diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c index 9118b8eb39..76be95bcd1 100644 --- a/drivers/power/regulator/regulator-uclass.c +++ b/drivers/power/regulator/regulator-uclass.c @@ -238,6 +238,9 @@ int regulator_autoset(struct udevice *dev) if (!uc_pdata->always_on && !uc_pdata->boot_on) return -EMEDIUMTYPE;
if (uc_pdata->type == REGULATOR_TYPE_FIXED)return regulator_set_enable(dev, true);if (uc_pdata->flags & REGULATOR_FLAG_AUTOSET_UV) ret = regulator_set_value(dev, uc_pdata->min_uV); if (!ret && (uc_pdata->flags & REGULATOR_FLAG_AUTOSET_UA))
That will probably work as well. It does, however, feel a little like a hack. What if there are other regulator types that require a similar workaround?
Sven
-----Original Message----- From: Sven Schwermer [mailto:sven@svenschwermer.de] Sent: 2019年5月31日 18:44 To: Peng Fan peng.fan@nxp.com Cc: u-boot@lists.denx.de; Jaehoon Chung jh80.chung@samsung.com Subject: Re: [PATCH] regulator: Allow autosetting fixed regulators
How about the following patch? not tested
diff --git a/drivers/power/regulator/regulator-uclass.c
b/drivers/power/regulator/regulator-uclass.c
index 9118b8eb39..76be95bcd1 100644 --- a/drivers/power/regulator/regulator-uclass.c +++ b/drivers/power/regulator/regulator-uclass.c @@ -238,6 +238,9 @@ int regulator_autoset(struct udevice *dev) if (!uc_pdata->always_on && !uc_pdata->boot_on) return -EMEDIUMTYPE;
if (uc_pdata->type == REGULATOR_TYPE_FIXED)return regulator_set_enable(dev, true);if (uc_pdata->flags & REGULATOR_FLAG_AUTOSET_UV) ret = regulator_set_value(dev, uc_pdata->min_uV); if (!ret && (uc_pdata->flags & REGULATOR_FLAG_AUTOSET_UA))That will probably work as well. It does, however, feel a little like a hack. What if there are other regulator types that require a similar workaround?
When ret is -ENOSYS, it might be a fixed regulator. So directly use FIXED regulator check make more sense to me.
Regards, Peng.
Sven
Fixed regulators don't have a set_value method. Therefore, trying to set their value will always return -ENOSYS.
Signed-off-by: Sven Schwermer sven@svenschwermer.de Cc: Jaehoon Chung jh80.chung@samsung.com Cc: Peng Fan peng.fan@nxp.com --- drivers/power/regulator/regulator-uclass.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c index 9118b8eb39..76be95bcd1 100644 --- a/drivers/power/regulator/regulator-uclass.c +++ b/drivers/power/regulator/regulator-uclass.c @@ -238,6 +238,9 @@ int regulator_autoset(struct udevice *dev) if (!uc_pdata->always_on && !uc_pdata->boot_on) return -EMEDIUMTYPE;
+ if (uc_pdata->type == REGULATOR_TYPE_FIXED) + return regulator_set_enable(dev, true); + if (uc_pdata->flags & REGULATOR_FLAG_AUTOSET_UV) ret = regulator_set_value(dev, uc_pdata->min_uV); if (!ret && (uc_pdata->flags & REGULATOR_FLAG_AUTOSET_UA))
On Wed, 12 Jun 2019 at 07:32, Sven Schwermer sven@svenschwermer.de wrote:
Fixed regulators don't have a set_value method. Therefore, trying to set their value will always return -ENOSYS.
Signed-off-by: Sven Schwermer sven@svenschwermer.de Cc: Jaehoon Chung jh80.chung@samsung.com Cc: Peng Fan peng.fan@nxp.com
drivers/power/regulator/regulator-uclass.c | 3 +++ 1 file changed, 3 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
It should be possible to create a simple test for this in test/dm/regulator.c
On Wed, Jun 12, 2019 at 08:32:38AM +0200, Sven Schwermer wrote:
Fixed regulators don't have a set_value method. Therefore, trying to set their value will always return -ENOSYS.
Signed-off-by: Sven Schwermer sven@svenschwermer.de Cc: Jaehoon Chung jh80.chung@samsung.com Cc: Peng Fan peng.fan@nxp.com Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
participants (4)
-
Peng Fan -
Simon Glass -
Sven Schwermer -
Tom Rini