[PATCH 0/2] Make TWL4030_ALLOW_UNSUPPORTED more configurable.

[PATCH 0/2] Make TWL4030_ALLOW_UNSUPPORTED more configurable.

[NeilBrown]

This is a revised version on an earlier patch based on feedback from
Mark.  And it is now two patches :-)

First patch collects some related #defines together because I wasted
way too much time being confused because they were separate.
The second converts TWL4030_ALLOW_UNSUPPORTED from a CONFIG option
to a feature flag.

Thanks,
NeilBrown


---

NeilBrown (2):
      twl-regulator: make TWL4030_ALLOW_UNSUPPORTED more configurable.
      twl-regulator: define all feature flags in one place.


 drivers/mfd/twl-core.c            |    7 -------
 drivers/regulator/twl-regulator.c |   12 +++++-------
 include/linux/i2c/twl.h           |   13 +++++++++++--
 3 files changed, 16 insertions(+), 16 deletions(-)

-- 
Signature

[PATCH 1/2] twl-regulator: define all feature flags in one place.

[NeilBrown]

twl-regulator has a collection of feature flags, some defined
in twl-core.c and  one defined in i2c/twl.h.
This is confusing for anyone adding a new feature flag.

So collect them together and place them in twl.h immediately
after the structure in which they are initially set.

Signed-off-by: NeilBrown <neilb@suse.de>
---

 drivers/mfd/twl-core.c  |    7 -------
 include/linux/i2c/twl.h |    8 ++++++--
 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
index 4cbf285..269b296 100644
--- a/drivers/mfd/twl-core.c
+++ b/drivers/mfd/twl-core.c
@@ -224,13 +224,6 @@
 #define HIGH_PERF_SQ			(1 << 3)
 #define CK32K_LOWPWR_EN			(1 << 7)
 
-
-/* chip-specific feature flags, for i2c_device_id.driver_data */
-#define TWL4030_VAUX2		BIT(0)	/* pre-5030 voltage ranges */
-#define TPS_SUBSET		BIT(1)	/* tps659[23]0 have fewer LDOs */
-#define TWL5031			BIT(2)  /* twl5031 has different registers */
-#define TWL6030_CLASS		BIT(3)	/* TWL6030 class */
-
 /*----------------------------------------------------------------------*/
 
 /* is driver active, bound to a chip? */
diff --git a/include/linux/i2c/twl.h b/include/linux/i2c/twl.h
index b526031..88e8cd8 100644
--- a/include/linux/i2c/twl.h
+++ b/include/linux/i2c/twl.h
@@ -171,8 +171,6 @@ static inline int twl_class_is_ ##class(void)	\
 TWL_CLASS_IS(4030, TWL4030_CLASS_ID)
 TWL_CLASS_IS(6030, TWL6030_CLASS_ID)
 
-#define TWL6025_SUBCLASS	BIT(4)  /* TWL6025 has changed registers */
-
 /*
  * Read and write single 8-bit registers
  */
@@ -748,6 +746,12 @@ struct twl_regulator_driver_data {
 	void		*data;
 	unsigned long	features;
 };
+/* chip-specific feature flags, for twl_regulator_driver_data.features */
+#define TWL4030_VAUX2		BIT(0)	/* pre-5030 voltage ranges */
+#define TPS_SUBSET		BIT(1)	/* tps659[23]0 have fewer LDOs */
+#define TWL5031			BIT(2)  /* twl5031 has different registers */
+#define TWL6030_CLASS		BIT(3)	/* TWL6030 class */
+#define TWL6025_SUBCLASS	BIT(4)  /* TWL6025 has changed registers */
 
 /*----------------------------------------------------------------------*/
 

[PATCH 2/2] twl-regulator: make TWL4030_ALLOW_UNSUPPORTED more configurable.

[NeilBrown]

The regulators in the twl4030 can provide some voltage settings
that are not offically supported.
These settings are disabled by default, but can be enabled with
  CONFIG_TWL4030_ALLOW_UNSUPPORTED=y
However
 - that config variable is not mentioned in any Kconfig so cannot
   be used, and
 - a global setting is clumsy - a per regulator setting would be
   better.

So define a new 'feature' flag that a board file can set to enable
these unsupported volatages for boards which need them.

This flag cannot (yet) be set using device-tree.

