| Message ID | alpine.LNX.2.00.1111250133321.6314@nippy.intranet (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Benjamin Herrenschmidt |
| Headers | show |
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
> 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....) David
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.
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.
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
> 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.
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);