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

Groups > linux.kernel > #1534281 > unrolled thread

[PATCH] regulator: tps65086: Fix 25mV ranges for BUCK regulators

Started by"Andrew F. Davis" <afd@ti.com>
First post2016-12-01 17:50 +0100
Last post2016-12-01 23:30 +0100
Articles 4 — 2 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH] regulator: tps65086: Fix 25mV ranges for BUCK regulators "Andrew F. Davis" <afd@ti.com> - 2016-12-01 17:50 +0100
    Re: [PATCH] regulator: tps65086: Fix 25mV ranges for BUCK regulators "Andrew F. Davis" <afd@ti.com> - 2016-12-01 23:10 +0100
      Re: [PATCH] regulator: tps65086: Fix 25mV ranges for BUCK regulators Mark Brown <broonie@kernel.org> - 2016-12-01 23:20 +0100
        Re: [PATCH] regulator: tps65086: Fix 25mV ranges for BUCK regulators "Andrew F. Davis" <afd@ti.com> - 2016-12-01 23:30 +0100

#1534281 — [PATCH] regulator: tps65086: Fix 25mV ranges for BUCK regulators

From"Andrew F. Davis" <afd@ti.com>
Date2016-12-01 17:50 +0100
Subject[PATCH] regulator: tps65086: Fix 25mV ranges for BUCK regulators
Message-ID<sJCIy-6wt-19@gated-at.bofh.it>
The BUCK regulators 3, 4, and 5 also have a 10mV step mode,
adjust the tables and logic to reflect the data-sheet for
these regulators.

fixes: d2a2e729a666 ("regulator: tps65086: Add regulator driver for the TPS65086 PMIC")
Signed-off-by: Andrew F. Davis <afd@ti.com>
---
 Documentation/devicetree/bindings/mfd/tps65086.txt |  2 +-
 drivers/regulator/tps65086-regulator.c             | 54 +++++++++++-----------
 2 files changed, 29 insertions(+), 27 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/tps65086.txt b/Documentation/devicetree/bindings/mfd/tps65086.txt
index d370561..9cfa886 100644
--- a/Documentation/devicetree/bindings/mfd/tps65086.txt
+++ b/Documentation/devicetree/bindings/mfd/tps65086.txt
@@ -23,7 +23,7 @@ Required properties:
                             defined below.
 
 Optional regulator properties:
- - ti,regulator-step-size-25mv	: This is applicable for buck[1,2,6], set this
+ - ti,regulator-step-size-25mv	: This is applicable for buck[1-6], set this
 				    if the regulator is factory set with a 25mv
 				    step voltage mapping.
  - ti,regulator-decay		: This is applicable for buck[1-6], set this if
diff --git a/drivers/regulator/tps65086-regulator.c b/drivers/regulator/tps65086-regulator.c
index 33f389d..caf174f 100644
--- a/drivers/regulator/tps65086-regulator.c
+++ b/drivers/regulator/tps65086-regulator.c
@@ -71,18 +71,17 @@ struct tps65086_regulator {
 	unsigned int decay_mask;
 };
 