Signed-off-by: NeilBrown <neilb@suse.de>
---

 drivers/regulator/twl-regulator.c |   12 +++++-------
 include/linux/i2c/twl.h           |    5 +++++
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/regulator/twl-regulator.c b/drivers/regulator/twl-regulator.c
index 9cdfc38..03a316d 100644
--- a/drivers/regulator/twl-regulator.c
+++ b/drivers/regulator/twl-regulator.c
@@ -388,14 +388,12 @@ static int twl6030reg_set_mode(struct regulator_dev *rdev, unsigned mode)
  * VAUX3 at 3V is incorrectly listed in some TI manuals as unsupported.
  * TI are revising the twl5030/tps659x0 specs to support that 3.0V setting.
  */
-#ifdef CONFIG_TWL4030_ALLOW_UNSUPPORTED
-#define UNSUP_MASK	0x0000
-#else
 #define UNSUP_MASK	0x8000
-#endif
 
 #define UNSUP(x)	(UNSUP_MASK | (x))
-#define IS_UNSUP(x)	(UNSUP_MASK & (x))
+#define IS_UNSUP(info, x)			\
+	((UNSUP_MASK & (x)) &&			\
+	 !((info)->features & TWL4030_ALLOW_UNSUPPORTED))
 #define LDO_MV(x)	(~UNSUP_MASK & (x))
 
 
@@ -469,7 +467,7 @@ static int twl4030ldo_list_voltage(struct regulator_dev *rdev, unsigned index)
 	struct twlreg_info	*info = rdev_get_drvdata(rdev);
 	int			mV = info->table[index];
 
-	return IS_UNSUP(mV) ? 0 : (LDO_MV(mV) * 1000);
+	return IS_UNSUP(info, mV) ? 0 : (LDO_MV(mV) * 1000);
 }
 
 static int
@@ -483,7 +481,7 @@ twl4030ldo_set_voltage(struct regulator_dev *rdev, int min_uV, int max_uV,
 		int mV = info->table[vsel];
 		int uV;
 
-		if (IS_UNSUP(mV))
+		if (IS_UNSUP(info, mV))
 			continue;
 		uV = LDO_MV(mV) * 1000;
 
diff --git a/include/linux/i2c/twl.h b/include/linux/i2c/twl.h
index 88e8cd8..8eec440 100644
--- a/include/linux/i2c/twl.h
+++ b/include/linux/i2c/twl.h
@@ -752,6 +752,11 @@ struct twl_regulator_driver_data {
 #define TWL5031			BIT(2)  /* twl5031 has different registers */
 #define TWL6030_CLASS		BIT(3)	/* TWL6030 class */
 #define TWL6025_SUBCLASS	BIT(4)  /* TWL6025 has changed registers */
+#define TWL4030_ALLOW_UNSUPPORTED BIT(5) /* Some voltages are possible
+					  * but not officially supported.
+					  * This flag is necessary to
+					  * enable them.
+					  */
 
 /*----------------------------------------------------------------------*/
 

Re: [PATCH 1/2] twl-regulator: define all feature flags in one place.

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 2523 bytes --]

On Wed, May 02, 2012 at 06:44:50AM +1000, NeilBrown wrote:
> twl-regulator has a collection of feature flags, some defined
> in twl-core.c and  one defined in i2c/twl.h.
> This is confusing for anyone adding a new feature flag.
> 
> So collect them together and place them in twl.h immediately
> after the structure in which they are initially set.

Please remember to always CC maintainers on patches and always use
subject lines appropriate for the subsystem.  Adding Samuel since this
is not a regulator patch.

> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
> 
>  drivers/mfd/twl-core.c  |    7 -------
>  include/linux/i2c/twl.h |    8 ++++++--
>  2 files changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
> index 4cbf285..269b296 100644
> --- a/drivers/mfd/twl-core.c
> +++ b/drivers/mfd/twl-core.c
> @@ -224,13 +224,6 @@
>  #define HIGH_PERF_SQ			(1 << 3)
>  #define CK32K_LOWPWR_EN			(1 << 7)
>  
> -
> -/* chip-specific feature flags, for i2c_device_id.driver_data */
> -#define TWL4030_VAUX2		BIT(0)	/* pre-5030 voltage ranges */
> -#define TPS_SUBSET		BIT(1)	/* tps659[23]0 have fewer LDOs */
> -#define TWL5031			BIT(2)  /* twl5031 has different registers */
> -#define TWL6030_CLASS		BIT(3)	/* TWL6030 class */
> -
>  /*----------------------------------------------------------------------*/
>  
>  /* is driver active, bound to a chip? */
> diff --git a/include/linux/i2c/twl.h b/include/linux/i2c/twl.h
> index b526031..88e8cd8 100644
> --- a/include/linux/i2c/twl.h
> +++ b/include/linux/i2c/twl.h
> @@ -171,8 +171,6 @@ static inline int twl_class_is_ ##class(void)	\
>  TWL_CLASS_IS(4030, TWL4030_CLASS_ID)
>  TWL_CLASS_IS(6030, TWL6030_CLASS_ID)
>  
> -#define TWL6025_SUBCLASS	BIT(4)  /* TWL6025 has changed registers */
> -
>  /*
>   * Read and write single 8-bit registers
>   */
> @@ -748,6 +746,12 @@ struct twl_regulator_driver_data {
>  	void		*data;
>  	unsigned long	features;
>  };
> +/* chip-specific feature flags, for twl_regulator_driver_data.features */
> +#define TWL4030_VAUX2		BIT(0)	/* pre-5030 voltage ranges */
> +#define TPS_SUBSET		BIT(1)	/* tps659[23]0 have fewer LDOs */
> +#define TWL5031			BIT(2)  /* twl5031 has different registers */
> +#define TWL6030_CLASS		BIT(3)	/* TWL6030 class */
> +#define TWL6025_SUBCLASS	BIT(4)  /* TWL6025 has changed registers */
>  
>  /*----------------------------------------------------------------------*/
>  
> 
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/2] twl-regulator: make TWL4030_ALLOW_UNSUPPORTED more configurable.

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 352 bytes --]

