From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753175AbcAEUno (ORCPT ); Tue, 5 Jan 2016 15:43:44 -0500 Received: from lxorguk.ukuu.org.uk ([81.2.110.251]:49932 "EHLO lxorguk.ukuu.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752870AbcAEUnk (ORCPT ); Tue, 5 Jan 2016 15:43:40 -0500 Date: Tue, 5 Jan 2016 20:43:22 +0000 From: One Thousand Gnomes To: Joshua Henderson Cc: , , , Andrei Pistirica , Greg Kroah-Hartman , Jiri Slaby , , Subject: Re: [PATCH v2 10/14] serial: pic32_uart: Add PIC32 UART driver Message-ID: <20160105204322.2dc5ab3f@lxorguk.ukuu.org.uk> In-Reply-To: <1450133093-7053-11-git-send-email-joshua.henderson@microchip.com> References: <1450133093-7053-1-git-send-email-joshua.henderson@microchip.com> <1450133093-7053-11-git-send-email-joshua.henderson@microchip.com> Organization: Intel Corporation X-Mailer: Claws Mail 3.12.0 (GTK+ 2.24.29; x86_64-redhat-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > +#define PIC32_SDEV_NAME "ttyS" > +#define PIC32_SDEV_MAJOR TTY_MAJOR > +#define PIC32_SDEV_MINOR 64 No. Same goes for you as every one of the forty other people a year who try and claim their console is ttyS. If it's not an 8250 it isn't. ttyS is the 8250, use dynamic major and minor and a different name. > +/* serial core request to change current uart setting */ > +static void pic32_uart_set_termios(struct uart_port *port, > + struct ktermios *new, > + struct ktermios *old) > +{ You need to clear any termios features requested but not supported. In your case that appears to be CMSPAR, as you don't seem to support mark/space parity. Similarly if you only support 8N1 or 7E1/7O1 you need to force the CSIZE bits to match what you ended up setting the UART to do. > + /* update baud */ > + baud = uart_get_baud_rate(port, new, old, 0, port->uartclk / 16); > + quot = uart_get_divisor(port, baud) - 1; > + pic32_uart_write(quot, sport, PIC32_UART_BRG); > + uart_update_timeout(port, new->c_cflag, baud); See the 8250 driver for an example: you probably need to write back the actual rate you got. > +/* serial core request to release uart iomem */ > +static void pic32_uart_release_port(struct uart_port *port) > +{ > + struct platform_device *pdev = to_platform_device(port->dev); > + struct resource *res_mem; > + unsigned int res_size; resource_size_t for resources. Or you could just avoid the pointless variable in the first place 8) Other oddments - things like kasprintf() returns should be checked Alan