* [PATCH] regulator: of: removing two variables min_uV and max_uV
@ 2015-10-31 15:01 Saurabh Sengar
2015-11-01 2:29 ` Mark Brown
0 siblings, 1 reply; 4+ messages in thread
From: Saurabh Sengar @ 2015-10-31 15:01 UTC (permalink / raw)
To: lgirdwood, broonie, linux-kernel; +Cc: 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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] regulator: of: removing two variables min_uV and max_uV
2015-10-31 15:01 [PATCH] regulator: of: removing two variables min_uV and max_uV Saurabh Sengar
@ 2015-11-01 2:29 ` Mark Brown
2015-11-02 9:48 ` [PATCH v2] " Saurabh Sengar
0 siblings, 1 reply; 4+ messages in thread
From: Mark Brown @ 2015-11-01 2:29 UTC (permalink / raw)
To: Saurabh Sengar; +Cc: lgirdwood, linux-kernel
[-- 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 --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2] regulator: of: removing two variables min_uV and max_uV
2015-11-01 2:29 ` Mark Brown
@ 2015-11-02 9:48 ` Saurabh Sengar
2015-11-03 17:29 ` Mark Brown
0 siblings, 1 reply; 4+ messages in thread
From: Saurabh Sengar @ 2015-11-02 9:48 UTC (permalink / raw)
To: broonie; +Cc: linux-kernel, lgirdwood, saurabh.truth
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] regulator: of: removing two variables min_uV and max_uV
2015-11-02 9:48 ` [PATCH v2] " Saurabh Sengar
@ 2015-11-03 17:29 ` Mark Brown
0 siblings, 0 replies; 4+ messages in thread
From: Mark Brown @ 2015-11-03 17:29 UTC (permalink / raw)
To: Saurabh Sengar; +Cc: linux-kernel, lgirdwood
[-- 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 --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-11-03 17:29 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-31 15:01 [PATCH] regulator: of: removing two variables min_uV and max_uV Saurabh Sengar
2015-11-01 2:29 ` Mark Brown
2015-11-02 9:48 ` [PATCH v2] " Saurabh Sengar
2015-11-03 17:29 ` Mark Brown
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).