On Mon, 2016-02-29 at 12:29 +0200, Andy Shevchenko wrote:

On Sat, 2016-02-27 at 19:14 +0300, Sergei Ianovich wrote:

quoted

+struct lp8841_serial_data {
+	int			line;
+	void			*ios_mem;

__iomem

OK

quoted

+};

quoted

+	if (termios->c_cflag & CSTOPB)
+		len++;
+	if (termios->c_cflag & PARENB)
+		len++;
+	if (!(termios->c_cflag & PARODD))
+		len++;
+#ifdef CMSPAR
+	if (termios->c_cflag & CMSPAR)
+		len++;
+#endif
+
+	len -= 9;

If you have 5 bit mode, no parity, 1/1.5 stop bits, you may end up
with
negative value here. Am I right? If so, is it expected?

quoted

+	len &= 3;
+	len <<= 3;

I haven't tested this mode. I am pretty sure it will fail. There is
also no support for speeds higher than 115200.

CS7 and CS8 at speeds up to 115200 work well.

However, there is no way to report errors from set_termios(). Should
anything be done about those limitations?

+	mmres = platform_get_resource(pdev, IORESOURCE_MEM, 0);

quoted

+	mires = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	if (!mmres || !mires)
+		return -ENODEV;

No need to check mires here, devm_ioremap_resource() will take care
about.

OK

quoted

+	data->ios_mem = devm_ioremap_resource(&pdev->dev,

quoted

quoted

mires);

+	if (!data->ios_mem)
+		return -EFAULT;

You have to propagate the actual error code from
devm_ioremap_resource().

OK

quoted

+
+	uart.port.iotype = UPIO_MEM;

quoted

+	uart.port.mapbase = mmres->start;
+	uart.port.iobase = mmres->start;

I'm not sure about this. If you ask for UPIO_MEM why do you need to
fill iobase?
And I suppose iobase can't hold (at the end inb/outb calls) big port
numbers anyway (16 bit on x86, for example).

There is no need for iobase. I'll remove that line.