On Thu, 10 Apr 2008 17:15:58 +0200
Rodolfo Giometti <giometti@linux.it> wrote:

quoted text
> Adds support for the PPS sources connected with the CD (Carrier
> Detect) pin of a serial port.

Ok why does this need to hack the driver layer ?

quoted text
> -static void uart_change_speed(struct uart_state *state,
> -					struct ktermios *old_termios);
> +static void uart_change_speed(struct uart_state *state, struct ktermios *old_termios);

NAK. Please remove random formatting changes

quoted text
> -#define uart_set_mctrl(port, set)	uart_update_mctrl(port, set, 0)
> -#define uart_clear_mctrl(port, clear)	uart_update_mctrl(port, 0, clear)
> +#define uart_set_mctrl(port,set)	uart_update_mctrl(port,set,0)
> +#define uart_clear_mctrl(port,clear)	uart_update_mctrl(port,0,clear)

Ditto

quoted text
>  
>  /*
>   * Startup the port.  This will be called once per open.  All calls
> @@ -291,7 +291,7 @@ uart_update_timeout(struct uart_port *port, unsigned int cflag,
>  		break;
>  	default:
>  		bits = 10;
> -		break; /* CS8 */
> +		break; // CS8

Ditto

quoted text
> -			baud = tty_termios_baud_rate(old);
> -			tty_termios_encode_baud_rate(termios, baud, baud);
> +			termios->c_cflag |= old->c_cflag & CBAUD;

NAK. Breaks the tty layer code

quoted text
>  			old = NULL;
>  			continue;
>  		}
> @@ -382,7 +381,7 @@ uart_get_baud_rate(struct uart_port *port, struct ktermios *termios,
>  		 * As a last resort, if the quotient is zero,
>  		 * default to 9600 bps
>  		 */
> -		tty_termios_encode_baud_rate(termios, 9600, 9600);
> +		termios->c_cflag |= B9600;

NAK breaks the tty layer code

quoted text
> +#ifdef CONFIG_PPS_CLIENT_UART

wrong layer - this means only some ports will work.


quoted text
> @@ -789,8 +845,7 @@ static int uart_set_info(struct uart_state *state,
>  			 * We failed anyway.
>  			 */
>  			retval = -EBUSY;
> -			/* Added to return the correct error -Ram Gupta */
> -			goto exit;
> +			goto exit;  // Added to return the correct error -Ram Gupta

More format mangling

quoted text
> +	/* PPS support enabled/disabled? */
> +	if ((old_flags & UPF_HARDPPS_CD) != (new_flags & UPF_HARDPPS_CD)) {

How is this flag set ?


quoted text
> -static void uart_set_termios(struct tty_struct *tty,
> -						struct ktermios *old_termios)
> +static void uart_set_termios(struct tty_struct *tty, struct ktermios *old_termios)

NAK - more format mangling

quoted text
>  {
>  	struct uart_state *state = tty->driver_data;
>  	unsigned long flags;
> @@ -1464,8 +1526,9 @@ uart_block_til_ready(struct file *filp, struct uart_state *state)
>  		 */
>  		if ((filp->f_flags & O_NONBLOCK) ||
>  		    (info->tty->termios->c_cflag & CLOCAL) ||
> -		    (info->tty->flags & (1 << TTY_IO_ERROR)))
> +		    (info->tty->flags & (1 << TTY_IO_ERROR))) {

And more - and so it goes on. Please clean up the mess and submit a
proposal which is just the relevant changes so it can be read. Then we
can talk about putting it in the right place.

My guess is that PPS like FIR should be a line discipline and they both
and a couple of other cases want the tty layer to grow

	tty->ops->report_event();

and
	tty->ops->set_event_mask();

for nicer ways to monitor modem bits in the ldisc.

Alan
--
unsubscribe notice
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/