Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1425698 > unrolled thread

[PATCH] HID: i2c-hid: set power sleep before shutdown

Started byGuohua Zhong <ghzhong@yifangdigital.com>
First post2016-06-18 13:00 +0200
Last post2016-06-21 09:50 +0200
Articles 6 — 5 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH] HID: i2c-hid: set power sleep before shutdown Guohua Zhong <ghzhong@yifangdigital.com> - 2016-06-18 13:00 +0200
    Re: [PATCH] HID: i2c-hid: set power sleep before shutdown Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-06-20 11:40 +0200
      Re: [PATCH] HID: i2c-hid: set power sleep before shutdown ghzhong <ghzhong@yifangdigital.com> - 2016-06-21 09:50 +0200
        Re: [PATCH] HID: i2c-hid: set power sleep before shutdown Jiri Kosina <jikos@kernel.org> - 2016-06-21 12:10 +0200
    Re: [PATCH] HID: i2c-hid: set power sleep before shutdown Mika Westerberg <mika.westerberg@linux.intel.com> - 2016-06-20 12:50 +0200
      Re: [PATCH] HID: i2c-hid: set power sleep before shutdown ghzhong <ghzhong@yifangdigital.com> - 2016-06-21 09:50 +0200

#1425698 — [PATCH] HID: i2c-hid: set power sleep before shutdown

FromGuohua Zhong <ghzhong@yifangdigital.com>
Date2016-06-18 13:00 +0200
Subject[PATCH] HID: i2c-hid: set power sleep before shutdown
Message-ID<rLm8N-2Yq-3@gated-at.bofh.it>
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

[toc] | [next] | [standalone]


#1426430

FromBenjamin Tissoires <benjamin.tissoires@redhat.com>
Date2016-06-20 11:40 +0200
Message-ID<rM3Qv-6sw-65@gated-at.bofh.it>
In reply to#1425698
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
> 
> 

[toc] | [prev] | [next] | [standalone]


#1427419

Fromghzhong <ghzhong@yifangdigital.com>
Date2016-06-21 09:50 +0200
Message-ID<rMoBz-2OV-21@gated-at.bofh.it>
In reply to#1426430
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
>>
>>
>

[toc] | [prev] | [next] | [standalone]


#1427535

FromJiri Kosina <jikos@kernel.org>
Date2016-06-21 12:10 +0200
Message-ID<rMqN3-4mp-11@gated-at.bofh.it>
In reply to#1427419
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

[toc] | [prev] | [next] | [standalone]


#1426479

FromMika Westerberg <mika.westerberg@linux.intel.com>
Date2016-06-20 12:50 +0200
Message-ID<rM4We-77a-15@gated-at.bofh.it>
In reply to#1425698
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
> 

[toc] | [prev] | [next] | [standalone]


#1427417

Fromghzhong <ghzhong@yifangdigital.com>
Date2016-06-21 09:50 +0200
Message-ID<rMoBz-2OV-17@gated-at.bofh.it>
In reply to#1426479
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
>>
>

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web