[PATCH v7] OMAP4:keypad: PM runtime

[PATCH v7] OMAP4:keypad: PM runtime

[Shubhrajyoti D]

From: Abraham Arce <x0066660@ti.com>

Enable Runtime PM functionality in OMAP4 driver based on the following assumptions

A minimal pm runtime get/put approach is implemented in probe/remove calls
respectively.

- Keyboard controller in wakeup domain so it is always on and
  power impact may be minimal
- In OMAP4 the device control is at module/device level and ick/fclk level control is
  difficult so cutting of clocks will prevent interrupts.

Signed-off-by: Abraham Arce <x0066660@ti.com>
Cc: Kevin Hilman <khilman@deeprootsystems.com>
---
Updating the changelogs as per Kevin's suggestion
 drivers/input/keyboard/omap4-keypad.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/keyboard/omap4-keypad.c
index 45bd097..3d35774 100644
--- a/drivers/input/keyboard/omap4-keypad.c
+++ b/drivers/input/keyboard/omap4-keypad.c
@@ -29,6 +29,7 @@
 #include <linux/io.h>
 #include <linux/input.h>
 #include <linux/slab.h>
+#include <linux/pm_runtime.h>
 
 #include <plat/omap4-keypad.h>
 
@@ -239,8 +240,11 @@ static int __devinit omap4_keypad_probe(struct platform_device *pdev)
 	matrix_keypad_build_keymap(pdata->keymap_data, row_shift,
 			input_dev->keycode, input_dev->keybit);
 
-	omap4_keypad_config(keypad_data);
+	pm_runtime_enable(&pdev->dev);
+	pm_runtime_get_sync(&pdev->dev);
 
+	omap4_keypad_config(keypad_data);
+	
 	error = request_irq(keypad_data->irq, omap4_keypad_interrupt,
 			     IRQF_TRIGGER_RISING,
 			     "omap4-keypad", keypad_data);
@@ -278,6 +282,10 @@ static int __devexit omap4_keypad_remove(struct platform_device *pdev)
 	struct resource *res;
 
 	free_irq(keypad_data->irq, keypad_data);
+
+	pm_runtime_put_sync(&pdev->dev);
+	pm_runtime_disable(&pdev->dev);
+
 	input_unregister_device(keypad_data->input);
 
 	iounmap(keypad_data->base);
-- 
1.7.0.4

Re: [PATCH v7] OMAP4:keypad: PM runtime

[Varadarajan, Charulatha]

* Shubhrajyoti D <a0393217@india.ti.com> [2010-12-15 09:39:58 +0530]:

> From: Abraham Arce <x0066660@ti.com>
> 
> Enable Runtime PM functionality in OMAP4 driver based on the following assumptions
> 
> A minimal pm runtime get/put approach is implemented in probe/remove calls
> respectively.
> 
> - Keyboard controller in wakeup domain so it is always on and
>   power impact may be minimal
> - In OMAP4 the device control is at module/device level and ick/fclk level control is
>   difficult so cutting of clocks will prevent interrupts.
> 
> Signed-off-by: Abraham Arce <x0066660@ti.com>
> Cc: Kevin Hilman <khilman@deeprootsystems.com>

This patch is sent thrice (once with a different subject) but the
version numbers are the same. It is not clear what is the intention of this
patch without hwmod database update. Am I missing any more patch here?

> ---
> Updating the changelogs as per Kevin's suggestion
>  drivers/input/keyboard/omap4-keypad.c |   10 +++++++++-
>  1 files changed, 9 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/keyboard/omap4-keypad.c
> index 45bd097..3d35774 100644
> --- a/drivers/input/keyboard/omap4-keypad.c
> +++ b/drivers/input/keyboard/omap4-keypad.c
> @@ -29,6 +29,7 @@
>  #include <linux/io.h>
>  #include <linux/input.h>
>  #include <linux/slab.h>
> +#include <linux/pm_runtime.h>
>  
>  #include <plat/omap4-keypad.h>
>  
> @@ -239,8 +240,11 @@ static int __devinit omap4_keypad_probe(struct platform_device *pdev)
>  	matrix_keypad_build_keymap(pdata->keymap_data, row_shift,
>  			input_dev->keycode, input_dev->keybit);
>  
> -	omap4_keypad_config(keypad_data);
> +	pm_runtime_enable(&pdev->dev);
> +	pm_runtime_get_sync(&pdev->dev);
>  
> +	omap4_keypad_config(keypad_data);
> +	
>  	error = request_irq(keypad_data->irq, omap4_keypad_interrupt,
>  			     IRQF_TRIGGER_RISING,
>  			     "omap4-keypad", keypad_data);
> @@ -278,6 +282,10 @@ static int __devexit omap4_keypad_remove(struct platform_device *pdev)
>  	struct resource *res;
>  
>  	free_irq(keypad_data->irq, keypad_data);
> +
> +	pm_runtime_put_sync(&pdev->dev);
> +	pm_runtime_disable(&pdev->dev);
> +
>  	input_unregister_device(keypad_data->input);
>  
>  	iounmap(keypad_data->base);
> -- 
> 1.7.0.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7] OMAP4:keypad: PM runtime

