Patchwork [v2,1/4] mfd: tps65217: Set PMIC to shutdowm on PWR_EN toggle

login
register
mail settings
Submitter AnilKumar Chimata
Date Nov. 16, 2012, 12:08 p.m.
Message ID <1353067706-29491-2-git-send-email-anilkumar@ti.com>
Download mbox | patch
Permalink /patch/199559/
State New
Headers show

Comments

AnilKumar Chimata - Nov. 16, 2012, 12:08 p.m.
From: Colin Foe-Parker <colin.foeparker@logicpd.com>

Set tps65217 PMIC status to OFF if power enable toggle is
supported. Also adds platform data flag, which should be
passed from board init data.

Signed-off-by: Colin Foe-Parker <colin.foeparker@logicpd.com>
[anilkumar@ti.com: move the additions to tps65217 MFD driver]
Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
---
 .../devicetree/bindings/regulator/tps65217.txt     |    4 ++++
 drivers/mfd/tps65217.c                             |   12 ++++++++++++
 2 files changed, 16 insertions(+)
Kevin Hilman - Nov. 16, 2012, 6:53 p.m.
AnilKumar Ch <anilkumar@ti.com> writes:

> From: Colin Foe-Parker <colin.foeparker@logicpd.com>
>
> Set tps65217 PMIC status to OFF if power enable toggle is
> supported. Also adds platform data flag, which should be
> passed from board init data.

nit: changelog mentions platform_data, but the code is using DT.

Kevin

