All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] OMAP3: Fixed crash bug with serial + suspend
@ 2009-03-03  9:45 Tero Kristo
  2009-03-03 16:57 ` Kevin Hilman
  0 siblings, 1 reply; 6+ messages in thread
From: Tero Kristo @ 2009-03-03  9:45 UTC (permalink / raw)
  To: linux-omap

It was possible for an unhandled interrupt to occur if there was incoming
serial traffic during wakeup from suspend. This was caused by the code
in arch-arm/mach-omap2/serial.c keeping interrupt enabled all the time,
but not acking its interrupts. Applies on top of PM branch.

Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
---
 arch/arm/mach-omap2/pm34xx.c             |   15 +++++++++++++++
 arch/arm/mach-omap2/serial.c             |   18 ++++++++++++++++++
 arch/arm/plat-omap/include/mach/serial.h |    1 +
 3 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 9102cee..5a627db 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -629,7 +629,22 @@ static void omap3_pm_finish(void)
 	pm_idle = saved_idle;
 }
 
+/* Hooks to enable / disable UART interrupts during suspend */
+static int omap3_pm_begin(suspend_state_t state)
+{
+	omap_uart_enable_irqs(0);
+	return 0;
+}
+
+static void omap3_pm_end(void)
+{
+	omap_uart_enable_irqs(1);
+	return;
+}
+
 static struct platform_suspend_ops omap_pm_ops = {
+	.begin		= omap3_pm_begin,
+	.end		= omap3_pm_end,
 	.prepare	= omap3_pm_prepare,
 	.enter		= omap3_pm_enter,
 	.finish		= omap3_pm_finish,
diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
index 90f1c67..952da79 100644
--- a/arch/arm/mach-omap2/serial.c
+++ b/arch/arm/mach-omap2/serial.c
@@ -432,6 +432,24 @@ static void omap_uart_idle_init(struct omap_uart_state *uart)
 	WARN_ON(ret);
 }
 
+void omap_uart_enable_irqs(int enable)
+{
+	int i, ret;
+
+	for (i = 0; i < OMAP_MAX_NR_PORTS; i++) {
+		struct plat_serial8250_port *p = serial_platform_data + i;
+		struct omap_uart_state *uart = &omap_uart[i];
+
+		if (!p->membase)
+			continue;
+		if (enable)
+			ret = request_irq(p->irq, omap_uart_interrupt,
+				IRQF_SHARED, "serial idle", (void *)uart);
+		else
+			free_irq(p->irq, (void *)uart);
+	}
+}
+
 static ssize_t sleep_timeout_show(struct kobject *kobj,
 				  struct kobj_attribute *attr,
 				  char *buf)
diff --git a/arch/arm/plat-omap/include/mach/serial.h b/arch/arm/plat-omap/include/mach/serial.h
index 8e89585..7ca1f27 100644
--- a/arch/arm/plat-omap/include/mach/serial.h
+++ b/arch/arm/plat-omap/include/mach/serial.h
@@ -47,6 +47,7 @@ extern void omap_uart_check_wakeup(void);
 extern void omap_uart_prepare_suspend(void);
 extern void omap_uart_prepare_idle(int num);
 extern void omap_uart_resume_idle(int num);
+extern void omap_uart_enable_irqs(int enable);
 #endif
 
 #endif
-- 
1.5.4.3


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] OMAP3: Fixed crash bug with serial + suspend
  2009-03-03  9:45 [PATCH] OMAP3: Fixed crash bug with serial + suspend Tero Kristo
@ 2009-03-03 16:57 ` Kevin Hilman
  2009-03-04  8:52   ` Tero.Kristo
  0 siblings, 1 reply; 6+ messages in thread
From: Kevin Hilman @ 2009-03-03 16:57 UTC (permalink / raw)
  To: Tero Kristo; +Cc: linux-omap

Tero Kristo <tero.kristo@nokia.com> writes:

> It was possible for an unhandled interrupt to occur if there was incoming
> serial traffic during wakeup from suspend. This was caused by the code
> in arch-arm/mach-omap2/serial.c keeping interrupt enabled all the time,
> but not acking its interrupts. Applies on top of PM branch.
>
> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
> ---
>  arch/arm/mach-omap2/pm34xx.c             |   15 +++++++++++++++
>  arch/arm/mach-omap2/serial.c             |   18 ++++++++++++++++++
>  arch/arm/plat-omap/include/mach/serial.h |    1 +
>  3 files changed, 34 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 9102cee..5a627db 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -629,7 +629,22 @@ static void omap3_pm_finish(void)
>  	pm_idle = saved_idle;
>  }
>  
> +/* Hooks to enable / disable UART interrupts during suspend */
> +static int omap3_pm_begin(suspend_state_t state)
> +{
> +	omap_uart_enable_irqs(0);
> +	return 0;
> +}
> +
> +static void omap3_pm_end(void)
> +{
> +	omap_uart_enable_irqs(1);
> +	return;
> +}
> +
>  static struct platform_suspend_ops omap_pm_ops = {
> +	.begin		= omap3_pm_begin,
> +	.end		= omap3_pm_end,
>  	.prepare	= omap3_pm_prepare,
>  	.enter		= omap3_pm_enter,
>  	.finish		= omap3_pm_finish,

I think this functionality is more appropriate for the prepare/finish
hooks than the begin/end hooks.  serial.c already has
omap_uart_prepare_suspend() hook where the interrupt disable could be
added., and a omap_uart_prepare_resume() call could be added to the
omap3_pm_finish.

> diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
> index 90f1c67..952da79 100644
> --- a/arch/arm/mach-omap2/serial.c
> +++ b/arch/arm/mach-omap2/serial.c
> @@ -432,6 +432,24 @@ static void omap_uart_idle_init(struct omap_uart_state *uart)
>  	WARN_ON(ret);
>  }
>  
> +void omap_uart_enable_irqs(int enable)
> +{
> +	int i, ret;
> +
> +	for (i = 0; i < OMAP_MAX_NR_PORTS; i++) {
> +		struct plat_serial8250_port *p = serial_platform_data + i;
> +		struct omap_uart_state *uart = &omap_uart[i];
> +

Can you use the "list_for_each_entry(uart, &uart_list, node)"
construct instead of a for loop?  Or was there a reason to enable/ack
all serial interrupts, even those that have not been enabled.

> +		if (!p->membase)
> +			continue;
> +		if (enable)
> +			ret = request_irq(p->irq, omap_uart_interrupt,
> +				IRQF_SHARED, "serial idle", (void *)uart);
> +		else
> +			free_irq(p->irq, (void *)uart);
> +	}
> +}
> +
>  static ssize_t sleep_timeout_show(struct kobject *kobj,
>  				  struct kobj_attribute *attr,
>  				  char *buf)
> diff --git a/arch/arm/plat-omap/include/mach/serial.h b/arch/arm/plat-omap/include/mach/serial.h
> index 8e89585..7ca1f27 100644
> --- a/arch/arm/plat-omap/include/mach/serial.h
> +++ b/arch/arm/plat-omap/include/mach/serial.h
> @@ -47,6 +47,7 @@ extern void omap_uart_check_wakeup(void);
>  extern void omap_uart_prepare_suspend(void);
>  extern void omap_uart_prepare_idle(int num);
>  extern void omap_uart_resume_idle(int num);
> +extern void omap_uart_enable_irqs(int enable);
>  #endif
>  
>  #endif
> -- 
> 1.5.4.3
>
> --
> 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] OMAP3: Fixed crash bug with serial + suspend
  2009-03-03 16:57 ` Kevin Hilman
