On Sat, Oct 31, 2015 at 08:31:31PM +0530, Saurabh Sengar wrote:
> - min_uV = of_get_property(np, "regulator-min-microvolt", NULL);
> - if (min_uV)
> - constraints->min_uV = be32_to_cpu(*min_uV);
> + if (!of_property_read_u32(np, "regulator-max-microvolt", &pval))
> + constraints->max_uV = pval;
> /* Only one voltage? Then make sure it's set. */
> - if (min_uV && max_uV && constraints->min_uV == constraints->max_uV)
> + if (constraints->min_uV == constraints->max_uV)
> constraints->apply_uV = true;
Your new code is not equivalent to the existing code. The new code will
set apply_uV even if the DT properties are not present which will in
turn mean that we will end up attempting to apply a setting of 0V if
that happens which is not desirable.
replacing the of_get_property function with of_property_read_u32 function
as its help removing two variables.
Signed-off-by: Saurabh Sengar <[email protected]>
---
Hi Mark,
>> /* Only one voltage? Then make sure it's set. */
>> - if (min_uV && max_uV && constraints->min_uV == constraints->max_uV)
>> + if (constraints->min_uV == constraints->max_uV)
>> constraints->apply_uV = true;
> Your new code is not equivalent to the existing code. The new code will
> set apply_uV even if the DT properties are not present which will in
> turn mean that we will end up attempting to apply a setting of 0V if
> that happens which is not desirable.
I have put these check back, please let me know if this v2 patch is worth
drivers/regulator/of_regulator.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
index 499e437..3710206 100644
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -28,7 +28,6 @@ static void of_get_regulation_constraints(struct device_node *np,
struct regulator_init_data **init_data,
const struct regulator_desc *desc)
{
- const __be32 *min_uV, *max_uV;
struct regulation_constraints *constraints = &(*init_data)->constraints;
struct regulator_state *suspend_state;
struct device_node *suspend_np;
@@ -37,18 +36,17 @@ static void of_get_regulation_constraints(struct device_node *np,
constraints->name = of_get_property(np, "regulator-name", NULL);
- min_uV = of_get_property(np, "regulator-min-microvolt", NULL);
- if (min_uV)
- constraints->min_uV = be32_to_cpu(*min_uV);
- max_uV = of_get_property(np, "regulator-max-microvolt", NULL);
- if (max_uV)
- constraints->max_uV = be32_to_cpu(*max_uV);
+ if (!of_property_read_u32(np, "regulator-min-microvolt", &pval))
+ constraints->min_uV = pval;
+ if (!of_property_read_u32(np, "regulator-max-microvolt", &pval))
+ constraints->max_uV = pval;
/* Voltage change possible? */
if (constraints->min_uV != constraints->max_uV)
constraints->valid_ops_mask |= REGULATOR_CHANGE_VOLTAGE;
/* Only one voltage? Then make sure it's set. */
- if (min_uV && max_uV && constraints->min_uV == constraints->max_uV)
+ if (constraints->min_uV && constraints->max_uV
+ && constraints->min_uV == constraints->max_uV)
constraints->apply_uV = true;
if (!of_property_read_u32(np, "regulator-microvolt-offset", &pval))
--
1.9.1
On Mon, Nov 02, 2015 at 03:18:00PM +0530, Saurabh Sengar wrote:
> replacing the of_get_property function with of_property_read_u32 function
> as its help removing two variables.
> I have put these check back, please let me know if this v2 patch is worth
It's a bit marginal TBH and...
> /* Only one voltage? Then make sure it's set. */
> - if (min_uV && max_uV && constraints->min_uV == constraints->max_uV)
> + if (constraints->min_uV && constraints->max_uV
> + && constraints->min_uV == constraints->max_uV)
> constraints->apply_uV = true;
...this does disallow a constraint of 0-XuV which was previously legal
and potentially meaningful. Hrm.