[U-Boot] [PATCH 1/3] power: regulator: Introduce regulator_set_value_force function

[U-Boot] [PATCH 1/3] power: regulator: Introduce regulator_set_value_force function

[Keerthy]

In case we want to force a particular value on a regulator
irrespective of the min/max constraints for testing purposes
one can call regulator_set_value_force function.

Signed-off-by: Keerthy <j-keerthy@ti.com>
---
 cmd/regulator.c                            |  5 ++++-
 drivers/power/regulator/regulator-uclass.c | 14 ++++++++++++++
 include/power/regulator.h                  | 10 ++++++++++
 3 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/cmd/regulator.c b/cmd/regulator.c
index bfea6e0..2ef5bc9 100644
--- a/cmd/regulator.c
+++ b/cmd/regulator.c
@@ -292,7 +292,10 @@ static int do_value(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		return CMD_RET_FAILURE;
 	}
 
-	ret = regulator_set_value(dev, value);
+	if (!force)
+		ret = regulator_set_value(dev, value);
+	else
+		ret = regulator_set_value_force(dev, value);
 	if (ret) {
 		printf("Regulator: %s - can't set the Voltage!\n",
 		       uc_pdata->name);
diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c
index 4434e36..d644009 100644
--- a/drivers/power/regulator/regulator-uclass.c
+++ b/drivers/power/regulator/regulator-uclass.c
@@ -48,6 +48,20 @@ int regulator_set_value(struct udevice *dev, int uV)
 	return ops->set_value(dev, uV);
 }
 
