* [PATCH 1/2] regulator: core: Use a bitfield for continuous_voltage_range
@ 2016-01-21 20:24 Mark Brown
2016-01-21 20:24 ` [PATCH 2/2] regulator: core: Provide per-regulator runtime PM support Mark Brown
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Mark Brown @ 2016-01-21 20:24 UTC (permalink / raw)
To: Paul Kocialkowski; +Cc: Liam Girdwood, linux-kernel, Mark Brown
Using a bitfield enables the compiler to lay out the structure more
efficiently when we have other boolean flags since multiple values can
be included in a single byte.
Signed-off-by: Mark Brown <broonie@kernel.org>
---
include/linux/regulator/driver.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index 16ac9e108806..3ac0f306f033 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -281,7 +281,7 @@ struct regulator_desc {
const struct regulator_desc *,
struct regulator_config *);
int id;
- bool continuous_voltage_range;
+ unsigned int continuous_voltage_range:1;
unsigned n_voltages;
const struct regulator_ops *ops;
int irq;
--
2.7.0.rc3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/2] regulator: core: Provide per-regulator runtime PM support
2016-01-21 20:24 [PATCH 1/2] regulator: core: Use a bitfield for continuous_voltage_range Mark Brown
@ 2016-01-21 20:24 ` Mark Brown
2016-01-21 21:49 ` kbuild test robot
` (2 more replies)
2016-01-22 19:15 ` [PATCH 1/2] regulator: core: Use a bitfield for continuous_voltage_range Joe Perches
2016-04-21 16:11 ` Applied "regulator: core: Use a bitfield for continuous_voltage_range" to the regulator tree Mark Brown
2 siblings, 3 replies; 12+ messages in thread
From: Mark Brown @ 2016-01-21 20:24 UTC (permalink / raw)
To: Paul Kocialkowski; +Cc: Liam Girdwood, linux-kernel, Mark Brown
Provide a flag auto_runtime_pm in the regulator_desc which causes the
regulator core to take a runtime PM reference to a regulator while it
is enabled. This helps integration with chip wide power management for
auxiliary PMICs, they may be able to implement chip wide power savings
if nothing on the PMIC is in use.
Signed-off-by: Mark Brown <broonie@kernel.org>
---
Not tested at all yet, pushing out for testing by others who have
devices that could benefit from this.
drivers/regulator/core.c | 20 ++++++++++++++++++--
include/linux/regulator/driver.h | 1 +
2 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 3308c6bb83db..968ff3081e6c 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -26,6 +26,7 @@
#include <linux/gpio.h>
#include <linux/gpio/consumer.h>
#include <linux/of.h>
+#include <linux/pm_runtime.h>
#include <linux/regmap.h>
#include <linux/regulator/of_regulator.h>
#include <linux/regulator/consumer.h>
@@ -2059,17 +2060,23 @@ static int _regulator_do_enable(struct regulator_dev *rdev)
}
}
+ if (rdev->desc->auto_runtime_pm) {
+ ret = pm_runtime_get_sync(rdev->dev.parent);
+ if (ret < 0)
+ goto err;
+ }
+
if (rdev->ena_pin) {
if (!rdev->ena_gpio_state) {
ret = regulator_ena_gpio_ctrl(rdev, true);
if (ret < 0)
- return ret;
+ goto err_pm;
rdev->ena_gpio_state = 1;
}
} else if (rdev->desc->ops->enable) {
ret = rdev->desc->ops->enable(rdev);
if (ret < 0)
- return ret;
+ goto err_pm;
} else {
return -EINVAL;
}
@@ -2084,6 +2091,12 @@ static int _regulator_do_enable(struct regulator_dev *rdev)
trace_regulator_enable_complete(rdev_get_name(rdev));
return 0;
+
+err_pm:
+ if (rdev->desc->auto_runtime_pm)
+ pm_runtime_put_autosuspend(rdev->dev.parent);
+err:
+ return ret;
}
/* locks held by regulator_enable() */
@@ -2177,6 +2190,9 @@ static int _regulator_do_disable(struct regulator_dev *rdev)
return ret;
}
+ if (rdev->desc->auto_runtime_pm)
+ pm_runtime_put_autosuspend(rdev->dev.parent);
+
/* cares about last_off_jiffy only if off_on_delay is required by
* device.
*/
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index 3ac0f306f033..dccea032a143 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -282,6 +282,7 @@ struct regulator_desc {
struct regulator_config *);
int id;
unsigned int continuous_voltage_range:1;
+ unsigned int auto_runtime_pm:1;
unsigned n_voltages;
const struct regulator_ops *ops;
int irq;
--
2.7.0.rc3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] regulator: core: Provide per-regulator runtime PM support
2016-01-21 20:24 ` [PATCH 2/2] regulator: core: Provide per-regulator runtime PM support Mark Brown
@ 2016-01-21 21:49 ` kbuild test robot
2016-01-29 12:02 ` Paul Kocialkowski
2016-02-09 20:51 ` Paul Kocialkowski
2 siblings, 0 replies; 12+ messages in thread
From: kbuild test robot @ 2016-01-21 21:49 UTC (permalink / raw)
To: Mark Brown
Cc: kbuild-all, Paul Kocialkowski, Liam Girdwood, linux-kernel, Mark Brown
[-- Attachment #1: Type: text/plain, Size: 13685 bytes --]
Hi Mark,
[auto build test WARNING on regulator/for-next]
[also build test WARNING on v4.4 next-20160121]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
url: https://github.com/0day-ci/linux/commits/Mark-Brown/regulator-core-Use-a-bitfield-for-continuous_voltage_range/20160122-042835
base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next
reproduce: make htmldocs
All warnings (new ones prefixed by >>):
include/linux/regulator/machine.h:151: warning: No description found for parameter 'over_current_protection'
include/linux/regulator/driver.h:202: warning: No description found for parameter 'set_over_current_protection'
>> include/linux/regulator/driver.h:325: warning: No description found for parameter 'auto_runtime_pm'
include/linux/regulator/driver.h:325: warning: No description found for parameter 'csel_reg'
include/linux/regulator/driver.h:325: warning: No description found for parameter 'csel_mask'
vim +/auto_runtime_pm +325 include/linux/regulator/driver.h
571a354b15 Liam Girdwood 2008-04-30 196 int (*set_suspend_disable) (struct regulator_dev *);
571a354b15 Liam Girdwood 2008-04-30 197
fde297bb4d Kim, Milo 2012-02-16 198 /* set regulator suspend operating mode (defined in consumer.h) */
571a354b15 Liam Girdwood 2008-04-30 199 int (*set_suspend_mode) (struct regulator_dev *, unsigned int mode);
23c779b9f9 Stephen Boyd 2015-06-11 200
23c779b9f9 Stephen Boyd 2015-06-11 201 int (*set_pull_down) (struct regulator_dev *);
571a354b15 Liam Girdwood 2008-04-30 @202 };
571a354b15 Liam Girdwood 2008-04-30 203
571a354b15 Liam Girdwood 2008-04-30 204 /*
571a354b15 Liam Girdwood 2008-04-30 205 * Regulators can either control voltage or current.
571a354b15 Liam Girdwood 2008-04-30 206 */
571a354b15 Liam Girdwood 2008-04-30 207 enum regulator_type {
571a354b15 Liam Girdwood 2008-04-30 208 REGULATOR_VOLTAGE,
571a354b15 Liam Girdwood 2008-04-30 209 REGULATOR_CURRENT,
571a354b15 Liam Girdwood 2008-04-30 210 };
571a354b15 Liam Girdwood 2008-04-30 211
571a354b15 Liam Girdwood 2008-04-30 212 /**
c172708d38 Mark Brown 2012-04-04 213 * struct regulator_desc - Static regulator descriptor
571a354b15 Liam Girdwood 2008-04-30 214 *
c172708d38 Mark Brown 2012-04-04 215 * Each regulator registered with the core is described with a
c172708d38 Mark Brown 2012-04-04 216 * structure of this type and a struct regulator_config. This
c172708d38 Mark Brown 2012-04-04 217 * structure contains the non-varying parts of the regulator
c172708d38 Mark Brown 2012-04-04 218 * description.
c8e7e4640f Mark Brown 2008-12-31 219 *
c8e7e4640f Mark Brown 2008-12-31 220 * @name: Identifying name for the regulator.
69511a452e Rajendra Nayak 2011-11-18 221 * @supply_name: Identifying the regulator supply
a0c7b164ad Mark Brown 2014-09-09 222 * @of_match: Name used to identify regulator in DT.
a0c7b164ad Mark Brown 2014-09-09 223 * @regulators_node: Name of node containing regulator definitions in DT.
bfa21a0dfe Krzysztof Kozlowski 2015-01-05 224 * @of_parse_cb: Optional callback called only if of_match is present.
bfa21a0dfe Krzysztof Kozlowski 2015-01-05 225 * Will be called for each regulator parsed from DT, during
bfa21a0dfe Krzysztof Kozlowski 2015-01-05 226 * init_data parsing.
bfa21a0dfe Krzysztof Kozlowski 2015-01-05 227 * The regulator_config passed as argument to the callback will
bfa21a0dfe Krzysztof Kozlowski 2015-01-05 228 * be a copy of config passed to regulator_register, valid only
bfa21a0dfe Krzysztof Kozlowski 2015-01-05 229 * for this particular call. Callback may freely change the
bfa21a0dfe Krzysztof Kozlowski 2015-01-05 230 * config but it cannot store it for later usage.
bfa21a0dfe Krzysztof Kozlowski 2015-01-05 231 * Callback should return 0 on success or negative ERRNO
bfa21a0dfe Krzysztof Kozlowski 2015-01-05 232 * indicating failure.
c8e7e4640f Mark Brown 2008-12-31 233 * @id: Numerical identifier for the regulator.
c8e7e4640f Mark Brown 2008-12-31 234 * @ops: Regulator operations table.
0ba4887c63 Randy Dunlap 2009-01-08 235 * @irq: Interrupt number for the regulator.
c8e7e4640f Mark Brown 2008-12-31 236 * @type: Indicates if the regulator is a voltage or current regulator.
c8e7e4640f Mark Brown 2008-12-31 237 * @owner: Module providing the regulator, used for refcounting.
bca7bbfff3 Mark Brown 2012-05-09 238 *
bd7a2b600a Pawel Moll 2012-09-24 239 * @continuous_voltage_range: Indicates if the regulator can set any
bd7a2b600a Pawel Moll 2012-09-24 240 * voltage within constrains range.
bca7bbfff3 Mark Brown 2012-05-09 241 * @n_voltages: Number of selectors available for ops.list_voltage().
bca7bbfff3 Mark Brown 2012-05-09 242 *
bca7bbfff3 Mark Brown 2012-05-09 243 * @min_uV: Voltage given by the lowest selector (if linear mapping)
bca7bbfff3 Mark Brown 2012-05-09 244 * @uV_step: Voltage increase with each selector (if linear mapping)
33234e791d Axel Lin 2012-11-27 245 * @linear_min_sel: Minimal selector for starting linear mapping
5a523605af Laxman Dewangan 2013-09-10 246 * @fixed_uV: Fixed voltage of rails.
ea38d13fd1 Axel Lin 2012-06-18 247 * @ramp_delay: Time to settle down after voltage change (unit: uV/us)
5abe4f223e Sascha Hauer 2015-10-13 248 * @min_dropout_uV: The minimum dropout voltage this regulator can handle
a8dbfeedfe Randy Dunlap 2014-08-27 249 * @linear_ranges: A constant table of possible voltage ranges.
a8dbfeedfe Randy Dunlap 2014-08-27 250 * @n_linear_ranges: Number of entries in the @linear_ranges table.
cffc9592fd Axel Lin 2012-05-20 251 * @volt_table: Voltage mapping table (if table based mapping)
bca7bbfff3 Mark Brown 2012-05-09 252 *
4ab5b3d92c Mark Brown 2012-04-15 253 * @vsel_reg: Register for selector when using regulator_regmap_X_voltage_
4ab5b3d92c Mark Brown 2012-04-15 254 * @vsel_mask: Mask for register bitfield used for selector
c8520b4c5d Axel Lin 2012-12-18 255 * @apply_reg: Register for initiate voltage change on the output when
c8520b4c5d Axel Lin 2012-12-18 256 * using regulator_set_voltage_sel_regmap
c8520b4c5d Axel Lin 2012-12-18 257 * @apply_bit: Register bitfield used for initiate voltage change on the
c8520b4c5d Axel Lin 2012-12-18 258 * output when using regulator_set_voltage_sel_regmap
cd6dffb4c6 Mark Brown 2012-04-15 259 * @enable_reg: Register for control when using regmap enable/disable ops
cd6dffb4c6 Mark Brown 2012-04-15 260 * @enable_mask: Mask for control when using regmap enable/disable ops
ca5d1b3524 Carlo Caione 2014-03-05 261 * @enable_val: Enabling value for control when using regmap enable/disable ops
ca5d1b3524 Carlo Caione 2014-03-05 262 * @disable_val: Disabling value for control when using regmap enable/disable ops
51dcdafcb7 Axel Lin 2013-03-05 263 * @enable_is_inverted: A flag to indicate set enable_mask bits to disable
51dcdafcb7 Axel Lin 2013-03-05 264 * when using regulator_enable_regmap and friends APIs.
5838b032fd Nishanth Menon 2013-02-28 265 * @bypass_reg: Register for control when using regmap set_bypass
5838b032fd Nishanth Menon 2013-02-28 266 * @bypass_mask: Mask for control when using regmap set_bypass
ca5d1b3524 Carlo Caione 2014-03-05 267 * @bypass_val_on: Enabling value for control when using regmap set_bypass
ca5d1b3524 Carlo Caione 2014-03-05 268 * @bypass_val_off: Disabling value for control when using regmap set_bypass
79511ed322 Mark Brown 2012-06-27 269 *
79511ed322 Mark Brown 2012-06-27 270 * @enable_time: Time taken for initial enable of regulator (in uS).
871f565055 Guodong Xu 2014-08-13 271 * @off_on_delay: guard time (in uS), before re-enabling a regulator
87e1e0f29f Javier Martinez Canillas 2014-11-10 272 *
87e1e0f29f Javier Martinez Canillas 2014-11-10 273 * @of_map_mode: Maps a hardware mode defined in a DeviceTree to a standard mode
571a354b15 Liam Girdwood 2008-04-30 274 */
571a354b15 Liam Girdwood 2008-04-30 275 struct regulator_desc {
571a354b15 Liam Girdwood 2008-04-30 276 const char *name;
69511a452e Rajendra Nayak 2011-11-18 277 const char *supply_name;
a0c7b164ad Mark Brown 2014-09-09 278 const char *of_match;
a0c7b164ad Mark Brown 2014-09-09 279 const char *regulators_node;
bfa21a0dfe Krzysztof Kozlowski 2015-01-05 280 int (*of_parse_cb)(struct device_node *,
bfa21a0dfe Krzysztof Kozlowski 2015-01-05 281 const struct regulator_desc *,
bfa21a0dfe Krzysztof Kozlowski 2015-01-05 282 struct regulator_config *);
571a354b15 Liam Girdwood 2008-04-30 283 int id;
5105f4655a Mark Brown 2016-01-21 284 unsigned int continuous_voltage_range:1;
cfe771af73 Mark Brown 2016-01-21 285 unsigned int auto_runtime_pm:1;
4367cfdc7c David Brownell 2009-02-26 286 unsigned n_voltages;
df11e506d3 Axel Lin 2014-08-21 287 const struct regulator_ops *ops;
571a354b15 Liam Girdwood 2008-04-30 288 int irq;
571a354b15 Liam Girdwood 2008-04-30 289 enum regulator_type type;
571a354b15 Liam Girdwood 2008-04-30 290 struct module *owner;
4ab5b3d92c Mark Brown 2012-04-15 291
bca7bbfff3 Mark Brown 2012-05-09 292 unsigned int min_uV;
bca7bbfff3 Mark Brown 2012-05-09 293 unsigned int uV_step;
33234e791d Axel Lin 2012-11-27 294 unsigned int linear_min_sel;
5a523605af Laxman Dewangan 2013-09-10 295 int fixed_uV;
98a175b60f Yadwinder Singh Brar 2012-06-09 296 unsigned int ramp_delay;
5abe4f223e Sascha Hauer 2015-10-13 297 int min_dropout_uV;
bca7bbfff3 Mark Brown 2012-05-09 298
94d33c02c7 Mark Brown 2013-07-02 299 const struct regulator_linear_range *linear_ranges;
94d33c02c7 Mark Brown 2013-07-02 300 int n_linear_ranges;
94d33c02c7 Mark Brown 2013-07-02 301
cffc9592fd Axel Lin 2012-05-20 302 const unsigned int *volt_table;
cffc9592fd Axel Lin 2012-05-20 303
4ab5b3d92c Mark Brown 2012-04-15 304 unsigned int vsel_reg;
4ab5b3d92c Mark Brown 2012-04-15 305 unsigned int vsel_mask;
c0ea88b890 Nikita Kiryanov 2015-11-25 306 unsigned int csel_reg;
c0ea88b890 Nikita Kiryanov 2015-11-25 307 unsigned int csel_mask;
c8520b4c5d Axel Lin 2012-12-18 308 unsigned int apply_reg;
c8520b4c5d Axel Lin 2012-12-18 309 unsigned int apply_bit;
cd6dffb4c6 Mark Brown 2012-04-15 310 unsigned int enable_reg;
cd6dffb4c6 Mark Brown 2012-04-15 311 unsigned int enable_mask;
ca5d1b3524 Carlo Caione 2014-03-05 312 unsigned int enable_val;
ca5d1b3524 Carlo Caione 2014-03-05 313 unsigned int disable_val;
51dcdafcb7 Axel Lin 2013-03-05 314 bool enable_is_inverted;
df36793115 Mark Brown 2012-08-27 315 unsigned int bypass_reg;
df36793115 Mark Brown 2012-08-27 316 unsigned int bypass_mask;
ca5d1b3524 Carlo Caione 2014-03-05 317 unsigned int bypass_val_on;
ca5d1b3524 Carlo Caione 2014-03-05 318 unsigned int bypass_val_off;
79511ed322 Mark Brown 2012-06-27 319
79511ed322 Mark Brown 2012-06-27 320 unsigned int enable_time;
871f565055 Guodong Xu 2014-08-13 321
871f565055 Guodong Xu 2014-08-13 322 unsigned int off_on_delay;
87e1e0f29f Javier Martinez Canillas 2014-11-10 323
87e1e0f29f Javier Martinez Canillas 2014-11-10 324 unsigned int (*of_map_mode)(unsigned int mode);
571a354b15 Liam Girdwood 2008-04-30 @325 };
571a354b15 Liam Girdwood 2008-04-30 326
c172708d38 Mark Brown 2012-04-04 327 /**
c172708d38 Mark Brown 2012-04-04 328 * struct regulator_config - Dynamic regulator descriptor
:::::: The code at line 325 was first introduced by commit
:::::: 571a354b1542a274d88617e1f6703f3fe7a517f1 regulator: regulator driver interface
:::::: TO: Liam Girdwood <lg@opensource.wolfsonmicro.com>
:::::: CC: Liam Girdwood <lg@opensource.wolfsonmicro.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 6122 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] regulator: core: Use a bitfield for continuous_voltage_range
2016-01-21 20:24 [PATCH 1/2] regulator: core: Use a bitfield for continuous_voltage_range Mark Brown
2016-01-21 20:24 ` [PATCH 2/2] regulator: core: Provide per-regulator runtime PM support Mark Brown
@ 2016-01-22 19:15 ` Joe Perches
2016-01-22 21:31 ` Mark Brown
2016-04-21 16:11 ` Applied "regulator: core: Use a bitfield for continuous_voltage_range" to the regulator tree Mark Brown
2 siblings, 1 reply; 12+ messages in thread
From: Joe Perches @ 2016-01-22 19:15 UTC (permalink / raw)
To: Mark Brown, Paul Kocialkowski; +Cc: Liam Girdwood, linux-kernel
On Thu, 2016-01-21 at 20:24 +0000, Mark Brown wrote:
> Using a bitfield enables the compiler to lay out the structure more
> efficiently when we have other boolean flags since multiple values can
> be included in a single byte.
>
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
> include/linux/regulator/driver.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
> index 16ac9e108806..3ac0f306f033 100644
> --- a/include/linux/regulator/driver.h
> +++ b/include/linux/regulator/driver.h
> @@ -281,7 +281,7 @@ struct regulator_desc {
> const struct regulator_desc *,
> struct regulator_config *);
> int id;
> - bool continuous_voltage_range;
> + unsigned int continuous_voltage_range:1;
Is this really valuable?
There are already padding bytes that are unused
and adding a couple more bools would be space
cost-free and more readable.
I believe that read/write of bytes is also more
efficient on some architectures than bit field
read/modify/write uses.
> unsigned n_voltages;
> const struct regulator_ops *ops;
> int irq;
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] regulator: core: Use a bitfield for continuous_voltage_range
2016-01-22 19:15 ` [PATCH 1/2] regulator: core: Use a bitfield for continuous_voltage_range Joe Perches
@ 2016-01-22 21:31 ` Mark Brown
2016-01-22 21:42 ` Joe Perches
0 siblings, 1 reply; 12+ messages in thread
From: Mark Brown @ 2016-01-22 21:31 UTC (permalink / raw)
To: Joe Perches; +Cc: Paul Kocialkowski, Liam Girdwood, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 579 bytes --]
On Fri, Jan 22, 2016 at 11:15:28AM -0800, Joe Perches wrote:
> On Thu, 2016-01-21 at 20:24 +0000, Mark Brown wrote:
> > - bool continuous_voltage_range;
> > + unsigned int continuous_voltage_range:1;
> Is this really valuable?
> There are already padding bytes that are unused
> and adding a couple more bools would be space
> cost-free and more readable.
> I believe that read/write of bytes is also more
> efficient on some architectures than bit field
> read/modify/write uses.
It adds up when you get more flags and these are not in the least bit
performance sensitive.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] regulator: core: Use a bitfield for continuous_voltage_range
2016-01-22 21:31 ` Mark Brown
@ 2016-01-22 21:42 ` Joe Perches
2016-01-23 15:16 ` Mark Brown
0 siblings, 1 reply; 12+ messages in thread
From: Joe Perches @ 2016-01-22 21:42 UTC (permalink / raw)
To: Mark Brown; +Cc: Paul Kocialkowski, Liam Girdwood, linux-kernel
On Fri, 2016-01-22 at 21:31 +0000, Mark Brown wrote:
> On Fri, Jan 22, 2016 at 11:15:28AM -0800, Joe Perches wrote:
> > On Thu, 2016-01-21 at 20:24 +0000, Mark Brown wrote:
>
> > > - bool continuous_voltage_range;
> > > + unsigned int continuous_voltage_range:1;
>
> > Is this really valuable?
>
> > There are already padding bytes that are unused
> > and adding a couple more bools would be space
> > cost-free and more readable.
>
> > I believe that read/write of bytes is also more
> > efficient on some architectures than bit field
> > read/modify/write uses.
>
> It adds up when you get more flags and these are not in the least bit
> performance sensitive.
Sure, but intelligibility is useful too.
Do you expect to have more than 4 of these flags?
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] regulator: core: Use a bitfield for continuous_voltage_range
2016-01-22 21:42 ` Joe Perches
@ 2016-01-23 15:16 ` Mark Brown
0 siblings, 0 replies; 12+ messages in thread
From: Mark Brown @ 2016-01-23 15:16 UTC (permalink / raw)
To: Joe Perches; +Cc: Paul Kocialkowski, Liam Girdwood, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 456 bytes --]
On Fri, Jan 22, 2016 at 01:42:33PM -0800, Joe Perches wrote:
> On Fri, 2016-01-22 at 21:31 +0000, Mark Brown wrote:
> > It adds up when you get more flags and these are not in the least bit
> > performance sensitive.
> Sure, but intelligibility is useful too.
I'm really not sure the use of small integers for boolean flags is going
to be an especially troubling barrier for people.
> Do you expect to have more than 4 of these flags?
It's plausible.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] regulator: core: Provide per-regulator runtime PM support
2016-01-21 20:24 ` [PATCH 2/2] regulator: core: Provide per-regulator runtime PM support Mark Brown
2016-01-21 21:49 ` kbuild test robot
@ 2016-01-29 12:02 ` Paul Kocialkowski
2016-02-09 20:51 ` Paul Kocialkowski
2 siblings, 0 replies; 12+ messages in thread
From: Paul Kocialkowski @ 2016-01-29 12:02 UTC (permalink / raw)
To: Mark Brown; +Cc: Liam Girdwood, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 3394 bytes --]
Le jeudi 21 janvier 2016 à 20:24 +0000, Mark Brown a écrit :
> Provide a flag auto_runtime_pm in the regulator_desc which causes the
> regulator core to take a runtime PM reference to a regulator while it
> is enabled. This helps integration with chip wide power management
> for
> auxiliary PMICs, they may be able to implement chip wide power
> savings
> if nothing on the PMIC is in use.
Thanks for working on this!
I'm having a major drawback on my development unit (fried it
accidentally) so I'm unable to test this with the LP8720 regulator on
the Optimus Black, but I'll manage to get a working development unit
soon.
I'll let you know how it goes. Feel free to merge this before I do the
work on the lp8720 regulator, though.
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>
> Not tested at all yet, pushing out for testing by others who have
> devices that could benefit from this.
>
> drivers/regulator/core.c | 20 ++++++++++++++++++--
> include/linux/regulator/driver.h | 1 +
> 2 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
> index 3308c6bb83db..968ff3081e6c 100644
> --- a/drivers/regulator/core.c
> +++ b/drivers/regulator/core.c
> @@ -26,6 +26,7 @@
> #include <linux/gpio.h>
> #include <linux/gpio/consumer.h>
> #include <linux/of.h>
> +#include <linux/pm_runtime.h>
> #include <linux/regmap.h>
> #include <linux/regulator/of_regulator.h>
> #include <linux/regulator/consumer.h>
> @@ -2059,17 +2060,23 @@ static int _regulator_do_enable(struct
> regulator_dev *rdev)
> }
> }
>
> + if (rdev->desc->auto_runtime_pm) {
> + ret = pm_runtime_get_sync(rdev->dev.parent);
> + if (ret < 0)
> + goto err;
> + }
> +
> if (rdev->ena_pin) {
> if (!rdev->ena_gpio_state) {
> ret = regulator_ena_gpio_ctrl(rdev, true);
> if (ret < 0)
> - return ret;
> + goto err_pm;
> rdev->ena_gpio_state = 1;
> }
> } else if (rdev->desc->ops->enable) {
> ret = rdev->desc->ops->enable(rdev);
> if (ret < 0)
> - return ret;
> + goto err_pm;
> } else {
> return -EINVAL;
> }
> @@ -2084,6 +2091,12 @@ static int _regulator_do_enable(struct
> regulator_dev *rdev)
> trace_regulator_enable_complete(rdev_get_name(rdev));
>
> return 0;
> +
> +err_pm:
> + if (rdev->desc->auto_runtime_pm)
> + pm_runtime_put_autosuspend(rdev->dev.parent);
> +err:
> + return ret;
> }
>
> /* locks held by regulator_enable() */
> @@ -2177,6 +2190,9 @@ static int _regulator_do_disable(struct
> regulator_dev *rdev)
> return ret;
> }
>
> + if (rdev->desc->auto_runtime_pm)
> + pm_runtime_put_autosuspend(rdev->dev.parent);
> +
> /* cares about last_off_jiffy only if off_on_delay is
> required by
> * device.
> */
> diff --git a/include/linux/regulator/driver.h
> b/include/linux/regulator/driver.h
> index 3ac0f306f033..dccea032a143 100644
> --- a/include/linux/regulator/driver.h
> +++ b/include/linux/regulator/driver.h
> @@ -282,6 +282,7 @@ struct regulator_desc {
> struct regulator_config *);
> int id;
> unsigned int continuous_voltage_range:1;
> + unsigned int auto_runtime_pm:1;
> unsigned n_voltages;
> const struct regulator_ops *ops;
> int irq;
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] regulator: core: Provide per-regulator runtime PM support
2016-01-21 20:24 ` [PATCH 2/2] regulator: core: Provide per-regulator runtime PM support Mark Brown
2016-01-21 21:49 ` kbuild test robot
2016-01-29 12:02 ` Paul Kocialkowski
@ 2016-02-09 20:51 ` Paul Kocialkowski
2016-02-12 19:24 ` Paul Kocialkowski
2 siblings, 1 reply; 12+ messages in thread
From: Paul Kocialkowski @ 2016-02-09 20:51 UTC (permalink / raw)
To: Mark Brown; +Cc: Liam Girdwood, linux-kernel, Rafael J. Wysocki
[-- Attachment #1: Type: text/plain, Size: 11141 bytes --]
Hi,
Le jeudi 21 janvier 2016 à 20:24 +0000, Mark Brown a écrit :
> Provide a flag auto_runtime_pm in the regulator_desc which causes the
> regulator core to take a runtime PM reference to a regulator while it
> is enabled. This helps integration with chip wide power management for
> auxiliary PMICs, they may be able to implement chip wide power savings
> if nothing on the PMIC is in use.
>
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>
> Not tested at all yet, pushing out for testing by others who have
> devices that could benefit from this.
Thanks for pushing this, I didn't think it would be that easy to
implement.
I have tried using those patches on top of my Optimus Black support
series (as of v2), on top of 4.5.0-rc2. As-is, it fails at run-time, due
to badly-ordered calls to runtime pm functions. Here is the relevant
part of the kernel log:
[ 1.943359] regulator disable, pm autosuspend
[ 1.949615] regulator enable, pm sync
[ 1.953491] lp872x_runtime_resume() 37
[ 1.957519] lp872x_runtime_suspend() 37
Despite having the regulator disabled and enabled in that precise order,
the runtime functions are called in the opposite order, which causes the
enable pin (and thus the whole regulator chip) to be disabled.
Is that behavior intended? I suppose this is not an issue at the
regulator core level.
The diff I used to produce this is as follows. Perhaps I'm doing
something wrong?
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index e39c75d..416701d 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2061,6 +2061,7 @@ static int _regulator_do_enable(struct
regulator_dev *rdev)
}
if (rdev->desc->auto_runtime_pm) {
+ printk(KERN_ERR "regulator enable, pm sync\n");
ret = pm_runtime_get_sync(rdev->dev.parent);
if (ret < 0)
goto err;
@@ -2190,8 +2191,10 @@ static int _regulator_do_disable(struct
regulator_dev *rdev)
return ret;
}
- if (rdev->desc->auto_runtime_pm)
+ if (rdev->desc->auto_runtime_pm) {
+ printk(KERN_ERR "regulator disable, pm autosuspend\n");
pm_runtime_put_autosuspend(rdev->dev.parent);
+ }
/* cares about last_off_jiffy only if off_on_delay is required
by
* device.
diff --git a/drivers/regulator/lp872x.c b/drivers/regulator/lp872x.c
index 3e74861..5b99a34 100644
--- a/drivers/regulator/lp872x.c
+++ b/drivers/regulator/lp872x.c
@@ -15,6 +15,9 @@
#include <linux/regmap.h>
#include <linux/err.h>
#include <linux/gpio.h>
+#include <linux/delay.h>
+#include <linux/pm.h>
+#include <linux/pm_runtime.h>
#include <linux/regulator/lp872x.h>
#include <linux/regulator/driver.h>
#include <linux/platform_device.h>
@@ -522,6 +525,7 @@ static struct regulator_desc lp8720_regulator_desc[]
= {
.of_match = of_match_ptr("ldo1"),
.id = LP8720_ID_LDO1,
.ops = &lp872x_ldo_ops,
+ .auto_runtime_pm = 1,
.n_voltages = ARRAY_SIZE(lp872x_ldo_vtbl),
.volt_table = lp872x_ldo_vtbl,
.type = REGULATOR_VOLTAGE,
@@ -536,6 +540,7 @@ static struct regulator_desc lp8720_regulator_desc[]
= {
.of_match = of_match_ptr("ldo2"),
.id = LP8720_ID_LDO2,
.ops = &lp872x_ldo_ops,
+ .auto_runtime_pm = 1,
.n_voltages = ARRAY_SIZE(lp872x_ldo_vtbl),
.volt_table = lp872x_ldo_vtbl,
.type = REGULATOR_VOLTAGE,
@@ -550,6 +555,7 @@ static struct regulator_desc lp8720_regulator_desc[]
= {
.of_match = of_match_ptr("ldo3"),
.id = LP8720_ID_LDO3,
.ops = &lp872x_ldo_ops,
+ .auto_runtime_pm = 1,
.n_voltages = ARRAY_SIZE(lp872x_ldo_vtbl),
.volt_table = lp872x_ldo_vtbl,
.type = REGULATOR_VOLTAGE,
@@ -564,6 +570,7 @@ static struct regulator_desc lp8720_regulator_desc[]
= {
.of_match = of_match_ptr("ldo4"),
.id = LP8720_ID_LDO4,
.ops = &lp872x_ldo_ops,
+ .auto_runtime_pm = 1,
.n_voltages = ARRAY_SIZE(lp8720_ldo4_vtbl),
.volt_table = lp8720_ldo4_vtbl,
.type = REGULATOR_VOLTAGE,
@@ -578,6 +585,7 @@ static struct regulator_desc lp8720_regulator_desc[]
= {
.of_match = of_match_ptr("ldo5"),
.id = LP8720_ID_LDO5,
.ops = &lp872x_ldo_ops,
+ .auto_runtime_pm = 1,
.n_voltages = ARRAY_SIZE(lp872x_ldo_vtbl),
.volt_table = lp872x_ldo_vtbl,
.type = REGULATOR_VOLTAGE,
@@ -592,6 +600,7 @@ static struct regulator_desc lp8720_regulator_desc[]
= {
.of_match = of_match_ptr("buck"),
.id = LP8720_ID_BUCK,
.ops = &lp8720_buck_ops,
+ .auto_runtime_pm = 1,
.n_voltages = ARRAY_SIZE(lp8720_buck_vtbl),
.volt_table = lp8720_buck_vtbl,
.type = REGULATOR_VOLTAGE,
@@ -607,6 +616,7 @@ static struct regulator_desc lp8725_regulator_desc[]
= {
.of_match = of_match_ptr("ldo1"),
.id = LP8725_ID_LDO1,
.ops = &lp872x_ldo_ops,
+ .auto_runtime_pm = 1,
.n_voltages = ARRAY_SIZE(lp872x_ldo_vtbl),
.volt_table = lp872x_ldo_vtbl,
.type = REGULATOR_VOLTAGE,
@@ -621,6 +631,7 @@ static struct regulator_desc lp8725_regulator_desc[]
= {
.of_match = of_match_ptr("ldo2"),
.id = LP8725_ID_LDO2,
.ops = &lp872x_ldo_ops,
+ .auto_runtime_pm = 1,
.n_voltages = ARRAY_SIZE(lp872x_ldo_vtbl),
.volt_table = lp872x_ldo_vtbl,
.type = REGULATOR_VOLTAGE,
@@ -635,6 +646,7 @@ static struct regulator_desc lp8725_regulator_desc[]
= {
.of_match = of_match_ptr("ldo3"),
.id = LP8725_ID_LDO3,
.ops = &lp872x_ldo_ops,
+ .auto_runtime_pm = 1,
.n_voltages = ARRAY_SIZE(lp872x_ldo_vtbl),
.volt_table = lp872x_ldo_vtbl,
.type = REGULATOR_VOLTAGE,
@@ -649,6 +661,7 @@ static struct regulator_desc lp8725_regulator_desc[]
= {
.of_match = of_match_ptr("ldo4"),
.id = LP8725_ID_LDO4,
.ops = &lp872x_ldo_ops,
+ .auto_runtime_pm = 1,
.n_voltages = ARRAY_SIZE(lp872x_ldo_vtbl),
.volt_table = lp872x_ldo_vtbl,
.type = REGULATOR_VOLTAGE,
@@ -663,6 +676,7 @@ static struct regulator_desc lp8725_regulator_desc[]
= {
.of_match = of_match_ptr("ldo5"),
.id = LP8725_ID_LDO5,
.ops = &lp872x_ldo_ops,
+ .auto_runtime_pm = 1,
.n_voltages = ARRAY_SIZE(lp872x_ldo_vtbl),
.volt_table = lp872x_ldo_vtbl,
.type = REGULATOR_VOLTAGE,
@@ -677,6 +691,7 @@ static struct regulator_desc lp8725_regulator_desc[]
= {
.of_match = of_match_ptr("lilo1"),
.id = LP8725_ID_LILO1,
.ops = &lp872x_ldo_ops,
+ .auto_runtime_pm = 1,
.n_voltages = ARRAY_SIZE(lp8725_lilo_vtbl),
.volt_table = lp8725_lilo_vtbl,
.type = REGULATOR_VOLTAGE,
@@ -691,6 +706,7 @@ static struct regulator_desc lp8725_regulator_desc[]
= {
.of_match = of_match_ptr("lilo2"),
.id = LP8725_ID_LILO2,
.ops = &lp872x_ldo_ops,
+ .auto_runtime_pm = 1,
.n_voltages = ARRAY_SIZE(lp8725_lilo_vtbl),
.volt_table = lp8725_lilo_vtbl,
.type = REGULATOR_VOLTAGE,
@@ -705,6 +721,7 @@ static struct regulator_desc lp8725_regulator_desc[]
= {
.of_match = of_match_ptr("buck1"),
.id = LP8725_ID_BUCK1,
.ops = &lp8725_buck_ops,
+ .auto_runtime_pm = 1,
.n_voltages = ARRAY_SIZE(lp8725_buck_vtbl),
.volt_table = lp8725_buck_vtbl,
.type = REGULATOR_VOLTAGE,
@@ -717,6 +734,7 @@ static struct regulator_desc lp8725_regulator_desc[]
= {
.of_match = of_match_ptr("buck2"),
.id = LP8725_ID_BUCK2,
.ops = &lp8725_buck_ops,
+ .auto_runtime_pm = 1,
.n_voltages = ARRAY_SIZE(lp8725_buck_vtbl),
.volt_table = lp8725_buck_vtbl,
.type = REGULATOR_VOLTAGE,
@@ -985,9 +1003,63 @@ static int lp872x_probe(struct i2c_client *cl,
const struct i2c_device_id *id)
if (ret)
return ret;
- return lp872x_regulator_register(lp);
+ pm_runtime_enable(&cl->dev);
+
+ ret = lp872x_regulator_register(lp);
+ if (ret) {
+ pm_runtime_disable(&cl->dev);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int lp872x_runtime_suspend(struct device *dev)
+{
+ struct lp872x *lp = i2c_get_clientdata(to_i2c_client(dev));
+ struct gpio_desc *gpiod;
+ int gpio;
+
+ printk(KERN_ERR "%s() %d\n", __func__, lp->pdata->enable_gpio);
+
+ gpio = lp->pdata->enable_gpio;
+ if (!gpio_is_valid(gpio))
+ return 0;
+
+ gpiod = gpio_to_desc(gpio);
+ gpiod_set_value(gpiod, 0);
+
+ return 0;
+}
+
+static int lp872x_runtime_resume(struct device *dev)
+{
+ struct lp872x *lp = i2c_get_clientdata(to_i2c_client(dev));
+ struct gpio_desc *gpiod;
+ int gpio;
+
+ printk(KERN_ERR "%s() %d\n", __func__, lp->pdata->enable_gpio);
+
+ gpio = lp->pdata->enable_gpio;
+ if (!gpio_is_valid(gpio))
+ return 0;
+
+ gpiod = gpio_to_desc(gpio);
+ gpiod_set_value(gpiod, 0);
+
+ /* Each chip has a different enable delay. */
+ if (lp->chipid == LP8720)
+ usleep_range(LP8720_ENABLE_DELAY, 1.5 *
LP8720_ENABLE_DELAY);
+ else
+ usleep_range(LP8725_ENABLE_DELAY, 1.5 *
LP8725_ENABLE_DELAY);
+
+ return 0;
}
+static const struct dev_pm_ops lp872x_pm_ops = {
+ SET_RUNTIME_PM_OPS(lp872x_runtime_suspend,
lp872x_runtime_resume, NULL)
+};
+
static const struct of_device_id lp872x_dt_ids[] = {
{ .compatible = "ti,lp8720", },
{ .compatible = "ti,lp8725", },
@@ -1006,6 +1078,7 @@ static struct i2c_driver lp872x_driver = {
.driver = {
.name = "lp872x",
.of_match_table = of_match_ptr(lp872x_dt_ids),
+ .pm = &lp872x_pm_ops,
},
.probe = lp872x_probe,
.id_table = lp872x_ids,
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] regulator: core: Provide per-regulator runtime PM support
2016-02-09 20:51 ` Paul Kocialkowski
@ 2016-02-12 19:24 ` Paul Kocialkowski
2016-02-26 2:13 ` Mark Brown
0 siblings, 1 reply; 12+ messages in thread
From: Paul Kocialkowski @ 2016-02-12 19:24 UTC (permalink / raw)
To: Mark Brown; +Cc: Liam Girdwood, linux-kernel, Rafael J. Wysocki
[-- Attachment #1: Type: text/plain, Size: 13257 bytes --]
Hi,
Le mardi 09 février 2016 à 21:51 +0100, Paul Kocialkowski a écrit :
> Le jeudi 21 janvier 2016 à 20:24 +0000, Mark Brown a écrit :
> > Provide a flag auto_runtime_pm in the regulator_desc which causes the
> > regulator core to take a runtime PM reference to a regulator while it
> > is enabled. This helps integration with chip wide power management for
> > auxiliary PMICs, they may be able to implement chip wide power savings
> > if nothing on the PMIC is in use.
> >
> > Signed-off-by: Mark Brown <broonie@kernel.org>
> > ---
> >
> > Not tested at all yet, pushing out for testing by others who have
> > devices that could benefit from this.
>
> Thanks for pushing this, I didn't think it would be that easy to
> implement.
>
> I have tried using those patches on top of my Optimus Black support
> series (as of v2), on top of 4.5.0-rc2. As-is, it fails at run-time, due
> to badly-ordered calls to runtime pm functions. Here is the relevant
> part of the kernel log:
>
> [ 1.943359] regulator disable, pm autosuspend
> [ 1.949615] regulator enable, pm sync
> [ 1.953491] lp872x_runtime_resume() 37
> [ 1.957519] lp872x_runtime_suspend() 37
>
> Despite having the regulator disabled and enabled in that precise order,
> the runtime functions are called in the opposite order, which causes the
> enable pin (and thus the whole regulator chip) to be disabled.
>
> Is that behavior intended? I suppose this is not an issue at the
> regulator core level.
I have added more tracing today. Here is some more context:
[ 2.016510] omap_hsmmc 4809c000.mmc: Got CD GPIO
[ 2.021697] omap_hsmmc 4809c000.mmc: omap_device:
omap_device_enable() called from invalid state 1
[ 2.031616] omap_hsmmc_disable_boot_regulators() vmmc disable
[ 2.038177] omap_hsmmc_disable_boot_regulator(): regulator enable
[ 2.045104] omap_hsmmc_disable_boot_regulator(): regulator disable
[ 2.052368] regulator disable, pm autosuspend
[ 2.056976] omap_hsmmc_disable_boot_regulators() vqmmc disable
[ 2.063110] omap_hsmmc_disable_boot_regulators() pbias disable
[ 2.069244] omap_hsmmc_disable_boot_regulator(): regulator enable
[ 2.075653] omap_hsmmc_disable_boot_regulator(): regulator disable
[ 2.082397] omap_hsmmc_enable_supply(): vmmc regulator enable
[ 2.088958] mmc_regulator_set_ocr(): regulator enable
[ 2.095764] regulator enable, pm sync
[ 2.099639] lp872x_runtime_resume() 37
[ 2.104309] lp872x_runtime_suspend() 37
This is mostly the omap_hsmmc driver enabling and disabling the
regulator to sort out the boot state (in
omap_hsmmc_disable_boot_regulators). There is a comment in that function
that explains this behavior.
Apparently, the first enable is not acted upon by the regulator core as
the regulator is enabled at boot.
So I think the question here is why runtime pm is calling the device
functions in reverse order. Any clue? Is that intended behavior?
> The diff I used to produce this is as follows. Perhaps I'm doing
> something wrong?
>
> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
> index e39c75d..416701d 100644
> --- a/drivers/regulator/core.c
> +++ b/drivers/regulator/core.c
> @@ -2061,6 +2061,7 @@ static int _regulator_do_enable(struct
> regulator_dev *rdev)
> }
>
> if (rdev->desc->auto_runtime_pm) {
> + printk(KERN_ERR "regulator enable, pm sync\n");
> ret = pm_runtime_get_sync(rdev->dev.parent);
> if (ret < 0)
> goto err;
> @@ -2190,8 +2191,10 @@ static int _regulator_do_disable(struct
> regulator_dev *rdev)
> return ret;
> }
>
> - if (rdev->desc->auto_runtime_pm)
> + if (rdev->desc->auto_runtime_pm) {
> + printk(KERN_ERR "regulator disable, pm autosuspend\n");
> pm_runtime_put_autosuspend(rdev->dev.parent);
> + }
>
> /* cares about last_off_jiffy only if off_on_delay is required
> by
> * device.
> diff --git a/drivers/regulator/lp872x.c b/drivers/regulator/lp872x.c
> index 3e74861..5b99a34 100644
> --- a/drivers/regulator/lp872x.c
> +++ b/drivers/regulator/lp872x.c
> @@ -15,6 +15,9 @@
> #include <linux/regmap.h>
> #include <linux/err.h>
> #include <linux/gpio.h>
> +#include <linux/delay.h>
> +#include <linux/pm.h>
> +#include <linux/pm_runtime.h>
> #include <linux/regulator/lp872x.h>
> #include <linux/regulator/driver.h>
> #include <linux/platform_device.h>
> @@ -522,6 +525,7 @@ static struct regulator_desc lp8720_regulator_desc[]
> = {
> .of_match = of_match_ptr("ldo1"),
> .id = LP8720_ID_LDO1,
> .ops = &lp872x_ldo_ops,
> + .auto_runtime_pm = 1,
> .n_voltages = ARRAY_SIZE(lp872x_ldo_vtbl),
> .volt_table = lp872x_ldo_vtbl,
> .type = REGULATOR_VOLTAGE,
> @@ -536,6 +540,7 @@ static struct regulator_desc lp8720_regulator_desc[]
> = {
> .of_match = of_match_ptr("ldo2"),
> .id = LP8720_ID_LDO2,
> .ops = &lp872x_ldo_ops,
> + .auto_runtime_pm = 1,
> .n_voltages = ARRAY_SIZE(lp872x_ldo_vtbl),
> .volt_table = lp872x_ldo_vtbl,
> .type = REGULATOR_VOLTAGE,
> @@ -550,6 +555,7 @@ static struct regulator_desc lp8720_regulator_desc[]
> = {
> .of_match = of_match_ptr("ldo3"),
> .id = LP8720_ID_LDO3,
> .ops = &lp872x_ldo_ops,
> + .auto_runtime_pm = 1,
> .n_voltages = ARRAY_SIZE(lp872x_ldo_vtbl),
> .volt_table = lp872x_ldo_vtbl,
> .type = REGULATOR_VOLTAGE,
> @@ -564,6 +570,7 @@ static struct regulator_desc lp8720_regulator_desc[]
> = {
> .of_match = of_match_ptr("ldo4"),
> .id = LP8720_ID_LDO4,
> .ops = &lp872x_ldo_ops,
> + .auto_runtime_pm = 1,
> .n_voltages = ARRAY_SIZE(lp8720_ldo4_vtbl),
> .volt_table = lp8720_ldo4_vtbl,
> .type = REGULATOR_VOLTAGE,
> @@ -578,6 +585,7 @@ static struct regulator_desc lp8720_regulator_desc[]
> = {
> .of_match = of_match_ptr("ldo5"),
> .id = LP8720_ID_LDO5,
> .ops = &lp872x_ldo_ops,
> + .auto_runtime_pm = 1,
> .n_voltages = ARRAY_SIZE(lp872x_ldo_vtbl),
> .volt_table = lp872x_ldo_vtbl,
> .type = REGULATOR_VOLTAGE,
> @@ -592,6 +600,7 @@ static struct regulator_desc lp8720_regulator_desc[]
> = {
> .of_match = of_match_ptr("buck"),
> .id = LP8720_ID_BUCK,
> .ops = &lp8720_buck_ops,
> + .auto_runtime_pm = 1,
> .n_voltages = ARRAY_SIZE(lp8720_buck_vtbl),
> .volt_table = lp8720_buck_vtbl,
> .type = REGULATOR_VOLTAGE,
> @@ -607,6 +616,7 @@ static struct regulator_desc lp8725_regulator_desc[]
> = {
> .of_match = of_match_ptr("ldo1"),
> .id = LP8725_ID_LDO1,
> .ops = &lp872x_ldo_ops,
> + .auto_runtime_pm = 1,
> .n_voltages = ARRAY_SIZE(lp872x_ldo_vtbl),
> .volt_table = lp872x_ldo_vtbl,
> .type = REGULATOR_VOLTAGE,
> @@ -621,6 +631,7 @@ static struct regulator_desc lp8725_regulator_desc[]
> = {
> .of_match = of_match_ptr("ldo2"),
> .id = LP8725_ID_LDO2,
> .ops = &lp872x_ldo_ops,
> + .auto_runtime_pm = 1,
> .n_voltages = ARRAY_SIZE(lp872x_ldo_vtbl),
> .volt_table = lp872x_ldo_vtbl,
> .type = REGULATOR_VOLTAGE,
> @@ -635,6 +646,7 @@ static struct regulator_desc lp8725_regulator_desc[]
> = {
> .of_match = of_match_ptr("ldo3"),
> .id = LP8725_ID_LDO3,
> .ops = &lp872x_ldo_ops,
> + .auto_runtime_pm = 1,
> .n_voltages = ARRAY_SIZE(lp872x_ldo_vtbl),
> .volt_table = lp872x_ldo_vtbl,
> .type = REGULATOR_VOLTAGE,
> @@ -649,6 +661,7 @@ static struct regulator_desc lp8725_regulator_desc[]
> = {
> .of_match = of_match_ptr("ldo4"),
> .id = LP8725_ID_LDO4,
> .ops = &lp872x_ldo_ops,
> + .auto_runtime_pm = 1,
> .n_voltages = ARRAY_SIZE(lp872x_ldo_vtbl),
> .volt_table = lp872x_ldo_vtbl,
> .type = REGULATOR_VOLTAGE,
> @@ -663,6 +676,7 @@ static struct regulator_desc lp8725_regulator_desc[]
> = {
> .of_match = of_match_ptr("ldo5"),
> .id = LP8725_ID_LDO5,
> .ops = &lp872x_ldo_ops,
> + .auto_runtime_pm = 1,
> .n_voltages = ARRAY_SIZE(lp872x_ldo_vtbl),
> .volt_table = lp872x_ldo_vtbl,
> .type = REGULATOR_VOLTAGE,
> @@ -677,6 +691,7 @@ static struct regulator_desc lp8725_regulator_desc[]
> = {
> .of_match = of_match_ptr("lilo1"),
> .id = LP8725_ID_LILO1,
> .ops = &lp872x_ldo_ops,
> + .auto_runtime_pm = 1,
> .n_voltages = ARRAY_SIZE(lp8725_lilo_vtbl),
> .volt_table = lp8725_lilo_vtbl,
> .type = REGULATOR_VOLTAGE,
> @@ -691,6 +706,7 @@ static struct regulator_desc lp8725_regulator_desc[]
> = {
> .of_match = of_match_ptr("lilo2"),
> .id = LP8725_ID_LILO2,
> .ops = &lp872x_ldo_ops,
> + .auto_runtime_pm = 1,
> .n_voltages = ARRAY_SIZE(lp8725_lilo_vtbl),
> .volt_table = lp8725_lilo_vtbl,
> .type = REGULATOR_VOLTAGE,
> @@ -705,6 +721,7 @@ static struct regulator_desc lp8725_regulator_desc[]
> = {
> .of_match = of_match_ptr("buck1"),
> .id = LP8725_ID_BUCK1,
> .ops = &lp8725_buck_ops,
> + .auto_runtime_pm = 1,
> .n_voltages = ARRAY_SIZE(lp8725_buck_vtbl),
> .volt_table = lp8725_buck_vtbl,
> .type = REGULATOR_VOLTAGE,
> @@ -717,6 +734,7 @@ static struct regulator_desc lp8725_regulator_desc[]
> = {
> .of_match = of_match_ptr("buck2"),
> .id = LP8725_ID_BUCK2,
> .ops = &lp8725_buck_ops,
> + .auto_runtime_pm = 1,
> .n_voltages = ARRAY_SIZE(lp8725_buck_vtbl),
> .volt_table = lp8725_buck_vtbl,
> .type = REGULATOR_VOLTAGE,
> @@ -985,9 +1003,63 @@ static int lp872x_probe(struct i2c_client *cl,
> const struct i2c_device_id *id)
> if (ret)
> return ret;
>
> - return lp872x_regulator_register(lp);
> + pm_runtime_enable(&cl->dev);
> +
> + ret = lp872x_regulator_register(lp);
> + if (ret) {
> + pm_runtime_disable(&cl->dev);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int lp872x_runtime_suspend(struct device *dev)
> +{
> + struct lp872x *lp = i2c_get_clientdata(to_i2c_client(dev));
> + struct gpio_desc *gpiod;
> + int gpio;
> +
> + printk(KERN_ERR "%s() %d\n", __func__, lp->pdata->enable_gpio);
> +
> + gpio = lp->pdata->enable_gpio;
> + if (!gpio_is_valid(gpio))
> + return 0;
> +
> + gpiod = gpio_to_desc(gpio);
> + gpiod_set_value(gpiod, 0);
> +
> + return 0;
> +}
> +
> +static int lp872x_runtime_resume(struct device *dev)
> +{
> + struct lp872x *lp = i2c_get_clientdata(to_i2c_client(dev));
> + struct gpio_desc *gpiod;
> + int gpio;
> +
> + printk(KERN_ERR "%s() %d\n", __func__, lp->pdata->enable_gpio);
> +
> + gpio = lp->pdata->enable_gpio;
> + if (!gpio_is_valid(gpio))
> + return 0;
> +
> + gpiod = gpio_to_desc(gpio);
> + gpiod_set_value(gpiod, 0);
> +
> + /* Each chip has a different enable delay. */
> + if (lp->chipid == LP8720)
> + usleep_range(LP8720_ENABLE_DELAY, 1.5 *
> LP8720_ENABLE_DELAY);
> + else
> + usleep_range(LP8725_ENABLE_DELAY, 1.5 *
> LP8725_ENABLE_DELAY);
> +
> + return 0;
> }
>
> +static const struct dev_pm_ops lp872x_pm_ops = {
> + SET_RUNTIME_PM_OPS(lp872x_runtime_suspend,
> lp872x_runtime_resume, NULL)
> +};
> +
> static const struct of_device_id lp872x_dt_ids[] = {
> { .compatible = "ti,lp8720", },
> { .compatible = "ti,lp8725", },
> @@ -1006,6 +1078,7 @@ static struct i2c_driver lp872x_driver = {
> .driver = {
> .name = "lp872x",
> .of_match_table = of_match_ptr(lp872x_dt_ids),
> + .pm = &lp872x_pm_ops,
> },
> .probe = lp872x_probe,
> .id_table = lp872x_ids,
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] regulator: core: Provide per-regulator runtime PM support
2016-02-12 19:24 ` Paul Kocialkowski
@ 2016-02-26 2:13 ` Mark Brown
0 siblings, 0 replies; 12+ messages in thread
From: Mark Brown @ 2016-02-26 2:13 UTC (permalink / raw)
To: Paul Kocialkowski; +Cc: Liam Girdwood, linux-kernel, Rafael J. Wysocki
[-- Attachment #1: Type: text/plain, Size: 244 bytes --]
On Fri, Feb 12, 2016 at 08:24:11PM +0100, Paul Kocialkowski wrote:
> So I think the question here is why runtime pm is calling the device
> functions in reverse order. Any clue? Is that intended behavior?
Definitely not intended behaviour...
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Applied "regulator: core: Use a bitfield for continuous_voltage_range" to the regulator tree
2016-01-21 20:24 [PATCH 1/2] regulator: core: Use a bitfield for continuous_voltage_range Mark Brown
2016-01-21 20:24 ` [PATCH 2/2] regulator: core: Provide per-regulator runtime PM support Mark Brown
2016-01-22 19:15 ` [PATCH 1/2] regulator: core: Use a bitfield for continuous_voltage_range Joe Perches
@ 2016-04-21 16:11 ` Mark Brown
2 siblings, 0 replies; 12+ messages in thread
From: Mark Brown @ 2016-04-21 16:11 UTC (permalink / raw)
To: Mark Brown; +Cc: Paul Kocialkowski, Liam Girdwood, linux-kernel
The patch
regulator: core: Use a bitfield for continuous_voltage_range
has been applied to the regulator tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
>From de4a54c4dfaed0604565c1b27488dce56997acc0 Mon Sep 17 00:00:00 2001
From: Mark Brown <broonie@kernel.org>
Date: Thu, 21 Jan 2016 20:19:41 +0000
Subject: [PATCH] regulator: core: Use a bitfield for continuous_voltage_range
Using a bitfield enables the compiler to lay out the structure more
efficiently when we have other boolean flags since multiple values can
be included in a single byte.
Signed-off-by: Mark Brown <broonie@kernel.org>
---
include/linux/regulator/driver.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index cd271e89a7e6..9ac3f9879576 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -292,7 +292,7 @@ struct regulator_desc {
const struct regulator_desc *,
struct regulator_config *);
int id;
- bool continuous_voltage_range;
+ unsigned int continuous_voltage_range:1;
unsigned n_voltages;
const struct regulator_ops *ops;
int irq;
--
2.8.0.rc3
^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2016-04-21 16:12 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-21 20:24 [PATCH 1/2] regulator: core: Use a bitfield for continuous_voltage_range Mark Brown
2016-01-21 20:24 ` [PATCH 2/2] regulator: core: Provide per-regulator runtime PM support Mark Brown
2016-01-21 21:49 ` kbuild test robot
2016-01-29 12:02 ` Paul Kocialkowski
2016-02-09 20:51 ` Paul Kocialkowski
2016-02-12 19:24 ` Paul Kocialkowski
2016-02-26 2:13 ` Mark Brown
2016-01-22 19:15 ` [PATCH 1/2] regulator: core: Use a bitfield for continuous_voltage_range Joe Perches
2016-01-22 21:31 ` Mark Brown
2016-01-22 21:42 ` Joe Perches
2016-01-23 15:16 ` Mark Brown
2016-04-21 16:11 ` Applied "regulator: core: Use a bitfield for continuous_voltage_range" to the regulator tree Mark Brown
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.