On Mon, 12 Jan 2009 10:42:02 +0100, Haavard Skinnemoen <haavard.skinnemoen@xxxxxxxxx> wrote: > > +++ a/drivers/serial/atmel_serial.c > > @@ -877,6 +877,9 @@ static int atmel_startup(struct uart_por > > } > > } > > > > + /* Save current CSR for comparison in atmel_tasklet_func() */ > > + atmel_port->irq_status_prev = UART_GET_CSR(port); > > + > > ...Itai Levi pointed out that we need to initialize > atmel_port->irq_status as well here. His analysis is as follows: > > > Regarding the second part of the patch (which resets irq_status_prev), > > it turns out that both versions of the patch (mine and Atsushi's) > > still leave enough room for faulty behavior when opening the port. > > > > This is because we are not resetting both irq_status_prev and > > irq_status in atmel_startup() to CSR, which leads faulty behavior in > > the following sequences: ... > @@ -805,6 +805,9 @@ static int atmel_startup(struct uart_por > */ > UART_PUT_IDR(port, -1); > > + atmel_port->irq_status_prev = UART_GET_CSR(port); > + atmel_port->irq_status = atmel_port->irq_status_prev; > + > /* > * Allocate the IRQ > */ Looks correct for me too. Then, what is preffered final fix? Put irq_status initialization into my patch, or replace with Itai's patch? These patch put initialization in another place. Any comments for preferred place to fix? --- Atsushi Nemoto -- To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