On Wed, May 02, 2012 at 06:44:50AM +1000, NeilBrown wrote:
> The regulators in the twl4030 can provide some voltage settings
> that are not offically supported.

This looks basically OK but depends on the previous patch so it'll have
to wait for now.  Once more, please always use subject lines appropriate
for the subsystem you're submitting against.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/2] twl-regulator: make TWL4030_ALLOW_UNSUPPORTED more configurable.

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 321 bytes --]

On Wed, May 02, 2012 at 06:44:50AM +1000, NeilBrown wrote:
> The regulators in the twl4030 can provide some voltage settings
> that are not offically supported.
> These settings are disabled by default, but can be enabled with

This doesn't apply against current code, please submit against current
development versions.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/2] twl-regulator: make TWL4030_ALLOW_UNSUPPORTED more configurable.

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 780 bytes --]

On Tue, 8 May 2012 19:40:49 +0100 Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:

> On Wed, May 02, 2012 at 06:44:50AM +1000, NeilBrown wrote:
> > The regulators in the twl4030 can provide some voltage settings
> > that are not offically supported.
> > These settings are disabled by default, but can be enabled with
> 
> This doesn't apply against current code, please submit against current
> development versions.

Hi Mark,
 I hope linux-next is suitably current - it did cause a conflict as you say
 which was fortunately easy to resolve.

 I've also fixed up the subject lines (I wonder if MAINTAINERS and
 checkpatch.pl need to learn about preferred subject lines :-) and added
 Samuel Ortez to the 'To:' list.

Thanks for your help,
NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH 2/2] twl-regulator: make TWL4030_ALLOW_UNSUPPORTED more configurable.

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 399 bytes --]

On Wed, May 09, 2012 at 05:47:51AM +1000, NeilBrown wrote:

>  I've also fixed up the subject lines (I wonder if MAINTAINERS and
>  checkpatch.pl need to learn about preferred subject lines :-) and added
>  Samuel Ortez to the 'To:' list.

Joe seemed to think that was far too hard or something.  In general just
look at the logs and follow the style there (in the same way one would
do with code).

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/2] twl-regulator: define all feature flags in one place.

[Samuel Ortiz]

Hi Neil,

On Wed, May 02, 2012 at 10:08:49AM +0100, Mark Brown wrote:
> On Wed, May 02, 2012 at 06:44:50AM +1000, NeilBrown wrote:
> > twl-regulator has a collection of feature flags, some defined
> > in twl-core.c and  one defined in i2c/twl.h.
> > This is confusing for anyone adding a new feature flag.
> > 
> > So collect them together and place them in twl.h immediately
> > after the structure in which they are initially set.
> 
> Please remember to always CC maintainers on patches and always use
> subject lines appropriate for the subsystem.  Adding Samuel since this
> is not a regulator patch.
Could you please re-send this patch to me ?

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: [PATCH 1/2] twl-regulator: define all feature flags in one place.

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 441 bytes --]

On Wed, May 09, 2012 at 09:50:41PM +0200, Samuel Ortiz wrote:

> Could you please re-send this patch to me ?

Neil did actually resend this the other day as the regulator bit needed
rebasing to current regulator code - I ended up applying it since it
looked like you'd missed it after you went through things the other day
and the main thing was the regulator change based off it but I can
easily drop the patches again if that's a problem.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]