[PATCH v2 0/2] mfd: dt: add power off support for Tegra20/Tegra30

This patch series add new property into regulator DT for telling whether or not
to hook pmic's power off routine to system call "pm_power_off".

Patch 1 add power off support for Tegra20 boards using TPS6586x
Patch 2 add power off support for Tegra30 boards using TPS65910

Verified on Seaboard (Tegra20) and Cardhu (Tegra30)

V2:
* Take multiple pmic instances into consideration while assigning global variables
  as per suggestion from Thierry Reding [off-list ref]

V1:
* Based on master branch of sameo/mfd-2.6.git

Bill Huang (2):
  mfd: dt: tps6586x: Add power off control
  mfd: dt: tps65910: add power off control

 Documentation/devicetree/bindings/mfd/tps65910.txt |    4 +++
 .../devicetree/bindings/regulator/tps6586x.txt     |    6 +++++
 drivers/mfd/tps6586x.c                             |   19 +++++++++++++++++
 drivers/mfd/tps65910.c                             |   22 ++++++++++++++++++++
 include/linux/mfd/tps6586x.h                       |    1 +
 include/linux/mfd/tps65910.h                       |    3 ++
 6 files changed, 55 insertions(+), 0 deletions(-)

-- 
1.7.4.1

Add DT property "ti,system-power-controller" telling whether or not this
pmic is in charge of controlling the system power, so the power off
routine can be hooked up to system call "pm_power_off".

Based on the work by:
Dan Willemsen [off-list ref]

Signed-off-by: Bill Huang <redacted>
Tested-by: Stephen Warren <redacted>
---
 Documentation/devicetree/bindings/mfd/tps65910.txt |    4 +++
 drivers/mfd/tps65910.c                             |   22 ++++++++++++++++++++
 include/linux/mfd/tps65910.h                       |    3 ++
 3 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/tps65910.txt b/Documentation/devicetree/bindings/mfd/tps65910.txt
index db03599..2e33048 100644
--- a/Documentation/devicetree/bindings/mfd/tps65910.txt
+++ b/Documentation/devicetree/bindings/mfd/tps65910.txt

@@ -59,6 +59,8 @@ Optional properties:
   in TPS6591X datasheet)
 - ti,en-gpio-sleep: enable sleep control for gpios
   There should be 9 entries here, one for each gpio.
+- ti,system-power-controller: Telling whether or not this pmic is controlling
+  the system power.
 
 Regulator Optional properties:
 - ti,regulator-ext-sleep-control: enable external sleep

@@ -79,6 +81,8 @@ Example:
 		#interrupt-cells = <2>;
 		interrupt-controller;
 
+		ti,system-power-controller;
+
 		ti,vmbch-threshold = 0;
 		ti,vmbch2-threshold = 0;
 		ti,en-ck32k-xtal;

diff --git a/drivers/mfd/tps65910.c b/drivers/mfd/tps65910.c
index 1c56379..574ae2b 100644
--- a/drivers/mfd/tps65910.c
+++ b/drivers/mfd/tps65910.c

@@ -198,6 +198,8 @@ static struct tps65910_board *tps65910_parse_dt(struct i2c_client *client,
 
 	board_info->irq = client->irq;
 	board_info->irq_base = -1;
+	board_info->pm_off = of_property_read_bool(np,
+			"ti,system-power-controller");
 
 	return board_info;
 }

@@ -210,6 +212,21 @@ struct tps65910_board *tps65910_parse_dt(struct i2c_client *client,
 }
 #endif
 
