From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andy Shevchenko Subject: Re: [PATCH v9] serial: support for 16550A serial ports on LP-8x4x Date: Tue, 1 Mar 2016 22:23:40 +0200 Message-ID: References: <1456862078-11795-1-git-send-email-ynvich@gmail.com> <1456862903-12392-1-git-send-email-ynvich@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Return-path: In-Reply-To: <1456862903-12392-1-git-send-email-ynvich-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Sergei Ianovich Cc: "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , Alan Cox , Andy Shevchenko , Arnd Bergmann , Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , Greg Kroah-Hartman , Jiri Slaby , Heikki Krogerus , Masahiro Yamada , Joachim Eastwood , Paul Burton , Mans Rullgard , Scott Wood , Paul Gortmaker , Peter Ujfalusi , Peter Hurley , open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE List-Id: devicetree@vger.kernel.org On Tue, Mar 1, 2016 at 10:08 PM, Sergei Ianovich 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 > Reviewed-by: Heikki Krogerus Sorry, but still few nitpicks and then Reviewed-by: Andy Shevchenko > +#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