[PATCH 0/2] tty slave devices support - version 2

[PATCH 0/2] tty slave devices support - version 2

[NeilBrown]

Thanks for all the great feedback.  I have incorporated a lot of it,
though not all ... partly because there was not yet unanimity on some
issues.

Big changes:
 - children of a uart are no longer automatically managed.
   The driver for the child device must register with the tty
   after which it will be told when the tty is opened or closed.
   The driver can then do whatever it likes, which may involve
   powering the device on.

 - I am now only providing a single drivers: serial-power-manager.
   It can be used for devices which only want power management.
   Currently it can provide this using a regulator and/or a
   toggle GPIO.  This one driver supports both of my devices.

non-changes:
 - The interface functionality is still provided by the 'tty'
   layer, not the 'serial' layer.  I have no strong feelings on
   this and doubt that I would have until some other user
   appeared for this functionality.  That would help show
   if the current arrangement was helpful or inconvenient.

 - We still treat any child node of a tty device which has a
   'compatible' field gets allocated a device.  If there is no
   platform driver which supports that 'compatible' value, then
   the device will be inactive.  Specific action will only be
   taken if there is a child node with a 'compatible' field for which
   there is a supporting driver.

small changes:
 - proper 'compatible' names are used
 - switch to use 'gpiod' instead of 'gpio'
 - no longer misuse the 'tty_' prefix.


Comments and suggestions always welcome,
Thanks,
NeilBrown

---

NeilBrown (2):
      TTY: add support for "tty slave" devices.
      misc: add a driver to power on/off UART attached devices.


 .../devicetree/bindings/misc/wi2wi,w2cbw003.txt    |   19 +
 .../devicetree/bindings/misc/wi2wi,w2sg0004.txt    |   37 +
 .../devicetree/bindings/serial/of-serial.txt       |    4 
 .../devicetree/bindings/vendor-prefixes.txt        |    1 
 drivers/misc/Kconfig                               |   12 
 drivers/misc/Makefile                              |    1 
 drivers/misc/serial-power-manager.c                |  494 ++++++++++++++++++++
 drivers/tty/tty_io.c                               |   73 +++
 include/linux/tty.h                                |   16 +
 9 files changed, 654 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/misc/wi2wi,w2cbw003.txt
 create mode 100644 Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
 create mode 100644 drivers/misc/serial-power-manager.c

--
Signature

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

[NeilBrown]

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 = "wi2wi,w2cbw003";
		vdd-supply = <&vaux4>;
	};
};

The driver can attach to the tty by calling
   tty_set_slave(dev->parent, dev, &slave_ops);

where slave_ops' is a set of interface that
the tty layer must call when appropriate.
Currently only 'open' and 'release' are defined.
They are called at first open and last close.
They cannot fail.

Signed-off-by: NeilBrown <neilb@suse.de>
---
 .../devicetree/bindings/serial/of-serial.txt       |    4 +
 drivers/tty/tty_io.c                               |   73 +++++++++++++++++++-
 include/linux/tty.h                                |   16 ++++
 3 files changed, 90 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/serial/of-serial.txt b/Documentation/devicetree/bindings/serial/of-serial.txt
index 8c4fd0332028..fc5d00c3c474 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 can
+  register with the tty for open/close events to manage power.
+
 Example:
 
 	uart@80230000 {
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 0508a1d8e4cd..6c67a3fd257e 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>
 
@@ -110,6 +111,13 @@
 #define TTY_PARANOIA_CHECK 1
 #define CHECK_TTY_COUNT 1
 
+struct tty_device {
+	struct device dev;
+
+	struct tty_slave_operations *slave_ops;
+	struct device *slave;
+};
+
 struct ktermios tty_std_termios = {	/* for the benefit of tty drivers  */
 	.c_iflag = ICRNL | IXON,
 	.c_oflag = OPOST | ONLCR,
@@ -1825,6 +1833,17 @@ int tty_release(struct inode *inode, struct file *filp)
 				__func__, tty->count, tty_name(tty, buf));
 		tty->count = 0;
 	}
+	if (tty->dev && tty->count == 0) {
+		struct tty_device *ttyd = container_of(tty->dev,
+						       struct tty_device,
+						       dev);
+		if (ttyd->slave) {
+			mutex_lock(&ttyd->dev.mutex);
+			if (ttyd->slave)
+				ttyd->slave_ops->release(ttyd->slave, tty);
+			mutex_unlock(&ttyd->dev.mutex);
+		}
+	}
 
 	/*
 	 * We've decremented tty->count, so we need to remove this file
@@ -2105,6 +2124,18 @@ retry_open:
 		goto retry_open;
 	}
 	clear_bit(TTY_HUPPED, &tty->flags);
+	if (tty->dev && tty->count == 1) {
+		struct tty_device *ttyd = container_of(tty->dev,
+						       struct tty_device,
+						       dev);
+		if (ttyd->slave) {
+			mutex_lock(&ttyd->dev.mutex);
+			if (ttyd->slave &&
+			    ttyd->slave_ops->open)
+				ttyd->slave_ops->open(ttyd->slave, tty);
+			mutex_unlock(&ttyd->dev.mutex);
+		}
+	}
 	tty_unlock(tty);
 
 
@@ -3168,6 +3199,7 @@ struct device *tty_register_device_attr(struct tty_driver *driver,
 {
 	char name[64];
 	dev_t devt = MKDEV(driver->major, driver->minor_start) + index;
+	struct tty_device *tty_dev;
 	struct device *dev = NULL;
 	int retval = -ENODEV;
 	bool cdev = false;
@@ -3190,12 +3222,12 @@ struct device *tty_register_device_attr(struct tty_driver *driver,
 		cdev = true;
 	}
 
-	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
-	if (!dev) {
+	tty_dev = kzalloc(sizeof(*tty_dev), GFP_KERNEL);
+	if (!tty_dev) {
 		retval = -ENOMEM;
 		goto error;
 	}
-
+	dev = &tty_dev->dev;
 	dev->devt = devt;
 	dev->class = tty_class;
 	dev->parent = device;
@@ -3207,6 +3239,12 @@ 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 can register
+		 * for various call-backs on tty operations.
+		 */
+		of_platform_populate(device->of_node, NULL, NULL, dev);
+
 
 	return dev;
 
