Re: [PATCH 5/5] tty: serial: uartlite: Prevent changing fixed parameters

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 7/29/21 11:01 AM, Greg Kroah-Hartman wrote:
On Fri, Jul 23, 2021 at 06:31:51PM -0400, Sean Anderson wrote:
This device does not support changing baud, parity, data bits, stop
bits, or detecting breaks. Disable "changing" these settings to prevent
their termios from diverging from the actual state of the uart. To inform
users of these limitations, warn if the new termios change these
parameters. We only do this once to avoid spamming the log. These
warnings are inspired by those in the sifive driver.

Signed-off-by: Sean Anderson <sean.anderson@xxxxxxxx>
---

 drivers/tty/serial/uartlite.c | 52 +++++++++++++++++++++++++++++++++--
 1 file changed, 49 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/uartlite.c b/drivers/tty/serial/uartlite.c
index 39c17ab206ca..0aed70039f46 100644
--- a/drivers/tty/serial/uartlite.c
+++ b/drivers/tty/serial/uartlite.c
@@ -314,7 +314,54 @@ static void ulite_set_termios(struct uart_port *port, struct ktermios *termios,
 			      struct ktermios *old)
 {
 	unsigned long flags;
-	unsigned int baud;
+	struct uartlite_data *pdata = port->private_data;
+	tcflag_t old_cflag;
+
+	if (termios->c_iflag & BRKINT)
+		dev_err_once(port->dev, "BREAK detection not supported\n");
+	termios->c_iflag &= ~BRKINT;
+
+	if (termios->c_cflag & CSTOPB)
+		dev_err_once(port->dev, "only one stop bit supported\n");
+	termios->c_cflag &= ~CSTOPB;
+
+	old_cflag = termios->c_cflag;
+	termios->c_cflag &= ~(PARENB | PARODD);
+	if (pdata->parity == 'e')
+		termios->c_cflag |= PARENB;
+	else if (pdata->parity == 'o')
+		termios->c_cflag |= PARENB | PARODD;
+
+	if (termios->c_cflag != old_cflag)
+		dev_err_once(port->dev, "only '%c' parity supported\n",
+			     pdata->parity);

Through all of this, you are warning that nothing is supported, yet you
are continuing on as if all of this worked just fine.

We don't. The idea is that we see if (e.g.) CSIZE is something the
hardware can't produce, warn about it (once), and then set it to what we
can support. That way the user can (programmatically) detect if this
device can support their use-case. So e.g. if you you have a serial bus
or something, the driver can find out that (e.g.) the UART has the wrong
CSIZE, and it can fail to probe. Before this series, it would continue
along as if nothing was wrong, and the user then has to debug why their
device does not work as expected.

--Sean


That feels odd, why is this needed at all?  If you really do not support
any changes here, why even fake it?

thanks,

greg k-h




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux