[PATCH] regulator: of: removing two variables min_uV and max_uV

[PATCH] regulator: of: removing two variables min_uV and max_uV

[Saurabh Sengar]

replacing the of_get_property function with of_property_read_u32 function
as its help removing two variables.
also the check for min_uV and max_uV is not required, even if they are
zero and equal we should set apply_uV as true

Signed-off-by: Saurabh Sengar <saurabh.truth@gmail.com>
---
 drivers/regulator/of_regulator.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
index 250700c..e419789 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,16 @@ 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->apply_uV = true;
 
 	if (!of_property_read_u32(np, "regulator-microvolt-offset", &pval))
-- 
1.9.1

Re: [PATCH] regulator: of: removing two variables min_uV and max_uV

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 763 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[PATCH v2] regulator: of: removing two variables min_uV and max_uV

[Saurabh Sengar]

replacing the of_get_property function with of_property_read_u32 function
as its help removing two variables.

Signed-off-by: Saurabh Sengar <saurabh.truth@gmail.com>
---
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

Re: [PATCH v2] regulator: of: removing two variables min_uV and max_uV

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 659 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]