On Mon, 2016-02-29 at 12:29 +0200, Andy Shevchenko wrote:
> On Sat, 2016-02-27 at 19:14 +0300, Sergei Ianovich wrote:
> > +struct lp8841_serial_data {
> > + int line;
> > + void *ios_mem;
>
> __iomem
OK
> > +};
>
> > + 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?
>
> > + 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);
> > + 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
> > + data->ios_mem = devm_ioremap_resource(&pdev->dev,
> > > > mires);
> > + if (!data->ios_mem)
> > + return -EFAULT;
>
> You have to propagate the actual error code from
> devm_ioremap_resource().
OK
> > +
> > + uart.port.iotype = UPIO_MEM;
> >
>
> > + 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
