Return-Path: MIME-Version: 1.0 In-Reply-To: <20160818105333.GA7031@kroah.com> References: <20160818011445.22726-1-robh@kernel.org> <20160818102208.GA20476@kroah.com> <20160818105333.GA7031@kroah.com> From: Rob Herring Date: Thu, 18 Aug 2016 08:53:15 -0500 Message-ID: Subject: Re: [RFC PATCH 0/3] UART slave device bus To: Greg Kroah-Hartman Cc: Marcel Holtmann , Jiri Slaby , Sebastian Reichel , Pavel Machek , Peter Hurley , NeilBrown , "Dr . H . Nikolaus Schaller" , Arnd Bergmann , Linus Walleij , "open list:BLUETOOTH DRIVERS" , "linux-serial@vger.kernel.org" , Linux Kernel Mailing List , Loic Poulain Content-Type: text/plain; charset=UTF-8 List-ID: On Thu, Aug 18, 2016 at 5:53 AM, Greg Kroah-Hartman wrote: > 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 :) I fully expected that from you. :) It is no different than any other bus we have. Each discovery/enumeration method needs hooks for matching and creating devices. It just happens that DT is the only one added ATM. > >> >> 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? DT overlay. Just like greybus serial. :) That's a good question though as usb-serial doesn't use uart_port. Perhaps there needs to be a uart controller/host driver that's a line discipline so existing tty drivers can work. That somewhat defeats the point of getting line disciplines out of the picture, but would provide a solution for h/w hacking (and no worse than what can be supported today). Longer term, the drivers would need to be adapted to use the uart slave bus directly. Rob