Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1260004 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [PATCH] regulator: of: removing two variables min_uV and max_uV Saurabh Sengar <saurabh.truth@gmail.com> - 2015-10-31 16:10 +0100
    Re: [PATCH] regulator: of: removing two variables min_uV and max_uV Mark Brown <broonie@kernel.org> - 2015-11-02 10:00 +0100
      [PATCH v2] regulator: of: removing two variables min_uV and max_uV Saurabh Sengar <saurabh.truth@gmail.com> - 2015-11-02 10:50 +0100
        Re: [PATCH v2] regulator: of: removing two variables min_uV and  max_uV Mark Brown <broonie@kernel.org> - 2015-11-03 18:30 +0100

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

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1260459

[Multipart message — attachments visible in raw view] — view raw

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.

[toc] | [prev] | [next] | [standalone]

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

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

[Multipart message — attachments visible in raw view] — view raw

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.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web