Return-Path: Subject: Re: [RFC PATCH 0/3] UART slave device bus To: "H. Nikolaus Schaller" References: <20160818011445.22726-1-robh@kernel.org> <20160818202900.hyvm4hfxedifuefn@earth> <20160819052125.ze5zilppwoe3f2lx@earth> <53A846F1-33E5-48C3-B3A6-DB251661CDD5@goldelico.com> Cc: Sebastian Reichel , Rob Herring , Greg Kroah-Hartman , Marcel Holtmann , Jiri Slaby , Pavel Machek , Peter Hurley , NeilBrown , Arnd Bergmann , Linus Walleij , "open list:BLUETOOTH DRIVERS" , "linux-serial@vger.kernel.org" , "linux-kernel@vger.kernel.org" From: Oleksij Rempel Message-ID: Date: Fri, 19 Aug 2016 22:19:53 +0200 MIME-Version: 1.0 In-Reply-To: <53A846F1-33E5-48C3-B3A6-DB251661CDD5@goldelico.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="liHdv0E9IWGai95bsdNlhDI6gUjGMDS8G" List-ID: This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --liHdv0E9IWGai95bsdNlhDI6gUjGMDS8G Content-Type: multipart/mixed; boundary="qif54pghWw8IPkaMKbJpoLOpAT9CV7At7" From: Oleksij Rempel To: "H. Nikolaus Schaller" Cc: Sebastian Reichel , Rob Herring , Greg Kroah-Hartman , Marcel Holtmann , Jiri Slaby , Pavel Machek , Peter Hurley , NeilBrown , Arnd Bergmann , Linus Walleij , "open list:BLUETOOTH DRIVERS" , "linux-serial@vger.kernel.org" , "linux-kernel@vger.kernel.org" Message-ID: Subject: Re: [RFC PATCH 0/3] UART slave device bus References: <20160818011445.22726-1-robh@kernel.org> <20160818202900.hyvm4hfxedifuefn@earth> <20160819052125.ze5zilppwoe3f2lx@earth> <53A846F1-33E5-48C3-B3A6-DB251661CDD5@goldelico.com> In-Reply-To: <53A846F1-33E5-48C3-B3A6-DB251661CDD5@goldelico.com> --qif54pghWw8IPkaMKbJpoLOpAT9CV7At7 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Am 19.08.2016 um 19:50 schrieb H. Nikolaus Schaller: > Hi, >=20 >> Am 19.08.2016 um 09:49 schrieb Oleksij Rempel = : >> >> 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? >=20 > 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 daem= ons. >=20 > So it is not necessarily about multiple kernel drivers to use the same = UART, although > that could also be required. >=20 > 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 - unle= ss the > UART is completely disabled). >=20 > 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 I2= C slave devices > * it potentially could help to better implement IrDA (although that is = mostly legacy) >=20 > 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). O= r 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 : >>>> >>>> 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 = 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, an= d >>>>>>> in userspace with utilities such as hciattach. >>>>>>> >>>>>>> There have been several attempts to improve support, but they suf= fer from >>>>>>> still being tied into the tty layer and/or abusing the platform b= us. This >>>>>>> is a prototype to show creating a proper UART bus for UART device= s. 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 th= e slave >>>>>>> device). It still needs lots of work and polish. >>>>>>> >>>>>>> TODOs: >>>>>>> - Figure out the port locking. mutex plus spinlock plus refcounti= ng? I'm >>>>>>> hoping all that complexity is from the tty layer and not needed h= ere. >>>>>>> - Split out the controller for uart_ports into separate driver. D= o we see >>>>>>> a need for controller drivers that are not standard serial driver= s? >>>>>>> - 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 buffe= r or >>>>>>> perhaps a different receive interface (e.g. direct to client buff= er)? >>>>>>> - 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 'func= tion'. >>> >>> 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 diff= icult >>> to handle. >>> >>> BR, >>> Nikolaus >>> >>> >> >> >> -- >> Regards, >> Oleksij >> >=20 --=20 Regards, Oleksij --qif54pghWw8IPkaMKbJpoLOpAT9CV7At7-- --liHdv0E9IWGai95bsdNlhDI6gUjGMDS8G Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iF4EAREIAAYFAle3aeoACgkQHwImuRkmbWnhoAD+N6uMO1BfLYcw27ySJbCN5qPa s1SLCzrsAU8rJd7NY+kA/0wnnaRrATdnRxGGC0y7tqwImVuNf15eDyu3ELhWh/xU =CUTx -----END PGP SIGNATURE----- --liHdv0E9IWGai95bsdNlhDI6gUjGMDS8G--