linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ppc32: fix CONFIG_TASK_SIZE handling on 40x
@ 2005-05-18 17:09 Eugene Surovegin
  2005-05-19 17:59 ` [PATCH] ppc32: fix cpm_uart_int() missing interrupts Guillaume Autran
  0 siblings, 1 reply; 9+ messages in thread
From: Eugene Surovegin @ 2005-05-18 17:09 UTC (permalink / raw)
  To: linuxppc-embedded

Hi!

This patch is virtually identical to my previous 44x one. It removes 
0x8000'0000 TASK_SIZE hardcoded assumption from head_4xx.S.

I don't have any 40x board which runs 2.6, so this one is untested, 
bit it compiles :).

Signed-off-by: Eugene Surovegin <ebs@ebshome.net>

Index: arch/ppc/kernel/head_4xx.S
===================================================================
--- 59c3218467807e1793fb4fc5d90141e072ab2212/arch/ppc/kernel/head_4xx.S  (mode:100644)
+++ uncommitted/arch/ppc/kernel/head_4xx.S  (mode:100644)
@@ -291,8 +291,9 @@
 	/* If we are faulting a kernel address, we have to use the
 	 * kernel page tables.
 	 */
-	andis.	r11, r10, 0x8000
-	beq	3f
+	lis	r11, TASK_SIZE@h 
+	cmplw	r10, r11
+	blt+	3f
 	lis	r11, swapper_pg_dir@h
 	ori	r11, r11, swapper_pg_dir@l
 	li	r9, 0
@@ -479,8 +480,9 @@
 	/* If we are faulting a kernel address, we have to use the
 	 * kernel page tables.
 	 */
-	andis.	r11, r10, 0x8000
-	beq	3f
+	lis	r11, TASK_SIZE@h 
+	cmplw	r10, r11
+	blt+	3f
 	lis	r11, swapper_pg_dir@h
 	ori	r11, r11, swapper_pg_dir@l
 	li	r9, 0
@@ -578,8 +580,9 @@
 	/* If we are faulting a kernel address, we have to use the
 	 * kernel page tables.
 	 */
-	andis.	r11, r10, 0x8000
-	beq	3f
+	lis	r11, TASK_SIZE@h 
+	cmplw	r10, r11
+	blt+	3f
 	lis	r11, swapper_pg_dir@h
 	ori	r11, r11, swapper_pg_dir@l
 	li	r9, 0

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

* Re: [PATCH] ppc32: fix cpm_uart_int() missing interrupts
  2005-05-18 17:09 [PATCH] ppc32: fix CONFIG_TASK_SIZE handling on 40x Eugene Surovegin
@ 2005-05-19 17:59 ` Guillaume Autran
  2005-05-19 18:11   ` Dan Malek
  0 siblings, 1 reply; 9+ messages in thread
From: Guillaume Autran @ 2005-05-19 17:59 UTC (permalink / raw)
  To: linuxppc-embedded


[-- Attachment #1.1: Type: text/plain, Size: 1189 bytes --]

This is a patch to fix a problem that occurs at high baud rates in the 
cpm uart interrupt handling.

In cpm_uart_int(), the existing code reads the event register, processes 
the events, and then clears bits in the event register before returning. 
The problem here is that sometimes event processing generates new events 
quite quickly (i.e. at higher baud rates, the transmit interrupt handler 
puts another character into an SCC's transmit buffer and the SCC clears 
the READY bit almost immediately). In this case, the second interrupt 
can be missed because the scc_scce event register gets cleared after 
processing the first. The port can get hung.

The fix adds a while loop. It reads the event register saving its value 
in the local variable 'events' (as before) then clears the event 
register in the device immediately. It processes the events and tests 
the event register again handling new events that might get generated 
during handling.

Any comment ?

Guillaume.

-- 

=======================================
Guillaume Autran
Senior Software Engineer
MRV Communications, Inc.
Tel: (978) 952-4932 office
E-mail: gautran@mrv.com
======================================= 


[-- Attachment #1.2: Type: text/html, Size: 1782 bytes --]

[-- Attachment #2: cpm_uart_core.patch --]
[-- Type: text/plain, Size: 1407 bytes --]

diff -Nru linux-2.6.12-rc4.org/drivers/serial/cpm_uart/cpm_uart_core.c linux-2.6.12-rc4.new/drivers/serial/cpm_uart/cpm_uart_core.c
--- linux-2.6.12-rc4.org/drivers/serial/cpm_uart/cpm_uart_core.c	2005-05-07 01:20:31.000000000 -0400
+++ linux-2.6.12-rc4.new/drivers/serial/cpm_uart/cpm_uart_core.c	2005-05-19 13:48:58.000000000 -0400
@@ -335,23 +335,25 @@
 	pr_debug("CPM uart[%d]:IRQ\n", port->line);
 
 	if (IS_SMC(pinfo)) {
-		events = smcp->smc_smce;
+	    while ((events = smcp->smc_smce) & (SMCM_BRKE | SMCM_RX | SMCM_TX)) {
+		smcp->smc_smce = events;
 		if (events & SMCM_BRKE)
-			uart_handle_break(port);
+		    uart_handle_break(port);
 		if (events & SMCM_RX)
-			cpm_uart_int_rx(port, regs);
+		    cpm_uart_int_rx(port, regs);
 		if (events & SMCM_TX)
-			cpm_uart_int_tx(port, regs);
-		smcp->smc_smce = events;
+		    cpm_uart_int_tx(port, regs);
+	    }
 	} else {
-		events = sccp->scc_scce;
+	    while ((events = sccp->scc_scce) & (UART_SCCM_BRKE | UART_SCCM_RX | UART_SCCM_TX)) {
+		sccp->scc_scce = events;
 		if (events & UART_SCCM_BRKE)
-			uart_handle_break(port);
+		    uart_handle_break(port);
 		if (events & UART_SCCM_RX)
-			cpm_uart_int_rx(port, regs);
+		    cpm_uart_int_rx(port, regs);
 		if (events & UART_SCCM_TX)
-			cpm_uart_int_tx(port, regs);
-		sccp->scc_scce = events;
+		    cpm_uart_int_tx(port, regs);
+	    }
 	}
 	return (events) ? IRQ_HANDLED : IRQ_NONE;
 }

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

* Re: [PATCH] ppc32: fix cpm_uart_int() missing interrupts
  2005-05-19 17:59 ` [PATCH] ppc32: fix cpm_uart_int() missing interrupts Guillaume Autran
