regulator: Fix _regulator_get_voltage if get_voltage callback is NULL
RFC PATCH 1/1 regulator: Fix _regulator_get_voltage if get_voltage callback is NULL
In the case of get_voltage callback is NULL, current implementation in _regulator_get_voltage will return -EINVAL.
Also returns proper error if ret is negative value.
---
hi Mark,
I just found in commit 6b8e70 "regulator: Convert WM8400 to get_voltage_sel()", you added get_voltage_sel callback and remove get_voltage callback. I suspect you will hit this bug. Can you check it?
Thanks,
Axel
drivers/regulator/core.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-)
drivers/regulator/core.c +3 -1
@@ -1886,13 +1886,15 @@ static int _regulator_get_voltage(struct regulator_dev *rdev)if (sel <0 )return sel; ret = rdev->desc->ops->list_voltage (rdev, sel); + goto out; }if (rdev->desc->ops->get_voltage) ret = rdev->desc->ops->get_voltage (rdev);else return -EINVAL ; -return ret - rdev->constraints->uV_offset; +out: +return ret <0 ? ret : ret - rdev->constraints->uV_offset; }/**
Signature · 1 line
1.7.1
drivers/regulator/core.c static int _regulator_get_voltage(struct regulator_dev *rdev) · line 1890
Quoted 6 lines · Axel Lin
drivers/regulator/core.c +1
-0@@ -1886,13 +1886,15 @@ static int _regulator_get_voltage(struct regulator_dev *rdev)if (sel <0 )return sel; ret = rdev->desc->ops->list_voltage (rdev, sel); + goto out; }
Why are you adding this goto? It's not going to change anything...
2011/5/19 Mark Brown <[email protected]>:
Quoted 11 lines · Axel Lin
drivers/regulator/core.c static int _regulator_get_voltage(struct regulator_dev *rdev) · line 1890
Quoted 7 lines
drivers/regulator/core.c +1
-0@@ -1886,13 +1886,15 @@ static int _regulator_get_voltage(struct regulator_dev *rdev) if (sel < 0) return sel; ret = rdev->desc->ops->list_voltage(rdev, sel); + goto out; }Why are you adding this goto? It's not going to change anything...
no matter what ret value returns from rdev->desc->ops->list_voltage(rdev, sel);
In the case of rdev->desc->ops->get_voltage is NULL, current implementation will always return -EINVAL;
Regards,
Axel
2011/5/19 Axel Lin <[email protected]>:
Quoted 12 lines
2011/5/19 Mark Brown <[email protected]>:
Quoted 11 lines · Axel Lin
Review static int _regulator_get_voltage(struct regulator_dev *rdev) · line 1890
Quoted 7 lines
(quoted diff) +1
-0@@ -1886,13 +1886,15 @@ static int _regulator_get_voltage(struct regulator_dev *rdev) if (sel < 0) return sel; ret = rdev->desc->ops->list_voltage(rdev, sel); + goto out; }Why are you adding this goto? It's not going to change anything...
Quoted 5 lines
no matter what ret value returns from
rdev->desc->ops->list_voltage(rdev, sel);In the case of
rdev->desc->ops->get_voltageis NULL, current implementation will always return-EINVAL;
hi Mark,
I'm thinking maybe you prefer changes like below:
How do you think?
drivers/regulator/core.c +0 -0
@@ -1887,12 +1887,12 @@ static int _regulator_get_voltage(struct
regulator_dev *rdev)
return sel;
ret = rdev->desc->ops->list_voltage(rdev, sel);
}
- if (rdev->desc->ops->get_voltage)
+ else if (rdev->desc->ops->get_voltage)
ret = rdev->desc->ops->get_voltage(rdev);
else
return -EINVAL;- return ret - rdev->constraints->uV_offset;
+ return ret < 0 ? ret : ret - rdev->constraints->uV_offset;
}
/**
Regards,
Axel
Quoted 2 lines · Axel Lin
I'm thinking maybe you prefer changes like below:
How do you think?
Yes, something like that. The goto is obscure and hard to read.