LinuxLists.cc - [PATCH 0/3] Add support for 'tty-slaves' described by devicetree.

2014-12-11 22:00:36

Subject: [PATCH 0/3] Add support for 'tty-slaves' described by devicetree.

Greetings TTY and Devicetree folks.

On my phone (GTA04) there are two devices which are each accessed via
a UART: the bluetooth module and the GPS.

I would like these to be powered on when the relevant /dev/ttyXX
is opened, and to be powered off when the tty is closed.

This patch series adds support for "tty slaves" and creates two
appropriate drivers of this type.

A "tty slave" is a platform device which is primarily attached by a
TTY (i.e. a uart). Like other platform devices it might have other
interconnections like gpios and regulators etc.

In devicetree, any child node of a uart with a 'compatible' attribute
is treated as a 'tty slave'. This means that it is probed like any
other platform device. Also, when the tty is opened the device is
powered on by taking a pm_runtime reference.. Similarly when the tty is
closed the pm_runtime reference is dropped.

One of the drivers is a generic "tty-regulator" drive which simply
enables a regulator when powered on, and disables it when no longer
in use. This suffices for my bluetooth device.

The other handles the slightly awkward details of powering on my
particular GPS as well as a regulator which powers the antenna.
It also registers an 'rfkill' which can power-off the antenna
independently of the TTY.

Comments and review most welcome.

Thanks,
NeilBrown

---

NeilBrown (3):
TTY: add support for "tty slave" devices.
TTY: add slave driver to power-on device via a regulator.
TTY/slave: add driver for w2sg0004 GPS

.../devicetree/bindings/serial/of-serial.txt | 4
.../devicetree/bindings/serial/slave-reg.txt | 18 +
.../devicetree/bindings/serial/slave-w2sg0004.txt | 35 ++
drivers/tty/Kconfig | 2
drivers/tty/Makefile | 1
drivers/tty/slaves/Kconfig | 18 +
drivers/tty/slaves/Makefile | 3
drivers/tty/slaves/tty-reg.c | 102 +++++
drivers/tty/slaves/tty-w2sg0004.c | 412 ++++++++++++++++++++
drivers/tty/tty_io.c | 22 +
10 files changed, 617 insertions(+)
create mode 100644 Documentation/devicetree/bindings/serial/slave-reg.txt
create mode 100644 Documentation/devicetree/bindings/serial/slave-w2sg0004.txt
create mode 100644 drivers/tty/slaves/Kconfig
create mode 100644 drivers/tty/slaves/Makefile
create mode 100644 drivers/tty/slaves/tty-reg.c
create mode 100644 drivers/tty/slaves/tty-w2sg0004.c

--
Signature

2014-12-11 22:00:43

by NeilBrown

[permalink] [raw]

Subject: [PATCH 1/3] TTY: add support for "tty slave" devices.

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.

A "tty slave" is a platform device which is declared as a
child of the uart in device-tree:

&uart1 {
bluetooth {
compatible = "tty,regulator";
vdd-supply = <&vaux4>;
};
};

runtime power management is used to power-up the device
on tty_open() and power-down on tty_release().

Signed-off-by: NeilBrown <[email protected]>
---
.../devicetree/bindings/serial/of-serial.txt | 4 ++++
drivers/tty/tty_io.c | 22 ++++++++++++++++++++
2 files changed, 26 insertions(+)

diff --git a/Documentation/devicetree/bindings/serial/of-serial.txt b/Documentation/devicetree/bindings/serial/of-serial.txt
index 8c4fd0332028..b59501ee2f21 100644
--- a/Documentation/devicetree/bindings/serial/of-serial.txt
+++ b/Documentation/devicetree/bindings/serial/of-serial.txt
@@ -39,6 +39,10 @@ Optional properties:
driver is allowed to detect support for the capability even without this
property.

+Optional child node:
+- a platform device listed as a child node will be probed and
+ powered-on whenever the tty is in use (open).
+
Example:

