Skip to contentSkip to conversation

ppc32: fix CONFIG_TASK_SIZE handling on 40x

PATCH In review Eugene Surovegin 3 participants 9 messages May 18, 2005 – May 20, 2005

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ppc32: fix CONFIG_TASK_SIZE handling on 40x

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 :).

arch/ppc/kernel/head_4xx.S (mode:100644) +9 -6
Index: arch/ppc/kernel/head_4xx.S
===================================================================
@@ -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
#1 lore raw

Re: [PATCH] ppc32: fix cpm_uart_int() missing interrupts

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.

Signature · 7 lines
=======================================
Guillaume Autran
Senior Software Engineer
MRV Communications, Inc.
Tel: (978) 952-4932 office
E-mail: [email protected]
======================================= 
drivers/serial/cpm_uart/cpm_uart_core.c +12 -10
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
@@ -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;
 }
#2 lore raw

Re: [PATCH] ppc32: fix cpm_uart_int() missing interrupts

Quoted 1 line · Guillaume Autran

Any comment ?

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

Thanks.

	-- Dan
#3 lore raw

Re: [PATCH] ppc32: fix cpm_uart_int() missing interrupts

Is it better like this ?

Quoted 13 lines · Dan Malek
Quoted 1 line · Guillaume Autran

Any comment ?

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

Thanks.

    -- Dan
Signature · 7 lines
=======================================
Guillaume Autran
Senior Software Engineer
MRV Communications, Inc.
Tel: (978) 952-4932 office
E-mail: [email protected]
======================================= 
drivers/serial/cpm_uart/cpm_uart_core.c +18 -16
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
@@ -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;
 }
#4 lore raw

Re: [PATCH] ppc32: fix cpm_uart_int() missing interrupts

Quoted 1 line · Guillaume Autran

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
#5 lore raw

Re: [PATCH] ppc32: fix cpm_uart_int() missing interrupts

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

Quoted 29 lines · Dan Malek
Quoted 1 line · Guillaume Autran

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
Signature · 7 lines
=======================================
Guillaume Autran
Senior Software Engineer
MRV Communications, Inc.
Tel: (978) 952-4932 office
E-mail: [email protected]
======================================= 
drivers/serial/cpm_uart/cpm_uart_core.c +2 -2
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
@@ -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;
 }
#6 lore raw

Re: [PATCH] ppc32: fix cpm_uart_int() missing interrupts

Quoted 1 line · Guillaume Autran

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
#7 lore raw

Re: [PATCH] ppc32: fix cpm_uart_int() missing interrupts

In reply to Guillaume Autran

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 ?

Quoted 76 lines · Guillaume Autran

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

Quoted 25 lines · Dan Malek
Quoted 1 line · Guillaume Autran

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.

Quoted 5 lines

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

drivers/serial/cpm_uart/cpm_uart_core.c +2 -3
@@ -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
[email protected]
https://ozlabs.org/mailman/listinfo/linuxppc-embedded

Signature · 7 lines
=======================================
Guillaume Autran
Senior Software Engineer
MRV Communications, Inc.
Tel: (978) 952-4932 office
E-mail: [email protected]
======================================= 
#8 lore raw

Re: [PATCH] ppc32: fix cpm_uart_int() missing interrupts

Quoted 3 lines · Guillaume Autran

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.

Quoted 3 lines

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