Re: [PATCH] serial: DCC(JTAG) serial and console emulation support

[Alan Cox <alan@lxorguk.ukuu.org.uk>]

> +	  Say Y here if you want to install DCC driver as a normal serial port
> +	  /dev/ttyS0 (major 4, minor 64). Otherwise, it appears as /dev/ttyJ0
> +	  (major 4, minor 128) and can co-exist with other UARTs, such as
> +	  8250/16C550 compatibles.
> +

NAK to both

ttyJ0 is 204,186 for "JTAG1 DCC protocol based serial"

so there is an existing name and minor allocation, which as you won't be
using two of them should be quite usable.

If you want to be able to switch at runtime to pretend it is ttyS0 please
deal with that in your user space. The same rules apply to you as have
been applied to everyone else who has tried to implement this same crap
in their uart driver too (we'd have it in about 30 by now otherwise)

>
> +static inline void dcc_rx_chars(struct uart_port *port)
> +{
> +	unsigned char ch;
> +	struct tty_struct *tty = port->state->port.tty;

Not safe in the general case - you might get a hangup here then call
through a NULL pointer. Please use the proper tty_port helpers.


> +static inline void dcc_overrun_chars(struct uart_port *port)
> +{
> +	port->icount.overrun++;
> +}

Why make this a function?


> +static void
> +dcc_set_termios(struct uart_port *port, struct ktermios *termios,
> +		struct ktermios *old)
> +{
> +	unsigned int baud, quot;
> +
> +	/*
> +	 * We don't support parity, stop bits, or anything other
> +	 * than 8 bits, so clear these termios flags.
> +	 */
> +	termios->c_cflag &= ~(CSIZE | CSTOPB | PARENB | PARODD | CREAD);
> +	termios->c_cflag |= CS8;
> +
> +	/*
> +	 * We don't appear to support any error conditions either.
> +	 */
> +	termios->c_iflag &= ~(INPCK | IGNPAR | IGNBRK | BRKINT);

There is a helper function for keeping the hardware bits fixed. If your
hardware is fixed then please use it.

Alan