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

Groups > linux.kernel > #1207500 > unrolled thread

[PATCH] regulator: core: Define regulator_set_voltage_triplet()

Back to article view | Back to linux.kernel

Contents

  [PATCH] regulator: core: Define regulator_set_voltage_triplet() Viresh Kumar <viresh.kumar@linaro.org> - 2015-08-14 14:10 +0200
    Re: [PATCH] regulator: core: Define regulator_set_voltage_triplet() Mark Brown <broonie@kernel.org> - 2015-08-15 04:20 +0200
      Re: [PATCH] regulator: core: Define regulator_set_voltage_triplet() Viresh Kumar <viresh.kumar@linaro.org> - 2015-08-15 05:20 +0200
        Re: [PATCH] regulator: core: Define regulator_set_voltage_triplet() Mark Brown <broonie@kernel.org> - 2015-08-15 16:10 +0200
          Re: [PATCH] regulator: core: Define regulator_set_voltage_triplet() Viresh Kumar <viresh.kumar@linaro.org> - 2015-08-17 04:20 +0200

#1207500 — [PATCH] regulator: core: Define regulator_set_voltage_triplet()

The OPP (Operating performance points) v2 bindings allows regulator
voltage to be supplied as a triplet of <target min max> voltages.

Add regulator_set_voltage_triplet() API in regulator core to support
that.

This first tries to set a voltage between the target voltage and the
upper limit, then fall back on the full range. The idea behind this is
to set regulator's voltage as close to the target voltage, as possible.

Based on regulator_set_voltage_tol().

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
Hi Mark,

This is required for OPP-V2 bindings that we introduced recently, where
the voltage can be given as a triplet of <target min max>.

I don't have any dependency on this until the time I update regulator
support in OPP core and so sending it separately. You can take it
through your tree. Ofcourse, only if it looks fine :)

 include/linux/regulator/consumer.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/include/linux/regulator/consumer.h b/include/linux/regulator/consumer.h
index f8a689ed62a5..621e89315c55 100644
--- a/include/linux/regulator/consumer.h
+++ b/include/linux/regulator/consumer.h
@@ -552,6 +552,16 @@ static inline int regulator_count_voltages(struct regulator *regulator)
 }
 #endif
 
+static inline int regulator_set_voltage_triplet(struct regulator *regulator,
+						int target_uV, int min_uV,
+						int max_uV)
+{
+	if (!regulator_set_voltage(regulator, target_uV, max_uV))
+		return 0;
+
+	return regulator_set_voltage(regulator, min_uV, max_uV);
+}
+
 static inline int regulator_set_voltage_tol(struct regulator *regulator,
 					    int new_uV, int tol_uV)
 {
-- 
2.4.0

--
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]

#1207985

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

On Fri, Aug 14, 2015 at 05:30:55PM +0530, Viresh Kumar wrote:

> The OPP (Operating performance points) v2 bindings allows regulator
> voltage to be supplied as a triplet of <target min max> voltages.

Looking at this I really don't understand why you tried to get me to
review this via an IRC pastebin instead of following the normal patch
submission process.  Please don't do that, it's not at all helpful.  IRC
can be useful for interactive discussions or things that are really
urgent, not for normal upstream code review.

> +static inline int regulator_set_voltage_triplet(struct regulator *regulator,
> +						int target_uV, int min_uV,
> +						int max_uV)

This seems awkward, these things are normally written as min <= target
<= max but this is target, min, max.

