* [PATCH] HID: i2c-hid: set power sleep before shutdown
@ 2016-06-18 10:50 Guohua Zhong
2016-06-20 9:34 ` Benjamin Tissoires
2016-06-20 10:39 ` Mika Westerberg
0 siblings, 2 replies; 6+ messages in thread
From: Guohua Zhong @ 2016-06-18 10:50 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Mika Westerberg,
Dmitry Torokhov, Benson Leung, Andrew Duggan
Cc: linux-input, linux-kernel, Guohua Zhong
Add i2c_hid_shutdown for i2c-hid driver to send suspend cmd &
disable irq before device shutdown.
Some HW design is that the power to i2c hid device won't down
after device shutdown. Also the i2c-hid driver do not send suspend
cmd to the hid i2c device and disable its irq before shutdown.So
if We touch the touchscreen or some other i2c hid device ,the power
consumtion will be go up even when the device is in shutdown state.
Though the root cause maybe a HW issue. But it seems that it is a
good pratice to set power sleep for i2c-hid device before shutdown.
Signed-off-by: Guohua Zhong <ghzhong@yifangdigital.com>
---
drivers/hid/i2c-hid/i2c-hid.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index 2e021ba..d6ea97b 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -1106,6 +1106,12 @@ static int i2c_hid_remove(struct i2c_client *client)
return 0;
}
+static void i2c_hid_shutdown(struct i2c_client *client)
+{
+ i2c_hid_set_power(client, I2C_HID_PWR_SLEEP);
+ disable_irq(client->irq);
+}
+
#ifdef CONFIG_PM_SLEEP
static int i2c_hid_suspend(struct device *dev)
{
@@ -1230,7 +1236,7 @@ static struct i2c_driver i2c_hid_driver = {
.probe = i2c_hid_probe,
.remove = i2c_hid_remove,
-
+ .shutdown = i2c_hid_shutdown,
.id_table = i2c_hid_id_table,
};
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] HID: i2c-hid: set power sleep before shutdown
2016-06-18 10:50 [PATCH] HID: i2c-hid: set power sleep before shutdown Guohua Zhong
@ 2016-06-20 9:34 ` Benjamin Tissoires
2016-06-21 7:35 ` ghzhong
2016-06-20 10:39 ` Mika Westerberg
1 sibling, 1 reply; 6+ messages in thread
From: Benjamin Tissoires @ 2016-06-20 9:34 UTC (permalink / raw)
To: Guohua Zhong
Cc: Jiri Kosina, Mika Westerberg, Dmitry Torokhov, Benson Leung,
Andrew Duggan, linux-input, linux-kernel
On Jun 18 2016 or thereabouts, Guohua Zhong wrote:
> Add i2c_hid_shutdown for i2c-hid driver to send suspend cmd &
> disable irq before device shutdown.
>
> Some HW design is that the power to i2c hid device won't down
> after device shutdown. Also the i2c-hid driver do not send suspend
> cmd to the hid i2c device and disable its irq before shutdown.So
> if We touch the touchscreen or some other i2c hid device ,the power
> consumtion will be go up even when the device is in shutdown state.
Wow, that's a new one! Could you tell us which device board is behaving
that way (if not under NDA or such)?
>
> Though the root cause maybe a HW issue. But it seems that it is a
> good pratice to set power sleep for i2c-hid device before shutdown.
Works for me:
Acked-By: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cheers,
Benjamin
>
> Signed-off-by: Guohua Zhong <ghzhong@yifangdigital.com>
> ---
> drivers/hid/i2c-hid/i2c-hid.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
> index 2e021ba..d6ea97b 100644
> --- a/drivers/hid/i2c-hid/i2c-hid.c
> +++ b/drivers/hid/i2c-hid/i2c-hid.c
> @@ -1106,6 +1106,12 @@ static int i2c_hid_remove(struct i2c_client *client)
> return 0;
> }
>
> +static void i2c_hid_shutdown(struct i2c_client *client)
> +{
> + i2c_hid_set_power(client, I2C_HID_PWR_SLEEP);
> + disable_irq(client->irq);
> +}
> +
> #ifdef CONFIG_PM_SLEEP
> static int i2c_hid_suspend(struct device *dev)
> {
> @@ -1230,7 +1236,7 @@ static struct i2c_driver i2c_hid_driver = {
>
> .probe = i2c_hid_probe,
> .remove = i2c_hid_remove,
> -
> + .shutdown = i2c_hid_shutdown,
> .id_table = i2c_hid_id_table,
> };
>
> --
> 1.9.1
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] HID: i2c-hid: set power sleep before shutdown
2016-06-18 10:50 [PATCH] HID: i2c-hid: set power sleep before shutdown Guohua Zhong
2016-06-20 9:34 ` Benjamin Tissoires
@ 2016-06-20 10:39 ` Mika Westerberg
2016-06-21 7:37 ` ghzhong
1 sibling, 1 reply; 6+ messages in thread
From: Mika Westerberg @ 2016-06-20 10:39 UTC (permalink / raw)
To: Guohua Zhong
Cc: Jiri Kosina, Benjamin Tissoires, Dmitry Torokhov, Benson Leung,
Andrew Duggan, linux-input, linux-kernel
On Sat, Jun 18, 2016 at 06:50:30PM +0800, Guohua Zhong wrote:
> Add i2c_hid_shutdown for i2c-hid driver to send suspend cmd &
> disable irq before device shutdown.
>
> Some HW design is that the power to i2c hid device won't down
> after device shutdown. Also the i2c-hid driver do not send suspend
> cmd to the hid i2c device and disable its irq before shutdown.So
> if We touch the touchscreen or some other i2c hid device ,the power
> consumtion will be go up even when the device is in shutdown state.
>
> Though the root cause maybe a HW issue. But it seems that it is a
> good pratice to set power sleep for i2c-hid device before shutdown.
>
> Signed-off-by: Guohua Zhong <ghzhong@yifangdigital.com>
> ---
> drivers/hid/i2c-hid/i2c-hid.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
> index 2e021ba..d6ea97b 100644
> --- a/drivers/hid/i2c-hid/i2c-hid.c
> +++ b/drivers/hid/i2c-hid/i2c-hid.c
> @@ -1106,6 +1106,12 @@ static int i2c_hid_remove(struct i2c_client *client)
> return 0;
> }
>
> +static void i2c_hid_shutdown(struct i2c_client *client)
> +{
> + i2c_hid_set_power(client, I2C_HID_PWR_SLEEP);
> + disable_irq(client->irq);
Why not free_irq() instead?
> +}
> +
> #ifdef CONFIG_PM_SLEEP
> static int i2c_hid_suspend(struct device *dev)
> {
> @@ -1230,7 +1236,7 @@ static struct i2c_driver i2c_hid_driver = {
>
> .probe = i2c_hid_probe,
> .remove = i2c_hid_remove,
> -
> + .shutdown = i2c_hid_shutdown,
> .id_table = i2c_hid_id_table,
> };
>
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] HID: i2c-hid: set power sleep before shutdown
2016-06-20 9:34 ` Benjamin Tissoires
@ 2016-06-21 7:35 ` ghzhong
2016-06-21 9:49 ` Jiri Kosina
0 siblings, 1 reply; 6+ messages in thread
From: ghzhong @ 2016-06-21 7:35 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Jiri Kosina, Mika Westerberg, Dmitry Torokhov, Benson Leung,
Andrew Duggan, linux-input, linux-kernel
On 06/20/2016 05:34 PM, Benjamin Tissoires wrote:
> On Jun 18 2016 or thereabouts, Guohua Zhong wrote:
>> Add i2c_hid_shutdown for i2c-hid driver to send suspend cmd &
>> disable irq before device shutdown.
>>
>> Some HW design is that the power to i2c hid device won't down
>> after device shutdown. Also the i2c-hid driver do not send suspend
>> cmd to the hid i2c device and disable its irq before shutdown.So
>> if We touch the touchscreen or some other i2c hid device ,the power
>> consumtion will be go up even when the device is in shutdown state.
> Wow, that's a new one! Could you tell us which device board is behaving
> that way (if not under NDA or such)
Umaro, a chromebook model.
>> Though the root cause maybe a HW issue. But it seems that it is a
>> good pratice to set power sleep for i2c-hid device before shutdown.
> Works for me:
> Acked-By: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>
> Cheers,
> Benjamin
>
>> Signed-off-by: Guohua Zhong <ghzhong@yifangdigital.com>
>> ---
>> drivers/hid/i2c-hid/i2c-hid.c | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
>> index 2e021ba..d6ea97b 100644
>> --- a/drivers/hid/i2c-hid/i2c-hid.c
>> +++ b/drivers/hid/i2c-hid/i2c-hid.c
>> @@ -1106,6 +1106,12 @@ static int i2c_hid_remove(struct i2c_client *client)
>> return 0;
>> }
>>
>> +static void i2c_hid_shutdown(struct i2c_client *client)
>> +{
>> + i2c_hid_set_power(client, I2C_HID_PWR_SLEEP);
>> + disable_irq(client->irq);
>> +}
>> +
>> #ifdef CONFIG_PM_SLEEP
>> static int i2c_hid_suspend(struct device *dev)
>> {
>> @@ -1230,7 +1236,7 @@ static struct i2c_driver i2c_hid_driver = {
>>
>> .probe = i2c_hid_probe,
>> .remove = i2c_hid_remove,
>> -
>> + .shutdown = i2c_hid_shutdown,
>> .id_table = i2c_hid_id_table,
>> };
>>
>> --
>> 1.9.1
>>
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] HID: i2c-hid: set power sleep before shutdown
2016-06-20 10:39 ` Mika Westerberg
@ 2016-06-21 7:37 ` ghzhong
0 siblings, 0 replies; 6+ messages in thread
From: ghzhong @ 2016-06-21 7:37 UTC (permalink / raw)
To: Mika Westerberg
Cc: Jiri Kosina, Benjamin Tissoires, Dmitry Torokhov, Benson Leung,
Andrew Duggan, linux-input, linux-kernel
On 06/20/2016 06:39 PM, Mika Westerberg wrote:
> On Sat, Jun 18, 2016 at 06:50:30PM +0800, Guohua Zhong wrote:
>> Add i2c_hid_shutdown for i2c-hid driver to send suspend cmd &
>> disable irq before device shutdown.
>>
>> Some HW design is that the power to i2c hid device won't down
>> after device shutdown. Also the i2c-hid driver do not send suspend
>> cmd to the hid i2c device and disable its irq before shutdown.So
>> if We touch the touchscreen or some other i2c hid device ,the power
>> consumtion will be go up even when the device is in shutdown state.
>>
>> Though the root cause maybe a HW issue. But it seems that it is a
>> good pratice to set power sleep for i2c-hid device before shutdown.
>>
>> Signed-off-by: Guohua Zhong <ghzhong@yifangdigital.com>
>> ---
>> drivers/hid/i2c-hid/i2c-hid.c | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
>> index 2e021ba..d6ea97b 100644
>> --- a/drivers/hid/i2c-hid/i2c-hid.c
>> +++ b/drivers/hid/i2c-hid/i2c-hid.c
>> @@ -1106,6 +1106,12 @@ static int i2c_hid_remove(struct i2c_client *client)
>> return 0;
>> }
>>
>> +static void i2c_hid_shutdown(struct i2c_client *client)
>> +{
>> + i2c_hid_set_power(client, I2C_HID_PWR_SLEEP);
>> + disable_irq(client->irq);
> Why not free_irq() instead?
Oh, you are right, the free_irq is better.
>
>> +}
>> +
>> #ifdef CONFIG_PM_SLEEP
>> static int i2c_hid_suspend(struct device *dev)
>> {
>> @@ -1230,7 +1236,7 @@ static struct i2c_driver i2c_hid_driver = {
>>
>> .probe = i2c_hid_probe,
>> .remove = i2c_hid_remove,
>> -
>> + .shutdown = i2c_hid_shutdown,
>> .id_table = i2c_hid_id_table,
>> };
>>
>> --
>> 1.9.1
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] HID: i2c-hid: set power sleep before shutdown
2016-06-21 7:35 ` ghzhong
@ 2016-06-21 9:49 ` Jiri Kosina
0 siblings, 0 replies; 6+ messages in thread
From: Jiri Kosina @ 2016-06-21 9:49 UTC (permalink / raw)
To: ghzhong
Cc: Benjamin Tissoires, Mika Westerberg, Dmitry Torokhov,
Benson Leung, Andrew Duggan, linux-input, linux-kernel
On Tue, 21 Jun 2016, ghzhong wrote:
> > > Some HW design is that the power to i2c hid device won't down
> > > after device shutdown. Also the i2c-hid driver do not send suspend
> > > cmd to the hid i2c device and disable its irq before shutdown.So
> > > if We touch the touchscreen or some other i2c hid device ,the power
> > > consumtion will be go up even when the device is in shutdown state.
> > Wow, that's a new one! Could you tell us which device board is behaving
> > that way (if not under NDA or such)
> Umaro, a chromebook model.
Could you please send me a v2 with updated changelog (specifying the model
that is known to need this) and the free_irq() change?
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-06-21 10:03 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-18 10:50 [PATCH] HID: i2c-hid: set power sleep before shutdown Guohua Zhong
2016-06-20 9:34 ` Benjamin Tissoires
2016-06-21 7:35 ` ghzhong
2016-06-21 9:49 ` Jiri Kosina
2016-06-20 10:39 ` Mika Westerberg
2016-06-21 7:37 ` ghzhong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).