linux-kernel - Re: [PATCH v7] serial: support for 16550A serial ports on LP-8x4x

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: <1456854532.13244.215.camel@linux.intel.com>
Date:	Tue, 01 Mar 2016 19:48:52 +0200
From:	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To:	Sergei Ianovich <ynvich@...il.com>, linux-kernel@...r.kernel.org
Cc:	Alan Cox <gnomes@...rguk.ukuu.org.uk>,
	Arnd Bergmann <arnd@...db.de>,
	Rob Herring <robh+dt@...nel.org>,
	Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Jiri Slaby <jslaby@...e.com>,
	Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
	Peter Hurley <peter@...leysoftware.com>,
	Masahiro Yamada <yamada.masahiro@...ionext.com>,
	Paul Burton <paul.burton@...tec.com>,
	Mans Rullgard <mans@...sr.com>,
	Joachim Eastwood <manabian@...il.com>,
	Scott Wood <scottwood@...escale.com>,
	Paul Gortmaker <paul.gortmaker@...driver.com>,
	Peter Ujfalusi <peter.ujfalusi@...com>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
	<devicetree@...r.kernel.org>,
	"open list:SERIAL DRIVERS" <linux-serial@...r.kernel.org>
Subject: Re: [PATCH v7] serial: support for 16550A serial ports on LP-8x4x

On Tue, 2016-03-01 at 20:14 +0300, Sergei Ianovich wrote:
> On Tue, 2016-03-01 at 18:46 +0200, Andy Shevchenko wrote:
> > On Tue, 2016-03-01 at 19:25 +0300, Sergei Ianovich wrote:
> > > On Tue, 2016-03-01 at 13:06 +0200, Andy Shevchenko wrote:
> > > > On Tue, 2016-03-01 at 00:26 +0300, Sergei Ianovich wrote:

> > So, but if you support only fixed rates, why do you care about
> > BOTHER
> > at all?
> 
> If BOTHER is defined, tty_termios_baud_rate()
> and tty_termios_encode_baud_rate() allow non-standard baud rates. I
> should clear it from c_cflag to indicate I don't support it.
> 
> > > > >  
> > > > I think you can call this unconditionally together with case >
> > > > 115200.
> > > 
> > > The calls are orthogonal. This one deals with the case when
> > > BOTHER
> > > is
> > > defined and set, and we have non-zero rate with BOTHER, but we
> > > have
> > > zero rate after BOTHER is cleared. So we set 9600 as a sane
> > > default
> > > speed.

Maybe you just set a baud rate nearest to the one from the table in
case of BOTHER?

In that case perhaps you have to supply +-1 to the range. That's why I
asked about uart_get_baud_rate(). 

Maybe this flow will work for you

if (BOTHER)
 clear BOTHER
 call uart_get_baud_rate()

?

> The warning seems to be the result of initializing a spinlock with
> zero. Spinlocks are intentionally obfuscated, but I didn't
> investigate
> further.
> 
> > $ git grep -n 'struct .* = {0};' | wc -l
> > 338
> > 
> > $ git grep -n 'struct .* = { \?0 \?};' | wc -l
> > 550
> > 
> > ( '… = { 0 };' included)
> 
> The first structure member is most likely not a spinlock in those
> cases.

Hmm... Interesting. On one hand the poison is reasonable, on the other
we often do a memset() or {0} on structures, i.o.w. assign 0 as initial
value until spinlock_init().

Arnd, what do you think about this (and similar) case(s)?

-- 
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Intel Finland Oy