[Datta, Shubhrajyoti]

On Wed, Dec 15, 2010 at 11:01 AM, Varadarajan, Charulatha <charu@ti.com> wrote:
> * Shubhrajyoti D <a0393217@india.ti.com> [2010-12-15 09:39:58 +0530]:
>
>> From: Abraham Arce <x0066660@ti.com>
>>
>> Enable Runtime PM functionality in OMAP4 driver based on the following assumptions
>>
>> A minimal pm runtime get/put approach is implemented in probe/remove calls
>> respectively.
>>
>> - Keyboard controller in wakeup domain so it is always on and
>>   power impact may be minimal
>> - In OMAP4 the device control is at module/device level and ick/fclk level control is
>>   difficult so cutting of clocks will prevent interrupts.
>>
>> Signed-off-by: Abraham Arce <x0066660@ti.com>
>> Cc: Kevin Hilman <khilman@deeprootsystems.com>
>
> This patch is sent thrice (once with a different subject) but the
> version numbers are the same. It is not clear what is the intention of this
> patch without hwmod database update. Am I missing any more patch here?
Yes missed the version.
 I have updated the change logs to be more descriptive and the subject line.


Regarding the hwmod database update. That I deffered for reasons
- The clock changes need to be there as the update would mean reset of clocks.
- Thought of completing the drivers/input before the hwmod database
and the board changes.
- Also currently the driver relies on the uboot settings for clock
this might remove that dependency.

>
>> ---
>> Updating the changelogs as per Kevin's suggestion
>>  drivers/input/keyboard/omap4-keypad.c |   10 +++++++++-
>>  1 files changed, 9 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/keyboard/omap4-keypad.c
>> index 45bd097..3d35774 100644
>> --- a/drivers/input/keyboard/omap4-keypad.c
>> +++ b/drivers/input/keyboard/omap4-keypad.c
>> @@ -29,6 +29,7 @@
>>  #include <linux/io.h>
>>  #include <linux/input.h>
>>  #include <linux/slab.h>
>> +#include <linux/pm_runtime.h>
>>
>>  #include <plat/omap4-keypad.h>
>>
>> @@ -239,8 +240,11 @@ static int __devinit omap4_keypad_probe(struct platform_device *pdev)
>>       matrix_keypad_build_keymap(pdata->keymap_data, row_shift,
>>                       input_dev->keycode, input_dev->keybit);
>>
>> -     omap4_keypad_config(keypad_data);
>> +     pm_runtime_enable(&pdev->dev);
>> +     pm_runtime_get_sync(&pdev->dev);
>>
>> +     omap4_keypad_config(keypad_data);
>> +
>>       error = request_irq(keypad_data->irq, omap4_keypad_interrupt,
>>                            IRQF_TRIGGER_RISING,
>>                            "omap4-keypad", keypad_data);
>> @@ -278,6 +282,10 @@ static int __devexit omap4_keypad_remove(struct platform_device *pdev)
>>       struct resource *res;
>>
>>       free_irq(keypad_data->irq, keypad_data);
>> +
>> +     pm_runtime_put_sync(&pdev->dev);
>> +     pm_runtime_disable(&pdev->dev);
>> +
>>       input_unregister_device(keypad_data->input);
>>
>>       iounmap(keypad_data->base);
>> --
>> 1.7.0.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> 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
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7] OMAP4:keypad: PM runtime

[Varadarajan, Charulatha]

Shubhro,