-static const struct regulator_linear_range tps65086_buck126_10mv_ranges[] = {
+static const struct regulator_linear_range tps65086_10mv_ranges[] = {
 	REGULATOR_LINEAR_RANGE(0, 0x0, 0x0, 0),
 	REGULATOR_LINEAR_RANGE(410000, 0x1, 0x7F, 10000),
 };
 
 static const struct regulator_linear_range tps65086_buck126_25mv_ranges[] = {
-	REGULATOR_LINEAR_RANGE(0, 0x0, 0x0, 0),
-	REGULATOR_LINEAR_RANGE(1000000, 0x1, 0x18, 0),
+	REGULATOR_LINEAR_RANGE(1000000, 0x0, 0x18, 0),
 	REGULATOR_LINEAR_RANGE(1025000, 0x19, 0x7F, 25000),
 };
 
-static const struct regulator_linear_range tps65086_buck345_ranges[] = {
+static const struct regulator_linear_range tps65086_buck345_25mv_ranges[] = {
 	REGULATOR_LINEAR_RANGE(0, 0x0, 0x0, 0),
 	REGULATOR_LINEAR_RANGE(425000, 0x1, 0x7F, 25000),
 };
@@ -125,27 +124,27 @@ static int tps65086_of_parse_cb(struct device_node *dev,
 static struct tps65086_regulator regulators[] = {
 	TPS65086_REGULATOR("BUCK1", "buck1", BUCK1, 0x80, TPS65086_BUCK1CTRL,
 			   BUCK_VID_MASK, TPS65086_BUCK123CTRL, BIT(0),
-			   tps65086_buck126_10mv_ranges, TPS65086_BUCK1CTRL,
+			   tps65086_10mv_ranges, TPS65086_BUCK1CTRL,
 			   BIT(0)),
 	TPS65086_REGULATOR("BUCK2", "buck2", BUCK2, 0x80, TPS65086_BUCK2CTRL,
 			   BUCK_VID_MASK, TPS65086_BUCK123CTRL, BIT(1),
-			   tps65086_buck126_10mv_ranges, TPS65086_BUCK2CTRL,
+			   tps65086_10mv_ranges, TPS65086_BUCK2CTRL,
 			   BIT(0)),
 	TPS65086_REGULATOR("BUCK3", "buck3", BUCK3, 0x80, TPS65086_BUCK3VID,
 			   BUCK_VID_MASK, TPS65086_BUCK123CTRL, BIT(2),
-			   tps65086_buck345_ranges, TPS65086_BUCK3DECAY,
+			   tps65086_10mv_ranges, TPS65086_BUCK3DECAY,
 			   BIT(0)),
 	TPS65086_REGULATOR("BUCK4", "buck4", BUCK4, 0x80, TPS65086_BUCK4VID,
 			   BUCK_VID_MASK, TPS65086_BUCK4CTRL, BIT(0),
-			   tps65086_buck345_ranges, TPS65086_BUCK4VID,
+			   tps65086_10mv_ranges, TPS65086_BUCK4VID,
 			   BIT(0)),
 	TPS65086_REGULATOR("BUCK5", "buck5", BUCK5, 0x80, TPS65086_BUCK5VID,
 			   BUCK_VID_MASK, TPS65086_BUCK5CTRL, BIT(0),
-			   tps65086_buck345_ranges, TPS65086_BUCK5CTRL,
+			   tps65086_10mv_ranges, TPS65086_BUCK5CTRL,
 			   BIT(0)),
 	TPS65086_REGULATOR("BUCK6", "buck6", BUCK6, 0x80, TPS65086_BUCK6VID,
 			   BUCK_VID_MASK, TPS65086_BUCK6CTRL, BIT(0),
-			   tps65086_buck126_10mv_ranges, TPS65086_BUCK6CTRL,
+			   tps65086_10mv_ranges, TPS65086_BUCK6CTRL,
 			   BIT(0)),
 	TPS65086_REGULATOR("LDOA1", "ldoa1", LDOA1, 0xF, TPS65086_LDOA1CTRL,
 			   VDOA1_VID_MASK, TPS65086_LDOA1CTRL, BIT(0),
@@ -162,18 +161,6 @@ static struct tps65086_regulator regulators[] = {
 	TPS65086_SWITCH("VTT", "vtt", VTT, TPS65086_SWVTT_EN, BIT(4)),
 };
 
-static inline bool has_25mv_mode(int id)
-{
-	switch (id) {
-	case BUCK1:
-	case BUCK2:
-	case BUCK6:
-		return true;
-	default:
-		return false;
-	}
-}
-
 static int tps65086_of_parse_cb(struct device_node *dev,
 				const struct regulator_desc *desc,
 				struct regulator_config *config)
@@ -181,12 +168,27 @@ static int tps65086_of_parse_cb(struct device_node *dev,
 	int ret;
 
 	/* Check for 25mV step mode */
-	if (has_25mv_mode(desc->id) &&
-			of_property_read_bool(config->of_node, "ti,regulator-step-size-25mv")) {
-		regulators[desc->id].desc.linear_ranges =
+	if (of_property_read_bool(config->of_node, "ti,regulator-step-size-25mv")) {
+		switch (desc->id) {
+		case BUCK1:
+		case BUCK2:
+		case BUCK6:
+			regulators[desc->id].desc.linear_ranges =
 				tps65086_buck126_25mv_ranges;
-		regulators[desc->id].desc.n_linear_ranges =
+			regulators[desc->id].desc.n_linear_ranges =
 				ARRAY_SIZE(tps65086_buck126_25mv_ranges);
+			break;
+		case BUCK3:
+		case BUCK4:
+		case BUCK5:
+			regulators[desc->id].desc.linear_ranges =
+				tps65086_buck345_25mv_ranges;
+			regulators[desc->id].desc.n_linear_ranges =
+				ARRAY_SIZE(tps65086_buck345_25mv_ranges);
+			break;
+		default:
+			dev_warn(config->dev, "25mV step mode only valid for BUCK regulators\n");
+		}
 	}
 
 	/* Check for decay mode */
-- 
2.10.2

[toc] | [next] | [standalone]

#1534490

From"Andrew F. Davis" <afd@ti.com>
Date2016-12-01 23:10 +0100
Message-ID<sJHIe-2WG-29@gated-at.bofh.it>
In reply to#1534281
On 12/01/2016 10:44 AM, Andrew F. Davis wrote:
> The BUCK regulators 3, 4, and 5 also have a 10mV step mode,
> adjust the tables and logic to reflect the data-sheet for
> these regulators.
> 
> fixes: d2a2e729a666 ("regulator: tps65086: Add regulator driver for the TPS65086 PMIC")
> Signed-off-by: Andrew F. Davis <afd@ti.com>
> ---
>  Documentation/devicetree/bindings/mfd/tps65086.txt |  2 +-
>  drivers/regulator/tps65086-regulator.c             | 54 +++++++++++-----------
>  2 files changed, 29 insertions(+), 27 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/tps65086.txt b/Documentation/devicetree/bindings/mfd/tps65086.txt
> index d370561..9cfa886 100644
> --- a/Documentation/devicetree/bindings/mfd/tps65086.txt
> +++ b/Documentation/devicetree/bindings/mfd/tps65086.txt
> @@ -23,7 +23,7 @@ Required properties:
>                              defined below.
>  
>  Optional regulator properties:
> - - ti,regulator-step-size-25mv	: This is applicable for buck[1,2,6], set this
> + - ti,regulator-step-size-25mv	: This is applicable for buck[1-6], set this
>  				    if the regulator is factory set with a 25mv
>  				    step voltage mapping.
>   - ti,regulator-decay		: This is applicable for buck[1-6], set this if
> diff --git a/drivers/regulator/tps65086-regulator.c b/drivers/regulator/tps65086-regulator.c
> index 33f389d..caf174f 100644
> --- a/drivers/regulator/tps65086-regulator.c
> +++ b/drivers/regulator/tps65086-regulator.c
> @@ -71,18 +71,17 @@ struct tps65086_regulator {
>  	unsigned int decay_mask;
>  };
>  
> -static const struct regulator_linear_range tps65086_buck126_10mv_ranges[] = {
> +static const struct regulator_linear_range tps65086_10mv_ranges[] = {
>  	REGULATOR_LINEAR_RANGE(0, 0x0, 0x0, 0),
>  	REGULATOR_LINEAR_RANGE(410000, 0x1, 0x7F, 10000),
>  };
>  
>  static const struct regulator_linear_range tps65086_buck126_25mv_ranges[] = {

\/\/\/\/\/

> -	REGULATOR_LINEAR_RANGE(0, 0x0, 0x0, 0),
> -	REGULATOR_LINEAR_RANGE(1000000, 0x1, 0x18, 0),
> +	REGULATOR_LINEAR_RANGE(1000000, 0x0, 0x18, 0),

I just got word that there was a typo in the datasheet, 0x0 really does
map to 0v out, the above change chunk can be dropped. It looks this
patch also just got pulled, should I submit a correction patch or can
this be fixed up locally?

Thanks,
Andrew

>  	REGULATOR_LINEAR_RANGE(1025000, 0x19, 0x7F, 25000),
>  };
>  
> -static const struct regulator_linear_range tps65086_buck345_ranges[] = {
> +static const struct regulator_linear_range tps65086_buck345_25mv_ranges[] = {
>  	REGULATOR_LINEAR_RANGE(0, 0x0, 0x0, 0),
>  	REGULATOR_LINEAR_RANGE(425000, 0x1, 0x7F, 25000),
>  };
> @@ -125,27 +124,27 @@ static int tps65086_of_parse_cb(struct device_node *dev,
>  static struct tps65086_regulator regulators[] = {
>  	TPS65086_REGULATOR("BUCK1", "buck1", BUCK1, 0x80, TPS65086_BUCK1CTRL,
>  			   BUCK_VID_MASK, TPS65086_BUCK123CTRL, BIT(0),
> -			   tps65086_buck126_10mv_ranges, TPS65086_BUCK1CTRL,
> +			   tps65086_10mv_ranges, TPS65086_BUCK1CTRL,
>  			   BIT(0)),
>  	TPS65086_REGULATOR("BUCK2", "buck2", BUCK2, 0x80, TPS65086_BUCK2CTRL,
>  			   BUCK_VID_MASK, TPS65086_BUCK123CTRL, BIT(1),
> -			   tps65086_buck126_10mv_ranges, TPS65086_BUCK2CTRL,
> +			   tps65086_10mv_ranges, TPS65086_BUCK2CTRL,
>  			   BIT(0)),
>  	TPS65086_REGULATOR("BUCK3", "buck3", BUCK3, 0x80, TPS65086_BUCK3VID,
>  			   BUCK_VID_MASK, TPS65086_BUCK123CTRL, BIT(2),
> -			   tps65086_buck345_ranges, TPS65086_BUCK3DECAY,
> +			   tps65086_10mv_ranges, TPS65086_BUCK3DECAY,
>  			   BIT(0)),
>  	TPS65086_REGULATOR("BUCK4", "buck4", BUCK4, 0x80, TPS65086_BUCK4VID,
>  			   BUCK_VID_MASK, TPS65086_BUCK4CTRL, BIT(0),
> -			   tps65086_buck345_ranges, TPS65086_BUCK4VID,
> +			   tps65086_10mv_ranges, TPS65086_BUCK4VID,
>  			   BIT(0)),
>  	TPS65086_REGULATOR("BUCK5", "buck5", BUCK5, 0x80, TPS65086_BUCK5VID,
>  			   BUCK_VID_MASK, TPS65086_BUCK5CTRL, BIT(0),
> -			   tps65086_buck345_ranges, TPS65086_BUCK5CTRL,
> +			   tps65086_10mv_ranges, TPS65086_BUCK5CTRL,
>  			   BIT(0)),
>  	TPS65086_REGULATOR("BUCK6", "buck6", BUCK6, 0x80, TPS65086_BUCK6VID,
>  			   BUCK_VID_MASK, TPS65086_BUCK6CTRL, BIT(0),
> -			   tps65086_buck126_10mv_ranges, TPS65086_BUCK6CTRL,
> +			   tps65086_10mv_ranges, TPS65086_BUCK6CTRL,
>  			   BIT(0)),
>  	TPS65086_REGULATOR("LDOA1", "ldoa1", LDOA1, 0xF, TPS65086_LDOA1CTRL,
>  			   VDOA1_VID_MASK, TPS65086_LDOA1CTRL, BIT(0),
> @@ -162,18 +161,6 @@ static struct tps65086_regulator regulators[] = {
>  	TPS65086_SWITCH("VTT", "vtt", VTT, TPS65086_SWVTT_EN, BIT(4)),
>  };
>  
> -static inline bool has_25mv_mode(int id)
> -{
> -	switch (id) {
> -	case BUCK1:
> -	case BUCK2:
> -	case BUCK6:
> -		return true;
> -	default:
> -		return false;
> -	}
> -}
> -
>  static int tps65086_of_parse_cb(struct device_node *dev,
>  				const struct regulator_desc *desc,
>  				struct regulator_config *config)
> @@ -181,12 +168,27 @@ static int tps65086_of_parse_cb(struct device_node *dev,
>  	int ret;
>  
>  	/* Check for 25mV step mode */
> -	if (has_25mv_mode(desc->id) &&
> -			of_property_read_bool(config->of_node, "ti,regulator-step-size-25mv")) {
> -		regulators[desc->id].desc.linear_ranges =
> +	if (of_property_read_bool(config->of_node, "ti,regulator-step-size-25mv")) {
> +		switch (desc->id) {
> +		case BUCK1:
> +		case BUCK2:
> +		case BUCK6:
> +			regulators[desc->id].desc.linear_ranges =
>  				tps65086_buck126_25mv_ranges;
> -		regulators[desc->id].desc.n_linear_ranges =
> +			regulators[desc->id].desc.n_linear_ranges =
>  				ARRAY_SIZE(tps65086_buck126_25mv_ranges);
> +			break;
> +		case BUCK3:
> +		case BUCK4:
> +		case BUCK5:
> +			regulators[desc->id].desc.linear_ranges =
> +				tps65086_buck345_25mv_ranges;
> +			regulators[desc->id].desc.n_linear_ranges =
> +				ARRAY_SIZE(tps65086_buck345_25mv_ranges);
> +			break;
> +		default:
> +			dev_warn(config->dev, "25mV step mode only valid for BUCK regulators\n");
> +		}
>  	}
>  
>  	/* Check for decay mode */
>

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

#1534499

FromMark Brown <broonie@kernel.org>
Date2016-12-01 23:20 +0100
Message-ID<sJHRT-35y-11@gated-at.bofh.it>
In reply to#1534490

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

On Thu, Dec 01, 2016 at 04:02:17PM -0600, Andrew F. Davis wrote:

> I just got word that there was a typo in the datasheet, 0x0 really does
> map to 0v out, the above change chunk can be dropped. It looks this
> patch also just got pulled, should I submit a correction patch or can
> this be fixed up locally?

A followup would be easier if only to make sure I don't make any
mistakes.

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

#1534508

From"Andrew F. Davis" <afd@ti.com>
Date2016-12-01 23:30 +0100
Message-ID<sJI1A-3bb-31@gated-at.bofh.it>
In reply to#1534499
On 12/01/2016 04:18 PM, Mark Brown wrote:
> On Thu, Dec 01, 2016 at 04:02:17PM -0600, Andrew F. Davis wrote:
> 
>> I just got word that there was a typo in the datasheet, 0x0 really does
>> map to 0v out, the above change chunk can be dropped. It looks this
>> patch also just got pulled, should I submit a correction patch or can
>> this be fixed up locally?
> 
> A followup would be easier if only to make sure I don't make any
> mistakes.
> 

Works for me, will send.

Andrew

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web