| Message ID | 1477469552-10510-3-git-send-email-j-keerthy@ti.com |
|---|---|
| State | Accepted |
| Delegated to: | Simon Glass |
| Headers | show |
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
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 >
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
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!
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;
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(+)