by Marcel Holtmann

On Tue, 23 Aug 2016 00:00:17 +0200
Pavel Machek <[email protected]> wrote:

> On Mon 2016-08-22 22:32:23, One Thousand Gnomes wrote:
> > > why would we even have it create a /dev/ttyX for these devices in the first place. Lets just not create an uevent for it and lets not create a dev_t for it.
> >
> > Because if you don't it's a regression. It's not permissible to break
> > existing userspace.
>
> Well... it would be good to do the right thing, at least in the places
> where we can.
>
> Yes, renumbering people's serials is bad, OTOH for new platforms it
> would be nice not to expose ttyS15 which can only return -EBUSY.

That would still be a regression. Not everyone even uses the kernel
bluetooth stack. It would only return EBUSY if you had done an "up" on it
via the direct bluetooth stack.

Alan

2016-08-22 22:52:56

by Alan Cox

[permalink] [raw]

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

> There are usb-serial devices, which could benefit from support
> btw. I would find it really useful, if the Dangerous Prototype's
> Bus Pirate would expose native /dev/i2c and /dev/spi and it's
> based on FT232.

That should just need an ldisc. I2C and SPI should at this point be sane
for hotplugging as needed for an ldisc.

And having an ldisc also has another nice effect. You can plug the bus
pirate into a remote machine, run an 8bit clean link over a pty/tty pair
half way around the world and get a local i2c/spi to the remote machine's
i2c/spi bus pirate ports and devices.

It also means that if Bus Pirate 5 changes USB uart nothing breaks.

Alan

2016-08-22 22:46:10

by Alan Cox

Hi Sebastian,

> Am 22.08.2016 um 22:39 schrieb Sebastian Reichel <[email protected]>:
>
> Hi,
>
> On Sun, Aug 21, 2016 at 09:50:57AM +0200, H. Nikolaus Schaller wrote:
>>> Am 20.08.2016 um 15:34 schrieb One Thousand Gnomes <[email protected]>:
>>>> What it is not about are UART/RS232 converters connected through USB or virtual
>>>> serial ports created for WWAN modems (e.g. /dev/ttyACM, /dev/ttyHSO). Or BT devices
>>>> connected through USB (even if they also run HCI protocol).
>>>
>>> It actually has to be about both because you will find the exact same
>>> device wired via USB SSIC/HSIC to a USB UART or via a classic UART. Not is
>>> it just about embedded boards.
>>
>> Not necessarily.
>>
>> We often have two interface options for exactly the sam sensor chips. They can be connected
>> either through SPI or I2C. Which means that there is a core driver for the chip and two different
>> transport glue components (see e.g. iio/accel/bmc150).
>>
>> This does not require I2C to be able to handle SPI or vice versa or provide a common API.
>
> I don't understand this comparison. I2C and SPI are different
> protocols,

Yes, they are different on protocol level, but on both you transfer blocks of data from/to a slave device
which usually can be addressed. And for some chips they are just two slightly alternative serial interfaces.

> while native UART and USB-connected UART are both UART.

I see what you mean, but kernel divides between directly connected UART and USB-connected UART.

drivers/usb/serial/ vs. drivers/tty/serial/

to implement two different groups of UARTs. Although on user space level they are harmonized again.
This is why I compare with i2c and spi. But each such comparison is not perfect.

Anyways, to me it looks as if everybody wants to make the solution work for usb-uarts as well
(although I still would like to see a real world use-case).

>
>> And most Bluetooth devices I know have either UART or a direct
>> USB interface. So in the USB case there is no need to connect
>> it through some USB-UART bridge and treat it as an UART at all.
>
> I think having support for USB-UART dongles is useful for
> driver development and testing on non-embedded HW.

Hm. I assume you mean the Bluetooth situation where both, embedded UART
connected chips and USB dongles are available. I am not a specialist for such things,
but I think you have three options to connect bluetooth:

a) SoC-UART <-> BT-Chip-UART-port
b) USB-UART (FT232, PL2303 etc.) <-> BT-Chip-UART-port
c) USB <-> BT-Chip-USB-port (not UART involved at all)

Case c) IMHO means you anyways need a special USB driver for the BT-Chip connected
through USB and plugging it into a non-embedded USB port does not automatically
show it as a tty interface. So you can't use it for testing the UART drivers.

BTW: the Wi2Wi W2CBW003 chip comes in two firmware variants: one for UART and
one for USB. So they are also not exchangeable.

Variant b) is IMHO of no practical relevance (but I may be wrong) because it would
mean to add some costly FT232 or PL2302 chip where a different firmware variant works
with direct USB connection.

So to me it looks as if you need to develop different low-level drivers anyways.

BR,
Nikolaus

Attachments:

signature.asc (801.00 B)
Message signed with OpenPGP using GPGMail

2016-08-22 21:16:21

by Marcel Holtmann

[permalink] [raw]

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

Hi Alan,

>>>> - a child of the uart node
>>>> - a reg property containing the line number if the parent has multiple
>>>> uarts (I'd expect this to rarely be used).
>>>
>>> That surprises me as for current x86 platforms it would be the norm,
>>> except that we use ACPI.
>>
>> Exactly, we're talking DT bindings here. Each port will be a separate
>> node otherwise things like serial aliases and stdout-path won't work
>> correctly. Compatible strings for 8250 uarts are for a single port.
>> But if you had h/w such that it has common and per port registers then
>> it may be a single node. I'm not aware of any example offhand (maybe
>> PPC CPM). But it doesn't matter as reg can handle this case just fine
>> if we need to.
>
> For the tty side by the way here's a first RFC of one approach we could
> take. This should (unless I missed anything) allow the core tty framework
> to be used directly from a kernel created tty object rather than one
> backed by a file.
>
> commit fcd072e755594f9c9c0533d45223f56f76e3d104
> Author: Alan <[email protected]>
> Date: Mon Aug 22 18:05:56 2016 +0100
>
> [RFC] tty_port: allow a port to be opened with a tty that has no file handle
>
> Let us create tty objects entirely in kernel space. Untested proposal to
> show why all the ideas around rewriting half the uart stack are not needed.
>
> With this a kernel created non file backed tty object could be used to handle
> data, and set terminal modes. Not all ldiscs can cope with this as N_TTY in
> particular has to work back to the fs/tty layer.
>
> The tty_port code is however otherwise clean of file handles as far as I can
> tell as is the low level tty port write path used by the ldisc, the
> configuration low level interfaces and most of the ldiscs.
>
> Currently you don't have any exposure to see tty hangups because those are
> built around the file layer. However a) it's a fixed port so you probably
> don't care about that b) if you do we can add a callback and c) you almost
> certainly don't want the userspace tear down/rebuild behaviour anyway.
>
> This should however be sufficient if we wanted for example to enumerate all
> the bluetooth bound fixed ports via ACPI and make them directly available.
>
> It doesn't deal with the case of a user opening a port that's also kernel
> opened and that would need some locking out (so it returned EBUSY if bound
> to a kernel device of some kind). That needs resolving along with how you
> "up" or "down" your new bluetooth device, or enumerate it while providing
> the existing tty API to avoid regressions (and to debug).