@ 2009-03-04  8:52   ` Tero.Kristo
  2009-03-04 20:47     ` Kevin Hilman
  0 siblings, 1 reply; 6+ messages in thread
From: Tero.Kristo @ 2009-03-04  8:52 UTC (permalink / raw)
  To: khilman; +Cc: linux-omap

 

>-----Original Message-----
>From: ext Kevin Hilman [mailto:khilman@deeprootsystems.com] 
>Sent: 03 March, 2009 18:58
>To: Kristo Tero (Nokia-D/Tampere)
>Cc: linux-omap@vger.kernel.org
>Subject: Re: [PATCH] OMAP3: Fixed crash bug with serial + suspend
>
>Tero Kristo <tero.kristo@nokia.com> writes:
>
>> It was possible for an unhandled interrupt to occur if there was 
>> incoming serial traffic during wakeup from suspend. This was 
>caused by 
>> the code in arch-arm/mach-omap2/serial.c keeping interrupt 
>enabled all 
>> the time, but not acking its interrupts. Applies on top of PM branch.
>>
>> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
>> ---
>>  arch/arm/mach-omap2/pm34xx.c             |   15 +++++++++++++++
>>  arch/arm/mach-omap2/serial.c             |   18 ++++++++++++++++++
>>  arch/arm/plat-omap/include/mach/serial.h |    1 +
>>  3 files changed, 34 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/pm34xx.c 
>> b/arch/arm/mach-omap2/pm34xx.c index 9102cee..5a627db 100644
>> --- a/arch/arm/mach-omap2/pm34xx.c
>> +++ b/arch/arm/mach-omap2/pm34xx.c
>> @@ -629,7 +629,22 @@ static void omap3_pm_finish(void)
>>  	pm_idle = saved_idle;
>>  }
>>  
>> +/* Hooks to enable / disable UART interrupts during suspend 
>*/ static 
>> +int omap3_pm_begin(suspend_state_t state) {
>> +	omap_uart_enable_irqs(0);
>> +	return 0;
>> +}
>> +
>> +static void omap3_pm_end(void)
>> +{
>> +	omap_uart_enable_irqs(1);
>> +	return;
>> +}
>> +
>>  static struct platform_suspend_ops omap_pm_ops = {
>> +	.begin		= omap3_pm_begin,
>> +	.end		= omap3_pm_end,
>>  	.prepare	= omap3_pm_prepare,
>>  	.enter		= omap3_pm_enter,
>>  	.finish		= omap3_pm_finish,
>
>I think this functionality is more appropriate for the 
>prepare/finish hooks than the begin/end hooks.  serial.c already has
>omap_uart_prepare_suspend() hook where the interrupt disable 
>could be added., and a omap_uart_prepare_resume() call could 
>be added to the omap3_pm_finish.

This would cause sequencing issues and it would still be possible to get unhandled interrupts if I understand the behavior of suspend correctly. I think it works (simplified) like this:

call begin hook
suspend devices [ this is where the driver interrupt gets disabled ]
call prepare hook
enter state [ Zzzz ]
call finish hook
resume devices [ this is where the driver interrupt gets enabled ]
call end hook

So, if you use prepare / finish hooks to enable the interrupt in serial.c, you will get a short time period where you have the interrupt enabled but you do not have "real" handler for the interrupt. This time is longer if you have some drivers in the system that take a long time to resume/suspend and you are unlucky with the ordering of drivers.

>
>> diff --git a/arch/arm/mach-omap2/serial.c 
>> b/arch/arm/mach-omap2/serial.c index 90f1c67..952da79 100644
>> --- a/arch/arm/mach-omap2/serial.c
>> +++ b/arch/arm/mach-omap2/serial.c
>> @@ -432,6 +432,24 @@ static void omap_uart_idle_init(struct 
>omap_uart_state *uart)
>>  	WARN_ON(ret);
>>  }
>>  
>> +void omap_uart_enable_irqs(int enable) {
>> +	int i, ret;
>> +
>> +	for (i = 0; i < OMAP_MAX_NR_PORTS; i++) {
>> +		struct plat_serial8250_port *p = 
>serial_platform_data + i;
>> +		struct omap_uart_state *uart = &omap_uart[i];
>> +
>
>Can you use the "list_for_each_entry(uart, &uart_list, node)"
>construct instead of a for loop?  Or was there a reason to 
>enable/ack all serial interrupts, even those that have not 
>been enabled.

Hmm, yea that's true. I'll fix this one.

>
>> +		if (!p->membase)
>> +			continue;
>> +		if (enable)
>> +			ret = request_irq(p->irq, omap_uart_interrupt,
>> +				IRQF_SHARED, "serial idle", 
>(void *)uart);
>> +		else
>> +			free_irq(p->irq, (void *)uart);
>> +	}
>> +}
>> +
>>  static ssize_t sleep_timeout_show(struct kobject *kobj,
>>  				  struct kobj_attribute *attr,
>>  				  char *buf)
>> diff --git a/arch/arm/plat-omap/include/mach/serial.h 
>> b/arch/arm/plat-omap/include/mach/serial.h
>> index 8e89585..7ca1f27 100644
>> --- a/arch/arm/plat-omap/include/mach/serial.h
>> +++ b/arch/arm/plat-omap/include/mach/serial.h
>> @@ -47,6 +47,7 @@ extern void omap_uart_check_wakeup(void);  extern 
>> void omap_uart_prepare_suspend(void);  extern void 
>> omap_uart_prepare_idle(int num);  extern void 
>> omap_uart_resume_idle(int num);
>> +extern void omap_uart_enable_irqs(int enable);
>>  #endif
>>  
>>  #endif
>> --
>> 1.5.4.3
>>
>> --
>> 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
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] OMAP3: Fixed crash bug with serial + suspend
  2009-03-04  8:52   ` Tero.Kristo
@ 2009-03-04 20:47     ` Kevin Hilman
  0 siblings, 0 replies; 6+ messages in thread
