Skip to contentSkip to conversation

Make TWL4030_ALLOW_UNSUPPORTED more configurable.

PATCH In review NeilBrown 3 participants 10 messages May 1, 2012 – May 9, 2012

Subsystems (3)
Multifunction devices (MFD) OMAP2+ support Voltage and current regulator framework
lore
Share & tools
lore

NeilBrown

author
#0 lore raw

cover Make TWL4030_ALLOW_UNSUPPORTED more configurable.

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 · 1 line
Signature
author
#1 lore raw

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

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 <[email protected]>

---

 drivers/mfd/twl-core.c  |    7 -------
 include/linux/i2c/twl.h |    8 ++++++--
 2 files changed, 6 insertions(+), 9 deletions(-)
drivers/mfd/twl-core.c +0 -7
index 4cbf285..269b296 100644
@@ -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? */
include/linux/i2c/twl.h +6 -2
index b526031..88e8cd8 100644
@@ -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 */
 
 /*----------------------------------------------------------------------*/
#2 lore raw

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

Quoted 6 lines · 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.

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.

Quoted 53 lines
Signed-off-by NeilBrown <[email protected]>

---

 drivers/mfd/twl-core.c  |    7 -------
 include/linux/i2c/twl.h |    8 ++++++--
 2 files changed, 6 insertions(+), 9 deletions(-)
drivers/mfd/twl-core.c +0 -7
index 4cbf285..269b296 100644
@@ -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? */
include/linux/i2c/twl.h +6 -2
index b526031..88e8cd8 100644
@@ -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 */
 
 /*----------------------------------------------------------------------*/
#3 lore raw

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

Hi Neil,

Quoted 11 lines · Mark Brown
Quoted 6 lines · 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.

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.

Signature · 2 lines
Intel Open Source Technology Centre
http://oss.intel.com/
#4 lore raw

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

Quoted 1 line · Samuel Ortiz

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.

author
#5 lore raw

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

In reply to 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 <[email protected]>

---

 drivers/regulator/twl-regulator.c |   12 +++++-------
 include/linux/i2c/twl.h           |    5 +++++
 2 files changed, 10 insertions(+), 7 deletions(-)
drivers/regulator/twl-regulator.c +5 -7
index 9cdfc38..03a316d 100644
@@ -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;
include/linux/i2c/twl.h +5 -0
index 88e8cd8..8eec440 100644
@@ -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.
+					  */
 
 /*----------------------------------------------------------------------*/
#6 lore raw

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

Quoted 2 lines · NeilBrown

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.

#7 lore raw

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

In reply to NeilBrown
Quoted 3 lines · 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

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

author
#8 lore raw

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

Quoted 7 lines · On Tue, 8 May 2012 19:40:49 +0100 Mark Brown…
Quoted 3 lines · 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

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

#9 lore raw

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

Quoted 3 lines · NeilBrown

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).