why would we even have it create a /dev/ttyX for these devices in the first place. Lets just not create an uevent for it and lets not create a dev_t for it.

The Bluetooth power on (aka hciconfig hci0 up/down) is already solved with modern Intel and Broadcom UART drivers. Essentially we attach the ldisc and tell it which vendor it is. That is it. Everything else is done inside the kernel from that point on.

So attaching the ldisc (via btattach tool) is similar to plugging in a dongle via USB. It runs that basic setup like firmware loading and configuration and then goes back into sleep mode. Only when powering the Bluetooth hciX device up (via bluetoothd or manually) it gets out of sleep mode. And the details for that are left up to the driver.

Internally the setup stage does a hciconfig hci0 up and it is already abstracted out that way. So there has been a lot of work in the Bluetooth subsystem to allow for this. That part is really solved.

Regards

Marcel

2016-08-22 21:07:06

by Marcel Holtmann

[permalink] [raw]

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

Hi Alan,

>> Would it make sense then to define a DT binding that can cover these
>> four cases independent of the Linux usage:
>>
>> a) an existing tty line discipline matched to a tty port
>> b) a serio device using the N_MOUSE line discipline (which
>> happens to cover non-mouse devices these days)
>
> These two are the same basic thing
>
> port x expects ldisc y {with properties ...}
>
>> c) a uart_port slave attached directly to the uart (like in your
>> current code)
>
> c) needs to be a tty_port slave attached directly to a tty_port.
> Important detail, but as uart_port is just a subset of tty_port it's a
> trivial detail to the DT.
>
>> d) the same slave drivers using a new tty line discipline
>
> and this is also just a/b again.
>
>
> What use cases and connectivity do you need to describe. Looking at the
> ACPI platforms we have
>
> - the expected serial port configuration
> - the properties of the port (FIFO etc)
> - the power management for the port
>
> - the children of the port
> - the power management of the children (at a very simplistic abstracted
> level)
>
> So we want to be able to describe something like
>
> ttyS0 {
> baud: 1152008N1
> protocol: bluetooth hci
> fixed: yes
> powermanagement: { ... }
> }

we also need to know what Bluetooth vendor is there. Since we need to match the vendor to the firmware loading and configuration.

Additionally there might be PCM audio configurations that need to be considered. Since we have to configure direct PCM interconnect with the audio codec.