From: Kevin Hilman @ 2009-03-04 20:47 UTC (permalink / raw)
  To: Tero.Kristo; +Cc: linux-omap

Tero.Kristo@nokia.com wrote:
>  
> 
>> -----Original Message-----
>> From: ext Kevin Hilman [mailto:khilman@deeprootsystems.com] 
>> Sent: 03 March, 2009 18:58
>> To: Kristo Tero (Nokia-D/Tampere)
>> Cc: linux-omap@vger.kernel.org
>> Subject: Re: [PATCH] OMAP3: Fixed crash bug with serial + suspend
>>
>> Tero Kristo <tero.kristo@nokia.com> writes:
>>
>>> It was possible for an unhandled interrupt to occur if there was 
>>> incoming serial traffic during wakeup from suspend. This was 
>> caused by 
>>> the code in arch-arm/mach-omap2/serial.c keeping interrupt 
>> enabled all 
>>> the time, but not acking its interrupts. Applies on top of PM branch.
>>>
>>> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
>>> ---
>>>  arch/arm/mach-omap2/pm34xx.c             |   15 +++++++++++++++
>>>  arch/arm/mach-omap2/serial.c             |   18 ++++++++++++++++++
>>>  arch/arm/plat-omap/include/mach/serial.h |    1 +
>>>  3 files changed, 34 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/pm34xx.c 
>>> b/arch/arm/mach-omap2/pm34xx.c index 9102cee..5a627db 100644
>>> --- a/arch/arm/mach-omap2/pm34xx.c
>>> +++ b/arch/arm/mach-omap2/pm34xx.c
>>> @@ -629,7 +629,22 @@ static void omap3_pm_finish(void)
>>>  	pm_idle = saved_idle;
>>>  }
>>>  
>>> +/* Hooks to enable / disable UART interrupts during suspend 
>> */ static 
>>> +int omap3_pm_begin(suspend_state_t state) {
>>> +	omap_uart_enable_irqs(0);
>>> +	return 0;
>>> +}
>>> +
>>> +static void omap3_pm_end(void)
>>> +{
>>> +	omap_uart_enable_irqs(1);
>>> +	return;
>>> +}
>>> +
>>>  static struct platform_suspend_ops omap_pm_ops = {
>>> +	.begin		= omap3_pm_begin,
>>> +	.end		= omap3_pm_end,
>>>  	.prepare	= omap3_pm_prepare,
>>>  	.enter		= omap3_pm_enter,
>>>  	.finish		= omap3_pm_finish,
>> I think this functionality is more appropriate for the 
>> prepare/finish hooks than the begin/end hooks.  serial.c already has
>> omap_uart_prepare_suspend() hook where the interrupt disable 
>> could be added., and a omap_uart_prepare_resume() call could 
>> be added to the omap3_pm_finish.
> 
> This would cause sequencing issues and it would still be possible to get unhandled interrupts if I understand the behavior of suspend correctly. I think it works (simplified) like this:
> 
> call begin hook
> suspend devices [ this is where the driver interrupt gets disabled ]
> call prepare hook
> enter state [ Zzzz ]
> call finish hook
> resume devices [ this is where the driver interrupt gets enabled ]
> call end hook
> 
> So, if you use prepare / finish hooks to enable the interrupt in serial.c, you will get a short time period where you have the interrupt enabled but you do not have "real" handler for the interrupt. This time is longer if you have some drivers in the system that take a long time to resume/suspend and you are unlucky with the ordering of drivers.

OK, makes sense.  I'm OK with the begin/end hook then.

There are some other conventions when using the begin/end hook that I'll 
probably add.  e.g., if the begin/end hooks are used then the next state
should be set based on the state passed in from begin, and ignored in 
the 'enter' hook.

Anyways, I will fix that part up.

>>> diff --git a/arch/arm/mach-omap2/serial.c 
>>> b/arch/arm/mach-omap2/serial.c index 90f1c67..952da79 100644
>>> --- a/arch/arm/mach-omap2/serial.c
>>> +++ b/arch/arm/mach-omap2/serial.c
>>> @@ -432,6 +432,24 @@ static void omap_uart_idle_init(struct 
>> omap_uart_state *uart)
>>>  	WARN_ON(ret);
>>>  }
>>>  
>>> +void omap_uart_enable_irqs(int enable) {
>>> +	int i, ret;
>>> +
>>> +	for (i = 0; i < OMAP_MAX_NR_PORTS; i++) {
>>> +		struct plat_serial8250_port *p = 
>> serial_platform_data + i;
>>> +		struct omap_uart_state *uart = &omap_uart[i];
>>> +
>> Can you use the "list_for_each_entry(uart, &uart_list, node)"
>> construct instead of a for loop?  Or was there a reason to 
>> enable/ack all serial interrupts, even those that have not 
>> been enabled.
> 
> Hmm, yea that's true. I'll fix this one.

ok, thanks.

Kevin

>>> +		if (!p->membase)
>>> +			continue;
>>> +		if (enable)
>>> +			ret = request_irq(p->irq, omap_uart_interrupt,
>>> +				IRQF_SHARED, "serial idle", 
>> (void *)uart);
>>> +		else
>>> +			free_irq(p->irq, (void *)uart);
>>> +	}
>>> +}
>>> +
>>>  static ssize_t sleep_timeout_show(struct kobject *kobj,
>>>  				  struct kobj_attribute *attr,
>>>  				  char *buf)
>>> diff --git a/arch/arm/plat-omap/include/mach/serial.h 
>>> b/arch/arm/plat-omap/include/mach/serial.h
>>> index 8e89585..7ca1f27 100644
>>> --- a/arch/arm/plat-omap/include/mach/serial.h
>>> +++ b/arch/arm/plat-omap/include/mach/serial.h
>>> @@ -47,6 +47,7 @@ extern void omap_uart_check_wakeup(void);  extern 
>>> void omap_uart_prepare_suspend(void);  extern void 
>>> omap_uart_prepare_idle(int num);  extern void 
>>> omap_uart_resume_idle(int num);
>>> +extern void omap_uart_enable_irqs(int enable);
>>>  #endif
>>>  
>>>  #endif
>>> --
>>> 1.5.4.3
>>>
>>> --
>>> 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
>>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] OMAP3: Fixed crash bug with serial + suspend
  2009-03-05 14:32 Tero Kristo
