On Tue, Mar 1, 2016 at 10:08 PM, Sergei Ianovich <ynvich@xxxxxxxxx> 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 <ynvich@xxxxxxxxx>
> Reviewed-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
Sorry, but still few nitpicks and then
Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> +#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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