@ 2005-05-19 18:11   ` Dan Malek
  2005-05-19 19:34     ` Guillaume Autran
  0 siblings, 1 reply; 9+ messages in thread
From: Dan Malek @ 2005-05-19 18:11 UTC (permalink / raw)
  To: Guillaume Autran; +Cc: linuxppc-embedded


On May 19, 2005, at 1:59 PM, Guillaume Autran wrote:

> Any comment ?

The idea is fine.  Read Documentation/CodingStyle and try again .....

Thanks.


	-- Dan

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

* Re: [PATCH] ppc32: fix cpm_uart_int() missing interrupts
  2005-05-19 18:11   ` Dan Malek
@ 2005-05-19 19:34     ` Guillaume Autran
  2005-05-19 19:54       ` Dan Malek
  0 siblings, 1 reply; 9+ messages in thread
From: Guillaume Autran @ 2005-05-19 19:34 UTC (permalink / raw)
  To: Dan Malek; +Cc: linuxppc-embedded

[-- Attachment #1: Type: text/plain, Size: 432 bytes --]

Is it better like this ?

Dan Malek wrote:

>
> On May 19, 2005, at 1:59 PM, Guillaume Autran wrote:
>
>> Any comment ?
>
>
> The idea is fine.  Read Documentation/CodingStyle and try again .....
>
> Thanks.
>
>
>     -- Dan
>

-- 
=======================================
Guillaume Autran
Senior Software Engineer
MRV Communications, Inc.
Tel: (978) 952-4932 office
E-mail: gautran@mrv.com
======================================= 


[-- Attachment #2: cpm_uart_core.patch --]
[-- Type: text/plain, Size: 1560 bytes --]

diff -Nru linux-2.6.12-rc4.org/drivers/serial/cpm_uart/cpm_uart_core.c linux-2.6.12-rc4.new/drivers/serial/cpm_uart/cpm_uart_core.c
--- linux-2.6.12-rc4.org/drivers/serial/cpm_uart/cpm_uart_core.c	2005-05-07 01:20:31.000000000 -0400
+++ linux-2.6.12-rc4.new/drivers/serial/cpm_uart/cpm_uart_core.c	2005-05-19 15:31:03.000000000 -0400
@@ -335,23 +335,25 @@
 	pr_debug("CPM uart[%d]:IRQ\n", port->line);
 
 	if (IS_SMC(pinfo)) {
-		events = smcp->smc_smce;
-		if (events & SMCM_BRKE)
-			uart_handle_break(port);
-		if (events & SMCM_RX)
-			cpm_uart_int_rx(port, regs);
-		if (events & SMCM_TX)
-			cpm_uart_int_tx(port, regs);
-		smcp->smc_smce = events;
+		while ((events = smcp->smc_smce) & (SMCM_BRKE | SMCM_RX | SMCM_TX)) {
+			smcp->smc_smce = events;
+			if (events & SMCM_BRKE)
+				uart_handle_break(port);
+			if (events & SMCM_RX)
+				cpm_uart_int_rx(port, regs);
+			if (events & SMCM_TX)
+				cpm_uart_int_tx(port, regs);
+		}
 	} else {
-		events = sccp->scc_scce;
-		if (events & UART_SCCM_BRKE)
-			uart_handle_break(port);
-		if (events & UART_SCCM_RX)
-			cpm_uart_int_rx(port, regs);
-		if (events & UART_SCCM_TX)
-			cpm_uart_int_tx(port, regs);
-		sccp->scc_scce = events;
+		while ((events = sccp->scc_scce) & (UART_SCCM_BRKE | UART_SCCM_RX | UART_SCCM_TX)) {
+			sccp->scc_scce = events;
+			if (events & UART_SCCM_BRKE)
+				uart_handle_break(port);
+			if (events & UART_SCCM_RX)
+				cpm_uart_int_rx(port, regs);
+			if (events & UART_SCCM_TX)
+				cpm_uart_int_tx(port, regs);
+		}
 	}
 	return (events) ? IRQ_HANDLED : IRQ_NONE;
 }

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

* Re: [PATCH] ppc32: fix cpm_uart_int() missing interrupts
  2005-05-19 19:34     ` Guillaume Autran