@ 2009-03-05 17:10 ` Kevin Hilman
  0 siblings, 0 replies; 6+ messages in thread
From: Kevin Hilman @ 2009-03-05 17:10 UTC (permalink / raw)
  To: Tero Kristo; +Cc: linux-omap

Tero Kristo <tero.kristo@nokia.com> writes:

> It was possible for an unhandled interrupt to occur if there was incoming
> serial traffic during wakeup from suspend. This was caused by the code
> in arch-arm/mach-omap2/serial.c keeping interrupt enabled all the time,
> but not acking its interrupts. Applies on top of PM branch.
>
> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>

Thanks, pushed to PM branch.

Kevin

> ---
>  arch/arm/mach-omap2/pm34xx.c             |   15 +++++++++++++++
>  arch/arm/mach-omap2/serial.c             |   14 ++++++++++++++
>  arch/arm/plat-omap/include/mach/serial.h |    1 +
>  3 files changed, 30 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 9102cee..5a627db 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -629,7 +629,22 @@ static void omap3_pm_finish(void)
>  	pm_idle = saved_idle;
>  }
>  
> +/* Hooks to enable / disable UART interrupts during suspend */
> +static int omap3_pm_begin(suspend_state_t state)
> +{
> +	omap_uart_enable_irqs(0);
> +	return 0;
> +}
> +
> +static void omap3_pm_end(void)
> +{
> +	omap_uart_enable_irqs(1);
> +	return;
> +}
> +
>  static struct platform_suspend_ops omap_pm_ops = {
> +	.begin		= omap3_pm_begin,
> +	.end		= omap3_pm_end,
>  	.prepare	= omap3_pm_prepare,
>  	.enter		= omap3_pm_enter,
>  	.finish		= omap3_pm_finish,
> diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
> index 90f1c67..e77567a 100644
> --- a/arch/arm/mach-omap2/serial.c
> +++ b/arch/arm/mach-omap2/serial.c
> @@ -432,6 +432,20 @@ static void omap_uart_idle_init(struct omap_uart_state *uart)
>  	WARN_ON(ret);
>  }
>  
> +void omap_uart_enable_irqs(int enable)
> +{
> +	int ret;
> +	struct omap_uart_state *uart;
> +
> +	list_for_each_entry(uart, &uart_list, node) {
> +		if (enable)
> +			ret = request_irq(uart->p->irq, omap_uart_interrupt,
> +				IRQF_SHARED, "serial idle", (void *)uart);
> +		else
> +			free_irq(uart->p->irq, (void *)uart);
> +	}
> +}
> +
>  static ssize_t sleep_timeout_show(struct kobject *kobj,
>  				  struct kobj_attribute *attr,
>  				  char *buf)
> diff --git a/arch/arm/plat-omap/include/mach/serial.h b/arch/arm/plat-omap/include/mach/serial.h
> index 8e89585..7ca1f27 100644
> --- a/arch/arm/plat-omap/include/mach/serial.h
> +++ b/arch/arm/plat-omap/include/mach/serial.h
> @@ -47,6 +47,7 @@ extern void omap_uart_check_wakeup(void);
>  extern void omap_uart_prepare_suspend(void);
>  extern void omap_uart_prepare_idle(int num);
>  extern void omap_uart_resume_idle(int num);
> +extern void omap_uart_enable_irqs(int enable);
>  #endif
>  
>  #endif
> -- 
> 1.5.4.3
>
> --
> 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] OMAP3: Fixed crash bug with serial + suspend
@ 2009-03-05 14:32 Tero Kristo
  2009-03-05 17:10 ` Kevin Hilman
  0 siblings, 1 reply; 6+ messages in thread