> and if I look at the usermode crapfest on a lot of Android systems it
> looks similar but with the notion of things like being able to describe
>
> - Use GPIO mode sleeping and assume first char is X to save power
>
> - Power up, wait n ms, write, read, wait n ms, power down (which
> has to be driven at the ldisc/user level as only the ldisc
> understands transactions, or via ioctls (right now Android user
> space tends to do hardcoded writes to /sys.. gpio to drive power
>
> - And a few variants thereof (power up on write, off on a timer
> etc)

Actually the sad part about the Android mess is that we can fix it for Bluetooth. We have HCI User Channel that allows to grab a HCI device and assign it to Bluedroid stack on Android. So it would work with whatever bus or whatever vendor is underneath. All this hacking would go away. And we have used this successfully for Intel based Android platforms. We know this works.

> So I can imagine wanting to describe something like
>
> - The bluetooth HCI hardware is managed by gpio 11 (or UART DTR,
> or PMIC n etc)
> The uart can switch into GPIO mode and is gpio 15
>
> or
>
> - Raise gpio 4 when writing, drop it after 50mS with no read/write
>
> Then the ldisc needs to make port->ops. calls for enabling/disabling low
> power mode and expected char, and the uarts that can do it need to
> implement the gpio/uart switching and any timers.

I now wonder if we can not just turn the ldisc into a bus. So we have a ldisc bus that exposes devices that have no business of having a userspace /dev/ttyX exposed. And our Bluetooth UART support just turns into a ldisc driver on the ldisc bus.

One of the problems is that attaching the ldisc from userspace you still need to figure out what /dev/ttyX you get assigned in the end. And figure out which one is the Bluetooth UART. If we want single images where things just work out of the box, we need to get extra information for doing auto-detection. So some sort of bus enumeration is key to make this work smoothly.

Regards

Marcel

2016-08-22 20:39:48

by Sebastian Reichel

[permalink] [raw]

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

Hi,

On Sun, Aug 21, 2016 at 09:50:57AM +0200, H. Nikolaus Schaller wrote:
> > Am 20.08.2016 um 15:34 schrieb One Thousand Gnomes <[email protected]>:
> >> What it is not about are UART/RS232 converters connected through USB or virtual
> >> serial ports created for WWAN modems (e.g. /dev/ttyACM, /dev/ttyHSO). Or BT devices
> >> connected through USB (even if they also run HCI protocol).
> >
> > It actually has to be about both because you will find the exact same
> > device wired via USB SSIC/HSIC to a USB UART or via a classic UART. Not is
> > it just about embedded boards.
>
> Not necessarily.
>
> We often have two interface options for exactly the sam sensor chips. They can be connected
> either through SPI or I2C. Which means that there is a core driver for the chip and two different
> transport glue components (see e.g. iio/accel/bmc150).
>
> This does not require I2C to be able to handle SPI or vice versa or provide a common API.

I don't understand this comparison. I2C and SPI are different
protocols, while native UART and USB-connected UART are both UART.

> And most Bluetooth devices I know have either UART or a direct
> USB interface. So in the USB case there is no need to connect
> it through some USB-UART bridge and treat it as an UART at all.

I think having support for USB-UART dongles is useful for
driver development and testing on non-embedded HW.

-- Sebastian

Attachments:

(No filename) (1.38 kB)
signature.asc (819.00 B)
Download all attachments

2016-08-22 20:00:28

Am 19.08.2016 um 19:50 schrieb H. Nikolaus Schaller:
> Hi,
>
>> Am 19.08.2016 um 09:49 schrieb Oleksij Rempel <[email protected]>:
>>
>> Hallo Nikolaus,
>>
>> do i understand it correctly. This driver is to make kind of interchip
>> communication and represent uart as a bus to allow use this bus from
>> multiple kernel driver or expose it to user space?
>
> The idea for UART slave devices is to handle devices connected on an
> embedded board to an UART port in kernel. Currently most such devices
> are just passed through to some /dev/tty and handled by user-space daemons.
>
> So it is not necessarily about multiple kernel drivers to use the same UART, although
> that could also be required.
>
> A single one is already difficult... And some scenarios need to shield the UART
> from user space (currently there is always one /dev/tty per UART - unless the
> UART is completely disabled).
>
> Some ideas where it might be needed:
> * bluetooth HCI over UART
> * a weird GPS device whose power state can only reliably be detected by monitoring data activity
> * other chips (microcontrollers) connected through UART - similar to I2C slave devices
> * it potentially could help to better implement IrDA (although that is mostly legacy)
>
> What it is not about are UART/RS232 converters connected through USB or virtual
> serial ports created for WWAN modems (e.g. /dev/ttyACM, /dev/ttyHSO). Or BT devices
> connected through USB (even if they also run HCI protocol).

Ah... ok. thank you for explanation.

I was thinking it is going in similar direction with my project - use
SPI for communication between two SoCs. It is based on SSI32 protocol
from Bosch.

In case it is going to this direction:
Master implementation for linux side (tested on Banana Pi and iMX6):
https://github.com/olerem/linux-2.6/commits/bpi-spi-variant2-2016.07.26.2

Slave implementation for stm32f303 (tested on f3 discovery):
https://github.com/olerem/libopencm3-examples/commits/ssi32-2016.08.17.1

protocol decoder for logic analyzer (sigrok):
https://github.com/olerem/libsigrokdecode/commits/ssi32_dec-2016.08.11

>> Correct?
>>
>> Am 19.08.2016 um 09:29 schrieb H. Nikolaus Schaller:
>>> Hi,
>>>
>>>> Am 19.08.2016 um 07:21 schrieb Sebastian Reichel <[email protected]>:
>>>>
>>>> Hi,
>>>>
>>>> On Thu, Aug 18, 2016 at 06:08:24PM -0500, Rob Herring wrote:
>>>>> On Thu, Aug 18, 2016 at 3:29 PM, Sebastian Reichel <[email protected]> wrote:
>>>>>> Thanks for going forward and implementing this. I also started,
>>>>>> but was far from a functional state.
>>>>>>
>>>>>> On Wed, Aug 17, 2016 at 08:14:42PM -0500, Rob Herring wrote:
>>>>>>> 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.
>>>>>>> - Split out the controller for uart_ports into separate driver. Do we see
>>>>>>> a need for controller drivers that are not standard serial drivers?
>>>>>>> - 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)?
>>>>>>> - Test with other UART drivers
>>>>>>> - Convert a real driver/line discipline over to UART bus.
>>>>>>>
>>>>>>> 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).
>>>>>>
>>>>>> I had a look at the uart_dev API:
>>>>>>
>>>>>> int uart_dev_config(struct uart_device *udev, int baud, int parity, int bits, int flow);
>>>>>> int uart_dev_connect(struct uart_device *udev);
>>>>>>
>>>>>> The flow control configuration should be done separately. e.g.:
>>>>>> uart_dev_flow_control(struct uart_device *udev, bool enable);
>>>>>
>>>>> No objection, but out of curiosity, why?
>>>>
>>>> Nokia's bluetooth uart protocol disables flow control during speed
>>>> changes.
>>>>
>>>>>> int uart_dev_tx(struct uart_device *udev, u8 *buf, size_t count);
>>>>>> int uart_dev_rx(struct uart_device *udev, u8 *buf, size_t count);
>>>>>>
>>>>>> UART communication does not have to be host-initiated, so this
>>>>>> API requires polling. Either some function similar to poll in
>>>>>> userspace is needed, or it should be implemented as callback.
>>>>>
>>>>> What's the userspace need?
>>>>
>>>> I meant "Either some function similar to userspace's poll() is
>>>> needed, ...". Something like uart_dev_wait_for_rx()
>>>>
>>>> Alternatively the rx function could be a callback, that
>>>> is called when there is new data.
>>>>
>>>>> I'm assuming the only immediate consumers are in-kernel.
>>>>
>>>> Yes, but the driver should be notified about incoming data.
>>>
>>> Yes, this is very important.
>>>
>>> If possible, please do a callback for every character that arrives.
>>> And not only if the rx buffer becomes full, to give the slave driver
>>> a chance to trigger actions almost immediately after every character.
>>> This probably runs in interrupt context and can happen often.
>>>
>>> In our proposal some months ago we have implemented such
>>> an rx_notification callback for every character. This allows to work
>>> without rx buffer and implement one in the driver if needed. This
>>> gives the driver full control over the rx buffer dimensions.
>>>
>>> And we have made the callback to return a boolean flag which
>>> tells if the character is to be queued in the tty layer so that the
>>> driver can decide for every byte if it is to be hidden from user
>>> space or passed. Since we pass a pointer, the driver could even
>>> modify the character passed back, but we have not used this
>>> feature.
>>>
>>> This should cover most (but certainly not all) situations of
>>> implementing protocol engines in uart slave drivers.
>>>
>>> Our API therefore was defined as:
>>>
>>> void uart_register_slave(struct uart_port *uport, void *slave);
>>> void uart_register_rx_notification(struct uart_port *uport,
>>> bool (*function)(void *slave, unsigned int *c),
>>> struct ktermios *termios);
>>>
>>> Registering a slave appears to be comparable to uart_dev_connect()
>>> and registering an rx_notification combines uart_dev_config() and
>>> setting the callback.
>>>
>>> Unregistration is done by passing a NULL pointer for 'slave' or 'function'.
>>>
>>> If there will be a very similar API with a callback like this, we won't have
>>> to change our driver architecture much.
>>>
>>> If there is a uart_dev_wait_for_rx() with buffer it is much more difficult
>>> to handle.
>>>
>>> BR,
>>> Nikolaus
>>>
>>>
>>
>>
>> --
>> Regards,
>> Oleksij
>>
>

--
Regards,
Oleksij

Attachments:

signature.asc (213.00 B)
OpenPGP digital signature

2016-08-19 17:50:45

Hi,

On Fri, Aug 19, 2016 at 12:38:08PM +0100, One Thousand Gnomes wrote:
> There are also some other slight complications when you look at
> real world implementations. Android devices tend to keep the GPS
> in userspace so most of them can't use some magic extra API but
> just drive GPIO lines via the sysfs GPIO interface. Most Android
> doesn't use the kernel BT stack either.

I don't get the reasoning for this one. What has it to do with
an in-kernel API? People are also using libusb or doing i2c/spi
from userspace. Nevertheless we have an in-kernel API for those.

