[PATCH] mfd: tps6586x: add api to power off tps6586x

[PATCH] mfd: tps6586x: add api to power off tps6586x

[wni]

From: Wei Ni <wni@nvidia.com>

Add an api to put tps6586x in sleep mode by resetting EXITSLREQ and
setting SLEEP_MODE in SUPPLYENE register. It will power off ldos.

Signed-off-by: Wei Ni <wni@nvidia.com>
Signed-off-by: Nitin Kumbhar <nkumbhar@nvidia.com>
---
 drivers/mfd/tps6586x.c       |   29 +++++++++++++++++++++++++++++
 include/linux/mfd/tps6586x.h |    1 +
 2 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/drivers/mfd/tps6586x.c b/drivers/mfd/tps6586x.c
index bba26d9..d0eaa0a 100644
--- a/drivers/mfd/tps6586x.c
+++ b/drivers/mfd/tps6586x.c
@@ -27,6 +27,10 @@
 #include <linux/mfd/core.h>
 #include <linux/mfd/tps6586x.h>
 
+#define TPS6586X_SUPPLYENE	0x14
+#define EXITSLREQ_BIT		BIT(1) /* Exit sleep mode request */
+#define SLEEP_MODE_BIT		BIT(3) /* Sleep mode */
+
 /* GPIO control registers */
 #define TPS6586X_GPIOSET1	0x5d
 #define TPS6586X_GPIOSET2	0x5e
@@ -251,6 +255,29 @@ out:
 }
 EXPORT_SYMBOL_GPL(tps6586x_update);
 