From: Tero Kristo @ 2009-03-05 14:32 UTC (permalink / raw)
  To: linux-omap

It was possible for an unhandled interrupt to occur if there was incoming
serial traffic during wakeup from suspend. This was caused by the code
in arch-arm/mach-omap2/serial.c keeping interrupt enabled all the time,
but not acking its interrupts. Applies on top of PM branch.

Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
---
 arch/arm/mach-omap2/pm34xx.c             |   15 +++++++++++++++
 arch/arm/mach-omap2/serial.c             |   14 ++++++++++++++
 arch/arm/plat-omap/include/mach/serial.h |    1 +
 3 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 9102cee..5a627db 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -629,7 +629,22 @@ static void omap3_pm_finish(void)
 	pm_idle = saved_idle;
 }
 
+/* Hooks to enable / disable UART interrupts during suspend */
+static int omap3_pm_begin(suspend_state_t state)
+{
+	omap_uart_enable_irqs(0);
+	return 0;
+}
+
+static void omap3_pm_end(void)
+{
+	omap_uart_enable_irqs(1);
+	return;
+}
+
 static struct platform_suspend_ops omap_pm_ops = {
+	.begin		= omap3_pm_begin,
+	.end		= omap3_pm_end,
 	.prepare	= omap3_pm_prepare,
 	.enter		= omap3_pm_enter,
 	.finish		= omap3_pm_finish,
diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
index 90f1c67..e77567a 100644
--- a/arch/arm/mach-omap2/serial.c
+++ b/arch/arm/mach-omap2/serial.c
@@ -432,6 +432,20 @@ static void omap_uart_idle_init(struct omap_uart_state *uart)
 	WARN_ON(ret);
 }
 