@@ -3238,6 +3276,35 @@ void tty_unregister_device(struct tty_driver *driver, unsigned index)
 }
 EXPORT_SYMBOL(tty_unregister_device);
 
+int tty_set_slave(struct device *tty, struct device *slave,
+		  struct tty_slave_operations *ops)
+{
+	struct tty_device *ttyd = container_of(tty, struct tty_device, dev);
+	int err;
+	if (tty->class != tty_class)
+		return -ENODEV;
+	if (ttyd->slave)
+		err = -EBUSY;
+	else {
+		ttyd->slave = slave;
+		ttyd->slave_ops = ops;
+		err = 0;
+	}
+	return err;
+}
+EXPORT_SYMBOL_GPL(tty_set_slave);
+
+void tty_clear_slave(struct device *tty, struct device *slave)
+{
+	struct tty_device *ttyd = container_of(tty, struct tty_device, dev);
+
+	WARN_ON(ttyd->slave != slave);
+	ttyd->slave = NULL;
+	ttyd->slave_ops = NULL;
+}
+EXPORT_SYMBOL_GPL(tty_clear_slave);
+
+
 /**
  * __tty_alloc_driver -- allocate tty driver
  * @lines: count of lines this driver can handle at most
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 5171ef8f7b85..fab8af995bd3 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -299,6 +299,22 @@ struct tty_file_private {
 	struct list_head list;
 };
 
+/* A "tty slave" device is permanently attached to a tty, typically
+ * via a UART.
+ * The driver can register for notifications for power management
+ * etc.  Any operation can be NULL.
+ * Operations are called under dev->mutex for the tty device.
+ */
+struct tty_slave_operations {
+	/* 'open' is called when the device is first opened */
+	void (*open)(struct device *slave, struct tty_struct *tty);
+	/* 'release' is called on last close */
+	void (*release)(struct device *slave, struct tty_struct *tty);
+};
+int tty_set_slave(struct device *tty, struct device *slave,
+		  struct tty_slave_operations *ops);
+void tty_clear_slave(struct device *tty, struct device *slave);
+
 /* tty magic number */
 #define TTY_MAGIC		0x5401
 

[PATCH 2/2] misc: add a driver to power on/off UART attached devices.

[NeilBrown]

If a platform has a particular device permanently attached to a UART,
there may be out-of-band signaling necessary to power the device
on and off.

This drive controls that signalling for a number of different devices.
It can
 - enable/disable a regulator
 - toggle a GPIO
 - register an 'rfkill' which can for the device to be off.

When the rfkill is absent of unblocked, the device will be on when the
associated tty device is open, and closed otherwise.

Signed-off-by: NeilBrown <neil@brown.name>
---
 .../devicetree/bindings/misc/wi2wi,w2cbw003.txt    |   19 +
 .../devicetree/bindings/misc/wi2wi,w2sg0004.txt    |   37 +
 .../devicetree/bindings/vendor-prefixes.txt        |    1 
 drivers/misc/Kconfig                               |   12 
 drivers/misc/Makefile                              |    1 
 drivers/misc/serial-power-manager.c                |  494 ++++++++++++++++++++
 6 files changed, 564 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/wi2wi,w2cbw003.txt
 create mode 100644 Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
 create mode 100644 drivers/misc/serial-power-manager.c

diff --git a/Documentation/devicetree/bindings/misc/wi2wi,w2cbw003.txt b/Documentation/devicetree/bindings/misc/wi2wi,w2cbw003.txt
new file mode 100644
index 000000000000..0001ea74a0b1
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/wi2wi,w2cbw003.txt
@@ -0,0 +1,19 @@
+wi2wi bluetooth module
+
+This is access via a serial port and is largely controlled via that
+link.  Extra configuration is needed to enable power on/off
+
+Required properties:
+- compatible: "wi2wi,w2cbw003"
+- vdd-supply: regulator used to power the device.
+
+The node for this device must be the child of a UART.
+
+Example:
+
+&uart1 {
+       bluetooth {
+               compatible = "wi2wi,w2cbw003";
+               vdd-supply = <&vaux4>;
+       };
+};
diff --git a/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt b/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
new file mode 100644
index 000000000000..933bd4ac1ce8
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
@@ -0,0 +1,37 @@
+wi2wi GPS device
+
+This is access via a serial port and is largely controlled via that
+link.  Extra configuration is needed to enable power on/off
+
+Required properties:
+- compatible: "wi2wi,w2sg0004"
+- gpios: gpios used to toggle 'on/off' pin
+- interrupts: interrupt generated by RX pin when device
+      should be off
+
+Optional properties:
+- vdd-supply: regulator used to power antenna
+- pinctrl: "default", "off"
+      if "off" setting is provided it is imposed when device should
+      be off.  This can route the RX pin to a GPIO interrupt.
+
+The w2sg0004 uses a pin-toggle both to power-on and to
+power-off, so the driver needs to detect what state it is in.
+It does this by detecting characters on the RX line.
+When it should be off, these can optionally be detected by a GPIO.
+
+The node for this device must be the child of a UART.
+
+Example:
+&uart2 {
+       gps {
+               compatible = "wi2iw,w2sg0004";
+               vdd-supply = <&vsim>;
+               gpios = <&gpio5 17 0>; /* GPIO_145 */
+               interrupts-extended = <&gpio5 19 0>; /* GPIO_147 */
+               /* When off, switch RX to be an interrupt */
+               pinctrl-names = "default", "off";
+               pinctrl-0 = <&uart2_pins>;
+               pinctrl-1 = <&uart2_pins_rx_gpio>;
+       };
+};
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index a344ec2713a5..7e24c0049bf4 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -160,6 +160,7 @@ v3	V3 Semiconductor
 variscite	Variscite Ltd.
 via	VIA Technologies, Inc.
 voipac	Voipac Technologies s.r.o.
