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

August 17th, 2012 - 05:20 am ET by Bill Huang | Report spam
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)

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

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
email Follow the discussionReplies 7 repliesReplies Make a reply

Similar topics

Replies

#1 Bill Huang
August 17th, 2012 - 05:20 am ET | Report spam
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

Signed-off-by: Bill Huang

.../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..93d57df 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;
}

+ tps6586x_dev = &client->dev;
+
+ if (pdata->pm_off && !pm_power_off)
+ 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

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Replies Reply to this message
#2 Thierry Reding
August 17th, 2012 - 06:50 am ET | Report spam

On Fri, Aug 17, 2012 at 02:16:28AM -0700, Bill Huang wrote:
[...]
diff --git a/drivers/mfd/tps6586x.c b/drivers/mfd/tps6586x.c


[...]
@@ -505,6 +519,11 @@ static int __devinit tps6586x_i2c_probe(struct i2c_client *client,
goto err_add_devs;
}

+ tps6586x_dev = &client->dev;
+
+ if (pdata->pm_off && !pm_power_off)
+ pm_power_off = tps6586x_power_off;
+



I think the assignment of tps6586x_dev needs to go inside the if block
as well. Otherwise it might be overwritten by another instance for
systems that actually have more than one tps6586x. Since currently the
driver can't be built as a module it probably makes little sense to
clean this up in .remove(), but it might still be worth adding so it
isn't forgotten if and when somebody tries to convert the driver to a
module.

I should note that I don't like very much how the pm_power_off works.
Maybe this should really be changed to allow passing a context for the
function to work from. Something like:

pm_power_off = tps6586x_power_off;
pm_power_off_data = &client->dev;

Where pm_power_off() would receive pm_power_off_data as an argument.

Even that's not very pretty. On the other hand this doesn't really buy
us much because only the storage location of the variable would change
and nothing else. But it would still make the association of the data
clearer.

Thierry



To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Replies Reply to this message
#3 Thierry Reding
August 17th, 2012 - 06:50 am ET | Report spam

On Fri, Aug 17, 2012 at 02:16:29AM -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

Signed-off-by: Bill Huang

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



The same comments as for the tps6586x driver apply here as well.

Thierry



To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Replies Reply to this message
#4 Stephen Warren
August 17th, 2012 - 03:10 pm ET | Report spam
On 08/17/2012 03:16 AM, 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

Signed-off-by: Bill Huang



Tested-by: Stephen Warren

Note that this conflicts (only context in tps6586x.txt I think) with
commit b93fffb "regulator: tps6586x: add support for SYS rail" in the
regulator tree. Also note that the patch is based on linux-next, which
contains a mis-merge of tps6586x.txt from the regulator tree. In other
words, expect some slight conflicts when you apply this. Taking it
through the regulator tree would eliminate them.
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Replies Reply to this message
#5 Stephen Warren
August 17th, 2012 - 03:10 pm ET | Report spam
On 08/17/2012 03:16 AM, 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

Signed-off-by: Bill Huang



Tested-by: Stephen Warren

AFAIK, this one doesn't have any conflicts.
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Replies Reply to this message
Help Create a new topicNext page Replies Make a reply
Search Make your own search