Re: [PATCH v6] serial: support for 16550A serial ports on LP-8x4x

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

 



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



[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