+static struct i2c_client *tps6586x_i2c_client;
+int tps6586x_power_off(void)
+{
+	struct device *dev = NULL;
+	int ret = -EINVAL;
+
+	if (!tps6586x_i2c_client)
+		return ret;
+
+	dev = &tps6586x_i2c_client->dev;
+
+	ret = tps6586x_clr_bits(dev, TPS6586X_SUPPLYENE, EXITSLREQ_BIT);
+	if (ret)
+		return ret;
+
+	ret = tps6586x_set_bits(dev, TPS6586X_SUPPLYENE, SLEEP_MODE_BIT);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(tps6586x_power_off);
+
 static int tps6586x_gpio_get(struct gpio_chip *gc, unsigned offset)
 {
 	struct tps6586x *tps6586x = container_of(gc, struct tps6586x, gpio);
@@ -525,6 +552,8 @@ static int __devinit tps6586x_i2c_probe(struct i2c_client *client,
 		goto err_add_devs;
 	}
 
+	tps6586x_i2c_client = client;
+
 	return 0;
 
 err_add_devs:
diff --git a/include/linux/mfd/tps6586x.h b/include/linux/mfd/tps6586x.h
index b6bab1b..cfbf387 100644
--- a/include/linux/mfd/tps6586x.h
+++ b/include/linux/mfd/tps6586x.h
@@ -74,5 +74,6 @@ extern int tps6586x_set_bits(struct device *dev, int reg, uint8_t bit_mask);
 extern int tps6586x_clr_bits(struct device *dev, int reg, uint8_t bit_mask);
 extern int tps6586x_update(struct device *dev, int reg, uint8_t val,
 			   uint8_t mask);
+extern int tps6586x_power_off(void);
 
 #endif /*__LINUX_MFD_TPS6586X_H */
-- 
1.7.0

RE: [PATCH] mfd: tps6586x: add api to power off tps6586x

[Wei Ni]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2814 bytes --]

Could anyone review this patch?

Thanks
Wei.
nvpublic

>-----Original Message-----
>From: Wei Ni
>Sent: 2011年7月28日 14:31
>To: sameo@linux.intel.com; mike@compulab.co.il; gking@nvidia.com; Nitin Kumbhar
>Cc: linux-kernel@vger.kernel.org; Wei Ni
>Subject: [PATCH] mfd: tps6586x: add api to power off tps6586x
>
>From: Wei Ni <wni@nvidia.com>
>
>Add an api to put tps6586x in sleep mode by resetting EXITSLREQ and
>setting SLEEP_MODE in SUPPLYENE register. It will power off ldos.
>
>Signed-off-by: Wei Ni <wni@nvidia.com>
>Signed-off-by: Nitin Kumbhar <nkumbhar@nvidia.com>
>---
> drivers/mfd/tps6586x.c       |   29 +++++++++++++++++++++++++++++
> include/linux/mfd/tps6586x.h |    1 +
> 2 files changed, 30 insertions(+), 0 deletions(-)
>
>diff --git a/drivers/mfd/tps6586x.c b/drivers/mfd/tps6586x.c
>index bba26d9..d0eaa0a 100644
>--- a/drivers/mfd/tps6586x.c
>+++ b/drivers/mfd/tps6586x.c
>@@ -27,6 +27,10 @@
> #include <linux/mfd/core.h>
> #include <linux/mfd/tps6586x.h>
>
>+#define TPS6586X_SUPPLYENE	0x14
>+#define EXITSLREQ_BIT		BIT(1) /* Exit sleep mode request */
>+#define SLEEP_MODE_BIT		BIT(3) /* Sleep mode */
>+
> /* GPIO control registers */
> #define TPS6586X_GPIOSET1	0x5d
> #define TPS6586X_GPIOSET2	0x5e
>@@ -251,6 +255,29 @@ out:
> }
> EXPORT_SYMBOL_GPL(tps6586x_update);
>
>+static struct i2c_client *tps6586x_i2c_client;
>+int tps6586x_power_off(void)
>+{
>+	struct device *dev = NULL;
>+	int ret = -EINVAL;
>+
>+	if (!tps6586x_i2c_client)
>+		return ret;
>+
>+	dev = &tps6586x_i2c_client->dev;
>+
>+	ret = tps6586x_clr_bits(dev, TPS6586X_SUPPLYENE, EXITSLREQ_BIT);
>+	if (ret)
>+		return ret;
>+
>+	ret = tps6586x_set_bits(dev, TPS6586X_SUPPLYENE, SLEEP_MODE_BIT);
>+	if (ret)
>+		return ret;
>+
>+	return 0;
>+}
>+EXPORT_SYMBOL_GPL(tps6586x_power_off);
>+
> static int tps6586x_gpio_get(struct gpio_chip *gc, unsigned offset)
> {
> 	struct tps6586x *tps6586x = container_of(gc, struct tps6586x, gpio);
>@@ -525,6 +552,8 @@ static int __devinit tps6586x_i2c_probe(struct i2c_client *client,
> 		goto err_add_devs;
> 	}
>
>+	tps6586x_i2c_client = client;
>+
> 	return 0;
>
> err_add_devs:
>diff --git a/include/linux/mfd/tps6586x.h b/include/linux/mfd/tps6586x.h
>index b6bab1b..cfbf387 100644
>--- a/include/linux/mfd/tps6586x.h
>+++ b/include/linux/mfd/tps6586x.h
>@@ -74,5 +74,6 @@ extern int tps6586x_set_bits(struct device *dev, int reg, uint8_t bit_mask);
> extern int tps6586x_clr_bits(struct device *dev, int reg, uint8_t bit_mask);
> extern int tps6586x_update(struct device *dev, int reg, uint8_t val,
> 			   uint8_t mask);
>+extern int tps6586x_power_off(void);
>
> #endif /*__LINUX_MFD_TPS6586X_H */
>--
>1.7.0

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

RE: [PATCH] mfd: tps6586x: add api to power off tps6586x

[Wei Ni]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2830 bytes --]

Hello, could anyone review this patch?

Thanks
Wei.
nvpublic

