linux-kernel - Re: [PATCH] tty: serial: 8250: remove U6715 16550A auto-detection

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: <20130321082821.GA2862@xps8300>
Date:	Thu, 21 Mar 2013 10:28:21 +0200
From:	Heikki Krogerus <heikki.krogerus@...ux.intel.com>
To:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:	Philippe Langlais <philippe.langlais@...com>,
	linux-kernel@...r.kernel.org, linux-serial@...r.kernel.org,
	Hans-Peter Oeri <hp@...i.ch>
Subject: Re: [PATCH] tty: serial: 8250: remove U6715 16550A auto-detection

Hi guys,

On Wed, Mar 20, 2013 at 10:21:25AM -0700, Greg Kroah-Hartman wrote:
> On Tue, Mar 19, 2013 at 08:50:09PM +0100, Philippe Langlais wrote:
> > -	if (up->port.type == PORT_16550A && size_fifo(up) == 64) {
> > -		up->port.type = PORT_U6_16550A;
> > -		up->capabilities |= UART_CAP_AFE;
> > -	}
> 
> So you just broke this functionality for all of the other devices it
> does work properly for?  That's not nice, and not acceptable.
> 
> Please fix this properly.

Is there actually anything depending on this quirk? It looks like
U6715 support newer made it to upstream, and I believe the type
PORT_U6_16550A is designed for U6715 only.

In any case, replacing the type PORT_16550A with a type that has
custom capabilities based on fifosize alone feels dangerous to me. 64B
fifo for 16550 compatible UARTs is very common these days. 

Since we have this conditions in autoconfig_16550a(), then would it be
OK to replace it with something like:

-       /*
-        * We distinguish between 16550A and U6 16550A by counting
-        * how many bytes are in the FIFO.
-        */
-       if (up->port.type == PORT_16550A && size_fifo(up) == 64) {
-               up->port.type = PORT_U6_16550A;
-               up->capabilities |= UART_CAP_AFE;
-       }
+       if (up->port.type == PORT_16550A && size_fifo(up) == 64)
+               up->port.fifosize = 64;
+
+       if (up->port.type == PORT_16550A && size_fifo(up) == 128)
+               up->port.fifosize = 128;
 }


We set the port->up.fifosize for a few common fifo sizes, like 64 and
128, instead of replacing the type and capabilities?

Thanks,

-- 
heikki
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/