linux-kernel - Re: [PATCH v4 16/28] tty: serial: owl: Implement console driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d3c107f3-5df6-a771-2510-ef5578216d0f@suse.de>
Date:   Sun, 2 Jul 2017 22:27:22 +0200
From:   Andreas Färber <afaerber@...e.de>
To:     Alan Cox <gnomes@...rguk.ukuu.org.uk>
Cc:     support@...aker.org,
        张天益 <tyzhang@...ions-semi.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        96boards@...obotics.com, linux-kernel@...r.kernel.org,
        Thomas Liau <thomas.liau@...ions-semi.com>,
        mp-cs@...ions-semi.com, linux-serial@...r.kernel.org,
        刘炜 <liuwei@...ions-semi.com>,
        Jiri Slaby <jslaby@...e.com>,
        linux-arm-kernel@...ts.infradead.org,
        张东风 <zhangdf@...ions-semi.com>,
        梅利 <harrymei@...ions-semi.com>,
        support@...ietech.com, lee@...ietech.com
Subject: Re: [PATCH v4 16/28] tty: serial: owl: Implement console driver

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)