[PATCH v2 00/11] MFD: twl-core: Cleanup part 2 for 3.9

[PATCH v2 00/11] MFD: twl-core: Cleanup part 2 for 3.9

[Peter Ujfalusi]

Hello Samuel,

Changes since v1:
- Patch for zoom-display to avoid build issus with this set

I had a patch on top of this series to move the zoom board to use bl-pwm for
display backlight. Because of this I have not noticed that the zoom-display.c
still have twl related code in upstream.

Cover letter from v1:

Second part of the cleanup of twl-core which aims to make the code a bit more
readable.
It has been tested on: OMAP4 PandaBoard, OMAP4 Blaze, OMAP3 BeagleBoard, OMAP3
Zoom2.

Regards,
Peter
---
Peter Ujfalusi (11):
  ARM: OMAP: zoom-display: Remove the use of TWL4030_MODULE_PWM1
  mfd: twl-core: Clean up module id lookup and definitions
  mfd: twl-core: No need to check for invalid subchip ID
  mfd: twl-core: Use the lookup table to find the correct subchip for
    the modules
  mfd: twl-core: Allocate twl_modules dynamically
  mfd: twl-core: Do not try to call legacy mfd add_children() when
    booted with DT
  mfd: twl-core: Do not create dummy pdata when booted with DT
  mfd: twl-core: Move 'inuse' check early at probe time
  mfd: twl-core: Collect global variables behind one private structure
    (global)
  mfd: twl-core: Remove no longer valid comment regarding to write
    buffer size
  mfd: twl-core: Move twl_i2c_read/write_u8 to header as inline function
 arch/arm/mach-omap2/board-zoom-display.c |  14 +-
 drivers/mfd/twl-core.c                   | 362 ++++++++++++++-----------------
 include/linux/i2c/twl.h                  |  84 +++----
 3 files changed, 212 insertions(+), 248 deletions(-)

-- 
1.8.1

[PATCH v2 01/11] ARM: OMAP: zoom-display: Remove the use of TWL4030_MODULE_PWM1

[Peter Ujfalusi]

Use the future proof TWL_MODULE_PWM module id instead to aim the twl-core
cleanup planed for 3.9 kernel cycle.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
Acked-by: Tony Lindgren <tony@atomide.com>
---
 arch/arm/mach-omap2/board-zoom-display.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-omap2/board-zoom-display.c b/arch/arm/mach-omap2/board-zoom-display.c
index 1c7c834..8cef477 100644
--- a/arch/arm/mach-omap2/board-zoom-display.c
+++ b/arch/arm/mach-omap2/board-zoom-display.c
@@ -49,13 +49,13 @@ static void zoom_panel_disable_lcd(struct omap_dss_device *dssdev)
 {
 }
 
-/*
- * PWMA/B register offsets (TWL4030_MODULE_PWMA)
- */
+/* Register offsets in TWL4030_MODULE_INTBR */
 #define TWL_INTBR_PMBR1	0xD
 #define TWL_INTBR_GPBR1	0xC
-#define TWL_LED_PWMON	0x0
-#define TWL_LED_PWMOFF	0x1
+
+/* Register offsets in TWL_MODULE_PWM */
+#define TWL_LED_PWMON	0x3
+#define TWL_LED_PWMOFF	0x4
 
 static int zoom_set_bl_intensity(struct omap_dss_device *dssdev, int level)
 {
@@ -93,8 +93,8 @@ static int zoom_set_bl_intensity(struct omap_dss_device *dssdev, int level)
 	}
 
 	c = ((50 * (100 - level)) / 100) + 1;
-	twl_i2c_write_u8(TWL4030_MODULE_PWM1, 0x7F, TWL_LED_PWMOFF);
-	twl_i2c_write_u8(TWL4030_MODULE_PWM1, c, TWL_LED_PWMON);
+	twl_i2c_write_u8(TWL_MODULE_PWM, 0x7F, TWL_LED_PWMOFF);
+	twl_i2c_write_u8(TWL_MODULE_PWM, c, TWL_LED_PWMON);
 #else
 	pr_warn("Backlight not enabled\n");
 #endif
-- 
1.8.1

[PATCH v2 02/11] mfd: twl-core: Clean up module id lookup and definitions

[Peter Ujfalusi]

Use enums for all module definitions:
twl_module_ids for common functionality among twl4030/twl6030
twl4030_module_ids for twl4030 specific ids
twl6030_module_ids for twl6030 specific ids

In this way the list can be managed easier when new functionality going to
be implemented.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 drivers/mfd/twl-core.c  | 105 +++++++++++++++++++++++-------------------------
 include/linux/i2c/twl.h |  66 ++++++++++++++++--------------
 2 files changed, 86 insertions(+), 85 deletions(-)

diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
index 4f3baad..b781cdd 100644
--- a/drivers/mfd/twl-core.c
+++ b/drivers/mfd/twl-core.c
@@ -74,8 +74,6 @@
 #define SUB_CHIP_ID3 3
 #define SUB_CHIP_ID_INVAL 0xff
 
-#define TWL_MODULE_LAST TWL4030_MODULE_LAST
-
 /* Base Address defns for twl4030_map[] */
 
 /* subchip/slave 0 - USB ID */
@@ -94,10 +92,7 @@
 #define TWL4030_BASEADD_MADC		0x0000
 #define TWL4030_BASEADD_MAIN_CHARGE	0x0074
 #define TWL4030_BASEADD_PRECHARGE	0x00AA
-#define TWL4030_BASEADD_PWM0		0x00F8
-#define TWL4030_BASEADD_PWM1		0x00FB
-#define TWL4030_BASEADD_PWMA		0x00EF
-#define TWL4030_BASEADD_PWMB		0x00F1
+#define TWL4030_BASEADD_PWM		0x00F8
 #define TWL4030_BASEADD_KEYPAD		0x00D2
 
 #define TWL5031_BASEADD_ACCESSORY	0x0074 /* Replaces Main Charge */
@@ -117,7 +112,7 @@
 
 /* subchip/slave 0 0x48 - POWER */
 #define TWL6030_BASEADD_RTC		0x0000
-#define TWL6030_BASEADD_MEM		0x0017
+#define TWL6030_BASEADD_SECURED_REG	0x0017
 #define TWL6030_BASEADD_PM_MASTER	0x001F
 #define TWL6030_BASEADD_PM_SLAVE_MISC	0x0030 /* PM_RECEIVER */
 #define TWL6030_BASEADD_PM_MISC		0x00E2
@@ -132,6 +127,7 @@
 #define TWL6030_BASEADD_PIH		0x00D0
 #define TWL6030_BASEADD_CHARGER		0x00E0
 #define TWL6025_BASEADD_CHARGER		0x00DA
+#define TWL6030_BASEADD_LED		0x00F4
 
 /* subchip/slave 2 0x4A - DFT */
 #define TWL6030_BASEADD_DIEID		0x00C0
