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


Groups > linux.kernel > #1314493 > unrolled thread

[PATCH 1/2] regulator: core: Use a bitfield for continuous_voltage_range

Started byMark Brown <broonie@kernel.org>
First post2016-01-21 21:30 +0100
Last post2016-01-23 16:20 +0100
Articles 7 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH 1/2] regulator: core: Use a bitfield for continuous_voltage_range Mark Brown <broonie@kernel.org> - 2016-01-21 21:30 +0100
    [PATCH 2/2] regulator: core: Provide per-regulator runtime PM support Mark Brown <broonie@kernel.org> - 2016-01-21 21:30 +0100
      Re: [PATCH 2/2] regulator: core: Provide per-regulator runtime PM  support kbuild test robot <lkp@intel.com> - 2016-01-21 23:00 +0100
    Re: [PATCH 1/2] regulator: core: Use a bitfield for  continuous_voltage_range Joe Perches <joe@perches.com> - 2016-01-22 20:20 +0100
      Re: [PATCH 1/2] regulator: core: Use a bitfield for  continuous_voltage_range Mark Brown <broonie@kernel.org> - 2016-01-22 22:40 +0100
        Re: [PATCH 1/2] regulator: core: Use a bitfield for  continuous_voltage_range Joe Perches <joe@perches.com> - 2016-01-22 22:50 +0100
          Re: [PATCH 1/2] regulator: core: Use a bitfield for  continuous_voltage_range Mark Brown <broonie@kernel.org> - 2016-01-23 16:20 +0100

#1314493 — [PATCH 1/2] regulator: core: Use a bitfield for continuous_voltage_range

FromMark Brown <broonie@kernel.org>
Date2016-01-21 21:30 +0100
Subject[PATCH 1/2] regulator: core: Use a bitfield for continuous_voltage_range
Message-ID<qTu1H-1V0-1@gated-at.bofh.it>
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

[toc] | [next] | [standalone]


#1314500 — [PATCH 2/2] regulator: core: Provide per-regulator runtime PM support

FromMark Brown <broonie@kernel.org>
Date2016-01-21 21:30 +0100
Subject[PATCH 2/2] regulator: core: Provide per-regulator runtime PM support
Message-ID<qTu1I-1V0-17@gated-at.bofh.it>
In reply to#1314493
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

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


#1314561 — Re: [PATCH 2/2] regulator: core: Provide per-regulator runtime PM support

Fromkbuild test robot <lkp@intel.com>
Date2016-01-21 23:00 +0100
SubjectRe: [PATCH 2/2] regulator: core: Provide per-regulator runtime PM support
Message-ID<qTvqP-2KT-27@gated-at.bofh.it>
In reply to#1314500

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

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

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


#1315229 — Re: [PATCH 1/2] regulator: core: Use a bitfield for continuous_voltage_range

FromJoe Perches <joe@perches.com>
Date2016-01-22 20:20 +0100
SubjectRe: [PATCH 1/2] regulator: core: Use a bitfield for continuous_voltage_range
Message-ID<qTPpw-8pj-3@gated-at.bofh.it>
In reply to#1314493
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;

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


#1315297 — Re: [PATCH 1/2] regulator: core: Use a bitfield for continuous_voltage_range

FromMark Brown <broonie@kernel.org>
Date2016-01-22 22:40 +0100
SubjectRe: [PATCH 1/2] regulator: core: Use a bitfield for continuous_voltage_range
Message-ID<qTRB1-1oj-33@gated-at.bofh.it>
In reply to#1315229

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

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.

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


#1315304 — Re: [PATCH 1/2] regulator: core: Use a bitfield for continuous_voltage_range

FromJoe Perches <joe@perches.com>
Date2016-01-22 22:50 +0100
SubjectRe: [PATCH 1/2] regulator: core: Use a bitfield for continuous_voltage_range
Message-ID<qTRKG-1ss-9@gated-at.bofh.it>
In reply to#1315297
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?

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


#1315659 — Re: [PATCH 1/2] regulator: core: Use a bitfield for continuous_voltage_range

FromMark Brown <broonie@kernel.org>
Date2016-01-23 16:20 +0100
SubjectRe: [PATCH 1/2] regulator: core: Use a bitfield for continuous_voltage_range
Message-ID<qU88O-4xQ-7@gated-at.bofh.it>
In reply to#1315304

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

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.

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web