@ 2005-05-19 19:54       ` Dan Malek
  2005-05-19 20:26         ` Guillaume Autran
  0 siblings, 1 reply; 9+ messages in thread
From: Dan Malek @ 2005-05-19 19:54 UTC (permalink / raw)
  To: Guillaume Autran; +Cc: linuxppc-embedded


On May 19, 2005, at 3:34 PM, Guillaume Autran wrote:

> Is it better like this ?

Yes, but now I see one problem with it.  :-)

We have to clear all events even though we may not
handle all of them.  Your while loop filters only the events
we process, but there could be others causing the interrupt
which will never get cleared.  In this case we end up with
an infinite interrupt loop where we don't process anything,
but we don't make the interrupt go away, either.   It may be
easier to forget the loop, just read/clear the event register
up front, then process based on the events based upon
what we found. It's what we tend to do in the other drivers.

The functions called to do the rx/tx processing have loops
in them to process all of the data they find, so it isn't likely
you have left anything behind.  If you want to try to save
the interrupt overhead, change it to do/while, check again
at the end before the exit.

Thanks.


	-- Dan

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

* Re: [PATCH] ppc32: fix cpm_uart_int() missing interrupts
  2005-05-19 19:54       ` Dan Malek
@ 2005-05-19 20:26         ` Guillaume Autran
  2005-05-19 20:32           ` Dan Malek
  2005-05-19 20:36           ` Guillaume Autran
  0 siblings, 2 replies; 9+ messages in thread
From: Guillaume Autran @ 2005-05-19 20:26 UTC (permalink / raw)
  To: Dan Malek; +Cc: linuxppc-embedded

[-- Attachment #1: Type: text/plain, Size: 1349 bytes --]

You are right, moving the event clearing statement is much easier and 
cleaner. Let the interrupt be the loop.
Thanks Dan !


Dan Malek wrote:

>
> On May 19, 2005, at 3:34 PM, Guillaume Autran wrote:
>
>> Is it better like this ?
>
>
> Yes, but now I see one problem with it.  :-)
>
> We have to clear all events even though we may not
> handle all of them.  Your while loop filters only the events
> we process, but there could be others causing the interrupt
> which will never get cleared.  In this case we end up with
> an infinite interrupt loop where we don't process anything,
> but we don't make the interrupt go away, either.   It may be
> easier to forget the loop, just read/clear the event register
> up front, then process based on the events based upon
> what we found. It's what we tend to do in the other drivers.
>
> The functions called to do the rx/tx processing have loops
> in them to process all of the data they find, so it isn't likely
> you have left anything behind.  If you want to try to save
> the interrupt overhead, change it to do/while, check again
> at the end before the exit.
>
> Thanks.
>
>
>     -- Dan
>

