Re: [PATCH] TTY: serial 8250: Support MCR CLK_SEL bit.

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

>     * Allows the CLK_SEL bit to be forced to /1, /4 or left as is.

This seems to be the wrong place to set such stuff. The kernel knows what
speed is selected and therefore what clock would be best.

Having a clocksel (and probably an of value indicating which type of
clock sel) is a good thing, but it also ought to be set based on the
requested baud rate, so a custom set_termios would be better.

That would also handle serial console, and suspend/resume without other
tweaks as far as I can see.

It also seems some devices use different clksel algorithms so rather than
a CAP_CLKSEL flag we probably need to key it off the uart type.


>  static void serial8250_set_sleep(struct uart_8250_port *p, int sleep)
>  {
>  	if (p->capabilities & UART_CAP_SLEEP) {
> -		if (p->capabilities & UART_CAP_EFR) {
> -			serial_out(p, UART_LCR, UART_LCR_CONF_MODE_B);
> -			serial_out(p, UART_EFR, UART_EFR_ECB);
> -			serial_out(p, UART_LCR, 0);
> -		}
> +		serial8250_enable_extended(p);
>  		serial_out(p, UART_IER, sleep ? UART_IERX_SLEEP : 0);

This seems wrong if theere is no EFR

> +static int serial8250_configure_clock_predivisor(struct uart_8250_port *p)
> +{

Ought to be named to reflect that it appears to be exar specific

Architecturally the patch seems wrong though. I'd expect a device with a
clock predivisor to be treated as a device with a wider clock range and
just handle it when the speed is set. Eg wrapping set_termios or adding a 

	->select_clock(port, bitrate);

helper.

Alan