> Signed-off-by: Colin Foe-Parker <colin.foeparker@logicpd.com>
> [anilkumar@ti.com: move the additions to tps65217 MFD driver]
> Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
> ---
>  .../devicetree/bindings/regulator/tps65217.txt     |    4 ++++
>  drivers/mfd/tps65217.c                             |   12 ++++++++++++
>  2 files changed, 16 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/regulator/tps65217.txt b/Documentation/devicetree/bindings/regulator/tps65217.txt
> index d316fb8..4f05d20 100644
> --- a/Documentation/devicetree/bindings/regulator/tps65217.txt
> +++ b/Documentation/devicetree/bindings/regulator/tps65217.txt
> @@ -11,6 +11,9 @@ Required properties:
>    using the standard binding for regulators found at
>    Documentation/devicetree/bindings/regulator/regulator.txt.
>  
> +Optional properties:
> +- ti,pmic-shutdown-controller: Telling the PMIC to shutdown on PWR_EN toggle.
> +
>    The valid names for regulators are:
>    tps65217: dcdc1, dcdc2, dcdc3, ldo1, ldo2, ldo3 and ldo4
>  
> @@ -20,6 +23,7 @@ Example:
>  
>  	tps: tps@24 {
>  		compatible = "ti,tps65217";
> +		ti,pmic-shutdown-controller;
>  
>  		regulators {
>  			dcdc1_reg: dcdc1 {
> diff --git a/drivers/mfd/tps65217.c b/drivers/mfd/tps65217.c
> index 3fb32e6..c7f17d8 100644
> --- a/drivers/mfd/tps65217.c
> +++ b/drivers/mfd/tps65217.c
> @@ -160,6 +160,7 @@ static int __devinit tps65217_probe(struct i2c_client *client,
>  	unsigned int version;
>  	unsigned int chip_id = ids->driver_data;
>  	const struct of_device_id *match;
> +	bool status_off = false;
>  	int ret;
>  
>  	if (client->dev.of_node) {
> @@ -170,6 +171,8 @@ static int __devinit tps65217_probe(struct i2c_client *client,
>  			return -EINVAL;
>  		}
>  		chip_id = (unsigned int)match->data;
> +		status_off = of_property_read_bool(client->dev.of_node,
> +					"ti,pmic-shutdown-controller");
>  	}
>  
>  	if (!chip_id) {
> @@ -207,6 +210,15 @@ static int __devinit tps65217_probe(struct i2c_client *client,
>  		return ret;
>  	}
>  
> +	/* Set the PMIC to shutdown on PWR_EN toggle */
> +	if (status_off) {
> +		ret = tps65217_set_bits(tps, TPS65217_REG_STATUS,
> +				TPS65217_STATUS_OFF, TPS65217_STATUS_OFF,
> +				TPS65217_PROTECT_NONE);
> +		if (ret)
> +			dev_warn(tps->dev, "unable to set the status OFF\n");
> +	}
> +
>  	dev_info(tps->dev, "TPS65217 ID %#x version 1.%d\n",
>  			(version & TPS65217_CHIPID_CHIP_MASK) >> 4,
>  			version & TPS65217_CHIPID_REV_MASK);
Peter Korsgaard - Nov. 19, 2012, 10:50 a.m.
>>>>> "AnilKumar" == AnilKumar Ch <anilkumar@ti.com> writes:

s/shutdowm/shutdown/ in the subject.

 AnilKumar> From: Colin Foe-Parker <colin.foeparker@logicpd.com>
 AnilKumar> Set tps65217 PMIC status to OFF if power enable toggle is
 AnilKumar> supported. Also adds platform data flag, which should be
 AnilKumar> passed from board init data.

You're adding dt binding, not platform data.


 AnilKumar> Signed-off-by: Colin Foe-Parker <colin.foeparker@logicpd.com>
 AnilKumar> [anilkumar@ti.com: move the additions to tps65217 MFD driver]
 AnilKumar> Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
 AnilKumar> ---
 AnilKumar>  .../devicetree/bindings/regulator/tps65217.txt     |    4 ++++
 AnilKumar>  drivers/mfd/tps65217.c                             |   12 ++++++++++++
 AnilKumar>  2 files changed, 16 insertions(+)

 AnilKumar> diff --git a/Documentation/devicetree/bindings/regulator/tps65217.txt b/Documentation/devicetree/bindings/regulator/tps65217.txt
 AnilKumar> index d316fb8..4f05d20 100644
 AnilKumar> --- a/Documentation/devicetree/bindings/regulator/tps65217.txt
 AnilKumar> +++ b/Documentation/devicetree/bindings/regulator/tps65217.txt
 AnilKumar> @@ -11,6 +11,9 @@ Required properties:
 AnilKumar>    using the standard binding for regulators found at
 AnilKumar>    Documentation/devicetree/bindings/regulator/regulator.txt.
 
 AnilKumar> +Optional properties:
 AnilKumar> +- ti,pmic-shutdown-controller: Telling the PMIC to shutdown on PWR_EN toggle.

Ehh, I don't know the tps65217 particular well, but according to the
datasheet the REG_STATUS_OFF bit enables/disables the PB_IN pin, not
PWR_EN. It's also not only about powering down but also powering up
again.

I don't feel this property name is very clear. Perhaps something about
power button or push button monitor as it is called in the datasheet?
AnilKumar Chimata - Nov. 19, 2012, 11:36 a.m.
On Mon, Nov 19, 2012 at 16:20:27, Peter Korsgaard wrote:
> >>>>> "AnilKumar" == AnilKumar Ch <anilkumar@ti.com> writes:
> 
> s/shutdowm/shutdown/ in the subject.

I will change

> 
>  AnilKumar> From: Colin Foe-Parker <colin.foeparker@logicpd.com>
>  AnilKumar> Set tps65217 PMIC status to OFF if power enable toggle is
>  AnilKumar> supported. Also adds platform data flag, which should be
>  AnilKumar> passed from board init data.
> 
> You're adding dt binding, not platform data.

I will change

> 
>  AnilKumar> Signed-off-by: Colin Foe-Parker <colin.foeparker@logicpd.com>
>  AnilKumar> [anilkumar@ti.com: move the additions to tps65217 MFD driver]
>  AnilKumar> Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
>  AnilKumar> ---
>  AnilKumar>  .../devicetree/bindings/regulator/tps65217.txt     |    4 ++++
>  AnilKumar>  drivers/mfd/tps65217.c                             |   12 ++++++++++++
>  AnilKumar>  2 files changed, 16 insertions(+)
> 
>  AnilKumar> diff --git a/Documentation/devicetree/bindings/regulator/tps65217.txt b/Documentation/devicetree/bindings/regulator/tps65217.txt
>  AnilKumar> index d316fb8..4f05d20 100644
>  AnilKumar> --- a/Documentation/devicetree/bindings/regulator/tps65217.txt
>  AnilKumar> +++ b/Documentation/devicetree/bindings/regulator/tps65217.txt
>  AnilKumar> @@ -11,6 +11,9 @@ Required properties:
>  AnilKumar>    using the standard binding for regulators found at
>  AnilKumar>    Documentation/devicetree/bindings/regulator/regulator.txt.
>  
>  AnilKumar> +Optional properties:
>  AnilKumar> +- ti,pmic-shutdown-controller: Telling the PMIC to shutdown on PWR_EN toggle.
> 
> Ehh, I don't know the tps65217 particular well, but according to the
> datasheet the REG_STATUS_OFF bit enables/disables the PB_IN pin, not
> PWR_EN. It's also not only about powering down but also powering up
> again.
> 
> I don't feel this property name is very clear. Perhaps something about
> power button or push button monitor as it is called in the datasheet?

Here I am writing "TPS65217_STATUS_OFF" to status register, BIT(7).
Description of this bit field: "OFF bit. Set this bit to 1 to enter OFF state
when PWR_EN pin is pulled low. Bit is automatically reset to 0."

PWR_EN pin pull-down is doing by RTC module.

Regards
AnilKumar
Peter Korsgaard - Nov. 19, 2012, 12:17 p.m.
>>>>> "A" == AnilKumar, Chimata <anilkumar@ti.com> writes:

Hi,

 >> I don't feel this property name is very clear. Perhaps something about
 >> power button or push button monitor as it is called in the datasheet?

 A> Here I am writing "TPS65217_STATUS_OFF" to status register, BIT(7).
 A> Description of this bit field: "OFF bit. Set this bit to 1 to enter
 A> OFF state when PWR_EN pin is pulled low. Bit is automatically reset
 A> to 0."

 A> PWR_EN pin pull-down is doing by RTC module.

Ahh yes, sorry -ENOCOFFEE. I somehow misread the datasheet and looked at
bit 0 instead.

Patch

diff --git a/Documentation/devicetree/bindings/regulator/tps65217.txt b/Documentation/devicetree/bindings/regulator/tps65217.txt
index d316fb8..4f05d20 100644
--- a/Documentation/devicetree/bindings/regulator/tps65217.txt
+++ b/Documentation/devicetree/bindings/regulator/tps65217.txt
@@ -11,6 +11,9 @@  Required properties:
   using the standard binding for regulators found at
   Documentation/devicetree/bindings/regulator/regulator.txt.
 
+Optional properties:
+- ti,pmic-shutdown-controller: Telling the PMIC to shutdown on PWR_EN toggle.
+
   The valid names for regulators are:
   tps65217: dcdc1, dcdc2, dcdc3, ldo1, ldo2, ldo3 and ldo4
 
@@ -20,6 +23,7 @@  Example:
 
 	tps: tps@24 {
 		compatible = "ti,tps65217";
+		ti,pmic-shutdown-controller;
 
 		regulators {
 			dcdc1_reg: dcdc1 {
diff --git a/drivers/mfd/tps65217.c b/drivers/mfd/tps65217.c
index 3fb32e6..c7f17d8 100644
--- a/drivers/mfd/tps65217.c
+++ b/drivers/mfd/tps65217.c
@@ -160,6 +160,7 @@  static int __devinit tps65217_probe(struct i2c_client *client,
 	unsigned int version;
 	unsigned int chip_id = ids->driver_data;
 	const struct of_device_id *match;
+	bool status_off = false;
 	int ret;
 
 	if (client->dev.of_node) {
@@ -170,6 +171,8 @@  static int __devinit tps65217_probe(struct i2c_client *client,
 			return -EINVAL;
 		}
 		chip_id = (unsigned int)match->data;
+		status_off = of_property_read_bool(client->dev.of_node,
+					"ti,pmic-shutdown-controller");
 	}
 
 	if (!chip_id) {
@@ -207,6 +210,15 @@  static int __devinit tps65217_probe(struct i2c_client *client,
 		return ret;
 	}
 
+	/* Set the PMIC to shutdown on PWR_EN toggle */
+	if (status_off) {
+		ret = tps65217_set_bits(tps, TPS65217_REG_STATUS,
+				TPS65217_STATUS_OFF, TPS65217_STATUS_OFF,
+				TPS65217_PROTECT_NONE);
+		if (ret)
+			dev_warn(tps->dev, "unable to set the status OFF\n");
+	}
+
 	dev_info(tps->dev, "TPS65217 ID %#x version 1.%d\n",
 			(version & TPS65217_CHIPID_CHIP_MASK) >> 4,
 			version & TPS65217_CHIPID_REV_MASK);