On 2010-02-25 at 16:26:22 +0100, Alan Cox <alan@xxxxxxxxxxxxxxxxxxx> wrote:
> > +static void altera_jtaguart_set_termios(struct uart_port *port,
> > + struct ktermios *termios,
> > + struct ktermios *old)
> > +{
> > +}
>
> Erm no .. it may be wrong in the code you are copying but please don't
> further that. The requirement is that the termios handed back reflects
> the actual settings not the requested ones.
>
> Use tty_termios_copy_hw() to copy the old termios hardware settings back
> so that the caller sees it cannot set them.
Would the following be sufficient?
if (old)
tty_termios_copy_hw(termios, old);
> > +static void altera_jtaguart_rx_chars(struct altera_jtaguart *pp)
> > +{
> > + struct uart_port *port = &pp->port;
> > + unsigned char ch, flag;
> > + unsigned long status;
> > +
> > + while ((status = readl(port->membase + ALTERA_JTAGUART_DATA_REG)) &
> > + ALTERA_JTAGUART_DATA_RVALID_MSK) {
> > + ch = status & ALTERA_JTAGUART_DATA_DATA_MSK;
> > + flag = TTY_NORMAL;
> > + port->icount.rx++;
> > +
> > + if (uart_handle_sysrq_char(port, ch))
> > + continue;
> > + uart_insert_char(port, 0, 0, ch, flag);
> > + }
> > +
> > + tty_flip_buffer_push(port->state->port.tty);
>
> port.tty needs to be protected here - you might race an unplug/hangup as
> far as I can see - I think you need to take the lock over a bigger area
So should we take port->lock in altera_jtaguart_interrupt already,
something like the following?
spin_lock(&port->lock);
if (isr & ALTERA_JTAGUART_CONTROL_RE_MSK)
altera_jtaguart_rx_chars(pp);
if (isr & ALTERA_JTAGUART_CONTROL_WE_MSK)
altera_jtaguart_tx_chars(pp);
spin_unlock(&port->lock);
And of course remove the lock/unlock from altera_jtaguart_tx_chars.
> [I'm working on switching this to krefs in all the drivers currently so
> that will end up cleaner]
>
> Basically fine other than that. The proposed UART number clashes with
> some other pending patches but that will get sorted out when they get
> merged
Thanks a lot for your review.
Tobias
--
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
