Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1461579 > unrolled thread

[RFC 2/7] tty: add support for "tty slave" devices

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [RFC 2/7] tty: add support for "tty slave" devices Sebastian Reichel <sre@kernel.org> - 2016-08-13 05:20 +0200
    Re: [RFC 2/7] tty: add support for "tty slave" devices Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-08-13 12:10 +0200
      Re: [RFC 2/7] tty: add support for "tty slave" devices Pavel Machek <pavel@ucw.cz> - 2016-08-14 10:50 +0200
        Re: [RFC 2/7] tty: add support for "tty slave" devices Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-08-14 13:40 +0200
      Re: [RFC 2/7] tty: add support for "tty slave" devices Sebastian Reichel <sre@kernel.org> - 2016-08-14 11:40 +0200
        Re: [RFC 2/7] tty: add support for "tty slave" devices Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-08-14 13:40 +0200

#1461579 — [RFC 2/7] tty: add support for "tty slave" devices

From: NeilBrown <neilb@suse.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@suse.de>
Signed-off-by: Sebastian Reichel <sre@kernel.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);
 
 	return dev;
 
-- 
2.8.1

[toc] | [next] | [standalone]

#1461613

On Sat, Aug 13, 2016 at 05:14:33AM +0200, Sebastian Reichel wrote:
> From: NeilBrown <neilb@suse.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@suse.de>
> Signed-off-by: Sebastian Reichel <sre@kernel.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?

thanks,

greg k-h

[toc] | [prev] | [next] | [standalone]

#1461709

On Sat 2016-08-13 12:03:45, Greg Kroah-Hartman wrote:
> On Sat, Aug 13, 2016 at 05:14:33AM +0200, Sebastian Reichel wrote:
> > From: NeilBrown <neilb@suse.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@suse.de>
> > Signed-off-by: Sebastian Reichel <sre@kernel.org>

Acked-by: Pavel Machek <pavel@ucw.cz>

> > @@ -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?

OF based systems are the only ones that have this problem, so that's
the only place where we can test this solution.

Given that these devices are connected over the UART, it seems right
to categorize them as platform devices... You can't connect PCI, SATA
or USB device over UART port.


									
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1461969

On Sun, Aug 14, 2016 at 10:48:22AM +0200, Pavel Machek wrote:
> On Sat 2016-08-13 12:03:45, Greg Kroah-Hartman wrote:
> > On Sat, Aug 13, 2016 at 05:14:33AM +0200, Sebastian Reichel wrote:
> > > From: NeilBrown <neilb@suse.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@suse.de>
> > > Signed-off-by: Sebastian Reichel <sre@kernel.org>
> 
> Acked-by: Pavel Machek <pavel@ucw.cz>
> 
> > > @@ -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?
> 
> OF based systems are the only ones that have this problem, so that's
> the only place where we can test this solution.
> 
> Given that these devices are connected over the UART, it seems right
> to categorize them as platform devices... You can't connect PCI, SATA
> or USB device over UART port.

No, that's a total abuse of the platform bus, please don't.

I've said before that a "serial" bus should be created and you can hang
devices off of it.  For some reason that message keeps getting
ignored...

thanks,

greg k-h

[toc] | [prev] | [next] | [standalone]

#1461754

[Multipart message — attachments visible in raw view] — view raw

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@suse.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@suse.de>
> > Signed-off-by: Sebastian Reichel <sre@kernel.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?

-- Sebastian

[toc] | [prev] | [next] | [standalone]

#1462009

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@suse.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@suse.de>
> > > Signed-off-by: Sebastian Reichel <sre@kernel.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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web