>-----Original Message-----
>From: Wei Ni
>Sent: Thursday, July 28, 2011 2:31 PM
>To: sameo@linux.intel.com; mike@compulab.co.il; gking@nvidia.com; Nitin Kumbhar
>Cc: linux-kernel@vger.kernel.org; Wei Ni
>Subject: [PATCH] mfd: tps6586x: add api to power off tps6586x
>
>From: Wei Ni <wni@nvidia.com>
>
>Add an api to put tps6586x in sleep mode by resetting EXITSLREQ and
>setting SLEEP_MODE in SUPPLYENE register. It will power off ldos.
>
>Signed-off-by: Wei Ni <wni@nvidia.com>
>Signed-off-by: Nitin Kumbhar <nkumbhar@nvidia.com>
>---
> drivers/mfd/tps6586x.c       |   29 +++++++++++++++++++++++++++++
> include/linux/mfd/tps6586x.h |    1 +
> 2 files changed, 30 insertions(+), 0 deletions(-)
>
>diff --git a/drivers/mfd/tps6586x.c b/drivers/mfd/tps6586x.c
>index bba26d9..d0eaa0a 100644
>--- a/drivers/mfd/tps6586x.c
>+++ b/drivers/mfd/tps6586x.c
>@@ -27,6 +27,10 @@
> #include <linux/mfd/core.h>
> #include <linux/mfd/tps6586x.h>
>
>+#define TPS6586X_SUPPLYENE	0x14
>+#define EXITSLREQ_BIT		BIT(1) /* Exit sleep mode request */
>+#define SLEEP_MODE_BIT		BIT(3) /* Sleep mode */
>+
> /* GPIO control registers */
> #define TPS6586X_GPIOSET1	0x5d
> #define TPS6586X_GPIOSET2	0x5e
>@@ -251,6 +255,29 @@ out:
> }
> EXPORT_SYMBOL_GPL(tps6586x_update);
>
>+static struct i2c_client *tps6586x_i2c_client;
>+int tps6586x_power_off(void)
>+{
>+	struct device *dev = NULL;
>+	int ret = -EINVAL;
>+
>+	if (!tps6586x_i2c_client)
>+		return ret;
>+
>+	dev = &tps6586x_i2c_client->dev;
>+
>+	ret = tps6586x_clr_bits(dev, TPS6586X_SUPPLYENE, EXITSLREQ_BIT);
>+	if (ret)
>+		return ret;
>+
>+	ret = tps6586x_set_bits(dev, TPS6586X_SUPPLYENE, SLEEP_MODE_BIT);
>+	if (ret)
>+		return ret;
>+
>+	return 0;
>+}
>+EXPORT_SYMBOL_GPL(tps6586x_power_off);
>+
> static int tps6586x_gpio_get(struct gpio_chip *gc, unsigned offset)
> {
> 	struct tps6586x *tps6586x = container_of(gc, struct tps6586x, gpio);
>@@ -525,6 +552,8 @@ static int __devinit tps6586x_i2c_probe(struct i2c_client *client,
> 		goto err_add_devs;
> 	}
>
>+	tps6586x_i2c_client = client;
>+
> 	return 0;
>
> err_add_devs:
>diff --git a/include/linux/mfd/tps6586x.h b/include/linux/mfd/tps6586x.h
>index b6bab1b..cfbf387 100644
>--- a/include/linux/mfd/tps6586x.h
>+++ b/include/linux/mfd/tps6586x.h
>@@ -74,5 +74,6 @@ extern int tps6586x_set_bits(struct device *dev, int reg, uint8_t bit_mask);
> extern int tps6586x_clr_bits(struct device *dev, int reg, uint8_t bit_mask);
> extern int tps6586x_update(struct device *dev, int reg, uint8_t val,
> 			   uint8_t mask);
>+extern int tps6586x_power_off(void);
>
> #endif /*__LINUX_MFD_TPS6586X_H */
>--
>1.7.0

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH] mfd: tps6586x: add api to power off tps6586x

[Samuel Ortiz]

Hi Wei,

On Thu, Jul 28, 2011 at 02:30:36PM +0800, wni@nvidia.com wrote:
> +static struct i2c_client *tps6586x_i2c_client;
That's not vey nice. By doing so, you're preventing anyone to have more than
one of those chips in their boards.


> +int tps6586x_power_off(void)
> +{
> +	struct device *dev = NULL;
> +	int ret = -EINVAL;
> +
> +	if (!tps6586x_i2c_client)
> +		return ret;
> +
> +	dev = &tps6586x_i2c_client->dev;
> +
> +	ret = tps6586x_clr_bits(dev, TPS6586X_SUPPLYENE, EXITSLREQ_BIT);
> +	if (ret)
> +		return ret;
> +
> +	ret = tps6586x_set_bits(dev, TPS6586X_SUPPLYENE, SLEEP_MODE_BIT);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(tps6586x_power_off);
Who is going to call this one ?

Cheers,
Samuel.

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

RE: [PATCH] mfd: tps6586x: add api to power off tps6586x

[Wei Ni]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1767 bytes --]

Hi, Samuel
Thanks for your comments.
Please check the following lines with [Wei Ni]

Thanks
Wei.