uart@80230000 {
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 0508a1d8e4cd..7acdc6f093f4 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -95,6 +95,8 @@
#include <linux/seq_file.h>
#include <linux/serial.h>
#include <linux/ratelimit.h>
+#include <linux/pm_runtime.h>
+#include <linux/of_platform.h>

#include <linux/uaccess.h>

@@ -1683,6 +1685,17 @@ static int tty_release_checks(struct tty_struct *tty, struct tty_struct *o_tty,
return 0;
}

+static int open_child(struct device *dev, void *data)
+{
+ pm_runtime_get_sync(dev);
+ return 0;
+}
+static int release_child(struct device *dev, void *data)
+{
+ pm_runtime_put_autosuspend(dev);
+ return 0;
+}
+
/**
* tty_release - vfs callback for close
* @inode: inode of tty
@@ -1712,6 +1725,8 @@ int tty_release(struct inode *inode, struct file *filp)
long timeout = 0;
int once = 1;

+ if (tty->dev)
+ device_for_each_child(tty->dev, NULL, release_child);
if (tty_paranoia_check(tty, inode, __func__))
return 0;

@@ -2118,6 +2133,8 @@ retry_open:
__proc_set_tty(current, tty);
spin_unlock_irq(&current->sighand->siglock);
tty_unlock(tty);
+ if (tty->dev)
+ device_for_each_child(tty->dev, NULL, open_child);
mutex_unlock(&tty_mutex);
return 0;
err_unlock:
@@ -3207,6 +3224,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;

2014-12-11 22:00:51

by NeilBrown

[permalink] [raw]

Subject: [PATCH 2/3] TTY: add slave driver to power-on device via a regulator.

The regulator is identified in devicetree as 'vdd-supply'

Signed-off-by: NeilBrown <[email protected]>
---
.../devicetree/bindings/serial/slave-reg.txt | 18 ++++
drivers/tty/Kconfig | 2
drivers/tty/Makefile | 1
drivers/tty/slaves/Kconfig | 12 ++
drivers/tty/slaves/Makefile | 2
drivers/tty/slaves/tty-reg.c | 102 ++++++++++++++++++++
6 files changed, 137 insertions(+)
create mode 100644 Documentation/devicetree/bindings/serial/slave-reg.txt
create mode 100644 drivers/tty/slaves/Kconfig
create mode 100644 drivers/tty/slaves/Makefile
create mode 100644 drivers/tty/slaves/tty-reg.c

diff --git a/Documentation/devicetree/bindings/serial/slave-reg.txt b/Documentation/devicetree/bindings/serial/slave-reg.txt
new file mode 100644
index 000000000000..7896bce8dfe4
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/slave-reg.txt
@@ -0,0 +1,18 @@
+Regulator powered UART-attached device
+
+Required properties:
+- compatible: "tty,regulator"
+- vdd-supply: regulator to power the device
+
+
+This is listed as a child node of a UART. When the
+UART is opened, the device is powered.
+
+Example:
+
+&uart1 {
+ bluetooth {
+ compatible = "tty,regulator";
+ vdd-supply = <&vaux4>;
+ };
+};
diff --git a/drivers/tty/Kconfig b/drivers/tty/Kconfig
index b24aa010f68c..ea3383c71701 100644
--- a/drivers/tty/Kconfig
+++ b/drivers/tty/Kconfig
@@ -419,4 +419,6 @@ config DA_CONSOLE
help
This enables a console on a Dash channel.

+source drivers/tty/slaves/Kconfig
+
endif # TTY
diff --git a/drivers/tty/Makefile b/drivers/tty/Makefile
index 58ad1c05b7f8..45957d671e33 100644
--- a/drivers/tty/Makefile
+++ b/drivers/tty/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_R3964) += n_r3964.o
obj-y += vt/
obj-$(CONFIG_HVC_DRIVER) += hvc/
obj-y += serial/
+obj-$(CONFIG_TTY_SLAVE) += slaves/

# tty drivers
obj-$(CONFIG_AMIGA_BUILTIN_SERIAL) += amiserial.o
diff --git a/drivers/tty/slaves/Kconfig b/drivers/tty/slaves/Kconfig
new file mode 100644
index 000000000000..2dd1acf80f8c
--- /dev/null
+++ b/drivers/tty/slaves/Kconfig
@@ -0,0 +1,12 @@
+menuconfig TTY_SLAVE
+ bool "TTY slave devices"
+ help
+ Devices which attach via a uart, but need extra
+ driver support for power management etc.
+
+if TTY_SLAVE
+
+config TTY_SLAVE_REGULATOR
+ tristate "TTY slave device powered by regulator"
+
+endif
diff --git a/drivers/tty/slaves/Makefile b/drivers/tty/slaves/Makefile
new file mode 100644
index 000000000000..e636bf49f1cc
--- /dev/null
+++ b/drivers/tty/slaves/Makefile
@@ -0,0 +1,2 @@
+
+obj-$(CONFIG_TTY_SLAVE_REGULATOR) += tty-reg.o
diff --git a/drivers/tty/slaves/tty-reg.c b/drivers/tty/slaves/tty-reg.c
new file mode 100644
index 000000000000..613f8b10967c
--- /dev/null
+++ b/drivers/tty/slaves/tty-reg.c
@@ -0,0 +1,102 @@
+/*
+ * tty-reg:
+ * Support for any device which needs a regulator turned on
+ * when a tty is opened.
+ */
+
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/err.h>
+#include <linux/regulator/consumer.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/tty.h>
+
+struct tty_reg_data {
+ struct regulator *reg;
+ bool reg_enabled;
+};
+
+static int tty_reg_runtime_resume(struct device *slave)
+{
+ struct tty_reg_data *data = dev_get_drvdata(slave);
+ if (!data->reg_enabled &&
+ regulator_enable(data->reg) == 0) {
+ dev_dbg(slave, "power on\n");
+ data->reg_enabled = true;
+ }
+ return 0;
+}
+
+static int tty_reg_runtime_suspend(struct device *slave)
+{
+ struct tty_reg_data *data = dev_get_drvdata(slave);
+
+ if (data->reg_enabled &&
+ regulator_disable(data->reg) == 0) {
+ dev_dbg(slave, "power off\n");
+ data->reg_enabled = false;
+ }
+ return 0;
+}
+
+static int tty_reg_probe(struct platform_device *pdev)
+{
+ struct tty_reg_data *data;
+ struct regulator *reg;
+ int err;
+
+ err = -ENODEV;
+ if (pdev->dev.parent == NULL)
+ goto out;
+ reg = devm_regulator_get(&pdev->dev, "vdd");
+ err = PTR_ERR(reg);
+ if (IS_ERR(reg))
+ goto out;
+
+ data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+ err = -ENOMEM;
+ if (!data)
+ goto out;
+ data->reg = reg;
+ data->reg_enabled = false;
+ pm_runtime_enable(&pdev->dev);
+ platform_set_drvdata(pdev, data);
+ err = 0;
+out:
+ return err;
+}
+
+static struct of_device_id tty_reg_dt_ids[] = {
+ { .compatible = "tty,regulator", },
+ {}
+};
+
+static const struct dev_pm_ops tty_reg_pm_ops = {
+ SET_RUNTIME_PM_OPS(tty_reg_runtime_suspend,
+ tty_reg_runtime_resume, NULL)
+};
+
+static struct platform_driver tty_reg_driver = {
+ .driver.name = "tty-regulator",
+ .driver.owner = THIS_MODULE,
+ .driver.of_match_table = tty_reg_dt_ids,
+ .driver.pm = &tty_reg_pm_ops,
+ .probe = tty_reg_probe,
+};
+
+static int __init tty_reg_init(void)
+{
+ return platform_driver_register(&tty_reg_driver);
+}
+module_init(tty_reg_init);
+
+static void __exit tty_reg_exit(void)
+{
+ platform_driver_unregister(&tty_reg_driver);
+}
+module_exit(tty_reg_exit);
+
+MODULE_AUTHOR("NeilBrown <[email protected]>");
+MODULE_DESCRIPTION("Serial port device which requires a regulator when open.");
+MODULE_LICENSE("GPL v2");

2014-12-11 22:01:01

by NeilBrown

On Fri, 12 Dec 2014 08:02:48 -0500
> Which brings up another point: why not do this in the runtime power management;
> ie, turn on the slave device when the master device is turned on? Sebastian
> recently made the 8250 driver power-managed per access, which would enable
> significant power savings over just leaving the slave device on the whole time.

That per access model only works on 8250 (in fact only on some 8250). On
most devices if the UART is in low power you lose bytes send to it rather
than then queuing up anywhere.

It doesn't handle cases where you need to keep power on to different rules
(for example there are cases where you do things like power the uart down
after no bits have been received for 'n' millseconds). Right now if you
look into Androidspace you'll find people doing this in userspace by
having the sysfs node open and playing crazy games with sysfs, Android
glue hacks and gpio poking via the gpio sysfs/gpio lib routines.

Good example is some of the GPS and serial based sensor chips. The
latency of tty open/close is unacceptable and risks losing bits so they
keep the tty open and whack the power management by hand right now.

The kind of thing many of them want is stuff like

"hold power on until we see receive data, then keep it on until it shuts
up for a bit"

"assert gpio, send, receive, wait idle, drop gpio"

"on GPIO interrupt wake uart, wait for data, when idle, power uart down"

> >> 2) why is this tied to the tty core and not the serial core
> >> if this is only for UART?
> >
> > Because the knowledge of "the device is being opened" or "the device is being
> > closed" seems to exist in the tty core but not in the serial core.
>
> uart_open() = device file is being opened
> uart_close() = device file is being released

