On Tue, Mar 1, 2016 at 10:08 PM, Sergei Ianovich [off-list ref] wrote:

The patch adds support for 3 additional LP-8x4x built-in serial
ports.

The device can also host up to 8 extension cards with 4 serial ports
on each card for a total of 35 ports. However, I don't have
the hardware to test extension cards, so they are not supported, yet.

Signed-off-by: Sergei Ianovich <redacted>
Reviewed-by: Heikki Krogerus <heikki.krogerus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

Sorry, but still few nitpicks and then
Reviewed-by: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

+#define LP8841_DATA_LEN_MASK           0x3
+#define LP8841_DATA_LEN_SHIFT_OFFSET   3

No need to have _OFFSET suffix.

+
+static void lp8841_serial_set_termios(struct uart_port *port,
+               struct ktermios *termios, struct ktermios *old)
+{
+       unsigned int baud;
+       u8 value;

+       unsigned int old_csize = old ? old->c_cflag & CSIZE : CS8;
+       struct lp8841_serial_data *data = port->private_data;

I would rearrange to have assignments first in the definition block.

Code duplication (no idea if it worth to fix):

+       case 2400:
+               value |= 1;
+               break;
+       default:
+               value |= 1;
+               tty_termios_encode_baud_rate(termios, 2400, 2400);
+               break;

+static int lp8841_serial_probe(struct platform_device *pdev)
+{
+       struct uart_8250_port uart = {};

No need {} since memset().

+       struct lp8841_serial_data *data;
+       struct resource *mmres, *mires;
+       int ret;
+
+       memset(&uart, 0, sizeof(uart));

Move this closer to the first assignment of a field.

+
+       mmres = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+       if (!mmres)
+               return -ENODEV;
+
+       data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+       if (!data)
+               return -ENOMEM;
+
+       mires = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+       data->ios_mem = devm_ioremap_resource(&pdev->dev, mires);
+       if (IS_ERR(data->ios_mem))
+               return PTR_ERR(data->ios_mem);
+

+       memset(&uart, 0, sizeof(uart));

+       uart.port.iotype = UPIO_MEM;
+       uart.port.mapbase = mmres->start;
+       uart.port.regshift = 1;
+       uart.port.irq = platform_get_irq(pdev, 0);
+       uart.port.flags = UPF_IOREMAP;
+       uart.port.dev = &pdev->dev;
+       uart.port.uartclk = 14745600;
+       uart.port.set_termios = lp8841_serial_set_termios;
+       uart.port.private_data = data;

-- 
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html