+/*
+ * To be called with at most caution as there is no check
+ * before setting the actual voltage value.
+ */
+int regulator_set_value_force(struct udevice *dev, int uV)
+{
+	const struct dm_regulator_ops *ops = dev_get_driver_ops(dev);
+
+	if (!ops || !ops->set_value)
+		return -ENOSYS;
+
+	return ops->set_value(dev, uV);
+}
+
 int regulator_get_current(struct udevice *dev)
 {
 	const struct dm_regulator_ops *ops = dev_get_driver_ops(dev);
diff --git a/include/power/regulator.h b/include/power/regulator.h
index f47ab67..1a8e575 100644
--- a/include/power/regulator.h
+++ b/include/power/regulator.h
@@ -261,6 +261,16 @@ int regulator_get_value(struct udevice *dev);
 int regulator_set_value(struct udevice *dev, int uV);
 
 /**
+ * regulator_set_value_force: set the microvoltage value of a given regulator
+ *			      without any min-,max condition check
+ *
+ * @dev    - pointer to the regulator device
+ * @uV     - the output value to set [micro Volts]
+ * @return - 0 on success or -errno val if fails
+ */
+int regulator_set_value_force(struct udevice *dev, int uV);
+
+/**
  * regulator_get_current: get microampere value of a given regulator
  *
  * @dev    - pointer to the regulator device
-- 
1.9.1

[U-Boot] [PATCH 2/3] power: regulator: Add limits checking while setting voltage

[Keerthy]

Currently the specific set ops functions are directly
called without any check for voltage limits for a regulator.
Check for them and proceed.

Signed-off-by: Keerthy <j-keerthy@ti.com>
---
 drivers/power/regulator/regulator-uclass.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c
index d644009..34087c8 100644
--- a/drivers/power/regulator/regulator-uclass.c
+++ b/drivers/power/regulator/regulator-uclass.c
@@ -41,6 +41,11 @@ int regulator_get_value(struct udevice *dev)
 int regulator_set_value(struct udevice *dev, int uV)
 {
 	const struct dm_regulator_ops *ops = dev_get_driver_ops(dev);
+	struct dm_regulator_uclass_platdata *uc_pdata;
+
+	uc_pdata = dev_get_uclass_platdata(dev);
+	if (uV < uc_pdata->min_uV || uV > uc_pdata->max_uV)
+		return -EINVAL;
 
 	if (!ops || !ops->set_value)
 		return -ENOSYS;
-- 
1.9.1

[U-Boot] [PATCH 3/3] power: regulator: Add limits checking while setting current

[Keerthy]

Currently the specific set ops functions are directly
called without any check for min/max current limits for a regulator.
Check for them and proceed.

Signed-off-by: Keerthy <j-keerthy@ti.com>
---
 drivers/power/regulator/regulator-uclass.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c
index 34087c8..4c4bd29 100644
--- a/drivers/power/regulator/regulator-uclass.c
+++ b/drivers/power/regulator/regulator-uclass.c
@@ -80,6 +80,11 @@ int regulator_get_current(struct udevice *dev)
 int regulator_set_current(struct udevice *dev, int uA)
 {
 	const struct dm_regulator_ops *ops = dev_get_driver_ops(dev);
+	struct dm_regulator_uclass_platdata *uc_pdata;
+
+	uc_pdata = dev_get_uclass_platdata(dev);
+	if (uA < uc_pdata->min_uA || uA > uc_pdata->max_uA)
+		return -EINVAL;
 
 	if (!ops || !ops->set_current)
 		return -ENOSYS;
-- 
1.9.1

[U-Boot] [PATCH 1/3] power: regulator: Introduce regulator_set_value_force function

[Simon Glass]

On 26 October 2016 at 01:12, Keerthy <j-keerthy@ti.com> wrote:
> In case we want to force a particular value on a regulator
> irrespective of the min/max constraints for testing purposes
> one can call regulator_set_value_force function.
>
> Signed-off-by: Keerthy <j-keerthy@ti.com>
> ---
>  cmd/regulator.c                            |  5 ++++-
>  drivers/power/regulator/regulator-uclass.c | 14 ++++++++++++++
>  include/power/regulator.h                  | 10 ++++++++++
>  3 files changed, 28 insertions(+), 1 deletion(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 2/3] power: regulator: Add limits checking while setting voltage

[Simon Glass]

On 26 October 2016 at 01:12, Keerthy <j-keerthy@ti.com> wrote:
> Currently the specific set ops functions are directly
> called without any check for voltage limits for a regulator.
> Check for them and proceed.
>
> Signed-off-by: Keerthy <j-keerthy@ti.com>
> ---
>  drivers/power/regulator/regulator-uclass.c | 5 +++++
>  1 file changed, 5 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 3/3] power: regulator: Add limits checking while setting current

[Simon Glass]

Hi Keerthy,

On 26 October 2016 at 01:12, Keerthy <j-keerthy@ti.com> wrote:
> Currently the specific set ops functions are directly
> called without any check for min/max current limits for a regulator.
> Check for them and proceed.
>
> Signed-off-by: Keerthy <j-keerthy@ti.com>
> ---
>  drivers/power/regulator/regulator-uclass.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c
> index 34087c8..4c4bd29 100644
> --- a/drivers/power/regulator/regulator-uclass.c
> +++ b/drivers/power/regulator/regulator-uclass.c
> @@ -80,6 +80,11 @@ int regulator_get_current(struct udevice *dev)
>  int regulator_set_current(struct udevice *dev, int uA)
>  {
>         const struct dm_regulator_ops *ops = dev_get_driver_ops(dev);
> +       struct dm_regulator_uclass_platdata *uc_pdata;
> +
> +       uc_pdata = dev_get_uclass_platdata(dev);
> +       if (uA < uc_pdata->min_uA || uA > uc_pdata->max_uA)
> +               return -EINVAL;

Do all drivers have these values set?

>
>         if (!ops || !ops->set_current)
>                 return -ENOSYS;
> --
> 1.9.1
>

Regards,
Simon

[U-Boot] [PATCH 3/3] power: regulator: Add limits checking while setting current

[Keerthy]

On Wednesday 26 October 2016 10:01 PM, Simon Glass wrote:
> Hi Keerthy,
>
> On 26 October 2016 at 01:12, Keerthy <j-keerthy@ti.com> wrote:
>> Currently the specific set ops functions are directly
>> called without any check for min/max current limits for a regulator.
>> Check for them and proceed.
>>
>> Signed-off-by: Keerthy <j-keerthy@ti.com>
>> ---
>>  drivers/power/regulator/regulator-uclass.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c
>> index 34087c8..4c4bd29 100644
>> --- a/drivers/power/regulator/regulator-uclass.c
>> +++ b/drivers/power/regulator/regulator-uclass.c
>> @@ -80,6 +80,11 @@ int regulator_get_current(struct udevice *dev)
>>  int regulator_set_current(struct udevice *dev, int uA)
>>  {
>>         const struct dm_regulator_ops *ops = dev_get_driver_ops(dev);
>> +       struct dm_regulator_uclass_platdata *uc_pdata;
>> +
>> +       uc_pdata = dev_get_uclass_platdata(dev);
>> +       if (uA < uc_pdata->min_uA || uA > uc_pdata->max_uA)
>> +               return -EINVAL;
>
> Do all drivers have these values set?
Simon,

Agree that not all drivers set this. But when someone calls set_current 
with some value there needs to be some boundary conditions for this 
right? Hence i made this patch.

- Keerthy

>
>>
>>         if (!ops || !ops->set_current)
>>                 return -ENOSYS;
>> --
>> 1.9.1
>>
>
> Regards,
> Simon
>

[U-Boot] [PATCH 3/3] power: regulator: Add limits checking while setting current

[Simon Glass]

Hi Keethy,

On 26 October 2016 at 20:20, Keerthy <j-keerthy@ti.com> wrote:
>
>
> On Wednesday 26 October 2016 10:01 PM, Simon Glass wrote:
>>
>> Hi Keerthy,
>>
>> On 26 October 2016 at 01:12, Keerthy <j-keerthy@ti.com> wrote:
>>>
>>> Currently the specific set ops functions are directly
>>> called without any check for min/max current limits for a regulator.
>>> Check for them and proceed.
>>>
>>> Signed-off-by: Keerthy <j-keerthy@ti.com>
>>> ---
>>>  drivers/power/regulator/regulator-uclass.c | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/power/regulator/regulator-uclass.c
>>> b/drivers/power/regulator/regulator-uclass.c
>>> index 34087c8..4c4bd29 100644
>>> --- a/drivers/power/regulator/regulator-uclass.c
>>> +++ b/drivers/power/regulator/regulator-uclass.c
>>> @@ -80,6 +80,11 @@ int regulator_get_current(struct udevice *dev)
>>>  int regulator_set_current(struct udevice *dev, int uA)
>>>  {
>>>         const struct dm_regulator_ops *ops = dev_get_driver_ops(dev);
>>> +       struct dm_regulator_uclass_platdata *uc_pdata;
>>> +
>>> +       uc_pdata = dev_get_uclass_platdata(dev);
>>> +       if (uA < uc_pdata->min_uA || uA > uc_pdata->max_uA)
>>> +               return -EINVAL;
>>
>>
>> Do all drivers have these values set?
>
> Simon,
>
> Agree that not all drivers set this. But when someone calls set_current with
> some value there needs to be some boundary conditions for this right? Hence
> i made this patch.
>

I think your patch is good. I'm just worried about breaking boards.
Can you take a quick look at existing users and make sure that won't
happen?

Regards,
Simon

[U-Boot] [PATCH 1/3] power: regulator: Introduce regulator_set_value_force function

[Simon Glass]

On 26 October 2016 at 10:31, Simon Glass <sjg@chromium.org> wrote:
> On 26 October 2016 at 01:12, Keerthy <j-keerthy@ti.com> wrote:
>> In case we want to force a particular value on a regulator
>> irrespective of the min/max constraints for testing purposes
>> one can call regulator_set_value_force function.
>>
>> Signed-off-by: Keerthy <j-keerthy@ti.com>
>> ---
>>  cmd/regulator.c                            |  5 ++++-
>>  drivers/power/regulator/regulator-uclass.c | 14 ++++++++++++++
>>  include/power/regulator.h                  | 10 ++++++++++
>>  3 files changed, 28 insertions(+), 1 deletion(-)
>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot-rockchip, thanks!

[U-Boot] [PATCH 2/3] power: regulator: Add limits checking while setting voltage

[Simon Glass]

On 26 October 2016 at 10:31, Simon Glass <sjg@chromium.org> wrote:
> On 26 October 2016 at 01:12, Keerthy <j-keerthy@ti.com> wrote:
>> Currently the specific set ops functions are directly
>> called without any check for voltage limits for a regulator.
>> Check for them and proceed.
>>
>> Signed-off-by: Keerthy <j-keerthy@ti.com>
>> ---
>>  drivers/power/regulator/regulator-uclass.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot-rockchip, thanks!

[U-Boot] [PATCH 3/3] power: regulator: Add limits checking while setting current

[Simon Glass]

On 27 October 2016 at 19:52, Simon Glass <sjg@chromium.org> wrote:
> Hi Keethy,
>
> On 26 October 2016 at 20:20, Keerthy <j-keerthy@ti.com> wrote:
>>
>>
>> On Wednesday 26 October 2016 10:01 PM, Simon Glass wrote:
>>>
>>> Hi Keerthy,
>>>
>>> On 26 October 2016 at 01:12, Keerthy <j-keerthy@ti.com> wrote:
>>>>
>>>> Currently the specific set ops functions are directly
>>>> called without any check for min/max current limits for a regulator.
>>>> Check for them and proceed.
>>>>
>>>> Signed-off-by: Keerthy <j-keerthy@ti.com>
>>>> ---
>>>>  drivers/power/regulator/regulator-uclass.c | 5 +++++
>>>>  1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/drivers/power/regulator/regulator-uclass.c
>>>> b/drivers/power/regulator/regulator-uclass.c
>>>> index 34087c8..4c4bd29 100644
>>>> --- a/drivers/power/regulator/regulator-uclass.c
>>>> +++ b/drivers/power/regulator/regulator-uclass.c
>>>> @@ -80,6 +80,11 @@ int regulator_get_current(struct udevice *dev)
>>>>  int regulator_set_current(struct udevice *dev, int uA)
>>>>  {
>>>>         const struct dm_regulator_ops *ops = dev_get_driver_ops(dev);
>>>> +       struct dm_regulator_uclass_platdata *uc_pdata;
>>>> +
>>>> +       uc_pdata = dev_get_uclass_platdata(dev);
>>>> +       if (uA < uc_pdata->min_uA || uA > uc_pdata->max_uA)
>>>> +               return -EINVAL;
>>>
>>>
>>> Do all drivers have these values set?
>>
>> Simon,
>>
>> Agree that not all drivers set this. But when someone calls set_current with
>> some value there needs to be some boundary conditions for this right? Hence
>> i made this patch.
>>
>
> I think your patch is good. I'm just worried about breaking boards.
> Can you take a quick look at existing users and make sure that won't
> happen?

This seems OK in my testing, so:

Applied to u-boot-rockchip, thanks!