+wi2wi	wi2wi Inc.  http://www.wi2wi.com/
 winbond Winbond Electronics corp.
 wlf	Wolfson Microelectronics
 wm	Wondermedia Technologies, Inc.
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index bbeb4516facf..bef98dc83f96 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -515,6 +515,18 @@ config VEXPRESS_SYSCFG
 	  bus. System Configuration interface is one of the possible means
 	  of generating transactions on this bus.
 
+config SERIAL_POWER_MANAGER
+	tristate "Power Management controller for serial-attached devices"
+	depends on TTY && OF
+	default n
+	help
+	  Some devices permanently attached via a UART can benefit from
+	  being power-managed when the tty device is openned or closed.
+	  This driver can support several such devices with simple
+	  power requirements such as enabling a regulator.
+
+	  If in doubt, say 'N'
+
 source "drivers/misc/c2port/Kconfig"
 source "drivers/misc/eeprom/Kconfig"
 source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 7d5c4cd118c4..d20450356eef 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -55,4 +55,5 @@ obj-y				+= mic/
 obj-$(CONFIG_GENWQE)		+= genwqe/
 obj-$(CONFIG_ECHO)		+= echo/
 obj-$(CONFIG_VEXPRESS_SYSCFG)	+= vexpress-syscfg.o
+obj-$(CONFIG_SERIAL_POWER_MANAGER) += serial-power-manager.o
 obj-$(CONFIG_CXL_BASE)		+= cxl/
diff --git a/drivers/misc/serial-power-manager.c b/drivers/misc/serial-power-manager.c
new file mode 100644
index 000000000000..a5abf9d65539
--- /dev/null
+++ b/drivers/misc/serial-power-manager.c
@@ -0,0 +1,494 @@
+/*
+ * Serial-power-manager
+ * Register for  open/close events on a tty, and turn power
+ * on/off for the device which is connected.
+ *
+ * Currently supported devices:
+ *  wi2wi,w2sg0004 - GPS with on/off toggle on a GPIO
+ *  wi2wi,w2cbw003 - bluetooth port; powered by regulator.
+ *
+ * When appropriate, an RFKILL will be registered which
+ * can power-down the device even when it is open.
+ *
+ * Device can be turned on either by
+ *  - enabling a regulator.  Disable to turn off
+ *  - toggling a GPIO.  Toggle again to turn off.  This requires
+ *     that we know the current state.  It is assumed to be 'off'
+ *     at boot, however if an interrupt can be generated when on,
+ *     such as by connecting RX to a GPIO, that can be used to detect
+ *     if the device is on when it should be off.
+ */
+
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/err.h>
+#include <linux/regulator/consumer.h>
+#include <linux/platform_device.h>
+#include <linux/of_device.h>
+#include <linux/tty.h>
+#include <linux/gpio.h>
+#include <linux/of_gpio.h>
+#include <linux/interrupt.h>
+#include <linux/delay.h>
+#include <linux/rfkill.h>
+
+
+/* This is used for testing. Setting this module parameter
+ * will simulate booting with the device "on"
+ */
+static bool toggle_on_probe = false;
+module_param(toggle_on_probe, bool, 0);
+MODULE_PARM_DESC(toggle_on_probe, "simulate power-on with devices active");
+
+struct spm_config {
+	int	rfkill_type;		/* type of rfkill to register */
+	int	toggle_time;		/* msec to pulse GPIO for on/off */
+	int	toggle_gap;		/* min msecs between toggles */
+	bool	off_in_suspend;
+}
+	simple_config = {
+		.off_in_suspend = true,
+	},
+	w2sg_config = {
+		.rfkill_type = RFKILL_TYPE_GPS,
+		.toggle_time = 10,
+		.toggle_gap = 500,
+		.off_in_suspend = true,
+	};
+
+const static struct of_device_id spm_dt_ids[] = {
+       { .compatible = "wi2wi,w2sg0004", .data = &w2sg_config},
+       { .compatible = "wi2wi,w2cbw003", .data = &simple_config},
+       {}
+};
+
+struct spm_data {
+	const struct spm_config *config;
+	struct gpio_desc *gpiod;
+	int		irq;	/* irq line from RX pin when pinctrl
+				 * set to 'idle' */
+	struct regulator *reg;
+
+	unsigned long	toggle_time;
+	unsigned long	toggle_gap;
+	unsigned long	last_toggle;	/* jiffies when last toggle completed. */
+	unsigned long	backoff;	/* jiffies since last_toggle when
+					 * we try again
+					 */
+	enum {Idle, Down, Up} state;	/* state-machine state. */
+
+	bool		is_open;
+	bool		requested, is_on;
+	bool		suspended;
+	bool		reg_enabled;
+
+	struct pinctrl	*pins;
+	struct pinctrl_state *pins_off;
+
+	struct delayed_work work;
+	spinlock_t	lock;
+	struct device	*dev;
+
+	struct rfkill	*rfkill;
+};
+
+/* When a device is powered on/off by toggling a GPIO we perform
+ * all the toggling via a workqueue to ensure only one toggle happens
+ * at a time and to allow easy timing.
+ * This is managed as a state machine which transitions
+ *  Idle -> Down -> Up -> Idle
+ * The GPIO is held down for toggle_time and then up for toggle_time,
+ * and then we assume the device has changed state.
+ * We never toggle until at least toggle_gap has passed since the
+ * last toggle.
+ */
+static void toggle_work(struct work_struct *work)
+{
+	struct spm_data *data = container_of(
+		work, struct spm_data, work.work);
+
+	if (data->gpiod == NULL)
+		return;
+
+	spin_lock_irq(&data->lock);
+	switch (data->state) {
+	case Up:
+		data->state = Idle;
+		if (data->requested == data->is_on)
+			break;
+		if (!data->requested)
+			/* Assume it is off unless activity is detected */
+			break;
+		/* Try again in a while unless we get some activity */
+		dev_dbg(data->dev, "Wait %dusec until retry\n",
+			jiffies_to_msecs(data->backoff));
+		schedule_delayed_work(&data->work, data->backoff);
+		break;
+	case Idle:
+		if (data->requested == data->is_on)
+			break;
+
+		/* Time to toggle */
+		dev_dbg(data->dev, "Starting toggle to turn %s\n",
+			data->requested ? "on" : "off");
+		data->state = Down;
+		spin_unlock_irq(&data->lock);
+		gpiod_set_value_cansleep(data->gpiod, 1);
+		schedule_delayed_work(&data->work, data->toggle_time);
+
+		return;
+
+	case Down:
+		data->state = Up;
+		data->last_toggle = jiffies;
+		dev_dbg(data->dev, "Toggle completed, should be %s now.\n",
+			data->is_on ? "off" : "on");
+		data->is_on = ! data->is_on;
+		spin_unlock_irq(&data->lock);
+
+		gpiod_set_value_cansleep(data->gpiod, 0);
+		schedule_delayed_work(&data->work, data->toggle_time);
+
+		return;
+	}
+	spin_unlock_irq(&data->lock);
+}
+
+static irqreturn_t spm_isr(int irq, void *dev_id)
+{
+	struct spm_data *data = dev_id;
+	unsigned long flags;
+
+	spin_lock_irqsave(&data->lock, flags);
+	if (!data->requested && !data->is_on && data->state == Idle &&
+	    time_after(jiffies, data->last_toggle + data->backoff)) {
+		data->is_on = 1;
+		data->backoff *= 2;
+		dev_dbg(data->dev, "Received data, must be on. Try to turn off\n");
+		if (!data->suspended)
+			schedule_delayed_work(&data->work, 0);
+	}
+	spin_unlock_irqrestore(&data->lock, flags);
+	return IRQ_HANDLED;
+}
+
+static void spm_open(struct device *slave, struct tty_struct *tty)
+{
+	struct spm_data *data = dev_get_drvdata(slave);
+	unsigned long flags;
+
+	data->is_open = true;
+
+	if (data->rfkill && rfkill_blocked(data->rfkill))
+		return;
+
+	if (!data->reg_enabled &&
+	    data->reg &&
+	    regulator_enable(data->reg) == 0)
+		data->reg_enabled = true;
+
+	spin_lock_irqsave(&data->lock, flags);
+	if (!data->requested) {
+		dev_dbg(data->dev, "TTY open - turn device on\n");
+		data->requested = true;
+		data->backoff = data->toggle_gap;
+		if (data->irq > 0) {
+			disable_irq(data->irq);
+			pinctrl_pm_select_default_state(slave);
+		}
+		if (!data->suspended && data->state == Idle)
+			schedule_delayed_work(&data->work, 0);
+	}
+	spin_unlock_irqrestore(&data->lock, flags);
+}
+
+static void spm_off(struct spm_data *data)
+{
+	unsigned long flags;
+
+	if (data->reg && data->reg_enabled)
+		if (regulator_disable(data->reg) == 0)
+			data->reg_enabled = false;
+
+	spin_lock_irqsave(&data->lock, flags);
+	if (data->requested) {
+		data->requested = false;
+		data->backoff = data->toggle_gap;
+		if (data->pins_off) {
+			pinctrl_select_state(data->pins,
+					     data->pins_off);
+			enable_irq(data->irq);
+		}
+		if (!data->suspended && data->state == Idle)
+			schedule_delayed_work(&data->work, 0);
+	}
+	spin_unlock_irqrestore(&data->lock, flags);
+}
+
+static void spm_release(struct device *slave, struct tty_struct *tty)
+{
+	struct spm_data *data = dev_get_drvdata(slave);
+
+	data->is_open = false;
+	dev_dbg(data->dev, "TTY closed - turn device off\n");
+	spm_off(data);
+}
+
+static int spm_rfkill_set_block(void *vdata, bool blocked)
+{
+	struct spm_data *data = vdata;
+
+	dev_dbg(data->dev, "rfkill_set_blocked %d\n", blocked);
+	if (blocked)
+		spm_off(data);
+
+	if (!blocked &&
+	    data->is_open)
+		spm_open(data->dev, NULL);
+
+	return 0;
+}
+
+static struct rfkill_ops spm_rfkill_ops = {
+	.set_block = spm_rfkill_set_block,
+};
+
+static int spm_suspend(struct device *dev)
+{
+	/* Ignore incoming data and just turn device off.
+	 * we cannot really wait for a separate thread to
+	 * do things, so we disable that and do it all
+	 * here
+	 */
+	struct spm_data *data = dev_get_drvdata(dev);
+
+	spin_lock_irq(&data->lock);
+	data->suspended = true;
+	spin_unlock_irq(&data->lock);
+	if (!data->config->off_in_suspend)
+		return 0;
+
+	if (data->gpiod) {
+
+		cancel_delayed_work_sync(&data->work);
+		if (data->state == Down) {
+			dev_dbg(data->dev, "Suspending while GPIO down - raising\n");
+			msleep(data->config->toggle_time);
+			gpiod_set_value_cansleep(data->gpiod, 0);
+			data->last_toggle = jiffies;
+			data->is_on = !data->is_on;
+			data->state = Up;
+		}
+		if (data->state == Up) {
+			msleep(data->config->toggle_time);
+			data->state = Idle;
+		}
+		if (data->is_on) {
+			dev_dbg(data->dev, "Suspending while device on: toggling\n");
+			gpiod_set_value_cansleep(data->gpiod, 1);
+			msleep(data->config->toggle_time);
+			gpiod_set_value_cansleep(data->gpiod, 0);
+			data->is_on = 0;
+		}
+	}
+
+	if (data->reg && data->reg_enabled)
+		if (regulator_disable(data->reg) == 0)
+			data->reg_enabled = false;
+
+	return 0;
+}
+
+static int spm_resume(struct device *dev)
+{
+	struct spm_data *data = dev_get_drvdata(dev);
+
+	spin_lock_irq(&data->lock);
+	data->suspended = false;
+	spin_unlock_irq(&data->lock);
+	schedule_delayed_work(&data->work, 0);
+
+	if (data->is_open &&
+	    (!data->rfkill || !rfkill_blocked(data->rfkill))) {
+		if (!data->reg_enabled &&
+		    data->reg &&
+		    regulator_enable(data->reg) == 0)
+			data->reg_enabled = true;
+	}
+	return 0;
+}
+
+static const struct dev_pm_ops spm_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(spm_suspend, spm_resume)
+};
+
+static struct tty_slave_operations spm_ops = {
+	.open	=	spm_open,
+	.release=	spm_release,
+};
+
+static int spm_probe(struct platform_device *pdev)
+{
+	struct spm_data *data;
+	struct regulator *reg;
+	int err;
+	const struct of_device_id *id;
+	const char *name;
+
+	if (pdev->dev.parent == NULL)
+		return -ENODEV;
+
+	id = of_match_device(spm_dt_ids, &pdev->dev);
+	if (!id)
+		return -ENODEV;
+
+	if (pdev->dev.of_node && pdev->dev.of_node->name)
+		name = pdev->dev.of_node->name;
+	else
+		name = "serial-power-manager";
+
+	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->config = id->data;
+	data->toggle_time = msecs_to_jiffies(data->config->toggle_time) + 1;
+	data->toggle_gap = msecs_to_jiffies(data->config->toggle_gap) + 1;
+	data->last_toggle = jiffies;
+	data->backoff = data->toggle_gap;
+	data->state = Idle;
+	spin_lock_init(&data->lock);
+	INIT_DELAYED_WORK(&data->work, toggle_work);
+
+	/* If a regulator is provided, it is enabled on 'open'
+	 * and disabled on 'release'
+	 */
+	reg = devm_regulator_get(&pdev->dev, "vdd");
+	if (IS_ERR(reg)) {
+		err = PTR_ERR(reg);
+		if (err != -ENODEV)
+			goto out;
+	} else
+		data->reg = reg;
+
+	/* If an irq is provided, any transitions are taken as
+	 * indication that the device is currently "on"
+	 */
+	data->irq = platform_get_irq(pdev, 0);
+	if (data->irq < 0) {
+		err = data->irq;
+		if (err != -ENXIO)
+			goto out;
+	} else {
+		dev_dbg(&pdev->dev, "IRQ configured: %d\n", data->irq);
+
+		irq_set_status_flags(data->irq, IRQ_NOAUTOEN);
+		err = devm_request_irq(&pdev->dev, data->irq, spm_isr,
+				       IRQF_TRIGGER_FALLING,
+				       name, data);
+
+		if (err)
+			goto out;
+
+	}
+
+	/* If a gpio is provided, then it is used to turn the device
+	 * on/off.
+	 * If toggle_time is zero, then the GPIO directly controls
+	 * the device.  If non-zero, then the GPIO must be toggled to
+	 * change the state of the device.
+	 */
+	data->gpiod = devm_gpiod_get(&pdev->dev, NULL, GPIOD_OUT_LOW);
+	if (IS_ERR(data->gpiod)) {
+		err = PTR_ERR(data->gpiod);
+		if (err != -ENOENT)
+			goto out;
+		data->gpiod = NULL;
+	} else
+		dev_dbg(&pdev->dev, "GPIO configured: %d\n",
+			desc_to_gpio(data->gpiod));
+
+	/* If an 'off' pinctrl state is defined, we apply that
+	 * when the device is assumed to be off.  This is expected to
+	 * route the 'rx' line to the 'irq' interrupt.
+	 */
+	data->pins = devm_pinctrl_get(&pdev->dev);
+	if (data->pins && data->irq > 0) {
+		data->pins_off = pinctrl_lookup_state(data->pins, "off");
+		if (IS_ERR(data->pins_off))
+			data->pins_off = NULL;
+	}
+
+	if (data->config->rfkill_type) {
+		data->rfkill = rfkill_alloc(name, &pdev->dev,
+					    data->config->rfkill_type,
+					    &spm_rfkill_ops, data);
+		if (!data->rfkill) {
+			err = -ENOMEM;
+			goto out;
+		}
+		err = rfkill_register(data->rfkill);
+		if (err) {
+			dev_err(&pdev->dev, "Cannot register rfkill device");
+			rfkill_destroy(data->rfkill);
+			goto out;
+		}
+	}
+	platform_set_drvdata(pdev, data);
+	data->dev = &pdev->dev;
+	tty_set_slave(pdev->dev.parent, &pdev->dev, &spm_ops);
+
+	if (data->pins_off)
+		pinctrl_select_state(data->pins, data->pins_off);
+	if (data->irq > 0)
+		enable_irq(data->irq);
+
+	if (toggle_on_probe && data->gpiod) {
+		dev_dbg(data->dev, "Performing initial toggle\n");
+		gpiod_set_value_cansleep(data->gpiod, 1);
+		msleep(data->config->toggle_time);
+		gpiod_set_value_cansleep(data->gpiod, 0);
+		msleep(data->config->toggle_time);
+	}
+	err = 0;
+out:
+	dev_dbg(data->dev, "Probed: err=%d\n", err);
+	return err;
+}
+
+static int spm_remove(struct platform_device *pdev)
+{
+       struct spm_data *data = dev_get_drvdata(&pdev->dev);
+
+       tty_clear_slave(pdev->dev.parent, &pdev->dev);
+       if (data->rfkill) {
+               rfkill_unregister(data->rfkill);
+               rfkill_destroy(data->rfkill);
+       }
+       return 0;
+}
+
+static struct platform_driver spm_driver = {
+       .driver.name		= "serial-power-manager",
+       .driver.owner		= THIS_MODULE,
+       .driver.of_match_table	= spm_dt_ids,
+       .probe			= spm_probe,
+       .remove			= spm_remove,
+};
+
+static int __init spm_init(void)
+{
+       return platform_driver_register(&spm_driver);
+}
+module_init(spm_init);
+
+static void __exit spm_exit(void)
+{
+       platform_driver_unregister(&spm_driver);
+}
+module_exit(spm_exit);
+
+MODULE_AUTHOR("NeilBrown <neil@brown.name>");
+MODULE_DEVICE_TABLE(of, spm_dt_ids);
+MODULE_DESCRIPTION("Power management for Serial-attached device.");
+MODULE_LICENSE("GPL v2");

Re: [PATCH 2/2] misc: add a driver to power on/off UART attached devices.

[One Thousand Gnomes]

On Sat, 20 Dec 2014 11:09:21 +1100
NeilBrown <neilb@suse.de> wrote:

> If a platform has a particular device permanently attached to a UART,
> there may be out-of-band signaling necessary to power the device
> on and off.
> 
> This drive controls that signalling for a number of different devices.
> It can
>  - enable/disable a regulator
>  - toggle a GPIO
>  - register an 'rfkill' which can for the device to be off.
> 
> When the rfkill is absent of unblocked, the device will be on when the
> associated tty device is open, and closed otherwise.
> 

Looks sound to me. Would drivers/tty/slave/.. be a better place for
these to collect up than "misc" ?

Re: [Gta04-owner] [PATCH 2/2] misc: add a driver to power on/off UART attached devices.

[Christ van Willegen]

Hi,

One remark:

+config SERIAL_POWER_MANAGER
+       tristate "Power Management controller for serial-attached devices"
+       depends on TTY && OF
+       default n
+       help
+         Some devices permanently attached via a UART can benefit from
+         being power-managed when the tty device is openned or closed.
+         This driver can support several such devices with simple
+         power requirements such as enabling a regulator.
+
+         If in doubt, say 'N'
+

There's a typo: openned.

Christ van Willegen

Re: [PATCH 1/2] TTY: add support for "tty slave" devices.

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 8019 bytes --]

Hi Neil,

On Sat, Dec 20, 2014 at 11:09:20AM +1100, NeilBrown wrote:
> 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.

How about (reads a bit easier to me, but I'm not a
native speaker):

Such a device may need its own driver, e.g. for powering
it up on tty open and powering it down on tty release.

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

maybe make this into its own device class instead of making
it a platform device?

> &uart1 {
> 	bluetooth {
> 		compatible = "wi2wi,w2cbw003";
> 		vdd-supply = <&vaux4>;
> 	};
> };
> 
> The driver can attach to the tty by calling
>    tty_set_slave(dev->parent, dev, &slave_ops);

this could be handled by the tty core if a custom tty slave device
class is used (similar to spi_device for spi slaves or i2c_client
for i2c slaves).

> where slave_ops' is a set of interface that
> the tty layer must call when appropriate.
> Currently only 'open' and 'release' are defined.
> They are called at first open and last close.
> They cannot fail.
> 
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
>  .../devicetree/bindings/serial/of-serial.txt       |    4 +
>  drivers/tty/tty_io.c                               |   73 +++++++++++++++++++-
>  include/linux/tty.h                                |   16 ++++
>  3 files changed, 90 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/serial/of-serial.txt b/Documentation/devicetree/bindings/serial/of-serial.txt
> index 8c4fd0332028..fc5d00c3c474 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 can
> +  register with the tty for open/close events to manage power.
> +

Drop the Linux specific bits and add the requirement of a compatible
value here. Suggestion:

Optional child node:
  A slave device connected to the serial port. It must contain at
  least a compatible property with a name string following generic
  names recommended practice.

>  Example:
>  
>  	uart@80230000 {
> diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
> index 0508a1d8e4cd..6c67a3fd257e 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>
>  
> @@ -110,6 +111,13 @@
>  #define TTY_PARANOIA_CHECK 1
>  #define CHECK_TTY_COUNT 1
>  
> +struct tty_device {
> +	struct device dev;
> +
> +	struct tty_slave_operations *slave_ops;
> +	struct device *slave;
> +};
> +
>  struct ktermios tty_std_termios = {	/* for the benefit of tty drivers  */
>  	.c_iflag = ICRNL | IXON,
>  	.c_oflag = OPOST | ONLCR,
> @@ -1825,6 +1833,17 @@ int tty_release(struct inode *inode, struct file *filp)
>  				__func__, tty->count, tty_name(tty, buf));
>  		tty->count = 0;
>  	}
> +	if (tty->dev && tty->count == 0) {
> +		struct tty_device *ttyd = container_of(tty->dev,
> +						       struct tty_device,
> +						       dev);
> +		if (ttyd->slave) {
> +			mutex_lock(&ttyd->dev.mutex);
> +			if (ttyd->slave)
> +				ttyd->slave_ops->release(ttyd->slave, tty);
> +			mutex_unlock(&ttyd->dev.mutex);
> +		}
> +	}
>  
>  	/*
>  	 * We've decremented tty->count, so we need to remove this file
> @@ -2105,6 +2124,18 @@ retry_open:
>  		goto retry_open;
>  	}
>  	clear_bit(TTY_HUPPED, &tty->flags);
> +	if (tty->dev && tty->count == 1) {
> +		struct tty_device *ttyd = container_of(tty->dev,
> +						       struct tty_device,
> +						       dev);
> +		if (ttyd->slave) {
> +			mutex_lock(&ttyd->dev.mutex);
> +			if (ttyd->slave &&
> +			    ttyd->slave_ops->open)
> +				ttyd->slave_ops->open(ttyd->slave, tty);
> +			mutex_unlock(&ttyd->dev.mutex);
> +		}
> +	}
>  	tty_unlock(tty);
>  
>  
> @@ -3168,6 +3199,7 @@ struct device *tty_register_device_attr(struct tty_driver *driver,
>  {
>  	char name[64];
>  	dev_t devt = MKDEV(driver->major, driver->minor_start) + index;
> +	struct tty_device *tty_dev;
>  	struct device *dev = NULL;
>  	int retval = -ENODEV;
>  	bool cdev = false;
> @@ -3190,12 +3222,12 @@ struct device *tty_register_device_attr(struct tty_driver *driver,
>  		cdev = true;
>  	}
>  
> -	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
> -	if (!dev) {
> +	tty_dev = kzalloc(sizeof(*tty_dev), GFP_KERNEL);
> +	if (!tty_dev) {
>  		retval = -ENOMEM;
>  		goto error;
>  	}
> -
> +	dev = &tty_dev->dev;
>  	dev->devt = devt;
>  	dev->class = tty_class;
>  	dev->parent = device;
> @@ -3207,6 +3239,12 @@ 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 can register
> +		 * for various call-backs on tty operations.
> +		 */
> +		of_platform_populate(device->of_node, NULL, NULL, dev);
> +
>  
>  	return dev;
>  
> @@ -3238,6 +3276,35 @@ void tty_unregister_device(struct tty_driver *driver, unsigned index)
>  }
>  EXPORT_SYMBOL(tty_unregister_device);
>  
> +int tty_set_slave(struct device *tty, struct device *slave,
> +		  struct tty_slave_operations *ops)
> +{
> +	struct tty_device *ttyd = container_of(tty, struct tty_device, dev);
> +	int err;
> +	if (tty->class != tty_class)
> +		return -ENODEV;
> +	if (ttyd->slave)
> +		err = -EBUSY;
> +	else {
> +		ttyd->slave = slave;
> +		ttyd->slave_ops = ops;
> +		err = 0;
> +	}
> +	return err;
> +}
> +EXPORT_SYMBOL_GPL(tty_set_slave);
> +
> +void tty_clear_slave(struct device *tty, struct device *slave)
> +{
> +	struct tty_device *ttyd = container_of(tty, struct tty_device, dev);
> +
> +	WARN_ON(ttyd->slave != slave);
> +	ttyd->slave = NULL;
> +	ttyd->slave_ops = NULL;
> +}
> +EXPORT_SYMBOL_GPL(tty_clear_slave);
> +
> +
>  /**
>   * __tty_alloc_driver -- allocate tty driver
>   * @lines: count of lines this driver can handle at most
> diff --git a/include/linux/tty.h b/include/linux/tty.h
> index 5171ef8f7b85..fab8af995bd3 100644
> --- a/include/linux/tty.h
> +++ b/include/linux/tty.h
> @@ -299,6 +299,22 @@ struct tty_file_private {
>  	struct list_head list;
>  };
>  
> +/* A "tty slave" device is permanently attached to a tty, typically
> + * via a UART.
> + * The driver can register for notifications for power management
> + * etc.  Any operation can be NULL.
> + * Operations are called under dev->mutex for the tty device.
> + */
> +struct tty_slave_operations {
> +	/* 'open' is called when the device is first opened */
> +	void (*open)(struct device *slave, struct tty_struct *tty);
> +	/* 'release' is called on last close */
> +	void (*release)(struct device *slave, struct tty_struct *tty);
> +};

Something like the following would be really useful for remote
devices, that can/must be woken up from idle states via an GPIO
(e.g.  the bluetooth chip from the Nokia N900):

/* 'write' is called when data should be sent to the remote device */
void (*write)(struct device *slave, struct tty_struct *tty);

The same kind of GPIO exists for waking up the host's UART chip from
idle, but that can simply be implemented by incrementing the runtime
usage of the tty_slave's parent device :)

> +int tty_set_slave(struct device *tty, struct device *slave,
> +		  struct tty_slave_operations *ops);
> +void tty_clear_slave(struct device *tty, struct device *slave);
> +
>  /* tty magic number */
>  #define TTY_MAGIC		0x5401

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Gta04-owner] [PATCH 2/2] misc: add a driver to power on/off UART attached devices.

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 700 bytes --]

On Sat, 20 Dec 2014 17:02:04 +0100 Christ van Willegen <cvwillegen@gmail.com>
wrote:

> Hi,
> 
> One remark:
> 
> +config SERIAL_POWER_MANAGER
> +       tristate "Power Management controller for serial-attached devices"
> +       depends on TTY && OF
> +       default n
> +       help
> +         Some devices permanently attached via a UART can benefit from
> +         being power-managed when the tty device is openned or closed.
> +         This driver can support several such devices with simple
> +         power requirements such as enabling a regulator.
> +
> +         If in doubt, say 'N'
> +
> 
> There's a typo: openned.

Thanks - I've fix that now.

NeilBrown

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

