Messages in this thread |  | | Date | Thu, 15 Feb 2007 23:31:32 +0300 | From | Sergei Shtylyov <> | Subject | Re: [PATCH] serial driver PMC MSP71xx, kernel linux-mips.git master |
| |
Hello.
Marc St-Jean wrote:
> There are three different fixes: > 1. Fix for DesignWare APB THRE errata: > In brief, this is a non-standard 16550 in that the THRE interrupt > will not re-assert itself simply by disabling and re-enabling the > THRI bit in the IER, it is only re-enabled if a character is actually > sent out. > It appears that the "8250-uart-backup-timer.patch" in the "mm" tree also > fixes it so we have dropped our initial workaround. > This patch now needs to be applied on top of that "mm" patch.
This patch has hit the mainline kernel already.
> 2. Fix for Busy Detect on LCR write: > The DesignWare APB UART has a feature which causes a new Busy Detect > interrupt to be generated if it's busy when the LCR is written. This > fix saves the value of the LCR and rewrites it after clearing the > interrupt.
> 3. Workaround for interrupt/data concurrency issue: > The SoC needs to ensure that writes that can cause interrupts to be > cleared reach the UART before returning from the ISR. This fix reads > a non-destructive register on the UART so the read transaction > completion ensures the previously queued write transaction has > also completed.
> The differences from the last attempt is dropping the call to > in_irq() and including more detailed description of the fixes.
The difference part would better be under the "---" cutoff line, along with diffstat. This way it gets auto-removed by quilt/git when applying the patch.
> Signed-off-by: Marc St-Jean <Marc_St-Jean@pmc-sierra.com>
> diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c > index 3d91bfc..bfaacc5 100644 > --- a/drivers/serial/8250.c > +++ b/drivers/serial/8250.c > @@ -308,6 +308,7 @@ static unsigned int serial_in(struct uar > return inb(up->port.iobase + 1); > > case UPIO_MEM: > + case UPIO_DWAPB: > return readb(up->port.membase + offset); > > case UPIO_MEM32: > @@ -333,6 +334,8 @@ #endif > static void > serial_out(struct uart_8250_port *up, int offset, int value) > { > + /* Save the offset before it's remapped */ > + int save_offset = offset; > offset = map_8250_out_reg(up, offset) << up->port.regshift; > > switch (up->port.iotype) {
I've just got an idea how to "beautify" this code -- rename the 'offset' arg to something like reg, and then declare/init 'offset' as local variable. Would certainly look better (and worth doing in all serial_* accessros).
> @@ -359,6 +362,18 @@ #endif > writeb(value, up->port.membase + offset); > break; > > + case UPIO_DWAPB: > + /* Save the LCR value so it can be re-written when a > + * Busy Detect interrupt occurs. */ > + if (save_offset == UART_LCR) > + up->lcr = value; > + writeb(value, up->port.membase + offset); > + /* Read the IER to ensure any interrupt is cleared before > + * returning from ISR. */ > + if (save_offset == UART_TX || save_offset == UART_IER) > + value = serial_in(up, UART_IER); > + break; > + > default: > outb(value, up->port.iobase + offset); > } > @@ -2454,8 +2485,8 @@ int __init serial8250_start_console(stru > > add_preferred_console("ttyS", line, options); > printk("Adding console on ttyS%d at %s 0x%lx (options '%s')\n", > - line, port->iotype == UPIO_MEM ? "MMIO" : "I/O port", > - port->iotype == UPIO_MEM ? (unsigned long) port->mapbase : > + line, port->iotype >= UPIO_MEM ? "MMIO" : "I/O port", > + port->iotype >= UPIO_MEM ? (unsigned long) port->mapbase : > (unsigned long) port->iobase, options); > if (!(serial8250_console.flags & CON_ENABLED)) { > serial8250_console.flags &= ~CON_PRINTBUFFER;
I've remembered why I decided not to fix it: this code only gets called from 8250__eraly.c which can't handle anything beyond UPIO_MEM anyway.
WBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|  |