> Quite a few Android and other embedded devices also do power
> management by shutting off the UART, routing the rx line to an
> edge triggered GPIO and on the interrupt flipping the UART back
> on and losing the first byte, picking a protocol that can recover
> from it.
>
> Your model doesn't I think cover that, although I am somewhat at a
> loss as to how to do that nicely!

On OMAP this is supported by the serial driver via runtime PM and
wakeirq.

Actually my usecase for the API (bluetooth on Nokia N900, N950, N9),
there is an extra GPIO for the power management (high = uart
should be able to receive sth., low = uart can sleep). For this
I can just disable the wakeirq in the UART by not adding it to DT
and instead runtime manage it from the uart_dev child device.

While we are on that topic: The omap-serial driver does not enable
runtime PM by default, since it does not know if the remote side is
ok with loosing the first byte(s). One is expected to enable it
using the sysfs API.
But I think it should be safe to enable runtime PM for the serial
device in uart_dev_connect(). Due to the child device it will still
be kept disabled, except when the uart_dev also implements runtime_pm.

-- Sebastian

Attachments:

(No filename) (1.75 kB)
signature.asc (819.00 B)
Download all attachments

2016-08-19 14:44:29

Hallo Nikolaus,

do i understand it correctly. This driver is to make kind of interchip
communication and represent uart as a bus to allow use this bus from
multiple kernel driver or expose it to user space?

Correct?

Am 19.08.2016 um 09:29 schrieb H. Nikolaus Schaller:
> Hi,
>
>> Am 19.08.2016 um 07:21 schrieb Sebastian Reichel <[email protected]>:
>>
>> Hi,
>>
>> On Thu, Aug 18, 2016 at 06:08:24PM -0500, Rob Herring wrote:
>>> On Thu, Aug 18, 2016 at 3:29 PM, Sebastian Reichel <[email protected]> wrote:
>>>> Thanks for going forward and implementing this. I also started,
>>>> but was far from a functional state.
>>>>
>>>> On Wed, Aug 17, 2016 at 08:14:42PM -0500, Rob Herring wrote:
>>>>> 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.
>>>>> - Split out the controller for uart_ports into separate driver. Do we see
>>>>> a need for controller drivers that are not standard serial drivers?
>>>>> - 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)?
>>>>> - Test with other UART drivers
>>>>> - Convert a real driver/line discipline over to UART bus.
>>>>>
>>>>> 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).
>>>>
>>>> I had a look at the uart_dev API:
>>>>
>>>> int uart_dev_config(struct uart_device *udev, int baud, int parity, int bits, int flow);
>>>> int uart_dev_connect(struct uart_device *udev);
>>>>
>>>> The flow control configuration should be done separately. e.g.:
>>>> uart_dev_flow_control(struct uart_device *udev, bool enable);
>>>
>>> No objection, but out of curiosity, why?
>>
>> Nokia's bluetooth uart protocol disables flow control during speed
>> changes.
>>
>>>> int uart_dev_tx(struct uart_device *udev, u8 *buf, size_t count);
>>>> int uart_dev_rx(struct uart_device *udev, u8 *buf, size_t count);
>>>>
>>>> UART communication does not have to be host-initiated, so this
>>>> API requires polling. Either some function similar to poll in
>>>> userspace is needed, or it should be implemented as callback.
>>>
>>> What's the userspace need?
>>
>> I meant "Either some function similar to userspace's poll() is
>> needed, ...". Something like uart_dev_wait_for_rx()
>>
>> Alternatively the rx function could be a callback, that
>> is called when there is new data.
>>
>>> I'm assuming the only immediate consumers are in-kernel.
>>
>> Yes, but the driver should be notified about incoming data.
>
> Yes, this is very important.
>
> If possible, please do a callback for every character that arrives.
> And not only if the rx buffer becomes full, to give the slave driver
> a chance to trigger actions almost immediately after every character.
> This probably runs in interrupt context and can happen often.
>
> In our proposal some months ago we have implemented such
> an rx_notification callback for every character. This allows to work
> without rx buffer and implement one in the driver if needed. This
> gives the driver full control over the rx buffer dimensions.
>
> And we have made the callback to return a boolean flag which
> tells if the character is to be queued in the tty layer so that the
> driver can decide for every byte if it is to be hidden from user
> space or passed. Since we pass a pointer, the driver could even
> modify the character passed back, but we have not used this
> feature.
>
> This should cover most (but certainly not all) situations of
> implementing protocol engines in uart slave drivers.
>
> Our API therefore was defined as:
>
> void uart_register_slave(struct uart_port *uport, void *slave);
> void uart_register_rx_notification(struct uart_port *uport,
> bool (*function)(void *slave, unsigned int *c),
> struct ktermios *termios);
>
> Registering a slave appears to be comparable to uart_dev_connect()
> and registering an rx_notification combines uart_dev_config() and
> setting the callback.
>
> Unregistration is done by passing a NULL pointer for 'slave' or 'function'.
>
> If there will be a very similar API with a callback like this, we won't have
> to change our driver architecture much.
>
> If there is a uart_dev_wait_for_rx() with buffer it is much more difficult
> to handle.
>
> BR,
> Nikolaus
>
>

--
Regards,
Oleksij

Attachments:

signature.asc (213.00 B)
OpenPGP digital signature

2016-08-19 07:29:38

by H. Nikolaus Schaller

[permalink] [raw]

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

Hi,

> Am 19.08.2016 um 07:21 schrieb Sebastian Reichel <[email protected]>:
>
> Hi,
>
> On Thu, Aug 18, 2016 at 06:08:24PM -0500, Rob Herring wrote:
>> On Thu, Aug 18, 2016 at 3:29 PM, Sebastian Reichel <[email protected]> wrote:
>>> Thanks for going forward and implementing this. I also started,
>>> but was far from a functional state.
>>>
>>> On Wed, Aug 17, 2016 at 08:14:42PM -0500, Rob Herring wrote:
>>>> 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.
>>>> - Split out the controller for uart_ports into separate driver. Do we see
>>>> a need for controller drivers that are not standard serial drivers?
>>>> - 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)?
>>>> - Test with other UART drivers
>>>> - Convert a real driver/line discipline over to UART bus.
>>>>
>>>> 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).
>>>
>>> I had a look at the uart_dev API:
>>>
>>> int uart_dev_config(struct uart_device *udev, int baud, int parity, int bits, int flow);
>>> int uart_dev_connect(struct uart_device *udev);
>>>
>>> The flow control configuration should be done separately. e.g.:
>>> uart_dev_flow_control(struct uart_device *udev, bool enable);
>>
>> No objection, but out of curiosity, why?
>
> Nokia's bluetooth uart protocol disables flow control during speed
> changes.
>
>>> int uart_dev_tx(struct uart_device *udev, u8 *buf, size_t count);
>>> int uart_dev_rx(struct uart_device *udev, u8 *buf, size_t count);
>>>
>>> UART communication does not have to be host-initiated, so this
>>> API requires polling. Either some function similar to poll in
>>> userspace is needed, or it should be implemented as callback.
>>
>> What's the userspace need?
>
> I meant "Either some function similar to userspace's poll() is
> needed, ...". Something like uart_dev_wait_for_rx()
>
> Alternatively the rx function could be a callback, that
> is called when there is new data.
>
>> I'm assuming the only immediate consumers are in-kernel.
>
> Yes, but the driver should be notified about incoming data.

