[PATCH V1] gpio: tps65910: Add sleep control support

[PATCH V1] gpio: tps65910: Add sleep control support

[Laxman Dewangan]

The device tps65910/tps65911 supports the sleep
functionality in some of gpios. If gpio is configured
in output mode and sleep is enabled then during device
sleep state, the output of gpio becomes LOW regardless
of non-sleep output value.
Such gpio can be used to control regulator switch such
that output of regulator is off in device sleep state.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/gpio/gpio-tps65910.c |   16 ++++++++++++++++
 include/linux/mfd/tps65910.h |    6 ++++++
 2 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/drivers/gpio/gpio-tps65910.c b/drivers/gpio/gpio-tps65910.c
index 91f45b9..ec7323a 100644
--- a/drivers/gpio/gpio-tps65910.c
+++ b/drivers/gpio/gpio-tps65910.c
@@ -69,6 +69,7 @@ static int tps65910_gpio_input(struct gpio_chip *gc, unsigned offset)
 void tps65910_gpio_init(struct tps65910 *tps65910, int gpio_base)
 {
 	int ret;
+	struct tps65910_board *board_data;
 
 	if (!gpio_base)
 		return;
@@ -95,6 +96,21 @@ void tps65910_gpio_init(struct tps65910 *tps65910, int gpio_base)
 	tps65910->gpio.set		= tps65910_gpio_set;
 	tps65910->gpio.get		= tps65910_gpio_get;
 
+	/* Configure sleep control for gpios */
+	board_data = dev_get_platdata(tps65910->dev);
+	if (board_data) {
+		int i;
+		for (i = 0; i < tps65910->gpio.ngpio; ++i) {
+			if (board_data->en_gpio_sleep[i]) {
+				ret = tps65910_set_bits(tps65910,
+					TPS65910_GPIO0 + i, GPIO_SLEEP_MASK);
+				if (ret < 0)
+					dev_warn(tps65910->dev,
+						"GPIO Sleep setting failed\n");
+			}
+		}
+	}
+
 	ret = gpiochip_add(&tps65910->gpio);
 
 	if (ret)
diff --git a/include/linux/mfd/tps65910.h b/include/linux/mfd/tps65910.h
index fa6c6bf..b641690 100644
--- a/include/linux/mfd/tps65910.h
+++ b/include/linux/mfd/tps65910.h
@@ -657,6 +657,8 @@
 
 
 /*Register GPIO  (0x80) register.RegisterDescription */
+#define GPIO_SLEEP_MASK                         0x80
+#define GPIO_SLEEP_SHIFT                        7
 #define GPIO_DEB_MASK                           0x10
 #define GPIO_DEB_SHIFT                          4
 #define GPIO_PUEN_MASK                          0x08
@@ -740,6 +742,9 @@
 #define TPS65910_GPIO_STS				BIT(1)
 #define TPS65910_GPIO_SET				BIT(0)
 
+/* Max number of TPS65910/11 GPIOs */
+#define TPS65911_NUM_GPIO				9
+
 /* Regulator Index Definitions */
 #define TPS65910_REG_VRTC				0
 #define TPS65910_REG_VIO				1
@@ -786,6 +791,7 @@ struct tps65910_board {
 	int irq_base;
 	int vmbch_threshold;
 	int vmbch2_threshold;
+	int en_gpio_sleep[TPS65911_NUM_GPIO];
 	unsigned long regulator_ext_sleep_control[TPS65910_NUM_REGS];
 	struct regulator_init_data *tps65910_pmic_init_data[TPS65910_NUM_REGS];
 };
-- 
1.7.1.1

Re: [PATCH V1] gpio: tps65910: Add sleep control support

[Linus Walleij]

On Tue, Jan 31, 2012 at 9:45 AM, Laxman Dewangan <ldewangan@nvidia.com> wrote:

> The device tps65910/tps65911 supports the sleep
> functionality in some of gpios. If gpio is configured
> in output mode and sleep is enabled then during device
> sleep state, the output of gpio becomes LOW regardless
> of non-sleep output value.
> Such gpio can be used to control regulator switch such
> that output of regulator is off in device sleep state.
>
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>

Strictly speaking this is pin control business, but since it is already
doing other stuff like this from platform data why not...

Comments:

> diff --git a/include/linux/mfd/tps65910.h b/include/linux/mfd/tps65910.h
> index fa6c6bf..b641690 100644
> --- a/include/linux/mfd/tps65910.h
> +++ b/include/linux/mfd/tps65910.h
> @@ -657,6 +657,8 @@
>
>
>  /*Register GPIO  (0x80) register.RegisterDescription */
> +#define GPIO_SLEEP_MASK                         0x80
> +#define GPIO_SLEEP_SHIFT                        7
>  #define GPIO_DEB_MASK                           0x10
>  #define GPIO_DEB_SHIFT                          4
>  #define GPIO_PUEN_MASK                          0x08
> @@ -740,6 +742,9 @@
>  #define TPS65910_GPIO_STS                              BIT(1)
>  #define TPS65910_GPIO_SET                              BIT(0)
>
> +/* Max number of TPS65910/11 GPIOs */
> +#define TPS65911_NUM_GPIO                              9

Then name it TPS_6591X_MAX_NUM_GPIO

I would write:

#define TPS65910_NUM_GPIO 6
#define TPS65911_NUM_GPIO 9
#define TPS6591X_MAX_NUM_GPIO 9

And then also refactor this code in the driver:

        switch(tps65910_chip_id(tps65910)) {
        case TPS65910:
                tps65910->gpio.ngpio    = 6;
                break;
        case TPS65911:
                tps65910->gpio.ngpio    = 9;
                break;
        default:
                return;

to use the new defines instead of magic numbers while
you're at it.

> +
>  /* Regulator Index Definitions */
>  #define TPS65910_REG_VRTC                              0
>  #define TPS65910_REG_VIO                               1
> @@ -786,6 +791,7 @@ struct tps65910_board {
>        int irq_base;
>        int vmbch_threshold;
>        int vmbch2_threshold;
> +       int en_gpio_sleep[TPS65911_NUM_GPIO];

This is used as a bool so use bool.

>        unsigned long regulator_ext_sleep_control[TPS65910_NUM_REGS];
>        struct regulator_init_data *tps65910_pmic_init_data[TPS65910_NUM_REGS];
>  };
> --
> 1.7.1.1

Thanks,
Linus Walleij

Re: [PATCH V1] gpio: tps65910: Add sleep control support

[Laxman Dewangan]

On Wednesday 01 February 2012 03:01 AM, Linus Walleij wrote:
> On Tue, Jan 31, 2012 at 9:45 AM, Laxman Dewangan<ldewangan@nvidia.com>  wrote:
>
>> The device tps65910/tps65911 supports the sleep
>> functionality in some of gpios. If gpio is configured
>> in output mode and sleep is enabled then during device
>> sleep state, the output of gpio becomes LOW regardless
>> of non-sleep output value.
>> Such gpio can be used to control regulator switch such
>> that output of regulator is off in device sleep state.
>>
>> Signed-off-by: Laxman Dewangan<ldewangan@nvidia.com>
> Strictly speaking this is pin control business, but since it is already
> doing other stuff like this from platform data why not...
>
> Comments:
>
Thanks for review. I have taken care of all your comments and will send 
another version with  these changes.