linux-kernel - Re: [RFC PATCH 0/3] UART slave device bus

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: <20160818105333.GA7031@kroah.com>
Date:	Thu, 18 Aug 2016 12:53:33 +0200
From:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:	Marcel Holtmann <marcel@...tmann.org>
Cc:	Rob Herring <robh@...nel.org>, Jiri Slaby <jslaby@...e.com>,
	Sebastian Reichel <sre@...nel.org>,
	Pavel Machek <pavel@....cz>,
	Peter Hurley <peter@...leysoftware.com>,
	NeilBrown <neil@...wn.name>,
	"Dr . H . Nikolaus Schaller" <hns@...delico.com>,
	Arnd Bergmann <arnd@...db.de>,
	Linus Walleij <linus.walleij@...aro.org>,
	"open list:BLUETOOTH DRIVERS" <linux-bluetooth@...r.kernel.org>,
	linux-serial@...r.kernel.org,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Loic Poulain <loic.poulain@...el.com>
Subject: Re: [RFC PATCH 0/3] UART slave device bus

On Thu, Aug 18, 2016 at 12:30:32PM +0200, Marcel Holtmann wrote:
> Hi Greg,
> 
> >> Currently, devices attached via a UART are not well supported in the
> >> kernel. The problem is the device support is done in tty line disciplines,
> >> various platform drivers to handle some sideband, and in userspace with
> >> utilities such as hciattach.
> >> 
> >> There have been several attempts to improve support, but they suffer from
> >> still being tied into the tty layer and/or abusing the platform bus. This
> >> is a prototype to show creating a proper UART bus for UART devices. It is
> >> tied into the serial core (really struct uart_port) below the tty layer
> >> in order to use existing serial drivers.
> >> 
> >> This is functional with minimal testing using the loopback driver and
> >> pl011 (w/o DMA) UART under QEMU (modified to add a DT node for the slave
> >> device). It still needs lots of work and polish.
> >> 
> >> TODOs:
> >> - Figure out the port locking. mutex plus spinlock plus refcounting? I'm
> >>  hoping all that complexity is from the tty layer and not needed here.
> > 
> > It should be.
> > 
> >> - Split out the controller for uart_ports into separate driver. Do we see
> >>  a need for controller drivers that are not standard serial drivers?
> > 
> > What do you mean by "controller" drivers here?  I didn't understand them
> > in the code.
> > 
> >> - Implement/test the removal paths
> >> - Fix the receive callbacks for more than character at a time (i.e. DMA)
> >> - Need better receive buffering than just a simple circular buffer or
> >>  perhaps a different receive interface (e.g. direct to client buffer)?
> > 
> > Why?  Is the code as-is slow?
> > 
> >> - Test with other UART drivers
> >> - Convert a real driver/line discipline over to UART bus.
> > 
> > That's going to be the real test, I recommend trying that as soon as
> > possible as it will show where the real pain points are :)
> 
> maybe we can get the Intel LnP driver ported over and see how that one
> works out. It is one of the more complex ones when it comes to
> bootloader and firmware loading. Maybe Loic can take a stab at this.
> We would then also see how we can map the ACPI tables into a driver.

Yes, I was going to complain about the OF-only bent of this patch, but I
figured it would get fixed up once Rob started to use a "real" machine
for his testing of this code :)

> >> Before I spend more time on this, I'm looking mainly for feedback on the
> >> general direction and structure (the interface with the existing serial
> >> drivers in particular).
> > 
> > Yes, I like the idea (minor nit, you still have SPMI in a lot of places
> > instead of UART), so I recommend keeping going with it.
> > 
> >> drivers/uart/Kconfig             |  17 ++
> >> drivers/uart/Makefile            |   3 +
> >> drivers/uart/core.c              | 458 +++++++++++++++++++++++++++++++++++++++
> >> drivers/uart/loopback.c          |  72 ++++++
> > 
> > Why not just put this in drivers/tty/uart/ ?
> 
> Is it really then a TTY at all. Would be the UART become the basic
> core for a TTY?

Hm, interesting idea.  Not for all TTYs of course, but for those that
are on UART devices, maybe?  How would a usb-serial device fit into that
picture?

> Having tty/uart/ seems a bit backward. Then again, it is just a
> directory name ;)

And as we know, naming is hard :)

thanks,

greg k-h