>-----Original Message-----
>From: Samuel Ortiz [mailto:sameo@linux.intel.com]
>Sent: Monday, August 22, 2011 7:05 AM
>To: Wei Ni
>Cc: mike@compulab.co.il; gking@nvidia.com; Nitin Kumbhar; linux-kernel@vger.kernel.org
>Subject: Re: [PATCH] mfd: tps6586x: add api to power off tps6586x
>
>Hi Wei,
>
>On Thu, Jul 28, 2011 at 02:30:36PM +0800, wni@nvidia.com wrote:
>> +static struct i2c_client *tps6586x_i2c_client;
>That's not vey nice. By doing so, you're preventing anyone to have more than
>one of those chips in their boards.
[Wei Ni] oh yes, I didn’t consider this issue. We use i2c_register_board_info()
to register the tps6586x in the board file, but it didn't return the handle.
Do you have any ideas?
>
>
>> +int tps6586x_power_off(void)
>> +{
>> +	struct device *dev = NULL;
>> +	int ret = -EINVAL;
>> +
>> +	if (!tps6586x_i2c_client)
>> +		return ret;
>> +
>> +	dev = &tps6586x_i2c_client->dev;
>> +
>> +	ret = tps6586x_clr_bits(dev, TPS6586X_SUPPLYENE, EXITSLREQ_BIT);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = tps6586x_set_bits(dev, TPS6586X_SUPPLYENE, SLEEP_MODE_BIT);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(tps6586x_power_off);
>Who is going to call this one ?
[Wei Ni] the tegra board will call this function to power off system.
It will put tps6586x in sleep mode by resetting EXITSLREQ and
setting SLEEP_MODE in SUPPLYENE register. It will power off ldos.
>
>Cheers,
>Samuel.
>
>--
>Intel Open Source Technology Centre
>http://oss.intel.com/
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH] mfd: tps6586x: add api to power off tps6586x

[Haojian Zhuang]

On Mon, Aug 22, 2011 at 11:47 AM, Wei Ni <wni@nvidia.com> wrote:
> Hi, Samuel
> Thanks for your comments.
> Please check the following lines with [Wei Ni]
>
> Thanks
> Wei.
>
>
>>-----Original Message-----
>>From: Samuel Ortiz [mailto:sameo@linux.intel.com]
>>Sent: Monday, August 22, 2011 7:05 AM
>>To: Wei Ni
>>Cc: mike@compulab.co.il; gking@nvidia.com; Nitin Kumbhar; linux-kernel@vger.kernel.org
>>Subject: Re: [PATCH] mfd: tps6586x: add api to power off tps6586x
>>
>>Hi Wei,
>>
>>On Thu, Jul 28, 2011 at 02:30:36PM +0800, wni@nvidia.com wrote:
>>> +static struct i2c_client *tps6586x_i2c_client;
>>That's not vey nice. By doing so, you're preventing anyone to have more than
>>one of those chips in their boards.
> [Wei Ni] oh yes, I didn’t consider this issue. We use i2c_register_board_info()
> to register the tps6586x in the board file, but it didn't return the handle.
> Do you have any ideas?
>>
>>

Since there's no parameter in pm_power_off(), invoking power off
function is a known issue
for i2c devices. And we can't get i2c client in board file.

Maybe we can add a LDO mapping in regulator driver. Since power-off
function is always
customized in board file. We can disable this regulator while
power_off() is called. So even
multiple PMICs is installed in one system, we only handle the real one
to shutdown the
whole system.

Samuel & Mark,
Do you think that it's worth to do?

Thanks
Haojian

Re: [PATCH] mfd: tps6586x: add api to power off tps6586x

[Mark Brown]

On Mon, Aug 22, 2011 at 01:06:02PM +0800, Haojian Zhuang wrote:

> Maybe we can add a LDO mapping in regulator driver. Since power-off
> function is always
> customized in board file. We can disable this regulator while
> power_off() is called. So even
> multiple PMICs is installed in one system, we only handle the real one
> to shutdown the
> whole system.

Some (ST?) platform had an issue like that.  What they did IIRC is
define a regulator consumer driver which is hooked up as normal then had
that driver implemented a power_off() which used regulator_disable() to do
the honours.  Perhaps this could be made more generic somehow?  Though
it may have been part of their cpufreq driver or something - I can't
remember the detail right now.