Re: [PATCH 1/2] TTY: add support for "tty slave" devices.

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 6625 bytes --]

On Sun, 21 Dec 2014 11:20:19 +0100 Sebastian Reichel <sre@kernel.org> wrote:

> Hi Neil,
> 
> On Sat, Dec 20, 2014 at 11:09:20AM +1100, NeilBrown wrote:
> > 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.
> 
> How about (reads a bit easier to me, but I'm not a
> native speaker):
> 
> Such a device may need its own driver, e.g. for powering
> it up on tty open and powering it down on tty release.

Yes, that does read better - thanks.

> 
> > A "tty slave" is a platform device which is declared as a
> > child of the uart in device-tree:
> 
> maybe make this into its own device class instead of making
> it a platform device?

Did you mean "class" or "bus"?? or "type".  I'm going to have to figure out
exactly what role each of those has.

In any case, the real question is "why?".  Where is the gain?

> 
> > &uart1 {
> > 	bluetooth {
> > 		compatible = "wi2wi,w2cbw003";
> > 		vdd-supply = <&vaux4>;
> > 	};
> > };
> > 
> > The driver can attach to the tty by calling
> >    tty_set_slave(dev->parent, dev, &slave_ops);
> 
> this could be handled by the tty core if a custom tty slave device
> class is used (similar to spi_device for spi slaves or i2c_client
> for i2c slaves).

spi_device seems to be a 'bus' device - spi_bus_type.

i2_client is also a 'bus' device - i2c_bus_type.  But there is also an
i2c_client_type.

Having a specific bus type (rather than the more generic 'platform') allows
these drivers to use the functionality of the bus to access the device.
e.g. the probe function of an i2c device gets a 'struct i2c_client' handle to
send commands  to the device.

But that is not the functionality that my 'tty slave' needs.  The driver
doesn't want to access the bus (the parent) - rather we need to arrange for
the parent (the uart/tty) to access the slave driver.

i.e. even if we had a 'serial bus', we would still need to register some
call-backs with the parent.  I don't see that the 'bus' model provides any
particular simplified way to do this.

If there were a 'tty_client' bus type, I would expect it to behave a lot like
the current "line disciplines".  They use the tty as a bus to provide a
higher-level channel to a specific uart attached device such as an hci
interface to a bluetooth module.

I has occasionally been suggested that the functionality I want should be
implemented as a line discipline.  As I understand it, the line discipline
cannot be imposed until you open the device, which make it too late for me...


Note that I'm not convinced that I have the model correct - I just don't see
how the change you suggest would be an improvement.

> 
> > where slave_ops' is a set of interface that
> > the tty layer must call when appropriate.
> > Currently only 'open' and 'release' are defined.
> > They are called at first open and last close.
> > They cannot fail.
> > 
> > Signed-off-by: NeilBrown <neilb@suse.de>
> > ---
> >  .../devicetree/bindings/serial/of-serial.txt       |    4 +
> >  drivers/tty/tty_io.c                               |   73 +++++++++++++++++++-
> >  include/linux/tty.h                                |   16 ++++
> >  3 files changed, 90 insertions(+), 3 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/serial/of-serial.txt b/Documentation/devicetree/bindings/serial/of-serial.txt
> > index 8c4fd0332028..fc5d00c3c474 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 can
> > +  register with the tty for open/close events to manage power.
> > +
> 
> Drop the Linux specific bits and add the requirement of a compatible
> value here. Suggestion:
> 
> Optional child node:
>   A slave device connected to the serial port. It must contain at
>   least a compatible property with a name string following generic
>   names recommended practice.

That looks good, thanks.

> 
> >  Example:
> >  
> >  	uart@80230000 {
> > diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
> > index 0508a1d8e4cd..6c67a3fd257e 100644
....
> > diff --git a/include/linux/tty.h b/include/linux/tty.h
> > index 5171ef8f7b85..fab8af995bd3 100644
> > --- a/include/linux/tty.h
> > +++ b/include/linux/tty.h
> > @@ -299,6 +299,22 @@ struct tty_file_private {
> >  	struct list_head list;
> >  };
> >  
> > +/* A "tty slave" device is permanently attached to a tty, typically
> > + * via a UART.
> > + * The driver can register for notifications for power management
> > + * etc.  Any operation can be NULL.
> > + * Operations are called under dev->mutex for the tty device.
> > + */
> > +struct tty_slave_operations {
> > +	/* 'open' is called when the device is first opened */
> > +	void (*open)(struct device *slave, struct tty_struct *tty);
> > +	/* 'release' is called on last close */
> > +	void (*release)(struct device *slave, struct tty_struct *tty);
> > +};
> 
> Something like the following would be really useful for remote
> devices, that can/must be woken up from idle states via an GPIO
> (e.g.  the bluetooth chip from the Nokia N900):
> 
> /* 'write' is called when data should be sent to the remote device */
> void (*write)(struct device *slave, struct tty_struct *tty);
> 
> The same kind of GPIO exists for waking up the host's UART chip from
> idle, but that can simply be implemented by incrementing the runtime
> usage of the tty_slave's parent device :)

I agree that could be useful.
I've also toyed with the idea of a 'recv' callback which tells the driver
whenever data is received.  That would confirm that the device is 'on'.
I couldn't convince myself that it was *really* useful and left it out for
simplicity.

Either could certainly be added, but I don't want to add some interface that
doesn't have an immediate user - the risk of choose imperfect semantics is
too high.

> 
> > +int tty_set_slave(struct device *tty, struct device *slave,
> > +		  struct tty_slave_operations *ops);
> > +void tty_clear_slave(struct device *tty, struct device *slave);
> > +
> >  /* tty magic number */
> >  #define TTY_MAGIC		0x5401
> 
> -- Sebastian

Thanks a lot for the review.

NeilBrown

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]