Possible suspend/resume regression in .32-rc?

Possible suspend/resume regression in .32-rc?

[Daniel Mack]

Is anyone using suspend/resume with a recent git mainline kernel on PXA
or other ARM embedded boards? My platform used to suspend and resume
just fine on 2.6.31 but now as I rebased it, it fails the resume part.

Unfortunately, I can't bisect it as the platform is not mainline yet and
so I always have mandatory patches (without my platform won't do
anything) on top of the git repository. Which breaks the bisect logic.

What puzzles me is that I see the current raising at wakeup time, so at
least the processor seems to resume, but I can't see any serial console
output, just like if the kernel crashed very early after wakeup.
'no_console_suspend' didn't help either.

Does anyone use that successfully? Any idea for possible regressions?

Daniel

Possible suspend/resume regression in .32-rc?

[Eric Miao]

On Sat, Oct 31, 2009 at 9:34 AM, Daniel Mack <daniel@caiaq.de> wrote:
> Is anyone using suspend/resume with a recent git mainline kernel on PXA
> or other ARM embedded boards? My platform used to suspend and resume
> just fine on 2.6.31 but now as I rebased it, it fails the resume part.
>
> Unfortunately, I can't bisect it as the platform is not mainline yet and
> so I always have mandatory patches (without my platform won't do
> anything) on top of the git repository. Which breaks the bisect logic.
>
> What puzzles me is that I see the current raising at wakeup time, so at
> least the processor seems to resume, but I can't see any serial console
> output, just like if the kernel crashed very early after wakeup.
> 'no_console_suspend' didn't help either.
>

That will be very difficult to debug then. There are not many changes to
the suspend/resume as I can recall since 2.6.31. One suggestion if
you want to debug this is - add a dead loop at the suspicious place,
and see if you JTAG debugger can connect at that time - and of course
the PC is pointing to that dead loop to identify the point it can reach.

> Does anyone use that successfully? Any idea for possible regressions?
>
> Daniel
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

Possible suspend/resume regression in .32-rc?

[Robert Jarzmik]

Daniel Mack <daniel@caiaq.de> writes:

> Does anyone use that successfully? Any idea for possible regressions?
The issue seems confirmed. My mioa701 phone doesn't resume either, with
2.6.32-rc5 kernel.

I have no console available (except pxafb, but that's much later in the resume
sequence), so I won't be of much help, sorry.

Cheers.

--
Robert

Possible suspend/resume regression in .32-rc?

[Ben Dooks]

On Sat, Oct 31, 2009 at 05:39:32PM +0100, Robert Jarzmik wrote:
> Daniel Mack <daniel@caiaq.de> writes:
> 
> > Does anyone use that successfully? Any idea for possible regressions?
> The issue seems confirmed. My mioa701 phone doesn't resume either, with
> 2.6.32-rc5 kernel.

Works with several S3C24XX based machines, so it doesn't seem to be a
core problem.

-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.

Possible suspend/resume regression in .32-rc?

[Ryan Mallon]

Daniel Mack wrote:
> Is anyone using suspend/resume with a recent git mainline kernel on PXA
> or other ARM embedded boards? My platform used to suspend and resume
> just fine on 2.6.31 but now as I rebased it, it fails the resume part.

Are you using the same .config? I have had drivers with dodgy
suspend/resume code before. You could try removing unneeded drivers and
see if the problem still exists.

> Unfortunately, I can't bisect it as the platform is not mainline yet and
> so I always have mandatory patches (without my platform won't do
> anything) on top of the git repository. Which breaks the bisect logic.

You can do the bisect by creating a patch (or series of patches) between
mainline and your code. Do the bisect on the clean mainline and then
apply the patch on top of the bisect point at each stage.

> What puzzles me is that I see the current raising at wakeup time, so at
> least the processor seems to resume, but I can't see any serial console
> output, just like if the kernel crashed very early after wakeup.
> 'no_console_suspend' didn't help either.

You could try enabling CONFIG_DEBUG_LL and adding the following to
kernel/printk.c just above the call to vscnprintf:

	{
		extern void printascii(const char *);
		printascii(printk_buf);
	}

The alternative is to connect to the unit with a JTAG debugger and get a
stack trace and print the contents of __log_buf (the printk buffer).

~Ryan

-- 
Bluewater Systems Ltd - ARM Technology Solution Centre

Ryan Mallon         		5 Amuri Park, 404 Barbadoes St
ryan at bluewatersys.com         	PO Box 13 889, Christchurch 8013
http://www.bluewatersys.com	New Zealand
Phone: +64 3 3779127		Freecall: Australia 1800 148 751
Fax:   +64 3 3779135			  USA 1800 261 2934

Possible suspend/resume regression in .32-rc?

[Uwe Kleine-König]

Hallo Daniel, 

On Sat, Oct 31, 2009 at 02:34:27AM +0100, Daniel Mack wrote:
> Is anyone using suspend/resume with a recent git mainline kernel on PXA
> or other ARM embedded boards? My platform used to suspend and resume
> just fine on 2.6.31 but now as I rebased it, it fails the resume part.
> 
> Unfortunately, I can't bisect it as the platform is not mainline yet and
> so I always have mandatory patches (without my platform won't do
> anything) on top of the git repository. Which breaks the bisect logic.
What you can do is call 2.6.31 good and to test a vanilla state, do
something like

	git merge my_support_branch_based_on_2.6.31
	... test ...
	git reset --hard HEAD^
	git bisect --good_or_bad_as_found_by_test_above
 
Best regards
Uwe

-- 
Pengutronix e.K.                              | Uwe Kleine-K?nig            |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |

Possible suspend/resume regression in .32-rc?

[Daniel Mack]

On Sat, Oct 31, 2009 at 02:34:27AM +0100, Daniel Mack wrote:
> Is anyone using suspend/resume with a recent git mainline kernel on PXA
> or other ARM embedded boards? My platform used to suspend and resume
> just fine on 2.6.31 but now as I rebased it, it fails the resume part.
> 
> Unfortunately, I can't bisect it as the platform is not mainline yet and
> so I always have mandatory patches (without my platform won't do
> anything) on top of the git repository. Which breaks the bisect logic.
> 
> What puzzles me is that I see the current raising at wakeup time, so at
> least the processor seems to resume, but I can't see any serial console
> output, just like if the kernel crashed very early after wakeup.
> 'no_console_suspend' didn't help either.

Ok, got it. The culprit is commit d2c37068 ("[ARM] pxa: initialize
default interrupt priority and use ICHP for IRQ handling"). Reverting it
make suspend/resume work again on my board.

Haojian, Eric, could you have a look at this?

Many thanks for all input, everyone :)

Thanks,
Daniel

Possible suspend/resume regression in .32-rc?

[Pavel Machek]

Hi!

> Is anyone using suspend/resume with a recent git mainline kernel on PXA
> or other ARM embedded boards? My platform used to suspend and resume
> just fine on 2.6.31 but now as I rebased it, it fails the resume part.

2.6.32-rc* broke resume on spitz (pxa). I narrowed it down to crash in
arch_suspend_enable_irqs() in kernel/power/suspend.c...

Can you try to light a led or something just before it to see if you
hit same problem?
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Possible suspend/resume regression in .32-rc?

[Daniel Mack]

On Sun, Nov 01, 2009 at 10:13:25PM +0100, Pavel Machek wrote:
> > Is anyone using suspend/resume with a recent git mainline kernel on PXA
> > or other ARM embedded boards? My platform used to suspend and resume
> > just fine on 2.6.31 but now as I rebased it, it fails the resume part.
> 
> 2.6.32-rc* broke resume on spitz (pxa). I narrowed it down to crash in
> arch_suspend_enable_irqs() in kernel/power/suspend.c...
> 
> Can you try to light a led or something just before it to see if you
> hit same problem?

The problem seems more related to how the IRQs are set up, as I wrote in
my follow-up. Does reverting d2c370684 fix the problem for you, too?

Daniel

Re: Possible suspend/resume regression in .32-rc?

[Pavel Machek]

Hi!

