[PATCH] Fix typo in pmac_zilog

[PATCH] Fix typo in pmac_zilog

[Benjamin Herrenschmidt]

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 <benh@kernel.crashing.org>

--- 1.10/drivers/serial/pmac_zilog.c	2004-05-07 21:06:49 +10:00
+++ edited/drivers/serial/pmac_zilog.c	2004-05-28 13:38:07 +10:00
@@ -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;

Re: [PATCH] Fix typo in pmac_zilog

[David S. Miller]

On Fri, 28 May 2004 13:40:55 +1000
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

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

Re: [PATCH] Fix typo in pmac_zilog

[Benjamin Herrenschmidt]

On Sun, 2004-05-30 at 16:42, David S. Miller wrote:
> On Fri, 28 May 2004 13:40:55 +1000
> Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> 
> > 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 :-)

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.

Re: [PATCH] Fix typo in pmac_zilog

[Benjamin Herrenschmidt]

On Mon, 2004-05-31 at 10:32, David S. Miller wrote:
> On Sun, 30 May 2004 17:13:38 +1000
> Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> 
> > 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.

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

Yes, I told Russell about it.

> I'll fix up the Sparc drivers meanwhile.
-- 
Benjamin Herrenschmidt <benh@kernel.crashing.org>

Re: [PATCH] Fix typo in pmac_zilog

[David S. Miller]

On Sun, 30 May 2004 17:13:38 +1000
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

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