linux-kernel - Re: [RFC 2/7] tty: add support for "tty slave" devices

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]

Date:	Sun, 14 Aug 2016 13:36:12 +0200
From:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:	Sebastian Reichel <sre@...nel.org>
Cc:	Tony Lindgren <tony@...mide.com>, Rob Herring <robh+dt@...nel.org>,
	Mark Rutland <mark.rutland@....com>,
	Marcel Holtmann <marcel@...tmann.org>,
	Jiri Slaby <jslaby@...e.com>, Ville Tervo <ville.tervo@....fi>,
	Filip Matijević <filip.matijevic.pz@...il.com>,
	Aaro Koskinen <aaro.koskinen@....fi>,
	Pavel Machek <pavel@....cz>,
	Pali Rohár <pali.rohar@...il.com>,
	ivo.g.dimitrov.75@...il.com, linux-bluetooth@...r.kernel.org,
	linux-serial@...r.kernel.org, linux-omap@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	NeilBrown <neilb@...e.de>
Subject: Re: [RFC 2/7] tty: add support for "tty slave" devices

On Sat, Aug 13, 2016 at 10:31:24PM +0200, Sebastian Reichel wrote:
> Hi,
> 
> On Sat, Aug 13, 2016 at 12:03:45PM +0200, Greg Kroah-Hartman wrote:
> > On Sat, Aug 13, 2016 at 05:14:33AM +0200, Sebastian Reichel wrote:
> > > From: NeilBrown <neilb@...e.de>
> > > 
> > > A "tty slave" is a device connected via UART.  It may need a driver to,
> > > for example, power the device on when the tty is opened, and power it
> > > off when the tty is released.
> > > 
> > > Signed-off-by: NeilBrown <neilb@...e.de>
> > > Signed-off-by: Sebastian Reichel <sre@...nel.org>
> > > ---
> > >  Documentation/devicetree/bindings/serial/8250.txt | 4 ++++
> > >  drivers/tty/tty_io.c                              | 6 ++++++
> > >  2 files changed, 10 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/serial/8250.txt b/Documentation/devicetree/bindings/serial/8250.txt
> > > index f5561ac7e17e..ecb74730f71b 100644
> > > --- a/Documentation/devicetree/bindings/serial/8250.txt
> > > +++ b/Documentation/devicetree/bindings/serial/8250.txt
> > > @@ -46,6 +46,10 @@ Optional properties:
> > >    line respectively. It will use specified GPIO instead of the peripheral
> > >    function pin for the UART feature. If unsure, don't specify this property.
> > >  
> > > +Optional child node:
> > > +- a device connected to the uart can be specified as child node with
> > > +  compatible value.
> > > +
> > >  Note:
> > >  * fsl,ns16550:
> > >    ------------
> > > diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
> > > index 734a635e7363..39ff5dcdfd50 100644
> > > --- a/drivers/tty/tty_io.c
> > > +++ b/drivers/tty/tty_io.c
> > > @@ -95,6 +95,7 @@
> > >  #include <linux/seq_file.h>
> > >  #include <linux/serial.h>
> > >  #include <linux/ratelimit.h>
> > > +#include <linux/of_platform.h>
> > >  
> > >  #include <linux/uaccess.h>
> > >  
> > > @@ -3317,6 +3318,11 @@ struct device *tty_register_device_attr(struct tty_driver *driver,
> > >  	retval = device_register(dev);
> > >  	if (retval)
> > >  		goto error;
> > > +	if (device && device->of_node)
> > > +		/* Children are platform devices and will be
> > > +		 * runtime_pm managed by this tty.
> > > +		 */
> > > +		of_platform_populate(device->of_node, NULL, NULL, dev);
> >
> > Why are these platform devices?
> > And why only OF?
> 
> I just took this patch over from Neil to get bluetooth working
> on N900/N950. Both of them are DT only (well N900 still has
> boardcode, but that will be removed shortly), so it was enough
> for me.
> 
> I guess you have something in mind, that's similar to e.g. i2c
> and spi with a serial_client device and support to instanciate
> it from DT, ACPI and boardcode?

I'm not going to accept a OF-only patch to the tty layer like this,
especially one that abuses platform drivers.

See my response to Pavel for what this "should" look like.

thanks,

greg k-h