@@ -188,34 +184,33 @@ static struct twl_mapping twl4030_map[] = {
 	 * so they continue to match the order in this table.
 	 */
 
+	/* Common IPs */
 	{ 0, TWL4030_BASEADD_USB },
+	{ 1, TWL4030_BASEADD_PIH },
+	{ 2, TWL4030_BASEADD_MAIN_CHARGE },
+	{ 3, TWL4030_BASEADD_PM_MASTER },
+	{ 3, TWL4030_BASEADD_PM_RECEIVER },
+
+	{ 3, TWL4030_BASEADD_RTC },
+	{ 2, TWL4030_BASEADD_PWM },
+	{ 2, TWL4030_BASEADD_LED },
+	{ 3, TWL4030_BASEADD_SECURED_REG },
+
+	/* TWL4030 specific IPs */
 	{ 1, TWL4030_BASEADD_AUDIO_VOICE },
 	{ 1, TWL4030_BASEADD_GPIO },
 	{ 1, TWL4030_BASEADD_INTBR },
-	{ 1, TWL4030_BASEADD_PIH },
-
 	{ 1, TWL4030_BASEADD_TEST },
 	{ 2, TWL4030_BASEADD_KEYPAD },
+
 	{ 2, TWL4030_BASEADD_MADC },
 	{ 2, TWL4030_BASEADD_INTERRUPTS },
-	{ 2, TWL4030_BASEADD_LED },
-
-	{ 2, TWL4030_BASEADD_MAIN_CHARGE },
 	{ 2, TWL4030_BASEADD_PRECHARGE },
-	{ 2, TWL4030_BASEADD_PWM0 },
-	{ 2, TWL4030_BASEADD_PWM1 },
-	{ 2, TWL4030_BASEADD_PWMA },
-
-	{ 2, TWL4030_BASEADD_PWMB },
-	{ 2, TWL5031_BASEADD_ACCESSORY },
-	{ 2, TWL5031_BASEADD_INTERRUPTS },
 	{ 3, TWL4030_BASEADD_BACKUP },
 	{ 3, TWL4030_BASEADD_INT },
 
-	{ 3, TWL4030_BASEADD_PM_MASTER },
-	{ 3, TWL4030_BASEADD_PM_RECEIVER },
-	{ 3, TWL4030_BASEADD_RTC },
-	{ 3, TWL4030_BASEADD_SECURED_REG },
+	{ 2, TWL5031_BASEADD_ACCESSORY },
+	{ 2, TWL5031_BASEADD_INTERRUPTS },
 };
 
 static struct regmap_config twl4030_regmap_config[4] = {
@@ -251,35 +246,25 @@ static struct twl_mapping twl6030_map[] = {
 	 * <linux/i2c/twl.h> defines for TWL4030_MODULE_*
 	 * so they continue to match the order in this table.
 	 */
-	{ SUB_CHIP_ID1, TWL6030_BASEADD_USB },
-	{ SUB_CHIP_ID_INVAL, TWL6030_BASEADD_AUDIO },
-	{ SUB_CHIP_ID2, TWL6030_BASEADD_DIEID },
-	{ SUB_CHIP_ID2, TWL6030_BASEADD_RSV },
-	{ SUB_CHIP_ID1, TWL6030_BASEADD_PIH },
-
-	{ SUB_CHIP_ID2, TWL6030_BASEADD_RSV },
-	{ SUB_CHIP_ID2, TWL6030_BASEADD_RSV },
-	{ SUB_CHIP_ID1, TWL6030_BASEADD_GPADC_CTRL },
-	{ SUB_CHIP_ID2, TWL6030_BASEADD_RSV },
-	{ SUB_CHIP_ID2, TWL6030_BASEADD_RSV },
-
-	{ SUB_CHIP_ID1, TWL6030_BASEADD_CHARGER },
-	{ SUB_CHIP_ID1, TWL6030_BASEADD_GASGAUGE },
-	{ SUB_CHIP_ID1, TWL6030_BASEADD_PWM },
-	{ SUB_CHIP_ID0, TWL6030_BASEADD_ZERO },
-	{ SUB_CHIP_ID1, TWL6030_BASEADD_ZERO },
-
-	{ SUB_CHIP_ID2, TWL6030_BASEADD_ZERO },
-	{ SUB_CHIP_ID2, TWL6030_BASEADD_ZERO },
-	{ SUB_CHIP_ID2, TWL6030_BASEADD_RSV },
-	{ SUB_CHIP_ID2, TWL6030_BASEADD_RSV },
-	{ SUB_CHIP_ID2, TWL6030_BASEADD_RSV },
-
-	{ SUB_CHIP_ID0, TWL6030_BASEADD_PM_MASTER },
-	{ SUB_CHIP_ID0, TWL6030_BASEADD_PM_SLAVE_MISC },
-	{ SUB_CHIP_ID0, TWL6030_BASEADD_RTC },
-	{ SUB_CHIP_ID0, TWL6030_BASEADD_MEM },
-	{ SUB_CHIP_ID1, TWL6025_BASEADD_CHARGER },
+
+	/* Common IPs */
+	{ 1, TWL6030_BASEADD_USB },
+	{ 1, TWL6030_BASEADD_PIH },
+	{ 1, TWL6030_BASEADD_CHARGER },
+	{ 0, TWL6030_BASEADD_PM_MASTER },
+	{ 0, TWL6030_BASEADD_PM_SLAVE_MISC },
+
+	{ 0, TWL6030_BASEADD_RTC },
+	{ 1, TWL6030_BASEADD_PWM },
+	{ 1, TWL6030_BASEADD_LED },
+	{ 0, TWL6030_BASEADD_SECURED_REG },
+
+	/* TWL6030 specific IPs */
+	{ 0, TWL6030_BASEADD_ZERO },
+	{ 1, TWL6030_BASEADD_ZERO },
+	{ 2, TWL6030_BASEADD_ZERO },
+	{ 1, TWL6030_BASEADD_GPADC_CTRL },
+	{ 1, TWL6030_BASEADD_GASGAUGE },
 };
 
 static struct regmap_config twl6030_regmap_config[3] = {
@@ -305,6 +290,14 @@ static struct regmap_config twl6030_regmap_config[3] = {
 
 /*----------------------------------------------------------------------*/
 
+static inline int twl_get_last_module(void)
+{
+	if (twl_class_is_4030())
+		return TWL4030_MODULE_LAST;
+	else
+		return TWL6030_MODULE_LAST;
+}
+
 /* Exported Functions */
 
 /**
@@ -325,7 +318,7 @@ int twl_i2c_write(u8 mod_no, u8 *value, u8 reg, unsigned num_bytes)
 	int sid;
 	struct twl_client *twl;
 
-	if (unlikely(mod_no >= TWL_MODULE_LAST)) {
+	if (unlikely(mod_no >= twl_get_last_module())) {
 		pr_err("%s: invalid module number %d\n", DRIVER_NAME, mod_no);
 		return -EPERM;
 	}
@@ -367,7 +360,7 @@ int twl_i2c_read(u8 mod_no, u8 *value, u8 reg, unsigned num_bytes)
 	int sid;
 	struct twl_client *twl;
 
-	if (unlikely(mod_no >= TWL_MODULE_LAST)) {
+	if (unlikely(mod_no >= twl_get_last_module())) {
 		pr_err("%s: invalid module number %d\n", DRIVER_NAME, mod_no);
 		return -EPERM;
 	}
@@ -1228,6 +1221,10 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	if ((id->driver_data) & TWL6030_CLASS) {
 		twl_id = TWL6030_CLASS_ID;
 		twl_map = &twl6030_map[0];
+		/* The charger base address is different in twl6025 */
+		if ((id->driver_data) & TWL6025_SUBCLASS)
+			twl_map[TWL_MODULE_MAIN_CHARGE].base =
+							TWL6025_BASEADD_CHARGER;
 		twl_regmap_config = twl6030_regmap_config;
 		num_slaves = TWL_NUM_SLAVES - 1;
 	} else {
diff --git a/include/linux/i2c/twl.h b/include/linux/i2c/twl.h
index 1ff54b1..72adc88 100644
--- a/include/linux/i2c/twl.h
+++ b/include/linux/i2c/twl.h
@@ -39,51 +39,55 @@
  * address each module uses within a given i2c slave.
  */
 
+/* Module IDs for similar functionalities found in twl4030/twl6030 */
+enum twl_module_ids {
+	TWL_MODULE_USB,
+	TWL_MODULE_PIH,
+	TWL_MODULE_MAIN_CHARGE,
+	TWL_MODULE_PM_MASTER,
+	TWL_MODULE_PM_RECEIVER,
+
+	TWL_MODULE_RTC,
+	TWL_MODULE_PWM,
+	TWL_MODULE_LED,
+	TWL_MODULE_SECURED_REG,
+
+	TWL_MODULE_LAST,
+};
+
+/* Modules only available in twl4030 series */
 enum twl4030_module_ids {
-	TWL4030_MODULE_USB = 0,		/* Slave 0 (i2c address 0x48) */
-	TWL4030_MODULE_AUDIO_VOICE,	/* Slave 1 (i2c address 0x49) */
+	TWL4030_MODULE_AUDIO_VOICE = TWL_MODULE_LAST,
 	TWL4030_MODULE_GPIO,
 	TWL4030_MODULE_INTBR,
-	TWL4030_MODULE_PIH,
-
 	TWL4030_MODULE_TEST,
-	TWL4030_MODULE_KEYPAD,		/* Slave 2 (i2c address 0x4a) */
+	TWL4030_MODULE_KEYPAD,
+
 	TWL4030_MODULE_MADC,
 	TWL4030_MODULE_INTERRUPTS,
-	TWL4030_MODULE_LED,
-
-	TWL4030_MODULE_MAIN_CHARGE,
 	TWL4030_MODULE_PRECHARGE,
-	TWL4030_MODULE_PWM0,
-	TWL4030_MODULE_PWM1,
-	TWL4030_MODULE_PWMA,
+	TWL4030_MODULE_BACKUP,
+	TWL4030_MODULE_INT,
 
-	TWL4030_MODULE_PWMB,
 	TWL5031_MODULE_ACCESSORY,
 	TWL5031_MODULE_INTERRUPTS,
-	TWL4030_MODULE_BACKUP,		/* Slave 3 (i2c address 0x4b) */
-	TWL4030_MODULE_INT,
 
-	TWL4030_MODULE_PM_MASTER,
-	TWL4030_MODULE_PM_RECEIVER,
-	TWL4030_MODULE_RTC,
-	TWL4030_MODULE_SECURED_REG,
 	TWL4030_MODULE_LAST,
 };
 
-/* Similar functionalities implemented in TWL4030/6030 */
-#define TWL_MODULE_USB		TWL4030_MODULE_USB
-#define TWL_MODULE_PIH		TWL4030_MODULE_PIH
-#define TWL_MODULE_MAIN_CHARGE	TWL4030_MODULE_MAIN_CHARGE
-#define TWL_MODULE_PM_MASTER	TWL4030_MODULE_PM_MASTER
-#define TWL_MODULE_PM_RECEIVER	TWL4030_MODULE_PM_RECEIVER
-#define TWL_MODULE_RTC		TWL4030_MODULE_RTC
-#define TWL_MODULE_PWM		TWL4030_MODULE_PWM0
-#define TWL_MODULE_LED		TWL4030_MODULE_LED
-
-#define TWL6030_MODULE_ID0	13
-#define TWL6030_MODULE_ID1	14
-#define TWL6030_MODULE_ID2	15
+/* Modules only available in twl6030 series */
+enum twl6030_module_ids {
+	TWL6030_MODULE_ID0 = TWL_MODULE_LAST,
+	TWL6030_MODULE_ID1,
+	TWL6030_MODULE_ID2,
+	TWL6030_MODULE_GPADC,
+	TWL6030_MODULE_GASGAUGE,
+
+	TWL6030_MODULE_LAST,
+};
+
+/* Until the clients has been converted to use TWL_MODULE_LED */
+#define TWL4030_MODULE_LED	TWL_MODULE_LED
 
 #define GPIO_INTR_OFFSET	0
 #define KEYPAD_INTR_OFFSET	1
-- 
1.8.1

[PATCH v2 03/11] mfd: twl-core: No need to check for invalid subchip ID

[Peter Ujfalusi]

The module id table no longer can have invalid/unused entries.
No need for checking the ID for validity.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 drivers/mfd/twl-core.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
index b781cdd..fa1a5a0 100644
--- a/drivers/mfd/twl-core.c
+++ b/drivers/mfd/twl-core.c
@@ -72,7 +72,6 @@
 #define SUB_CHIP_ID1 1
 #define SUB_CHIP_ID2 2
 #define SUB_CHIP_ID3 3
-#define SUB_CHIP_ID_INVAL 0xff
 
 /* Base Address defns for twl4030_map[] */
 
@@ -326,12 +325,8 @@ int twl_i2c_write(u8 mod_no, u8 *value, u8 reg, unsigned num_bytes)
 		pr_err("%s: not initialized\n", DRIVER_NAME);
 		return -EPERM;
 	}
+
 	sid = twl_map[mod_no].sid;
-	if (unlikely(sid == SUB_CHIP_ID_INVAL)) {
-		pr_err("%s: module %d is not part of the pmic\n",
-		       DRIVER_NAME, mod_no);
-		return -EINVAL;
-	}
 	twl = &twl_modules[sid];
 
 	ret = regmap_bulk_write(twl->regmap, twl_map[mod_no].base + reg,
@@ -368,12 +363,8 @@ int twl_i2c_read(u8 mod_no, u8 *value, u8 reg, unsigned num_bytes)
 		pr_err("%s: not initialized\n", DRIVER_NAME);
 		return -EPERM;
 	}
+
 	sid = twl_map[mod_no].sid;
-	if (unlikely(sid == SUB_CHIP_ID_INVAL)) {
-		pr_err("%s: module %d is not part of the pmic\n",
-		       DRIVER_NAME, mod_no);
-		return -EINVAL;
-	}
 	twl = &twl_modules[sid];
 
 	ret = regmap_bulk_read(twl->regmap, twl_map[mod_no].base + reg,
-- 
1.8.1

[PATCH v2 04/11] mfd: twl-core: Use the lookup table to find the correct subchip for the modules

[Peter Ujfalusi]

Instead of using SUB_CHIP_ID* or magic numbers use the twl_mapping table to
look for the subchip ID.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 drivers/mfd/twl-core.c | 56 ++++++++++++++++++++++++--------------------------
 1 file changed, 27 insertions(+), 29 deletions(-)

diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
index fa1a5a0..f07317b 100644
--- a/drivers/mfd/twl-core.c
+++ b/drivers/mfd/twl-core.c
@@ -68,11 +68,6 @@
 
 #define TWL_NUM_SLAVES		4
 
-#define SUB_CHIP_ID0 0
-#define SUB_CHIP_ID1 1
-#define SUB_CHIP_ID2 2
-#define SUB_CHIP_ID3 3
-
 /* Base Address defns for twl4030_map[] */
 
 /* subchip/slave 0 - USB ID */
@@ -493,13 +488,20 @@ int twl_get_hfclk_rate(void)
 EXPORT_SYMBOL_GPL(twl_get_hfclk_rate);
 
 static struct device *
-add_numbered_child(unsigned chip, const char *name, int num,
+add_numbered_child(unsigned mod_no, const char *name, int num,
 		void *pdata, unsigned pdata_len,
 		bool can_wakeup, int irq0, int irq1)
 {
 	struct platform_device	*pdev;
-	struct twl_client	*twl = &twl_modules[chip];
-	int			status;
+	struct twl_client	*twl;
+	int			status, sid;
+
+	if (unlikely(mod_no >= twl_get_last_module())) {
+		pr_err("%s: invalid module number %d\n", DRIVER_NAME, mod_no);
+		return ERR_PTR(-EPERM);
+	}
+	sid = twl_map[mod_no].sid;
+	twl = &twl_modules[sid];
 
 	pdev = platform_device_alloc(name, num);
 	if (!pdev) {
@@ -544,11 +546,11 @@ err:
 	return &pdev->dev;
 }
 
-static inline struct device *add_child(unsigned chip, const char *name,
+static inline struct device *add_child(unsigned mod_no, const char *name,
 		void *pdata, unsigned pdata_len,
 		bool can_wakeup, int irq0, int irq1)
 {
-	return add_numbered_child(chip, name, -1, pdata, pdata_len,
+	return add_numbered_child(mod_no, name, -1, pdata, pdata_len,
 		can_wakeup, irq0, irq1);
 }
 
@@ -557,7 +559,6 @@ add_regulator_linked(int num, struct regulator_init_data *pdata,
 		struct regulator_consumer_supply *consumers,
 		unsigned num_consumers, unsigned long features)
 {
-	unsigned sub_chip_id;
 	struct twl_regulator_driver_data drv_data;
 
 	/* regulator framework demands init_data ... */
@@ -584,8 +585,7 @@ add_regulator_linked(int num, struct regulator_init_data *pdata,
 	}
 
 	/* NOTE:  we currently ignore regulator IRQs, e.g. for short circuits */
-	sub_chip_id = twl_map[TWL_MODULE_PM_MASTER].sid;
-	return add_numbered_child(sub_chip_id, "twl_reg", num,
+	return add_numbered_child(TWL_MODULE_PM_MASTER, "twl_reg", num,
 		pdata, sizeof(*pdata), false, 0, 0);
 }
 
@@ -607,10 +607,9 @@ add_children(struct twl4030_platform_data *pdata, unsigned irq_base,
 		unsigned long features)
 {
 	struct device	*child;
-	unsigned sub_chip_id;
 
 	if (IS_ENABLED(CONFIG_GPIO_TWL4030) && pdata->gpio) {
-		child = add_child(SUB_CHIP_ID1, "twl4030_gpio",
+		child = add_child(TWL4030_MODULE_GPIO, "twl4030_gpio",
 				pdata->gpio, sizeof(*pdata->gpio),
 				false, irq_base + GPIO_INTR_OFFSET, 0);
 		if (IS_ERR(child))
@@ -618,7 +617,7 @@ add_children(struct twl4030_platform_data *pdata, unsigned irq_base,
 	}
 
 	if (IS_ENABLED(CONFIG_KEYBOARD_TWL4030) && pdata->keypad) {
-		child = add_child(SUB_CHIP_ID2, "twl4030_keypad",
+		child = add_child(TWL4030_MODULE_KEYPAD, "twl4030_keypad",
 				pdata->keypad, sizeof(*pdata->keypad),
 				true, irq_base + KEYPAD_INTR_OFFSET, 0);
 		if (IS_ERR(child))
@@ -627,7 +626,7 @@ add_children(struct twl4030_platform_data *pdata, unsigned irq_base,
 
 	if (IS_ENABLED(CONFIG_TWL4030_MADC) && pdata->madc &&
 	    twl_class_is_4030()) {
-		child = add_child(SUB_CHIP_ID2, "twl4030_madc",
+		child = add_child(TWL4030_MODULE_MADC, "twl4030_madc",
 				pdata->madc, sizeof(*pdata->madc),
 				true, irq_base + MADC_INTR_OFFSET, 0);
 		if (IS_ERR(child))
@@ -642,22 +641,21 @@ add_children(struct twl4030_platform_data *pdata, unsigned irq_base,
 		 * Eventually, Linux might become more aware of such
 		 * HW security concerns, and "least privilege".
 		 */
-		sub_chip_id = twl_map[TWL_MODULE_RTC].sid;
-		child = add_child(sub_chip_id, "twl_rtc", NULL, 0,
+		child = add_child(TWL_MODULE_RTC, "twl_rtc", NULL, 0,
 				true, irq_base + RTC_INTR_OFFSET, 0);
 		if (IS_ERR(child))
 			return PTR_ERR(child);
 	}
 
 	if (IS_ENABLED(CONFIG_PWM_TWL)) {
-		child = add_child(SUB_CHIP_ID1, "twl-pwm", NULL, 0,
+		child = add_child(TWL_MODULE_PWM, "twl-pwm", NULL, 0,
 				  false, 0, 0);
 		if (IS_ERR(child))
 			return PTR_ERR(child);
 	}
 
 	if (IS_ENABLED(CONFIG_PWM_TWL_LED)) {
-		child = add_child(SUB_CHIP_ID1, "twl-pwmled", NULL, 0,
+		child = add_child(TWL_MODULE_LED, "twl-pwmled", NULL, 0,
 				  false, 0, 0);
 		if (IS_ERR(child))
 			return PTR_ERR(child);
@@ -709,7 +707,7 @@ add_children(struct twl4030_platform_data *pdata, unsigned irq_base,
 
 		}
 
-		child = add_child(SUB_CHIP_ID0, "twl4030_usb",
+		child = add_child(TWL_MODULE_USB, "twl4030_usb",
 				pdata->usb, sizeof(*pdata->usb), true,
 				/* irq0 = USB_PRES, irq1 = USB */
 				irq_base + USB_PRES_INTR_OFFSET,
@@ -758,7 +756,7 @@ add_children(struct twl4030_platform_data *pdata, unsigned irq_base,
 
 		pdata->usb->features = features;
 
-		child = add_child(SUB_CHIP_ID0, "twl6030_usb",
+		child = add_child(TWL_MODULE_USB, "twl6030_usb",
 			pdata->usb, sizeof(*pdata->usb), true,
 			/* irq1 = VBUS_PRES, irq0 = USB ID */
 			irq_base + USBOTG_INTR_OFFSET,
@@ -783,22 +781,22 @@ add_children(struct twl4030_platform_data *pdata, unsigned irq_base,
 	}
 
 	if (IS_ENABLED(CONFIG_TWL4030_WATCHDOG) && twl_class_is_4030()) {
-		child = add_child(SUB_CHIP_ID3, "twl4030_wdt", NULL, 0,
-				  false, 0, 0);
+		child = add_child(TWL_MODULE_PM_RECEIVER, "twl4030_wdt", NULL,
+				  0, false, 0, 0);
 		if (IS_ERR(child))
 			return PTR_ERR(child);
 	}
 
 	if (IS_ENABLED(CONFIG_INPUT_TWL4030_PWRBUTTON) && twl_class_is_4030()) {
-		child = add_child(SUB_CHIP_ID3, "twl4030_pwrbutton", NULL, 0,
-				  true, irq_base + 8 + 0, 0);
+		child = add_child(TWL_MODULE_PM_MASTER, "twl4030_pwrbutton",
+				  NULL, 0, true, irq_base + 8 + 0, 0);
 		if (IS_ERR(child))
 			return PTR_ERR(child);
 	}
 
 	if (IS_ENABLED(CONFIG_MFD_TWL4030_AUDIO) && pdata->audio &&
 	    twl_class_is_4030()) {
-		child = add_child(SUB_CHIP_ID1, "twl4030-audio",
+		child = add_child(TWL4030_MODULE_AUDIO_VOICE, "twl4030-audio",
 				pdata->audio, sizeof(*pdata->audio),
 				false, 0, 0);
 		if (IS_ERR(child))
@@ -1038,7 +1036,7 @@ add_children(struct twl4030_platform_data *pdata, unsigned irq_base,
 
 	if (IS_ENABLED(CONFIG_CHARGER_TWL4030) && pdata->bci &&
 			!(features & (TPS_SUBSET | TWL5031))) {
-		child = add_child(SUB_CHIP_ID3, "twl4030_bci",
+		child = add_child(TWL_MODULE_MAIN_CHARGE, "twl4030_bci",
 				pdata->bci, sizeof(*pdata->bci), false,
 				/* irq0 = CHG_PRES, irq1 = BCI */
 				irq_base + BCI_PRES_INTR_OFFSET,
-- 
1.8.1

[PATCH v2 05/11] mfd: twl-core: Allocate twl_modules dynamically

[Peter Ujfalusi]

At boot time we can allocate the twl_modules array dynamically based on the
twl class we are using with devm_kzalloc() instead of the static
twl_modules[] array.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 drivers/mfd/twl-core.c | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
index f07317b..fbff830 100644
--- a/drivers/mfd/twl-core.c
+++ b/drivers/mfd/twl-core.c
@@ -66,8 +66,6 @@
 
 /* Triton Core internal information (BEGIN) */
 
-#define TWL_NUM_SLAVES		4
-
 /* Base Address defns for twl4030_map[] */
 
 /* subchip/slave 0 - USB ID */
@@ -162,7 +160,7 @@ struct twl_client {
 	struct regmap *regmap;
 };
 
-static struct twl_client twl_modules[TWL_NUM_SLAVES];
+static struct twl_client *twl_modules;
 
 /* mapping the module id to slave id and base address */
 struct twl_mapping {
@@ -284,6 +282,14 @@ static struct regmap_config twl6030_regmap_config[3] = {
 
 /*----------------------------------------------------------------------*/
 
+static inline int twl_get_num_slaves(void)
+{
+	if (twl_class_is_4030())
+		return 4; /* TWL4030 class have four slave address */
+	else
+		return 3; /* TWL6030 class have three slave address */
+}
+
 static inline int twl_get_last_module(void)
 {
 	if (twl_class_is_4030())
@@ -1127,17 +1133,15 @@ static int twl_remove(struct i2c_client *client)
 	unsigned i, num_slaves;
 	int status;
 
-	if (twl_class_is_4030()) {
+	if (twl_class_is_4030())
 		status = twl4030_exit_irq();
-		num_slaves = TWL_NUM_SLAVES;
-	} else {
+	else
 		status = twl6030_exit_irq();
-		num_slaves = TWL_NUM_SLAVES - 1;
-	}
 
 	if (status < 0)
 		return status;
 
+	num_slaves = twl_get_num_slaves();
 	for (i = 0; i < num_slaves; i++) {
 		struct twl_client	*twl = &twl_modules[i];
 
@@ -1215,12 +1219,19 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
 			twl_map[TWL_MODULE_MAIN_CHARGE].base =
 							TWL6025_BASEADD_CHARGER;
 		twl_regmap_config = twl6030_regmap_config;
-		num_slaves = TWL_NUM_SLAVES - 1;
 	} else {
 		twl_id = TWL4030_CLASS_ID;
 		twl_map = &twl4030_map[0];
 		twl_regmap_config = twl4030_regmap_config;
-		num_slaves = TWL_NUM_SLAVES;
+	}
+
+	num_slaves = twl_get_num_slaves();
+	twl_modules = devm_kzalloc(&client->dev,
+				   sizeof(struct twl_client) * num_slaves,
+				   GFP_KERNEL);
+	if (!twl_modules) {
+		status = -ENOMEM;
+		goto free;
 	}
 
 	for (i = 0; i < num_slaves; i++) {
-- 
1.8.1

[PATCH v2 06/11] mfd: twl-core: Do not try to call legacy mfd add_children() when booted with DT

[Peter Ujfalusi]

There is really no point to retry to add children devices in case the
of_platform_populate() fails.
We do not have any information provided via pdata in this case anyways.
Depending on the boot type (legacy or DT) only execute either one.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 drivers/mfd/twl-core.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
index fbff830..86cca9e 100644
--- a/drivers/mfd/twl-core.c
+++ b/drivers/mfd/twl-core.c
@@ -1305,10 +1305,9 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		twl_i2c_write_u8(TWL4030_MODULE_INTBR, temp, REG_GPPUPDCTR1);
 	}
 
-	status = -ENODEV;
 	if (node)
 		status = of_platform_populate(node, NULL, NULL, &client->dev);
-	if (status)
+	else
 		status = add_children(pdata, irq_base, id->driver_data);
 
 fail:
-- 
1.8.1

[PATCH v2 07/11] mfd: twl-core: Do not create dummy pdata when booted with DT

[Peter Ujfalusi]

When booted with DT we can manage without the dummy pdata.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 drivers/mfd/twl-core.c | 31 +++++++------------------------
 1 file changed, 7 insertions(+), 24 deletions(-)

diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
index 86cca9e..547fed5 100644
--- a/drivers/mfd/twl-core.c
+++ b/drivers/mfd/twl-core.c
@@ -1165,6 +1165,11 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	int				status;
 	unsigned			i, num_slaves;
 
+	if (!node && !pdata) {
+		dev_err(&client->dev, "no platform data\n");
+		return -EINVAL;
+	}
+
 	pdev = platform_device_alloc(DRIVER_NAME, -1);
 	if (!pdev) {
 		dev_err(&client->dev, "can't alloc pdev\n");
@@ -1177,28 +1182,6 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		return status;
 	}
 
-	if (node && !pdata) {
-		/*
-		 * XXX: Temporary pdata until the information is correctly
-		 * retrieved by every TWL modules from DT.
-		 */
-		pdata = devm_kzalloc(&client->dev,
-				     sizeof(struct twl4030_platform_data),
-				     GFP_KERNEL);
-		if (!pdata) {
-			status = -ENOMEM;
-			goto free;
-		}
-	}
-
-	if (!pdata) {
-		dev_dbg(&client->dev, "no platform data?\n");
-		status = -EINVAL;
-		goto free;
-	}
-
-	platform_set_drvdata(pdev, pdata);
-
 	if (i2c_check_functionality(client->adapter, I2C_FUNC_I2C) == 0) {
 		dev_dbg(&client->dev, "can't talk I2C?\n");
 		status = -EIO;
@@ -1264,7 +1247,7 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	inuse = true;
 
 	/* setup clock framework */
-	clocks_init(&pdev->dev, pdata->clock);
+	clocks_init(&pdev->dev, pdata ? pdata->clock : NULL);
 
 	/* read TWL IDCODE Register */
 	if (twl_id == TWL4030_CLASS_ID) {
@@ -1273,7 +1256,7 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	}
 
 	/* load power event scripts */
-	if (IS_ENABLED(CONFIG_TWL4030_POWER) && pdata->power)
+	if (IS_ENABLED(CONFIG_TWL4030_POWER) && pdata && pdata->power)
 		twl4030_power_init(pdata->power);
 
 	/* Maybe init the T2 Interrupt subsystem */
-- 
1.8.1

[PATCH v2 08/11] mfd: twl-core: Move 'inuse' check early at probe time

[Peter Ujfalusi]

We can fail earlier in case multiple instance of the twl-core is tried to
be loaded.
The twl-core by design only supports one instance.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 drivers/mfd/twl-core.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
index 547fed5..1827088 100644
--- a/drivers/mfd/twl-core.c
+++ b/drivers/mfd/twl-core.c
@@ -1170,6 +1170,12 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		return -EINVAL;
 	}
 
+	if (inuse) {
+		dev_dbg(&client->dev, "only one instance of %s allowed\n",
+			DRIVER_NAME);
+		return -EBUSY;
+	}
+
 	pdev = platform_device_alloc(DRIVER_NAME, -1);
 	if (!pdev) {
 		dev_err(&client->dev, "can't alloc pdev\n");
@@ -1188,12 +1194,6 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		goto free;
 	}
 
-	if (inuse) {
-		dev_dbg(&client->dev, "driver is already in use\n");
-		status = -EBUSY;
-		goto free;
-	}
-
 	if ((id->driver_data) & TWL6030_CLASS) {
 		twl_id = TWL6030_CLASS_ID;
 		twl_map = &twl6030_map[0];
-- 
1.8.1

[PATCH v2 09/11] mfd: twl-core: Collect global variables behind one private structure (global)

[Peter Ujfalusi]

Gather the global variables under a single structure and allocate it with
devm_kzalloc(). It is easier to see them and if in the future we try to add
support for multiple instance of twl in the system it is going to be much
simpler.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 drivers/mfd/twl-core.c | 104 +++++++++++++++++++++++++++----------------------
 1 file changed, 57 insertions(+), 47 deletions(-)

diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
index 1827088..e2895a4 100644
--- a/drivers/mfd/twl-core.c
+++ b/drivers/mfd/twl-core.c
@@ -141,33 +141,28 @@
 
 /*----------------------------------------------------------------------*/
 
-/* is driver active, bound to a chip? */
-static bool inuse;
-
-/* TWL IDCODE Register value */
-static u32 twl_idcode;
-
-static unsigned int twl_id;
-unsigned int twl_rev(void)
-{
-	return twl_id;
-}
-EXPORT_SYMBOL(twl_rev);
-
 /* Structure for each TWL4030/TWL6030 Slave */
 struct twl_client {
 	struct i2c_client *client;
 	struct regmap *regmap;
 };
 
-static struct twl_client *twl_modules;
-
 /* mapping the module id to slave id and base address */
 struct twl_mapping {
 	unsigned char sid;	/* Slave ID */
 	unsigned char base;	/* base address */
 };
-static struct twl_mapping *twl_map;
+
+struct twl_private {
+	bool ready; /* The core driver is ready to be used */
+	u32 twl_idcode; /* TWL IDCODE Register value */
+	unsigned int twl_id;
+
+	struct twl_mapping *twl_map;
+	struct twl_client *twl_modules;
+};
+
+static struct twl_private *twl_priv;
 
 static struct twl_mapping twl4030_map[] = {
 	/*
@@ -300,6 +295,12 @@ static inline int twl_get_last_module(void)
 
 /* Exported Functions */
 
+unsigned int twl_rev(void)
+{
+	return twl_priv ? twl_priv->twl_id : 0;
+}
+EXPORT_SYMBOL(twl_rev);
+
 /**
  * twl_i2c_write - Writes a n bit register in TWL4030/TWL5030/TWL60X0
  * @mod_no: module number
@@ -322,16 +323,17 @@ int twl_i2c_write(u8 mod_no, u8 *value, u8 reg, unsigned num_bytes)
 		pr_err("%s: invalid module number %d\n", DRIVER_NAME, mod_no);
 		return -EPERM;
 	}
-	if (unlikely(!inuse)) {
+	if (unlikely(!twl_priv->ready)) {
 		pr_err("%s: not initialized\n", DRIVER_NAME);
 		return -EPERM;
 	}
 
-	sid = twl_map[mod_no].sid;
-	twl = &twl_modules[sid];
+	sid = twl_priv->twl_map[mod_no].sid;
+	twl = &twl_priv->twl_modules[sid];
 
-	ret = regmap_bulk_write(twl->regmap, twl_map[mod_no].base + reg,
-				value, num_bytes);
+	ret = regmap_bulk_write(twl->regmap,
+				twl_priv->twl_map[mod_no].base + reg, value,
+				num_bytes);
 
 	if (ret)
 		pr_err("%s: Write failed (mod %d, reg 0x%02x count %d)\n",
@@ -360,16 +362,17 @@ int twl_i2c_read(u8 mod_no, u8 *value, u8 reg, unsigned num_bytes)
 		pr_err("%s: invalid module number %d\n", DRIVER_NAME, mod_no);
 		return -EPERM;
 	}
-	if (unlikely(!inuse)) {
+	if (unlikely(!twl_priv->ready)) {
 		pr_err("%s: not initialized\n", DRIVER_NAME);
 		return -EPERM;
 	}
 
-	sid = twl_map[mod_no].sid;
-	twl = &twl_modules[sid];
+	sid = twl_priv->twl_map[mod_no].sid;
+	twl = &twl_priv->twl_modules[sid];
 
-	ret = regmap_bulk_read(twl->regmap, twl_map[mod_no].base + reg,
-			       value, num_bytes);
+	ret = regmap_bulk_read(twl->regmap,
+			       twl_priv->twl_map[mod_no].base + reg, value,
+			       num_bytes);
 
 	if (ret)
 		pr_err("%s: Read failed (mod %d, reg 0x%02x count %d)\n",
@@ -425,7 +428,7 @@ static int twl_read_idcode_register(void)
 		goto fail;
 	}
 
-	err = twl_i2c_read(TWL4030_MODULE_INTBR, (u8 *)(&twl_idcode),
+	err = twl_i2c_read(TWL4030_MODULE_INTBR, (u8 *)(&twl_priv->twl_idcode),
 						REG_IDCODE_7_0, 4);
 	if (err) {
 		pr_err("TWL4030: unable to read IDCODE -%d\n", err);
@@ -446,7 +449,7 @@ fail:
  */
 int twl_get_type(void)
 {
-	return TWL_SIL_TYPE(twl_idcode);
+	return TWL_SIL_TYPE(twl_priv->twl_idcode);
 }
 EXPORT_SYMBOL_GPL(twl_get_type);
 
@@ -457,7 +460,7 @@ EXPORT_SYMBOL_GPL(twl_get_type);
  */
 int twl_get_version(void)
 {
-	return TWL_SIL_REV(twl_idcode);
+	return TWL_SIL_REV(twl_priv->twl_idcode);
 }
 EXPORT_SYMBOL_GPL(twl_get_version);
 
@@ -506,8 +509,8 @@ add_numbered_child(unsigned mod_no, const char *name, int num,
 		pr_err("%s: invalid module number %d\n", DRIVER_NAME, mod_no);
 		return ERR_PTR(-EPERM);
 	}
-	sid = twl_map[mod_no].sid;
-	twl = &twl_modules[sid];
+	sid = twl_priv->twl_map[mod_no].sid;
+	twl = &twl_priv->twl_modules[sid];
 
 	pdev = platform_device_alloc(name, num);
 	if (!pdev) {
@@ -1143,13 +1146,13 @@ static int twl_remove(struct i2c_client *client)
 
 	num_slaves = twl_get_num_slaves();
 	for (i = 0; i < num_slaves; i++) {
-		struct twl_client	*twl = &twl_modules[i];
+		struct twl_client	*twl = &twl_priv->twl_modules[i];
 
 		if (twl->client && twl->client != client)
 			i2c_unregister_device(twl->client);
-		twl_modules[i].client = NULL;
+		twl->client = NULL;
 	}
-	inuse = false;
+	twl_priv->ready = false;
 	return 0;
 }
 
@@ -1170,7 +1173,7 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		return -EINVAL;
 	}
 
-	if (inuse) {
+	if (twl_priv) {
 		dev_dbg(&client->dev, "only one instance of %s allowed\n",
 			DRIVER_NAME);
 		return -EBUSY;
@@ -1194,31 +1197,38 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		goto free;
 	}
 
+	twl_priv = devm_kzalloc(&client->dev, sizeof(struct twl_private),
+				GFP_KERNEL);
+	if (!twl_priv) {
+		status = -ENOMEM;
+		goto free;
+	}
+
 	if ((id->driver_data) & TWL6030_CLASS) {
-		twl_id = TWL6030_CLASS_ID;
-		twl_map = &twl6030_map[0];
+		twl_priv->twl_id = TWL6030_CLASS_ID;
+		twl_priv->twl_map = &twl6030_map[0];
 		/* The charger base address is different in twl6025 */
 		if ((id->driver_data) & TWL6025_SUBCLASS)
-			twl_map[TWL_MODULE_MAIN_CHARGE].base =
+			twl_priv->twl_map[TWL_MODULE_MAIN_CHARGE].base =
 							TWL6025_BASEADD_CHARGER;
 		twl_regmap_config = twl6030_regmap_config;
 	} else {
-		twl_id = TWL4030_CLASS_ID;
-		twl_map = &twl4030_map[0];
+		twl_priv->twl_id = TWL4030_CLASS_ID;
+		twl_priv->twl_map = &twl4030_map[0];
 		twl_regmap_config = twl4030_regmap_config;
 	}
 
 	num_slaves = twl_get_num_slaves();
-	twl_modules = devm_kzalloc(&client->dev,
-				   sizeof(struct twl_client) * num_slaves,
-				   GFP_KERNEL);
-	if (!twl_modules) {
+	twl_priv->twl_modules = devm_kzalloc(&client->dev,
+					 sizeof(struct twl_client) * num_slaves,
+					 GFP_KERNEL);
+	if (!twl_priv->twl_modules) {
 		status = -ENOMEM;
 		goto free;
 	}
 
 	for (i = 0; i < num_slaves; i++) {
-		struct twl_client *twl = &twl_modules[i];
+		struct twl_client *twl = &twl_priv->twl_modules[i];
 
 		if (i == 0) {
 			twl->client = client;
@@ -1244,13 +1254,13 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		}
 	}
 
-	inuse = true;
+	twl_priv->ready = true;
 
 	/* setup clock framework */
 	clocks_init(&pdev->dev, pdata ? pdata->clock : NULL);
 
 	/* read TWL IDCODE Register */
-	if (twl_id == TWL4030_CLASS_ID) {
+	if (twl_class_is_4030()) {
 		status = twl_read_idcode_register();
 		WARN(status < 0, "Error: reading twl_idcode register value\n");
 	}
-- 
1.8.1

[PATCH v2 10/11] mfd: twl-core: Remove no longer valid comment regarding to write buffer size

[Peter Ujfalusi]

With the regmap conversion there is no longeer a need to allocate bigger
buffer for writes

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 drivers/mfd/twl-core.c  | 3 ---
 include/linux/i2c/twl.h | 3 ---
 2 files changed, 6 deletions(-)

diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
index e2895a4..9661204 100644
--- a/drivers/mfd/twl-core.c
+++ b/drivers/mfd/twl-core.c
@@ -308,9 +308,6 @@ EXPORT_SYMBOL(twl_rev);
  * @reg: register address (just offset will do)
  * @num_bytes: number of bytes to transfer
  *
- * IMPORTANT: for 'value' parameter: Allocate value num_bytes+1 and
- * valid data starts at Offset 1.
- *
  * Returns the result of operation - 0 is success
  */
 int twl_i2c_write(u8 mod_no, u8 *value, u8 reg, unsigned num_bytes)
diff --git a/include/linux/i2c/twl.h b/include/linux/i2c/twl.h
index 72adc88..e441fd8 100644
--- a/include/linux/i2c/twl.h
+++ b/include/linux/i2c/twl.h
@@ -182,9 +182,6 @@ int twl_i2c_read_u8(u8 mod_no, u8 *val, u8 reg);
 
 /*
  * Read and write several 8-bit registers at once.
- *
- * IMPORTANT:  For twl_i2c_write(), allocate num_bytes + 1
- * for the value, and populate your data starting at offset 1.
  */
 int twl_i2c_write(u8 mod_no, u8 *value, u8 reg, unsigned num_bytes);
 int twl_i2c_read(u8 mod_no, u8 *value, u8 reg, unsigned num_bytes);
-- 
1.8.1

[PATCH v2 11/11] mfd: twl-core: Move twl_i2c_read/write_u8 to header as inline function

[Peter Ujfalusi]

twl_i2c_read/write_u8 become as a simple wrapper over the twl_i2c_read/write.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 drivers/mfd/twl-core.c  | 28 ----------------------------
 include/linux/i2c/twl.h | 17 +++++++++++------
 2 files changed, 11 insertions(+), 34 deletions(-)

diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
index 9661204..557f9ee 100644
--- a/drivers/mfd/twl-core.c
+++ b/drivers/mfd/twl-core.c
@@ -379,34 +379,6 @@ int twl_i2c_read(u8 mod_no, u8 *value, u8 reg, unsigned num_bytes)
 }
 EXPORT_SYMBOL(twl_i2c_read);
 
-/**
- * twl_i2c_write_u8 - Writes a 8 bit register in TWL4030/TWL5030/TWL60X0
- * @mod_no: module number
- * @value: the value to be written 8 bit
- * @reg: register address (just offset will do)
- *
- * Returns result of operation - 0 is success
- */
-int twl_i2c_write_u8(u8 mod_no, u8 value, u8 reg)
-{
-	return twl_i2c_write(mod_no, &value, reg, 1);
-}
-EXPORT_SYMBOL(twl_i2c_write_u8);
-
-/**
- * twl_i2c_read_u8 - Reads a 8 bit register from TWL4030/TWL5030/TWL60X0
- * @mod_no: module number
- * @value: the value read 8 bit
- * @reg: register address (just offset will do)
- *
- * Returns result of operation - 0 is success
- */
-int twl_i2c_read_u8(u8 mod_no, u8 *value, u8 reg)
-{
-	return twl_i2c_read(mod_no, value, reg, 1);
-}
-EXPORT_SYMBOL(twl_i2c_read_u8);
-
 /*----------------------------------------------------------------------*/
 
 /**
diff --git a/include/linux/i2c/twl.h b/include/linux/i2c/twl.h
index e441fd8..488debb 100644
--- a/include/linux/i2c/twl.h
+++ b/include/linux/i2c/twl.h
@@ -175,17 +175,22 @@ TWL_CLASS_IS(4030, TWL4030_CLASS_ID)
 TWL_CLASS_IS(6030, TWL6030_CLASS_ID)
 
 /*
- * Read and write single 8-bit registers
- */
-int twl_i2c_write_u8(u8 mod_no, u8 val, u8 reg);
-int twl_i2c_read_u8(u8 mod_no, u8 *val, u8 reg);
-
-/*
  * Read and write several 8-bit registers at once.
  */
 int twl_i2c_write(u8 mod_no, u8 *value, u8 reg, unsigned num_bytes);
 int twl_i2c_read(u8 mod_no, u8 *value, u8 reg, unsigned num_bytes);
 
+/*
+ * Read and write single 8-bit registers
+ */
+static inline int twl_i2c_write_u8(u8 mod_no, u8 val, u8 reg) {
+	return twl_i2c_write(mod_no, &val, reg, 1);
+}
+
+static inline int twl_i2c_read_u8(u8 mod_no, u8 *val, u8 reg) {
+	return twl_i2c_read(mod_no, val, reg, 1);
+}
+
 int twl_get_type(void);
 int twl_get_version(void);
 int twl_get_hfclk_rate(void);
-- 
1.8.1

Re: [PATCH v2 00/11] MFD: twl-core: Cleanup part 2 for 3.9

[Samuel Ortiz]

Hi Peter,

On Wed, Jan 16, 2013 at 02:53:48PM +0100, Peter Ujfalusi wrote:
> Hello Samuel,
> 
> Changes since v1:
> - Patch for zoom-display to avoid build issus with this set
> 
> I had a patch on top of this series to move the zoom board to use bl-pwm for
> display backlight. Because of this I have not noticed that the zoom-display.c
> still have twl related code in upstream.
> 
> Cover letter from v1:
> 
> Second part of the cleanup of twl-core which aims to make the code a bit more
> readable.
> It has been tested on: OMAP4 PandaBoard, OMAP4 Blaze, OMAP3 BeagleBoard, OMAP3
> Zoom2.
> 
> Regards,
> Peter
> ---
> Peter Ujfalusi (11):
>   ARM: OMAP: zoom-display: Remove the use of TWL4030_MODULE_PWM1
>   mfd: twl-core: Clean up module id lookup and definitions
>   mfd: twl-core: No need to check for invalid subchip ID
>   mfd: twl-core: Use the lookup table to find the correct subchip for
>     the modules
>   mfd: twl-core: Allocate twl_modules dynamically
>   mfd: twl-core: Do not try to call legacy mfd add_children() when
>     booted with DT
>   mfd: twl-core: Do not create dummy pdata when booted with DT
>   mfd: twl-core: Move 'inuse' check early at probe time
>   mfd: twl-core: Collect global variables behind one private structure
>     (global)
>   mfd: twl-core: Remove no longer valid comment regarding to write
>     buffer size
>   mfd: twl-core: Move twl_i2c_read/write_u8 to header as inline function
>  arch/arm/mach-omap2/board-zoom-display.c |  14 +-
>  drivers/mfd/twl-core.c                   | 362 ++++++++++++++-----------------
>  include/linux/i2c/twl.h                  |  84 +++----
>  3 files changed, 212 insertions(+), 248 deletions(-)
Thanks, all 11 patches applied to my for-next branch.

Cheers,
Samuel.

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

Re: [PATCH v2 09/11] mfd: twl-core: Collect global variables behind one private structure (global)

[Jon Hunter]

On 01/16/2013 07:53 AM, Peter Ujfalusi wrote:
> Gather the global variables under a single structure and allocate it with
> devm_kzalloc(). It is easier to see them and if in the future we try to add
> support for multiple instance of twl in the system it is going to be much
> simpler.
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> ---
>  drivers/mfd/twl-core.c | 104 +++++++++++++++++++++++++++----------------------
>  1 file changed, 57 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
> index 1827088..e2895a4 100644
> --- a/drivers/mfd/twl-core.c
> +++ b/drivers/mfd/twl-core.c
> @@ -141,33 +141,28 @@
>  
>  /*----------------------------------------------------------------------*/
>  
> -/* is driver active, bound to a chip? */
> -static bool inuse;
> -
> -/* TWL IDCODE Register value */
> -static u32 twl_idcode;
> -
> -static unsigned int twl_id;
> -unsigned int twl_rev(void)
> -{
> -	return twl_id;
> -}
> -EXPORT_SYMBOL(twl_rev);
> -
>  /* Structure for each TWL4030/TWL6030 Slave */
>  struct twl_client {
>  	struct i2c_client *client;
>  	struct regmap *regmap;
>  };
>  
> -static struct twl_client *twl_modules;
> -
>  /* mapping the module id to slave id and base address */
>  struct twl_mapping {
>  	unsigned char sid;	/* Slave ID */
>  	unsigned char base;	/* base address */
>  };
> -static struct twl_mapping *twl_map;
> +
> +struct twl_private {
> +	bool ready; /* The core driver is ready to be used */
> +	u32 twl_idcode; /* TWL IDCODE Register value */
> +	unsigned int twl_id;
> +
> +	struct twl_mapping *twl_map;
> +	struct twl_client *twl_modules;
> +};
> +
> +static struct twl_private *twl_priv;
>  
>  static struct twl_mapping twl4030_map[] = {
>  	/*
> @@ -300,6 +295,12 @@ static inline int twl_get_last_module(void)
>  
>  /* Exported Functions */
>  
> +unsigned int twl_rev(void)
> +{
> +	return twl_priv ? twl_priv->twl_id : 0;
> +}
> +EXPORT_SYMBOL(twl_rev);
> +
>  /**
>   * twl_i2c_write - Writes a n bit register in TWL4030/TWL5030/TWL60X0
>   * @mod_no: module number
> @@ -322,16 +323,17 @@ int twl_i2c_write(u8 mod_no, u8 *value, u8 reg, unsigned num_bytes)
>  		pr_err("%s: invalid module number %d\n", DRIVER_NAME, mod_no);
>  		return -EPERM;
>  	}
> -	if (unlikely(!inuse)) {
> +	if (unlikely(!twl_priv->ready)) {

This is causing the kernel to panic on all my omap2 boards when booting linux-next
because twl_priv is not initialised yet.

>  		pr_err("%s: not initialized\n", DRIVER_NAME);
>  		return -EPERM;
>  	}
>  
> -	sid = twl_map[mod_no].sid;
> -	twl = &twl_modules[sid];
> +	sid = twl_priv->twl_map[mod_no].sid;
> +	twl = &twl_priv->twl_modules[sid];
>  
> -	ret = regmap_bulk_write(twl->regmap, twl_map[mod_no].base + reg,
> -				value, num_bytes);
> +	ret = regmap_bulk_write(twl->regmap,
> +				twl_priv->twl_map[mod_no].base + reg, value,
> +				num_bytes);
>  
>  	if (ret)
>  		pr_err("%s: Write failed (mod %d, reg 0x%02x count %d)\n",
> @@ -360,16 +362,17 @@ int twl_i2c_read(u8 mod_no, u8 *value, u8 reg, unsigned num_bytes)
>  		pr_err("%s: invalid module number %d\n", DRIVER_NAME, mod_no);
>  		return -EPERM;
>  	}
> -	if (unlikely(!inuse)) {
> +	if (unlikely(!twl_priv->ready)) {

Same problem here. 

Here is a fix ...

>From 141fcbbdee6bdc14d5a444ff20fad6b3440215dc Mon Sep 17 00:00:00 2001
From: Jon Hunter <jon-hunter@ti.com>
Date: Fri, 8 Feb 2013 12:42:20 -0600
Subject: [PATCH] ARM: OMAP2+: Fix kernel panic on boot

Commit 8a6aaa3 (mfd: twl-core: Collect global variables behind one
private structure (global)) removed the variable "inuse" that is used
to determine if the device has been initialised and now use the
twl_priv structure instead. This is causing the kernel to panic on all
OMAP2+ devices, because we try to access the twl_priv->ready member
before checking if twl_priv is initialised. Fix this and move this test
to the beginning of the twl_i2c_read/write function because
twl_get_last_module() also uses the twl_priv structure.

Signed-off-by: Jon Hunter <jon-hunter@ti.com>
---
 drivers/mfd/twl-core.c |   16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
index 557f9ee..89ab4d9 100644
--- a/drivers/mfd/twl-core.c
+++ b/drivers/mfd/twl-core.c
@@ -316,12 +316,12 @@ int twl_i2c_write(u8 mod_no, u8 *value, u8 reg, unsigned num_bytes)
 	int sid;
 	struct twl_client *twl;
 
-	if (unlikely(mod_no >= twl_get_last_module())) {
-		pr_err("%s: invalid module number %d\n", DRIVER_NAME, mod_no);
+	if (unlikely(!twl_priv || !twl_priv->ready)) {
+		pr_err("%s: not initialized\n", DRIVER_NAME);
 		return -EPERM;
 	}
-	if (unlikely(!twl_priv->ready)) {
-		pr_err("%s: not initialized\n", DRIVER_NAME);
+	if (unlikely(mod_no >= twl_get_last_module())) {
+		pr_err("%s: invalid module number %d\n", DRIVER_NAME, mod_no);
 		return -EPERM;
 	}
 
@@ -355,12 +355,12 @@ int twl_i2c_read(u8 mod_no, u8 *value, u8 reg, unsigned num_bytes)
 	int sid;
 	struct twl_client *twl;
 
-	if (unlikely(mod_no >= twl_get_last_module())) {
-		pr_err("%s: invalid module number %d\n", DRIVER_NAME, mod_no);
+	if (unlikely(!twl_priv || !twl_priv->ready)) {
+		pr_err("%s: not initialized\n", DRIVER_NAME);
 		return -EPERM;
 	}
-	if (unlikely(!twl_priv->ready)) {
-		pr_err("%s: not initialized\n", DRIVER_NAME);
+	if (unlikely(mod_no >= twl_get_last_module())) {
+		pr_err("%s: invalid module number %d\n", DRIVER_NAME, mod_no);
 		return -EPERM;
 	}
 
-- 
1.7.10.4

Re: [PATCH v2 09/11] mfd: twl-core: Collect global variables behind one private structure (global)

[Peter Ujfalusi]

On 02/08/2013 07:56 PM, Jon Hunter wrote:
>>  /**
>>   * twl_i2c_write - Writes a n bit register in TWL4030/TWL5030/TWL60X0
>>   * @mod_no: module number
>> @@ -322,16 +323,17 @@ int twl_i2c_write(u8 mod_no, u8 *value, u8 reg, unsigned num_bytes)
>>  		pr_err("%s: invalid module number %d\n", DRIVER_NAME, mod_no);
>>  		return -EPERM;
>>  	}
>> -	if (unlikely(!inuse)) {
>> +	if (unlikely(!twl_priv->ready)) {
> 
> This is causing the kernel to panic on all my omap2 boards when booting linux-next
> because twl_priv is not initialised yet.

Good catch.
I just wonder from where the twl_* call is coming on OMAP2. AFAIK the twl code
is for OMAP3/4, for OMAP2 Menelaus is the one which is used.
I'm currently working on to remove all those twl_* calls from random places in
the kernel so we will only access twl via the MFD stack.

> 
>>  		pr_err("%s: not initialized\n", DRIVER_NAME);
>>  		return -EPERM;
>>  	}
>>  
>> -	sid = twl_map[mod_no].sid;
>> -	twl = &twl_modules[sid];
>> +	sid = twl_priv->twl_map[mod_no].sid;
>> +	twl = &twl_priv->twl_modules[sid];
>>  
>> -	ret = regmap_bulk_write(twl->regmap, twl_map[mod_no].base + reg,
>> -				value, num_bytes);
>> +	ret = regmap_bulk_write(twl->regmap,
>> +				twl_priv->twl_map[mod_no].base + reg, value,
>> +				num_bytes);
>>  
>>  	if (ret)
>>  		pr_err("%s: Write failed (mod %d, reg 0x%02x count %d)\n",
>> @@ -360,16 +362,17 @@ int twl_i2c_read(u8 mod_no, u8 *value, u8 reg, unsigned num_bytes)
>>  		pr_err("%s: invalid module number %d\n", DRIVER_NAME, mod_no);
>>  		return -EPERM;
>>  	}
>> -	if (unlikely(!inuse)) {
>> +	if (unlikely(!twl_priv->ready)) {
> 
> Same problem here. 
> 
> Here is a fix ...
> 
> From 141fcbbdee6bdc14d5a444ff20fad6b3440215dc Mon Sep 17 00:00:00 2001
> From: Jon Hunter <jon-hunter@ti.com>
> Date: Fri, 8 Feb 2013 12:42:20 -0600
> Subject: [PATCH] ARM: OMAP2+: Fix kernel panic on boot
> 
> Commit 8a6aaa3 (mfd: twl-core: Collect global variables behind one
> private structure (global)) removed the variable "inuse" that is used
> to determine if the device has been initialised and now use the
> twl_priv structure instead. This is causing the kernel to panic on all
> OMAP2+ devices, because we try to access the twl_priv->ready member
> before checking if twl_priv is initialised. Fix this and move this test
> to the beginning of the twl_i2c_read/write function because
> twl_get_last_module() also uses the twl_priv structure.
> 
> Signed-off-by: Jon Hunter <jon-hunter@ti.com>

Acked-by: Peter Ujfalusi <peter.ujfalusi@ti.com>

> ---
>  drivers/mfd/twl-core.c |   16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
> index 557f9ee..89ab4d9 100644
> --- a/drivers/mfd/twl-core.c
> +++ b/drivers/mfd/twl-core.c
> @@ -316,12 +316,12 @@ int twl_i2c_write(u8 mod_no, u8 *value, u8 reg, unsigned num_bytes)
>  	int sid;
>  	struct twl_client *twl;
>  
> -	if (unlikely(mod_no >= twl_get_last_module())) {
> -		pr_err("%s: invalid module number %d\n", DRIVER_NAME, mod_no);
> +	if (unlikely(!twl_priv || !twl_priv->ready)) {
> +		pr_err("%s: not initialized\n", DRIVER_NAME);
>  		return -EPERM;
>  	}
> -	if (unlikely(!twl_priv->ready)) {
> -		pr_err("%s: not initialized\n", DRIVER_NAME);
> +	if (unlikely(mod_no >= twl_get_last_module())) {
> +		pr_err("%s: invalid module number %d\n", DRIVER_NAME, mod_no);
>  		return -EPERM;
>  	}
>  
> @@ -355,12 +355,12 @@ int twl_i2c_read(u8 mod_no, u8 *value, u8 reg, unsigned num_bytes)
>  	int sid;
>  	struct twl_client *twl;
>  
> -	if (unlikely(mod_no >= twl_get_last_module())) {
> -		pr_err("%s: invalid module number %d\n", DRIVER_NAME, mod_no);
> +	if (unlikely(!twl_priv || !twl_priv->ready)) {
> +		pr_err("%s: not initialized\n", DRIVER_NAME);
>  		return -EPERM;
>  	}
> -	if (unlikely(!twl_priv->ready)) {
> -		pr_err("%s: not initialized\n", DRIVER_NAME);
> +	if (unlikely(mod_no >= twl_get_last_module())) {
> +		pr_err("%s: invalid module number %d\n", DRIVER_NAME, mod_no);
>  		return -EPERM;
>  	}
>  
> 


-- 
Péter

Re: [PATCH v2 09/11] mfd: twl-core: Collect global variables behind one private structure (global)

[Jon Hunter]

On 02/08/2013 11:50 PM, Peter Ujfalusi wrote:
> On 02/08/2013 07:56 PM, Jon Hunter wrote:
>>>  /**
>>>   * twl_i2c_write - Writes a n bit register in TWL4030/TWL5030/TWL60X0
>>>   * @mod_no: module number
>>> @@ -322,16 +323,17 @@ int twl_i2c_write(u8 mod_no, u8 *value, u8 reg, unsigned num_bytes)
>>>  		pr_err("%s: invalid module number %d\n", DRIVER_NAME, mod_no);
>>>  		return -EPERM;
>>>  	}
>>> -	if (unlikely(!inuse)) {
>>> +	if (unlikely(!twl_priv->ready)) {
>>
>> This is causing the kernel to panic on all my omap2 boards when booting linux-next
>> because twl_priv is not initialised yet.
> 
> Good catch.
> I just wonder from where the twl_* call is coming on OMAP2. AFAIK the twl code
> is for OMAP3/4, for OMAP2 Menelaus is the one which is used.
> I'm currently working on to remove all those twl_* calls from random places in
> the kernel so we will only access twl via the MFD stack.

Good point. I just noticed that none of my omap2+ board were booting and
on omap3/4 I was the panic in the twl code. I can't say that I checked
the panic on omap2, so may be that was another problem?

I will update the changelog and re-send the patch.

Cheers
Jon

Re: [PATCH v2 09/11] mfd: twl-core: Collect global variables behind one private structure (global)

[Peter Ujfalusi]

On 02/11/2013 09:22 PM, Jon Hunter wrote:
> Good point. I just noticed that none of my omap2+ board were booting and
> on omap3/4 I was the panic in the twl code. I can't say that I checked
> the panic on omap2, so may be that was another problem?

Do you have insights on the code path leading to a crash on OMAP3/4? I have
been running this code on several boards (BeagleBoard, Zoom2, PandaBoard,
Blaze) and have not seen a crash.
But the fix is valid.

-- 
Péter

Re: [PATCH v2 09/11] mfd: twl-core: Collect global variables behind one private structure (global)

[Jon Hunter]

On 02/12/2013 01:26 AM, Peter Ujfalusi wrote:
> On 02/11/2013 09:22 PM, Jon Hunter wrote:
>> Good point. I just noticed that none of my omap2+ board were booting and
>> on omap3/4 I was the panic in the twl code. I can't say that I checked
>> the panic on omap2, so may be that was another problem?
> 
> Do you have insights on the code path leading to a crash on OMAP3/4? I have
> been running this code on several boards (BeagleBoard, Zoom2, PandaBoard,
> Blaze) and have not seen a crash.

Here is the panic log ...

[    2.286132] Unable to handle kernel NULL pointer dereference at virtual address 00000000
[    2.294738] pgd = c0004000
[    2.297576] [00000000] *pgd=00000000
[    2.301361] Internal error: Oops: 5 [#1] SMP ARM
[    2.306243] Modules linked in:
[    2.309448] CPU: 0    Not tainted  (3.8.0-rc6-next-20130207-00016-g735c237 #35)
[    2.317169] PC is at twl_i2c_read+0x3c/0xec
[    2.321563] LR is at twl_i2c_read+0x1c/0xec
[    2.325988] pc : [<c0333950>]    lr : [<c0333930>]    psr: 80000153
[    2.325988] sp : c702fed0  ip : 00000000  fp : 00000000
[    2.338043] r10: c702e000  r9 : c06e84e8  r8 : c06e51c8
[    2.343536] r7 : 00000001  r6 : 00000006  r5 : c702fef6  r4 : 00000004
[    2.350402] r3 : c129508c  r2 : 00000006  r1 : c702fef6  r0 : 0000000e
[    2.357269] Flags: Nzcv  IRQs on  FIQs off  Mode SVC_32  ISA ARM  Segment kernel
[    2.365051] Control: 10c5387d  Table: 80004019  DAC: 00000017
[    2.371093] Process swapper/0 (pid: 1, stack limit = 0xc702e240)
[    2.377410] Stack: (0xc702fed0 to 0xc7030000)
[    2.382019] fec0:                                     c0d42180 c0d429d0 c70a7640 c07354c4
[    2.390624] fee0: 00000001 c0719798 c0d42180 c06f2cc0 c04e76cc c06ee1ac 00000000 c07354c4
[    2.399230] ff00: 00000007 c06f2d64 00000017 c06fb308 00000000 c06f07a4 c0cb8580 c06edee0
[    2.407836] ff20: c06eded4 c06e8504 c0d42180 c0008768 0000009e c00611b4 00000001 00000000
[    2.416442] ff40: c061994c c06b7548 00000007 00000007 00000000 c07354c4 00000007 c0719798
[    2.425048] ff60: c0d42180 c06e51c8 c07197a0 0000009e 00000000 c06e590c 00000007 00000007
[    2.433654] ff80: c06e51c8 00000000 00000000 c04d26ec 00000000 00000000 00000000 00000000
[    2.442260] ffa0: 00000000 c04d26f4 00000000 c00137b0 00000000 00000000 00000000 00000000
[    2.450866] ffc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[    2.459472] ffe0: 00000000 00000000 00000000 00000000 00000013 00000000 80fb6c10 71bbcd20
[    2.468078] [<c0333950>] (twl_i2c_read+0x3c/0xec) from [<c06f2cc0>] (omap3_twl_set_sr_bit+0x3c/0xb4)
[    2.477722] [<c06f2cc0>] (omap3_twl_set_sr_bit+0x3c/0xb4) from [<c06f2d64>] (omap3_twl_init+0x2c/0x70)
[    2.487518] [<c06f2d64>] (omap3_twl_init+0x2c/0x70) from [<c06fb308>] (omap_pmic_late_init+0x18/0x24)
[    2.497222] [<c06fb308>] (omap_pmic_late_init+0x18/0x24) from [<c06f07a4>] (omap2_common_pm_late_init+0x18/0xd0)
[    2.507934] [<c06f07a4>] (omap2_common_pm_late_init+0x18/0xd0) from [<c06edee0>] (omap3_init_late+0xc/0x18)
[    2.518188] [<c06edee0>] (omap3_init_late+0xc/0x18) from [<c06e8504>] (init_machine_late+0x1c/0x28)
[    2.527740] [<c06e8504>] (init_machine_late+0x1c/0x28) from [<c0008768>] (do_one_initcall+0x100/0x16c)
[    2.537536] [<c0008768>] (do_one_initcall+0x100/0x16c) from [<c06e590c>] (kernel_init_freeable+0xfc/0x1c8)
[    2.547698] [<c06e590c>] (kernel_init_freeable+0xfc/0x1c8) from [<c04d26f4>] (kernel_init+0x8/0xe4)
[    2.557250] [<c04d26f4>] (kernel_init+0x8/0xe4) from [<c00137b0>] (ret_from_fork+0x14/0x24)

> But the fix is valid.
 
Thanks. I saw this on linux-next earlier this week, but now I am not seeing it and the fix
I posted is not there. So I am not sure what changed to cause this to occur earlier this
week but the problem appears to be gone again. However, at least the fix will prevent such
panics if someone is calling twl_i2c_read/write too early.

Cheers
Jon

Re: [PATCH v2 09/11] mfd: twl-core: Collect global variables behind one private structure (global)

[Peter Ujfalusi]

Hi Jon,

On 02/12/2013 05:15 PM, Jon Hunter wrote:
> 
> On 02/12/2013 01:26 AM, Peter Ujfalusi wrote:
>> On 02/11/2013 09:22 PM, Jon Hunter wrote:
>>> Good point. I just noticed that none of my omap2+ board were booting and
>>> on omap3/4 I was the panic in the twl code. I can't say that I checked
>>> the panic on omap2, so may be that was another problem?
>>
>> Do you have insights on the code path leading to a crash on OMAP3/4? I have
>> been running this code on several boards (BeagleBoard, Zoom2, PandaBoard,
>> Blaze) and have not seen a crash.
> 
> Here is the panic log ...

Thanks.

> 
> [    2.286132] Unable to handle kernel NULL pointer dereference at virtual address 00000000
> [    2.294738] pgd = c0004000
> [    2.297576] [00000000] *pgd=00000000
> [    2.301361] Internal error: Oops: 5 [#1] SMP ARM
> [    2.306243] Modules linked in:
> [    2.309448] CPU: 0    Not tainted  (3.8.0-rc6-next-20130207-00016-g735c237 #35)
> [    2.317169] PC is at twl_i2c_read+0x3c/0xec
> [    2.321563] LR is at twl_i2c_read+0x1c/0xec
> [    2.325988] pc : [<c0333950>]    lr : [<c0333930>]    psr: 80000153
> [    2.325988] sp : c702fed0  ip : 00000000  fp : 00000000
> [    2.338043] r10: c702e000  r9 : c06e84e8  r8 : c06e51c8
> [    2.343536] r7 : 00000001  r6 : 00000006  r5 : c702fef6  r4 : 00000004
> [    2.350402] r3 : c129508c  r2 : 00000006  r1 : c702fef6  r0 : 0000000e
> [    2.357269] Flags: Nzcv  IRQs on  FIQs off  Mode SVC_32  ISA ARM  Segment kernel
> [    2.365051] Control: 10c5387d  Table: 80004019  DAC: 00000017
> [    2.371093] Process swapper/0 (pid: 1, stack limit = 0xc702e240)
> [    2.377410] Stack: (0xc702fed0 to 0xc7030000)
> [    2.382019] fec0:                                     c0d42180 c0d429d0 c70a7640 c07354c4
> [    2.390624] fee0: 00000001 c0719798 c0d42180 c06f2cc0 c04e76cc c06ee1ac 00000000 c07354c4
> [    2.399230] ff00: 00000007 c06f2d64 00000017 c06fb308 00000000 c06f07a4 c0cb8580 c06edee0
> [    2.407836] ff20: c06eded4 c06e8504 c0d42180 c0008768 0000009e c00611b4 00000001 00000000
> [    2.416442] ff40: c061994c c06b7548 00000007 00000007 00000000 c07354c4 00000007 c0719798
> [    2.425048] ff60: c0d42180 c06e51c8 c07197a0 0000009e 00000000 c06e590c 00000007 00000007
> [    2.433654] ff80: c06e51c8 00000000 00000000 c04d26ec 00000000 00000000 00000000 00000000
> [    2.442260] ffa0: 00000000 c04d26f4 00000000 c00137b0 00000000 00000000 00000000 00000000
> [    2.450866] ffc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> [    2.459472] ffe0: 00000000 00000000 00000000 00000000 00000013 00000000 80fb6c10 71bbcd20
> [    2.468078] [<c0333950>] (twl_i2c_read+0x3c/0xec) from [<c06f2cc0>] (omap3_twl_set_sr_bit+0x3c/0xb4)
> [    2.477722] [<c06f2cc0>] (omap3_twl_set_sr_bit+0x3c/0xb4) from [<c06f2d64>] (omap3_twl_init+0x2c/0x70)
> [    2.487518] [<c06f2d64>] (omap3_twl_init+0x2c/0x70) from [<c06fb308>] (omap_pmic_late_init+0x18/0x24)
> [    2.497222] [<c06fb308>] (omap_pmic_late_init+0x18/0x24) from [<c06f07a4>] (omap2_common_pm_late_init+0x18/0xd0)
> [    2.507934] [<c06f07a4>] (omap2_common_pm_late_init+0x18/0xd0) from [<c06edee0>] (omap3_init_late+0xc/0x18)
> [    2.518188] [<c06edee0>] (omap3_init_late+0xc/0x18) from [<c06e8504>] (init_machine_late+0x1c/0x28)
> [    2.527740] [<c06e8504>] (init_machine_late+0x1c/0x28) from [<c0008768>] (do_one_initcall+0x100/0x16c)
> [    2.537536] [<c0008768>] (do_one_initcall+0x100/0x16c) from [<c06e590c>] (kernel_init_freeable+0xfc/0x1c8)
> [    2.547698] [<c06e590c>] (kernel_init_freeable+0xfc/0x1c8) from [<c04d26f4>] (kernel_init+0x8/0xe4)
> [    2.557250] [<c04d26f4>] (kernel_init+0x8/0xe4) from [<c00137b0>] (ret_from_fork+0x14/0x24)

This is exactly the sort of thing which should not ever existed in the first
place...
The code in omap_twl.c, especially the omap3_twl_set_sr_bit() function is
executed in .init_late of the board. If the twl stack is not up by then we are
going to have issues.
Right now I don't see how to solve this (the call chain is related to
smartreflex) but IMHO we should not have these 'random' twl_write calls spread
across the kernel without integrating them to the twl stack in a proper
manner. What I mean is that we should only have twl_* calls from drivers,
which has been created by the twl-core MFD core.

-- 
Péter