[PATCH 1/3] input: keyboard: MCS5080: support suspend/resume.

[PATCH 1/3] input: keyboard: MCS5080: support suspend/resume.

[Kim, HeungJun]

This patch supports suspend/resume functions for mcs5080 touchkey
driver.

Signed-off-by: Heungjun Kim <riverful.kim@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/input/keyboard/mcs_touchkey.c |   41 +++++++++++++++++++++++++++++++++
 include/linux/i2c/mcs.h               |    1 +
 2 files changed, 42 insertions(+), 0 deletions(-)

diff --git a/drivers/input/keyboard/mcs_touchkey.c b/drivers/input/keyboard/mcs_touchkey.c
index 63b849d..06385f5 100644
--- a/drivers/input/keyboard/mcs_touchkey.c
+++ b/drivers/input/keyboard/mcs_touchkey.c
@@ -45,6 +45,8 @@ struct mcs_touchkey_chip {
 };

 struct mcs_touchkey_data {
+       void (*poweron)(int);
+
        struct i2c_client *client;
        struct input_dev *input_dev;
        struct mcs_touchkey_chip chip;
@@ -168,6 +170,8 @@ static int __devinit mcs_touchkey_probe(struct i2c_client *client,

        if (pdata->cfg_pin)
                pdata->cfg_pin();
+       if (pdata->poweron)
+               data->poweron = pdata->poweron;

        error = request_threaded_irq(client->irq, NULL, mcs_touchkey_interrupt,
                        IRQF_TRIGGER_FALLING, client->dev.driver->name, data);
@@ -202,6 +206,41 @@ static int __devexit mcs_touchkey_remove(struct i2c_client *client)
        return 0;
 }

+#ifdef CONFIG_PM
+static int mcs_touchkey_suspend(struct i2c_client *client, pm_message_t mesg)
+{
+       struct mcs_touchkey_data *data = i2c_get_clientdata(client);
+
+       /* Disable the work */
+       disable_irq(client->irq);
+
+       /* Don't I2C operation since we don't know I2C bus is dead already */
+
+       /* Finally turn off the power */
+       if (data->poweron)
+               data->poweron(0);
+
+       return 0;
+}
+
+static int mcs_touchkey_resume(struct i2c_client *client)
+{
+       struct mcs_touchkey_data *data = i2c_get_clientdata(client);
+
+       /* Enable the device first */
+       if (data->poweron)
+               data->poweron(1);
+
+       /* Enable irq again */
+       enable_irq(client->irq);
+
+       return 0;
+}
+#else
+#define mcs_touchkey_suspend   NULL
+#define mcs_touchkey_resume    NULL
+#endif
+
 static const struct i2c_device_id mcs_touchkey_id[] = {
        { "mcs5000_touchkey", MCS5000_TOUCHKEY },
        { "mcs5080_touchkey", MCS5080_TOUCHKEY },
@@ -216,6 +255,8 @@ static struct i2c_driver mcs_touchkey_driver = {
        },
        .probe          = mcs_touchkey_probe,
        .remove         = __devexit_p(mcs_touchkey_remove),
+       .suspend        = mcs_touchkey_suspend,
+       .resume         = mcs_touchkey_resume,
        .id_table       = mcs_touchkey_id,
 };

diff --git a/include/linux/i2c/mcs.h b/include/linux/i2c/mcs.h
index 725ae7c..c4a3869 100644
--- a/include/linux/i2c/mcs.h
+++ b/include/linux/i2c/mcs.h
@@ -18,6 +18,7 @@
 #define MCS_KEY_CODE(v)                ((v) & 0xffff)

 struct mcs_platform_data {
+       void (*poweron)(int);
        void (*cfg_pin)(void);

        /* touchscreen */
--
1.7.0.4

RE: [PATCH 1/3] input: keyboard: MCS5080: support suspend/resume.

[Datta, Shubhrajyoti]

Hi Kim,

> -----Original Message-----
> From: linux-input-owner@vger.kernel.org [mailto:linux-input-
> owner@vger.kernel.org] On Behalf Of Kim, HeungJun
> Sent: Monday, November 15, 2010 10:02 AM
> To: linux-input@vger.kernel.org; dmitry.torokhov@gmail.com
> Cc: kyungmin.park@samsung.com
> Subject: [PATCH 1/3] input: keyboard: MCS5080: support suspend/resume.
> 
> This patch supports suspend/resume functions for mcs5080 touchkey
> driver.
> 
> Signed-off-by: Heungjun Kim <riverful.kim@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/input/keyboard/mcs_touchkey.c |   41
> +++++++++++++++++++++++++++++++++
>  include/linux/i2c/mcs.h               |    1 +
>  2 files changed, 42 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/input/keyboard/mcs_touchkey.c
> b/drivers/input/keyboard/mcs_touchkey.c
> index 63b849d..06385f5 100644
> --- a/drivers/input/keyboard/mcs_touchkey.c
> +++ b/drivers/input/keyboard/mcs_touchkey.c
> @@ -45,6 +45,8 @@ struct mcs_touchkey_chip {
>  };
> 
>  struct mcs_touchkey_data {
> +       void (*poweron)(int);
Could you explain what it does?

> +
>         struct i2c_client *client;
>         struct input_dev *input_dev;
>         struct mcs_touchkey_chip chip;
> @@ -168,6 +170,8 @@ static int __devinit mcs_touchkey_probe(struct
> i2c_client *client,
> 
>         if (pdata->cfg_pin)
>                 pdata->cfg_pin();
> +       if (pdata->poweron)
> +               data->poweron = pdata->poweron;
Where is it called the first time?

> 
>         error = request_threaded_irq(client->irq, NULL,
> mcs_touchkey_interrupt,
>                         IRQF_TRIGGER_FALLING, client->dev.driver->name,
> data);
> @@ -202,6 +206,41 @@ static int __devexit mcs_touchkey_remove(struct
> i2c_client *client)
>         return 0;
>  }
> 
> +#ifdef CONFIG_PM
> +static int mcs_touchkey_suspend(struct i2c_client *client, pm_message_t
> mesg)
> +{
> +       struct mcs_touchkey_data *data = i2c_get_clientdata(client);
> +
> +       /* Disable the work */
> +       disable_irq(client->irq);
> +
> +       /* Don't I2C operation since we don't know I2C bus is dead already
> */
> +
> +       /* Finally turn off the power */
> +       if (data->poweron)
> +               data->poweron(0);
> +
> +       return 0;
> +}
> +
> +static int mcs_touchkey_resume(struct i2c_client *client)
> +{
> +       struct mcs_touchkey_data *data = i2c_get_clientdata(client);
> +
> +       /* Enable the device first */
> +       if (data->poweron)
> +               data->poweron(1);
> +
> +       /* Enable irq again */
> +       enable_irq(client->irq);
> +
> +       return 0;
> +}
> +#else
> +#define mcs_touchkey_suspend   NULL
> +#define mcs_touchkey_resume    NULL
> +#endif
> +
>  static const struct i2c_device_id mcs_touchkey_id[] = {
>         { "mcs5000_touchkey", MCS5000_TOUCHKEY },
>         { "mcs5080_touchkey", MCS5080_TOUCHKEY },
> @@ -216,6 +255,8 @@ static struct i2c_driver mcs_touchkey_driver = {
>         },
>         .probe          = mcs_touchkey_probe,
>         .remove         = __devexit_p(mcs_touchkey_remove),
> +       .suspend        = mcs_touchkey_suspend,
> +       .resume         = mcs_touchkey_resume,
>         .id_table       = mcs_touchkey_id,
>  };
> 
> diff --git a/include/linux/i2c/mcs.h b/include/linux/i2c/mcs.h
> index 725ae7c..c4a3869 100644
> --- a/include/linux/i2c/mcs.h
> +++ b/include/linux/i2c/mcs.h
> @@ -18,6 +18,7 @@
>  #define MCS_KEY_CODE(v)                ((v) & 0xffff)
> 
>  struct mcs_platform_data {
> +       void (*poweron)(int);
>         void (*cfg_pin)(void);
> 
>         /* touchscreen */
> --
> 1.7.0.4
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] input: keyboard: MCS5080: support suspend/resume.

[Kim, HeungJun]

Hi shubhrajyoti,


2010-11-15 오후 2:43, Datta, Shubhrajyoti 쓴 글:
> Hi Kim,
> 
>> -----Original Message-----
>> From: linux-input-owner@vger.kernel.org [mailto:linux-input-
>> owner@vger.kernel.org] On Behalf Of Kim, HeungJun
>> Sent: Monday, November 15, 2010 10:02 AM
>> To: linux-input@vger.kernel.org; dmitry.torokhov@gmail.com
>> Cc: kyungmin.park@samsung.com
>> Subject: [PATCH 1/3] input: keyboard: MCS5080: support suspend/resume.
>>
>> This patch supports suspend/resume functions for mcs5080 touchkey
>> driver.
>>
>> Signed-off-by: Heungjun Kim <riverful.kim@samsung.com>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
>>  drivers/input/keyboard/mcs_touchkey.c |   41
>> +++++++++++++++++++++++++++++++++
>>  include/linux/i2c/mcs.h               |    1 +
>>  2 files changed, 42 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/input/keyboard/mcs_touchkey.c
>> b/drivers/input/keyboard/mcs_touchkey.c
>> index 63b849d..06385f5 100644
>> --- a/drivers/input/keyboard/mcs_touchkey.c
>> +++ b/drivers/input/keyboard/mcs_touchkey.c
>> @@ -45,6 +45,8 @@ struct mcs_touchkey_chip {
>>  };
>>
>>  struct mcs_touchkey_data {
>> +       void (*poweron)(int);
> Could you explain what it does?
The MCS5080 chip has enable or poweron key, or may be LDO pins.
So, it used in the platformdata, like above cfg_pin().

There is one reason about using poweron().
The MCS5080 chip must be re-initialize after resume().
If it's not used power off/on method and use only disable/enable irq,
it happened that it dosen't issued interrupt.
This is chip-level issue.

So, I think that it's the very simplest method to support poweron() function
to machine or platform data. 

> 
>> +
>>         struct i2c_client *client;
>>         struct input_dev *input_dev;
>>         struct mcs_touchkey_chip chip;
>> @@ -168,6 +170,8 @@ static int __devinit mcs_touchkey_probe(struct
>> i2c_client *client,
>>
>>         if (pdata->cfg_pin)
>>                 pdata->cfg_pin();
>> +       if (pdata->poweron)
>> +               data->poweron = pdata->poweron;
> Where is it called the first time?
Actually, in the suspend(), this function called the first time.
but, the actual power-up() sequence is possible in the cfg_pin(), too.
So, I use this method.

> 
>>
>>         error = request_threaded_irq(client->irq, NULL,
>> mcs_touchkey_interrupt,
>>                         IRQF_TRIGGER_FALLING, client->dev.driver->name,
>> data);
>> @@ -202,6 +206,41 @@ static int __devexit mcs_touchkey_remove(struct
>> i2c_client *client)
>>         return 0;
>>  }
>>
>> +#ifdef CONFIG_PM
>> +static int mcs_touchkey_suspend(struct i2c_client *client, pm_message_t
>> mesg)
>> +{
>> +       struct mcs_touchkey_data *data = i2c_get_clientdata(client);
>> +
>> +       /* Disable the work */
>> +       disable_irq(client->irq);
>> +
>> +       /* Don't I2C operation since we don't know I2C bus is dead already
>> */
>> +
>> +       /* Finally turn off the power */
>> +       if (data->poweron)
>> +               data->poweron(0);
>> +
>> +       return 0;
>> +}
>> +
>> +static int mcs_touchkey_resume(struct i2c_client *client)
>> +{
>> +       struct mcs_touchkey_data *data = i2c_get_clientdata(client);
>> +
>> +       /* Enable the device first */
>> +       if (data->poweron)
>> +               data->poweron(1);
>> +
>> +       /* Enable irq again */
>> +       enable_irq(client->irq);
>> +
>> +       return 0;
>> +}
>> +#else
>> +#define mcs_touchkey_suspend   NULL
>> +#define mcs_touchkey_resume    NULL
>> +#endif
>> +
>>  static const struct i2c_device_id mcs_touchkey_id[] = {
>>         { "mcs5000_touchkey", MCS5000_TOUCHKEY },
>>         { "mcs5080_touchkey", MCS5080_TOUCHKEY },
>> @@ -216,6 +255,8 @@ static struct i2c_driver mcs_touchkey_driver = {
>>         },
>>         .probe          = mcs_touchkey_probe,
>>         .remove         = __devexit_p(mcs_touchkey_remove),
>> +       .suspend        = mcs_touchkey_suspend,
>> +       .resume         = mcs_touchkey_resume,
>>         .id_table       = mcs_touchkey_id,
>>  };
>>
>> diff --git a/include/linux/i2c/mcs.h b/include/linux/i2c/mcs.h
>> index 725ae7c..c4a3869 100644
>> --- a/include/linux/i2c/mcs.h
>> +++ b/include/linux/i2c/mcs.h
>> @@ -18,6 +18,7 @@
>>  #define MCS_KEY_CODE(v)                ((v) & 0xffff)
>>
>>  struct mcs_platform_data {
>> +       void (*poweron)(int);
>>         void (*cfg_pin)(void);
>>
>>         /* touchscreen */
>> --
>> 1.7.0.4
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-input" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Thanks.
If u any ideas, let me know.

Regards,
Heundjun Kim
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] input: keyboard: MCS5080: support suspend/resume.

[Dmitry Torokhov]

Hi Kim,

On Mon, Nov 15, 2010 at 01:31:45PM +0900, Kim, HeungJun wrote:
> This patch supports suspend/resume functions for mcs5080 touchkey
> driver.
> 
> Signed-off-by: Heungjun Kim <riverful.kim@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/input/keyboard/mcs_touchkey.c |   41 +++++++++++++++++++++++++++++++++
>  include/linux/i2c/mcs.h               |    1 +
>  2 files changed, 42 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/input/keyboard/mcs_touchkey.c b/drivers/input/keyboard/mcs_touchkey.c
> index 63b849d..06385f5 100644
> --- a/drivers/input/keyboard/mcs_touchkey.c
> +++ b/drivers/input/keyboard/mcs_touchkey.c
> @@ -45,6 +45,8 @@ struct mcs_touchkey_chip {
>  };
> 
>  struct mcs_touchkey_data {
> +       void (*poweron)(int);

int (*poweron)(bool);

The method must be able to signal errors.

> +
>         struct i2c_client *client;
>         struct input_dev *input_dev;
>         struct mcs_touchkey_chip chip;
> @@ -168,6 +170,8 @@ static int __devinit mcs_touchkey_probe(struct i2c_client *client,
> 
>         if (pdata->cfg_pin)
>                 pdata->cfg_pin();
> +       if (pdata->poweron)
> +               data->poweron = pdata->poweron;
> 
>         error = request_threaded_irq(client->irq, NULL, mcs_touchkey_interrupt,
>                         IRQF_TRIGGER_FALLING, client->dev.driver->name, data);
> @@ -202,6 +206,41 @@ static int __devexit mcs_touchkey_remove(struct i2c_client *client)
>         return 0;
>  }
> 
> +#ifdef CONFIG_PM
> +static int mcs_touchkey_suspend(struct i2c_client *client, pm_message_t mesg)

Newer code should use dev_pm_ops please.
`
> +{
> +       struct mcs_touchkey_data *data = i2c_get_clientdata(client);
> +
> +       /* Disable the work */
> +       disable_irq(client->irq);
> +
> +       /* Don't I2C operation since we don't know I2C bus is dead already */

? I am not quite sure what you are trying to say here...

> +
> +       /* Finally turn off the power */
> +       if (data->poweron)
> +               data->poweron(0);
> +
> +       return 0;
> +}
> +
> +static int mcs_touchkey_resume(struct i2c_client *client)
> +{
> +       struct mcs_touchkey_data *data = i2c_get_clientdata(client);
> +
> +       /* Enable the device first */
> +       if (data->poweron)
> +               data->poweron(1);
> +
> +       /* Enable irq again */
> +       enable_irq(client->irq);
> +
> +       return 0;
> +}
> +#else
> +#define mcs_touchkey_suspend   NULL
> +#define mcs_touchkey_resume    NULL
> +#endif
> +
>  static const struct i2c_device_id mcs_touchkey_id[] = {
>         { "mcs5000_touchkey", MCS5000_TOUCHKEY },
>         { "mcs5080_touchkey", MCS5080_TOUCHKEY },
> @@ -216,6 +255,8 @@ static struct i2c_driver mcs_touchkey_driver = {
>         },
>         .probe          = mcs_touchkey_probe,
>         .remove         = __devexit_p(mcs_touchkey_remove),
> +       .suspend        = mcs_touchkey_suspend,
> +       .resume         = mcs_touchkey_resume,
>         .id_table       = mcs_touchkey_id,
>  };
> 
> diff --git a/include/linux/i2c/mcs.h b/include/linux/i2c/mcs.h
> index 725ae7c..c4a3869 100644
> --- a/include/linux/i2c/mcs.h
> +++ b/include/linux/i2c/mcs.h
> @@ -18,6 +18,7 @@
>  #define MCS_KEY_CODE(v)                ((v) & 0xffff)
> 
>  struct mcs_platform_data {
> +       void (*poweron)(int);
>         void (*cfg_pin)(void);
> 
>         /* touchscreen */
> --
> 1.7.0.4

Shouldn't you call poweron() at driver bind and unbind time (if you do not want to
supply open() and close() methods which wold be the best option)?

Thanks.

-- 
Dmitry

Re: [PATCH 1/3] input: keyboard: MCS5080: support suspend/resume.

[Kim, HeungJun]

Hi Dmitry,

I'll fix the following points, and re-send patches, soon.
And, I gonna send the led-trigger patch, later.
Actually, the led-blink command & method is changed according to touchkey F/W, as you know.
The cause of my sending led-patch is that I think it's the just basic function.
But, If the touchkey-drv use led-trigger framework, the driver code is heavy, I guess.

After I consider that deeply, I'll re-send patch. 

Thansk to review.

2010-11-15 오후 5:32, Dmitry Torokhov 쓴 글:
> Hi Kim,
> 
> On Mon, Nov 15, 2010 at 01:31:45PM +0900, Kim, HeungJun wrote:
>> This patch supports suspend/resume functions for mcs5080 touchkey
>> driver.
>>
>> Signed-off-by: Heungjun Kim <riverful.kim@samsung.com>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
>>  drivers/input/keyboard/mcs_touchkey.c |   41 +++++++++++++++++++++++++++++++++
>>  include/linux/i2c/mcs.h               |    1 +
>>  2 files changed, 42 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/input/keyboard/mcs_touchkey.c b/drivers/input/keyboard/mcs_touchkey.c
>> index 63b849d..06385f5 100644
>> --- a/drivers/input/keyboard/mcs_touchkey.c
>> +++ b/drivers/input/keyboard/mcs_touchkey.c
>> @@ -45,6 +45,8 @@ struct mcs_touchkey_chip {
>>  };
>>
>>  struct mcs_touchkey_data {
>> +       void (*poweron)(int);
> 
> int (*poweron)(bool);
> 
> The method must be able to signal errors.
Ok, I'll fix this.

> 
>> +
>>         struct i2c_client *client;
>>         struct input_dev *input_dev;
>>         struct mcs_touchkey_chip chip;
>> @@ -168,6 +170,8 @@ static int __devinit mcs_touchkey_probe(struct i2c_client *client,
>>
>>         if (pdata->cfg_pin)
>>                 pdata->cfg_pin();
>> +       if (pdata->poweron)
>> +               data->poweron = pdata->poweron;
>>
>>         error = request_threaded_irq(client->irq, NULL, mcs_touchkey_interrupt,
>>                         IRQF_TRIGGER_FALLING, client->dev.driver->name, data);
>> @@ -202,6 +206,41 @@ static int __devexit mcs_touchkey_remove(struct i2c_client *client)
>>         return 0;
>>  }
>>
>> +#ifdef CONFIG_PM
>> +static int mcs_touchkey_suspend(struct i2c_client *client, pm_message_t mesg)
> 
> Newer code should use dev_pm_ops please.
I got it. I'll adapt new runtime-pm framework.

> `
>> +{
>> +       struct mcs_touchkey_data *data = i2c_get_clientdata(client);
>> +
>> +       /* Disable the work */
>> +       disable_irq(client->irq);
>> +
>> +       /* Don't I2C operation since we don't know I2C bus is dead already */
> 
> ? I am not quite sure what you are trying to say here...
It's kind of defensive(?) code for sure.
If don't use this code and the I2C operation is crashed once,
after I2C operation is crashed, too.
It happened very frequently in the MCS5080 chip.
But fortunatly, It affects just own bus, not any other one.

> 
>> +
>> +       /* Finally turn off the power */
>> +       if (data->poweron)
>> +               data->poweron(0);
>> +
>> +       return 0;
>> +}
>> +
>> +static int mcs_touchkey_resume(struct i2c_client *client)
>> +{
>> +       struct mcs_touchkey_data *data = i2c_get_clientdata(client);
>> +
>> +       /* Enable the device first */
>> +       if (data->poweron)
>> +               data->poweron(1);
>> +
>> +       /* Enable irq again */
>> +       enable_irq(client->irq);
>> +
>> +       return 0;
>> +}
>> +#else
>> +#define mcs_touchkey_suspend   NULL
>> +#define mcs_touchkey_resume    NULL
>> +#endif
>> +
>>  static const struct i2c_device_id mcs_touchkey_id[] = {
>>         { "mcs5000_touchkey", MCS5000_TOUCHKEY },
>>         { "mcs5080_touchkey", MCS5080_TOUCHKEY },
>> @@ -216,6 +255,8 @@ static struct i2c_driver mcs_touchkey_driver = {
>>         },
>>         .probe          = mcs_touchkey_probe,
>>         .remove         = __devexit_p(mcs_touchkey_remove),
>> +       .suspend        = mcs_touchkey_suspend,
>> +       .resume         = mcs_touchkey_resume,
>>         .id_table       = mcs_touchkey_id,
>>  };
>>
>> diff --git a/include/linux/i2c/mcs.h b/include/linux/i2c/mcs.h
>> index 725ae7c..c4a3869 100644
>> --- a/include/linux/i2c/mcs.h
>> +++ b/include/linux/i2c/mcs.h
>> @@ -18,6 +18,7 @@
>>  #define MCS_KEY_CODE(v)                ((v) & 0xffff)
>>
>>  struct mcs_platform_data {
>> +       void (*poweron)(int);
>>         void (*cfg_pin)(void);
>>
>>         /* touchscreen */
>> --
>> 1.7.0.4
> 
> Shouldn't you call poweron() at driver bind and unbind time (if you do not want to
> supply open() and close() methods which wold be the best option)?
I got it. I'll locate poweron() function at the bind & unbind time.

> 
> Thanks.
> 

Regards,
HeundJun Kim
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html