Only for a subset of uart type devices that use the uart layer. Quite a
few don't, including all the USB ones, and the sd ones.

> > I'm happy to move it to serial_core.c if that is preferred.
> > I'll also move the open/close handling if you can point to where it should go.
>
> Yes, please.

NAK - I certainly object to it being moved to uart. That's the wrong
abstraction layer for this. It's a generic behaviour, it's a tty level
behaviour without any uart specific properties. It should work for uarts
that don't use the legacy serial_core code, uarts that do, uarts that use
USB, uarts that are directly connected to things like sdio etc.

That means it has to be tty layer.

Alan

2014-12-13 17:46:48

Hi!

> index 8c4fd0332028..b59501ee2f21 100644
> --- a/Documentation/devicetree/bindings/serial/of-serial.txt
> +++ b/Documentation/devicetree/bindings/serial/of-serial.txt
> @@ -39,6 +39,10 @@ Optional properties:
> driver is allowed to detect support for the capability even without this
> property.
>
> +Optional child node:
> +- a platform device listed as a child node will be probed and
> + powered-on whenever the tty is in use (open).
> +
> Example:
>
> uart@80230000 {

Hmm. Other devices may want it the other way around: probe the device
behind the uart, make it control power and open/close of the uart, and
hide the /dev/ttyXX from userspace...
Pavel

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