Re: [PATCH v14 REPOST 07/12] OMAP: dmtimer: pm_runtime support
On Friday 26 August 2011 09:53 PM, Kevin Hilman wrote:
Santosh writes:
On Friday 15 July 2011 05:34 PM, Tarun Kanti DebBarma wrote:
Add pm_runtime feature to dmtimer whereby _get_sync() is called within
omap_dm_timer_enable(), _put_sync() is called in omap_dm_timer_disable().
Signed-off-by: Tarun Kanti DebBarma
[p-bas...@ti.com: added pm_runtime logic in probe()]
Signed-off-by: Partha Basak
Reviewed-by: Varadarajan, Charulatha
Acked-by: Cousson, Benoit
---
arch/arm/plat-omap/dmtimer.c | 31 +++-
arch/arm/plat-omap/include/plat/dmtimer.h |1 -
2 files changed, 8 insertions(+), 24 deletions(-)
diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
index 54564e9..0560248 100644
--- a/arch/arm/plat-omap/dmtimer.c
+++ b/arch/arm/plat-omap/dmtimer.c
@@ -38,6 +38,7 @@
#include
#include
#include
+#include
#include
@@ -193,33 +194,13 @@ EXPORT_SYMBOL_GPL(omap_dm_timer_free);
void omap_dm_timer_enable(struct omap_dm_timer *timer)
{
- struct dmtimer_platform_data *pdata = timer->pdev->dev.platform_data;
-
- if (timer->enabled)
- return;
-
- if (!pdata->needs_manual_reset) {
- clk_enable(timer->fclk);
- clk_enable(timer->iclk);
- }
-
- timer->enabled = 1;
+ pm_runtime_get_sync(&timer->pdev->dev);
}
EXPORT_SYMBOL_GPL(omap_dm_timer_enable);
void omap_dm_timer_disable(struct omap_dm_timer *timer)
{
- struct dmtimer_platform_data *pdata = timer->pdev->dev.platform_data;
-
- if (!timer->enabled)
- return;
-
- if (!pdata->needs_manual_reset) {
- clk_disable(timer->iclk);
- clk_disable(timer->fclk);
- }
-
- timer->enabled = 0;
+ pm_runtime_put_sync(&timer->pdev->dev);
Better to use pm_runtime_put_sync_suspend() to make
it irq safe.
Actually, better to just use the async version: pm_runtime_put()
Why does it need to be IRQ safe?
}
EXPORT_SYMBOL_GPL(omap_dm_timer_disable);
@@ -461,7 +442,7 @@ int omap_dm_timers_active(void)
struct omap_dm_timer *timer;
list_for_each_entry(timer,&omap_timer_list, node) {
- if (!timer->enabled)
+ if (!timer->reserved)
continue;
if (omap_dm_timer_read_reg(timer, OMAP_TIMER_CTRL_REG)&
@@ -536,6 +517,10 @@ static int __devinit omap_dm_timer_probe(struct
platform_device *pdev)
timer->irq = irq->start;
timer->pdev = pdev;
+ /* Skip pm_runtime_enable for OMAP1 */
+ if (!pdata->needs_manual_reset)
+ pm_runtime_enable(&pdev->dev);
+
Here too.
/s /pm_runtime_enable /pm_runtime_irq_safe
I think you mean adding _irq_safe, not removing the _enable?
I disagree with the IRQ-safe part. Let's avoid using IRQ-safe unless
*really* needed, and justified.
The reason may not be satisfactory, bust some the client drivers
are calling the timer APIs from IRQ context that created regressions
on internal trees. That's was the only reason behind my comment.
On other hand, none of this client drivers are supported in
mainline, so my comment from mainline point of is void.
Regards
Santosh
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v14 REPOST 07/12] OMAP: dmtimer: pm_runtime support
Santosh writes:
> On Friday 15 July 2011 05:34 PM, Tarun Kanti DebBarma wrote:
>> Add pm_runtime feature to dmtimer whereby _get_sync() is called within
>> omap_dm_timer_enable(), _put_sync() is called in omap_dm_timer_disable().
>>
>> Signed-off-by: Tarun Kanti DebBarma
>> [p-bas...@ti.com: added pm_runtime logic in probe()]
>> Signed-off-by: Partha Basak
>> Reviewed-by: Varadarajan, Charulatha
>> Acked-by: Cousson, Benoit
>> ---
>> arch/arm/plat-omap/dmtimer.c | 31
>> +++-
>> arch/arm/plat-omap/include/plat/dmtimer.h |1 -
>> 2 files changed, 8 insertions(+), 24 deletions(-)
>>
>> diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
>> index 54564e9..0560248 100644
>> --- a/arch/arm/plat-omap/dmtimer.c
>> +++ b/arch/arm/plat-omap/dmtimer.c
>> @@ -38,6 +38,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> #include
>>
>> @@ -193,33 +194,13 @@ EXPORT_SYMBOL_GPL(omap_dm_timer_free);
>>
>> void omap_dm_timer_enable(struct omap_dm_timer *timer)
>> {
>> -struct dmtimer_platform_data *pdata = timer->pdev->dev.platform_data;
>> -
>> -if (timer->enabled)
>> -return;
>> -
>> -if (!pdata->needs_manual_reset) {
>> -clk_enable(timer->fclk);
>> -clk_enable(timer->iclk);
>> -}
>> -
>> -timer->enabled = 1;
>> +pm_runtime_get_sync(&timer->pdev->dev);
>> }
>> EXPORT_SYMBOL_GPL(omap_dm_timer_enable);
>>
>> void omap_dm_timer_disable(struct omap_dm_timer *timer)
>> {
>> -struct dmtimer_platform_data *pdata = timer->pdev->dev.platform_data;
>> -
>> -if (!timer->enabled)
>> -return;
>> -
>> -if (!pdata->needs_manual_reset) {
>> -clk_disable(timer->iclk);
>> -clk_disable(timer->fclk);
>> -}
>> -
>> -timer->enabled = 0;
>> +pm_runtime_put_sync(&timer->pdev->dev);
> Better to use pm_runtime_put_sync_suspend() to make
> it irq safe.
Actually, better to just use the async version: pm_runtime_put()
Why does it need to be IRQ safe?
>> }
>> EXPORT_SYMBOL_GPL(omap_dm_timer_disable);
>>
>> @@ -461,7 +442,7 @@ int omap_dm_timers_active(void)
>> struct omap_dm_timer *timer;
>>
>> list_for_each_entry(timer,&omap_timer_list, node) {
>> -if (!timer->enabled)
>> +if (!timer->reserved)
>> continue;
>>
>> if (omap_dm_timer_read_reg(timer, OMAP_TIMER_CTRL_REG)&
>> @@ -536,6 +517,10 @@ static int __devinit omap_dm_timer_probe(struct
>> platform_device *pdev)
>> timer->irq = irq->start;
>> timer->pdev = pdev;
>>
>> +/* Skip pm_runtime_enable for OMAP1 */
>> +if (!pdata->needs_manual_reset)
>> +pm_runtime_enable(&pdev->dev);
>> +
> Here too.
> /s /pm_runtime_enable /pm_runtime_irq_safe
I think you mean adding _irq_safe, not removing the _enable?
I disagree with the IRQ-safe part. Let's avoid using IRQ-safe unless
*really* needed, and justified.
Kevin
>> /* add the timer element to the list */
>> spin_lock_irqsave(&dm_timer_lock, flags);
>> list_add_tail(&timer->node,&omap_timer_list);
>> diff --git a/arch/arm/plat-omap/include/plat/dmtimer.h
>> b/arch/arm/plat-omap/include/plat/dmtimer.h
>> index 90a504a..53d5da6 100644
>> --- a/arch/arm/plat-omap/include/plat/dmtimer.h
>> +++ b/arch/arm/plat-omap/include/plat/dmtimer.h
>> @@ -238,7 +238,6 @@ struct omap_dm_timer {
>> void __iomem *io_base;
>> unsigned long rate;
>> unsigned reserved:1;
>> -unsigned enabled:1;
>> unsigned posted:1;
>> u8 func_offset;
>> u8 intr_offset;
>
> With above fixed, you can add,
>
> Reviewed-by: Santosh Shilimkar
> Regards
> Santosh
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v14 REPOST 07/12] OMAP: dmtimer: pm_runtime support
On Friday 15 July 2011 05:34 PM, Tarun Kanti DebBarma wrote:
Add pm_runtime feature to dmtimer whereby _get_sync() is called within
omap_dm_timer_enable(), _put_sync() is called in omap_dm_timer_disable().
Signed-off-by: Tarun Kanti DebBarma
[p-bas...@ti.com: added pm_runtime logic in probe()]
Signed-off-by: Partha Basak
Reviewed-by: Varadarajan, Charulatha
Acked-by: Cousson, Benoit
---
arch/arm/plat-omap/dmtimer.c | 31 +++-
arch/arm/plat-omap/include/plat/dmtimer.h |1 -
2 files changed, 8 insertions(+), 24 deletions(-)
diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
index 54564e9..0560248 100644
--- a/arch/arm/plat-omap/dmtimer.c
+++ b/arch/arm/plat-omap/dmtimer.c
@@ -38,6 +38,7 @@
#include
#include
#include
+#include
#include
@@ -193,33 +194,13 @@ EXPORT_SYMBOL_GPL(omap_dm_timer_free);
void omap_dm_timer_enable(struct omap_dm_timer *timer)
{
- struct dmtimer_platform_data *pdata = timer->pdev->dev.platform_data;
-
- if (timer->enabled)
- return;
-
- if (!pdata->needs_manual_reset) {
- clk_enable(timer->fclk);
- clk_enable(timer->iclk);
- }
-
- timer->enabled = 1;
+ pm_runtime_get_sync(&timer->pdev->dev);
}
EXPORT_SYMBOL_GPL(omap_dm_timer_enable);
void omap_dm_timer_disable(struct omap_dm_timer *timer)
{
- struct dmtimer_platform_data *pdata = timer->pdev->dev.platform_data;
-
- if (!timer->enabled)
- return;
-
- if (!pdata->needs_manual_reset) {
- clk_disable(timer->iclk);
- clk_disable(timer->fclk);
- }
-
- timer->enabled = 0;
+ pm_runtime_put_sync(&timer->pdev->dev);
Better to use pm_runtime_put_sync_suspend() to make
it irq safe.
}
EXPORT_SYMBOL_GPL(omap_dm_timer_disable);
@@ -461,7 +442,7 @@ int omap_dm_timers_active(void)
struct omap_dm_timer *timer;
list_for_each_entry(timer,&omap_timer_list, node) {
- if (!timer->enabled)
+ if (!timer->reserved)
continue;
if (omap_dm_timer_read_reg(timer, OMAP_TIMER_CTRL_REG)&
@@ -536,6 +517,10 @@ static int __devinit omap_dm_timer_probe(struct
platform_device *pdev)
timer->irq = irq->start;
timer->pdev = pdev;
+ /* Skip pm_runtime_enable for OMAP1 */
+ if (!pdata->needs_manual_reset)
+ pm_runtime_enable(&pdev->dev);
+
Here too.
/s /pm_runtime_enable /pm_runtime_irq_safe
/* add the timer element to the list */
spin_lock_irqsave(&dm_timer_lock, flags);
list_add_tail(&timer->node,&omap_timer_list);
diff --git a/arch/arm/plat-omap/include/plat/dmtimer.h
b/arch/arm/plat-omap/include/plat/dmtimer.h
index 90a504a..53d5da6 100644
--- a/arch/arm/plat-omap/include/plat/dmtimer.h
+++ b/arch/arm/plat-omap/include/plat/dmtimer.h
@@ -238,7 +238,6 @@ struct omap_dm_timer {
void __iomem *io_base;
unsigned long rate;
unsigned reserved:1;
- unsigned enabled:1;
unsigned posted:1;
u8 func_offset;
u8 intr_offset;
With above fixed, you can add,
Reviewed-by: Santosh Shilimkar
Regards
Santosh
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v14 REPOST 07/12] OMAP: dmtimer: pm_runtime support
Add pm_runtime feature to dmtimer whereby _get_sync() is called within
omap_dm_timer_enable(), _put_sync() is called in omap_dm_timer_disable().
Signed-off-by: Tarun Kanti DebBarma
[p-bas...@ti.com: added pm_runtime logic in probe()]
Signed-off-by: Partha Basak
Reviewed-by: Varadarajan, Charulatha
Acked-by: Cousson, Benoit
---
arch/arm/plat-omap/dmtimer.c | 31 +++-
arch/arm/plat-omap/include/plat/dmtimer.h |1 -
2 files changed, 8 insertions(+), 24 deletions(-)
diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
index 54564e9..0560248 100644
--- a/arch/arm/plat-omap/dmtimer.c
+++ b/arch/arm/plat-omap/dmtimer.c
@@ -38,6 +38,7 @@
#include
#include
#include
+#include
#include
@@ -193,33 +194,13 @@ EXPORT_SYMBOL_GPL(omap_dm_timer_free);
void omap_dm_timer_enable(struct omap_dm_timer *timer)
{
- struct dmtimer_platform_data *pdata = timer->pdev->dev.platform_data;
-
- if (timer->enabled)
- return;
-
- if (!pdata->needs_manual_reset) {
- clk_enable(timer->fclk);
- clk_enable(timer->iclk);
- }
-
- timer->enabled = 1;
+ pm_runtime_get_sync(&timer->pdev->dev);
}
EXPORT_SYMBOL_GPL(omap_dm_timer_enable);
void omap_dm_timer_disable(struct omap_dm_timer *timer)
{
- struct dmtimer_platform_data *pdata = timer->pdev->dev.platform_data;
-
- if (!timer->enabled)
- return;
-
- if (!pdata->needs_manual_reset) {
- clk_disable(timer->iclk);
- clk_disable(timer->fclk);
- }
-
- timer->enabled = 0;
+ pm_runtime_put_sync(&timer->pdev->dev);
}
EXPORT_SYMBOL_GPL(omap_dm_timer_disable);
@@ -461,7 +442,7 @@ int omap_dm_timers_active(void)
struct omap_dm_timer *timer;
list_for_each_entry(timer, &omap_timer_list, node) {
- if (!timer->enabled)
+ if (!timer->reserved)
continue;
if (omap_dm_timer_read_reg(timer, OMAP_TIMER_CTRL_REG) &
@@ -536,6 +517,10 @@ static int __devinit omap_dm_timer_probe(struct
platform_device *pdev)
timer->irq = irq->start;
timer->pdev = pdev;
+ /* Skip pm_runtime_enable for OMAP1 */
+ if (!pdata->needs_manual_reset)
+ pm_runtime_enable(&pdev->dev);
+
/* add the timer element to the list */
spin_lock_irqsave(&dm_timer_lock, flags);
list_add_tail(&timer->node, &omap_timer_list);
diff --git a/arch/arm/plat-omap/include/plat/dmtimer.h
b/arch/arm/plat-omap/include/plat/dmtimer.h
index 90a504a..53d5da6 100644
--- a/arch/arm/plat-omap/include/plat/dmtimer.h
+++ b/arch/arm/plat-omap/include/plat/dmtimer.h
@@ -238,7 +238,6 @@ struct omap_dm_timer {
void __iomem *io_base;
unsigned long rate;
unsigned reserved:1;
- unsigned enabled:1;
unsigned posted:1;
u8 func_offset;
u8 intr_offset;
--
1.6.0.4
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
