[U-Boot] [PATCH] power: regulator: Add limits checking while setting voltage and current

[Przemyslaw Marczak]

Hello Keerthy,

On 09/15/2016 10:54 AM, Keerthy wrote:
> Currently the specific set ops functions are directly
> called without any check for voltage/current limits for a regulator.
> Check for them and proceed.
>
> Signed-off-by: Keerthy <j-keerthy at ti.com>
> ---
>   drivers/power/regulator/regulator-uclass.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)
>
> diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c
> index 4434e36..089455e 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;
> @@ -61,6 +66,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;

Adding those two checks will conflict with "force" option for 
cmd/regulator.c:283.
There was value range checking in the command's code, but it was left 
unchecked
for regulator direct calls.

This change is good, but then maybe the "force" option should be removed 
from command,
or API's prototypes should be updated by force flag argument?

I assumed that this option could be useful for quick over-voltage 
setting (until reboot),
since usually (min_uV == max_uV) - the voltage can't be changed in any 
range.

The driver should take care about ignore it or not, however probably 
nobody used this.

Of course this could potentially damage the device by wrong use,
which can be also made by passing the force flag as an argument - by 
mistake.

What do you thing about, update the dm_regulator_ops by:

  - int (*set_value)(struct udevice *dev, int uV, int flag);
  - int (*set_current)(struct udevice *dev, int uA, int flag);

and also new flag to the present defined:

  - REGULATOR_FLAG_FORCE = 1 << 2

This requires more work, but will provide the functionality in a proper way.

Best regards,

-- 
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com