From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752004AbdGBU13 (ORCPT ); Sun, 2 Jul 2017 16:27:29 -0400 Received: from mx2.suse.de ([195.135.220.15]:60229 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751876AbdGBU10 (ORCPT ); Sun, 2 Jul 2017 16:27:26 -0400 Subject: Re: [PATCH v4 16/28] tty: serial: owl: Implement console driver To: Alan Cox Cc: support@lemaker.org, =?UTF-8?B?5byg5aSp55uK?= , Greg Kroah-Hartman , 96boards@ucrobotics.com, linux-kernel@vger.kernel.org, Thomas Liau , mp-cs@actions-semi.com, linux-serial@vger.kernel.org, =?UTF-8?B?5YiY54Kc?= , Jiri Slaby , linux-arm-kernel@lists.infradead.org, =?UTF-8?B?5byg5Lic6aOO?= , =?UTF-8?B?5qKF5Yip?= , support@cubietech.com, lee@cubietech.com References: <20170606005426.26446-1-afaerber@suse.de> <20170606005426.26446-17-afaerber@suse.de> <20170606143407.56d15c3c@lxorguk.ukuu.org.uk> From: =?UTF-8?Q?Andreas_F=c3=a4rber?= Organization: SUSE Linux GmbH Message-ID: Date: Sun, 2 Jul 2017 22:27:22 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.0 MIME-Version: 1.0 In-Reply-To: <20170606143407.56d15c3c@lxorguk.ukuu.org.uk> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Am 06.06.2017 um 15:34 schrieb Alan Cox: >> +static void owl_uart_set_termios(struct uart_port *port, >> + struct ktermios *termios, >> + struct ktermios *old) >> +{ >> + struct owl_uart_port *owl_port = to_owl_uart_port(port); >> + unsigned int baud; >> + u32 ctl; >> + unsigned long flags; >> + >> + spin_lock_irqsave(&port->lock, flags); >> + >> + /* We don't support modem control lines. */ >> + termios->c_cflag &= ~(HUPCL | CMSPAR); >> + termios->c_cflag |= CLOCAL; > > CLOCAL and HUPCL are software not hardware properties so are probably > best not forced (it'll confuse some apps if you do) >> + >> + /* We don't support BREAK character recognition. */ >> + termios->c_iflag &= ~(IGNBRK | BRKINT); > > Ditto these Fixed. These were obviously forward-ported from the vendor tree. https://github.com/LeMaker/linux-actions/blob/linux-3.10.y/arch/arm/mach-owl/serial-owl.c > You do clear CMSPAR which is right if not supported but then later on we > have: > >> + if (termios->c_cflag & PARENB) { >> + if (termios->c_cflag & CMSPAR) { >> + if (termios->c_cflag & PARODD) >> + ctl |= OWL_UART_CTL_PRS_MARK; >> + else >> + ctl |= OWL_UART_CTL_PRS_SPACE; >> + } else if (termios->c_cflag & PARODD) >> + ctl |= OWL_UART_CTL_PRS_ODD; >> + else >> + ctl |= OWL_UART_CTL_PRS_EVEN; >> + } else >> + ctl |= OWL_UART_CTL_PRS_NONE; > > > Which seems to indicate you do support CMSPAR so shouldn't be clearing it. Again that's what the original code was like. Since these register values do exist, I would rather not rip the code out, unless it's wrong. In my testing, not clearing CMSPAR works so far. >> + baud = uart_get_baud_rate(port, termios, old, 9600, 3200000); >> + owl_uart_change_baudrate(owl_port, baud); > > You should re-encode the resulting baud rate into the termios > > /* Don't rewrite B0 */ > if (tty_termios_baud_rate(termios)) > tty_termios_encode_baud_rate(termios, baud, baud); Added, thanks. Regards, Andreas -- SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) From mboxrd@z Thu Jan 1 00:00:00 1970 From: afaerber@suse.de (=?UTF-8?Q?Andreas_F=c3=a4rber?=) Date: Sun, 2 Jul 2017 22:27:22 +0200 Subject: [PATCH v4 16/28] tty: serial: owl: Implement console driver In-Reply-To: <20170606143407.56d15c3c@lxorguk.ukuu.org.uk> References: <20170606005426.26446-1-afaerber@suse.de> <20170606005426.26446-17-afaerber@suse.de> <20170606143407.56d15c3c@lxorguk.ukuu.org.uk> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Am 06.06.2017 um 15:34 schrieb Alan Cox: >> +static void owl_uart_set_termios(struct uart_port *port, >> + struct ktermios *termios, >> + struct ktermios *old) >> +{ >> + struct owl_uart_port *owl_port = to_owl_uart_port(port); >> + unsigned int baud; >> + u32 ctl; >> + unsigned long flags; >> + >> + spin_lock_irqsave(&port->lock, flags); >> + >> + /* We don't support modem control lines. */ >> + termios->c_cflag &= ~(HUPCL | CMSPAR); >> + termios->c_cflag |= CLOCAL; > > CLOCAL and HUPCL are software not hardware properties so are probably > best not forced (it'll confuse some apps if you do) >> + >> + /* We don't support BREAK character recognition. */ >> + termios->c_iflag &= ~(IGNBRK | BRKINT); > > Ditto these Fixed. These were obviously forward-ported from the vendor tree. https://github.com/LeMaker/linux-actions/blob/linux-3.10.y/arch/arm/mach-owl/serial-owl.c > You do clear CMSPAR which is right if not supported but then later on we > have: > >> + if (termios->c_cflag & PARENB) { >> + if (termios->c_cflag & CMSPAR) { >> + if (termios->c_cflag & PARODD) >> + ctl |= OWL_UART_CTL_PRS_MARK; >> + else >> + ctl |= OWL_UART_CTL_PRS_SPACE; >> + } else if (termios->c_cflag & PARODD) >> + ctl |= OWL_UART_CTL_PRS_ODD; >> + else >> + ctl |= OWL_UART_CTL_PRS_EVEN; >> + } else >> + ctl |= OWL_UART_CTL_PRS_NONE; > > > Which seems to indicate you do support CMSPAR so shouldn't be clearing it. Again that's what the original code was like. Since these register values do exist, I would rather not rip the code out, unless it's wrong. In my testing, not clearing CMSPAR works so far. >> + baud = uart_get_baud_rate(port, termios, old, 9600, 3200000); >> + owl_uart_change_baudrate(owl_port, baud); > > You should re-encode the resulting baud rate into the termios > > /* Don't rewrite B0 */ > if (tty_termios_baud_rate(termios)) > tty_termios_encode_baud_rate(termios, baud, baud); Added, thanks. Regards, Andreas -- SUSE Linux GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany GF: Felix Imend?rffer, Jane Smithard, Graham Norton HRB 21284 (AG N?rnberg)