Yes, this is very important.

If possible, please do a callback for every character that arrives.
And not only if the rx buffer becomes full, to give the slave driver
a chance to trigger actions almost immediately after every character.
This probably runs in interrupt context and can happen often.

In our proposal some months ago we have implemented such
an rx_notification callback for every character. This allows to work
without rx buffer and implement one in the driver if needed. This
gives the driver full control over the rx buffer dimensions.

And we have made the callback to return a boolean flag which
tells if the character is to be queued in the tty layer so that the
driver can decide for every byte if it is to be hidden from user
space or passed. Since we pass a pointer, the driver could even
modify the character passed back, but we have not used this
feature.

This should cover most (but certainly not all) situations of
implementing protocol engines in uart slave drivers.

Our API therefore was defined as:

void uart_register_slave(struct uart_port *uport, void *slave);
void uart_register_rx_notification(struct uart_port *uport,
bool (*function)(void *slave, unsigned int *c),
struct ktermios *termios);

Registering a slave appears to be comparable to uart_dev_connect()
and registering an rx_notification combines uart_dev_config() and
setting the callback.

Unregistration is done by passing a NULL pointer for 'slave' or 'function'.

If there will be a very similar API with a callback like this, we won't have
to change our driver architecture much.

If there is a uart_dev_wait_for_rx() with buffer it is much more difficult
to handle.

BR,
Nikolaus

Attachments:

signature.asc (801.00 B)
Message signed with OpenPGP using GPGMail

2016-08-19 05:21:26

[permalink] [raw]

Subject: [RFC PATCH 1/3] uart bus: Introduce new bus for UART slave devices

The UART bus is designed for slave devices such as Bluetooth, WiFi, GPS
and NFC connected to UARTs on host processors. Tradionally these have
been handled with tty line disciplines, rfkill, and userspace glue such
as hci_attach. This approach has many drawbacks since it doesn't fit
into the Linux driver model. Handling of sideband signals, power control
and firmware loading are the main issues.

This creates a UART bus with controllers (i.e. host UARTs) and slave
devices. Typically, these are point to point connections, but some
devices have muxing protocols or a h/w mux is conceivable. Any muxing is
not yet supported.

Signed-off-by: Rob Herring <[email protected]>
---
drivers/Kconfig | 2 +
drivers/Makefile | 1 +
drivers/uart/Kconfig | 17 ++
drivers/uart/Makefile | 3 +
drivers/uart/core.c | 458 ++++++++++++++++++++++++++++++++++++++++++++
drivers/uart/loopback.c | 72 +++++++
include/linux/uart_device.h | 163 ++++++++++++++++
7 files changed, 716 insertions(+)
create mode 100644 drivers/uart/Kconfig
create mode 100644 drivers/uart/Makefile
create mode 100644 drivers/uart/core.c
create mode 100644 drivers/uart/loopback.c
create mode 100644 include/linux/uart_device.h

diff --git a/drivers/Kconfig b/drivers/Kconfig
index e1e2066..9145494 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -202,4 +202,6 @@ source "drivers/hwtracing/intel_th/Kconfig"

source "drivers/fpga/Kconfig"

