[PATCH 01/16] pmac_zilog: fix unexpected irq

[PATCH 01/16] pmac_zilog: fix unexpected irq

[Finn Thain]

On most 68k Macs the SCC IRQ is an autovector interrupt and cannot be masked. This can be a problem when pmac_zilog starts up.

For example, the serial debugging code in arch/m68k/kernel/head.S may be used beforehand. It disables the SCC interrupts at the chip but doesn't ack them. Then when a pmac_zilog port is opened and SCC chip interrupts become enabled, the machine locks up with "unexpected interrupt" because request_irq() hasn't happened yet.

Fix this by setting the SCC master interrupt enable bit only after the handler is installed. This is achieved by extracting that operation out of __pmz_startup() and placing it into a seperate routine.

A similar problem arises when the irq is freed. Fix this by resetting the chip first (on m68k mac).

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>

---

This patch has been tested on a variety of m68k Macs but no PowerMacs.

Index: linux-m68k/drivers/tty/serial/pmac_zilog.c
===================================================================
--- linux-m68k.orig/drivers/tty/serial/pmac_zilog.c	2011-10-22 23:02:22.000000000 +1100
+++ linux-m68k/drivers/tty/serial/pmac_zilog.c	2011-10-22 23:02:38.000000000 +1100
@@ -910,8 +910,8 @@ static int __pmz_startup(struct uart_pma
 	/* Clear handshaking, enable BREAK interrupts */
 	uap->curregs[R15] = BRKIE;
 
-	/* Master interrupt enable */
-	uap->curregs[R9] |= NV | MIE;
+	/* No vector */
+	uap->curregs[R9] |= NV;
 
 	pmz_load_zsregs(uap, uap->curregs);
 
@@ -925,6 +925,17 @@ static int __pmz_startup(struct uart_pma
 	return pwr_delay;
 }
 
+static void pmz_master_int_control(struct uart_pmac_port *uap, int enable)
+{
+	if (enable) {
+		uap->curregs[R9] |= MIE; /* Master interrupt enable */
+		write_zsreg(uap, R9, uap->curregs[R9]);
+	} else {
+		uap->curregs[R9] &= ~MIE;
+		write_zsreg(uap, 9, FHWRES);
+	}
+}
+
 static void pmz_irda_reset(struct uart_pmac_port *uap)
 {
 	uap->curregs[R5] |= DTR;
@@ -976,6 +987,19 @@ static int pmz_startup(struct uart_port
 		return -ENXIO;
 	}
 
+	/*
+	 * Most 68k Mac models cannot mask the SCC IRQ so they must enable
+	 * interrupts after the handler is installed and not before.
+	 */
+#ifndef CONFIG_MAC
+	if (!ZS_IS_CONS(uap))
+#endif
+	{
+		spin_lock_irqsave(&port->lock, flags);
+		pmz_master_int_control(uap, 1);
+		spin_unlock_irqrestore(&port->lock, flags);
+	}
+
 	mutex_unlock(&pmz_irq_mutex);
 
 	/* Right now, we deal with delay by blocking here, I'll be
@@ -1015,6 +1039,11 @@ static void pmz_shutdown(struct uart_por
 
 	mutex_lock(&pmz_irq_mutex);
 
+#ifdef CONFIG_MAC
+	if (!ZS_IS_OPEN(uap->mate))
+		pmz_master_int_control(uap, 0);
+#endif
+
 	/* Release interrupt handler */
 	free_irq(uap->port.irq, uap);
 
@@ -1734,6 +1763,7 @@ static int pmz_resume(struct macio_dev *
 		goto bail;
 	}
 	pwr_delay = __pmz_startup(uap);
+	pmz_master_int_control(uap, 1);
 
 	/* Take care of config that may have changed while asleep */
 	__pmz_set_termios(&uap->port, &uap->termios_cache, NULL);
@@ -2178,6 +2208,9 @@ static int __init pmz_console_setup(stru
 	 * Enable the hardware
 	 */
 	pwr_delay = __pmz_startup(uap);
+#ifndef CONFIG_MAC
+	pmz_master_int_control(uap, 1);
+#endif
 	if (pwr_delay)
 		mdelay(pwr_delay);
 	

[PATCH 01/16] pmac_zilog: fix unexpected irq

[Finn Thain]

On most 68k Macs the SCC IRQ is an autovector interrupt and cannot be 
masked. This can be a problem when pmac_zilog starts up.

For example, the serial debugging code in arch/m68k/kernel/head.S may be 
used beforehand. It disables the SCC interrupts at the chip but doesn't 
ack them. Then when a pmac_zilog port is opened and SCC chip interrupts 
become enabled, the machine locks up with "unexpected interrupt" because 
request_irq() hasn't happened yet.

Fix this by setting the SCC master interrupt enable bit only after the 
handler is installed. This is achieved by extracting that operation out of 
__pmz_startup() and placing it into a seperate routine.

A similar problem arises when the irq is freed. Fix this by resetting the 
chip first (on m68k mac).

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>

---

This patch has been tested on a variety of m68k Macs but no PowerMacs.

I am re-sending this patch Cc linux-serial. It still needs a suitable ack 
so that Geert can push it through his tree.

Index: linux-m68k/drivers/tty/serial/pmac_zilog.c
===================================================================
--- linux-m68k.orig/drivers/tty/serial/pmac_zilog.c	2011-10-22 23:02:22.000000000 +1100
+++ linux-m68k/drivers/tty/serial/pmac_zilog.c	2011-10-22 23:02:38.000000000 +1100
@@ -910,8 +910,8 @@ static int __pmz_startup(struct uart_pma
 	/* Clear handshaking, enable BREAK interrupts */
 	uap->curregs[R15] = BRKIE;
 
-	/* Master interrupt enable */
-	uap->curregs[R9] |= NV | MIE;
+	/* No vector */
+	uap->curregs[R9] |= NV;
 
 	pmz_load_zsregs(uap, uap->curregs);
 
@@ -925,6 +925,17 @@ static int __pmz_startup(struct uart_pma
 	return pwr_delay;
 }
 
+static void pmz_master_int_control(struct uart_pmac_port *uap, int enable)
+{
+	if (enable) {
+		uap->curregs[R9] |= MIE; /* Master interrupt enable */
+		write_zsreg(uap, R9, uap->curregs[R9]);
+	} else {
+		uap->curregs[R9] &= ~MIE;
+		write_zsreg(uap, 9, FHWRES);
+	}
+}
+
 static void pmz_irda_reset(struct uart_pmac_port *uap)
 {
 	uap->curregs[R5] |= DTR;
@@ -976,6 +987,19 @@ static int pmz_startup(struct uart_port
 		return -ENXIO;
 	}
 
+	/*
+	 * Most 68k Mac models cannot mask the SCC IRQ so they must enable
+	 * interrupts after the handler is installed and not before.
+	 */
+#ifndef CONFIG_MAC
+	if (!ZS_IS_CONS(uap))
+#endif
+	{
+		spin_lock_irqsave(&port->lock, flags);
+		pmz_master_int_control(uap, 1);
+		spin_unlock_irqrestore(&port->lock, flags);
+	}
+
 	mutex_unlock(&pmz_irq_mutex);
 
 	/* Right now, we deal with delay by blocking here, I'll be
@@ -1015,6 +1039,11 @@ static void pmz_shutdown(struct uart_por
 
 	mutex_lock(&pmz_irq_mutex);
 
+#ifdef CONFIG_MAC
+	if (!ZS_IS_OPEN(uap->mate))
+		pmz_master_int_control(uap, 0);
+#endif
+
 	/* Release interrupt handler */
 	free_irq(uap->port.irq, uap);
 
@@ -1734,6 +1763,7 @@ static int pmz_resume(struct macio_dev *
 		goto bail;
 	}
 	pwr_delay = __pmz_startup(uap);
+	pmz_master_int_control(uap, 1);
 
 	/* Take care of config that may have changed while asleep */
 	__pmz_set_termios(&uap->port, &uap->termios_cache, NULL);
@@ -2178,6 +2208,9 @@ static int __init pmz_console_setup(stru
 	 * Enable the hardware
 	 */
 	pwr_delay = __pmz_startup(uap);
+#ifndef CONFIG_MAC
+	pmz_master_int_control(uap, 1);
+#endif
 	if (pwr_delay)
 		mdelay(pwr_delay);
 	

Re: [PATCH 01/16] pmac_zilog: fix unexpected irq

[Alan Cox]

On Fri, 25 Nov 2011 01:34:58 +1100 (EST)
Finn Thain <fthain@telegraphics.com.au> wrote:

> 
> On most 68k Macs the SCC IRQ is an autovector interrupt and cannot be 
> masked. This can be a problem when pmac_zilog starts up.
> 
> For example, the serial debugging code in arch/m68k/kernel/head.S may be 
> used beforehand. It disables the SCC interrupts at the chip but doesn't 
> ack them. Then when a pmac_zilog port is opened and SCC chip interrupts 
> become enabled, the machine locks up with "unexpected interrupt" because 
> request_irq() hasn't happened yet.
> 
> Fix this by setting the SCC master interrupt enable bit only after the 
> handler is installed. This is achieved by extracting that operation out of 
> __pmz_startup() and placing it into a seperate routine.
> 
> A similar problem arises when the irq is freed. Fix this by resetting the 
> chip first (on m68k mac).
> 
> Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> 
> ---
> 
> This patch has been tested on a variety of m68k Macs but no PowerMacs.
> 
> I am re-sending this patch Cc linux-serial. It still needs a suitable ack 
> so that Geert can push it through his tree.

Given the change should work for all hardware do we really need the
ifdefs. Far better I would have thought to just change it so we don't
have to maintain what is effectively two versions of the code between now
and 2038.

So no ack from me yet - I'd like to understand the ifdef decision first.
Otherwise it looks sensible.

Alan

Re: [PATCH 01/16] pmac_zilog: fix unexpected irq

[Benjamin Herrenschmidt]

On Thu, 2011-11-24 at 14:56 +0000, Alan Cox wrote:
> > This patch has been tested on a variety of m68k Macs but no
> PowerMacs.
> > 
> > I am re-sending this patch Cc linux-serial. It still needs a
> suitable ack 
> > so that Geert can push it through his tree.
> 
> Given the change should work for all hardware do we really need the
> ifdefs. Far better I would have thought to just change it so we don't
> have to maintain what is effectively two versions of the code between
> now
> and 2038.
> 
> So no ack from me yet - I'd like to understand the ifdef decision
> first.
> Otherwise it looks sensible.

Yes, agreed. Sorry, that one was one my to-do list for a while, I meant
to look into more details and test on a ppc or two here see if it breaks
anything, and never got to do it.

I'll try to give it a go before hell freezes over.

Cheers,
Ben.

Re: [PATCH 01/16] pmac_zilog: fix unexpected irq

[Finn Thain]

On Thu, 24 Nov 2011, Alan Cox wrote:

> Given the change should work for all hardware do we really need the 
> ifdefs. Far better I would have thought to just change it so we don't 
> have to maintain what is effectively two versions of the code between 
> now and 2038.

I agree.

> 
> So no ack from me yet - I'd like to understand the ifdef decision first.

Removing ifdefs makes the changes more invasive and the suspend/resume 
code then has to be addressed, which I've avoided.

The suspend/resume code path can't be tested on m68k macs and the common 
code paths I can't easily test on a powermac.

This patch should not be needed because the chip reset shouldn't leave the 
tx and rx interrupts enabled. Those interrupts are explicitly enabled only 
after request_irq(), so patching the master interrupt enable behaviour 
should be redundant. But that's not the case in practice.

The chip reset code is already messy. I was inclined towards ifdefs and 
reluctant to share more code after practical experience suggested possible 
differences in the SCC/ESCC devices.

I guess I was hoping that the powermac maintainers might prefer ifdefs to 
increased risk of destabilising the driver on powermacs...

But a more invasive patch would make for better code. I will see if I can 
borrow a suitable PCI PowerMac.

Finn

> Otherwise it looks sensible.
> 
> Alan

Re: [PATCH 01/16] pmac_zilog: fix unexpected irq

[Benjamin Herrenschmidt]

> Removing ifdefs makes the changes more invasive and the suspend/resume 
> code then has to be addressed, which I've avoided.
> 
> The suspend/resume code path can't be tested on m68k macs and the common 
> code paths I can't easily test on a powermac.
> 
> This patch should not be needed because the chip reset shouldn't leave the 
> tx and rx interrupts enabled. Those interrupts are explicitly enabled only 
> after request_irq(), so patching the master interrupt enable behaviour 
> should be redundant. But that's not the case in practice.
> 
> The chip reset code is already messy. I was inclined towards ifdefs and 
> reluctant to share more code after practical experience suggested possible 
> differences in the SCC/ESCC devices.
> 
> I guess I was hoping that the powermac maintainers might prefer ifdefs to 
> increased risk of destabilising the driver on powermacs...
> 
> But a more invasive patch would make for better code. I will see if I can 
> borrow a suitable PCI PowerMac.

Please do the more invasive patch, I'll beat it up on powermacs.

Cheers,
Ben.

RE: [PATCH 01/16] pmac_zilog: fix unexpected irq

[David Laight]

=20
> On most 68k Macs the SCC IRQ is an autovector interrupt and cannot be=20
> masked. This can be a problem when pmac_zilog starts up.

Wouldn't this also happen if the interrupt were shared?
Hopefully nothing vaguely modern uses the borked Zilog 8530 SCC
(which I presume is the part in question - brings back
too many nightmares....)

	David

RE: [PATCH 01/16] pmac_zilog: fix unexpected irq

[Benjamin Herrenschmidt]

On Thu, 2011-11-24 at 15:28 +0000, David Laight wrote:
> > On most 68k Macs the SCC IRQ is an autovector interrupt and cannot be 
> > masked. This can be a problem when pmac_zilog starts up.
> 
> Wouldn't this also happen if the interrupt were shared?
> Hopefully nothing vaguely modern uses the borked Zilog 8530 SCC
> (which I presume is the part in question - brings back
> too many nightmares....)

Yup. Afaik, the most recent you can find with that are PowerMacs which
used it for their internal modem (even my G5 has one wired to the
internal slot afaik), tho none of those had shared interrupts.

Cheers,
Ben.

[PATCH 01/16 v2] pmac_zilog: fix unexpected irq

[Finn Thain]

On most 68k Macs the SCC IRQ is an autovector interrupt and cannot be 
masked. This can be a problem when pmac_zilog starts up.

For example, the serial debugging code in arch/m68k/kernel/head.S may be 
used beforehand. It disables the SCC interrupts at the chip but doesn't 
ack them. Then when a pmac_zilog port is opened and SCC chip interrupts 
become enabled, the machine locks up with "unexpected interrupt" because 
request_irq() hasn't happened yet.

Fix this by setting the interrupt enable bits only after the handler is 
installed and before it is uninstalled. Also move this bit flipping into a 
separate pmz_interrupt_control() routine. Replace all instances of these 
operations with calls to this routine.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>

---

Re-implemented since v1 using a simpler approach that avoids messing with 
the Master Interrupt Enable bit. As well as the ifdef problem, it turns 
out that v1 was not sufficient to entirely fix the problem.

This patch has been tested on a PowerBook 520 but no PowerMacs yet.

Index: linux-git/drivers/tty/serial/pmac_zilog.c
===================================================================
--- linux-git.orig/drivers/tty/serial/pmac_zilog.c	2011-12-07 01:56:43.000000000 +1100
+++ linux-git/drivers/tty/serial/pmac_zilog.c	2011-12-07 01:56:55.000000000 +1100
@@ -216,6 +216,18 @@ static void pmz_maybe_update_regs(struct
 	}
 }
 
+static void pmz_interrupt_control(struct uart_pmac_port *uap, int enable)
+{
+	if (enable) {
+		uap->curregs[1] |= INT_ALL_Rx | TxINT_ENAB;
+		if (!ZS_IS_EXTCLK(uap))
+			uap->curregs[1] |= EXT_INT_ENAB;
+	} else {
+		uap->curregs[1] &= ~(EXT_INT_ENAB | TxINT_ENAB | RxINT_MASK);
+	}
+	write_zsreg(uap, R1, uap->curregs[1]);
+}
+
 static struct tty_struct *pmz_receive_chars(struct uart_pmac_port *uap)
 {
 	struct tty_struct *tty = NULL;
@@ -339,9 +351,7 @@ static struct tty_struct *pmz_receive_ch
 
 	return tty;
  flood:
-	uap->curregs[R1] &= ~(EXT_INT_ENAB | TxINT_ENAB | RxINT_MASK);
-	write_zsreg(uap, R1, uap->curregs[R1]);
-	zssync(uap);
+	pmz_interrupt_control(uap, 0);
 	pmz_error("pmz: rx irq flood !\n");
 	return tty;
 }
@@ -990,12 +1000,9 @@ static int pmz_startup(struct uart_port
 	if (ZS_IS_IRDA(uap))
 		pmz_irda_reset(uap);
 
-	/* Enable interrupts emission from the chip */
+	/* Enable interrupt requests for the channel */
 	spin_lock_irqsave(&port->lock, flags);
-	uap->curregs[R1] |= INT_ALL_Rx | TxINT_ENAB;
-	if (!ZS_IS_EXTCLK(uap))
-		uap->curregs[R1] |= EXT_INT_ENAB;
-	write_zsreg(uap, R1, uap->curregs[R1]);
+	pmz_interrupt_control(uap, 1);
 	spin_unlock_irqrestore(&port->lock, flags);
 
 	pmz_debug("pmz: startup() done.\n");
@@ -1015,6 +1022,25 @@ static void pmz_shutdown(struct uart_por
 
 	mutex_lock(&pmz_irq_mutex);
 
+	if (!ZS_IS_ASLEEP(uap)) {
+		spin_lock_irqsave(&port->lock, flags);
+
+		if (!ZS_IS_CONS(uap)) {
+			/* Disable receiver and transmitter */
+			uap->curregs[R3] &= ~RxENABLE;
+			uap->curregs[R5] &= ~TxENABLE;
+
+			/* Disable BRK assertion */
+			uap->curregs[R5] &= ~SND_BRK;
+			pmz_maybe_update_regs(uap);
+		}
+
+		/* Disable interrupt requests for the channel */
+		pmz_interrupt_control(uap, 0);
+
+		spin_unlock_irqrestore(&port->lock, flags);
+	}
+
 	/* Release interrupt handler */
 	free_irq(uap->port.irq, uap);
 
@@ -1025,29 +1051,8 @@ static void pmz_shutdown(struct uart_por
 	if (!ZS_IS_OPEN(uap->mate))
 		pmz_get_port_A(uap)->flags &= ~PMACZILOG_FLAG_IS_IRQ_ON;
 
-	/* Disable interrupts */
-	if (!ZS_IS_ASLEEP(uap)) {
-		uap->curregs[R1] &= ~(EXT_INT_ENAB | TxINT_ENAB | RxINT_MASK);
-		write_zsreg(uap, R1, uap->curregs[R1]);
-		zssync(uap);
-	}
-
-	if (ZS_IS_CONS(uap) || ZS_IS_ASLEEP(uap)) {
-		spin_unlock_irqrestore(&port->lock, flags);
-		mutex_unlock(&pmz_irq_mutex);
-		return;
-	}
-
-	/* Disable receiver and transmitter.  */
-	uap->curregs[R3] &= ~RxENABLE;
-	uap->curregs[R5] &= ~TxENABLE;
-
-	/* Disable all interrupts and BRK assertion.  */
-	uap->curregs[R5] &= ~SND_BRK;
-	pmz_maybe_update_regs(uap);
-
-	/* Shut the chip down */
-	pmz_set_scc_power(uap, 0);
+	if (!ZS_IS_ASLEEP(uap) && !ZS_IS_CONS(uap))
+		pmz_set_scc_power(uap, 0);	/* Shut the chip down */
 
 	spin_unlock_irqrestore(&port->lock, flags);
 
@@ -1352,19 +1357,15 @@ static void pmz_set_termios(struct uart_
 	spin_lock_irqsave(&port->lock, flags);	
 
 	/* Disable IRQs on the port */
-	uap->curregs[R1] &= ~(EXT_INT_ENAB | TxINT_ENAB | RxINT_MASK);
-	write_zsreg(uap, R1, uap->curregs[R1]);
+	pmz_interrupt_control(uap, 0);
 
 	/* Setup new port configuration */
 	__pmz_set_termios(port, termios, old);
 
 	/* Re-enable IRQs on the port */
-	if (ZS_IS_OPEN(uap)) {
-		uap->curregs[R1] |= INT_ALL_Rx | TxINT_ENAB;
-		if (!ZS_IS_EXTCLK(uap))
-			uap->curregs[R1] |= EXT_INT_ENAB;
-		write_zsreg(uap, R1, uap->curregs[R1]);
-	}
+	if (ZS_IS_OPEN(uap))
+		pmz_interrupt_control(uap, 1);
+
 	spin_unlock_irqrestore(&port->lock, flags);
 }
 
@@ -1670,15 +1671,13 @@ static int pmz_suspend(struct macio_dev
 
 	spin_lock_irqsave(&uap->port.lock, flags);
 
+	/* Disable receiver and transmitter.  */
+	uap->curregs[R3] &= ~RxENABLE;
+	uap->curregs[R5] &= ~TxENABLE;
+
+	pmz_interrupt_control(uap, 0);
+
 	if (ZS_IS_OPEN(uap) || ZS_IS_CONS(uap)) {
-		/* Disable receiver and transmitter.  */
-		uap->curregs[R3] &= ~RxENABLE;
-		uap->curregs[R5] &= ~TxENABLE;
-
-		/* Disable all interrupts and BRK assertion.  */
-		uap->curregs[R1] &= ~(EXT_INT_ENAB | TxINT_ENAB | RxINT_MASK);
-		uap->curregs[R5] &= ~SND_BRK;
-		pmz_load_zsregs(uap, uap->curregs);
 		uap->flags |= PMACZILOG_FLAG_IS_ASLEEP;
 		mb();
 	}
@@ -1738,14 +1737,6 @@ static int pmz_resume(struct macio_dev *
 	/* Take care of config that may have changed while asleep */
 	__pmz_set_termios(&uap->port, &uap->termios_cache, NULL);
 
-	if (ZS_IS_OPEN(uap)) {
-		/* Enable interrupts */		
-		uap->curregs[R1] |= INT_ALL_Rx | TxINT_ENAB;
-		if (!ZS_IS_EXTCLK(uap))
-			uap->curregs[R1] |= EXT_INT_ENAB;
-		write_zsreg(uap, R1, uap->curregs[R1]);
-	}
-
 	spin_unlock_irqrestore(&uap->port.lock, flags);
 
 	if (ZS_IS_CONS(uap))
@@ -1757,6 +1748,12 @@ static int pmz_resume(struct macio_dev *
 		enable_irq(uap->port.irq);
 	}
 
+	if (ZS_IS_OPEN(uap)) {
+		spin_lock_irqsave(&uap->port.lock, flags);
+		pmz_interrupt_control(uap, 1);
+		spin_unlock_irqrestore(&uap->port.lock, flags);
+	}
+
  bail:
 	mutex_unlock(&state->port.mutex);
 	mutex_unlock(&pmz_irq_mutex);

Re: [PATCH 01/16 v2] pmac_zilog: fix unexpected irq

[Geert Uytterhoeven]

Hi Finn,

On Tue, Dec 6, 2011 at 16:13, Finn Thain <fthain@telegraphics.com.au> wrote=
:
> +static void pmz_interrupt_control(struct uart_pmac_port *uap, int enable=
)
> +{
> + =C2=A0 =C2=A0 =C2=A0 if (enable) {
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 uap->curregs[1] |=3D I=
NT_ALL_Rx | TxINT_ENAB;
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (!ZS_IS_EXTCLK(uap)=
)
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 uap->curregs[1] |=3D EXT_INT_ENAB;
> + =C2=A0 =C2=A0 =C2=A0 } else {
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 uap->curregs[1] &=3D ~=
(EXT_INT_ENAB | TxINT_ENAB | RxINT_MASK);

Should there be a call to zssync() here?
The old code always did that after disabling interrupts.

> + =C2=A0 =C2=A0 =C2=A0 }
> + =C2=A0 =C2=A0 =C2=A0 write_zsreg(uap, R1, uap->curregs[1]);
> +}
> +
> =C2=A0static struct tty_struct *pmz_receive_chars(struct uart_pmac_port *=
uap)
> =C2=A0{
> =C2=A0 =C2=A0 =C2=A0 =C2=A0struct tty_struct *tty =3D NULL;
> @@ -339,9 +351,7 @@ static struct tty_struct *pmz_receive_ch
>
> =C2=A0 =C2=A0 =C2=A0 =C2=A0return tty;
> =C2=A0flood:
> - =C2=A0 =C2=A0 =C2=A0 uap->curregs[R1] &=3D ~(EXT_INT_ENAB | TxINT_ENAB =
| RxINT_MASK);
> - =C2=A0 =C2=A0 =C2=A0 write_zsreg(uap, R1, uap->curregs[R1]);
> - =C2=A0 =C2=A0 =C2=A0 zssync(uap);

Cfr. e.g. here.

> + =C2=A0 =C2=A0 =C2=A0 pmz_interrupt_control(uap, 0);
> =C2=A0 =C2=A0 =C2=A0 =C2=A0pmz_error("pmz: rx irq flood !\n");
> =C2=A0 =C2=A0 =C2=A0 =C2=A0return tty;
> =C2=A0}

Gr{oetje,eeting}s,

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k=
.org

In personal conversations with technical people, I call myself a hacker. Bu=
t
when I'm talking to journalists I just say "programmer" or something like t=
hat.
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0=C2=A0 =C2=A0=C2=A0 -- Linus Torvalds

Re: [PATCH 01/16 v2] pmac_zilog: fix unexpected irq

[Finn Thain]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2634 bytes --]


On Tue, 6 Dec 2011, Geert Uytterhoeven wrote:

> Hi Finn,
> 
> On Tue, Dec 6, 2011 at 16:13, Finn Thain <fthain@telegraphics.com.au> wrote:
> > +static void pmz_interrupt_control(struct uart_pmac_port *uap, int enable)
> > +{
> > +       if (enable) {
> > +               uap->curregs[1] |= INT_ALL_Rx | TxINT_ENAB;
> > +               if (!ZS_IS_EXTCLK(uap))
> > +                       uap->curregs[1] |= EXT_INT_ENAB;
> > +       } else {
> > +               uap->curregs[1] &= ~(EXT_INT_ENAB | TxINT_ENAB | RxINT_MASK);
> 
> Should there be a call to zssync() here?

I don't think so. Though I should have mentioned the change in the patch 
header.

> The old code always did that after disabling interrupts.

pmz_load_zsregs(), pmz_set_termios() and pmz_suspend() don't do it.

sunzilog only does it on sparc64 and only when writing to the data 
register or writing command modifiers to the control register ("On 64-bit 
sparc we only need to flush single writes to ensure completion.")

I can't find any purpose for control register reads in the chip manual.

The zssync() calls following some WR1 writes originate here: 
http://git.kernel.org/?p=linux/kernel/git/tglx/history.git;a=commitdiff;h=9e9d9f693c7def3900725c04c6b64311655eea51

So I don't see any need for control register reads but hopefully Ben can 
say for sure.

Reading the patch now I notice that I dropped a pmz_maybe_update_regs() or 
pmz_load_zsregs() in the suspend path. I will send a new patch.

Finn

> 
> > +       }
> > +       write_zsreg(uap, R1, uap->curregs[1]);
> > +}
> > +
> >  static struct tty_struct *pmz_receive_chars(struct uart_pmac_port *uap)
> >  {
> >        struct tty_struct *tty = NULL;
> > @@ -339,9 +351,7 @@ static struct tty_struct *pmz_receive_ch
> >
> >        return tty;
> >  flood:
> > -       uap->curregs[R1] &= ~(EXT_INT_ENAB | TxINT_ENAB | RxINT_MASK);
> > -       write_zsreg(uap, R1, uap->curregs[R1]);
> > -       zssync(uap);
> 
> Cfr. e.g. here.
> 
> > +       pmz_interrupt_control(uap, 0);
> >        pmz_error("pmz: rx irq flood !\n");
> >        return tty;
> >  }
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
> 

Re: [PATCH 01/16 v2] pmac_zilog: fix unexpected irq

[Alan Cox]

On Wed, 7 Dec 2011 02:13:41 +1100 (EST)
Finn Thain <fthain@telegraphics.com.au> wrote:

> 
> On most 68k Macs the SCC IRQ is an autovector interrupt and cannot be 
> masked. This can be a problem when pmac_zilog starts up.
> 
> For example, the serial debugging code in arch/m68k/kernel/head.S may be 
> used beforehand. It disables the SCC interrupts at the chip but doesn't 
> ack them. Then when a pmac_zilog port is opened and SCC chip interrupts 
> become enabled, the machine locks up with "unexpected interrupt" because 
> request_irq() hasn't happened yet.
> 
> Fix this by setting the interrupt enable bits only after the handler is 
> installed and before it is uninstalled. Also move this bit flipping into a 
> separate pmz_interrupt_control() routine. Replace all instances of these 
> operations with calls to this routine.
> 
> Signed-off-by: Finn Thain <fthain@telegraphics.com.au>

Nice

Acked-by: Alan Cox <alan@linux.intel.com>

[PATCH 01/16 v3] pmac_zilog: fix unexpected irq

[Finn Thain]

On most 68k Macs the SCC IRQ is an autovector interrupt and cannot be 
masked. This can be a problem when pmac_zilog starts up.

For example, the serial debugging code in arch/m68k/kernel/head.S may be 
used beforehand. It disables the SCC interrupts at the chip but doesn't 
ack them. Then when a pmac_zilog port is used, the machine locks up with 
"unexpected interrupt".

This can happen in pmz_shutdown() since the irq is freed before the 
channel interrupts are disabled.

Fix this by clearing interrupt enable bits before the handler is 
uninstalled. Also move the interrupt control bit flipping into a separate 
pmz_interrupt_control() routine. Replace all instances of these operations 
with calls to this routine. Omit the zssync() calls that seem to serve no 
purpose.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
Acked-by: Alan Cox <alan@linux.intel.com>

---
   
Re-implemented as v2 using a simpler approach that avoids messing with the 
Master Interrupt Enable bit. As well as the ifdef problem, it turns out 
that v1 was not sufficient to entirely fix the problem.

v3 avoids needless changes to the logic and locking in the suspend and 
shutdown code and tries to keep register writes closer to their original 
sequence.

This patch has been tested on a PowerBook 520 but no PowerMacs yet.


Index: linux-git/drivers/tty/serial/pmac_zilog.c
===================================================================
--- linux-git.orig/drivers/tty/serial/pmac_zilog.c	2011-12-07 12:36:32.000000000 +1100
+++ linux-git/drivers/tty/serial/pmac_zilog.c	2011-12-07 14:29:21.000000000 +1100
@@ -216,6 +216,18 @@ static void pmz_maybe_update_regs(struct
 	}
 }
 
+static void pmz_interrupt_control(struct uart_pmac_port *uap, int enable)
+{
+	if (enable) {
+		uap->curregs[1] |= INT_ALL_Rx | TxINT_ENAB;
+		if (!ZS_IS_EXTCLK(uap))
+			uap->curregs[1] |= EXT_INT_ENAB;
+	} else {
+		uap->curregs[1] &= ~(EXT_INT_ENAB | TxINT_ENAB | RxINT_MASK);
+	}
+	write_zsreg(uap, R1, uap->curregs[1]);
+}
+
 static struct tty_struct *pmz_receive_chars(struct uart_pmac_port *uap)
 {
 	struct tty_struct *tty = NULL;
@@ -339,9 +351,7 @@ static struct tty_struct *pmz_receive_ch
 
 	return tty;
  flood:
-	uap->curregs[R1] &= ~(EXT_INT_ENAB | TxINT_ENAB | RxINT_MASK);
-	write_zsreg(uap, R1, uap->curregs[R1]);
-	zssync(uap);
+	pmz_interrupt_control(uap, 0);
 	pmz_error("pmz: rx irq flood !\n");
 	return tty;
 }
@@ -990,12 +1000,9 @@ static int pmz_startup(struct uart_port
 	if (ZS_IS_IRDA(uap))
 		pmz_irda_reset(uap);
 
-	/* Enable interrupts emission from the chip */
+	/* Enable interrupt requests for the channel */
 	spin_lock_irqsave(&port->lock, flags);
-	uap->curregs[R1] |= INT_ALL_Rx | TxINT_ENAB;
-	if (!ZS_IS_EXTCLK(uap))
-		uap->curregs[R1] |= EXT_INT_ENAB;
-	write_zsreg(uap, R1, uap->curregs[R1]);
+	pmz_interrupt_control(uap, 1);
 	spin_unlock_irqrestore(&port->lock, flags);
 
 	pmz_debug("pmz: startup() done.\n");
@@ -1015,6 +1022,25 @@ static void pmz_shutdown(struct uart_por
 
 	mutex_lock(&pmz_irq_mutex);
 
+	spin_lock_irqsave(&port->lock, flags);
+
+	if (!ZS_IS_ASLEEP(uap)) {
+		/* Disable interrupt requests for the channel */
+		pmz_interrupt_control(uap, 0);
+
+		if (!ZS_IS_CONS(uap)) {
+			/* Disable receiver and transmitter */
+			uap->curregs[R3] &= ~RxENABLE;
+			uap->curregs[R5] &= ~TxENABLE;
+
+			/* Disable break assertion */
+			uap->curregs[R5] &= ~SND_BRK;
+			pmz_maybe_update_regs(uap);
+		}
+	}
+
+	spin_unlock_irqrestore(&port->lock, flags);
+
 	/* Release interrupt handler */
 	free_irq(uap->port.irq, uap);
 
@@ -1025,29 +1051,8 @@ static void pmz_shutdown(struct uart_por
 	if (!ZS_IS_OPEN(uap->mate))
 		pmz_get_port_A(uap)->flags &= ~PMACZILOG_FLAG_IS_IRQ_ON;
 
-	/* Disable interrupts */
-	if (!ZS_IS_ASLEEP(uap)) {
-		uap->curregs[R1] &= ~(EXT_INT_ENAB | TxINT_ENAB | RxINT_MASK);
-		write_zsreg(uap, R1, uap->curregs[R1]);
-		zssync(uap);
-	}
-
-	if (ZS_IS_CONS(uap) || ZS_IS_ASLEEP(uap)) {
-		spin_unlock_irqrestore(&port->lock, flags);
-		mutex_unlock(&pmz_irq_mutex);
-		return;
-	}
-
-	/* Disable receiver and transmitter.  */
-	uap->curregs[R3] &= ~RxENABLE;
-	uap->curregs[R5] &= ~TxENABLE;
-
-	/* Disable all interrupts and BRK assertion.  */
-	uap->curregs[R5] &= ~SND_BRK;
-	pmz_maybe_update_regs(uap);
-
-	/* Shut the chip down */
-	pmz_set_scc_power(uap, 0);
+	if (!ZS_IS_ASLEEP(uap) && !ZS_IS_CONS(uap))
+		pmz_set_scc_power(uap, 0);	/* Shut the chip down */
 
 	spin_unlock_irqrestore(&port->lock, flags);
 
@@ -1352,19 +1357,15 @@ static void pmz_set_termios(struct uart_
 	spin_lock_irqsave(&port->lock, flags);	
 
 	/* Disable IRQs on the port */
-	uap->curregs[R1] &= ~(EXT_INT_ENAB | TxINT_ENAB | RxINT_MASK);
-	write_zsreg(uap, R1, uap->curregs[R1]);
+	pmz_interrupt_control(uap, 0);
 
 	/* Setup new port configuration */
 	__pmz_set_termios(port, termios, old);
 
 	/* Re-enable IRQs on the port */
-	if (ZS_IS_OPEN(uap)) {
-		uap->curregs[R1] |= INT_ALL_Rx | TxINT_ENAB;
-		if (!ZS_IS_EXTCLK(uap))
-			uap->curregs[R1] |= EXT_INT_ENAB;
-		write_zsreg(uap, R1, uap->curregs[R1]);
-	}
+	if (ZS_IS_OPEN(uap))
+		pmz_interrupt_control(uap, 1);
+
 	spin_unlock_irqrestore(&port->lock, flags);
 }
 
@@ -1671,14 +1672,17 @@ static int pmz_suspend(struct macio_dev
 	spin_lock_irqsave(&uap->port.lock, flags);
 
 	if (ZS_IS_OPEN(uap) || ZS_IS_CONS(uap)) {
-		/* Disable receiver and transmitter.  */
+		/* Disable interrupt requests for the channel */
+		pmz_interrupt_control(uap, 0);
+
+		/* Disable receiver and transmitter */
 		uap->curregs[R3] &= ~RxENABLE;
 		uap->curregs[R5] &= ~TxENABLE;
 
-		/* Disable all interrupts and BRK assertion.  */
-		uap->curregs[R1] &= ~(EXT_INT_ENAB | TxINT_ENAB | RxINT_MASK);
+		/* Disable break assertion */
 		uap->curregs[R5] &= ~SND_BRK;
 		pmz_load_zsregs(uap, uap->curregs);
+
 		uap->flags |= PMACZILOG_FLAG_IS_ASLEEP;
 		mb();
 	}
@@ -1738,14 +1742,6 @@ static int pmz_resume(struct macio_dev *
 	/* Take care of config that may have changed while asleep */
 	__pmz_set_termios(&uap->port, &uap->termios_cache, NULL);
 
-	if (ZS_IS_OPEN(uap)) {
-		/* Enable interrupts */		
-		uap->curregs[R1] |= INT_ALL_Rx | TxINT_ENAB;
-		if (!ZS_IS_EXTCLK(uap))
-			uap->curregs[R1] |= EXT_INT_ENAB;
-		write_zsreg(uap, R1, uap->curregs[R1]);
-	}
-
 	spin_unlock_irqrestore(&uap->port.lock, flags);
 
 	if (ZS_IS_CONS(uap))
@@ -1757,6 +1753,12 @@ static int pmz_resume(struct macio_dev *
 		enable_irq(uap->port.irq);
 	}
 
+	if (ZS_IS_OPEN(uap)) {
+		spin_lock_irqsave(&uap->port.lock, flags);
+		pmz_interrupt_control(uap, 1);
+		spin_unlock_irqrestore(&uap->port.lock, flags);
+	}
+
  bail:
 	mutex_unlock(&state->port.mutex);
 	mutex_unlock(&pmz_irq_mutex);

Re: [PATCH 01/16 v3] pmac_zilog: fix unexpected irq

[Benjamin Herrenschmidt]

On Wed, 2011-12-07 at 14:49 +1100, Finn Thain wrote:
> On most 68k Macs the SCC IRQ is an autovector interrupt and cannot be 
> masked. This can be a problem when pmac_zilog starts up.

Thanks. I'll test it on a powermac or two and will merge it via the
powerpc -next tree if it works out allright.

Cheers,
Ben.
 

> For example, the serial debugging code in arch/m68k/kernel/head.S may be 
> used beforehand. It disables the SCC interrupts at the chip but doesn't 
> ack them. Then when a pmac_zilog port is used, the machine locks up with 
> "unexpected interrupt".
> 
> This can happen in pmz_shutdown() since the irq is freed before the 
> channel interrupts are disabled.
> 
> Fix this by clearing interrupt enable bits before the handler is 
> uninstalled. Also move the interrupt control bit flipping into a separate 
> pmz_interrupt_control() routine. Replace all instances of these operations 
> with calls to this routine. Omit the zssync() calls that seem to serve no 
> purpose.
> 
> Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
> Acked-by: Alan Cox <alan@linux.intel.com>
> 
> ---
>    
> Re-implemented as v2 using a simpler approach that avoids messing with the 
> Master Interrupt Enable bit. As well as the ifdef problem, it turns out 
> that v1 was not sufficient to entirely fix the problem.
> 
> v3 avoids needless changes to the logic and locking in the suspend and 
> shutdown code and tries to keep register writes closer to their original 
> sequence.
> 
> This patch has been tested on a PowerBook 520 but no PowerMacs yet.
> 
> 
> Index: linux-git/drivers/tty/serial/pmac_zilog.c
> ===================================================================
> --- linux-git.orig/drivers/tty/serial/pmac_zilog.c	2011-12-07 12:36:32.000000000 +1100
> +++ linux-git/drivers/tty/serial/pmac_zilog.c	2011-12-07 14:29:21.000000000 +1100
> @@ -216,6 +216,18 @@ static void pmz_maybe_update_regs(struct
>  	}
>  }
>  
> +static void pmz_interrupt_control(struct uart_pmac_port *uap, int enable)
> +{
> +	if (enable) {
> +		uap->curregs[1] |= INT_ALL_Rx | TxINT_ENAB;
> +		if (!ZS_IS_EXTCLK(uap))
> +			uap->curregs[1] |= EXT_INT_ENAB;
> +	} else {
> +		uap->curregs[1] &= ~(EXT_INT_ENAB | TxINT_ENAB | RxINT_MASK);
> +	}
> +	write_zsreg(uap, R1, uap->curregs[1]);
> +}
> +
>  static struct tty_struct *pmz_receive_chars(struct uart_pmac_port *uap)
>  {
>  	struct tty_struct *tty = NULL;
> @@ -339,9 +351,7 @@ static struct tty_struct *pmz_receive_ch
>  
>  	return tty;
>   flood:
> -	uap->curregs[R1] &= ~(EXT_INT_ENAB | TxINT_ENAB | RxINT_MASK);
> -	write_zsreg(uap, R1, uap->curregs[R1]);
> -	zssync(uap);
> +	pmz_interrupt_control(uap, 0);
>  	pmz_error("pmz: rx irq flood !\n");
>  	return tty;
>  }
> @@ -990,12 +1000,9 @@ static int pmz_startup(struct uart_port
>  	if (ZS_IS_IRDA(uap))
>  		pmz_irda_reset(uap);
>  
> -	/* Enable interrupts emission from the chip */
> +	/* Enable interrupt requests for the channel */
>  	spin_lock_irqsave(&port->lock, flags);
> -	uap->curregs[R1] |= INT_ALL_Rx | TxINT_ENAB;
> -	if (!ZS_IS_EXTCLK(uap))
> -		uap->curregs[R1] |= EXT_INT_ENAB;
> -	write_zsreg(uap, R1, uap->curregs[R1]);
> +	pmz_interrupt_control(uap, 1);
>  	spin_unlock_irqrestore(&port->lock, flags);
>  
>  	pmz_debug("pmz: startup() done.\n");
> @@ -1015,6 +1022,25 @@ static void pmz_shutdown(struct uart_por
>  
>  	mutex_lock(&pmz_irq_mutex);
>  
> +	spin_lock_irqsave(&port->lock, flags);
> +
> +	if (!ZS_IS_ASLEEP(uap)) {
> +		/* Disable interrupt requests for the channel */
> +		pmz_interrupt_control(uap, 0);
> +
> +		if (!ZS_IS_CONS(uap)) {
> +			/* Disable receiver and transmitter */
> +			uap->curregs[R3] &= ~RxENABLE;
> +			uap->curregs[R5] &= ~TxENABLE;
> +
> +			/* Disable break assertion */
> +			uap->curregs[R5] &= ~SND_BRK;
> +			pmz_maybe_update_regs(uap);
> +		}
> +	}
> +
> +	spin_unlock_irqrestore(&port->lock, flags);
> +
>  	/* Release interrupt handler */
>  	free_irq(uap->port.irq, uap);
>  
> @@ -1025,29 +1051,8 @@ static void pmz_shutdown(struct uart_por
>  	if (!ZS_IS_OPEN(uap->mate))
>  		pmz_get_port_A(uap)->flags &= ~PMACZILOG_FLAG_IS_IRQ_ON;
>  
> -	/* Disable interrupts */
> -	if (!ZS_IS_ASLEEP(uap)) {
> -		uap->curregs[R1] &= ~(EXT_INT_ENAB | TxINT_ENAB | RxINT_MASK);
> -		write_zsreg(uap, R1, uap->curregs[R1]);
> -		zssync(uap);
> -	}
> -
> -	if (ZS_IS_CONS(uap) || ZS_IS_ASLEEP(uap)) {
> -		spin_unlock_irqrestore(&port->lock, flags);
> -		mutex_unlock(&pmz_irq_mutex);
> -		return;
> -	}
> -
> -	/* Disable receiver and transmitter.  */
> -	uap->curregs[R3] &= ~RxENABLE;
> -	uap->curregs[R5] &= ~TxENABLE;
> -
> -	/* Disable all interrupts and BRK assertion.  */
> -	uap->curregs[R5] &= ~SND_BRK;
> -	pmz_maybe_update_regs(uap);
> -
> -	/* Shut the chip down */
> -	pmz_set_scc_power(uap, 0);
> +	if (!ZS_IS_ASLEEP(uap) && !ZS_IS_CONS(uap))
> +		pmz_set_scc_power(uap, 0);	/* Shut the chip down */
>  
>  	spin_unlock_irqrestore(&port->lock, flags);
>  
> @@ -1352,19 +1357,15 @@ static void pmz_set_termios(struct uart_
>  	spin_lock_irqsave(&port->lock, flags);	
>  
>  	/* Disable IRQs on the port */
> -	uap->curregs[R1] &= ~(EXT_INT_ENAB | TxINT_ENAB | RxINT_MASK);
> -	write_zsreg(uap, R1, uap->curregs[R1]);
> +	pmz_interrupt_control(uap, 0);
>  
>  	/* Setup new port configuration */
>  	__pmz_set_termios(port, termios, old);
>  
>  	/* Re-enable IRQs on the port */
> -	if (ZS_IS_OPEN(uap)) {
> -		uap->curregs[R1] |= INT_ALL_Rx | TxINT_ENAB;
> -		if (!ZS_IS_EXTCLK(uap))
> -			uap->curregs[R1] |= EXT_INT_ENAB;
> -		write_zsreg(uap, R1, uap->curregs[R1]);
> -	}
> +	if (ZS_IS_OPEN(uap))
> +		pmz_interrupt_control(uap, 1);
> +
>  	spin_unlock_irqrestore(&port->lock, flags);
>  }
>  
> @@ -1671,14 +1672,17 @@ static int pmz_suspend(struct macio_dev
>  	spin_lock_irqsave(&uap->port.lock, flags);
>  
>  	if (ZS_IS_OPEN(uap) || ZS_IS_CONS(uap)) {
> -		/* Disable receiver and transmitter.  */
> +		/* Disable interrupt requests for the channel */
> +		pmz_interrupt_control(uap, 0);
> +
> +		/* Disable receiver and transmitter */
>  		uap->curregs[R3] &= ~RxENABLE;
>  		uap->curregs[R5] &= ~TxENABLE;
>  
> -		/* Disable all interrupts and BRK assertion.  */
> -		uap->curregs[R1] &= ~(EXT_INT_ENAB | TxINT_ENAB | RxINT_MASK);
> +		/* Disable break assertion */
>  		uap->curregs[R5] &= ~SND_BRK;
>  		pmz_load_zsregs(uap, uap->curregs);
> +
>  		uap->flags |= PMACZILOG_FLAG_IS_ASLEEP;
>  		mb();
>  	}
> @@ -1738,14 +1742,6 @@ static int pmz_resume(struct macio_dev *
>  	/* Take care of config that may have changed while asleep */
>  	__pmz_set_termios(&uap->port, &uap->termios_cache, NULL);
>  
> -	if (ZS_IS_OPEN(uap)) {
> -		/* Enable interrupts */		
> -		uap->curregs[R1] |= INT_ALL_Rx | TxINT_ENAB;
> -		if (!ZS_IS_EXTCLK(uap))
> -			uap->curregs[R1] |= EXT_INT_ENAB;
> -		write_zsreg(uap, R1, uap->curregs[R1]);
> -	}
> -
>  	spin_unlock_irqrestore(&uap->port.lock, flags);
>  
>  	if (ZS_IS_CONS(uap))
> @@ -1757,6 +1753,12 @@ static int pmz_resume(struct macio_dev *
>  		enable_irq(uap->port.irq);
>  	}
>  
> +	if (ZS_IS_OPEN(uap)) {
> +		spin_lock_irqsave(&uap->port.lock, flags);
> +		pmz_interrupt_control(uap, 1);
> +		spin_unlock_irqrestore(&uap->port.lock, flags);
> +	}
> +
>   bail:
>  	mutex_unlock(&state->port.mutex);
>  	mutex_unlock(&pmz_irq_mutex);

Re: [PATCH 01/16 v3] pmac_zilog: fix unexpected irq

[Benjamin Herrenschmidt]

On Wed, 2011-12-07 at 14:49 +1100, Finn Thain wrote:
> On most 68k Macs the SCC IRQ is an autovector interrupt and cannot be 
> masked. This can be a problem when pmac_zilog starts up.
> 
> For example, the serial debugging code in arch/m68k/kernel/head.S may be 
> used beforehand. It disables the SCC interrupts at the chip but doesn't 
> ack them. Then when a pmac_zilog port is used, the machine locks up with 
> "unexpected interrupt".
> 
> This can happen in pmz_shutdown() since the irq is freed before the 
> channel interrupts are disabled.
> 
> Fix this by clearing interrupt enable bits before the handler is 
> uninstalled. Also move the interrupt control bit flipping into a separate 
> pmz_interrupt_control() routine. Replace all instances of these operations 
> with calls to this routine. Omit the zssync() calls that seem to serve no 
> purpose.
> 
> Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
> Acked-by: Alan Cox <alan@linux.intel.com>
> 
> ---

So basic operations seem to work, I've applied the patch to
powerpc-next.

However, the internal modem on my Pismo powerbook doesn't appear to
survive suspend/resume. I'll dig into that and merge a fixup patch asap.

Cheers,
Ben.

Re: [PATCH 01/16 v3] pmac_zilog: fix unexpected irq

[Benjamin Herrenschmidt]

On Thu, 2011-12-08 at 15:20 +1100, Benjamin Herrenschmidt wrote:

> So basic operations seem to work, I've applied the patch to
> powerpc-next.
> 
> However, the internal modem on my Pismo powerbook doesn't appear to
> survive suspend/resume. I'll dig into that and merge a fixup patch asap.

BTW. I applied anyway because suspend/resume was already broken (you
spotted that we don't clear the suspended flag for example).

Fixing the flag alone helps a bit. We can't use the modem if we
suspend/resume with the open port, but closing and re-opening works.

Lockdep also picked-up a A->B B->A between the port mutex and the pmz
irq mutex on suspend.

I'll try to fix all these, and will let you know (I may not have time
today).

Cheers,
Ben.

Re: [PATCH 01/16 v3] pmac_zilog: fix unexpected irq

[Finn Thain]

On Thu, 8 Dec 2011, Benjamin Herrenschmidt wrote:

> On Thu, 2011-12-08 at 15:20 +1100, Benjamin Herrenschmidt wrote:
> 
> > So basic operations seem to work, I've applied the patch to 
> > powerpc-next.

Then I guess Geert should not push this for 3.3 -- or does it make no 
difference?

> > However, the internal modem on my Pismo powerbook doesn't appear to 
> > survive suspend/resume. I'll dig into that and merge a fixup patch 
> > asap.
> 
> BTW. I applied anyway because suspend/resume was already broken (you 
> spotted that we don't clear the suspended flag for example).
> 
> Fixing the flag alone helps a bit. We can't use the modem if we 
> suspend/resume with the open port,

If the SCC IRQ counters change across suspend/resume, perhaps the modem 
itself is not powering up...

> but closing and re-opening works.

Maybe the modem wants a transition on DTR or similar, but it hasn't had 
time to initialise when that happens during SCC resumption.

If so, calling pmz_shutdown() then pmz_startup() from the tail of 
pmz_resume() without delay should probably fail to revive it...

> 
> Lockdep also picked-up a A->B B->A between the port mutex and the pmz 
> irq mutex on suspend.
> 
> I'll try to fix all these, and will let you know (I may not have time 
> today).

Thanks.

Finn

> 
> Cheers,
> Ben.
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 01/16 v3] pmac_zilog: fix unexpected irq

[Geert Uytterhoeven]

Hi Finn,

On Thu, Dec 8, 2011 at 12:26, Finn Thain <fthain@telegraphics.com.au> wrote=
:
> On Thu, 8 Dec 2011, Benjamin Herrenschmidt wrote:
>> On Thu, 2011-12-08 at 15:20 +1100, Benjamin Herrenschmidt wrote:
>> > So basic operations seem to work, I've applied the patch to
>> > powerpc-next.
>
> Then I guess Geert should not push this for 3.3 -- or does it make no
> difference?

I do not plan to push it myself, that's why it's not in my for-next branch.
The for-3.3 is just indicative.

Gr{oetje,eeting}s,

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k=
.org

In personal conversations with technical people, I call myself a hacker. Bu=
t
when I'm talking to journalists I just say "programmer" or something like t=
hat.
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0=C2=A0 =C2=A0=C2=A0 -- Linus Torvalds

Re: [PATCH 01/16 v3] pmac_zilog: fix unexpected irq

[Benjamin Herrenschmidt]

On Thu, 2011-12-08 at 22:26 +1100, Finn Thain wrote:
> 
> Maybe the modem wants a transition on DTR or similar, but it hasn't had 
> time to initialise when that happens during SCC resumption.
> 
> If so, calling pmz_shutdown() then pmz_startup() from the tail of 
> pmz_resume() without delay should probably fail to revive it... 

Well, we power the modem down and back up... but it's possible that we
fail to re-enable something, I'll check. That used to work (at least
with macserial, maybe I never tried this specific torture with pmz...).
I'll figure it out eventually.

Cheers,
Ben.

Re: [PATCH 01/16 v3] pmac_zilog: fix unexpected irq

[Benjamin Herrenschmidt]

Any chance you can test this patch ? I would not be surprised if it
broke m68k since I had to do some of the changes in there "blind",
so let me know... with this, I can again suspend/resume properly on
a Pismo while using the internal modem among other things.

>From c2dbe7117bb94c59a4b2a215fc87fe7eabb7658d Mon Sep 17 00:00:00 2001
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: Mon, 12 Dec 2011 10:44:08 +1100
Subject: [PATCH 2/2] tty/serial/pmac_zilog: Fix suspend & resume

This patch reworks & simplifies pmac_zilog handling of suspend/resume,
essentially removing all the specific code in there and using the
generic uart helpers.

This required properly registering the tty as a child of the macio (or platform)
device, so I had to delay the registration a bit (we used to register the ports
very very early). We still register the kernel console early though.

I removed a couple of unused or useless flags as well, relying on the
core to not call us when asleep. I also removed the essentially useless
interrupt mutex, simplifying the locking a bit.

I removed some code for handling unexpected interrupt which should never
be hit and could potentially be harmful (causing us to access a register
on a powered off SCC). We diable port interrupts on close always so there
should be no need to drain data on a closed port.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 drivers/tty/serial/pmac_zilog.c |  350 ++++++++++-----------------------------
 drivers/tty/serial/pmac_zilog.h |   18 +-
 2 files changed, 99 insertions(+), 269 deletions(-)

diff --git a/drivers/tty/serial/pmac_zilog.c b/drivers/tty/serial/pmac_zilog.c
index 51941f0..46cb39b 100644
--- a/drivers/tty/serial/pmac_zilog.c
+++ b/drivers/tty/serial/pmac_zilog.c
@@ -106,7 +106,6 @@ MODULE_LICENSE("GPL");
  */
 static struct uart_pmac_port	pmz_ports[MAX_ZS_PORTS];
 static int			pmz_ports_count;
-static DEFINE_MUTEX(pmz_irq_mutex);
 
 static struct uart_driver pmz_uart_reg = {
 	.owner		=	THIS_MODULE,
@@ -126,9 +125,6 @@ static void pmz_load_zsregs(struct uart_pmac_port *uap, u8 *regs)
 {
 	int i;
 
-	if (ZS_IS_ASLEEP(uap))
-		return;
-
 	/* Let pending transmits finish.  */
 	for (i = 0; i < 1000; i++) {
 		unsigned char stat = read_zsreg(uap, R1);
@@ -234,26 +230,6 @@ static struct tty_struct *pmz_receive_chars(struct uart_pmac_port *uap)
 	unsigned char ch, r1, drop, error, flag;
 	int loops = 0;
 
-	/* The interrupt can be enabled when the port isn't open, typically
-	 * that happens when using one port is open and the other closed (stale
-	 * interrupt) or when one port is used as a console.
-	 */
-	if (!ZS_IS_OPEN(uap)) {
-		pmz_debug("pmz: draining input\n");
-		/* Port is closed, drain input data */
-		for (;;) {
-			if ((++loops) > 1000)
-				goto flood;
-			(void)read_zsreg(uap, R1);
-			write_zsreg(uap, R0, ERR_RES);
-			(void)read_zsdata(uap);
-			ch = read_zsreg(uap, R0);
-			if (!(ch & Rx_CH_AV))
-				break;
-		}
-		return NULL;
-	}
-
 	/* Sanity check, make sure the old bug is no longer happening */
 	if (uap->port.state == NULL || uap->port.state->port.tty == NULL) {
 		WARN_ON(1);
@@ -393,8 +369,6 @@ static void pmz_transmit_chars(struct uart_pmac_port *uap)
 {
 	struct circ_buf *xmit;
 
-	if (ZS_IS_ASLEEP(uap))
-		return;
 	if (ZS_IS_CONS(uap)) {
 		unsigned char status = read_zsreg(uap, R0);
 
@@ -485,12 +459,16 @@ static irqreturn_t pmz_interrupt(int irq, void *dev_id)
 	spin_lock(&uap_a->port.lock);
 	r3 = read_zsreg(uap_a, R3);
 
-#ifdef DEBUG_HARD
+#ifde DEBUG_HARD
 	pmz_debug("irq, r3: %x\n", r3);
 #endif
 	/* Channel A */
 	tty = NULL;
 	if (r3 & (CHAEXT | CHATxIP | CHARxIP)) {
+		if (!ZS_IS_OPEN(uap_a)) {
+			pmz_debug("ChanA interrupt while open !\n");
+			goto skip_a;
+		}
 		write_zsreg(uap_a, R0, RES_H_IUS);
 		zssync(uap_a);		
 		if (r3 & CHAEXT)
@@ -501,16 +479,21 @@ static irqreturn_t pmz_interrupt(int irq, void *dev_id)
 			pmz_transmit_chars(uap_a);
 		rc = IRQ_HANDLED;
 	}
+ skip_a:
 	spin_unlock(&uap_a->port.lock);
 	if (tty != NULL)
 		tty_flip_buffer_push(tty);
 
-	if (uap_b->node == NULL)
+	if (!uap_b)
 		goto out;
 
 	spin_lock(&uap_b->port.lock);
 	tty = NULL;
 	if (r3 & (CHBEXT | CHBTxIP | CHBRxIP)) {
+		if (!ZS_IS_OPEN(uap_a)) {
+			pmz_debug("ChanB interrupt while open !\n");
+			goto skip_b;
+		}
 		write_zsreg(uap_b, R0, RES_H_IUS);
 		zssync(uap_b);
 		if (r3 & CHBEXT)
@@ -521,14 +504,12 @@ static irqreturn_t pmz_interrupt(int irq, void *dev_id)
 			pmz_transmit_chars(uap_b);
 		rc = IRQ_HANDLED;
 	}
+ skip_b:
 	spin_unlock(&uap_b->port.lock);
 	if (tty != NULL)
 		tty_flip_buffer_push(tty);
 
  out:
-#ifdef DEBUG_HARD
-	pmz_debug("irq done.\n");
-#endif
 	return rc;
 }
 
@@ -553,12 +534,8 @@ static inline u8 pmz_peek_status(struct uart_pmac_port *uap)
  */
 static unsigned int pmz_tx_empty(struct uart_port *port)
 {
-	struct uart_pmac_port *uap = to_pmz(port);
 	unsigned char status;
 
-	if (ZS_IS_ASLEEP(uap) || uap->node == NULL)
-		return TIOCSER_TEMT;
-
 	status = pmz_peek_status(to_pmz(port));
 	if (status & Tx_BUF_EMP)
 		return TIOCSER_TEMT;
@@ -580,8 +557,7 @@ static void pmz_set_mctrl(struct uart_port *port, unsigned int mctrl)
 	if (ZS_IS_IRDA(uap))
 		return;
 	/* We get called during boot with a port not up yet */
-	if (ZS_IS_ASLEEP(uap) ||
-	    !(ZS_IS_OPEN(uap) || ZS_IS_CONS(uap)))
+	if (!(ZS_IS_OPEN(uap) || ZS_IS_CONS(uap)))
 		return;
 
 	set_bits = clear_bits = 0;
@@ -600,8 +576,7 @@ static void pmz_set_mctrl(struct uart_port *port, unsigned int mctrl)
 	/* NOTE: Not subject to 'transmitter active' rule.  */ 
 	uap->curregs[R5] |= set_bits;
 	uap->curregs[R5] &= ~clear_bits;
-	if (ZS_IS_ASLEEP(uap))
-		return;
+
 	write_zsreg(uap, R5, uap->curregs[R5]);
 	pmz_debug("pmz_set_mctrl: set bits: %x, clear bits: %x -> %x\n",
 		  set_bits, clear_bits, uap->curregs[R5]);
@@ -619,9 +594,6 @@ static unsigned int pmz_get_mctrl(struct uart_port *port)
 	unsigned char status;
 	unsigned int ret;
 
-	if (ZS_IS_ASLEEP(uap) || uap->node == NULL)
-		return 0;
-
 	status = read_zsreg(uap, R0);
 
 	ret = 0;
@@ -659,9 +631,6 @@ static void pmz_start_tx(struct uart_port *port)
 	uap->flags |= PMACZILOG_FLAG_TX_ACTIVE;
 	uap->flags &= ~PMACZILOG_FLAG_TX_STOPPED;
 
-	if (ZS_IS_ASLEEP(uap) || uap->node == NULL)
-		return;
-
 	status = read_zsreg(uap, R0);
 
 	/* TX busy?  Just wait for the TX done interrupt.  */
@@ -700,9 +669,6 @@ static void pmz_stop_rx(struct uart_port *port)
 {
 	struct uart_pmac_port *uap = to_pmz(port);
 
-	if (ZS_IS_ASLEEP(uap) || uap->node == NULL)
-		return;
-
 	pmz_debug("pmz: stop_rx()()\n");
 
 	/* Disable all RX interrupts.  */
@@ -721,14 +687,12 @@ static void pmz_enable_ms(struct uart_port *port)
 	struct uart_pmac_port *uap = to_pmz(port);
 	unsigned char new_reg;
 
-	if (ZS_IS_IRDA(uap) || uap->node == NULL)
+	if (ZS_IS_IRDA(uap))
 		return;
 	new_reg = uap->curregs[R15] | (DCDIE | SYNCIE | CTSIE);
 	if (new_reg != uap->curregs[R15]) {
 		uap->curregs[R15] = new_reg;
 
-		if (ZS_IS_ASLEEP(uap))
-			return;
 		/* NOTE: Not subject to 'transmitter active' rule. */
 		write_zsreg(uap, R15, uap->curregs[R15]);
 	}
@@ -744,8 +708,6 @@ static void pmz_break_ctl(struct uart_port *port, int break_state)
 	unsigned char set_bits, clear_bits, new_reg;
 	unsigned long flags;
 
-	if (uap->node == NULL)
-		return;
 	set_bits = clear_bits = 0;
 
 	if (break_state)
@@ -758,12 +720,6 @@ static void pmz_break_ctl(struct uart_port *port, int break_state)
 	new_reg = (uap->curregs[R5] | set_bits) & ~clear_bits;
 	if (new_reg != uap->curregs[R5]) {
 		uap->curregs[R5] = new_reg;
-
-		/* NOTE: Not subject to 'transmitter active' rule. */
-		if (ZS_IS_ASLEEP(uap)) {
-			spin_unlock_irqrestore(&port->lock, flags);
-			return;
-		}
 		write_zsreg(uap, R5, uap->curregs[R5]);
 	}
 
@@ -937,14 +893,21 @@ static int __pmz_startup(struct uart_pmac_port *uap)
 
 static void pmz_irda_reset(struct uart_pmac_port *uap)
 {
+	unsigned long flags;
+
+	spin_lock_irqsave(&uap->port.lock, flags);
 	uap->curregs[R5] |= DTR;
 	write_zsreg(uap, R5, uap->curregs[R5]);
 	zssync(uap);
-	mdelay(110);
+	spin_unlock_irqrestore(&uap->port.lock, flags);
+	msleep(110);
+
+	spin_lock_irqsave(&uap->port.lock, flags);
 	uap->curregs[R5] &= ~DTR;
 	write_zsreg(uap, R5, uap->curregs[R5]);
 	zssync(uap);
-	mdelay(10);
+	spin_unlock_irqrestore(&uap->port.lock, flags);
+	msleep(10);
 }
 
 /*
@@ -959,13 +922,6 @@ static int pmz_startup(struct uart_port *port)
 
 	pmz_debug("pmz: startup()\n");
 
-	if (ZS_IS_ASLEEP(uap))
-		return -EAGAIN;
-	if (uap->node == NULL)
-		return -ENODEV;
-
-	mutex_lock(&pmz_irq_mutex);
-
 	uap->flags |= PMACZILOG_FLAG_IS_OPEN;
 
 	/* A console is never powered down. Else, power up and
@@ -976,18 +932,14 @@ static int pmz_startup(struct uart_port *port)
 		pwr_delay = __pmz_startup(uap);
 		spin_unlock_irqrestore(&port->lock, flags);
 	}	
-
-	pmz_get_port_A(uap)->flags |= PMACZILOG_FLAG_IS_IRQ_ON;
+	sprintf(uap->irq_name, PMACZILOG_NAME"%d", uap->port.line);
 	if (request_irq(uap->port.irq, pmz_interrupt, IRQF_SHARED,
-			"SCC", uap)) {
+			uap->irq_name, uap)) {
 		pmz_error("Unable to register zs interrupt handler.\n");
 		pmz_set_scc_power(uap, 0);
-		mutex_unlock(&pmz_irq_mutex);
 		return -ENXIO;
 	}
 
-	mutex_unlock(&pmz_irq_mutex);
-
 	/* Right now, we deal with delay by blocking here, I'll be
 	 * smarter later on
 	 */
@@ -1017,26 +969,19 @@ static void pmz_shutdown(struct uart_port *port)
 
 	pmz_debug("pmz: shutdown()\n");
 
-	if (uap->node == NULL)
-		return;
-
-	mutex_lock(&pmz_irq_mutex);
-
 	spin_lock_irqsave(&port->lock, flags);
 
-	if (!ZS_IS_ASLEEP(uap)) {
-		/* Disable interrupt requests for the channel */
-		pmz_interrupt_control(uap, 0);
+	/* Disable interrupt requests for the channel */
+	pmz_interrupt_control(uap, 0);
 
-		if (!ZS_IS_CONS(uap)) {
-			/* Disable receiver and transmitter */
-			uap->curregs[R3] &= ~RxENABLE;
-			uap->curregs[R5] &= ~TxENABLE;
+	if (!ZS_IS_CONS(uap)) {
+		/* Disable receiver and transmitter */
+		uap->curregs[R3] &= ~RxENABLE;
+		uap->curregs[R5] &= ~TxENABLE;
 
-			/* Disable break assertion */
-			uap->curregs[R5] &= ~SND_BRK;
-			pmz_maybe_update_regs(uap);
-		}
+		/* Disable break assertion */
+		uap->curregs[R5] &= ~SND_BRK;
+		pmz_maybe_update_regs(uap);
 	}
 
 	spin_unlock_irqrestore(&port->lock, flags);
@@ -1048,16 +993,11 @@ static void pmz_shutdown(struct uart_port *port)
 
 	uap->flags &= ~PMACZILOG_FLAG_IS_OPEN;
 
-	if (!ZS_IS_OPEN(uap->mate))
-		pmz_get_port_A(uap)->flags &= ~PMACZILOG_FLAG_IS_IRQ_ON;
-
-	if (!ZS_IS_ASLEEP(uap) && !ZS_IS_CONS(uap))
+	if (!ZS_IS_CONS(uap))
 		pmz_set_scc_power(uap, 0);	/* Shut the chip down */
 
 	spin_unlock_irqrestore(&port->lock, flags);
 
-	mutex_unlock(&pmz_irq_mutex);
-
 	pmz_debug("pmz: shutdown() done.\n");
 }
 
@@ -1305,9 +1245,6 @@ static void __pmz_set_termios(struct uart_port *port, struct ktermios *termios,
 
 	pmz_debug("pmz: set_termios()\n");
 
-	if (ZS_IS_ASLEEP(uap))
-		return;
-
 	memcpy(&uap->termios_cache, termios, sizeof(struct ktermios));
 
 	/* XXX Check which revs of machines actually allow 1 and 4Mb speeds
@@ -1605,25 +1542,34 @@ static void pmz_dispose_port(struct uart_pmac_port *uap)
  */
 static int pmz_attach(struct macio_dev *mdev, const struct of_device_id *match)
 {
+	struct uart_pmac_port *uap;
 	int i;
 	
 	/* Iterate the pmz_ports array to find a matching entry
 	 */
 	for (i = 0; i < MAX_ZS_PORTS; i++)
-		if (pmz_ports[i].node == mdev->ofdev.dev.of_node) {
-			struct uart_pmac_port *uap = &pmz_ports[i];
-
-			uap->dev = mdev;
-			dev_set_drvdata(&mdev->ofdev.dev, uap);
-			if (macio_request_resources(uap->dev, "pmac_zilog"))
-				printk(KERN_WARNING "%s: Failed to request resource"
-				       ", port still active\n",
-				       uap->node->name);
-			else
-				uap->flags |= PMACZILOG_FLAG_RSRC_REQUESTED;				
-			return 0;
-		}
-	return -ENODEV;
+		if (pmz_ports[i].node == mdev->ofdev.dev.of_node)
+			break;
+	if (i >= MAX_ZS_PORTS)
+		return -ENODEV;
+
+
+	uap = &pmz_ports[i];
+	uap->dev = mdev;
+	uap->port.dev = &mdev->ofdev.dev;
+	dev_set_drvdata(&mdev->ofdev.dev, uap);
+
+	/* We still activate the port even when failing to request resources
+	 * to work around bugs in ancient Apple device-trees
+	 */
+	if (macio_request_resources(uap->dev, "pmac_zilog"))
+		printk(KERN_WARNING "%s: Failed to request resource"
+		       ", port still active\n",
+		       uap->node->name);
+	else
+		uap->flags |= PMACZILOG_FLAG_RSRC_REQUESTED;				
+
+	return uart_add_one_port(&pmz_uart_reg, &uap->port);
 }
 
 /*
@@ -1637,12 +1583,15 @@ static int pmz_detach(struct macio_dev *mdev)
 	if (!uap)
 		return -ENODEV;
 
+	uart_remove_one_port(&pmz_uart_reg, &uap->port);
+
 	if (uap->flags & PMACZILOG_FLAG_RSRC_REQUESTED) {
 		macio_release_resources(uap->dev);
 		uap->flags &= ~PMACZILOG_FLAG_RSRC_REQUESTED;
 	}
 	dev_set_drvdata(&mdev->ofdev.dev, NULL);
 	uap->dev = NULL;
+	uap->port.dev = NULL;
 	
 	return 0;
 }
@@ -1651,62 +1600,13 @@ static int pmz_detach(struct macio_dev *mdev)
 static int pmz_suspend(struct macio_dev *mdev, pm_message_t pm_state)
 {
 	struct uart_pmac_port *uap = dev_get_drvdata(&mdev->ofdev.dev);
-	struct uart_state *state;
-	unsigned long flags;
 
 	if (uap == NULL) {
 		printk("HRM... pmz_suspend with NULL uap\n");
 		return 0;
 	}
 
-	if (pm_state.event == mdev->ofdev.dev.power.power_state.event)
-		return 0;
-
-	pmz_debug("suspend, switching to state %d\n", pm_state.event);
-
-	state = pmz_uart_reg.state + uap->port.line;
-
-	mutex_lock(&pmz_irq_mutex);
-	mutex_lock(&state->port.mutex);
-
-	spin_lock_irqsave(&uap->port.lock, flags);
-
-	if (ZS_IS_OPEN(uap) || ZS_IS_CONS(uap)) {
-		/* Disable interrupt requests for the channel */
-		pmz_interrupt_control(uap, 0);
-
-		/* Disable receiver and transmitter */
-		uap->curregs[R3] &= ~RxENABLE;
-		uap->curregs[R5] &= ~TxENABLE;
-
-		/* Disable break assertion */
-		uap->curregs[R5] &= ~SND_BRK;
-		pmz_load_zsregs(uap, uap->curregs);
-
-		uap->flags |= PMACZILOG_FLAG_IS_ASLEEP;
-		mb();
-	}
-
-	spin_unlock_irqrestore(&uap->port.lock, flags);
-
-	if (ZS_IS_OPEN(uap) || ZS_IS_OPEN(uap->mate))
-		if (ZS_IS_ASLEEP(uap->mate) && ZS_IS_IRQ_ON(pmz_get_port_A(uap))) {
-			pmz_get_port_A(uap)->flags &= ~PMACZILOG_FLAG_IS_IRQ_ON;
-			disable_irq(uap->port.irq);
-		}
-
-	if (ZS_IS_CONS(uap))
-		uap->port.cons->flags &= ~CON_ENABLED;
-
-	/* Shut the chip down */
-	pmz_set_scc_power(uap, 0);
-
-	mutex_unlock(&state->port.mutex);
-	mutex_unlock(&pmz_irq_mutex);
-
-	pmz_debug("suspend, switching complete\n");
-
-	mdev->ofdev.dev.power.power_state = pm_state;
+	uart_suspend_port(&pmz_uart_reg, &uap->port);
 
 	return 0;
 }
@@ -1715,74 +1615,20 @@ static int pmz_suspend(struct macio_dev *mdev, pm_message_t pm_state)
 static int pmz_resume(struct macio_dev *mdev)
 {
 	struct uart_pmac_port *uap = dev_get_drvdata(&mdev->ofdev.dev);
-	struct uart_state *state;
-	unsigned long flags;
-	int pwr_delay = 0;
 
 	if (uap == NULL)
 		return 0;
 
-	if (mdev->ofdev.dev.power.power_state.event == PM_EVENT_ON)
-		return 0;
-	
-	pmz_debug("resume, switching to state 0\n");
-
-	state = pmz_uart_reg.state + uap->port.line;
-
-	mutex_lock(&pmz_irq_mutex);
-	mutex_lock(&state->port.mutex);
-
-	spin_lock_irqsave(&uap->port.lock, flags);
-	if (!ZS_IS_OPEN(uap) && !ZS_IS_CONS(uap)) {
-		spin_unlock_irqrestore(&uap->port.lock, flags);
-		goto bail;
-	}
-	pwr_delay = __pmz_startup(uap);
-
-	/* Take care of config that may have changed while asleep */
-	__pmz_set_termios(&uap->port, &uap->termios_cache, NULL);
-
-	spin_unlock_irqrestore(&uap->port.lock, flags);
-
-	if (ZS_IS_CONS(uap))
-		uap->port.cons->flags |= CON_ENABLED;
-
-	/* Re-enable IRQ on the controller */
-	if (ZS_IS_OPEN(uap) && !ZS_IS_IRQ_ON(pmz_get_port_A(uap))) {
-		pmz_get_port_A(uap)->flags |= PMACZILOG_FLAG_IS_IRQ_ON;
-		enable_irq(uap->port.irq);
-	}
-
-	if (ZS_IS_OPEN(uap)) {
-		spin_lock_irqsave(&uap->port.lock, flags);
-		pmz_interrupt_control(uap, 1);
-		spin_unlock_irqrestore(&uap->port.lock, flags);
-	}
-
- bail:
-	mutex_unlock(&state->port.mutex);
-	mutex_unlock(&pmz_irq_mutex);
-
-	/* Right now, we deal with delay by blocking here, I'll be
-	 * smarter later on
-	 */
-	if (pwr_delay != 0) {
-		pmz_debug("pmz: delaying %d ms\n", pwr_delay);
-		msleep(pwr_delay);
-	}
-
-	pmz_debug("resume, switching complete\n");
-
-	mdev->ofdev.dev.power.power_state.event = PM_EVENT_ON;
+	uart_resume_port(&pmz_uart_reg, &uap->port);
 
 	return 0;
 }
 
 /*
  * Probe all ports in the system and build the ports array, we register
- * with the serial layer at this point, the macio-type probing is only
- * used later to "attach" to the sysfs tree so we get power management
- * events
+ * with the serial layer later, so we get a proper struct device which
+ * allows the tty to attach properly. This is later than it used to be
+ * but the tty layer really wants it that way.
  */
 static int __init pmz_probe(void)
 {
@@ -1818,8 +1664,10 @@ static int __init pmz_probe(void)
 		/*
 		 * Fill basic fields in the port structures
 		 */
-		pmz_ports[count].mate		= &pmz_ports[count+1];
-		pmz_ports[count+1].mate		= &pmz_ports[count];
+		if (node_b != NULL) {
+			pmz_ports[count].mate		= &pmz_ports[count+1];
+			pmz_ports[count+1].mate		= &pmz_ports[count];
+		}
 		pmz_ports[count].flags		= PMACZILOG_FLAG_IS_CHANNEL_A;
 		pmz_ports[count].node		= node_a;
 		pmz_ports[count+1].node		= node_b;
@@ -1887,19 +1735,19 @@ static int __init pmz_probe(void)
 
 	pmz_ports_count = 0;
 
-	pmz_ports[0].mate      = &pmz_ports[1];
 	pmz_ports[0].port.line = 0;
 	pmz_ports[0].flags     = PMACZILOG_FLAG_IS_CHANNEL_A;
-	pmz_ports[0].node      = &scc_a_pdev;
+	pmz_ports[0].pdev      = &scc_a_pdev;
 	err = pmz_init_port(&pmz_ports[0]);
 	if (err)
 		return err;
 	pmz_ports_count++;
 
+	pmz_ports[0].mate      = &pmz_ports[1];
 	pmz_ports[1].mate      = &pmz_ports[0];
 	pmz_ports[1].port.line = 1;
 	pmz_ports[1].flags     = 0;
-	pmz_ports[1].node      = &scc_b_pdev;
+	pmz_ports[1].pdev      = &scc_b_pdev;
 	err = pmz_init_port(&pmz_ports[1]);
 	if (err)
 		return err;
@@ -1918,13 +1766,22 @@ static int __init pmz_attach(struct platform_device *pdev)
 	int i;
 
 	for (i = 0; i < pmz_ports_count; i++)
-		if (pmz_ports[i].node == pdev)
-			return 0;
-	return -ENODEV;
+		if (pmz_ports[i].pdev == pdev)
+			break;
+	if (i >= pmz_ports_count)
+		return -ENODEV;
+
+	uap = &pmz_ports[i];
+	uap->port.dev = &pdev->dev;
+	dev_set_drvdata(&mdev->ofdev.dev, uap);
+
+			return uart_add_one_port(&pmz_uart_reg,
+						 &pmz_ports[i]->port);
 }
 
 static int __exit pmz_detach(struct platform_device *pdev)
 {
+	uart_remove_one_port(&pmz_uart_reg, &uap->port);
 	return 0;
 }
 
@@ -1956,38 +1813,13 @@ static struct console pmz_console = {
  */
 static int __init pmz_register(void)
 {
-	int i, rc;
-	
 	pmz_uart_reg.nr = pmz_ports_count;
 	pmz_uart_reg.cons = PMACZILOG_CONSOLE;
 
 	/*
 	 * Register this driver with the serial core
 	 */
-	rc = uart_register_driver(&pmz_uart_reg);
-	if (rc)
-		return rc;
-
-	/*
-	 * Register each port with the serial core
-	 */
-	for (i = 0; i < pmz_ports_count; i++) {
-		struct uart_pmac_port *uport = &pmz_ports[i];
-		/* NULL node may happen on wallstreet */
-		if (uport->node != NULL)
-			rc = uart_add_one_port(&pmz_uart_reg, &uport->port);
-		if (rc)
-			goto err_out;
-	}
-
-	return 0;
-err_out:
-	while (i-- > 0) {
-		struct uart_pmac_port *uport = &pmz_ports[i];
-		uart_remove_one_port(&pmz_uart_reg, &uport->port);
-	}
-	uart_unregister_driver(&pmz_uart_reg);
-	return rc;
+	return uart_register_driver(&pmz_uart_reg);
 }
 
 #ifdef CONFIG_PPC_PMAC
@@ -2086,10 +1918,8 @@ static void __exit exit_pmz(void)
 
 	for (i = 0; i < pmz_ports_count; i++) {
 		struct uart_pmac_port *uport = &pmz_ports[i];
-		if (uport->node != NULL) {
-			uart_remove_one_port(&pmz_uart_reg, &uport->port);
+		if (uport->node != NULL)
 			pmz_dispose_port(uport);
-		}
 	}
 	/* Unregister UART driver */
 	uart_unregister_driver(&pmz_uart_reg);
@@ -2116,8 +1946,6 @@ static void pmz_console_write(struct console *con, const char *s, unsigned int c
 	struct uart_pmac_port *uap = &pmz_ports[con->index];
 	unsigned long flags;
 
-	if (ZS_IS_ASLEEP(uap))
-		return;
 	spin_lock_irqsave(&uap->port.lock, flags);
 
 	/* Turn of interrupts and enable the transmitter. */
@@ -2162,8 +1990,10 @@ static int __init pmz_console_setup(struct console *co, char *options)
 	if (co->index >= pmz_ports_count)
 		co->index = 0;
 	uap = &pmz_ports[co->index];
+#ifdef CONFIG_PPC_PMAC
 	if (uap->node == NULL)
 		return -ENODEV;
+#endif
 	port = &uap->port;
 
 	/*
diff --git a/drivers/tty/serial/pmac_zilog.h b/drivers/tty/serial/pmac_zilog.h
index cbc34fb..9ae4556 100644
--- a/drivers/tty/serial/pmac_zilog.h
+++ b/drivers/tty/serial/pmac_zilog.h
@@ -2,9 +2,12 @@
 #define __PMAC_ZILOG_H__
 
 #ifdef CONFIG_PPC_PMAC
-#define pmz_debug(fmt, arg...)	dev_dbg(&uap->dev->ofdev.dev, fmt, ## arg)
-#define pmz_error(fmt, arg...)	dev_err(&uap->dev->ofdev.dev, fmt, ## arg)
-#define pmz_info(fmt, arg...)	dev_info(&uap->dev->ofdev.dev, fmt, ## arg)
+/* We cannot use dev_* because this can be called early, way before
+ * we are matched with a device (when using it as a kernel console)
+ */
+#define pmz_debug(fmt, arg...)	pr_debug("ttyPZ%d: " fmt, uap->port.line, ## arg)
+#define pmz_error(fmt, arg...)	pr_err("ttyPZ%d: " fmt, uap->port.line, ## arg)
+#define pmz_info(fmt, arg...)	pr_info("ttyPZ%d: " fmt, uap->port.line, ## arg)
 #else
 #define pmz_debug(fmt, arg...)	dev_dbg(&uap->node->dev, fmt, ## arg)
 #define pmz_error(fmt, arg...)	dev_err(&uap->node->dev, fmt, ## arg)
@@ -35,7 +38,7 @@ struct uart_pmac_port {
 	 */
 	struct device_node		*node;
 #else
-	struct platform_device		*node;
+	struct platform_device		*pdev;
 #endif
 
 	/* Port type as obtained from device tree (IRDA, modem, ...) */
@@ -50,14 +53,11 @@ struct uart_pmac_port {
 #define PMACZILOG_FLAG_REGS_HELD	0x00000010
 #define PMACZILOG_FLAG_TX_STOPPED	0x00000020
 #define PMACZILOG_FLAG_TX_ACTIVE	0x00000040
-#define PMACZILOG_FLAG_ENABLED          0x00000080
 #define PMACZILOG_FLAG_IS_IRDA		0x00000100
 #define PMACZILOG_FLAG_IS_INTMODEM	0x00000200
 #define PMACZILOG_FLAG_HAS_DMA		0x00000400
 #define PMACZILOG_FLAG_RSRC_REQUESTED	0x00000800
-#define PMACZILOG_FLAG_IS_ASLEEP	0x00001000
 #define PMACZILOG_FLAG_IS_OPEN		0x00002000
-#define PMACZILOG_FLAG_IS_IRQ_ON	0x00004000
 #define PMACZILOG_FLAG_IS_EXTCLK	0x00008000
 #define PMACZILOG_FLAG_BREAK		0x00010000
 
@@ -74,6 +74,8 @@ struct uart_pmac_port {
 	volatile struct dbdma_regs	__iomem *rx_dma_regs;
 #endif
 
+	unsigned char			irq_name[8];
+
 	struct ktermios			termios_cache;
 };
 
@@ -388,9 +390,7 @@ static inline void zssync(struct uart_pmac_port *port)
 #define ZS_IS_IRDA(UP)			((UP)->flags & PMACZILOG_FLAG_IS_IRDA)
 #define ZS_IS_INTMODEM(UP)		((UP)->flags & PMACZILOG_FLAG_IS_INTMODEM)
 #define ZS_HAS_DMA(UP)			((UP)->flags & PMACZILOG_FLAG_HAS_DMA)
-#define ZS_IS_ASLEEP(UP)		((UP)->flags & PMACZILOG_FLAG_IS_ASLEEP)
 #define ZS_IS_OPEN(UP)			((UP)->flags & PMACZILOG_FLAG_IS_OPEN)
-#define ZS_IS_IRQ_ON(UP)		((UP)->flags & PMACZILOG_FLAG_IS_IRQ_ON)
 #define ZS_IS_EXTCLK(UP)		((UP)->flags & PMACZILOG_FLAG_IS_EXTCLK)
 
 #endif /* __PMAC_ZILOG_H__ */
-- 
1.7.7.3

Re: [PATCH 01/16 v3] pmac_zilog: fix unexpected irq

[Benjamin Herrenschmidt]

On Mon, 2011-12-12 at 10:48 +1100, Benjamin Herrenschmidt wrote:
> Any chance you can test this patch ? I would not be surprised if it
> broke m68k since I had to do some of the changes in there "blind",
> so let me know... with this, I can again suspend/resume properly on
> a Pismo while using the internal modem among other things.

 ..../....

Forgot to commit a fix before sending, but it's a trivial one:

> -#ifdef DEBUG_HARD
> +#ifde DEBUG_HARD
>  	pmz_debug("irq, r3: %x\n", r3);
>  #endif

Remove that hunk :-)

Cheers,
Ben.

Re: [PATCH 01/16 v3] pmac_zilog: fix unexpected irq

[Finn Thain]

On Mon, 12 Dec 2011, Benjamin Herrenschmidt wrote:

> Any chance you can test this patch ? I would not be surprised if it 
> broke m68k since I had to do some of the changes in there "blind", so 
> let me know... with this, I can again suspend/resume properly on a Pismo 
> while using the internal modem among other things.

The patch works on a PowerBook 520 given a few changes (below). This 
PowerBook only has one serial port that I can test (the internal modem is 
not supported on 68k Macs). Can you test a machine with two ports? The 
rest of my Mac hardware is in storage since I moved house last week.

Finn


Index: linux-git/drivers/tty/serial/pmac_zilog.c
===================================================================
--- linux-git.orig/drivers/tty/serial/pmac_zilog.c	2011-12-13 00:18:02.000000000 +1100
+++ linux-git/drivers/tty/serial/pmac_zilog.c	2011-12-13 00:23:55.000000000 +1100
@@ -1705,8 +1705,8 @@ static int __init pmz_init_port(struct u
 	struct resource *r_ports;
 	int irq;
 
-	r_ports = platform_get_resource(uap->node, IORESOURCE_MEM, 0);
-	irq = platform_get_irq(uap->node, 0);
+	r_ports = platform_get_resource(uap->pdev, IORESOURCE_MEM, 0);
+	irq = platform_get_irq(uap->pdev, 0);
 	if (!r_ports || !irq)
 		return -ENODEV;
 
@@ -1763,8 +1763,10 @@ static void pmz_dispose_port(struct uart
 
 static int __init pmz_attach(struct platform_device *pdev)
 {
+	struct uart_pmac_port *uap;
 	int i;
 
+	/* Iterate the pmz_ports array to find a matching entry */
 	for (i = 0; i < pmz_ports_count; i++)
 		if (pmz_ports[i].pdev == pdev)
 			break;
@@ -1773,15 +1775,23 @@ static int __init pmz_attach(struct plat
 
 	uap = &pmz_ports[i];
 	uap->port.dev = &pdev->dev;
-	dev_set_drvdata(&mdev->ofdev.dev, uap);
+	platform_set_drvdata(pdev, uap);
 
-			return uart_add_one_port(&pmz_uart_reg,
-						 &pmz_ports[i]->port);
+	return uart_add_one_port(&pmz_uart_reg, &uap->port);
 }
 
 static int __exit pmz_detach(struct platform_device *pdev)
 {
+	struct uart_pmac_port *uap = platform_get_drvdata(pdev);
+
+	if (!uap)
+		return -ENODEV;
+
 	uart_remove_one_port(&pmz_uart_reg, &uap->port);
+
+	platform_set_drvdata(pdev, NULL);
+	uap->port.dev = NULL;
+
 	return 0;
 }
 
@@ -1918,8 +1928,13 @@ static void __exit exit_pmz(void)
 
 	for (i = 0; i < pmz_ports_count; i++) {
 		struct uart_pmac_port *uport = &pmz_ports[i];
+#ifdef CONFIG_PPC_PMAC
 		if (uport->node != NULL)
 			pmz_dispose_port(uport);
+#else
+		if (uport->pdev != NULL)
+			pmz_dispose_port(uport);
+#endif
 	}
 	/* Unregister UART driver */
 	uart_unregister_driver(&pmz_uart_reg);
@@ -1993,6 +2008,9 @@ static int __init pmz_console_setup(stru
 #ifdef CONFIG_PPC_PMAC
 	if (uap->node == NULL)
 		return -ENODEV;
+#else
+	if (uap->pdev == NULL)
+		return -ENODEV;
 #endif
 	port = &uap->port;
 
Index: linux-git/drivers/tty/serial/pmac_zilog.h
===================================================================
--- linux-git.orig/drivers/tty/serial/pmac_zilog.h	2011-12-13 00:18:02.000000000 +1100
+++ linux-git/drivers/tty/serial/pmac_zilog.h	2011-12-13 00:23:55.000000000 +1100
@@ -1,18 +1,9 @@
 #ifndef __PMAC_ZILOG_H__
 #define __PMAC_ZILOG_H__
 
-#ifdef CONFIG_PPC_PMAC
-/* We cannot use dev_* because this can be called early, way before
- * we are matched with a device (when using it as a kernel console)
- */
 #define pmz_debug(fmt, arg...)	pr_debug("ttyPZ%d: " fmt, uap->port.line, ## arg)
 #define pmz_error(fmt, arg...)	pr_err("ttyPZ%d: " fmt, uap->port.line, ## arg)
 #define pmz_info(fmt, arg...)	pr_info("ttyPZ%d: " fmt, uap->port.line, ## arg)
-#else
-#define pmz_debug(fmt, arg...)	dev_dbg(&uap->node->dev, fmt, ## arg)
-#define pmz_error(fmt, arg...)	dev_err(&uap->node->dev, fmt, ## arg)
-#define pmz_info(fmt, arg...)	dev_info(&uap->node->dev, fmt, ## arg)
-#endif
 
 /*
  * At most 2 ESCCs with 2 ports each

Re: [PATCH 01/16 v3] pmac_zilog: fix unexpected irq

[Benjamin Herrenschmidt]

On Tue, 2011-12-13 at 00:34 +1100, Finn Thain wrote:
> On Mon, 12 Dec 2011, Benjamin Herrenschmidt wrote:
> 
> > Any chance you can test this patch ? I would not be surprised if it 
> > broke m68k since I had to do some of the changes in there "blind", so 
> > let me know... with this, I can again suspend/resume properly on a Pismo 
> > while using the internal modem among other things.
> 
> The patch works on a PowerBook 520 given a few changes (below). This 
> PowerBook only has one serial port that I can test (the internal modem is 
> not supported on 68k Macs).

Interesting. The modem is a soft-modem "geoport" or a hw serial modem ?
In the later case it's probably just a matter of finding the right GPIO
bit in Apple ASIC to turn the power on :-)

>  Can you test a machine with two ports? The 
> rest of my Mac hardware is in storage since I moved house last week.

I tried on 2 port powermacs, but I only have one adapter, so I've
basically been running with one serial port open and shooting irda frame
on the other (with nothing to check wether I got the frames on the other
hand), oh well ...

I'll apply your patch and commit via my tree.

Cheers,
Ben.

> Finn
> 
> 
> Index: linux-git/drivers/tty/serial/pmac_zilog.c
> ===================================================================
> --- linux-git.orig/drivers/tty/serial/pmac_zilog.c	2011-12-13 00:18:02.000000000 +1100
> +++ linux-git/drivers/tty/serial/pmac_zilog.c	2011-12-13 00:23:55.000000000 +1100
> @@ -1705,8 +1705,8 @@ static int __init pmz_init_port(struct u
>  	struct resource *r_ports;
>  	int irq;
>  
> -	r_ports = platform_get_resource(uap->node, IORESOURCE_MEM, 0);
> -	irq = platform_get_irq(uap->node, 0);
> +	r_ports = platform_get_resource(uap->pdev, IORESOURCE_MEM, 0);
> +	irq = platform_get_irq(uap->pdev, 0);
>  	if (!r_ports || !irq)
>  		return -ENODEV;
>  
> @@ -1763,8 +1763,10 @@ static void pmz_dispose_port(struct uart
>  
>  static int __init pmz_attach(struct platform_device *pdev)
>  {
> +	struct uart_pmac_port *uap;
>  	int i;
>  
> +	/* Iterate the pmz_ports array to find a matching entry */
>  	for (i = 0; i < pmz_ports_count; i++)
>  		if (pmz_ports[i].pdev == pdev)
>  			break;
> @@ -1773,15 +1775,23 @@ static int __init pmz_attach(struct plat
>  
>  	uap = &pmz_ports[i];
>  	uap->port.dev = &pdev->dev;
> -	dev_set_drvdata(&mdev->ofdev.dev, uap);
> +	platform_set_drvdata(pdev, uap);
>  
> -			return uart_add_one_port(&pmz_uart_reg,
> -						 &pmz_ports[i]->port);
> +	return uart_add_one_port(&pmz_uart_reg, &uap->port);
>  }
>  
>  static int __exit pmz_detach(struct platform_device *pdev)
>  {
> +	struct uart_pmac_port *uap = platform_get_drvdata(pdev);
> +
> +	if (!uap)
> +		return -ENODEV;
> +
>  	uart_remove_one_port(&pmz_uart_reg, &uap->port);
> +
> +	platform_set_drvdata(pdev, NULL);
> +	uap->port.dev = NULL;
> +
>  	return 0;
>  }
>  
> @@ -1918,8 +1928,13 @@ static void __exit exit_pmz(void)
>  
>  	for (i = 0; i < pmz_ports_count; i++) {
>  		struct uart_pmac_port *uport = &pmz_ports[i];
> +#ifdef CONFIG_PPC_PMAC
>  		if (uport->node != NULL)
>  			pmz_dispose_port(uport);
> +#else
> +		if (uport->pdev != NULL)
> +			pmz_dispose_port(uport);
> +#endif
>  	}
>  	/* Unregister UART driver */
>  	uart_unregister_driver(&pmz_uart_reg);
> @@ -1993,6 +2008,9 @@ static int __init pmz_console_setup(stru
>  #ifdef CONFIG_PPC_PMAC
>  	if (uap->node == NULL)
>  		return -ENODEV;
> +#else
> +	if (uap->pdev == NULL)
> +		return -ENODEV;
>  #endif
>  	port = &uap->port;
>  
> Index: linux-git/drivers/tty/serial/pmac_zilog.h
> ===================================================================
> --- linux-git.orig/drivers/tty/serial/pmac_zilog.h	2011-12-13 00:18:02.000000000 +1100
> +++ linux-git/drivers/tty/serial/pmac_zilog.h	2011-12-13 00:23:55.000000000 +1100
> @@ -1,18 +1,9 @@
>  #ifndef __PMAC_ZILOG_H__
>  #define __PMAC_ZILOG_H__
>  
> -#ifdef CONFIG_PPC_PMAC
> -/* We cannot use dev_* because this can be called early, way before
> - * we are matched with a device (when using it as a kernel console)
> - */
>  #define pmz_debug(fmt, arg...)	pr_debug("ttyPZ%d: " fmt, uap->port.line, ## arg)
>  #define pmz_error(fmt, arg...)	pr_err("ttyPZ%d: " fmt, uap->port.line, ## arg)
>  #define pmz_info(fmt, arg...)	pr_info("ttyPZ%d: " fmt, uap->port.line, ## arg)
> -#else
> -#define pmz_debug(fmt, arg...)	dev_dbg(&uap->node->dev, fmt, ## arg)
> -#define pmz_error(fmt, arg...)	dev_err(&uap->node->dev, fmt, ## arg)
> -#define pmz_info(fmt, arg...)	dev_info(&uap->node->dev, fmt, ## arg)
> -#endif
>  
>  /*
>   * At most 2 ESCCs with 2 ports each

Re: [PATCH 01/16 v3] pmac_zilog: fix unexpected irq

[Finn Thain]

On Tue, 13 Dec 2011, Benjamin Herrenschmidt wrote:

> On Tue, 2011-12-13 at 00:34 +1100, Finn Thain wrote:
> > On Mon, 12 Dec 2011, Benjamin Herrenschmidt wrote:
> > 
> > > Any chance you can test this patch ? I would not be surprised if it 
> > > broke m68k since I had to do some of the changes in there "blind", 
> > > so let me know... with this, I can again suspend/resume properly on 
> > > a Pismo while using the internal modem among other things.
> > 
> > The patch works on a PowerBook 520 given a few changes (below). This 
> > PowerBook only has one serial port that I can test (the internal modem 
> > is not supported on 68k Macs).
> 
> Interesting. The modem is a soft-modem "geoport" or a hw serial modem ? 

It's the latter.

> In the later case it's probably just a matter of finding the right GPIO 
> bit in Apple ASIC to turn the power on :-)

Surely feasible, but not high on the list of missing hardware support.

> 
> >  Can you test a machine with two ports? The rest of my Mac hardware is 
> > in storage since I moved house last week.
> 
> I tried on 2 port powermacs, but I only have one adapter, so I've 
> basically been running with one serial port open and shooting irda frame 
> on the other (with nothing to check wether I got the frames on the other 
> hand), oh well ...
> 
> I'll apply your patch and commit via my tree.

I forgot to include this fix for your logging change.

Finn

Index: linux-git/drivers/tty/serial/pmac_zilog.c
===================================================================
--- linux-git.orig/drivers/tty/serial/pmac_zilog.c	2011-12-13 12:12:05.000000000 +1100
+++ linux-git/drivers/tty/serial/pmac_zilog.c	2011-12-13 12:13:29.000000000 +1100
@@ -99,6 +99,10 @@ MODULE_LICENSE("GPL");
 #define PMACZILOG_NAME		"ttyPZ"
 #endif
 
+#define pmz_debug(fmt, arg...)	pr_debug(PMACZILOG_NAME "%d: " fmt, uap->port.line, ## arg)
+#define pmz_error(fmt, arg...)	pr_err(PMACZILOG_NAME "%d: " fmt, uap->port.line, ## arg)
+#define pmz_info(fmt, arg...)	pr_info(PMACZILOG_NAME "%d: " fmt, uap->port.line, ## arg)
+
 
 /*
  * For the sake of early serial console, we can do a pre-probe
Index: linux-git/drivers/tty/serial/pmac_zilog.h
===================================================================
--- linux-git.orig/drivers/tty/serial/pmac_zilog.h	2011-12-13 12:12:05.000000000 +1100
+++ linux-git/drivers/tty/serial/pmac_zilog.h	2011-12-13 12:12:28.000000000 +1100
@@ -1,10 +1,6 @@
 #ifndef __PMAC_ZILOG_H__
 #define __PMAC_ZILOG_H__
 
-#define pmz_debug(fmt, arg...)	pr_debug("ttyPZ%d: " fmt, uap->port.line, ## arg)
-#define pmz_error(fmt, arg...)	pr_err("ttyPZ%d: " fmt, uap->port.line, ## arg)
-#define pmz_info(fmt, arg...)	pr_info("ttyPZ%d: " fmt, uap->port.line, ## arg)
-
 /*
  * At most 2 ESCCs with 2 ports each
  */