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

Rob Herring
2016-08-18T13:53:53Z

On Thu, Aug 18, 2016 at 5:53 AM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> 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