>  static inline int regulator_set_voltage_tol(struct regulator *regulator,
>  					    int new_uV, int tol_uV)
>  {
> -- 
> 2.4.0

You've not added a stub for the new function.

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

#1207999

On 14-08-15, 17:25, Mark Brown wrote:
> On Fri, Aug 14, 2015 at 05:30:55PM +0530, Viresh Kumar wrote:
> 
> > The OPP (Operating performance points) v2 bindings allows regulator
> > voltage to be supplied as a triplet of <target min max> voltages.
> 
> Looking at this I really don't understand why you tried to get me to
> review this via an IRC pastebin instead of following the normal patch
> submission process.  Please don't do that, it's not at all helpful.  IRC
> can be useful for interactive discussions or things that are really
> urgent, not for normal upstream code review.

Okay.

> > +static inline int regulator_set_voltage_triplet(struct regulator *regulator,
> > +						int target_uV, int min_uV,
> > +						int max_uV)
> 
> This seems awkward, these things are normally written as min <= target
> <= max but this is target, min, max.

Hmm..

> >  static inline int regulator_set_voltage_tol(struct regulator *regulator,
> >  					    int new_uV, int tol_uV)
> >  {
> > -- 
> > 2.4.0
> 
> You've not added a stub for the new function.

This function will always be present as its not part of the #ifdef
CONFIG_REGULATOR region.

And so the stub isn't required.

Hopefully below version looks better ?

--------------------------------8<--------------------------------------
Message-Id: <3d0154d897bba396c5018cf3b3562871af4ca844.1439608146.git.viresh.kumar@linaro.org>
From: Viresh Kumar <viresh.kumar@linaro.org>
Date: Fri, 14 Aug 2015 15:06:10 +0530
Subject: [PATCH] regulator: core: Define regulator_set_voltage_triplet()

The OPP (Operating performance points) v2 bindings allows regulator
voltage to be supplied as a triplet of <target min max> voltages.

Add regulator_set_voltage_triplet() API in regulator core to support
that.

This first tries to set a voltage between the target voltage and the
upper limit, then fall back on the full range. The idea behind this is
to set regulator's voltage as close to the target voltage, as possible.

Based on regulator_set_voltage_tol().

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 include/linux/regulator/consumer.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/include/linux/regulator/consumer.h b/include/linux/regulator/consumer.h
index f8a689ed62a5..a6269c184b8c 100644
--- a/include/linux/regulator/consumer.h
+++ b/include/linux/regulator/consumer.h
@@ -552,6 +552,16 @@ static inline int regulator_count_voltages(struct regulator *regulator)
 }
 #endif
 
+static inline int regulator_set_voltage_triplet(struct regulator *regulator,
+						int min_uV, int target_uV,
+						int max_uV)
+{
+	if (!regulator_set_voltage(regulator, target_uV, max_uV))
+		return 0;
+
+	return regulator_set_voltage(regulator, min_uV, max_uV);
+}
+
 static inline int regulator_set_voltage_tol(struct regulator *regulator,
 					    int new_uV, int tol_uV)
 {
--
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]

#1208090

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

On Sat, Aug 15, 2015 at 08:41:49AM +0530, Viresh Kumar wrote:

> Hopefully below version looks better ?

Please don't bury patches at the end of replies to messages in the
middle of threads, send them in a form where they can be applied
directly.

> Subject: [PATCH] regulator: core: Define regulator_set_voltage_triplet()
> 
> The OPP (Operating performance points) v2 bindings allows regulator
> voltage to be supplied as a triplet of <target min max> voltages.
> 
> Add regulator_set_voltage_triplet() API in regulator core to support
> that.

This should explain why this is useful at a regulator API level, not
explain that some user decided to do something.

> +static inline int regulator_set_voltage_triplet(struct regulator *regulator,
> +						int min_uV, int target_uV,
> +						int max_uV)
> +{
> +	if (!regulator_set_voltage(regulator, target_uV, max_uV))

Make this a check for == 0 which is what you mean here, this isn't a
boolean function.

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

#1208308

On 15-08-15, 07:08, Mark Brown wrote:
> On Sat, Aug 15, 2015 at 08:41:49AM +0530, Viresh Kumar wrote:
> 
> > Hopefully below version looks better ?
> 
> Please don't bury patches at the end of replies to messages in the
> middle of threads, send them in a form where they can be applied
> directly.

Okay. I thought its fine to send patches like this and this can be
applied directly with the --scissors option to git am or with
mailinfo.scissors option set in configuration file.

But will surely send patches separately to you.

> > Subject: [PATCH] regulator: core: Define regulator_set_voltage_triplet()
> > 
> > The OPP (Operating performance points) v2 bindings allows regulator
> > voltage to be supplied as a triplet of <target min max> voltages.
> > 
> > Add regulator_set_voltage_triplet() API in regulator core to support
> > that.
> 
> This should explain why this is useful at a regulator API level, not
> explain that some user decided to do something.

Okay.

> > +static inline int regulator_set_voltage_triplet(struct regulator *regulator,
> > +						int min_uV, int target_uV,
> > +						int max_uV)
> > +{
> > +	if (!regulator_set_voltage(regulator, target_uV, max_uV))
> 
> Make this a check for == 0 which is what you mean here, this isn't a
> boolean function.

Okay.

-- 
viresh
--
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] | [standalone]

Back to top | Article view | linux.kernel

csiph-web