+void omap_uart_enable_irqs(int enable)
+{
+	int ret;
+	struct omap_uart_state *uart;
+
+	list_for_each_entry(uart, &uart_list, node) {
+		if (enable)
+			ret = request_irq(uart->p->irq, omap_uart_interrupt,
+				IRQF_SHARED, "serial idle", (void *)uart);
+		else
+			free_irq(uart->p->irq, (void *)uart);
+	}
+}
+
 static ssize_t sleep_timeout_show(struct kobject *kobj,
 				  struct kobj_attribute *attr,
 				  char *buf)
diff --git a/arch/arm/plat-omap/include/mach/serial.h b/arch/arm/plat-omap/include/mach/serial.h
index 8e89585..7ca1f27 100644
--- a/arch/arm/plat-omap/include/mach/serial.h
+++ b/arch/arm/plat-omap/include/mach/serial.h
@@ -47,6 +47,7 @@ extern void omap_uart_check_wakeup(void);
 extern void omap_uart_prepare_suspend(void);
 extern void omap_uart_prepare_idle(int num);
 extern void omap_uart_resume_idle(int num);
+extern void omap_uart_enable_irqs(int enable);
 #endif
 
 #endif
-- 
1.5.4.3


^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2009-03-05 17:10 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-03-03  9:45 [PATCH] OMAP3: Fixed crash bug with serial + suspend Tero Kristo
2009-03-03 16:57 ` Kevin Hilman
2009-03-04  8:52   ` Tero.Kristo
2009-03-04 20:47     ` Kevin Hilman
2009-03-05 14:32 Tero Kristo
2009-03-05 17:10 ` Kevin Hilman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.