-- 
=======================================
Guillaume Autran
Senior Software Engineer
MRV Communications, Inc.
Tel: (978) 952-4932 office
E-mail: gautran@mrv.com
======================================= 


[-- Attachment #2: cpm_uart_core.patch --]
[-- Type: text/plain, Size: 970 bytes --]

diff -Nru linux-2.6.12-rc4.org/drivers/serial/cpm_uart/cpm_uart_core.c linux-2.6.12-rc4.new/drivers/serial/cpm_uart/cpm_uart_core.c
--- linux-2.6.12-rc4.org/drivers/serial/cpm_uart/cpm_uart_core.c	2005-05-07 01:20:31.000000000 -0400
+++ linux-2.6.12-rc4.new/drivers/serial/cpm_uart/cpm_uart_core.c	2005-05-19 16:23:13.000000000 -0400
@@ -336,22 +336,22 @@
 
 	if (IS_SMC(pinfo)) {
 		events = smcp->smc_smce;
+		smcp->smc_smce = events;
 		if (events & SMCM_BRKE)
 			uart_handle_break(port);
 		if (events & SMCM_RX)
 			cpm_uart_int_rx(port, regs);
 		if (events & SMCM_TX)
 			cpm_uart_int_tx(port, regs);
-		smcp->smc_smce = events;
 	} else {
 		events = sccp->scc_scce;
+		sccp->scc_scce = events;
 		if (events & UART_SCCM_BRKE)
 			uart_handle_break(port);
 		if (events & UART_SCCM_RX)
 			cpm_uart_int_rx(port, regs);
 		if (events & UART_SCCM_TX)
 			cpm_uart_int_tx(port, regs);
-		sccp->scc_scce = events;
 	}
 	return (events) ? IRQ_HANDLED : IRQ_NONE;
 }

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