+static struct i2c_client *tps65910_i2c_client;
+static void tps65910_power_off(void)
+{
+	struct tps65910 *tps65910;
+
+	tps65910 = dev_get_drvdata(&tps65910_i2c_client->dev);
+
+	if (tps65910_reg_set_bits(tps65910, TPS65910_DEVCTRL,
+			DEVCTRL_PWR_OFF_MASK) < 0)
+		return;
+
+	tps65910_reg_clear_bits(tps65910, TPS65910_DEVCTRL,
+			DEVCTRL_DEV_ON_MASK);
+}
+
 static __devinit int tps65910_i2c_probe(struct i2c_client *i2c,
 					const struct i2c_device_id *id)
 {

@@ -267,6 +284,11 @@ static __devinit int tps65910_i2c_probe(struct i2c_client *i2c,
 	tps65910_ck32k_init(tps65910, pmic_plat_data);
 	tps65910_sleepinit(tps65910, pmic_plat_data);
 
+	if (pmic_plat_data->pm_off && !pm_power_off) {
+		tps65910_i2c_client = i2c;
+		pm_power_off = tps65910_power_off;
+	}
+
 	return ret;
 }
 

diff --git a/include/linux/mfd/tps65910.h b/include/linux/mfd/tps65910.h
index 9bf8767..ac772b3 100644
--- a/include/linux/mfd/tps65910.h
+++ b/include/linux/mfd/tps65910.h

@@ -366,6 +366,8 @@
 
 
 /*Register DEVCTRL  (0x80) register.RegisterDescription */
+#define DEVCTRL_PWR_OFF_MASK				0x80
+#define DEVCTRL_PWR_OFF_SHIFT				7
 #define DEVCTRL_RTC_PWDN_MASK				0x40
 #define DEVCTRL_RTC_PWDN_SHIFT				6
 #define DEVCTRL_CK32K_CTRL_MASK				0x20

@@ -809,6 +811,7 @@ struct tps65910_board {
 	int vmbch2_threshold;
 	bool en_ck32k_xtal;
 	bool en_dev_slp;
+	bool pm_off;
 	struct tps65910_sleep_keepon_data *slp_keepon;
 	bool en_gpio_sleep[TPS6591X_MAX_NUM_GPIO];
 	unsigned long regulator_ext_sleep_control[TPS65910_NUM_REGS];

-- 
1.7.4.1

Add DT property "ti,system-power-controller" telling whether or not this
pmic is in charge of controlling the system power, so the power off
routine can be hooked up to system call "pm_power_off".

Based on the work by:
Dan Willemsen [off-list ref]

Signed-off-by: Bill Huang <redacted>
Tested-by: Stephen Warren <redacted>
---
 .../devicetree/bindings/regulator/tps6586x.txt     |    6 ++++++
 drivers/mfd/tps6586x.c                             |   19 +++++++++++++++++++
 include/linux/mfd/tps6586x.h                       |    1 +
 3 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/Documentation/devicetree/bindings/regulator/tps6586x.txt b/Documentation/devicetree/bindings/regulator/tps6586x.txt
index d156e1b..03dfa4e 100644
--- a/Documentation/devicetree/bindings/regulator/tps6586x.txt
+++ b/Documentation/devicetree/bindings/regulator/tps6586x.txt

@@ -18,6 +18,10 @@ Required properties:
 - vinldo678-supply: The input supply for the LDO6, LDO7 and LDO8
 - vinldo9-supply: The input supply for the LDO9
 
+Optional properties:
+- ti,system-power-controller: Telling whether or not this pmic is controlling
+  the system power.
+
 Each regulator is defined using the standard binding for regulators.
 
 Example:

@@ -30,6 +34,8 @@ Example:
 		#gpio-cells = <2>;
 		gpio-controller;
 
+		ti,system-power-controller;
+
 		sm0-supply = <&some_reg>;
 		sm1-supply = <&some_reg>;
 		sm2-supply = <&some_reg>;

diff --git a/drivers/mfd/tps6586x.c b/drivers/mfd/tps6586x.c
index 353c348..d08f59c 100644
--- a/drivers/mfd/tps6586x.c
+++ b/drivers/mfd/tps6586x.c

@@ -29,6 +29,10 @@
 #include <linux/mfd/core.h>
 #include <linux/mfd/tps6586x.h>
 
+#define TPS6586X_SUPPLYENE	0x14
+#define EXITSLREQ_BIT		BIT(1)
+#define SLEEP_MODE_BIT		BIT(3)
+
 /* interrupt control registers */
 #define TPS6586X_INT_ACK1	0xb5
 #define TPS6586X_INT_ACK2	0xb6

@@ -409,6 +413,7 @@ static struct tps6586x_platform_data *tps6586x_parse_dt(struct i2c_client *clien
 	pdata->subdevs = devs;
 	pdata->gpio_base = -1;
 	pdata->irq_base = -1;
+	pdata->pm_off = of_property_read_bool(np, "ti,system-power-controller");
 
 	return pdata;
 }

@@ -441,6 +446,15 @@ static const struct regmap_config tps6586x_regmap_config = {
 	.cache_type = REGCACHE_RBTREE,
 };
 
+static struct device *tps6586x_dev;
+static void tps6586x_power_off(void)
+{
+	if (tps6586x_clr_bits(tps6586x_dev, TPS6586X_SUPPLYENE, EXITSLREQ_BIT))
+		return;
+
+	tps6586x_set_bits(tps6586x_dev, TPS6586X_SUPPLYENE, SLEEP_MODE_BIT);
+}
+
 static int __devinit tps6586x_i2c_probe(struct i2c_client *client,
 					const struct i2c_device_id *id)
 {

@@ -505,6 +519,11 @@ static int __devinit tps6586x_i2c_probe(struct i2c_client *client,
 		goto err_add_devs;
 	}
 
+	if (pdata->pm_off && !pm_power_off) {
+		tps6586x_dev = &client->dev;
+		pm_power_off = tps6586x_power_off;
+	}
+
 	return 0;
 
 err_add_devs:

diff --git a/include/linux/mfd/tps6586x.h b/include/linux/mfd/tps6586x.h
index f350fd0..08f109f 100644
--- a/include/linux/mfd/tps6586x.h
+++ b/include/linux/mfd/tps6586x.h

@@ -77,6 +77,7 @@ struct tps6586x_platform_data {
 
 	int gpio_base;
 	int irq_base;
+	bool pm_off;
 };
 
 /*

-- 
1.7.4.1

On Sun, Aug 19, 2012 at 06:07:55PM -0700, Bill Huang wrote:

Add DT property "ti,system-power-controller" telling whether or not this
pmic is in charge of controlling the system power, so the power off
routine can be hooked up to system call "pm_power_off".

Based on the work by:
Dan Willemsen [off-list ref]

Signed-off-by: Bill Huang <redacted>
Tested-by: Stephen Warren <redacted>
---
 .../devicetree/bindings/regulator/tps6586x.txt     |    6 ++++++
 drivers/mfd/tps6586x.c                             |   19 +++++++++++++++++++
 include/linux/mfd/tps6586x.h                       |    1 +
 3 files changed, 26 insertions(+), 0 deletions(-)

Hi,

I've seen the following while trying this patch applied on top of next-20120817:

[   40.581151] Power down.
[   41.583160] ------------[ cut here ]------------
[   41.587784] WARNING: at /home/thierry.reding/src/kernel/linux-ipmp.git/drivers/i2c/busses/i2c-tegra.c:525 tegra_i2c_xfer+0x21c/0x29c()
[   41.599850] Modules linked in:
[   41.602927] [<c0014074>] (unwind_backtrace+0x0/0xf8) from [<c00265a8>] (warn_slowpath_common+0x4c/0x64)
[   41.612304] [<c00265a8>] (warn_slowpath_common+0x4c/0x64) from [<c00265dc>] (warn_slowpath_null+0x1c/0x24)
[   41.621947] [<c00265dc>] (warn_slowpath_null+0x1c/0x24) from [<c02a2944>] (tegra_i2c_xfer+0x21c/0x29c)
[   41.631244] [<c02a2944>] (tegra_i2c_xfer+0x21c/0x29c) from [<c029f5e8>] (__i2c_transfer+0x44/0x80)
[   41.640192] [<c029f5e8>] (__i2c_transfer+0x44/0x80) from [<c02a0698>] (i2c_transfer+0x7c/0xb8)
[   41.648796] [<c02a0698>] (i2c_transfer+0x7c/0xb8) from [<c0232a60>] (regmap_i2c_read+0x48/0x64)
[   41.657485] [<c0232a60>] (regmap_i2c_read+0x48/0x64) from [<c0230304>] (_regmap_raw_read+0x90/0x98)
[   41.666518] [<c0230304>] (_regmap_raw_read+0x90/0x98) from [<c023035c>] (_regmap_read+0x50/0xa8)
[   41.675290] [<c023035c>] (_regmap_read+0x50/0xa8) from [<c02300c4>] (_regmap_update_bits+0x24/0x64)
[   41.684322] [<c02300c4>] (_regmap_update_bits+0x24/0x64) from [<c0230b88>] (regmap_update_bits+0x3c/0x58)
[   41.693885] [<c0230b88>] (regmap_update_bits+0x3c/0x58) from [<c0237c30>] (tps6586x_power_off+0x18/0x38)
[   41.703362] [<c0237c30>] (tps6586x_power_off+0x18/0x38) from [<c000edf4>] (machine_power_off+0x1c/0x24)
[   41.712749] [<c000edf4>] (machine_power_off+0x1c/0x24) from [<c0037ca4>] (sys_reboot+0x138/0x1b0)
[   41.721612] [<c0037ca4>] (sys_reboot+0x138/0x1b0) from [<c000e000>] (ret_fast_syscall+0x0/0x30)
[   41.730293] ---[ end trace 9af366974fefa459 ]---
[   41.734906] tegra-i2c tegra-i2c.3: i2c transfer timed out
[   41.740689] Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000000
[   41.740689] 
[   41.749823] [<c0014074>] (unwind_backtrace+0x0/0xf8) from [<c03e7090>] (panic+0x8c/0x1d8)
[   41.757993] [<c03e7090>] (panic+0x8c/0x1d8) from [<c002b7b8>] (do_exit+0x694/0x750)
[   41.765636] [<c002b7b8>] (do_exit+0x694/0x750) from [<c002bad0>] (do_group_exit+0x3c/0xb0)
[   41.773884] [<c002bad0>] (do_group_exit+0x3c/0xb0) from [<c002bb54>] (__wake_up_parent+0x0/0x18)

Thierry

nvpublic

On Sun, Aug 19, 2012 at 06:07:55PM -0700, Bill Huang wrote:

quoted

Add DT property "ti,system-power-controller" telling whether or not
this pmic is in charge of controlling the system power, so the power
off routine can be hooked up to system call "pm_power_off".

Based on the work by:
Dan Willemsen [off-list ref]

Signed-off-by: Bill Huang <redacted>
Tested-by: Stephen Warren <redacted>
---
 .../devicetree/bindings/regulator/tps6586x.txt     |    6 ++++++
 drivers/mfd/tps6586x.c                             |   19 +++++++++++++++++++
 include/linux/mfd/tps6586x.h                       |    1 +
 3 files changed, 26 insertions(+), 0 deletions(-)

Hi,

I've seen the following while trying this patch applied on top of next-20120817:

[   40.581151] Power down.
[   41.583160] ------------[ cut here ]------------
[   41.587784] WARNING: at /home/thierry.reding/src/kernel/linux-ipmp.git/drivers/i2c/busses/i2c-
tegra.c:525 tegra_i2c_xfer+0x21c/0x29c()
[   41.599850] Modules linked in:
[   41.602927] [<c0014074>] (unwind_backtrace+0x0/0xf8) from [<c00265a8>]
(warn_slowpath_common+0x4c/0x64)
[   41.612304] [<c00265a8>] (warn_slowpath_common+0x4c/0x64) from [<c00265dc>]
(warn_slowpath_null+0x1c/0x24)
[   41.621947] [<c00265dc>] (warn_slowpath_null+0x1c/0x24) from [<c02a2944>]
(tegra_i2c_xfer+0x21c/0x29c)
[   41.631244] [<c02a2944>] (tegra_i2c_xfer+0x21c/0x29c) from [<c029f5e8>] (__i2c_transfer+0x44/0x80)
[   41.640192] [<c029f5e8>] (__i2c_transfer+0x44/0x80) from [<c02a0698>] (i2c_transfer+0x7c/0xb8)
[   41.648796] [<c02a0698>] (i2c_transfer+0x7c/0xb8) from [<c0232a60>] (regmap_i2c_read+0x48/0x64)
[   41.657485] [<c0232a60>] (regmap_i2c_read+0x48/0x64) from [<c0230304>] (_regmap_raw_read+0x90/0x98)
[   41.666518] [<c0230304>] (_regmap_raw_read+0x90/0x98) from [<c023035c>] (_regmap_read+0x50/0xa8)
[   41.675290] [<c023035c>] (_regmap_read+0x50/0xa8) from [<c02300c4>] (_regmap_update_bits+0x24/0x64)
[   41.684322] [<c02300c4>] (_regmap_update_bits+0x24/0x64) from [<c0230b88>]
(regmap_update_bits+0x3c/0x58)
[   41.693885] [<c0230b88>] (regmap_update_bits+0x3c/0x58) from [<c0237c30>]
(tps6586x_power_off+0x18/0x38)
[   41.703362] [<c0237c30>] (tps6586x_power_off+0x18/0x38) from [<c000edf4>]
(machine_power_off+0x1c/0x24)
[   41.712749] [<c000edf4>] (machine_power_off+0x1c/0x24) from [<c0037ca4>] (sys_reboot+0x138/0x1b0)
[   41.721612] [<c0037ca4>] (sys_reboot+0x138/0x1b0) from [<c000e000>] (ret_fast_syscall+0x0/0x30)
[   41.730293] ---[ end trace 9af366974fefa459 ]---
[   41.734906] tegra-i2c tegra-i2c.3: i2c transfer timed out
[   41.740689] Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000000
[   41.740689]
[   41.749823] [<c0014074>] (unwind_backtrace+0x0/0xf8) from [<c03e7090>] (panic+0x8c/0x1d8)
[   41.757993] [<c03e7090>] (panic+0x8c/0x1d8) from [<c002b7b8>] (do_exit+0x694/0x750)
[   41.765636] [<c002b7b8>] (do_exit+0x694/0x750) from [<c002bad0>] (do_group_exit+0x3c/0xb0)
[   41.773884] [<c002bad0>] (do_group_exit+0x3c/0xb0) from [<c002bb54>] (__wake_up_parent+0x0/0x18)

Thanks Thierry, I can repro this on Tegra20 inconsistently and found, if current cpu is not cpu0 when doing "machine_shutdown" (it will call "smp_send_stop"), i2c controller will failed to do any transaction (looks like gic interrupt will be disabled), I'll debug further to find out the root cause.

By the way, Tegra30 is good since it will always be cpu0 when doing "machine_shutdown", I still don't know why it makes the difference against Tegra20 since I'm not familiar with those cpu stuffs and what make it behave differently, I'll study a bit, thanks.

Thierry

* Unknown Key
* 0x7F3EB3A1

nvpublic

quoted

On Sun, Aug 19, 2012 at 06:07:55PM -0700, Bill Huang wrote:

quoted

Add DT property "ti,system-power-controller" telling whether or not
this pmic is in charge of controlling the system power, so the power
off routine can be hooked up to system call "pm_power_off".

Based on the work by:
Dan Willemsen [off-list ref]

Signed-off-by: Bill Huang <redacted>
Tested-by: Stephen Warren <redacted>
---
 .../devicetree/bindings/regulator/tps6586x.txt     |    6 ++++++
 drivers/mfd/tps6586x.c                             |   19 +++++++++++++++++++
 include/linux/mfd/tps6586x.h                       |    1 +
 3 files changed, 26 insertions(+), 0 deletions(-)

Hi,

I've seen the following while trying this patch applied on top of next-20120817:

[   40.581151] Power down.
[   41.583160] ------------[ cut here ]------------
[   41.587784] WARNING: at /home/thierry.reding/src/kernel/linux-ipmp.git/drivers/i2c/busses/i2c-
tegra.c:525 tegra_i2c_xfer+0x21c/0x29c()
[   41.599850] Modules linked in:
[   41.602927] [<c0014074>] (unwind_backtrace+0x0/0xf8) from [<c00265a8>]
(warn_slowpath_common+0x4c/0x64)
[   41.612304] [<c00265a8>] (warn_slowpath_common+0x4c/0x64) from [<c00265dc>]
(warn_slowpath_null+0x1c/0x24)
[   41.621947] [<c00265dc>] (warn_slowpath_null+0x1c/0x24) from [<c02a2944>]
(tegra_i2c_xfer+0x21c/0x29c)
[   41.631244] [<c02a2944>] (tegra_i2c_xfer+0x21c/0x29c) from [<c029f5e8>] (__i2c_transfer+0x44/0x80)
[   41.640192] [<c029f5e8>] (__i2c_transfer+0x44/0x80) from [<c02a0698>] (i2c_transfer+0x7c/0xb8)
[   41.648796] [<c02a0698>] (i2c_transfer+0x7c/0xb8) from [<c0232a60>] (regmap_i2c_read+0x48/0x64)
[   41.657485] [<c0232a60>] (regmap_i2c_read+0x48/0x64) from [<c0230304>]

(_regmap_raw_read+0x90/0x98)

quoted

[   41.666518] [<c0230304>] (_regmap_raw_read+0x90/0x98) from [<c023035c>] (_regmap_read+0x50/0xa8)
[   41.675290] [<c023035c>] (_regmap_read+0x50/0xa8) from [<c02300c4>]

(_regmap_update_bits+0x24/0x64)

quoted

[   41.684322] [<c02300c4>] (_regmap_update_bits+0x24/0x64) from [<c0230b88>]
(regmap_update_bits+0x3c/0x58)
[   41.693885] [<c0230b88>] (regmap_update_bits+0x3c/0x58) from [<c0237c30>]
(tps6586x_power_off+0x18/0x38)
[   41.703362] [<c0237c30>] (tps6586x_power_off+0x18/0x38) from [<c000edf4>]
(machine_power_off+0x1c/0x24)
[   41.712749] [<c000edf4>] (machine_power_off+0x1c/0x24) from [<c0037ca4>] (sys_reboot+0x138/0x1b0)
[   41.721612] [<c0037ca4>] (sys_reboot+0x138/0x1b0) from [<c000e000>] (ret_fast_syscall+0x0/0x30)
[   41.730293] ---[ end trace 9af366974fefa459 ]---
[   41.734906] tegra-i2c tegra-i2c.3: i2c transfer timed out
[   41.740689] Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000000
[   41.740689]
[   41.749823] [<c0014074>] (unwind_backtrace+0x0/0xf8) from [<c03e7090>] (panic+0x8c/0x1d8)
[   41.757993] [<c03e7090>] (panic+0x8c/0x1d8) from [<c002b7b8>] (do_exit+0x694/0x750)
[   41.765636] [<c002b7b8>] (do_exit+0x694/0x750) from [<c002bad0>] (do_group_exit+0x3c/0xb0)
[   41.773884] [<c002bad0>] (do_group_exit+0x3c/0xb0) from [<c002bb54>] (__wake_up_parent+0x0/0x18)

Thanks Thierry, I can repro this on Tegra20 inconsistently and found, if current cpu is not cpu0 when
doing "machine_shutdown" (it will call "smp_send_stop"), i2c controller will failed to do any
transaction (looks like gic interrupt will be disabled), I'll debug further to find out the root cause.

By the way, Tegra30 is good since it will always be cpu0 when doing "machine_shutdown", I still don't
know why it makes the difference against Tegra20 since I'm not familiar with those cpu stuffs and what
make it behave differently, I'll study a bit, thanks.

I've sent the shutdown issue for discussion in ARM list: Shutdown problem in SMP system happened on Tegra20.
The cause of the i2c timeout is pretty clear now and it is not directly related to this patch, so is this
patch series acceptable? Any thoughts or comment? Thanks.

quoted

Thierry

* Unknown Key
* 0x7F3EB3A1

On 08/24/2012 06:36 PM, Bill Huang wrote:

quoted

quoted

On Sun, Aug 19, 2012 at 06:07:55PM -0700, Bill Huang wrote:

quoted

Add DT property "ti,system-power-controller" telling whether or not
this pmic is in charge of controlling the system power, so the power
off routine can be hooked up to system call "pm_power_off".

...

quoted

quoted

I've seen the following while trying this patch applied on top of next-20120817:

[   40.581151] Power down.
[   41.583160] ------------[ cut here ]------------
[   41.587784] WARNING: at /home/thierry.reding/src/kernel/linux-ipmp.git/drivers/i2c/busses/i2c-
tegra.c:525 tegra_i2c_xfer+0x21c/0x29c()

...

quoted

Thanks Thierry, I can repro this on Tegra20 inconsistently and found, if current cpu is not cpu0 when
doing "machine_shutdown" (it will call "smp_send_stop"), i2c controller will failed to do any
transaction (looks like gic interrupt will be disabled), I'll debug further to find out the root cause.

By the way, Tegra30 is good since it will always be cpu0 when doing "machine_shutdown", I still don't
know why it makes the difference against Tegra20 since I'm not familiar with those cpu stuffs and what
make it behave differently, I'll study a bit, thanks.

I've sent the shutdown issue for discussion in ARM list: Shutdown problem in SMP system happened on Tegra20.
The cause of the i2c timeout is pretty clear now and it is not directly related to this patch, so is this
patch series acceptable? Any thoughts or comment? Thanks.

I tend to agree; power off never worked without this patch, and
sometimes does with the patch, due to nothing wrong with this patch.

Bill, please do follow up on getting the underlying Tegra issue solved
somehow though. IIRC, Joseph Lo or Prashant has a patch which enabled
the config option that Russell mentioned, so the fix may just be to wait
for that patch to get finalized, but please double-check that solves it.
Thanks!

nvpublic

On 08/24/2012 06:36 PM, Bill Huang wrote:

quoted

quoted

quoted

On Sun, Aug 19, 2012 at 06:07:55PM -0700, Bill Huang wrote:

quoted

Add DT property "ti,system-power-controller" telling whether or not
this pmic is in charge of controlling the system power, so the
power off routine can be hooked up to system call "pm_power_off".

...

quoted

quoted

quoted

I've seen the following while trying this patch applied on top of next-20120817:

[   40.581151] Power down.
[   41.583160] ------------[ cut here ]------------
[   41.587784] WARNING: at /home/thierry.reding/src/kernel/linux-ipmp.git/drivers/i2c/busses/i2c-
tegra.c:525 tegra_i2c_xfer+0x21c/0x29c()

...

quoted

quoted

Thanks Thierry, I can repro this on Tegra20 inconsistently and found,
if current cpu is not cpu0 when doing "machine_shutdown" (it will
call "smp_send_stop"), i2c controller will failed to do any transaction (looks like gic interrupt

will be disabled), I'll debug further to find out the root cause.

quoted

quoted

By the way, Tegra30 is good since it will always be cpu0 when doing
"machine_shutdown", I still don't know why it makes the difference
against Tegra20 since I'm not familiar with those cpu stuffs and what make it behave differently,

I'll study a bit, thanks.

quoted

I've sent the shutdown issue for discussion in ARM list: Shutdown problem in SMP system happened on

Tegra20.

quoted

The cause of the i2c timeout is pretty clear now and it is not
directly related to this patch, so is this patch series acceptable? Any thoughts or comment? Thanks.

I tend to agree; power off never worked without this patch, and sometimes does with the patch, due to
nothing wrong with this patch.

Bill, please do follow up on getting the underlying Tegra issue solved somehow though. IIRC, Joseph Lo
or Prashant has a patch which enabled the config option that Russell mentioned, so the fix may just be
to wait for that patch to get finalized, but please double-check that solves it.
Thanks!

As per the shutdown issue discussion, enabling CONFIG_PM_SLEEP_SMP is the only solution and I've confirmed that fix the issue, thanks. 

nvpublic

quoted

On 08/24/2012 06:36 PM, Bill Huang wrote:

quoted

quoted

quoted

On Sun, Aug 19, 2012 at 06:07:55PM -0700, Bill Huang wrote:

quoted

Add DT property "ti,system-power-controller" telling whether or
not this pmic is in charge of controlling the system power, so
the power off routine can be hooked up to system call "pm_power_off".

...

quoted

quoted

quoted

I've seen the following while trying this patch applied on top of next-20120817:

[   40.581151] Power down.
[   41.583160] ------------[ cut here ]------------
[   41.587784] WARNING: at /home/thierry.reding/src/kernel/linux-

ipmp.git/drivers/i2c/busses/i2c-

quoted

quoted

quoted

quoted

tegra.c:525 tegra_i2c_xfer+0x21c/0x29c()

...

quoted

quoted

Thanks Thierry, I can repro this on Tegra20 inconsistently and
found, if current cpu is not cpu0 when doing "machine_shutdown" (it
will call "smp_send_stop"), i2c controller will failed to do any
transaction (looks like gic interrupt

will be disabled), I'll debug further to find out the root cause.

quoted

quoted

By the way, Tegra30 is good since it will always be cpu0 when doing
"machine_shutdown", I still don't know why it makes the difference
against Tegra20 since I'm not familiar with those cpu stuffs and
what make it behave differently,

I'll study a bit, thanks.

quoted

I've sent the shutdown issue for discussion in ARM list: Shutdown
problem in SMP system happened on

Tegra20.

quoted

The cause of the i2c timeout is pretty clear now and it is not
directly related to this patch, so is this patch series acceptable? Any thoughts or comment?

Thanks.

quoted

I tend to agree; power off never worked without this patch, and
sometimes does with the patch, due to nothing wrong with this patch.

Bill, please do follow up on getting the underlying Tegra issue solved
somehow though. IIRC, Joseph Lo or Prashant has a patch which enabled
the config option that Russell mentioned, so the fix may just be to wait for that patch to get

finalized, but please double-check that solves it.

quoted

Thanks!

As per the shutdown issue discussion, enabling CONFIG_PM_SLEEP_SMP is the only solution and I've
confirmed that fix the issue, thanks.

Hi sameo,

Is there any concern on this patch series?

Thanks,
Bill

Hi all, 

Could somebody review this?

Thanks,
Bill

This patch series add new property into regulator DT for telling whether or not to hook pmic's power off routine to system call
"pm_power_off".

Patch 1 add power off support for Tegra20 boards using TPS6586x Patch 2 add power off support for Tegra30 boards using TPS65910

Verified on Seaboard (Tegra20) and Cardhu (Tegra30)

V2:
* Take multiple pmic instances into consideration while assigning global variables
  as per suggestion from Thierry Reding [off-list ref]

V1:
* Based on master branch of sameo/mfd-2.6.git

Bill Huang (2):
  mfd: dt: tps6586x: Add power off control
  mfd: dt: tps65910: add power off control

 Documentation/devicetree/bindings/mfd/tps65910.txt |    4 +++
 .../devicetree/bindings/regulator/tps6586x.txt     |    6 +++++
 drivers/mfd/tps6586x.c                             |   19 +++++++++++++++++
 drivers/mfd/tps65910.c                             |   22 ++++++++++++++++++++
 include/linux/mfd/tps6586x.h                       |    1 +
 include/linux/mfd/tps65910.h                       |    3 ++
 6 files changed, 55 insertions(+), 0 deletions(-)

--
1.7.4.1

nvpublic

On Tue, Sep 11, 2012 at 06:25:14PM +0800, Bill Huang wrote:

Hi all, 

Could somebody review this?

Given that I haven't been able to test yet (due to time constraints)
with PM_SLEEP_SMP enabled, I don't want to give you a Tested-by, but the
code looks okay to me, so for both patches:

Reviewed-by: Thierry Reding <redacted>

On 09/11/2012 04:46 AM, Thierry Reding wrote:

On Tue, Sep 11, 2012 at 06:25:14PM +0800, Bill Huang wrote:

quoted

Hi all,

Could somebody review this?

Given that I haven't been able to test yet (due to time
constraints) with PM_SLEEP_SMP enabled, I don't want to give you a
Tested-by, but the code looks okay to me, so for both patches:

Reviewed-by: Thierry Reding <redacted>

I have tested this with PM_SLEEP_SMP enabled, and it solved the
problem. I think I already gave my Tested-by, but if not:

Tested-by: Stephen Warren <redacted>

I hope that this gets applied to the MFD tree early enough (i.e.
within the next 2-3 days or so) that I can rely on the binding be
accepted, and hence apply patches to Tegra's device tree to enable
this functionality for 3.7.

Bill, given this was posted about 3 weeks ago, perhaps repost the
series in case Samuel doesn't have it any more, and hence can't apply it.

Hi Bill,

On Sun, Aug 19, 2012 at 06:07:54PM -0700, Bill Huang wrote:

This patch series add new property into regulator DT for telling whether or not
to hook pmic's power off routine to system call "pm_power_off".

Patch 1 add power off support for Tegra20 boards using TPS6586x
Patch 2 add power off support for Tegra30 boards using TPS65910

Verified on Seaboard (Tegra20) and Cardhu (Tegra30)

V2:
* Take multiple pmic instances into consideration while assigning global variables
  as per suggestion from Thierry Reding [off-list ref]

V1:
* Based on master branch of sameo/mfd-2.6.git

Bill Huang (2):
  mfd: dt: tps6586x: Add power off control
  mfd: dt: tps65910: add power off control

 Documentation/devicetree/bindings/mfd/tps65910.txt |    4 +++
 .../devicetree/bindings/regulator/tps6586x.txt     |    6 +++++
 drivers/mfd/tps6586x.c                             |   19 +++++++++++++++++
 drivers/mfd/tps65910.c                             |   22 ++++++++++++++++++++
 include/linux/mfd/tps6586x.h                       |    1 +
 include/linux/mfd/tps65910.h                       |    3 ++
 6 files changed, 55 insertions(+), 0 deletions(-)

I applied those 2 patches to my for-next branch, thanks.

Cheers,
Samuel.

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

Hi Stephen,

On Tue, Sep 11, 2012 at 09:15:07AM -0600, Stephen Warren wrote:

On 09/11/2012 04:46 AM, Thierry Reding wrote:

quoted

On Tue, Sep 11, 2012 at 06:25:14PM +0800, Bill Huang wrote:

quoted

Hi all,

Could somebody review this?

Given that I haven't been able to test yet (due to time
constraints) with PM_SLEEP_SMP enabled, I don't want to give you a
Tested-by, but the code looks okay to me, so for both patches:

Reviewed-by: Thierry Reding <redacted>

I have tested this with PM_SLEEP_SMP enabled, and it solved the
problem. I think I already gave my Tested-by, but if not:

Tested-by: Stephen Warren <redacted>

I hope that this gets applied to the MFD tree early enough (i.e.
within the next 2-3 days or so) that I can rely on the binding be
accepted, and hence apply patches to Tegra's device tree to enable
this functionality for 3.7.

Bill, given this was posted about 3 weeks ago, perhaps repost the
series in case Samuel doesn't have it any more, and hence can't apply it.

No need for that. Sorry for the delay, I just applied and pushed those 2
patches.

Cheers,
Samuel.

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

On 09/11/2012 10:08 AM, Samuel Ortiz wrote:

Hi Stephen,

On Tue, Sep 11, 2012 at 09:15:07AM -0600, Stephen Warren wrote:

quoted

On 09/11/2012 04:46 AM, Thierry Reding wrote:

quoted

On Tue, Sep 11, 2012 at 06:25:14PM +0800, Bill Huang wrote:

quoted

Hi all,

Could somebody review this?

Given that I haven't been able to test yet (due to time
constraints) with PM_SLEEP_SMP enabled, I don't want to give you a
Tested-by, but the code looks okay to me, so for both patches:

Reviewed-by: Thierry Reding <redacted>

I have tested this with PM_SLEEP_SMP enabled, and it solved the
problem. I think I already gave my Tested-by, but if not:

Tested-by: Stephen Warren <redacted>

I hope that this gets applied to the MFD tree early enough (i.e.
within the next 2-3 days or so) that I can rely on the binding be
accepted, and hence apply patches to Tegra's device tree to enable
this functionality for 3.7.

Bill, given this was posted about 3 weeks ago, perhaps repost the
series in case Samuel doesn't have it any more, and hence can't apply it.

No need for that. Sorry for the delay, I just applied and pushed those 2
patches.

Great! Thanks.

Do you have "[PATCH V4 REPOST] mfd: add MAX8907 core driver" on your
list too?

On Tue, Sep 11, 2012 at 10:26:55AM -0600, Stephen Warren wrote:

On 09/11/2012 10:08 AM, Samuel Ortiz wrote:

quoted

Hi Stephen,

On Tue, Sep 11, 2012 at 09:15:07AM -0600, Stephen Warren wrote:

quoted

On 09/11/2012 04:46 AM, Thierry Reding wrote:

quoted

On Tue, Sep 11, 2012 at 06:25:14PM +0800, Bill Huang wrote:

quoted

Hi all,

Could somebody review this?

Given that I haven't been able to test yet (due to time
constraints) with PM_SLEEP_SMP enabled, I don't want to give you a
Tested-by, but the code looks okay to me, so for both patches:

Reviewed-by: Thierry Reding <redacted>

I have tested this with PM_SLEEP_SMP enabled, and it solved the
problem. I think I already gave my Tested-by, but if not:

Tested-by: Stephen Warren <redacted>

I hope that this gets applied to the MFD tree early enough (i.e.
within the next 2-3 days or so) that I can rely on the binding be
accepted, and hence apply patches to Tegra's device tree to enable
this functionality for 3.7.

Bill, given this was posted about 3 weeks ago, perhaps repost the
series in case Samuel doesn't have it any more, and hence can't apply it.

No need for that. Sorry for the delay, I just applied and pushed those 2
patches.

Great! Thanks.

Do you have "[PATCH V4 REPOST] mfd: add MAX8907 core driver" on your
list too?

Yes, I'll be chewing my MFD pending patches list in the next days, and this
one is on the list.

Cheers,
Samuel.

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