+source "drivers/uart/Kconfig"
+
endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index 53abb4a..c915417 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -173,3 +173,4 @@ obj-$(CONFIG_STM) += hwtracing/stm/
obj-$(CONFIG_ANDROID) += android/
obj-$(CONFIG_NVMEM) += nvmem/
obj-$(CONFIG_FPGA) += fpga/
+obj-$(CONFIG_UART_DEV) += uart/
diff --git a/drivers/uart/Kconfig b/drivers/uart/Kconfig
new file mode 100644
index 0000000..b232ce3
--- /dev/null
+++ b/drivers/uart/Kconfig
@@ -0,0 +1,17 @@
+#
+# UART slave driver configuration
+#
+menuconfig UART_DEV
+ tristate "UART device support"
+ help
+ Support for devices connected via a UART
+
+if UART_DEV
+
+config UART_DEV_LOOPBACK
+ tristate "Loopback Test Driver"
+ help
+ This option enables a loopback test driver. Anything received from
+ the connected device will be echoed back.
+
+endif
diff --git a/drivers/uart/Makefile b/drivers/uart/Makefile
new file mode 100644
index 0000000..704eec0
--- /dev/null
+++ b/drivers/uart/Makefile
@@ -0,0 +1,3 @@
+obj-$(CONFIG_UART_DEV) := core.o
+
+obj-$(CONFIG_UART_DEV_LOOPBACK) += loopback.o
diff --git a/drivers/uart/core.c b/drivers/uart/core.c
new file mode 100644
index 0000000..5b5d0e1
--- /dev/null
+++ b/drivers/uart/core.c
@@ -0,0 +1,458 @@
+/*
+ * Copyright (C) 2016 Linaro Ltd.
+ * Author: Rob Herring <[email protected]>
+ *
+ * Based on drivers/spmi/spmi.c:
+ * Copyright (c) 2012-2015, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+#define DEBUG
+
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/idr.h>
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/uart_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/serial_core.h>
+
+static bool is_registered;
+static DEFINE_IDA(ctrl_ida);
+
+static void uart_dev_release(struct device *dev)
+{
+ struct uart_device *udev = to_uart_device(dev);
+ kfree(udev);
+}
+
+static const struct device_type uart_dev_type = {
+ .release = uart_dev_release,
+};
+
+static void uart_ctrl_release(struct device *dev)
+{
+ struct uart_controller *ctrl = to_uart_controller(dev);
+ ida_simple_remove(&ctrl_ida, ctrl->nr);
+ kfree(ctrl);
+}
+
+static const struct device_type uart_ctrl_type = {
+ .release = uart_ctrl_release,
+};
+
+static int uart_device_match(struct device *dev, struct device_driver *drv)
+{
+ if (of_driver_match_device(dev, drv))
+ return 1;
+
+ if (drv->name)
+ return strncmp(dev_name(dev), drv->name,
+ SPMI_NAME_SIZE) == 0;
+
+ return 0;
+}
+
+/**
+ * uart_device_add() - add a device previously constructed via uart_device_alloc()
+ * @udev: uart_device to be added
+ */
+int uart_device_add(struct uart_device *udev)
+{
+ struct uart_controller *ctrl = udev->ctrl;
+ int err;
+
+ dev_set_name(&udev->dev, "uartdev-%d", ctrl->nr);
+
+ err = device_add(&udev->dev);
+ if (err < 0) {
+ dev_err(&udev->dev, "Can't add %s, status %d\n",
+ dev_name(&udev->dev), err);
+ goto err_device_add;
+ }
+
+ dev_dbg(&udev->dev, "device %s registered\n", dev_name(&udev->dev));
+
+err_device_add:
+ return err;
+}
+EXPORT_SYMBOL_GPL(uart_device_add);
+
+/**
+ * uart_device_remove(): remove an SPMI device
+ * @udev: uart_device to be removed
+ */
+void uart_device_remove(struct uart_device *udev)
+{
+ device_unregister(&udev->dev);
+}
+EXPORT_SYMBOL_GPL(uart_device_remove);
+
+int uart_dev_config(struct uart_device *udev, int baud, int parity, int bits, int flow)
+{
+ return uart_set_options(udev->ctrl->port, NULL, baud, parity, bits, flow);
+}
+
+int uart_dev_connect(struct uart_device *udev)
+{
+ unsigned long page;
+ struct uart_port *port = udev->ctrl->port;
+ struct uart_state *state = port->state;
+
+ WARN_ON(!state);
+
+ if (!state->xmit.buf) {
+ /* This is protected by the per port mutex */
+ page = get_zeroed_page(GFP_KERNEL);
+ if (!page)
+ return -ENOMEM;
+
+ state->xmit.buf = (unsigned char *) page;
+ uart_circ_clear(&state->xmit);
+ }
+
+ if (!udev->ctrl->recv.buf) {
+ /* This is protected by the per port mutex */
+ page = get_zeroed_page(GFP_KERNEL);
+ if (!page)
+ return -ENOMEM;
+
+ udev->ctrl->recv.buf = (unsigned char *) page;
+ uart_circ_clear(&udev->ctrl->recv);
+ }
+
+ if (port && port->ops->pm)
+ port->ops->pm(port, UART_PM_STATE_ON, state->pm_state);
+ state->pm_state = UART_PM_STATE_ON;
+
+ port->ops->set_mctrl(port, TIOCM_RTS | TIOCM_DTR);
+
+ return udev->ctrl->port->ops->startup(udev->ctrl->port);
+}
+
+int uart_dev_rx(struct uart_device *udev, u8 *buf, size_t count)
+{
+ struct uart_port *port = udev->ctrl->port;
+ struct uart_state *state = port->state;
+ struct circ_buf *circ = &udev->ctrl->recv;
+ int c, ret = 0;
+
+ if (!circ->buf)
+ return 0;
+
+ while (1) {
+ c = CIRC_CNT_TO_END(circ->head, circ->tail, UART_XMIT_SIZE);
+ if (count < c)
+ c = count;
+ if (c <= 0)
+ break;
+ memcpy(buf, circ->buf + circ->tail, c);
+ circ->tail = (circ->tail + c) & (PAGE_SIZE - 1);
+
+ buf += c;
+ count -= c;
+ ret += c;
+ }
+
+ return ret;
+}
+
+int uart_dev_tx(struct uart_device *udev, u8 *buf, size_t count)
+{
+ struct uart_port *port = udev->ctrl->port;
+ struct uart_state *state = port->state;
+ struct circ_buf *circ = &state->xmit;
+ unsigned long flags;
+ int c, ret = 0;
+
+ if (!circ->buf)
+ return 0;
+
+// port = uart_port_lock(state, flags);
+ while (1) {
+ c = CIRC_SPACE_TO_END(circ->head, circ->tail, UART_XMIT_SIZE);
+ if (count < c)
+ c = count;
+ if (c <= 0)
+ break;
+ memcpy(circ->buf + circ->head, buf, c);
+ circ->head = (circ->head + c) & (UART_XMIT_SIZE - 1);
+ buf += c;
+ count -= c;
+ ret += c;
+ }
+ port->ops->start_tx(port);
+
+// uart_port_unlock(port, flags);
+
+ return ret;
+}
+
+
+static int uart_drv_probe(struct device *dev)
+{
+ const struct uart_dev_driver *sdrv = to_uart_dev_driver(dev->driver);
+ struct uart_device *udev = to_uart_device(dev);
+ int err;
+
+ pm_runtime_get_noresume(dev);
+ pm_runtime_set_active(dev);
+ pm_runtime_enable(dev);
+
+ err = sdrv->probe(udev);
+ if (err)
+ goto fail_probe;
+
+ return 0;
+
+fail_probe:
+ pm_runtime_disable(dev);
+ pm_runtime_set_suspended(dev);
+ pm_runtime_put_noidle(dev);
+ return err;
+}
+
+static int uart_drv_remove(struct device *dev)
+{
+ const struct uart_dev_driver *sdrv = to_uart_dev_driver(dev->driver);
+
+ pm_runtime_get_sync(dev);
+ sdrv->remove(to_uart_device(dev));
+ pm_runtime_put_noidle(dev);
+
+ pm_runtime_disable(dev);
+ pm_runtime_set_suspended(dev);
+ pm_runtime_put_noidle(dev);
+ return 0;
+}
+
+static struct bus_type uart_bus_type = {
+ .name = "uart",
+ .match = uart_device_match,
+ .probe = uart_drv_probe,
+ .remove = uart_drv_remove,
+};
+
+/**
+ * uart_controller_alloc() - Allocate a new SPMI device
+ * @ctrl: associated controller
+ *
+ * Caller is responsible for either calling uart_device_add() to add the
+ * newly allocated controller, or calling uart_device_put() to discard it.
+ */
+struct uart_device *uart_device_alloc(struct uart_controller *ctrl)
+{
+ struct uart_device *udev;
+
+ udev = kzalloc(sizeof(*udev), GFP_KERNEL);
+ if (!udev)
+ return NULL;
+
+ udev->ctrl = ctrl;
+ device_initialize(&udev->dev);
+ udev->dev.parent = &ctrl->dev;
+ udev->dev.bus = &uart_bus_type;
+ udev->dev.type = &uart_dev_type;
+ return udev;
+}
+EXPORT_SYMBOL_GPL(uart_device_alloc);
+
+/**
+ * uart_controller_alloc() - Allocate a new SPMI controller
+ * @parent: parent device
+ * @size: size of private data
+ *
+ * Caller is responsible for either calling uart_controller_add() to add the
+ * newly allocated controller, or calling uart_controller_put() to discard it.
+ * The allocated private data region may be accessed via
+ * uart_controller_get_drvdata()
+ */
+struct uart_controller *uart_controller_alloc(struct device *parent,
+ size_t size)
+{
+ struct uart_controller *ctrl;
+ int id;
+
+ if (WARN_ON(!parent))
+ return NULL;
+
+ ctrl = kzalloc(sizeof(*ctrl) + size, GFP_KERNEL);
+ if (!ctrl)
+ return NULL;
+
+ device_initialize(&ctrl->dev);
+ ctrl->dev.type = &uart_ctrl_type;
+ ctrl->dev.bus = &uart_bus_type;
+ ctrl->dev.parent = parent;
+ ctrl->dev.of_node = parent->of_node;
+ uart_controller_set_drvdata(ctrl, &ctrl[1]);
+
+ id = ida_simple_get(&ctrl_ida, 0, 0, GFP_KERNEL);
+ if (id < 0) {
+ dev_err(parent,
+ "unable to allocate SPMI controller identifier.\n");
+ uart_controller_put(ctrl);
+ return NULL;
+ }
+
+ ctrl->nr = id;
+ dev_set_name(&ctrl->dev, "uart-%d", id);
+
+ dev_dbg(&ctrl->dev, "allocated controller 0x%p id %d\n", ctrl, id);
+ return ctrl;
+}
+EXPORT_SYMBOL_GPL(uart_controller_alloc);
+
+static void of_uart_register_devices(struct uart_controller *ctrl)
+{
+ struct device_node *node;
+ int err;
+
+ if (!ctrl->dev.of_node)
+ return;
+
+ for_each_available_child_of_node(ctrl->dev.of_node, node) {
+ struct uart_device *udev;
+ u32 reg[2];
+
+ dev_dbg(&ctrl->dev, "adding child %s\n", node->full_name);
+
+ udev = uart_device_alloc(ctrl);
+ if (!udev)
+ continue;
+
+ udev->dev.of_node = node;
+
+ err = uart_device_add(udev);
+ if (err) {
+ dev_err(&udev->dev,
+ "failure adding device. status %d\n", err);
+ uart_device_put(udev);
+ }
+ }
+}
+
+/**
+ * uart_controller_add() - Add an SPMI controller
+ * @ctrl: controller to be registered.
+ *
+ * Register a controller previously allocated via uart_controller_alloc() with
+ * the SPMI core.
+ */
+int uart_controller_add(struct uart_controller *ctrl)
+{
+ int ret;
+
+ /* Can't register until after driver model init */
+ if (WARN_ON(!is_registered))
+ return -EAGAIN;
+
+ ret = device_add(&ctrl->dev);
+ if (ret)
+ return ret;
+
+ if (IS_ENABLED(CONFIG_OF))
+ of_uart_register_devices(ctrl);
+
+ dev_dbg(&ctrl->dev, "uart-%d registered: dev:%p\n",
+ ctrl->nr, &ctrl->dev);
+
+ return 0;
+};
+EXPORT_SYMBOL_GPL(uart_controller_add);
+
+int uart_controller_rx(struct uart_controller *ctrl, unsigned int ch)
+{
+ struct circ_buf *circ = &ctrl->recv;
+ unsigned long flags;
+ int c, ret = 0;
+
+ if (!circ->buf)
+ return -ENODEV;
+
+ c = CIRC_SPACE_TO_END(circ->head, circ->tail, PAGE_SIZE);
+ if (c <= 0)
+ return 0;
+
+ circ->buf[circ->head] = ch;
+ circ->head = (circ->head + 1) & (PAGE_SIZE - 1);
+
+ return 1;
+}
+
+/* Remove a device associated with a controller */
+static int uart_ctrl_remove_device(struct device *dev, void *data)
+{
+ struct uart_device *spmidev = to_uart_device(dev);
+ if (dev->type == &uart_dev_type)
+ uart_device_remove(spmidev);
+ return 0;
+}
+
+/**
+ * uart_controller_remove(): remove an SPMI controller
+ * @ctrl: controller to remove
+ *
+ * Remove a SPMI controller. Caller is responsible for calling
+ * uart_controller_put() to discard the allocated controller.
+ */
+void uart_controller_remove(struct uart_controller *ctrl)
+{
+ int dummy;
+
+ if (!ctrl)
+ return;
+
+ dummy = device_for_each_child(&ctrl->dev, NULL,
+ uart_ctrl_remove_device);
+ device_del(&ctrl->dev);
+}
+EXPORT_SYMBOL_GPL(uart_controller_remove);
+
+/**
+ * uart_driver_register() - Register client driver with SPMI core
+ * @sdrv: client driver to be associated with client-device.
+ *
+ * This API will register the client driver with the SPMI framework.
+ * It is typically called from the driver's module-init function.
+ */
+int __uart_dev_driver_register(struct uart_dev_driver *sdrv, struct module *owner)
+{
+ sdrv->driver.bus = &uart_bus_type;
+ sdrv->driver.owner = owner;
+ return driver_register(&sdrv->driver);
+}
+EXPORT_SYMBOL_GPL(__uart_driver_register);
+
+static void __exit uart_exit(void)
+{
+ bus_unregister(&uart_bus_type);
+}
+module_exit(uart_exit);
+
+static int __init uart_init(void)
+{
+ int ret;
+
+ ret = bus_register(&uart_bus_type);
+ if (ret)
+ return ret;
+
+ is_registered = true;
+ return 0;
+}
+postcore_initcall(uart_init);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("UART module");
+//MODULE_ALIAS("platform:spmi");
diff --git a/drivers/uart/loopback.c b/drivers/uart/loopback.c
new file mode 100644
index 0000000..80b7769
--- /dev/null
+++ b/drivers/uart/loopback.c
@@ -0,0 +1,72 @@
+/*
+ * Copyright (C) 2016 Linaro Ltd.
+ * Author: Rob Herring <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/uart_device.h>
+#include <linux/workqueue.h>
+
+struct loopback_data {
+ struct uart_device *udev;
+ struct delayed_work work;
+};
+
+static void loopback_work(struct work_struct *work)
+{
+ struct loopback_data *data = container_of((struct delayed_work *)work, struct loopback_data, work);
+ struct uart_device *udev = data->udev;
+ int cnt;
+ char buf[64];
+
+ cnt = uart_dev_rx(udev, buf, sizeof(buf));
+ uart_dev_tx(udev, buf, cnt);
+
+ schedule_delayed_work(&data->work, 5);
+}
+
+static int loopback_probe(struct uart_device *udev)
+{
+ struct loopback_data *data;
+
+ data = devm_kzalloc(&udev->dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+ data->udev = udev;
+
+ dev_info(&udev->dev, "loopback probe!!!\n");
+
+ uart_dev_connect(udev);
+ uart_dev_config(udev, 115200, 'n', 8, 0);
+
+
+ INIT_DELAYED_WORK(&data->work, loopback_work);
+ schedule_delayed_work(&data->work, 100);
+
+ return 0;
+}
+
+static const struct of_device_id loopback_of_match[] = {
+ { .compatible = "loopback-uart", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, loopback_of_match);
+
+static struct uart_dev_driver loopback_driver = {
+ .probe = loopback_probe,
+ .driver = {
+ .name = "loopback-uart",
+ .of_match_table = of_match_ptr(loopback_of_match),
+ },
+};
+module_uart_dev_driver(loopback_driver);
diff --git a/include/linux/uart_device.h b/include/linux/uart_device.h
new file mode 100644
index 0000000..a91d778
--- /dev/null
+++ b/include/linux/uart_device.h
@@ -0,0 +1,163 @@
+/*
+ * Copyright (C) 2016 Linaro Ltd.
+ * Author: Rob Herring <[email protected]>
+ *
+ * Based on include/linux/spmi.h:
+ * Copyright (c) 2012-2013, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+#ifndef _LINUX_UART_DEVICE_H
+#define _LINUX_UART_DEVICE_H
+
+#include <linux/types.h>
+#include <linux/device.h>
+#include <linux/circ_buf.h>
+#include <linux/mod_devicetable.h>
+
+/**
+ * struct uart_device - Basic representation of an SPMI device
+ * @dev: Driver model representation of the device.
+ * @ctrl: SPMI controller managing the bus hosting this device.
+ * @usid: This devices' Unique Slave IDentifier.
+ */
+struct uart_device {
+ struct device dev;
+ struct uart_controller *ctrl;
+};
+
+static inline struct uart_device *to_uart_device(struct device *d)
+{
+ return container_of(d, struct uart_device, dev);
+}
+
+static inline void *uart_device_get_drvdata(const struct uart_device *sdev)
+{
+ return dev_get_drvdata(&sdev->dev);
+}
+
+static inline void uart_device_set_drvdata(struct uart_device *sdev, void *data)
+{
+ dev_set_drvdata(&sdev->dev, data);
+}
+
+struct uart_device *uart_device_alloc(struct uart_controller *ctrl);
+
+static inline void uart_device_put(struct uart_device *sdev)
+{
+ if (sdev)
+ put_device(&sdev->dev);
+}
+
+int uart_device_add(struct uart_device *sdev);
+
+void uart_device_remove(struct uart_device *sdev);
+
+struct uart_port;
+
+/**
+ * struct uart_controller - interface to the SPMI master controller
+ * @dev: Driver model representation of the device.
+ * @nr: board-specific number identifier for this controller/bus
+ * @cmd: sends a non-data command sequence on the SPMI bus.
+ * @read_cmd: sends a register read command sequence on the SPMI bus.
+ * @write_cmd: sends a register write command sequence on the SPMI bus.
+ */
+struct uart_controller {
+ struct device dev;
+ struct uart_port *port;
+ unsigned int nr;
+ struct circ_buf recv;
+};
+
+static inline struct uart_controller *to_uart_controller(struct device *d)
+{
+ return container_of(d, struct uart_controller, dev);
+}
+
+static inline
+void *uart_controller_get_drvdata(const struct uart_controller *ctrl)
+{
+ return dev_get_drvdata(&ctrl->dev);
+}
+
+static inline void uart_controller_set_drvdata(struct uart_controller *ctrl,
+ void *data)
+{
+ dev_set_drvdata(&ctrl->dev, data);
+}
+
+struct uart_controller *uart_controller_alloc(struct device *parent,
+ size_t size);
+
+/**
+ * uart_controller_put() - decrement controller refcount
+ * @ctrl SPMI controller.
+ */
+static inline void uart_controller_put(struct uart_controller *ctrl)
+{
+ if (ctrl)
+ put_device(&ctrl->dev);
+}
+
+int uart_controller_add(struct uart_controller *ctrl);
+void uart_controller_remove(struct uart_controller *ctrl);
+
+int uart_controller_rx(struct uart_controller *ctrl, unsigned int ch);
+
+/**
+ * struct spmi_driver - SPMI slave device driver
+ * @driver: SPMI device drivers should initialize name and owner field of
+ * this structure.
+ * @probe: binds this driver to a SPMI device.
+ * @remove: unbinds this driver from the SPMI device.
+ *
+ * If PM runtime support is desired for a slave, a device driver can call
+ * pm_runtime_put() from their probe() routine (and a balancing
+ * pm_runtime_get() in remove()). PM runtime support for a slave is
+ * implemented by issuing a SLEEP command to the slave on runtime_suspend(),
+ * transitioning the slave into the SLEEP state. On runtime_resume(), a WAKEUP
+ * command is sent to the slave to bring it back to ACTIVE.
+ */
+struct uart_dev_driver {
+ struct device_driver driver;
+ int (*probe)(struct uart_device *sdev);
+ void (*remove)(struct uart_device *sdev);
+};
+
+static inline struct uart_dev_driver *to_uart_dev_driver(struct device_driver *d)
+{
+ return container_of(d, struct uart_dev_driver, driver);
+}
+
+#define uart_dev_driver_register(sdrv) \
+ __uart_dev_driver_register(sdrv, THIS_MODULE)
+int __uart_dev_driver_register(struct uart_dev_driver *sdrv, struct module *owner);
+
+/**
+ * spmi_driver_unregister() - unregister an SPMI client driver
+ * @sdrv: the driver to unregister
+ */
+static inline void uart_dev_driver_unregister(struct uart_dev_driver *sdrv)
+{
+ if (sdrv)
+ driver_unregister(&sdrv->driver);
+}
+
+#define module_uart_dev_driver(__uart_dev_driver) \
+ module_driver(__uart_dev_driver, uart_dev_driver_register, \
+ uart_dev_driver_unregister)
+
+int uart_dev_config(struct uart_device *udev, int baud, int parity, int bits, int flow);
+int uart_dev_connect(struct uart_device *udev);
+int uart_dev_tx(struct uart_device *udev, u8 *buf, size_t count);
+int uart_dev_rx(struct uart_device *udev, u8 *buf, size_t count);
+
+#endif
--
2.9.2