* Re: [PATCH] ppc32: fix cpm_uart_int() missing interrupts
  2005-05-19 20:26         ` Guillaume Autran
@ 2005-05-19 20:32           ` Dan Malek
  2005-05-19 20:36           ` Guillaume Autran
  1 sibling, 0 replies; 9+ messages in thread
From: Dan Malek @ 2005-05-19 20:32 UTC (permalink / raw)
  To: Guillaume Autran; +Cc: linuxppc-embedded


On May 19, 2005, at 4:26 PM, Guillaume Autran wrote:

> You are right, moving the event clearing statement is much easier and 
> cleaner. Let the interrupt be the loop.

I also just had the thought we could check the events for non-zero,
clear them, and process only the ones we wanted.  That would save
the interrupt overhead if we had high speed ports (which I really
don't like for normal RS-232 anyway :-)).

I'll move this patch ahead.

Thanks.


	-- Dan

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

* Re: [PATCH] ppc32: fix cpm_uart_int() missing interrupts
  2005-05-19 20:26         ` Guillaume Autran
  2005-05-19 20:32           ` Dan Malek
@ 2005-05-19 20:36           ` Guillaume Autran
  2005-05-20 14:19             ` Dan Malek
  1 sibling, 1 reply; 9+ messages in thread
From: Guillaume Autran @ 2005-05-19 20:36 UTC (permalink / raw)
  To: Guillaume Autran; +Cc: linuxppc-embedded

[-- Attachment #1: Type: text/plain, Size: 2997 bytes --]

One thought though, if the event register is cleared _before_  the event 
is processed (clearing the cause), will the cpm set the bit again 
(before we have time to clear the cause) ? That would generate 2 
interrupts for the same event ? Am I right ?


Guillaume Autran wrote:

> You are right, moving the event clearing statement is much easier and 
> cleaner. Let the interrupt be the loop.
> Thanks Dan !
>
>
> Dan Malek wrote:
>
>>
>> On May 19, 2005, at 3:34 PM, Guillaume Autran wrote:
>>
>>> Is it better like this ?
>>
>>
>>
>> Yes, but now I see one problem with it.  :-)
>>
>> We have to clear all events even though we may not
>> handle all of them.  Your while loop filters only the events
>> we process, but there could be others causing the interrupt
>> which will never get cleared.  In this case we end up with
>> an infinite interrupt loop where we don't process anything,
>> but we don't make the interrupt go away, either.   It may be
>> easier to forget the loop, just read/clear the event register
>> up front, then process based on the events based upon
>> what we found. It's what we tend to do in the other drivers.
>>
>> The functions called to do the rx/tx processing have loops
>> in them to process all of the data they find, so it isn't likely
>> you have left anything behind.  If you want to try to save
>> the interrupt overhead, change it to do/while, check again
>> at the end before the exit.
>>
>> Thanks.
>>
>>
>>     -- Dan
>>
>
>------------------------------------------------------------------------
>
>diff -Nru linux-2.6.12-rc4.org/drivers/serial/cpm_uart/cpm_uart_core.c linux-2.6.12-rc4.new/drivers/serial/cpm_uart/cpm_uart_core.c
>--- linux-2.6.12-rc4.org/drivers/serial/cpm_uart/cpm_uart_core.c	2005-05-07 01:20:31.000000000 -0400
>+++ linux-2.6.12-rc4.new/drivers/serial/cpm_uart/cpm_uart_core.c	2005-05-19 16:23:13.000000000 -0400
>@@ -336,22 +336,22 @@
> 
> 	if (IS_SMC(pinfo)) {
> 		events = smcp->smc_smce;
>+		smcp->smc_smce = events;
> 		if (events & SMCM_BRKE)
> 			uart_handle_break(port);
> 		if (events & SMCM_RX)
> 			cpm_uart_int_rx(port, regs);
> 		if (events & SMCM_TX)
> 			cpm_uart_int_tx(port, regs);
>-		smcp->smc_smce = events;
> 	} else {
> 		events = sccp->scc_scce;
>+		sccp->scc_scce = events;
> 		if (events & UART_SCCM_BRKE)
> 			uart_handle_break(port);
> 		if (events & UART_SCCM_RX)
> 			cpm_uart_int_rx(port, regs);
> 		if (events & UART_SCCM_TX)
> 			cpm_uart_int_tx(port, regs);
>-		sccp->scc_scce = events;
> 	}
> 	return (events) ? IRQ_HANDLED : IRQ_NONE;
> }
>  
>
>------------------------------------------------------------------------
>
>_______________________________________________
>Linuxppc-embedded mailing list
>Linuxppc-embedded@ozlabs.org
>https://ozlabs.org/mailman/listinfo/linuxppc-embedded
>

-- 
=======================================
Guillaume Autran
Senior Software Engineer
MRV Communications, Inc.
Tel: (978) 952-4932 office
E-mail: gautran@mrv.com
======================================= 


[-- Attachment #2: Type: text/html, Size: 3900 bytes --]

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

* Re: [PATCH] ppc32: fix cpm_uart_int() missing interrupts
  2005-05-19 20:36           ` Guillaume Autran
@ 2005-05-20 14:19             ` Dan Malek
  0 siblings, 0 replies; 9+ messages in thread
From: Dan Malek @ 2005-05-20 14:19 UTC (permalink / raw)
  To: Guillaume Autran; +Cc: linuxppc-embedded


On May 19, 2005, at 4:36 PM, Guillaume Autran wrote:

>  One thought though, if the event register is cleared _before_=A0 the=20=

> event is processed (clearing the cause),

The cause of the event is it set completion flags on the buffer=20
descriptors.

>  will the cpm set the bit again (before we have time to clear the=20
> cause) ? That would generate 2 interrupts for the same event ? Am I=20
> right ?

No, it will work fine.

Thanks.


	-- Dan

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

end of thread, other threads:[~2005-05-20 14:19 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-05-18 17:09 [PATCH] ppc32: fix CONFIG_TASK_SIZE handling on 40x Eugene Surovegin
2005-05-19 17:59 ` [PATCH] ppc32: fix cpm_uart_int() missing interrupts Guillaume Autran
2005-05-19 18:11   ` Dan Malek
2005-05-19 19:34     ` Guillaume Autran
2005-05-19 19:54       ` Dan Malek
2005-05-19 20:26         ` Guillaume Autran
2005-05-19 20:32           ` Dan Malek
2005-05-19 20:36           ` Guillaume Autran
2005-05-20 14:19             ` Dan Malek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).