On Wed, Dec 15, 2010 at 13:24, Datta, Shubhrajyoti <shubhrajyoti@ti.com> wrote:
> On Wed, Dec 15, 2010 at 11:01 AM, Varadarajan, Charulatha <charu@ti.com> wrote:
>> * Shubhrajyoti D <a0393217@india.ti.com> [2010-12-15 09:39:58 +0530]:
>>
>>> From: Abraham Arce <x0066660@ti.com>
>>>
>>> Enable Runtime PM functionality in OMAP4 driver based on the following assumptions
>>>
>>> A minimal pm runtime get/put approach is implemented in probe/remove calls
>>> respectively.
>>>
>>> - Keyboard controller in wakeup domain so it is always on and
>>>   power impact may be minimal
>>> - In OMAP4 the device control is at module/device level and ick/fclk level control is
>>>   difficult so cutting of clocks will prevent interrupts.
>>>
>>> Signed-off-by: Abraham Arce <x0066660@ti.com>
>>> Cc: Kevin Hilman <khilman@deeprootsystems.com>
>>
>> This patch is sent thrice (once with a different subject) but the
>> version numbers are the same. It is not clear what is the intention of this
>> patch without hwmod database update. Am I missing any more patch here?
> Yes missed the version.
>  I have updated the change logs to be more descriptive and the subject line.
>
>
> Regarding the hwmod database update. That I deffered for reasons
> - The clock changes need to be there as the update would mean reset of clocks.
> - Thought of completing the drivers/input before the hwmod database
> and the board changes.
> - Also currently the driver relies on the uboot settings for clock
> this might remove that dependency.

I wish to differ here. This patch would not fix the above issues because
pm runtime APIs (for OMAP) purely rely on omap_device and the keypad
driver is not yet hwmod adapted. Atleast put a dependency on any
hwmod series if you had sent them before.

-V Charulatha
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7] OMAP4:keypad: PM runtime

[Datta, Shubhrajyoti]

On Wed, Dec 15, 2010 at 2:28 PM, Varadarajan, Charulatha <charu@ti.com> wrote:
> Shubhro,
>
> On Wed, Dec 15, 2010 at 13:24, Datta, Shubhrajyoti <shubhrajyoti@ti.com> wrote:
>> On Wed, Dec 15, 2010 at 11:01 AM, Varadarajan, Charulatha <charu@ti.com> wrote:
>>> * Shubhrajyoti D <a0393217@india.ti.com> [2010-12-15 09:39:58 +0530]:
>>>
>>>> From: Abraham Arce <x0066660@ti.com>
>>>>
>>>> Enable Runtime PM functionality in OMAP4 driver based on the following assumptions
>>>>
>>>> A minimal pm runtime get/put approach is implemented in probe/remove calls
>>>> respectively.
>>>>
>>>> - Keyboard controller in wakeup domain so it is always on and
>>>>   power impact may be minimal
>>>> - In OMAP4 the device control is at module/device level and ick/fclk level control is
>>>>   difficult so cutting of clocks will prevent interrupts.
>>>>
>>>> Signed-off-by: Abraham Arce <x0066660@ti.com>
>>>> Cc: Kevin Hilman <khilman@deeprootsystems.com>
>>>
>>> This patch is sent thrice (once with a different subject) but the
>>> version numbers are the same. It is not clear what is the intention of this
>>> patch without hwmod database update. Am I missing any more patch here?
>> Yes missed the version.
>>  I have updated the change logs to be more descriptive and the subject line.
>>
>>
>> Regarding the hwmod database update. That I deffered for reasons
>> - The clock changes need to be there as the update would mean reset of clocks.
>> - Thought of completing the drivers/input before the hwmod database
>> and the board changes.
>> - Also currently the driver relies on the uboot settings for clock
>> this might remove that dependency.
>
> I wish to differ here. This patch would not fix the above issues because
> pm runtime APIs (for OMAP) purely rely on omap_device and the keypad
> driver is not yet hwmod adapted. Atleast put a dependency on any
> hwmod series if you had sent them before.
OK  will send a series with both patches mentioning the dependency.
>
> -V Charulatha
>
--
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

[PATCH v7] OMAP4:keypad: PM runtime

[Shubhrajyoti D]

From: Abraham Arce <x0066660@ti.com>

Enable Runtime PM functionality in OMAP4 driver based on the following assumptions
- Runtime PM selected by ARCH_OMAP2PLUS_TYPICAL && ARCH_OMAP2PLUS,
  which is the default OMAP2+ defconfig including OMAP4
- Runtime PM APIs handles Clock Framework APIs
- Do not do the keypadconfig if the irq request fails

A minimal pm runtime get/put approach is implemented in probe/remove calls
respectively based on:

- Keyboard controller in wakeup domain so it is always on and
  power impact may be minimal
- Cutting of clocks will prevent interrupts


Signed-off-by: Abraham Arce <x0066660@ti.com>
Cc: Kevin Hilman <khilman@deeprootsystems.com>
---
Updating the changelogs as per Kevin's suggestion

 drivers/input/keyboard/omap4-keypad.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/keyboard/omap4-keypad.c
index 45bd097..36239e2 100644
--- a/drivers/input/keyboard/omap4-keypad.c
+++ b/drivers/input/keyboard/omap4-keypad.c
@@ -29,6 +29,7 @@
 #include <linux/io.h>
 #include <linux/input.h>
 #include <linux/slab.h>