> > Is anyone using suspend/resume with a recent git mainline kernel on PXA
> > or other ARM embedded boards? My platform used to suspend and resume
> > just fine on 2.6.31 but now as I rebased it, it fails the resume part.
> > 
> > Unfortunately, I can't bisect it as the platform is not mainline yet and
> > so I always have mandatory patches (without my platform won't do
> > anything) on top of the git repository. Which breaks the bisect logic.
> > 
> > What puzzles me is that I see the current raising at wakeup time, so at
> > least the processor seems to resume, but I can't see any serial console
> > output, just like if the kernel crashed very early after wakeup.
> > 'no_console_suspend' didn't help either.
> 
> Ok, got it. The culprit is commit d2c37068 ("[ARM] pxa: initialize
> default interrupt priority and use ICHP for IRQ handling"). Reverting it
> make suspend/resume work again on my board.
> 
> Haojian, Eric, could you have a look at this?

Okay, patch is this one: I'll test reverting it shortly.

commit d2c37068429b29d6549cf3486fc84b836689e122
Author: Haojian Zhuang <haojian.zhuang@marvell.com>
Date:   Wed Aug 19 19:49:31 2009 +0800

    [ARM] pxa: initialize default interrupt priority and use ICHP for IRQ handling
    
    Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
    Signed-off-by: Eric Miao <eric.y.miao@gmail.com>

diff --git a/arch/arm/mach-pxa/include/mach/entry-macro.S b/arch/arm/mach-pxa/include/mach/entry-macro.S
index f6b4bf3..2418806 100644
--- a/arch/arm/mach-pxa/include/mach/entry-macro.S
+++ b/arch/arm/mach-pxa/include/mach/entry-macro.S
@@ -24,34 +24,27 @@
 		mov	\tmp, \tmp, lsr #13
 		and	\tmp, \tmp, #0x7		@ Core G
 		cmp	\tmp, #1
-		bhi	1004f
+		bhi	1002f
 
+		@ Core Generation 1 (PXA25x)
 		mov	\base, #io_p2v(0x40000000)	@ IIR Ctl = 0x40d00000
 		add	\base, \base, #0x00d00000
 		ldr	\irqstat, [\base, #0]		@ ICIP
 		ldr	\irqnr, [\base, #4]		@ ICMR
-		b	1002f
 
-1004:
-		mrc	p6, 0, \irqstat, c6, c0, 0	@ ICIP2
-		mrc	p6, 0, \irqnr, c7, c0, 0	@ ICMR2
 		ands	\irqnr, \irqstat, \irqnr
-		beq	1003f
+		beq	1001f
 		rsb	\irqstat, \irqnr, #0
 		and	\irqstat, \irqstat, \irqnr
 		clz	\irqnr, \irqstat
-		rsb	\irqnr, \irqnr, #31
-		add	\irqnr, \irqnr, #(32 + PXA_IRQ(0))
+		rsb	\irqnr, \irqnr, #(31 + PXA_IRQ(0))
 		b	1001f
-1003:
-		mrc	p6, 0, \irqstat, c0, c0, 0	@ ICIP
-		mrc	p6, 0, \irqnr, c1, c0, 0	@ ICMR
 1002:
-		ands	\irqnr, \irqstat, \irqnr
+		@ Core Generation 2 (PXA27x) or Core Generation 3 (PXA3xx)
+		mrc	p6, 0, \irqstat, c5, c0, 0	@ ICHP
+		tst	\irqstat, #0x80000000
 		beq	1001f
-		rsb	\irqstat, \irqnr, #0
-		and	\irqstat, \irqstat, \irqnr
-		clz	\irqnr, \irqstat
-		rsb	\irqnr, \irqnr, #(31 + PXA_IRQ(0))
+		bic	\irqstat, \irqstat, #0x80000000
+		mov	\irqnr, \irqstat, lsr #16
 1001:
 		.endm
diff --git a/arch/arm/mach-pxa/irq.c b/arch/arm/mach-pxa/irq.c
index f6e0300..d694ce2 100644
--- a/arch/arm/mach-pxa/irq.c
+++ b/arch/arm/mach-pxa/irq.c
@@ -120,7 +120,7 @@ static void __init pxa_init_low_gpio_irq(set_wake_t fn)
 
 void __init pxa_init_irq(int irq_nr, set_wake_t fn)
 {
-	int irq;
+	int irq, i;
 
 	pxa_internal_irq_nr = irq_nr;
 
@@ -129,6 +129,12 @@ void __init pxa_init_irq(int irq_nr, set_wake_t fn)
 		_ICLR(irq) = 0;	/* all IRQs are IRQ, not FIQ */
 	}
 
+	/* initialize interrupt priority */
+	if (cpu_is_pxa27x() || cpu_is_pxa3xx()) {
+		for (i = 0; i < irq_nr; i++)
+			IPR(i) = i | (1 << 31);
+	}
+
 	/* only unmasked interrupts kick us out of idle */
 	ICCR = 1;
 


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Possible suspend/resume regression in .32-rc?

[Pavel Machek]

Hi!

> > Is anyone using suspend/resume with a recent git mainline kernel on PXA
> > or other ARM embedded boards? My platform used to suspend and resume
> > just fine on 2.6.31 but now as I rebased it, it fails the resume part.
> > 
> > Unfortunately, I can't bisect it as the platform is not mainline yet and
> > so I always have mandatory patches (without my platform won't do
> > anything) on top of the git repository. Which breaks the bisect logic.
> > 
> > What puzzles me is that I see the current raising at wakeup time, so at
> > least the processor seems to resume, but I can't see any serial console
> > output, just like if the kernel crashed very early after wakeup.
> > 'no_console_suspend' didn't help either.
> 
> Ok, got it. The culprit is commit d2c37068 ("[ARM] pxa: initialize
> default interrupt priority and use ICHP for IRQ handling"). Reverting it
> make suspend/resume work again on my board.
> 
> Haojian, Eric, could you have a look at this?

Okay, patch is this one: I'll test reverting it shortly.

commit d2c37068429b29d6549cf3486fc84b836689e122
Author: Haojian Zhuang <haojian.zhuang@marvell.com>
Date:   Wed Aug 19 19:49:31 2009 +0800

    [ARM] pxa: initialize default interrupt priority and use ICHP for IRQ handling
    
    Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
    Signed-off-by: Eric Miao <eric.y.miao@gmail.com>

diff --git a/arch/arm/mach-pxa/include/mach/entry-macro.S b/arch/arm/mach-pxa/include/mach/entry-macro.S
index f6b4bf3..2418806 100644
--- a/arch/arm/mach-pxa/include/mach/entry-macro.S
+++ b/arch/arm/mach-pxa/include/mach/entry-macro.S
@@ -24,34 +24,27 @@
 		mov	\tmp, \tmp, lsr #13
 		and	\tmp, \tmp, #0x7		@ Core G
 		cmp	\tmp, #1
-		bhi	1004f
+		bhi	1002f
 
+		@ Core Generation 1 (PXA25x)
 		mov	\base, #io_p2v(0x40000000)	@ IIR Ctl = 0x40d00000
 		add	\base, \base, #0x00d00000
 		ldr	\irqstat, [\base, #0]		@ ICIP
 		ldr	\irqnr, [\base, #4]		@ ICMR
-		b	1002f
 
-1004:
-		mrc	p6, 0, \irqstat, c6, c0, 0	@ ICIP2
-		mrc	p6, 0, \irqnr, c7, c0, 0	@ ICMR2
 		ands	\irqnr, \irqstat, \irqnr
-		beq	1003f
+		beq	1001f
 		rsb	\irqstat, \irqnr, #0
 		and	\irqstat, \irqstat, \irqnr
 		clz	\irqnr, \irqstat
-		rsb	\irqnr, \irqnr, #31
-		add	\irqnr, \irqnr, #(32 + PXA_IRQ(0))
+		rsb	\irqnr, \irqnr, #(31 + PXA_IRQ(0))
 		b	1001f
-1003:
-		mrc	p6, 0, \irqstat, c0, c0, 0	@ ICIP
-		mrc	p6, 0, \irqnr, c1, c0, 0	@ ICMR
 1002:
-		ands	\irqnr, \irqstat, \irqnr
+		@ Core Generation 2 (PXA27x) or Core Generation 3 (PXA3xx)
+		mrc	p6, 0, \irqstat, c5, c0, 0	@ ICHP
+		tst	\irqstat, #0x80000000
 		beq	1001f
-		rsb	\irqstat, \irqnr, #0
-		and	\irqstat, \irqstat, \irqnr
-		clz	\irqnr, \irqstat
-		rsb	\irqnr, \irqnr, #(31 + PXA_IRQ(0))
+		bic	\irqstat, \irqstat, #0x80000000
+		mov	\irqnr, \irqstat, lsr #16
 1001:
 		.endm
diff --git a/arch/arm/mach-pxa/irq.c b/arch/arm/mach-pxa/irq.c
index f6e0300..d694ce2 100644
--- a/arch/arm/mach-pxa/irq.c
+++ b/arch/arm/mach-pxa/irq.c
@@ -120,7 +120,7 @@ static void __init pxa_init_low_gpio_irq(set_wake_t fn)
 
 void __init pxa_init_irq(int irq_nr, set_wake_t fn)
 {
-	int irq;
+	int irq, i;
 
 	pxa_internal_irq_nr = irq_nr;
 
@@ -129,6 +129,12 @@ void __init pxa_init_irq(int irq_nr, set_wake_t fn)
 		_ICLR(irq) = 0;	/* all IRQs are IRQ, not FIQ */
 	}
 
+	/* initialize interrupt priority */
+	if (cpu_is_pxa27x() || cpu_is_pxa3xx()) {
+		for (i = 0; i < irq_nr; i++)
+			IPR(i) = i | (1 << 31);
+	}
+
 	/* only unmasked interrupts kick us out of idle */
 	ICCR = 1;
 


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: Possible suspend/resume regression in .32-rc?

[Pavel Machek]

> Hi!
> 
> > > Is anyone using suspend/resume with a recent git mainline kernel on PXA
> > > or other ARM embedded boards? My platform used to suspend and resume
> > > just fine on 2.6.31 but now as I rebased it, it fails the resume part.
> > > 
> > > Unfortunately, I can't bisect it as the platform is not mainline yet and
> > > so I always have mandatory patches (without my platform won't do
> > > anything) on top of the git repository. Which breaks the bisect logic.
> > > 
> > > What puzzles me is that I see the current raising at wakeup time, so at
> > > least the processor seems to resume, but I can't see any serial console
> > > output, just like if the kernel crashed very early after wakeup.
> > > 'no_console_suspend' didn't help either.
> > 
> > Ok, got it. The culprit is commit d2c37068 ("[ARM] pxa: initialize
> > default interrupt priority and use ICHP for IRQ handling"). Reverting it
> > make suspend/resume work again on my board.
> > 
> > Haojian, Eric, could you have a look at this?
> 
> Okay, patch is this one: I'll test reverting it shortly.
> 
> commit d2c37068429b29d6549cf3486fc84b836689e122
> Author: Haojian Zhuang <haojian.zhuang@marvell.com>
> Date:   Wed Aug 19 19:49:31 2009 +0800
> 
>     [ARM] pxa: initialize default interrupt priority and use ICHP for IRQ handling
>     
>     Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
>     Signed-off-by: Eric Miao <eric.y.miao@gmail.com>

And yes, reverting it _does_ fix suspend on spitz.

									Pavel


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Possible suspend/resume regression in .32-rc?

[Pavel Machek]

> Hi!
> 
> > > Is anyone using suspend/resume with a recent git mainline kernel on PXA
> > > or other ARM embedded boards? My platform used to suspend and resume
> > > just fine on 2.6.31 but now as I rebased it, it fails the resume part.
> > > 
> > > Unfortunately, I can't bisect it as the platform is not mainline yet and
> > > so I always have mandatory patches (without my platform won't do
> > > anything) on top of the git repository. Which breaks the bisect logic.
> > > 
> > > What puzzles me is that I see the current raising at wakeup time, so at
> > > least the processor seems to resume, but I can't see any serial console
> > > output, just like if the kernel crashed very early after wakeup.
> > > 'no_console_suspend' didn't help either.
> > 
> > Ok, got it. The culprit is commit d2c37068 ("[ARM] pxa: initialize
> > default interrupt priority and use ICHP for IRQ handling"). Reverting it
> > make suspend/resume work again on my board.
> > 
> > Haojian, Eric, could you have a look at this?
> 
> Okay, patch is this one: I'll test reverting it shortly.
> 
> commit d2c37068429b29d6549cf3486fc84b836689e122
> Author: Haojian Zhuang <haojian.zhuang@marvell.com>
> Date:   Wed Aug 19 19:49:31 2009 +0800
> 
>     [ARM] pxa: initialize default interrupt priority and use ICHP for IRQ handling
>     
>     Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
>     Signed-off-by: Eric Miao <eric.y.miao@gmail.com>

And yes, reverting it _does_ fix suspend on spitz.

									Pavel


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: Possible suspend/resume regression in .32-rc?

[Haojian Zhuang]

On Sun, Nov 1, 2009 at 6:03 PM, Pavel Machek <pavel@ucw.cz> wrote:
>> Hi!
>>
>> > > Is anyone using suspend/resume with a recent git mainline kernel on PXA
>> > > or other ARM embedded boards? My platform used to suspend and resume
>> > > just fine on 2.6.31 but now as I rebased it, it fails the resume part.
>> > >
>> > > Unfortunately, I can't bisect it as the platform is not mainline yet and
>> > > so I always have mandatory patches (without my platform won't do
>> > > anything) on top of the git repository. Which breaks the bisect logic.
>> > >
>> > > What puzzles me is that I see the current raising at wakeup time, so at
>> > > least the processor seems to resume, but I can't see any serial console
>> > > output, just like if the kernel crashed very early after wakeup.
>> > > 'no_console_suspend' didn't help either.
>> >
>> > Ok, got it. The culprit is commit d2c37068 ("[ARM] pxa: initialize
>> > default interrupt priority and use ICHP for IRQ handling"). Reverting it
>> > make suspend/resume work again on my board.
>> >
>> > Haojian, Eric, could you have a look at this?
>>
>> Okay, patch is this one: I'll test reverting it shortly.
>>
>> commit d2c37068429b29d6549cf3486fc84b836689e122
>> Author: Haojian Zhuang <haojian.zhuang@marvell.com>
>> Date:   Wed Aug 19 19:49:31 2009 +0800
>>
>>     [ARM] pxa: initialize default interrupt priority and use ICHP for IRQ handling
>>
>>     Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
>>     Signed-off-by: Eric Miao <eric.y.miao@gmail.com>
>
> And yes, reverting it _does_ fix suspend on spitz.
>
>                                                                        Pavel

Em, it's not caused by the IRQ patch.

The kernel is blocked in resume path. When console is resumed, IRQ is
already disabled and system is blocked. Actually, IRQ shouldn't be
disabled at here. Up to now, I only find which patch will cause this
issue. But I can't find the best solution on it. The patch with issue
is pasted in below.

So this issue is only occused when console suspend is enabled. If you
enable no_console_suspend in command, you won't meet this issue. It
seems that it's caused by removing termios setting in
uart_resume_port() in the below patch. If I add these code back, the
issue doesn't occur any more.

Deepak,
Could you help to investigate this issue also?

By the way, the suggested quick test command is in below. If we tried
these commands for 20 times, the issue would occur. I only test it in
PXA silicon. I don't know the symptom on other silicons.
$ echo devices > /sys/power/pm_test
$ echo mem > /sys/power/state

Thanks
Haojian

>From ba15ab0e8de0d4439a91342ad52d55ca9e313f3d Mon Sep 17 00:00:00 2001
From: Deepak Saxena <dsaxena@laptop.org>
Date: Sat, 19 Sep 2009 13:13:33 -0700
Subject: [PATCH] Set proper console speed on resume if console suspend
is disabled

Commit b5b82df6, from May 2007, breaks no_console_suspend on the OLPC
XO laptop. Basically what happens is that upon returning from resume,
serial8250_resume_port() will reconfigure the port for high speed
mode and all console output will be garbled, making debug of the
resume path painful. This patch modifies uart_resume_port() to
reset the port to the state it was in before we suspended.

Original patch by Marcelo Tosatti

Second patch by Deepak then reworked by Alan to fit with the tty changes
before it got submitted. Also fixed the console path to set c_i/ospeed as
some drivers require the termios fields are valid

Signed-off-by: Deepak Saxena <dsaxena@laptop.org>
Signed-off-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
---
 drivers/serial/serial_core.c |   32 ++++++++++++++++++--------------
 1 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
index 3fd0134..2514d00 100644
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -2068,11 +2068,29 @@ int uart_resume_port(struct uart_driver *drv,
struct uart_port *uport)
 	struct tty_port *port = &state->port;
 	struct device *tty_dev;
 	struct uart_match match = {uport, drv};
+	struct ktermios termios;

 	mutex_lock(&port->mutex);

 	if (!console_suspend_enabled && uart_console(uport)) {
 		/* no need to resume serial console, it wasn't suspended */
+		/*
+		 * First try to use the console cflag setting.
+		 */
+		memset(&termios, 0, sizeof(struct ktermios));
+		termios.c_cflag = uport->cons->cflag;
+		/*
+		 * If that's unset, use the tty termios setting.
+		 */
+		if (termios.c_cflag == 0)
+			termios = *state->port.tty->termios;
+		else {
+			termios.c_ispeed = termios.c_ospeed =
+				tty_termios_input_baud_rate(&termios);
+			termios.c_ispeed = termios.c_ospeed =
+				tty_termios_baud_rate(&termios);
+		}
+		uport->ops->set_termios(uport, &termios, NULL);
 		mutex_unlock(&port->mutex);
 		return 0;
 	}
@@ -2089,20 +2107,6 @@ int uart_resume_port(struct uart_driver *drv,
struct uart_port *uport)
 	 * Re-enable the console device after suspending.
 	 */
 	if (uart_console(uport)) {
-		struct ktermios termios;
-
-		/*
-		 * First try to use the console cflag setting.
-		 */
-		memset(&termios, 0, sizeof(struct ktermios));
-		termios.c_cflag = uport->cons->cflag;
-
-		/*
-		 * If that's unset, use the tty termios setting.
-		 */
-		if (port->tty && termios.c_cflag == 0)
-			termios = *port->tty->termios;
-
 		uart_change_pm(state, 0);
 		uport->ops->set_termios(uport, &termios, NULL);
 		console_start(uport->cons);
-- 
1.5.6.5

Possible suspend/resume regression in .32-rc?

[Haojian Zhuang]

On Sun, Nov 1, 2009 at 6:03 PM, Pavel Machek <pavel@ucw.cz> wrote:
>> Hi!
>>
>> > > Is anyone using suspend/resume with a recent git mainline kernel on PXA
>> > > or other ARM embedded boards? My platform used to suspend and resume
>> > > just fine on 2.6.31 but now as I rebased it, it fails the resume part.
>> > >
>> > > Unfortunately, I can't bisect it as the platform is not mainline yet and
>> > > so I always have mandatory patches (without my platform won't do
>> > > anything) on top of the git repository. Which breaks the bisect logic.
>> > >
>> > > What puzzles me is that I see the current raising at wakeup time, so at
>> > > least the processor seems to resume, but I can't see any serial console
>> > > output, just like if the kernel crashed very early after wakeup.
>> > > 'no_console_suspend' didn't help either.
>> >
>> > Ok, got it. The culprit is commit d2c37068 ("[ARM] pxa: initialize
>> > default interrupt priority and use ICHP for IRQ handling"). Reverting it
>> > make suspend/resume work again on my board.
>> >
>> > Haojian, Eric, could you have a look at this?
>>
>> Okay, patch is this one: I'll test reverting it shortly.
>>
>> commit d2c37068429b29d6549cf3486fc84b836689e122
>> Author: Haojian Zhuang <haojian.zhuang@marvell.com>
>> Date: ? Wed Aug 19 19:49:31 2009 +0800
>>
>> ? ? [ARM] pxa: initialize default interrupt priority and use ICHP for IRQ handling
>>
>> ? ? Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
>> ? ? Signed-off-by: Eric Miao <eric.y.miao@gmail.com>
>
> And yes, reverting it _does_ fix suspend on spitz.
>
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?Pavel

Em, it's not caused by the IRQ patch.

The kernel is blocked in resume path. When console is resumed, IRQ is
already disabled and system is blocked. Actually, IRQ shouldn't be
disabled at here. Up to now, I only find which patch will cause this
issue. But I can't find the best solution on it. The patch with issue
is pasted in below.

So this issue is only occused when console suspend is enabled. If you
enable no_console_suspend in command, you won't meet this issue. It
seems that it's caused by removing termios setting in
uart_resume_port() in the below patch. If I add these code back, the
issue doesn't occur any more.

Deepak,
Could you help to investigate this issue also?

By the way, the suggested quick test command is in below. If we tried
these commands for 20 times, the issue would occur. I only test it in
PXA silicon. I don't know the symptom on other silicons.
$ echo devices > /sys/power/pm_test
$ echo mem > /sys/power/state

Thanks
Haojian

Re: Possible suspend/resume regression in .32-rc?

[Daniel Mack]

On Mon, Nov 02, 2009 at 05:22:30AM -0400, Haojian Zhuang wrote:
> >> > Ok, got it. The culprit is commit d2c37068 ("[ARM] pxa: initialize
> >> > default interrupt priority and use ICHP for IRQ handling"). Reverting it
> >> > make suspend/resume work again on my board.
> >> >
> >> > Haojian, Eric, could you have a look at this?
> >>
> >> Okay, patch is this one: I'll test reverting it shortly.
> >>
> >> commit d2c37068429b29d6549cf3486fc84b836689e122
> >> Author: Haojian Zhuang <haojian.zhuang@marvell.com>
> >> Date:   Wed Aug 19 19:49:31 2009 +0800
> >>
> >>     [ARM] pxa: initialize default interrupt priority and use ICHP for IRQ handling
> >>
> >>     Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
> >>     Signed-off-by: Eric Miao <eric.y.miao@gmail.com>
> >
> > And yes, reverting it _does_ fix suspend on spitz.
> >
> >                                                                        Pavel
> 
> Em, it's not caused by the IRQ patch.
> 
> The kernel is blocked in resume path. When console is resumed, IRQ is
> already disabled and system is blocked. Actually, IRQ shouldn't be
> disabled at here. Up to now, I only find which patch will cause this
> issue. But I can't find the best solution on it. The patch with issue
> is pasted in below.
> 
> So this issue is only occused when console suspend is enabled. If you
> enable no_console_suspend in command, you won't meet this issue. It
> seems that it's caused by removing termios setting in
> uart_resume_port() in the below patch. If I add these code back, the
> issue doesn't occur any more.

Well no, not confirmed. I tested with no_console_suspend, and it still
hit me. And without too, btw.

Daniel

Possible suspend/resume regression in .32-rc?

[Daniel Mack]

On Mon, Nov 02, 2009 at 05:22:30AM -0400, Haojian Zhuang wrote:
> >> > Ok, got it. The culprit is commit d2c37068 ("[ARM] pxa: initialize
> >> > default interrupt priority and use ICHP for IRQ handling"). Reverting it
> >> > make suspend/resume work again on my board.
> >> >
> >> > Haojian, Eric, could you have a look at this?
> >>
> >> Okay, patch is this one: I'll test reverting it shortly.
> >>
> >> commit d2c37068429b29d6549cf3486fc84b836689e122
> >> Author: Haojian Zhuang <haojian.zhuang@marvell.com>
> >> Date: ? Wed Aug 19 19:49:31 2009 +0800
> >>
> >> ? ? [ARM] pxa: initialize default interrupt priority and use ICHP for IRQ handling
> >>
> >> ? ? Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
> >> ? ? Signed-off-by: Eric Miao <eric.y.miao@gmail.com>
> >
> > And yes, reverting it _does_ fix suspend on spitz.
> >
> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?Pavel
> 
> Em, it's not caused by the IRQ patch.
> 
> The kernel is blocked in resume path. When console is resumed, IRQ is
> already disabled and system is blocked. Actually, IRQ shouldn't be
> disabled at here. Up to now, I only find which patch will cause this
> issue. But I can't find the best solution on it. The patch with issue
> is pasted in below.
> 
> So this issue is only occused when console suspend is enabled. If you
> enable no_console_suspend in command, you won't meet this issue. It
> seems that it's caused by removing termios setting in
> uart_resume_port() in the below patch. If I add these code back, the
> issue doesn't occur any more.

Well no, not confirmed. I tested with no_console_suspend, and it still
hit me. And without too, btw.

Daniel

Re: Possible suspend/resume regression in .32-rc?

[Pavel Machek]

On Mon 2009-11-02 05:22:30, Haojian Zhuang wrote:
> On Sun, Nov 1, 2009 at 6:03 PM, Pavel Machek <pavel@ucw.cz> wrote:
> >> Hi!
> >>
> >> > > Is anyone using suspend/resume with a recent git mainline kernel on PXA
> >> > > or other ARM embedded boards? My platform used to suspend and resume
> >> > > just fine on 2.6.31 but now as I rebased it, it fails the resume part.
> >> > >
> >> > > Unfortunately, I can't bisect it as the platform is not mainline yet and
> >> > > so I always have mandatory patches (without my platform won't do
> >> > > anything) on top of the git repository. Which breaks the bisect logic.
> >> > >
> >> > > What puzzles me is that I see the current raising at wakeup time, so at
> >> > > least the processor seems to resume, but I can't see any serial console
> >> > > output, just like if the kernel crashed very early after wakeup.
> >> > > 'no_console_suspend' didn't help either.
> >> >
> >> > Ok, got it. The culprit is commit d2c37068 ("[ARM] pxa: initialize
> >> > default interrupt priority and use ICHP for IRQ handling"). Reverting it
> >> > make suspend/resume work again on my board.
> >> >
> >> > Haojian, Eric, could you have a look at this?
> >>
> >> Okay, patch is this one: I'll test reverting it shortly.
> >>
> >> commit d2c37068429b29d6549cf3486fc84b836689e122
> >> Author: Haojian Zhuang <haojian.zhuang@marvell.com>
> >> Date:   Wed Aug 19 19:49:31 2009 +0800
> >>
> >>     [ARM] pxa: initialize default interrupt priority and use ICHP for IRQ handling
> >>
> >>     Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
> >>     Signed-off-by: Eric Miao <eric.y.miao@gmail.com>
> >
> > And yes, reverting it _does_ fix suspend on spitz.
> 
> Em, it's not caused by the IRQ patch.
> 
> The kernel is blocked in resume path. When console is resumed, IRQ is
> already disabled and system is blocked. Actually, IRQ shouldn't be
> disabled at here. Up to now, I only find which patch will cause this
> issue. But I can't find the best solution on it. The patch with issue
> is pasted in below.
> 
> So this issue is only occused when console suspend is enabled. If you
> enable no_console_suspend in command, you won't meet this issue. It
> seems that it's caused by removing termios setting in
> uart_resume_port() in the below patch. If I add these code back, the
> issue doesn't occur any more.

Given that it hangs very early, in arch_suspend_enable_irqs() (see my
other mail), I don't trust your analysis.

> Deepak,
> Could you help to investigate this issue also?
> 
> By the way, the suggested quick test command is in below. If we tried
> these commands for 20 times, the issue would occur. I only test it in
> PXA silicon. I don't know the symptom on other silicons.
> $ echo devices > /sys/power/pm_test
> $ echo mem > /sys/power/state

I'm not using serial console on spitz, and I have never had successful
resume with the patch applied.

The original patch is also poorly changelogged. What is the advantage
of ICHP and why do we want default priorities?

								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Possible suspend/resume regression in .32-rc?

[Pavel Machek]

On Mon 2009-11-02 05:22:30, Haojian Zhuang wrote:
> On Sun, Nov 1, 2009 at 6:03 PM, Pavel Machek <pavel@ucw.cz> wrote:
> >> Hi!
> >>
> >> > > Is anyone using suspend/resume with a recent git mainline kernel on PXA
> >> > > or other ARM embedded boards? My platform used to suspend and resume
> >> > > just fine on 2.6.31 but now as I rebased it, it fails the resume part.
> >> > >
> >> > > Unfortunately, I can't bisect it as the platform is not mainline yet and
> >> > > so I always have mandatory patches (without my platform won't do
> >> > > anything) on top of the git repository. Which breaks the bisect logic.
> >> > >
> >> > > What puzzles me is that I see the current raising at wakeup time, so at
> >> > > least the processor seems to resume, but I can't see any serial console
> >> > > output, just like if the kernel crashed very early after wakeup.
> >> > > 'no_console_suspend' didn't help either.
> >> >
> >> > Ok, got it. The culprit is commit d2c37068 ("[ARM] pxa: initialize
> >> > default interrupt priority and use ICHP for IRQ handling"). Reverting it
> >> > make suspend/resume work again on my board.
> >> >
> >> > Haojian, Eric, could you have a look at this?
> >>
> >> Okay, patch is this one: I'll test reverting it shortly.
> >>
> >> commit d2c37068429b29d6549cf3486fc84b836689e122
> >> Author: Haojian Zhuang <haojian.zhuang@marvell.com>
> >> Date: ? Wed Aug 19 19:49:31 2009 +0800
> >>
> >> ? ? [ARM] pxa: initialize default interrupt priority and use ICHP for IRQ handling
> >>
> >> ? ? Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
> >> ? ? Signed-off-by: Eric Miao <eric.y.miao@gmail.com>
> >
> > And yes, reverting it _does_ fix suspend on spitz.
> 
> Em, it's not caused by the IRQ patch.
> 
> The kernel is blocked in resume path. When console is resumed, IRQ is
> already disabled and system is blocked. Actually, IRQ shouldn't be
> disabled at here. Up to now, I only find which patch will cause this
> issue. But I can't find the best solution on it. The patch with issue
> is pasted in below.
> 
> So this issue is only occused when console suspend is enabled. If you
> enable no_console_suspend in command, you won't meet this issue. It
> seems that it's caused by removing termios setting in
> uart_resume_port() in the below patch. If I add these code back, the
> issue doesn't occur any more.

Given that it hangs very early, in arch_suspend_enable_irqs() (see my
other mail), I don't trust your analysis.

> Deepak,
> Could you help to investigate this issue also?
> 
> By the way, the suggested quick test command is in below. If we tried
> these commands for 20 times, the issue would occur. I only test it in
> PXA silicon. I don't know the symptom on other silicons.
> $ echo devices > /sys/power/pm_test
> $ echo mem > /sys/power/state

I'm not using serial console on spitz, and I have never had successful
resume with the patch applied.

The original patch is also poorly changelogged. What is the advantage
of ICHP and why do we want default priorities?

								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: Possible suspend/resume regression in .32-rc?

[Eric Miao]

> The original patch is also poorly changelogged. What is the advantage
> of ICHP and why do we want default priorities?
>

That's going to simplify the "get_irqnr_and_base" macro significantly
and make it much easier to adopt pxa950 which has more than 64
IRQs. And besides, it's priority based, so we do want to handle IRQ
numbers from high priority to low from software either.

Possible suspend/resume regression in .32-rc?

[Eric Miao]

> The original patch is also poorly changelogged. What is the advantage
> of ICHP and why do we want default priorities?
>

That's going to simplify the "get_irqnr_and_base" macro significantly
and make it much easier to adopt pxa950 which has more than 64
IRQs. And besides, it's priority based, so we do want to handle IRQ
numbers from high priority to low from software either.

Re: Possible suspend/resume regression in .32-rc?

[Stanislav Brabec]

Haojian Zhuang wrote:

> Commit b5b82df6, from May 2007, breaks no_console_suspend on the OLPC
> XO laptop. Basically what happens is that upon returning from resume,
> serial8250_resume_port() will reconfigure the port for high speed
> mode and all console output will be garbled, making debug of the
> resume path painful. This patch modifies uart_resume_port() to
> reset the port to the state it was in before we suspended.

See my patch waiting for approval in LKML thread
"serial-core: resume serial hardware with no_console_suspend".

It attempts to fix it, but I was not yet able to test it due to spitz
resume breakage.


________________________________________________________________________
Stanislav Brabec
http://www.penguin.cz/~utx

Possible suspend/resume regression in .32-rc?

[Stanislav Brabec]

Haojian Zhuang wrote:

> Commit b5b82df6, from May 2007, breaks no_console_suspend on the OLPC
> XO laptop. Basically what happens is that upon returning from resume,
> serial8250_resume_port() will reconfigure the port for high speed
> mode and all console output will be garbled, making debug of the
> resume path painful. This patch modifies uart_resume_port() to
> reset the port to the state it was in before we suspended.

See my patch waiting for approval in LKML thread
"serial-core: resume serial hardware with no_console_suspend".

It attempts to fix it, but I was not yet able to test it due to spitz
resume breakage.


________________________________________________________________________
Stanislav Brabec
http://www.penguin.cz/~utx

Re: Possible suspend/resume regression in .32-rc?

[Haojian Zhuang]

On Mon, Nov 2, 2009 at 5:38 AM, Pavel Machek <pavel@ucw.cz> wrote:
> On Mon 2009-11-02 05:22:30, Haojian Zhuang wrote:
>> On Sun, Nov 1, 2009 at 6:03 PM, Pavel Machek <pavel@ucw.cz> wrote:
>>
>> Em, it's not caused by the IRQ patch.
>>
>> The kernel is blocked in resume path. When console is resumed, IRQ is
>> already disabled and system is blocked. Actually, IRQ shouldn't be
>> disabled at here. Up to now, I only find which patch will cause this
>> issue. But I can't find the best solution on it. The patch with issue
>> is pasted in below.
>>
>> So this issue is only occused when console suspend is enabled. If you
>> enable no_console_suspend in command, you won't meet this issue. It
>> seems that it's caused by removing termios setting in
>> uart_resume_port() in the below patch. If I add these code back, the
>> issue doesn't occur any more.
>
> Given that it hangs very early, in arch_suspend_enable_irqs() (see my
> other mail), I don't trust your analysis.
>
> I'm not using serial console on spitz, and I have never had successful
> resume with the patch applied.

It seems that we're talking on different issue with similar symptom.
Please check my test method. While I'm testing suspend with devices
level, kernel is blocked in console resume. In this level, it won't
call arch_suspend_enable_irqs(). This function call is only invoked in
processor level or below.

Up to now, I can't reproduce the issue you're talking on my platform
yet. I'll check this issue continuously. I also want to know your
hardware information.

Thanks
Haojian

Possible suspend/resume regression in .32-rc?

[Haojian Zhuang]

On Mon, Nov 2, 2009 at 5:38 AM, Pavel Machek <pavel@ucw.cz> wrote:
> On Mon 2009-11-02 05:22:30, Haojian Zhuang wrote:
>> On Sun, Nov 1, 2009 at 6:03 PM, Pavel Machek <pavel@ucw.cz> wrote:
>>
>> Em, it's not caused by the IRQ patch.
>>
>> The kernel is blocked in resume path. When console is resumed, IRQ is
>> already disabled and system is blocked. Actually, IRQ shouldn't be
>> disabled at here. Up to now, I only find which patch will cause this
>> issue. But I can't find the best solution on it. The patch with issue
>> is pasted in below.
>>
>> So this issue is only occused when console suspend is enabled. If you
>> enable no_console_suspend in command, you won't meet this issue. It
>> seems that it's caused by removing termios setting in
>> uart_resume_port() in the below patch. If I add these code back, the
>> issue doesn't occur any more.
>
> Given that it hangs very early, in arch_suspend_enable_irqs() (see my
> other mail), I don't trust your analysis.
>
> I'm not using serial console on spitz, and I have never had successful
> resume with the patch applied.

It seems that we're talking on different issue with similar symptom.
Please check my test method. While I'm testing suspend with devices
level, kernel is blocked in console resume. In this level, it won't
call arch_suspend_enable_irqs(). This function call is only invoked in
processor level or below.

Up to now, I can't reproduce the issue you're talking on my platform
yet. I'll check this issue continuously. I also want to know your
hardware information.

Thanks
Haojian

Re: Possible suspend/resume regression in .32-rc?

[Pavel Machek]

On Mon 2009-11-02 06:48:28, Haojian Zhuang wrote:
> On Mon, Nov 2, 2009 at 5:38 AM, Pavel Machek <pavel@ucw.cz> wrote:
> > On Mon 2009-11-02 05:22:30, Haojian Zhuang wrote:
> >> On Sun, Nov 1, 2009 at 6:03 PM, Pavel Machek <pavel@ucw.cz> wrote:
> >>
> >> Em, it's not caused by the IRQ patch.
> >>
> >> The kernel is blocked in resume path. When console is resumed, IRQ is
> >> already disabled and system is blocked. Actually, IRQ shouldn't be
> >> disabled at here. Up to now, I only find which patch will cause this
> >> issue. But I can't find the best solution on it. The patch with issue
> >> is pasted in below.
> >>
> >> So this issue is only occused when console suspend is enabled. If you
> >> enable no_console_suspend in command, you won't meet this issue. It
> >> seems that it's caused by removing termios setting in
> >> uart_resume_port() in the below patch. If I add these code back, the
> >> issue doesn't occur any more.
> >
> > Given that it hangs very early, in arch_suspend_enable_irqs() (see my
> > other mail), I don't trust your analysis.
> >
> > I'm not using serial console on spitz, and I have never had successful
> > resume with the patch applied.
> 
> It seems that we're talking on different issue with similar symptom.
> Please check my test method. While I'm testing suspend with devices
> level, kernel is blocked in console resume. In this level, it won't
> call arch_suspend_enable_irqs(). This function call is only invoked in
> processor level or below.

For me, everything but real suspend works. I do _not_ have serial
console for spitz.

> Up to now, I can't reproduce the issue you're talking on my platform
> yet. I'll check this issue continuously. I also want to know your
> hardware information.

Spitz, aka Sharp Zaurus c3000.

Can we get the patch reverted till its fixed, so other development can
continue?
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Possible suspend/resume regression in .32-rc?

[Pavel Machek]

On Mon 2009-11-02 06:48:28, Haojian Zhuang wrote:
> On Mon, Nov 2, 2009 at 5:38 AM, Pavel Machek <pavel@ucw.cz> wrote:
> > On Mon 2009-11-02 05:22:30, Haojian Zhuang wrote:
> >> On Sun, Nov 1, 2009 at 6:03 PM, Pavel Machek <pavel@ucw.cz> wrote:
> >>
> >> Em, it's not caused by the IRQ patch.
> >>
> >> The kernel is blocked in resume path. When console is resumed, IRQ is
> >> already disabled and system is blocked. Actually, IRQ shouldn't be
> >> disabled at here. Up to now, I only find which patch will cause this
> >> issue. But I can't find the best solution on it. The patch with issue
> >> is pasted in below.
> >>
> >> So this issue is only occused when console suspend is enabled. If you
> >> enable no_console_suspend in command, you won't meet this issue. It
> >> seems that it's caused by removing termios setting in
> >> uart_resume_port() in the below patch. If I add these code back, the
> >> issue doesn't occur any more.
> >
> > Given that it hangs very early, in arch_suspend_enable_irqs() (see my
> > other mail), I don't trust your analysis.
> >
> > I'm not using serial console on spitz, and I have never had successful
> > resume with the patch applied.
> 
> It seems that we're talking on different issue with similar symptom.
> Please check my test method. While I'm testing suspend with devices
> level, kernel is blocked in console resume. In this level, it won't
> call arch_suspend_enable_irqs(). This function call is only invoked in
> processor level or below.

For me, everything but real suspend works. I do _not_ have serial
console for spitz.

> Up to now, I can't reproduce the issue you're talking on my platform
> yet. I'll check this issue continuously. I also want to know your
> hardware information.

Spitz, aka Sharp Zaurus c3000.

Can we get the patch reverted till its fixed, so other development can
continue?
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: Possible suspend/resume regression in .32-rc?

[Haojian Zhuang]

On Mon, Nov 2, 2009 at 6:51 AM, Pavel Machek <pavel@ucw.cz> wrote:
> On Mon 2009-11-02 06:48:28, Haojian Zhuang wrote:
>> On Mon, Nov 2, 2009 at 5:38 AM, Pavel Machek <pavel@ucw.cz> wrote:
>> > On Mon 2009-11-02 05:22:30, Haojian Zhuang wrote:
>> >> On Sun, Nov 1, 2009 at 6:03 PM, Pavel Machek <pavel@ucw.cz> wrote:
>> >>
>> >> Em, it's not caused by the IRQ patch.
>> >>
>> >> The kernel is blocked in resume path. When console is resumed, IRQ is
>> >> already disabled and system is blocked. Actually, IRQ shouldn't be
>> >> disabled at here. Up to now, I only find which patch will cause this
>> >> issue. But I can't find the best solution on it. The patch with issue
>> >> is pasted in below.
>> >>
>> >> So this issue is only occused when console suspend is enabled. If you
>> >> enable no_console_suspend in command, you won't meet this issue. It
>> >> seems that it's caused by removing termios setting in
>> >> uart_resume_port() in the below patch. If I add these code back, the
>> >> issue doesn't occur any more.
>> >
>> > Given that it hangs very early, in arch_suspend_enable_irqs() (see my
>> > other mail), I don't trust your analysis.
>> >
>> > I'm not using serial console on spitz, and I have never had successful
>> > resume with the patch applied.
>>
>> It seems that we're talking on different issue with similar symptom.
>> Please check my test method. While I'm testing suspend with devices
>> level, kernel is blocked in console resume. In this level, it won't
>> call arch_suspend_enable_irqs(). This function call is only invoked in
>> processor level or below.
>
> For me, everything but real suspend works. I do _not_ have serial
> console for spitz.
>
>> Up to now, I can't reproduce the issue you're talking on my platform
>> yet. I'll check this issue continuously. I also want to know your
>> hardware information.
>
> Spitz, aka Sharp Zaurus c3000.
>

I see. You can chech my patch in below. I need to save IPRs in suspend
resume routine.

Thanks
Haojian

>From 1764e836424d42a0654b8b73c402a2dddb118dc4 Mon Sep 17 00:00:00 2001
From: Haojian Zhuang <haojian.zhuang@marvell.com>
Date: Mon, 2 Nov 2009 14:02:21 -0500
Subject: [PATCH] pxa: fix system resume issue on pxa27x and pxa3xx

Since interrupt handler is changed to use interrupt priority, we also need to
save and restore these interrupt controller registers in suspend/resume
routine.

Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
---
 arch/arm/mach-pxa/irq.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-pxa/irq.c b/arch/arm/mach-pxa/irq.c
index d694ce2..cdc272b 100644
--- a/arch/arm/mach-pxa/irq.c
+++ b/arch/arm/mach-pxa/irq.c
@@ -150,6 +150,7 @@ void __init pxa_init_irq(int irq_nr, set_wake_t fn)

 #ifdef CONFIG_PM
 static unsigned long saved_icmr[2];
+static unsigned long saved_ipr[128];

 static int pxa_irq_suspend(struct sys_device *dev, pm_message_t state)
 {
@@ -159,6 +160,10 @@ static int pxa_irq_suspend(struct sys_device
*dev, pm_message_t state)
 		saved_icmr[i] = _ICMR(irq);
 		_ICMR(irq) = 0;
 	}
+	if (pxa_internal_irq_nr > 128)
+		BUG();
+	for (i = 0; i < pxa_internal_irq_nr; i++)
+		saved_ipr[i] = IPR(i);

 	return 0;
 }
@@ -171,6 +176,8 @@ static int pxa_irq_resume(struct sys_device *dev)
 		_ICMR(irq) = saved_icmr[i];
 		_ICLR(irq) = 0;
 	}
+	for (i = 0; i < pxa_internal_irq_nr; i++)
+		IPR(i) = saved_ipr[i];

 	ICCR = 1;
 	return 0;
-- 
1.5.6.5

Possible suspend/resume regression in .32-rc?

[Haojian Zhuang]

On Mon, Nov 2, 2009 at 6:51 AM, Pavel Machek <pavel@ucw.cz> wrote:
> On Mon 2009-11-02 06:48:28, Haojian Zhuang wrote:
>> On Mon, Nov 2, 2009 at 5:38 AM, Pavel Machek <pavel@ucw.cz> wrote:
>> > On Mon 2009-11-02 05:22:30, Haojian Zhuang wrote:
>> >> On Sun, Nov 1, 2009 at 6:03 PM, Pavel Machek <pavel@ucw.cz> wrote:
>> >>
>> >> Em, it's not caused by the IRQ patch.
>> >>
>> >> The kernel is blocked in resume path. When console is resumed, IRQ is
>> >> already disabled and system is blocked. Actually, IRQ shouldn't be
>> >> disabled at here. Up to now, I only find which patch will cause this
>> >> issue. But I can't find the best solution on it. The patch with issue
>> >> is pasted in below.
>> >>
>> >> So this issue is only occused when console suspend is enabled. If you
>> >> enable no_console_suspend in command, you won't meet this issue. It
>> >> seems that it's caused by removing termios setting in
>> >> uart_resume_port() in the below patch. If I add these code back, the
>> >> issue doesn't occur any more.
>> >
>> > Given that it hangs very early, in arch_suspend_enable_irqs() (see my
>> > other mail), I don't trust your analysis.
>> >
>> > I'm not using serial console on spitz, and I have never had successful
>> > resume with the patch applied.
>>
>> It seems that we're talking on different issue with similar symptom.
>> Please check my test method. While I'm testing suspend with devices
>> level, kernel is blocked in console resume. In this level, it won't
>> call arch_suspend_enable_irqs(). This function call is only invoked in
>> processor level or below.
>
> For me, everything but real suspend works. I do _not_ have serial
> console for spitz.
>
>> Up to now, I can't reproduce the issue you're talking on my platform
>> yet. I'll check this issue continuously. I also want to know your
>> hardware information.
>
> Spitz, aka Sharp Zaurus c3000.
>

I see. You can chech my patch in below. I need to save IPRs in suspend
resume routine.

Thanks
Haojian

Re: Possible suspend/resume regression in .32-rc?

[Daniel Mack]

On Mon, Nov 02, 2009 at 07:18:02AM -0400, Haojian Zhuang wrote:
> On Mon, Nov 2, 2009 at 6:51 AM, Pavel Machek <pavel@ucw.cz> wrote:
> >>
> >> It seems that we're talking on different issue with similar symptom.
> >> Please check my test method. While I'm testing suspend with devices
> >> level, kernel is blocked in console resume. In this level, it won't
> >> call arch_suspend_enable_irqs(). This function call is only invoked in
> >> processor level or below.
> >
> > For me, everything but real suspend works. I do _not_ have serial
> > console for spitz.
> >
> >> Up to now, I can't reproduce the issue you're talking on my platform
> >> yet. I'll check this issue continuously. I also want to know your
> >> hardware information.
> >
> > Spitz, aka Sharp Zaurus c3000.
> >
> 
> I see. You can chech my patch in below. I need to save IPRs in suspend
> resume routine.

Yep - that fixes it on my platform. Great. Add my 'Tested-by' if you
like.

Thanks,
Daniel

> From 1764e836424d42a0654b8b73c402a2dddb118dc4 Mon Sep 17 00:00:00 2001
> From: Haojian Zhuang <haojian.zhuang@marvell.com>
> Date: Mon, 2 Nov 2009 14:02:21 -0500
> Subject: [PATCH] pxa: fix system resume issue on pxa27x and pxa3xx
> 
> Since interrupt handler is changed to use interrupt priority, we also need to
> save and restore these interrupt controller registers in suspend/resume
> routine.
> 
> Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
> ---
>  arch/arm/mach-pxa/irq.c |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-pxa/irq.c b/arch/arm/mach-pxa/irq.c
> index d694ce2..cdc272b 100644
> --- a/arch/arm/mach-pxa/irq.c
> +++ b/arch/arm/mach-pxa/irq.c
> @@ -150,6 +150,7 @@ void __init pxa_init_irq(int irq_nr, set_wake_t fn)
> 
>  #ifdef CONFIG_PM
>  static unsigned long saved_icmr[2];
> +static unsigned long saved_ipr[128];
> 
>  static int pxa_irq_suspend(struct sys_device *dev, pm_message_t state)
>  {
> @@ -159,6 +160,10 @@ static int pxa_irq_suspend(struct sys_device
> *dev, pm_message_t state)
>  		saved_icmr[i] = _ICMR(irq);
>  		_ICMR(irq) = 0;
>  	}
> +	if (pxa_internal_irq_nr > 128)
> +		BUG();
> +	for (i = 0; i < pxa_internal_irq_nr; i++)
> +		saved_ipr[i] = IPR(i);
> 
>  	return 0;
>  }
> @@ -171,6 +176,8 @@ static int pxa_irq_resume(struct sys_device *dev)
>  		_ICMR(irq) = saved_icmr[i];
>  		_ICLR(irq) = 0;
>  	}
> +	for (i = 0; i < pxa_internal_irq_nr; i++)
> +		IPR(i) = saved_ipr[i];
> 
>  	ICCR = 1;
>  	return 0;
> -- 
> 1.5.6.5

Possible suspend/resume regression in .32-rc?

[Daniel Mack]

On Mon, Nov 02, 2009 at 07:18:02AM -0400, Haojian Zhuang wrote:
> On Mon, Nov 2, 2009 at 6:51 AM, Pavel Machek <pavel@ucw.cz> wrote:
> >>
> >> It seems that we're talking on different issue with similar symptom.
> >> Please check my test method. While I'm testing suspend with devices
> >> level, kernel is blocked in console resume. In this level, it won't
> >> call arch_suspend_enable_irqs(). This function call is only invoked in
> >> processor level or below.
> >
> > For me, everything but real suspend works. I do _not_ have serial
> > console for spitz.
> >
> >> Up to now, I can't reproduce the issue you're talking on my platform
> >> yet. I'll check this issue continuously. I also want to know your
> >> hardware information.
> >
> > Spitz, aka Sharp Zaurus c3000.
> >
> 
> I see. You can chech my patch in below. I need to save IPRs in suspend
> resume routine.

Yep - that fixes it on my platform. Great. Add my 'Tested-by' if you
like.

Thanks,
Daniel

> From 1764e836424d42a0654b8b73c402a2dddb118dc4 Mon Sep 17 00:00:00 2001
> From: Haojian Zhuang <haojian.zhuang@marvell.com>
> Date: Mon, 2 Nov 2009 14:02:21 -0500
> Subject: [PATCH] pxa: fix system resume issue on pxa27x and pxa3xx
> 
> Since interrupt handler is changed to use interrupt priority, we also need to
> save and restore these interrupt controller registers in suspend/resume
> routine.
> 
> Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
> ---
>  arch/arm/mach-pxa/irq.c |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-pxa/irq.c b/arch/arm/mach-pxa/irq.c
> index d694ce2..cdc272b 100644
> --- a/arch/arm/mach-pxa/irq.c
> +++ b/arch/arm/mach-pxa/irq.c
> @@ -150,6 +150,7 @@ void __init pxa_init_irq(int irq_nr, set_wake_t fn)
> 
>  #ifdef CONFIG_PM
>  static unsigned long saved_icmr[2];
> +static unsigned long saved_ipr[128];
> 
>  static int pxa_irq_suspend(struct sys_device *dev, pm_message_t state)
>  {
> @@ -159,6 +160,10 @@ static int pxa_irq_suspend(struct sys_device
> *dev, pm_message_t state)
>  		saved_icmr[i] = _ICMR(irq);
>  		_ICMR(irq) = 0;
>  	}
> +	if (pxa_internal_irq_nr > 128)
> +		BUG();
> +	for (i = 0; i < pxa_internal_irq_nr; i++)
> +		saved_ipr[i] = IPR(i);
> 
>  	return 0;
>  }
> @@ -171,6 +176,8 @@ static int pxa_irq_resume(struct sys_device *dev)
>  		_ICMR(irq) = saved_icmr[i];
>  		_ICLR(irq) = 0;
>  	}
> +	for (i = 0; i < pxa_internal_irq_nr; i++)
> +		IPR(i) = saved_ipr[i];
> 
>  	ICCR = 1;
>  	return 0;
> -- 
> 1.5.6.5

Re: Possible suspend/resume regression in .32-rc?

[Pavel Machek]

> >> > Given that it hangs very early, in arch_suspend_enable_irqs() (see my
> >> > other mail), I don't trust your analysis.
> >> >
> >> > I'm not using serial console on spitz, and I have never had successful
> >> > resume with the patch applied.
...
> I see. You can chech my patch in below. I need to save IPRs in suspend
> resume routine.
...
> >From 1764e836424d42a0654b8b73c402a2dddb118dc4 Mon Sep 17 00:00:00 2001
> From: Haojian Zhuang <haojian.zhuang@marvell.com>
> Date: Mon, 2 Nov 2009 14:02:21 -0500
> Subject: [PATCH] pxa: fix system resume issue on pxa27x and pxa3xx
> 
> Since interrupt handler is changed to use interrupt priority, we also need to
> save and restore these interrupt controller registers in suspend/resume
> routine.
> 
> Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>

Acked-by: Pavel Machek <pavel@ucw.cz>
Tested-by: Pavel Machek <pavel@ucw.cz>

> @@ -150,6 +150,7 @@ void __init pxa_init_irq(int irq_nr, set_wake_t fn)
> 
>  #ifdef CONFIG_PM
>  static unsigned long saved_icmr[2];
> +static unsigned long saved_ipr[128];
> 
>  static int pxa_irq_suspend(struct sys_device *dev, pm_message_t state)
>  {
> @@ -159,6 +160,10 @@ static int pxa_irq_suspend(struct sys_device
> *dev, pm_message_t state)

Note: Your mail client is word-wrapping.

>  		saved_icmr[i] = _ICMR(irq);
>  		_ICMR(irq) = 0;
>  	}
> +	if (pxa_internal_irq_nr > 128)
> +		BUG();

BUG_ON()? WARN_ON() then irq_nr = 128? User is very unlikely to read
the BUG() message at this point...
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Possible suspend/resume regression in .32-rc?

[Pavel Machek]

> >> > Given that it hangs very early, in arch_suspend_enable_irqs() (see my
> >> > other mail), I don't trust your analysis.
> >> >
> >> > I'm not using serial console on spitz, and I have never had successful
> >> > resume with the patch applied.
...
> I see. You can chech my patch in below. I need to save IPRs in suspend
> resume routine.
...
> >From 1764e836424d42a0654b8b73c402a2dddb118dc4 Mon Sep 17 00:00:00 2001
> From: Haojian Zhuang <haojian.zhuang@marvell.com>
> Date: Mon, 2 Nov 2009 14:02:21 -0500
> Subject: [PATCH] pxa: fix system resume issue on pxa27x and pxa3xx
> 
> Since interrupt handler is changed to use interrupt priority, we also need to
> save and restore these interrupt controller registers in suspend/resume
> routine.
> 
> Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>

Acked-by: Pavel Machek <pavel@ucw.cz>
Tested-by: Pavel Machek <pavel@ucw.cz>

> @@ -150,6 +150,7 @@ void __init pxa_init_irq(int irq_nr, set_wake_t fn)
> 
>  #ifdef CONFIG_PM
>  static unsigned long saved_icmr[2];
> +static unsigned long saved_ipr[128];
> 
>  static int pxa_irq_suspend(struct sys_device *dev, pm_message_t state)
>  {
> @@ -159,6 +160,10 @@ static int pxa_irq_suspend(struct sys_device
> *dev, pm_message_t state)

Note: Your mail client is word-wrapping.

>  		saved_icmr[i] = _ICMR(irq);
>  		_ICMR(irq) = 0;
>  	}
> +	if (pxa_internal_irq_nr > 128)
> +		BUG();

BUG_ON()? WARN_ON() then irq_nr = 128? User is very unlikely to read
the BUG() message at this point...
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: Possible suspend/resume regression in .32-rc?

[Haojian Zhuang]

On Mon, Nov 2, 2009 at 5:57 AM, Stanislav Brabec <utx@penguin.cz> wrote:
> Haojian Zhuang wrote:
>
>> Commit b5b82df6, from May 2007, breaks no_console_suspend on the OLPC
>> XO laptop. Basically what happens is that upon returning from resume,
>> serial8250_resume_port() will reconfigure the port for high speed
>> mode and all console output will be garbled, making debug of the
>> resume path painful. This patch modifies uart_resume_port() to
>> reset the port to the state it was in before we suspended.
>
> See my patch waiting for approval in LKML thread
> "serial-core: resume serial hardware with no_console_suspend".
>
> It attempts to fix it, but I was not yet able to test it due to spitz
> resume breakage.
>

Hi Stanislav,

At first, your patch can't be applied into my latest codebase. I have
to update the patch. But the console resume is still blocked after
applied your patch. It works only after I merge some old code back. My
modified patch is in below.

diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
index 1689bda..73f41cc 100644
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -2005,12 +2005,6 @@ int uart_suspend_port(struct uart_driver *drv,
struct uart_port *uport)

 	mutex_lock(&port->mutex);

-	if (!console_suspend_enabled && uart_console(uport)) {
-		/* we're going to avoid suspending serial console */
-		mutex_unlock(&port->mutex);
-		return 0;
-	}
-
 	tty_dev = device_find_child(uport->dev, &match, serial_match_port);
 	if (device_may_wakeup(tty_dev)) {
 		enable_irq_wake(uport->irq);
@@ -2018,20 +2012,22 @@ int uart_suspend_port(struct uart_driver *drv,
struct uart_port *uport)
 		mutex_unlock(&port->mutex);
 		return 0;
 	}
-	uport->suspended = 1;
+	if (console_suspend_enabled || !uart_console(uport))
+		uport->suspended = 1;

 	if (port->flags & ASYNC_INITIALIZED) {
 		const struct uart_ops *ops = uport->ops;
 		int tries;

-		set_bit(ASYNCB_SUSPENDED, &port->flags);
-		clear_bit(ASYNCB_INITIALIZED, &port->flags);
-
-		spin_lock_irq(&uport->lock);
-		ops->stop_tx(uport);
-		ops->set_mctrl(uport, 0);
-		ops->stop_rx(uport);
-		spin_unlock_irq(&uport->lock);
+		if (console_suspend_enabled || !uart_console(uport)) {
+			set_bit(ASYNCB_SUSPENDED, &port->flags);
+			clear_bit(ASYNCB_INITIALIZED, &port->flags);
+			spin_lock_irq(&uport->lock);
+			ops->stop_tx(uport);
+			ops->set_mctrl(uport, 0);
+			ops->stop_rx(uport);
+			spin_unlock_irq(&uport->lock);
+		}

 		/*
 		 * Wait for the transmitter to empty.
@@ -2046,16 +2042,18 @@ int uart_suspend_port(struct uart_driver *drv,
struct uart_port *uport)
 			       drv->dev_name,
 			       drv->tty_driver->name_base + uport->line);

-		ops->shutdown(uport);
+		if (console_suspend_enabled || !uart_console(uport))
+			ops->shutdown(uport);
 	}

 	/*
 	 * Disable the console device before suspending.
 	 */
-	if (uart_console(uport))
+	if (console_suspend_enabled && uart_console(uport))
 		console_stop(uport->cons);

-	uart_change_pm(state, 3);
+	if (console_suspend_enabled || !uart_console(uport))
+		uart_change_pm(state, 3);

 	mutex_unlock(&port->mutex);

@@ -2072,8 +2070,18 @@ int uart_resume_port(struct uart_driver *drv,
struct uart_port *uport)

 	mutex_lock(&port->mutex);

-	if (!console_suspend_enabled && uart_console(uport)) {
-		/* no need to resume serial console, it wasn't suspended */
+	tty_dev = device_find_child(uport->dev, &match, serial_match_port);
+	if (!uport->suspended && device_may_wakeup(tty_dev)) {
+		disable_irq_wake(uport->irq);
+		mutex_unlock(&port->mutex);
+		return 0;
+	}
+	uport->suspended = 0;
+
+	/*
+	 * Re-enable the console device after suspending.
+	 */
+	if (uart_console(uport)) {
 		/*
 		 * First try to use the console cflag setting.
 		 */
@@ -2090,23 +2098,6 @@ int uart_resume_port(struct uart_driver *drv,
struct uart_port *uport)
 			termios.c_ispeed = termios.c_ospeed =
 				tty_termios_baud_rate(&termios);
 		}
-		uport->ops->set_termios(uport, &termios, NULL);
-		mutex_unlock(&port->mutex);
-		return 0;
-	}
-
-	tty_dev = device_find_child(uport->dev, &match, serial_match_port);
-	if (!uport->suspended && device_may_wakeup(tty_dev)) {
-		disable_irq_wake(uport->irq);
-		mutex_unlock(&port->mutex);
-		return 0;
-	}
-	uport->suspended = 0;
-
-	/*
-	 * Re-enable the console device after suspending.
-	 */
-	if (uart_console(uport)) {
 		uart_change_pm(state, 0);
 		uport->ops->set_termios(uport, &termios, NULL);
 		console_start(uport->cons);
@@ -2120,21 +2111,23 @@ int uart_resume_port(struct uart_driver *drv,
struct uart_port *uport)
 		spin_lock_irq(&uport->lock);
 		ops->set_mctrl(uport, 0);
 		spin_unlock_irq(&uport->lock);
-		ret = ops->startup(uport);
-		if (ret == 0) {
-			uart_change_speed(state, NULL);
-			spin_lock_irq(&uport->lock);
-			ops->set_mctrl(uport, uport->mctrl);
-			ops->start_tx(uport);
-			spin_unlock_irq(&uport->lock);
-			set_bit(ASYNCB_INITIALIZED, &port->flags);
-		} else {
-			/*
-			 * Failed to resume - maybe hardware went away?
-			 * Clear the "initialized" flag so we won't try
-			 * to call the low level drivers shutdown method.
-			 */
-			uart_shutdown(state);
+		if (console_suspend_enabled || !uart_console(uport)) {
+			ret = ops->startup(uport);
+			if (ret == 0) {
+				uart_change_speed(state, NULL);
+				spin_lock_irq(&uport->lock);
+				ops->set_mctrl(uport, uport->mctrl);
+				ops->start_tx(uport);
+				spin_unlock_irq(&uport->lock);
+				set_bit(ASYNCB_INITIALIZED, &port->flags);
+			} else {
+				/*
+				 * Failed to resume - maybe hardware went away?
+				 * Clear the "initialized" flag so we won't try
+				 * to call the low level drivers shutdown method.
+				 */
+				uart_shutdown(state);
+			}
 		}

 		clear_bit(ASYNCB_SUSPENDED, &port->flags);

Possible suspend/resume regression in .32-rc?

[Haojian Zhuang]

On Mon, Nov 2, 2009 at 5:57 AM, Stanislav Brabec <utx@penguin.cz> wrote:
> Haojian Zhuang wrote:
>
>> Commit b5b82df6, from May 2007, breaks no_console_suspend on the OLPC
>> XO laptop. Basically what happens is that upon returning from resume,
>> serial8250_resume_port() will reconfigure the port for high speed
>> mode and all console output will be garbled, making debug of the
>> resume path painful. This patch modifies uart_resume_port() to
>> reset the port to the state it was in before we suspended.
>
> See my patch waiting for approval in LKML thread
> "serial-core: resume serial hardware with no_console_suspend".
>
> It attempts to fix it, but I was not yet able to test it due to spitz
> resume breakage.
>

Hi Stanislav,

At first, your patch can't be applied into my latest codebase. I have
to update the patch. But the console resume is still blocked after
applied your patch. It works only after I merge some old code back. My
modified patch is in below.

diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
index 1689bda..73f41cc 100644
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -2005,12 +2005,6 @@ int uart_suspend_port(struct uart_driver *drv,
struct uart_port *uport)

 	mutex_lock(&port->mutex);

-	if (!console_suspend_enabled && uart_console(uport)) {
-		/* we're going to avoid suspending serial console */
-		mutex_unlock(&port->mutex);
-		return 0;
-	}
-
 	tty_dev = device_find_child(uport->dev, &match, serial_match_port);
 	if (device_may_wakeup(tty_dev)) {
 		enable_irq_wake(uport->irq);
@@ -2018,20 +2012,22 @@ int uart_suspend_port(struct uart_driver *drv,
struct uart_port *uport)
 		mutex_unlock(&port->mutex);
 		return 0;
 	}
-	uport->suspended = 1;
+	if (console_suspend_enabled || !uart_console(uport))
+		uport->suspended = 1;

 	if (port->flags & ASYNC_INITIALIZED) {
 		const struct uart_ops *ops = uport->ops;
 		int tries;

-		set_bit(ASYNCB_SUSPENDED, &port->flags);
-		clear_bit(ASYNCB_INITIALIZED, &port->flags);
-
-		spin_lock_irq(&uport->lock);
-		ops->stop_tx(uport);
-		ops->set_mctrl(uport, 0);
-		ops->stop_rx(uport);
-		spin_unlock_irq(&uport->lock);
+		if (console_suspend_enabled || !uart_console(uport)) {
+			set_bit(ASYNCB_SUSPENDED, &port->flags);
+			clear_bit(ASYNCB_INITIALIZED, &port->flags);
+			spin_lock_irq(&uport->lock);
+			ops->stop_tx(uport);
+			ops->set_mctrl(uport, 0);
+			ops->stop_rx(uport);
+			spin_unlock_irq(&uport->lock);
+		}

 		/*
 		 * Wait for the transmitter to empty.
@@ -2046,16 +2042,18 @@ int uart_suspend_port(struct uart_driver *drv,
struct uart_port *uport)
 			       drv->dev_name,
 			       drv->tty_driver->name_base + uport->line);

-		ops->shutdown(uport);
+		if (console_suspend_enabled || !uart_console(uport))
+			ops->shutdown(uport);
 	}

 	/*
 	 * Disable the console device before suspending.
 	 */
-	if (uart_console(uport))
+	if (console_suspend_enabled && uart_console(uport))
 		console_stop(uport->cons);

-	uart_change_pm(state, 3);
+	if (console_suspend_enabled || !uart_console(uport))
+		uart_change_pm(state, 3);

 	mutex_unlock(&port->mutex);

@@ -2072,8 +2070,18 @@ int uart_resume_port(struct uart_driver *drv,
struct uart_port *uport)

 	mutex_lock(&port->mutex);

-	if (!console_suspend_enabled && uart_console(uport)) {
-		/* no need to resume serial console, it wasn't suspended */
+	tty_dev = device_find_child(uport->dev, &match, serial_match_port);
+	if (!uport->suspended && device_may_wakeup(tty_dev)) {
+		disable_irq_wake(uport->irq);
+		mutex_unlock(&port->mutex);
+		return 0;
+	}
+	uport->suspended = 0;
+
+	/*
+	 * Re-enable the console device after suspending.
+	 */
+	if (uart_console(uport)) {
 		/*
 		 * First try to use the console cflag setting.
 		 */
@@ -2090,23 +2098,6 @@ int uart_resume_port(struct uart_driver *drv,
struct uart_port *uport)
 			termios.c_ispeed = termios.c_ospeed =
 				tty_termios_baud_rate(&termios);
 		}
-		uport->ops->set_termios(uport, &termios, NULL);
-		mutex_unlock(&port->mutex);
-		return 0;
-	}
-
-	tty_dev = device_find_child(uport->dev, &match, serial_match_port);
-	if (!uport->suspended && device_may_wakeup(tty_dev)) {
-		disable_irq_wake(uport->irq);
-		mutex_unlock(&port->mutex);
-		return 0;
-	}
-	uport->suspended = 0;
-
-	/*
-	 * Re-enable the console device after suspending.
-	 */
-	if (uart_console(uport)) {
 		uart_change_pm(state, 0);
 		uport->ops->set_termios(uport, &termios, NULL);
 		console_start(uport->cons);
@@ -2120,21 +2111,23 @@ int uart_resume_port(struct uart_driver *drv,
struct uart_port *uport)
 		spin_lock_irq(&uport->lock);
 		ops->set_mctrl(uport, 0);
 		spin_unlock_irq(&uport->lock);
-		ret = ops->startup(uport);
-		if (ret == 0) {
-			uart_change_speed(state, NULL);
-			spin_lock_irq(&uport->lock);
-			ops->set_mctrl(uport, uport->mctrl);
-			ops->start_tx(uport);
-			spin_unlock_irq(&uport->lock);
-			set_bit(ASYNCB_INITIALIZED, &port->flags);
-		} else {
-			/*
-			 * Failed to resume - maybe hardware went away?
-			 * Clear the "initialized" flag so we won't try
-			 * to call the low level drivers shutdown method.
-			 */
-			uart_shutdown(state);
+		if (console_suspend_enabled || !uart_console(uport)) {
+			ret = ops->startup(uport);
+			if (ret == 0) {
+				uart_change_speed(state, NULL);
+				spin_lock_irq(&uport->lock);
+				ops->set_mctrl(uport, uport->mctrl);
+				ops->start_tx(uport);
+				spin_unlock_irq(&uport->lock);
+				set_bit(ASYNCB_INITIALIZED, &port->flags);
+			} else {
+				/*
+				 * Failed to resume - maybe hardware went away?
+				 * Clear the "initialized" flag so we won't try
+				 * to call the low level drivers shutdown method.
+				 */
+				uart_shutdown(state);
+			}
 		}

 		clear_bit(ASYNCB_SUSPENDED, &port->flags);

Re: Possible suspend/resume regression in .32-rc?

[Stanislav Brabec]

Haojian Zhuang wrote:
> On Mon, Nov 2, 2009 at 5:57 AM, Stanislav Brabec <utx@penguin.cz> wrote:
> > Haojian Zhuang wrote:
> >
> >> Commit b5b82df6, from May 2007, breaks no_console_suspend on the OLPC
> >> XO laptop. Basically what happens is that upon returning from resume,
> >> serial8250_resume_port() will reconfigure the port for high speed
> >> mode and all console output will be garbled, making debug of the
> >> resume path painful. This patch modifies uart_resume_port() to
> >> reset the port to the state it was in before we suspended.
> >
> > See my patch waiting for approval in LKML thread
> > "serial-core: resume serial hardware with no_console_suspend".
> >
> > It attempts to fix it, but I was not yet able to test it due to spitz
> > resume breakage.
> >
> 
> Hi Stanislav,
> 
> At first, your patch can't be applied into my latest codebase. I have
> to update the patch. But the console resume is still blocked after
> applied your patch. It works only after I merge some old code back. My
> modified patch is in below.

Yes, code changed there a bit. I sent rebased patch later in the thread.
Did you try this one? (Well I did not test it yet - in time of sending
resume did not work at all on my Zaurus.)

Your patch is a bit different. I'll test both as soon as possible.


________________________________________________________________________
Stanislav Brabec
http://www.penguin.cz/~utx

Possible suspend/resume regression in .32-rc?

[Stanislav Brabec]

Haojian Zhuang wrote:
> On Mon, Nov 2, 2009 at 5:57 AM, Stanislav Brabec <utx@penguin.cz> wrote:
> > Haojian Zhuang wrote:
> >
> >> Commit b5b82df6, from May 2007, breaks no_console_suspend on the OLPC
> >> XO laptop. Basically what happens is that upon returning from resume,
> >> serial8250_resume_port() will reconfigure the port for high speed
> >> mode and all console output will be garbled, making debug of the
> >> resume path painful. This patch modifies uart_resume_port() to
> >> reset the port to the state it was in before we suspended.
> >
> > See my patch waiting for approval in LKML thread
> > "serial-core: resume serial hardware with no_console_suspend".
> >
> > It attempts to fix it, but I was not yet able to test it due to spitz
> > resume breakage.
> >
> 
> Hi Stanislav,
> 
> At first, your patch can't be applied into my latest codebase. I have
> to update the patch. But the console resume is still blocked after
> applied your patch. It works only after I merge some old code back. My
> modified patch is in below.

Yes, code changed there a bit. I sent rebased patch later in the thread.
Did you try this one? (Well I did not test it yet - in time of sending
resume did not work at all on my Zaurus.)

Your patch is a bit different. I'll test both as soon as possible.


________________________________________________________________________
Stanislav Brabec
http://www.penguin.cz/~utx

Re: Possible suspend/resume regression in .32-rc?

[Haojian Zhuang]

On Tue, Nov 3, 2009 at 4:50 AM, Stanislav Brabec <utx@penguin.cz> wrote:
> Haojian Zhuang wrote:
>> On Mon, Nov 2, 2009 at 5:57 AM, Stanislav Brabec <utx@penguin.cz> wrote:
>> > Haojian Zhuang wrote:
>> >
>> >> Commit b5b82df6, from May 2007, breaks no_console_suspend on the OLPC
>> >> XO laptop. Basically what happens is that upon returning from resume,
>> >> serial8250_resume_port() will reconfigure the port for high speed
>> >> mode and all console output will be garbled, making debug of the
>> >> resume path painful. This patch modifies uart_resume_port() to
>> >> reset the port to the state it was in before we suspended.
>> >
>> > See my patch waiting for approval in LKML thread
>> > "serial-core: resume serial hardware with no_console_suspend".
>> >
>> > It attempts to fix it, but I was not yet able to test it due to spitz
>> > resume breakage.
>> >
>>
>> Hi Stanislav,
>>
>> At first, your patch can't be applied into my latest codebase. I have
>> to update the patch. But the console resume is still blocked after
>> applied your patch. It works only after I merge some old code back. My
>> modified patch is in below.
>
> Yes, code changed there a bit. I sent rebased patch later in the thread.
> Did you try this one? (Well I did not test it yet - in time of sending
> resume did not work at all on my Zaurus.)
>
> Your patch is a bit different. I'll test both as soon as possible.
>
>

Your latest patch works well in my platform. Thanks a lot.

Possible suspend/resume regression in .32-rc?

[Haojian Zhuang]

On Tue, Nov 3, 2009 at 4:50 AM, Stanislav Brabec <utx@penguin.cz> wrote:
> Haojian Zhuang wrote:
>> On Mon, Nov 2, 2009 at 5:57 AM, Stanislav Brabec <utx@penguin.cz> wrote:
>> > Haojian Zhuang wrote:
>> >
>> >> Commit b5b82df6, from May 2007, breaks no_console_suspend on the OLPC
>> >> XO laptop. Basically what happens is that upon returning from resume,
>> >> serial8250_resume_port() will reconfigure the port for high speed
>> >> mode and all console output will be garbled, making debug of the
>> >> resume path painful. This patch modifies uart_resume_port() to
>> >> reset the port to the state it was in before we suspended.
>> >
>> > See my patch waiting for approval in LKML thread
>> > "serial-core: resume serial hardware with no_console_suspend".
>> >
>> > It attempts to fix it, but I was not yet able to test it due to spitz
>> > resume breakage.
>> >
>>
>> Hi Stanislav,
>>
>> At first, your patch can't be applied into my latest codebase. I have
>> to update the patch. But the console resume is still blocked after
>> applied your patch. It works only after I merge some old code back. My
>> modified patch is in below.
>
> Yes, code changed there a bit. I sent rebased patch later in the thread.
> Did you try this one? (Well I did not test it yet - in time of sending
> resume did not work at all on my Zaurus.)
>
> Your patch is a bit different. I'll test both as soon as possible.
>
>

Your latest patch works well in my platform. Thanks a lot.

Re: Possible suspend/resume regression in .32-rc?

[Daniel Mack]

On Thu, Nov 05, 2009 at 12:06:13AM -0500, Haojian Zhuang wrote:
> On Tue, Nov 3, 2009 at 4:50 AM, Stanislav Brabec <utx@penguin.cz> wrote:
> > Haojian Zhuang wrote:
> >> On Mon, Nov 2, 2009 at 5:57 AM, Stanislav Brabec <utx@penguin.cz> wrote:
> >> > Haojian Zhuang wrote:
> >> >
> >> >> Commit b5b82df6, from May 2007, breaks no_console_suspend on the OLPC
> >> >> XO laptop. Basically what happens is that upon returning from resume,
> >> >> serial8250_resume_port() will reconfigure the port for high speed
> >> >> mode and all console output will be garbled, making debug of the
> >> >> resume path painful. This patch modifies uart_resume_port() to
> >> >> reset the port to the state it was in before we suspended.
> >> >
> >> > See my patch waiting for approval in LKML thread
> >> > "serial-core: resume serial hardware with no_console_suspend".
> >> >
> >> > It attempts to fix it, but I was not yet able to test it due to spitz
> >> > resume breakage.
> >> >
> >>
> >> Hi Stanislav,
> >>
> >> At first, your patch can't be applied into my latest codebase. I have
> >> to update the patch. But the console resume is still blocked after
> >> applied your patch. It works only after I merge some old code back. My
> >> modified patch is in below.
> >
> > Yes, code changed there a bit. I sent rebased patch later in the thread.
> > Did you try this one? (Well I did not test it yet - in time of sending
> > resume did not work at all on my Zaurus.)
> >
> > Your patch is a bit different. I'll test both as soon as possible.
> >
> >
> 
> Your latest patch works well in my platform. Thanks a lot.

It seems Stanislav's patch is still unmerged in the current git HEAD.
Any idea why it is held back? I have it applied locally since quite a
while here, and it works well. So if it helps, take my Tested-by: for
it.

Daniel

Possible suspend/resume regression in .32-rc?

[Daniel Mack]

On Thu, Nov 05, 2009 at 12:06:13AM -0500, Haojian Zhuang wrote:
> On Tue, Nov 3, 2009 at 4:50 AM, Stanislav Brabec <utx@penguin.cz> wrote:
> > Haojian Zhuang wrote:
> >> On Mon, Nov 2, 2009 at 5:57 AM, Stanislav Brabec <utx@penguin.cz> wrote:
> >> > Haojian Zhuang wrote:
> >> >
> >> >> Commit b5b82df6, from May 2007, breaks no_console_suspend on the OLPC
> >> >> XO laptop. Basically what happens is that upon returning from resume,
> >> >> serial8250_resume_port() will reconfigure the port for high speed
> >> >> mode and all console output will be garbled, making debug of the
> >> >> resume path painful. This patch modifies uart_resume_port() to
> >> >> reset the port to the state it was in before we suspended.
> >> >
> >> > See my patch waiting for approval in LKML thread
> >> > "serial-core: resume serial hardware with no_console_suspend".
> >> >
> >> > It attempts to fix it, but I was not yet able to test it due to spitz
> >> > resume breakage.
> >> >
> >>
> >> Hi Stanislav,
> >>
> >> At first, your patch can't be applied into my latest codebase. I have
> >> to update the patch. But the console resume is still blocked after
> >> applied your patch. It works only after I merge some old code back. My
> >> modified patch is in below.
> >
> > Yes, code changed there a bit. I sent rebased patch later in the thread.
> > Did you try this one? (Well I did not test it yet - in time of sending
> > resume did not work at all on my Zaurus.)
> >
> > Your patch is a bit different. I'll test both as soon as possible.
> >
> >
> 
> Your latest patch works well in my platform. Thanks a lot.

It seems Stanislav's patch is still unmerged in the current git HEAD.
Any idea why it is held back? I have it applied locally since quite a
while here, and it works well. So if it helps, take my Tested-by: for
it.

Daniel

Re: Possible suspend/resume regression in .32-rc?

[Stanislav Brabec]

Daniel Mack wrote:

> It seems Stanislav's patch is still unmerged in the current git HEAD.
> Any idea why it is held back? I have it applied locally since quite a
> while here, and it works well. So if it helps, take my Tested-by: for
> it.

It seems to be on the right way:

From: gregkh@suse.de
Subject: patch serial-core-resume-serial-hardware-with-no_console_suspend.patch added to gregkh-2.6 tree
Date: Tue, 22 Dec 2009 12:22:35 -0800

This is a note to let you know that I've just added the patch titled

    Subject: serial-core: resume serial hardware with no_console_suspend

to my gregkh-2.6 tree.  Its filename is

    serial-core-resume-serial-hardware-with-no_console_suspend.patch

This tree can be found at 

http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/patches/


-- 
Best Regards / S pozdravem,

Stanislav Brabec
software developer
---------------------------------------------------------------------
SUSE LINUX, s. r. o.                          e-mail: sbrabec@suse.cz
Lihovarská 1060/12           tel: +420 284 028 966, +49 911 740538747
190 00 Praha 9                                  fax: +420 284 028 951
Czech Republic                                    http://www.suse.cz/

Possible suspend/resume regression in .32-rc?

[Stanislav Brabec]

Daniel Mack wrote:

> It seems Stanislav's patch is still unmerged in the current git HEAD.
> Any idea why it is held back? I have it applied locally since quite a
> while here, and it works well. So if it helps, take my Tested-by: for
> it.

It seems to be on the right way:

From: gregkh@suse.de
Subject: patch serial-core-resume-serial-hardware-with-no_console_suspend.patch added to gregkh-2.6 tree
Date: Tue, 22 Dec 2009 12:22:35 -0800

This is a note to let you know that I've just added the patch titled

    Subject: serial-core: resume serial hardware with no_console_suspend

to my gregkh-2.6 tree.  Its filename is

    serial-core-resume-serial-hardware-with-no_console_suspend.patch

This tree can be found at 

http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/patches/


-- 
Best Regards / S pozdravem,

Stanislav Brabec
software developer
---------------------------------------------------------------------
SUSE LINUX, s. r. o.                          e-mail: sbrabec at suse.cz
Lihovarsk? 1060/12           tel: +420 284 028 966, +49 911 740538747
190 00 Praha 9                                  fax: +420 284 028 951
Czech Republic                                    http://www.suse.cz/

Re: Possible suspend/resume regression in .32-rc?

[Greg KH]

On Wed, Jan 13, 2010 at 02:42:41PM +0100, Stanislav Brabec wrote:
> Daniel Mack wrote:
> 
> > It seems Stanislav's patch is still unmerged in the current git HEAD.
> > Any idea why it is held back? I have it applied locally since quite a
> > while here, and it works well. So if it helps, take my Tested-by: for
> > it.
> 
> It seems to be on the right way:
> 
> From: gregkh@suse.de
> Subject: patch serial-core-resume-serial-hardware-with-no_console_suspend.patch added to gregkh-2.6 tree
> Date: Tue, 22 Dec 2009 12:22:35 -0800
> 
> This is a note to let you know that I've just added the patch titled
> 
>     Subject: serial-core: resume serial hardware with no_console_suspend
> 
> to my gregkh-2.6 tree.  Its filename is
> 
>     serial-core-resume-serial-hardware-with-no_console_suspend.patch
> 
> This tree can be found at 
> 
> http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/patches/

Ah, I missed that this was fixing a regression, so I'll move it to my
"apply before .32 is out" queue.  Sorry about that.

greg k-h

Possible suspend/resume regression in .32-rc?

[Greg KH]

On Wed, Jan 13, 2010 at 02:42:41PM +0100, Stanislav Brabec wrote:
> Daniel Mack wrote:
> 
> > It seems Stanislav's patch is still unmerged in the current git HEAD.
> > Any idea why it is held back? I have it applied locally since quite a
> > while here, and it works well. So if it helps, take my Tested-by: for
> > it.
> 
> It seems to be on the right way:
> 
> From: gregkh at suse.de
> Subject: patch serial-core-resume-serial-hardware-with-no_console_suspend.patch added to gregkh-2.6 tree
> Date: Tue, 22 Dec 2009 12:22:35 -0800
> 
> This is a note to let you know that I've just added the patch titled
> 
>     Subject: serial-core: resume serial hardware with no_console_suspend
> 
> to my gregkh-2.6 tree.  Its filename is
> 
>     serial-core-resume-serial-hardware-with-no_console_suspend.patch
> 
> This tree can be found at 
> 
> http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/patches/

Ah, I missed that this was fixing a regression, so I'll move it to my
"apply before .32 is out" queue.  Sorry about that.

greg k-h