Fix typo in pmac_zilog

PATCH In review Benjamin Herrenschmidt 2 participants 5 messages May 28, 2004 – May 31, 2004 PowerPC

lore

Share & tools

lore

Benjamin Herrenschmidt

author

2004-05-28 03:46 UTC #0 lore raw

PATCH 1/1 Fix typo in pmac_zilog

Hi !

This patch fixes a typo preventing channel B from working on the Rx path of pmac zilog (never calling tty_flip_*). I think I never tested channel B :)

Please, apply. Thanks to Hollis Blanchard for spotting the bug.

Signed-off-by Benjamin Herrenschmidt <[email protected]>

drivers/serial/pmac_zilog.c +1 -1

@@ -483,7 +483,7 @@
        		if (r3 & CHBEXT)
        			pmz_status_handle(uap_b, regs);
        	       	if (r3 & CHBRxIP)
-       			pmz_receive_chars(uap_b, regs);
+       			tty = pmz_receive_chars(uap_b, regs);
        		if (r3 & CHBTxIP)
        			pmz_transmit_chars(uap_b);
 	       	rc = IRQ_HANDLED;

David S. Miller

2004-05-30 06:43 UTC #1 lore raw

Quoted 3 lines · On Fri, 28 May 2004 13:40:55 +1000 Benjamin Herrenschmidt…

This patch fixes a typo preventing channel B from working on the Rx path of pmac zilog (never calling tty_flip_*). I think I never tested channel B :)

Ben, why do you do the tty_flip_buffer_push() outside of the port lock? Just because it's expensive and therefore this decreases the lock hold time, or is there some deadlock issue?

Sounds like I should make the change to sunzilog :-)

Benjamin Herrenschmidt

author

2004-05-30 07:19 UTC #2 lore raw

Quoted 7 lines · David S. Miller

Quoted 3 lines · On Fri, 28 May 2004 13:40:55 +1000 Benjamin Herrenschmidt…
This patch fixes a typo preventing channel B from working on the Rx path of pmac zilog (never calling tty_flip_*). I think I never tested channel B :)

Quoted 6 lines

Ben, why do you do the tty_flip_buffer_push() outside of the port lock? Just because it's expensive and therefore this decreases the lock hold time, or is there some deadlock issue?
Sounds like I should make the change to sunzilog :-)

There is a deadlock issue. I triggered once when I had a bug where the driver was flooding the input with zero's. All serial drivers seem to be affected. Apparently, tty_flip_* may call back into your write() routine

Ben.

David S. Miller

2004-05-31 00:33 UTC #3 lore raw

Quoted 3 lines · On Sun, 30 May 2004 17:13:38 +1000 Benjamin Herrenschmidt…

There is a deadlock issue. I triggered once when I had a bug where the driver was flooding the input with zero's. All serial drivers seem to be affected. Apparently, tty_flip_* may call back into your write() routine

Yes indeed, one code path is:

tty_flip_buffer_push()
if (tty->low_latency)

	flush_to_ldisc() {
		tty->ldisc.receive_buf() == n_tty_receive_buf() {
			...
			n_tty_receive_char() {
				...
				start_tty() {
					tty->driver->start() == uart_start()

and that's where we try to grab the uart port lock again.

It seems lots of serial drivers have this bug, even 8250.c :-)

I'll fix up the Sparc drivers meanwhile.

Benjamin Herrenschmidt

author

2004-05-31 00:37 UTC #4 lore raw

Quoted 10 lines · David S. Miller

Quoted 3 lines · On Sun, 30 May 2004 17:13:38 +1000 Benjamin Herrenschmidt…
There is a deadlock issue. I triggered once when I had a bug where the driver was flooding the input with zero's. All serial drivers seem to be affected. Apparently, tty_flip_* may call back into your write() routine
Yes indeed, one code path is:
.../...

Yup, another one is when echo is enabled, you may call back into write.

Quoted 1 line

It seems lots of serial drivers have this bug, even 8250.c :-)

Yes, I told Russell about it.

Quoted 1 line

I'll fix up the Sparc drivers meanwhile.

Signature · 1 line

Benjamin Herrenschmidt <[email protected]>