+#include <linux/pm_runtime.h>
 
 #include <plat/omap4-keypad.h>
 
@@ -239,7 +240,8 @@ static int __devinit omap4_keypad_probe(struct platform_device *pdev)
 	matrix_keypad_build_keymap(pdata->keymap_data, row_shift,
 			input_dev->keycode, input_dev->keybit);
 
-	omap4_keypad_config(keypad_data);
+	pm_runtime_enable(&pdev->dev);
+	pm_runtime_get_sync(&pdev->dev);
 
 	error = request_irq(keypad_data->irq, omap4_keypad_interrupt,
 			     IRQF_TRIGGER_RISING,
@@ -255,8 +257,9 @@ static int __devinit omap4_keypad_probe(struct platform_device *pdev)
 		goto err_free_irq;
 	}
 
-
+	omap4_keypad_config(keypad_data);
 	platform_set_drvdata(pdev, keypad_data);
+
 	return 0;
 
 err_free_irq:
@@ -278,6 +281,10 @@ static int __devexit omap4_keypad_remove(struct platform_device *pdev)
 	struct resource *res;
 
 	free_irq(keypad_data->irq, keypad_data);
+
+	pm_runtime_put_sync(&pdev->dev);
+	pm_runtime_disable(&pdev->dev);
+
 	input_unregister_device(keypad_data->input);
 
 	iounmap(keypad_data->base);
-- 
1.7.0.4

Re: [PATCH v7] OMAP4:keypad: PM runtime

[Kevin Hilman]

Shubhrajyoti D <a0393217@india.ti.com> writes:

> From: Abraham Arce <x0066660@ti.com>
>
> Enable Runtime PM functionality in OMAP4 driver based on the following assumptions
> - Runtime PM selected by ARCH_OMAP2PLUS_TYPICAL && ARCH_OMAP2PLUS,
>   which is the default OMAP2+ defconfig including OMAP4

this does not need to be stated in the changelog

> - Runtime PM APIs handles Clock Framework APIs

OMAP Runtime PM does much more than handle clocks.

> - Do not do the keypadconfig if the irq request fails

This is a separate fix, unrelated to enabling runtime PM.

> A minimal pm runtime get/put approach is implemented in probe/remove calls
> respectively based on:
>
> - Keyboard controller in wakeup domain so it is always on and
>   power impact may be minimal
> - Cutting of clocks will prevent interrupts

Again, please describe in more detail why preventing interrupts causes a
problem.    I understand it may seem obvious right now, but these
changelogs  need to be understandable many months/years down the road.

Kevin

> Signed-off-by: Abraham Arce <x0066660@ti.com>
> Cc: Kevin Hilman <khilman@deeprootsystems.com>
> ---
> Updating the changelogs as per Kevin's suggestion
>
>  drivers/input/keyboard/omap4-keypad.c |   11 +++++++++--
>  1 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/keyboard/omap4-keypad.c
> index 45bd097..36239e2 100644
> --- a/drivers/input/keyboard/omap4-keypad.c
> +++ b/drivers/input/keyboard/omap4-keypad.c
> @@ -29,6 +29,7 @@
>  #include <linux/io.h>
>  #include <linux/input.h>
>  #include <linux/slab.h>
> +#include <linux/pm_runtime.h>
>  
>  #include <plat/omap4-keypad.h>
>  
> @@ -239,7 +240,8 @@ static int __devinit omap4_keypad_probe(struct platform_device *pdev)
>  	matrix_keypad_build_keymap(pdata->keymap_data, row_shift,
>  			input_dev->keycode, input_dev->keybit);
>  
> -	omap4_keypad_config(keypad_data);
> +	pm_runtime_enable(&pdev->dev);
> +	pm_runtime_get_sync(&pdev->dev);
>  
>  	error = request_irq(keypad_data->irq, omap4_keypad_interrupt,
>  			     IRQF_TRIGGER_RISING,
> @@ -255,8 +257,9 @@ static int __devinit omap4_keypad_probe(struct platform_device *pdev)
>  		goto err_free_irq;
>  	}
>  
> -
> +	omap4_keypad_config(keypad_data);
>  	platform_set_drvdata(pdev, keypad_data);
> +
>  	return 0;
>  
>  err_free_irq:
> @@ -278,6 +281,10 @@ static int __devexit omap4_keypad_remove(struct platform_device *pdev)
>  	struct resource *res;
>  
>  	free_irq(keypad_data->irq, keypad_data);
> +
> +	pm_runtime_put_sync(&pdev->dev);
> +	pm_runtime_disable(&pdev->dev);
> +
>  	input_unregister_device(keypad_data->input);
>  
>  	iounmap(keypad_data->base);