Re: [PATCH 4/5] tty: serial: uartlite: Initialize termios with fixed synthesis parameters

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

 





On 7/23/21 6:31 PM, Sean Anderson wrote:
This reads the various new devicetree parameters to discover how the
uart was configured when it was synthesized. Note that these properties
are fixed and undiscoverable. Once we have determined how the uart is
configured, we set the termios to let users know, and to initialize the
timeout to the correct value.

The defaults match ulite_console_setup. xlnx,use-parity,
xlnx,odd-parity, and xlnx,data-bits are optional since there were
in-tree users (and presumably out-of-tree users) who did not set them.

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

  drivers/tty/serial/uartlite.c | 66 +++++++++++++++++++++++++++++++----
  1 file changed, 60 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/uartlite.c b/drivers/tty/serial/uartlite.c
index f42ccc40ffa6..39c17ab206ca 100644
--- a/drivers/tty/serial/uartlite.c
+++ b/drivers/tty/serial/uartlite.c
@@ -60,9 +60,20 @@
  static struct uart_port *console_port;
  #endif

+/**
+ * struct uartlite_data: Driver private data
+ * reg_ops: Functions to read/write registers
+ * clk: Our parent clock, if present
+ * baud: The baud rate configured when this device was synthesized
+ * parity: The parity settings, like for uart_set_options()
+ * bits: The number of data bits
+ */
  struct uartlite_data {
  	const struct uartlite_reg_ops *reg_ops;
  	struct clk *clk;
+	int baud;
+	int parity;
+	int bits;
  };

  struct uartlite_reg_ops {
@@ -652,6 +663,9 @@ static int ulite_assign(struct device *dev, int id, u32 base, int irq,
  	port->type = PORT_UNKNOWN;
  	port->line = id;
  	port->private_data = pdata;
+	/* Initialize the termios to what was configured at synthesis-time */
+	uart_set_options(port, NULL, pdata->baud, pdata->parity, pdata->bits,
+			 'n');

I did some testing today, and discovered that the termios are not set
properly. I think I missed this the first time around because on
Microblaze QEMU this UART is the console, and the baud etc. gets set
properly because of stdout-path (or bootargs). However, uart_set_options
doesn't actually do anything with the termios when co is NULL.

The initial termios are set up by tty_init_termios (called from
tty_init_dev). They come from either tty->driver->init_termios, or from
tty->driver->termios[idx]. There is only one init_termios per-driver,
so we would need to have multiple drivers if we wanted to have multiple
UARTs with different (e.g.) bauds.

The indexed termios are designed to keep the settings from the previous
time the tty was opened. So I think (ab)using them is not too terrible,
especially since we will only set them once. Unfortunately, we cannot
use tty_save_termios to initialize the indexed termio, since the tty is
not set until tty_port_open, which is called after tty_init_dev.

Based on this, I think the neatest cut would be something like

/* perhaps just do this in ulite_assign? */
ulite_request_port () {
	/* ... */
	termios = &ulite_uart_driver.tty_driver->termios[port->line];
	termios = kzalloc(sizeof(*termios));
	if (!termios)
		/* ... */
	termios.c_cflags = /* ... */
	/* etc */
}

Unfortunately, according to include/linux/serial_core.h, tty_driver is
not supposed to be touched by the low-level driver. But I think we have
a bit of an unusual case here with a device that can't change baud. If
anyone has other suggestions, I'm all for them.

--Sean


  	dev_set_drvdata(dev, port);

@@ -756,18 +770,58 @@ static int ulite_probe(struct platform_device *pdev)
  	struct uartlite_data *pdata;
  	int irq, ret;
  	int id = pdev->id;
-#ifdef CONFIG_OF
-	const __be32 *prop;

-	prop = of_get_property(pdev->dev.of_node, "port-number", NULL);
-	if (prop)
-		id = be32_to_cpup(prop);
-#endif
  	pdata = devm_kzalloc(&pdev->dev, sizeof(struct uartlite_data),
  			     GFP_KERNEL);
  	if (!pdata)
  		return -ENOMEM;

+	if (IS_ENABLED(CONFIG_OF)) {
+		const char *prop;
+		struct device_node *np = pdev->dev.of_node;
+		u32 val;
+
+		prop = "port-number";
+		ret = of_property_read_u32(np, prop, &id);
+		if (ret && ret != -EINVAL)
+of_err:
+			return dev_err_probe(&pdev->dev, ret,
+					     "could not read %s\n", prop);
+
+		prop = "current-speed";
+		ret = of_property_read_u32(np, prop, &pdata->baud);
+		if (ret)
+			goto of_err;
+
+		prop = "xlnx,use-parity";
+		ret = of_property_read_u32(np, prop, &val);
+		if (ret && ret != -EINVAL)
+			goto of_err;
+
+		if (val) {
+			prop = "xlnx,odd-parity";
+			ret = of_property_read_u32(np, prop, &val);
+			if (ret)
+				goto of_err;
+
+			if (val)
+				pdata->parity = 'o';
+			else
+				pdata->parity = 'e';
+		} else {
+			pdata->parity = 'n';
+		}
+
+		prop = "xlnx,data-bits";
+		ret = of_property_read_u32(np, prop, &pdata->bits);
+		if (ret && ret != -EINVAL)
+			goto of_err;
+	} else {
+		pdata->baud = 9600;
+		pdata->parity = 'n';
+		pdata->bits = 8;
+	}
+
  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
  	if (!res)
  		return -ENODEV;




[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