Re: [PATCH 1/2] serial: Add driver for the Altera JTAG UART

From: Alan Cox
Date: Thu Feb 25 2010 - 10:22:55 EST


> +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.

> +
> +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

[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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/