Re: [RFC PATCH] always probe UART HW when options are not specified
From: Sebastian Frias
Date: Thu Dec 17 2015 - 11:48:52 EST
On 12/17/2015 05:29 PM, Peter Hurley wrote:
On 12/17/2015 07:15 AM, Sebastian Frias wrote:
---
I think there are a few minor bugs on the 8250 UART code.
Below you can find a patch with a proposed solution.
In a nutshell:
- probe_baud from 87515772c33ee8a0cc08d984a7d2401eeff074cd was
converted into probe_port so that it reads all the parameters that
uart_set_options require (namely baud, parity, bits, flow).
- reading/writing to UART_DLL/UART_DLM directly are converted to
using the read_dl/write_dl callbacks.
- the port is always probed if there are no options (*).
Because I don't want to probe the port at all.
But must when using the
earlycon=ttyS0,....
command-line (because the original hack expects that behavior).
Ok, we are using:
"console=ttyS0 earlyprintk"
and the 8250 (with CONFIG_SERIAL_8250_RT288X=y) driver.
The hardware is setup prior to Linux boot.
We don't want Linux to change the UART settings, just to pick up
whatever settings the UART has and take over UART.
How do you suggest we do that? Right now, since it does not probe, it
just messes up the UART config setup before booting Linux.
While on the subject, do you think you could explain the difference (or
similarity) between:
- "console=ttyS0"
- "console=uart"
- "earlycon=uart"
and how they relate to "earlyprintk" (if at all)?
Maybe some of those options are soon to be deprecated and we'd like to
stick with the standard and future-proof way.
Regards,
Peter Hurley
(*): I'm not sure why commit 87515772c33ee8a0cc08d984a7d2401eeff074cd
makes a difference in that regard, especially considering the commit
log states that if there are no options, the hardware is assumed to
be already initialised. Since uart_set_options is always called, the
current hardware setup could be overwritten with different parameters
if the actual hardware is not probed.
---
drivers/tty/serial/8250/8250_core.c | 84 ++++++++++++++++++++++++++---------
1 file changed, 63 insertions(+), 21 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 2c46a21..624667f 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -791,22 +791,19 @@ static int size_fifo(struct uart_8250_port *up)
*/
static unsigned int autoconfig_read_divisor_id(struct uart_8250_port *p)
{
- unsigned char old_dll, old_dlm, old_lcr;
+ unsigned char old_lcr;
unsigned int id;
+ unsigned int old_dl;
old_lcr = serial_in(p, UART_LCR);
- serial_out(p, UART_LCR, UART_LCR_CONF_MODE_A);
-
- old_dll = serial_in(p, UART_DLL);
- old_dlm = serial_in(p, UART_DLM);
- serial_out(p, UART_DLL, 0);
- serial_out(p, UART_DLM, 0);
+ serial_out(p, UART_LCR, UART_LCR_CONF_MODE_A);
- id = serial_in(p, UART_DLL) | serial_in(p, UART_DLM) << 8;
+ old_dl = serial_dl_read(p);
+ serial_dl_write(p, 0);
+ id = serial_dl_read(p);
+ serial_dl_write(p, old_dl);
- serial_out(p, UART_DLL, old_dll);
- serial_out(p, UART_DLM, old_dlm);
serial_out(p, UART_LCR, old_lcr);
return id;
@@ -3440,22 +3437,67 @@ static void univ8250_console_write(struct console *co, const char *s,
serial8250_console_write(up, s, count);
}
-static unsigned int probe_baud(struct uart_port *port)
+static int probe_port(struct uart_port *port, int *parity, int *bits, int *flow)
{
- unsigned char lcr, dll, dlm;
+ struct uart_8250_port *up = up_to_u8250p(port);
+ unsigned char lcr, efr;
unsigned int quot;
lcr = serial_port_in(port, UART_LCR);
serial_port_out(port, UART_LCR, lcr | UART_LCR_DLAB);
- dll = serial_port_in(port, UART_DLL);
- dlm = serial_port_in(port, UART_DLM);
+ quot = serial_dl_read(up);
+ serial_port_out(port, UART_LCR, UART_LCR_CONF_MODE_B);
+ if (port->flags & UPF_EXAR_EFR)
+ efr = serial_port_in(port, UART_XR_EFR);
+ else
+ efr = serial_port_in(port, UART_EFR);
serial_port_out(port, UART_LCR, lcr);
- quot = (dlm << 8) | dll;
- return (port->uartclk / 16) / quot;
+//word length select mask
+#define WLS_MASK (0x3)
+//parity enable
+#define PEN (0x8)
+//even parity select
+#define EPS (0x10)
+
+ switch (lcr & WLS_MASK) {
+ case 0: // 5bits
+ case 1: // 6bits
+ // Not supported by drivers/tty/serial/serial_core.c:uart_set_options() anyway
+ WARN(true, "%s: probed uart word length (%u bits) is not supported by uart_set_options()\n", __FUNCTION__, (lcr & WLS_MASK) ? 5 : 6 );
+ break;
+ case 2: // 7bits
+ *bits = 7;
+ break;
+ case 3: // 8bits
+ *bits = 8;
+ break;
+ };
+
+ if (lcr & PEN)
+ {
+ if (lcr & EPS)
+ *parity = 'e';
+ else
+ *parity = 'o';
+ }
+ else
+ *parity = 'n';
+
+ if (efr & UART_EFR_CTS)
+ *flow = 'r';
+ else
+ *flow = 'n';
+
+ if (quot)
+ return (port->uartclk / 16) / quot;
+ else
+ WARN(true, "%s: quot is zero!\n", __FUNCTION__);
+
+ return -1;
}
-static int serial8250_console_setup(struct uart_port *port, char *options, bool probe)
+static int serial8250_console_setup(struct uart_port *port, char *options)
{
int baud = 9600;
int bits = 8;
@@ -3467,8 +3509,8 @@ static int serial8250_console_setup(struct uart_port *port, char *options, bool
if (options)
uart_parse_options(options, &baud, &parity, &bits, &flow);
- else if (probe)
- baud = probe_baud(port);
+ else
+ baud = probe_port(port, &parity, &bits, &flow);
return uart_set_options(port, port->cons, baud, parity, bits, flow);
}
@@ -3488,7 +3530,7 @@ static int univ8250_console_setup(struct console *co, char *options)
/* link port to console */
port->cons = co;
- return serial8250_console_setup(port, options, false);
+ return serial8250_console_setup(port, options);
}
/**
@@ -3537,7 +3579,7 @@ static int univ8250_console_match(struct console *co, char *name, int idx,
co->index = i;
port->cons = co;
- return serial8250_console_setup(port, options, true);
+ return serial8250_console_setup(port, options);
}
return -ENODEV;
--
1.7.10.4
--
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/