devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/3] UART slave device support (goldelico version)
@ 2015-10-16 18:08 H. Nikolaus Schaller
  2015-10-16 18:08 ` [PATCH v3 1/3] tty: serial core: provide a method to search uart by phandle H. Nikolaus Schaller
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: H. Nikolaus Schaller @ 2015-10-16 18:08 UTC (permalink / raw)
  To: Jiri Slaby, Arnd Bergmann, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, Jonathan Corbet
  Cc: Sergei Zviagintsev, Peter Hurley, One Thousand Gnomes,
	Sebastian Reichel, NeilBrown, Grant Likely, LKML, linux-serial,
	Marek Belisko, devicetree, linux-doc, H. Nikolaus Schaller

H. Nikolaus Schaller (3):
  tty: serial core: provide a method to search uart by phandle
  tty: serial_core: add hooks for uart slave drivers
  misc: Add w2sg0004 gps receiver driver

 .../devicetree/bindings/misc/wi2wi,w2sg0004.txt    |  18 +
 .../devicetree/bindings/serial/slaves.txt          |  16 +
 .../devicetree/bindings/vendor-prefixes.txt        |   1 +
 Documentation/serial/slaves.txt                    |  36 ++
 drivers/misc/Kconfig                               |  18 +
 drivers/misc/Makefile                              |   1 +
 drivers/misc/w2sg0004.c                            | 443 +++++++++++++++++++++
 drivers/tty/serial/serial_core.c                   | 214 +++++++++-
 include/linux/serial_core.h                        |  25 +-
 include/linux/w2sg0004.h                           |  27 ++
 10 files changed, 793 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
 create mode 100644 Documentation/devicetree/bindings/serial/slaves.txt
 create mode 100644 Documentation/serial/slaves.txt
 create mode 100644 drivers/misc/w2sg0004.c
 create mode 100644 include/linux/w2sg0004.h

-- 
2.5.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v3 1/3] tty: serial core: provide a method to search uart by phandle
  2015-10-16 18:08 [PATCH v3 0/3] UART slave device support (goldelico version) H. Nikolaus Schaller
@ 2015-10-16 18:08 ` H. Nikolaus Schaller
  2015-10-16 18:39   ` Mark Rutland
                     ` (3 more replies)
  2015-10-16 18:08 ` [PATCH v3 2/3] tty: serial_core: add hooks for uart slave drivers H. Nikolaus Schaller
                   ` (2 subsequent siblings)
  3 siblings, 4 replies; 16+ messages in thread
From: H. Nikolaus Schaller @ 2015-10-16 18:08 UTC (permalink / raw)
  To: Jiri Slaby, Arnd Bergmann, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, Jonathan Corbet
  Cc: Sergei Zviagintsev, Peter Hurley, One Thousand Gnomes,
	Sebastian Reichel, NeilBrown, Grant Likely, LKML, linux-serial,
	Marek Belisko, devicetree, linux-doc, H. Nikolaus Schaller

1. add uart_ports to a search list as soon as they are registered
2. provide a function to search an uart_port by phandle. This copies the
   mechanism how devm_usb_get_phy_by_phandle() works
3. add a bindings document how serial slaves should use this feature
4. add Documentation how serla slaves work in general

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 .../devicetree/bindings/serial/slaves.txt          |  16 +++
 Documentation/serial/slaves.txt                    |  36 +++++++
 drivers/tty/serial/serial_core.c                   | 107 +++++++++++++++++++++
 include/linux/serial_core.h                        |  10 ++
 4 files changed, 169 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serial/slaves.txt
 create mode 100644 Documentation/serial/slaves.txt

diff --git a/Documentation/devicetree/bindings/serial/slaves.txt b/Documentation/devicetree/bindings/serial/slaves.txt
new file mode 100644
index 0000000..353b87f
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/slaves.txt
@@ -0,0 +1,16 @@
+Device-Tree bindings for UART slave devices
+
+A node describing a slave device defines a phandle to reference the UART
+the device is connected to. In the (unexpected) case of two or more UARTs
+a list of phandles can be specified.
+
+properties:
+	- uart: (list of) phandle(s) of UART(s) the device is connected to
+
+
+example:
+
+	gps {
+		compatible = "wi2wi,w2sg0004";
+		uart = <&uart1>;
+	};
diff --git a/Documentation/serial/slaves.txt b/Documentation/serial/slaves.txt
new file mode 100644
index 0000000..6f8d44d
--- /dev/null
+++ b/Documentation/serial/slaves.txt
@@ -0,0 +1,36 @@
+UART slave device support
+
+A remote device connected to a RS232 interface is usually power controlled by the DTR line.
+The DTR line is managed automatically by the UART driver for open() and close() syscalls
+and on demand by tcsetattr().
+
+With embedded devices, the serial peripheral might be directly and always connected to the UART
+and there might be no physical DTR line involved. Power control (on/off) has to be done by some
+chip specific device driver (which we call "UART slave") through some mechanisms (I2C, GPIOs etc.)
+not related to the serial interface. Some devices do not explicitly tell their power state except
+by sending or not sending data to the UART. In such a case the device driver must be able to monitor
+data activity. The role of the device driver is to encapsulate such power control in a single place.
+
+This patch series allows to support such drivers by providing:
+* a mechanism that a slave driver can identify the UART instance it is connected to
+* a mechanism that UART slave drivers can register to be notified
+* notfications for DTR (and other modem control) state changes
+* notifications that the UART has received some data from the UART
+
+A slave device simply adds a phandle reference to the UART it is connected to, e.g.
+
+	gps {
+		compatible = "wi2wi,w2sg0004";
+		uart = <&uart1>;
+	};
+
+The slave driver calls devm_serial_get_uart_by_phandle() to identify the uart driver.
+This API follows the concept of devm_usb_get_phy_by_phandle().
+
+A slave device driver registers itself with serial_register_slave() to receive notifications.
+Notification handler callbacks can be registered by serial_register_mctrl_notification() and
+serial_register_rx_notification(). If an UART has registered a NULL slave or a NULL handler,
+no notifications are sent.
+
+RX notification handlers can define a ktermios during setup and the handler function can modify
+or decide to throw away each character that is passed upwards.
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 603d2cc..9caa33e 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -38,6 +38,36 @@
 #include <asm/irq.h>
 #include <asm/uaccess.h>
 
+static LIST_HEAD(uart_list);
+static DEFINE_SPINLOCK(uart_lock);
+
+/* same concept as __of_usb_find_phy */
+static struct uart_port *__of_serial_find_uart(struct device_node *node)
+{
+	struct uart_port  *uart;
+
+	if (!of_device_is_available(node))
+		return ERR_PTR(-ENODEV);
+
+	list_for_each_entry(uart, &uart_list, head) {
+		if (node != uart->dev->of_node)
+			continue;
+
+		return uart;
+	}
+
+	return ERR_PTR(-EPROBE_DEFER);
+}
+
+static void devm_serial_uart_release(struct device *dev, void *res)
+{
+	struct uart_port *uart = *(struct uart_port **)res;
+
+	/* FIXME: I don't understand the serial subsystem well enough
+	 * to know if we should call serial_put_uart(uart); here
+	 */
+}
+
 /*
  * This is used to lock changes in serial line configuration.
  */
@@ -64,6 +94,79 @@ static int uart_dcd_enabled(struct uart_port *uport)
 	return !!(uport->status & UPSTAT_DCD_ENABLE);
 }
 
+/**
+ * devm_serial_get_uart_by_phandle - find the uart by phandle
+ * @dev - device that requests this uart
+ * @phandle - name of the property holding the uart phandle value
+ * @index - the index of the uart
+ *
+ * Returns the uart_port associated with the given phandle value,
+ * after getting a refcount to it, -ENODEV if there is no such uart or
+ * -EPROBE_DEFER if there is a phandle to the uart, but the device is
+ * not yet loaded. While at that, it also associates the device with
+ * the uart using devres. On driver detach, release function is invoked
+ * on the devres data, then, devres data is freed.
+ *
+ * For use by tty host and peripheral drivers.
+ */
+
+/* same concept as devm_usb_get_phy_by_phandle() */
+
+struct uart_port *devm_serial_get_uart_by_phandle(struct device *dev,
+		const char *phandle, u8 index)
+{
+	struct uart_port  *uart = ERR_PTR(-ENOMEM), **ptr;
+	unsigned long   flags;
+	struct device_node *node;
+
+	if (!dev->of_node) {
+		dev_err(dev, "device does not have a device node entry\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	node = of_parse_phandle(dev->of_node, phandle, index);
+	if (!node) {
+		dev_err(dev, "failed to get %s phandle in %s node\n", phandle,
+			dev->of_node->full_name);
+		return ERR_PTR(-ENODEV);
+	}
+
+	ptr = devres_alloc(devm_serial_uart_release, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr) {
+		dev_err(dev, "failed to allocate memory for devres\n");
+		goto err0;
+	}
+
+	spin_lock_irqsave(&uart_lock, flags);
+
+	uart = __of_serial_find_uart(node);
+	if (IS_ERR(uart)) {
+		devres_free(ptr);
+		goto err1;
+	}
+
+	if (!try_module_get(uart->dev->driver->owner)) {
+		uart = ERR_PTR(-ENODEV);
+		devres_free(ptr);
+		goto err1;
+	}
+
+	*ptr = uart;
+	devres_add(dev, ptr);
+
+	get_device(uart->dev);
+
+err1:
+	spin_unlock_irqrestore(&uart_lock, flags);
+
+err0:
+	of_node_put(node);
+
+	return uart;
+}
+EXPORT_SYMBOL_GPL(devm_serial_get_uart_by_phandle);
+
+
 /*
  * This routine is used by the interrupt handler to schedule processing in
  * the software interrupt portion of the driver.
@@ -2733,6 +2836,8 @@ int uart_add_one_port(struct uart_driver *drv, struct uart_port *uport)
 	 */
 	uport->flags &= ~UPF_DEAD;
 
+	list_add_tail(&uport->head, &uart_list);
+
  out:
 	mutex_unlock(&port->mutex);
 	mutex_unlock(&port_mutex);
@@ -2764,6 +2869,8 @@ int uart_remove_one_port(struct uart_driver *drv, struct uart_port *uport)
 
 	mutex_lock(&port_mutex);
 
+	list_del(&uport->head);
+
 	/*
 	 * Mark the port "dead" - this prevents any opens from
 	 * succeeding while we shut down the port.
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 297d4fa..d7a2e15 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -247,6 +247,7 @@ struct uart_port {
 	const struct attribute_group **tty_groups;	/* all attributes (serial core use only) */
 	struct serial_rs485     rs485;
 	void			*private_data;		/* generic platform data pointer */
+	struct list_head	head;			/* uarts list (lookup by phandle) */
 };
 
 static inline int serial_port_in(struct uart_port *up, int offset)
@@ -475,4 +476,13 @@ static inline int uart_handle_break(struct uart_port *port)
 					 (cflag) & CRTSCTS || \
 					 !((cflag) & CLOCAL))
 
+/*
+ * Helper functions for UART slave drivers
+ */
+
+/* find UART by phandle (e.g. with 'uart = <&uart2>;' then call as
+ * devm_serial_get_uart_by_phandle(dev, "uart", 0);
+ */
+extern struct uart_port *devm_serial_get_uart_by_phandle(struct device *dev,
+		const char *phandle, u8 index);
 #endif /* LINUX_SERIAL_CORE_H */
-- 
2.5.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 2/3] tty: serial_core: add hooks for uart slave drivers
  2015-10-16 18:08 [PATCH v3 0/3] UART slave device support (goldelico version) H. Nikolaus Schaller
  2015-10-16 18:08 ` [PATCH v3 1/3] tty: serial core: provide a method to search uart by phandle H. Nikolaus Schaller
@ 2015-10-16 18:08 ` H. Nikolaus Schaller
  2015-10-16 18:08 ` [PATCH v3 3/3] misc: Add w2sg0004 gps receiver driver H. Nikolaus Schaller
  2015-10-16 19:11 ` [PATCH v3 0/3] UART slave device support (goldelico version) H. Nikolaus Schaller
  3 siblings, 0 replies; 16+ messages in thread
From: H. Nikolaus Schaller @ 2015-10-16 18:08 UTC (permalink / raw)
  To: Jiri Slaby, Arnd Bergmann, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, Jonathan Corbet
  Cc: Sergei Zviagintsev, Peter Hurley, One Thousand Gnomes,
	Sebastian Reichel, NeilBrown, Grant Likely, LKML, linux-serial,
	Marek Belisko, devicetree, linux-doc, H. Nikolaus Schaller

1. allow drivers to get notified about mctrl changes
2. allow drivers to get notified about rx data (indicating to the
   driver that the connected chip is active)
3. the driver also has the option to modify or block the
   received character instead of passing to the tty layer

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 drivers/tty/serial/serial_core.c | 104 +++++++++++++++++++++++++++++++++++++--
 include/linux/serial_core.h      |  15 +++++-
 2 files changed, 113 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 9caa33e..b731100 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -166,6 +166,86 @@ err0:
 }
 EXPORT_SYMBOL_GPL(devm_serial_get_uart_by_phandle);
 
+void uart_register_slave(struct uart_port *uport, void *slave)
+{
+	if (!slave) {
+		uart_register_mctrl_notification(uport, NULL);
+		uart_register_rx_notification(uport, NULL, NULL);
+	}
+	uport->slave = slave;
+}
+EXPORT_SYMBOL_GPL(uart_register_slave);
+
+void uart_register_mctrl_notification(struct uart_port *uport,
+		int (*function)(void *slave, int state))
+{
+	uport->mctrl_notification = function;
+}
+EXPORT_SYMBOL_GPL(uart_register_mctrl_notification);
+
+static int uart_port_startup(struct tty_struct *tty, struct uart_state *state,
+		int init_hw);
+
+static void uart_port_shutdown(struct tty_port *port);
+
+void uart_register_rx_notification(struct uart_port *uport,
+		int (*function)(void *slave, unsigned int *c),
+				struct ktermios *termios)
+{
+	struct uart_state *state = uport->state;
+	struct tty_port *tty_port = &state->port;
+
+	if (!uport->slave)
+		return;	/* slave must be registered first */
+
+	uport->rx_notification = function;
+
+	if (tty_port->count == 0) {
+		if (function) {
+			int retval = 0;
+
+			uart_change_pm(state, UART_PM_STATE_ON);
+			retval = uport->ops->startup(uport);
+			if (retval == 0 && termios) {
+				int hw_stopped;
+				/*
+				 * Initialise the hardware port settings.
+				 */
+				uport->ops->set_termios(uport, termios, NULL);
+
+				/*
+				 * Set modem status enables based on termios cflag
+				 */
+				spin_lock_irq(&uport->lock);
+				if (termios->c_cflag & CRTSCTS)
+					uport->status |= UPSTAT_CTS_ENABLE;
+				else
+					uport->status &= ~UPSTAT_CTS_ENABLE;
+
+				if (termios->c_cflag & CLOCAL)
+					uport->status &= ~UPSTAT_DCD_ENABLE;
+				else
+					uport->status |= UPSTAT_DCD_ENABLE;
+
+				/* reset sw-assisted CTS flow control based on (possibly) new mode */
+				hw_stopped = uport->hw_stopped;
+				uport->hw_stopped = uart_softcts_mode(uport) &&
+					!(uport->ops->get_mctrl(uport)
+						& TIOCM_CTS);
+				if (uport->hw_stopped) {
+					if (!hw_stopped)
+						uport->ops->stop_tx(uport);
+				} else {
+					if (hw_stopped)
+						uport->ops->start_tx(uport);
+				}
+				spin_unlock_irq(&uport->lock);
+			}
+		} else
+			uart_port_shutdown(tty_port);
+	}
+}
+EXPORT_SYMBOL_GPL(uart_register_rx_notification);
 
 /*
  * This routine is used by the interrupt handler to schedule processing in
@@ -224,6 +304,10 @@ uart_update_mctrl(struct uart_port *port, unsigned int set, unsigned int clear)
 	port->mctrl = (old & ~clear) | set;
 	if (old != port->mctrl)
 		port->ops->set_mctrl(port, port->mctrl);
+
+	if (port->mctrl_notification)
+		(*port->mctrl_notification)(port->slave, port->mctrl);
+
 	spin_unlock_irqrestore(&port->lock, flags);
 }
 
@@ -263,7 +347,8 @@ static int uart_port_startup(struct tty_struct *tty, struct uart_state *state,
 		uart_circ_clear(&state->xmit);
 	}
 
-	retval = uport->ops->startup(uport);
+	if (!state->uart_port->rx_notification)
+		retval = uport->ops->startup(uport);
 	if (retval == 0) {
 		if (uart_console(uport) && uport->cons->cflag) {
 			tty->termios.c_cflag = uport->cons->cflag;
@@ -299,7 +384,7 @@ static int uart_startup(struct tty_struct *tty, struct uart_state *state,
 		int init_hw)
 {
 	struct tty_port *port = &state->port;
-	int retval;
+	int retval = 0;
 
 	if (port->flags & ASYNC_INITIALIZED)
 		return 0;
@@ -345,8 +430,12 @@ static void uart_shutdown(struct tty_struct *tty, struct uart_state *state)
 
 		if (!tty || (tty->termios.c_cflag & HUPCL))
 			uart_clear_mctrl(uport, TIOCM_DTR | TIOCM_RTS);
-
-		uart_port_shutdown(port);
+		/*
+		 * if we have a slave that has registered for rx_notifications
+		 * we do not shut down the uart port to be able to monitor the device
+		 */
+		if (!uport->rx_notification)
+			uart_port_shutdown(port);
 	}
 
 	/*
@@ -1503,8 +1592,9 @@ static void uart_close(struct tty_struct *tty, struct file *filp)
 	/*
 	 * At this point, we stop accepting input.  To do this, we
 	 * disable the receive line status interrupts.
+	 * Unless a slave driver wants to keep input running
 	 */
-	if (port->flags & ASYNC_INITIALIZED) {
+	if (!uport->rx_notification && (port->flags & ASYNC_INITIALIZED)) {
 		spin_lock_irq(&uport->lock);
 		uport->ops->stop_rx(uport);
 		spin_unlock_irq(&uport->lock);
@@ -3028,6 +3118,10 @@ void uart_insert_char(struct uart_port *port, unsigned int status,
 {
 	struct tty_port *tport = &port->state->port;
 
+	if (port->rx_notification)
+		if ((*port->rx_notification)(port->slave, &ch))
+			return;	/* slave told us to ignore character */
+
 	if ((status & port->ignore_status_mask & ~overrun) == 0)
 		if (tty_insert_flip_char(tport, ch, flag) == 0)
 			++port->icount.buf_overrun;
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index d7a2e15..af90800 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -35,7 +35,7 @@
 #define uart_console(port) \
 	((port)->cons && (port)->cons->index == (port)->line)
 #else
-#define uart_console(port)      ({ (void)port; 0; })
+#define uart_console(port)      (0)
 #endif
 
 struct uart_port;
@@ -247,7 +247,13 @@ struct uart_port {
 	const struct attribute_group **tty_groups;	/* all attributes (serial core use only) */
 	struct serial_rs485     rs485;
 	void			*private_data;		/* generic platform data pointer */
+	/* UART slave support */
 	struct list_head	head;			/* uarts list (lookup by phandle) */
+	void			*slave;			/* optional slave (there can be only one) */
+	int			(*mctrl_notification)(void *slave,
+						      int state);
+	int			(*rx_notification)(void *slave,
+						      unsigned int *c);
 };
 
 static inline int serial_port_in(struct uart_port *up, int offset)
@@ -485,4 +491,11 @@ static inline int uart_handle_break(struct uart_port *port)
  */
 extern struct uart_port *devm_serial_get_uart_by_phandle(struct device *dev,
 		const char *phandle, u8 index);
+/* register to receive notifications */
+extern void uart_register_slave(struct uart_port *uart, void *slave);
+extern void uart_register_mctrl_notification(struct uart_port *uart,
+		int (*function)(void *slave, int state));
+extern void uart_register_rx_notification(struct uart_port *uart,
+		int (*function)(void *slave, unsigned int *c), struct ktermios *termios);
+
 #endif /* LINUX_SERIAL_CORE_H */
-- 
2.5.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 3/3] misc: Add w2sg0004 gps receiver driver
  2015-10-16 18:08 [PATCH v3 0/3] UART slave device support (goldelico version) H. Nikolaus Schaller
  2015-10-16 18:08 ` [PATCH v3 1/3] tty: serial core: provide a method to search uart by phandle H. Nikolaus Schaller
  2015-10-16 18:08 ` [PATCH v3 2/3] tty: serial_core: add hooks for uart slave drivers H. Nikolaus Schaller
@ 2015-10-16 18:08 ` H. Nikolaus Schaller
       [not found]   ` <c53ab7a3ca68f8b9f802c1ea799d72e1cb04a1eb.1445018913.git.hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>
                     ` (2 more replies)
  2015-10-16 19:11 ` [PATCH v3 0/3] UART slave device support (goldelico version) H. Nikolaus Schaller
  3 siblings, 3 replies; 16+ messages in thread
From: H. Nikolaus Schaller @ 2015-10-16 18:08 UTC (permalink / raw)
  To: Jiri Slaby, Arnd Bergmann, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, Jonathan Corbet
  Cc: Sergei Zviagintsev, Peter Hurley, One Thousand Gnomes,
	Sebastian Reichel, NeilBrown, Grant Likely, LKML, linux-serial,
	Marek Belisko, devicetree, linux-doc, H. Nikolaus Schaller

Add driver for Wi2Wi W2SG0004/84 GPS module connected through uart.
Use uart slave + notification hooks to glue with tty and turn on/off the
module. Detect if the module is turned on (sends data) but should be off,
e.g. if already turned on during boot.

Additionally, rfkill block/unblock can be used to control an external LNA
(and power down the module if not needed).

The driver concept is based on code developed by NeilBrown <neilb@suse.de>
but simplified and adapted to use the serial slave API.

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 .../devicetree/bindings/misc/wi2wi,w2sg0004.txt    |  18 +
 .../devicetree/bindings/vendor-prefixes.txt        |   1 +
 drivers/misc/Kconfig                               |  18 +
 drivers/misc/Makefile                              |   1 +
 drivers/misc/w2sg0004.c                            | 443 +++++++++++++++++++++
 drivers/tty/serial/serial_core.c                   |  25 +-
 include/linux/w2sg0004.h                           |  27 ++
 7 files changed, 522 insertions(+), 11 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
 create mode 100644 drivers/misc/w2sg0004.c
 create mode 100644 include/linux/w2sg0004.h

diff --git a/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt b/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
new file mode 100644
index 0000000..ef0d6d5
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
@@ -0,0 +1,18 @@
+Wi2Wi GPS module connected through UART
+
+Required properties:
+- compatible: wi2wi,w2sg0004 or wi2wi,w2sg0084
+- on-off-gpio: the GPIO that controls the module's on-off toggle input
+- uart: the uart we are connected to (provides DTR for power control)
+
+Optional properties:
+- lna-suppy: an (optional) LNA regulator that is enabled together with the GPS receiver
+
+example:
+
+        gps_receiver: w2sg0004 {
+                compatible = "wi2wi,w2sg0004";
+                lna-supply = <&vsim>;	/* LNA regulator */
+                on-off-gpio = <&gpio5 17 0>;	/* GPIO_145: trigger for turning on/off w2sg0004 */
+                uart = <&uart1>;	/* we are a slave of uart1 */
+        }
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index 82d2ac9..a778eb5 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -230,6 +230,7 @@ via	VIA Technologies, Inc.
 virtio	Virtual I/O Device Specification, developed by the OASIS consortium
 voipac	Voipac Technologies s.r.o.
 wexler	Wexler
+wi2wi	Wi2Wi, Inc.
 winbond Winbond Electronics corp.
 wlf	Wolfson Microelectronics
 wm	Wondermedia Technologies, Inc.
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index ccccc29..1279faf 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -537,4 +537,22 @@ source "drivers/misc/mic/Kconfig"
 source "drivers/misc/genwqe/Kconfig"
 source "drivers/misc/echo/Kconfig"
 source "drivers/misc/cxl/Kconfig"
+
+menu "GTA04 misc hardware support"
+
+config W2SG0004
+	tristate "W2SG0004 on/off control"
+	depends on GPIOLIB
+	help
+	  Enable on/off control of W2SG0004 GPS to allow powering up/down if
+	  the /dev/tty$n is opened/closed.
+	  It also provides a rfkill gps node to control the LNA power.
+
+config W2SG0004_DEBUG
+	bool "W2SG0004 on/off debugging"
+	depends on W2SG0004
+	help
+	  Enable driver debugging mode of W2SG0004 GPS.
+
+endmenu
 endmenu
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 537d7f3..a153a89 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -54,5 +54,6 @@ obj-$(CONFIG_SRAM)		+= sram.o
 obj-y				+= mic/
 obj-$(CONFIG_GENWQE)		+= genwqe/
 obj-$(CONFIG_ECHO)		+= echo/
+obj-$(CONFIG_W2SG0004)		+= w2sg0004.o
 obj-$(CONFIG_VEXPRESS_SYSCFG)	+= vexpress-syscfg.o
 obj-$(CONFIG_CXL_BASE)		+= cxl/
diff --git a/drivers/misc/w2sg0004.c b/drivers/misc/w2sg0004.c
new file mode 100644
index 0000000..6aadf44
--- /dev/null
+++ b/drivers/misc/w2sg0004.c
@@ -0,0 +1,443 @@
+/*
+ * w2sg0004.c
+ * Driver for power controlling the w2sg0004/w2sg0084 GPS receiver.
+ *
+ * This receiver has an ON/OFF pin which must be toggled to
+ * turn the device 'on' of 'off'.  A high->low->high toggle
+ * will switch the device on if it is off, and off if it is on.
+ *
+ * To enable receiving on/off requests we register with the
+ * UART power management notifications.
+ *
+ * It is not possible to directly detect the state of the device.
+ * However when it is on it will send characters on a UART line
+ * regularly.
+ *
+ * To detect that the power state is out of sync (e.g. if GPS
+ * was enabled before a reboot), we register for UART data received
+ * notifications.
+ *
+ * In addition we register as a rfkill client so that we can
+ * control the LNA power.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/sched.h>
+#include <linux/irq.h>
+#include <linux/slab.h>
+#include <linux/err.h>
+#include <linux/delay.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/of_gpio.h>
+#include <linux/platform_device.h>
+#include <linux/w2sg0004.h>
+#include <linux/workqueue.h>
+#include <linux/rfkill.h>
+#include <linux/serial_core.h>
+
+#ifdef CONFIG_W2SG0004_DEBUG
+#undef pr_debug
+#define pr_debug printk
+#endif
+
+/*
+ * There seems to be restrictions on how quickly we can toggle the
+ * on/off line.  data sheets says "two rtc ticks", whatever that means.
+ * If we do it too soon it doesn't work.
+ * So we have a state machine which uses the common work queue to ensure
+ * clean transitions.
+ * When a change is requested we record that request and only act on it
+ * once the previous change has completed.
+ * A change involves a 10ms low pulse, and a 990ms raised level, so only
+ * one change per second.
+ */
+
+enum w2sg_state {
+	W2SG_IDLE,	/* is not changing state */
+	W2SG_PULSE,	/* activate on/off impulse */
+	W2SG_NOPULSE	/* deactivate on/off impulse */
+};
+
+struct w2sg_data {
+	struct		rfkill *rf_kill;
+	struct		regulator *lna_regulator;
+	int		lna_blocked;	/* rfkill block gps active */
+	int		lna_is_off;	/* LNA is currently off */
+	int		is_on;		/* current state (0/1) */
+	unsigned long	last_toggle;
+	unsigned long	backoff;	/* time to wait since last_toggle */
+	int		on_off_gpio;	/* the on-off gpio number */
+	struct uart_port *uart;		/* the drvdata of the uart or tty */
+	enum w2sg_state	state;
+	int		requested;	/* requested state (0/1) */
+	int		suspended;
+	spinlock_t	lock;
+	struct delayed_work work;
+};
+
+static int w2sg_data_set_lna_power(struct w2sg_data *data)
+{
+	int ret = 0;
+	int off = data->suspended || !data->requested || data->lna_blocked;
+
+	pr_debug("%s: %s\n", __func__, off ? "off" : "on");
+
+	if (off != data->lna_is_off) {
+		data->lna_is_off = off;
+		if (!IS_ERR_OR_NULL(data->lna_regulator)) {
+			if (off)
+				regulator_disable(data->lna_regulator);
+			else
+				ret = regulator_enable(data->lna_regulator);
+		}
+	}
+
+	return ret;
+}
+
+static void w2sg_data_set_power(void *pdata, int val)
+{
+	struct w2sg_data *data = (struct w2sg_data *) pdata;
+	unsigned long flags;
+
+	pr_debug("%s to %d (%d)\n", __func__, val, data->requested);
+
+	spin_lock_irqsave(&data->lock, flags);
+
+	if (val && !data->requested) {
+		data->requested = true;
+	} else if (!val && data->requested) {
+		data->backoff = HZ;
+		data->requested = false;
+	} else
+		goto unlock;
+
+	pr_debug("w2sg scheduled for %d\n", data->requested);
+
+	if (!data->suspended)
+		schedule_delayed_work(&data->work, 0);
+unlock:
+	spin_unlock_irqrestore(&data->lock, flags);
+}
+
+/* called each time data is received by the host (i.e. sent by the w2sg0004) */
+
+static int rx_notification(void *pdata, unsigned int *c)
+{
+	struct w2sg_data *data = (struct w2sg_data *) pdata;
+	unsigned long flags;
+
+	if (!data->requested && !data->is_on) {
+		/* we have received a RX signal while GPS should be off */
+		pr_debug("%02x!", *c);
+
+		if ((data->state == W2SG_IDLE) &&
+		    time_after(jiffies,
+		    data->last_toggle + data->backoff)) {
+			/* Should be off by now, time to toggle again */
+			pr_debug("w2sg has sent data although it should be off!\n");
+			data->is_on = true;
+			data->backoff *= 2;
+			spin_lock_irqsave(&data->lock, flags);
+			if (!data->suspended)
+				schedule_delayed_work(&data->work, 0);
+			spin_unlock_irqrestore(&data->lock, flags);
+		}
+	}
+	return 0;	/* forward to tty */
+}
+
+/* called by uart modem control line changes (DTR) */
+
+static int w2sg_mctrl(void *pdata, int val)
+{
+	pr_debug("%s(...,%x)\n", __func__, val);
+	val = (val & TIOCM_DTR) != 0;	/* DTR controls power on/off */
+	w2sg_data_set_power((struct w2sg_data *) pdata, val);
+	return 0;
+}
+
+/* try to toggle the power state by sending a pulse to the on-off GPIO */
+
+static void toggle_work(struct work_struct *work)
+{
+	struct w2sg_data *data = container_of(work, struct w2sg_data,
+					      work.work);
+
+	switch (data->state) {
+	case W2SG_IDLE:
+		spin_lock_irq(&data->lock);
+		if (data->requested == data->is_on) {
+			spin_unlock_irq(&data->lock);
+			return;
+		}
+		spin_unlock_irq(&data->lock);
+		w2sg_data_set_lna_power(data);	/* update LNA power state */
+		gpio_set_value_cansleep(data->on_off_gpio, 0);
+		data->state = W2SG_PULSE;
+
+		pr_debug("w2sg: power gpio ON\n");
+
+		schedule_delayed_work(&data->work,
+				      msecs_to_jiffies(10));
+		break;
+
+	case W2SG_PULSE:
+		gpio_set_value_cansleep(data->on_off_gpio, 1);
+		data->last_toggle = jiffies;
+		data->state = W2SG_NOPULSE;
+		data->is_on = !data->is_on;
+
+		pr_debug("w2sg: power gpio OFF\n");
+
+		schedule_delayed_work(&data->work,
+				      msecs_to_jiffies(10));
+		break;
+
+	case W2SG_NOPULSE:
+		data->state = W2SG_IDLE;
+		pr_debug("w2sg: idle\n");
+
+		break;
+
+	}
+}
+
+static int w2sg_data_rfkill_set_block(void *pdata, bool blocked)
+{
+	struct w2sg_data *data = pdata;
+
+	pr_debug("%s: blocked: %d\n", __func__, blocked);
+
+	data->lna_blocked = blocked;
+
+	return w2sg_data_set_lna_power(data);
+}
+
+static struct rfkill_ops w2sg_data0004_rfkill_ops = {
+	.set_block = w2sg_data_rfkill_set_block,
+};
+
+static int w2sg_data_probe(struct platform_device *pdev)
+{
+	struct w2sg_pdata *pdata = dev_get_platdata(&pdev->dev);
+	struct w2sg_data *data;
+	struct rfkill *rf_kill;
+	int err;
+
+	pr_debug("%s()\n", __func__);
+
+	if (pdev->dev.of_node) {
+		struct device *dev = &pdev->dev;
+		enum of_gpio_flags flags;
+
+		pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+		if (!pdata)
+			return -ENOMEM;
+
+		pdata->on_off_gpio = of_get_named_gpio_flags(dev->of_node,
+							     "on-off-gpio", 0,
+							     &flags);
+
+		if (pdata->on_off_gpio == -EPROBE_DEFER)
+			return -EPROBE_DEFER;	/* defer until we have all gpios */
+
+		pdata->lna_regulator = devm_regulator_get_optional(dev, "lna");
+
+		pr_debug("%s() lna_regulator = %p\n", __func__, pdata->lna_regulator);
+
+		pdev->dev.platform_data = pdata;
+	}
+
+	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+	if (data == NULL)
+		return -ENOMEM;
+
+	data->lna_regulator = pdata->lna_regulator;
+	data->lna_blocked = true;
+	data->lna_is_off = true;
+
+	data->on_off_gpio = pdata->on_off_gpio;
+
+	data->is_on = false;
+	data->requested = false;
+	data->state = W2SG_IDLE;
+	data->last_toggle = jiffies;
+	data->backoff = HZ;
+
+#ifdef CONFIG_OF
+	data->uart = devm_serial_get_uart_by_phandle(&pdev->dev, "uart", 0);
+	if (IS_ERR(data->uart)) {
+		if (PTR_ERR(data->uart) == -EPROBE_DEFER)
+			return -EPROBE_DEFER;	/* we can't probe yet */
+		data->uart = NULL;	/* no UART */
+		}
+#endif
+
+	INIT_DELAYED_WORK(&data->work, toggle_work);
+	spin_lock_init(&data->lock);
+
+	err = devm_gpio_request(&pdev->dev, data->on_off_gpio, "w2sg0004-on-off");
+	if (err < 0)
+		goto out;
+
+	gpio_direction_output(data->on_off_gpio, false);
+
+	if (data->uart) {
+		struct ktermios termios = {
+			.c_iflag = IGNBRK,
+			.c_oflag = 0,
+			.c_cflag = B9600 | CS8,
+			.c_lflag = 0,
+			.c_line = 0,
+			.c_ispeed = 9600,
+			.c_ospeed = 9600
+		};
+		uart_register_slave(data->uart, data);
+		uart_register_mctrl_notification(data->uart, w2sg_mctrl);
+		uart_register_rx_notification(data->uart, rx_notification,
+					      &termios);
+	}
+
+	rf_kill = rfkill_alloc("GPS", &pdev->dev, RFKILL_TYPE_GPS,
+				&w2sg_data0004_rfkill_ops, data);
+	if (rf_kill == NULL) {
+		err = -ENOMEM;
+		goto err_rfkill;
+	}
+
+	err = rfkill_register(rf_kill);
+	if (err) {
+		dev_err(&pdev->dev, "Cannot register rfkill device\n");
+		goto err_rfkill;
+	}
+
+	data->rf_kill = rf_kill;
+
+	platform_set_drvdata(pdev, data);
+
+	pr_debug("w2sg0004 probed\n");
+
+#ifdef CONFIG_W2SG0004_DEBUG
+	/* turn on for debugging rx notifications */
+	pr_debug("w2sg power gpio ON\n");
+	gpio_set_value_cansleep(data->on_off_gpio, 1);
+	mdelay(100);
+	pr_debug("w2sg power gpio OFF\n");
+	gpio_set_value_cansleep(data->on_off_gpio, 0);
+	mdelay(300);
+#endif
+
+	/* if we won't receive mctrl notifications, turn on.
+	 * otherwise keep off until DTR is asserted through mctrl.
+	 */
+
+	w2sg_data_set_power(data, !data->uart);
+
+	return 0;
+
+err_rfkill:
+	rfkill_destroy(rf_kill);
+out:
+	return err;
+}
+
+static int w2sg_data_remove(struct platform_device *pdev)
+{
+	struct w2sg_data *data = platform_get_drvdata(pdev);
+
+	cancel_delayed_work_sync(&data->work);
+
+	if (data->uart) {
+		uart_register_rx_notification(data->uart, NULL, NULL);
+		uart_register_slave(data->uart, NULL);
+	}
+	return 0;
+}
+
+static int w2sg_data_suspend(struct device *dev)
+{
+	struct w2sg_data *data = dev_get_drvdata(dev);
+
+	spin_lock_irq(&data->lock);
+	data->suspended = true;
+	spin_unlock_irq(&data->lock);
+
+	cancel_delayed_work_sync(&data->work);
+
+	w2sg_data_set_lna_power(data);	/* shuts down if needed */
+
+	if (data->state == W2SG_PULSE) {
+		msleep(10);
+		gpio_set_value_cansleep(data->on_off_gpio, 1);
+		data->last_toggle = jiffies;
+		data->is_on = !data->is_on;
+		data->state = W2SG_NOPULSE;
+	}
+
+	if (data->state == W2SG_NOPULSE) {
+		msleep(10);
+		data->state = W2SG_IDLE;
+	}
+
+	if (data->is_on) {
+		pr_info("GPS off for suspend %d %d %d\n", data->requested,
+			data->is_on, data->lna_is_off);
+
+		gpio_set_value_cansleep(data->on_off_gpio, 0);
+		msleep(10);
+		gpio_set_value_cansleep(data->on_off_gpio, 1);
+		data->is_on = 0;
+	}
+
+	return 0;
+}
+
+static int w2sg_data_resume(struct device *dev)
+{
+	struct w2sg_data *data = dev_get_drvdata(dev);
+
+	spin_lock_irq(&data->lock);
+	data->suspended = false;
+	spin_unlock_irq(&data->lock);
+
+	pr_info("GPS resuming %d %d %d\n", data->requested,
+		data->is_on, data->lna_is_off);
+
+	schedule_delayed_work(&data->work, 0);	/* enables LNA if needed */
+
+	return 0;
+}
+
+#if defined(CONFIG_OF)
+static const struct of_device_id w2sg0004_of_match[] = {
+	{ .compatible = "wi2wi,w2sg0004" },
+	{ .compatible = "wi2wi,w2sg0084" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, w2sg0004_of_match);
+#endif
+
+SIMPLE_DEV_PM_OPS(w2sg_pm_ops, w2sg_data_suspend, w2sg_data_resume);
+
+static struct platform_driver w2sg_data_driver = {
+	.probe		= w2sg_data_probe,
+	.remove		= w2sg_data_remove,
+	.driver = {
+		.name	= "w2sg0004",
+		.owner	= THIS_MODULE,
+		.pm	= &w2sg_pm_ops,
+		.of_match_table = of_match_ptr(w2sg0004_of_match)
+	},
+};
+
+module_platform_driver(w2sg_data_driver);
+
+MODULE_ALIAS("w2sg0004");
+
+MODULE_AUTHOR("NeilBrown <neilb@suse.de>");
+MODULE_DESCRIPTION("w2sg0004 GPS power management driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index b731100..c8ae0dd 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -177,7 +177,7 @@ void uart_register_slave(struct uart_port *uport, void *slave)
 EXPORT_SYMBOL_GPL(uart_register_slave);
 
 void uart_register_mctrl_notification(struct uart_port *uport,
-		int (*function)(void *slave, int state))
+			int (*function)(void *slave, int state))
 {
 	uport->mctrl_notification = function;
 }
@@ -208,14 +208,14 @@ void uart_register_rx_notification(struct uart_port *uport,
 			retval = uport->ops->startup(uport);
 			if (retval == 0 && termios) {
 				int hw_stopped;
-				/*
-				 * Initialise the hardware port settings.
-				 */
+/*
+ * Initialise the hardware port settings.
+ */
 				uport->ops->set_termios(uport, termios, NULL);
 
-				/*
-				 * Set modem status enables based on termios cflag
-				 */
+/*
+ * Set modem status enables based on termios cflag
+ */
 				spin_lock_irq(&uport->lock);
 				if (termios->c_cflag & CRTSCTS)
 					uport->status |= UPSTAT_CTS_ENABLE;
@@ -227,11 +227,13 @@ void uart_register_rx_notification(struct uart_port *uport,
 				else
 					uport->status |= UPSTAT_DCD_ENABLE;
 
-				/* reset sw-assisted CTS flow control based on (possibly) new mode */
+/*
+ * reset sw-assisted CTS flow control based on (possibly) new mode
+ */
 				hw_stopped = uport->hw_stopped;
 				uport->hw_stopped = uart_softcts_mode(uport) &&
-					!(uport->ops->get_mctrl(uport)
-						& TIOCM_CTS);
+					!(uport->ops->get_mctrl(uport) &
+						TIOCM_CTS);
 				if (uport->hw_stopped) {
 					if (!hw_stopped)
 						uport->ops->stop_tx(uport);
@@ -432,7 +434,8 @@ static void uart_shutdown(struct tty_struct *tty, struct uart_state *state)
 			uart_clear_mctrl(uport, TIOCM_DTR | TIOCM_RTS);
 		/*
 		 * if we have a slave that has registered for rx_notifications
-		 * we do not shut down the uart port to be able to monitor the device
+		 * we do not shut down the uart port to be able to monitor the
+		 * device
 		 */
 		if (!uport->rx_notification)
 			uart_port_shutdown(port);
diff --git a/include/linux/w2sg0004.h b/include/linux/w2sg0004.h
new file mode 100644
index 0000000..ad0c4a1
--- /dev/null
+++ b/include/linux/w2sg0004.h
@@ -0,0 +1,27 @@
+/*
+ * UART slave to allow ON/OFF control of w2sg0004 GPS receiver.
+ *
+ * Copyright (C) 2011 Neil Brown <neil@brown.name>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ */
+
+
+
+#ifndef __LINUX_W2SG0004_H
+#define __LINUX_W2SG0004_H
+
+#include <linux/regulator/consumer.h>
+
+struct w2sg_pdata {
+	struct regulator *lna_regulator;	/* enable LNA power */
+	int	on_off_gpio;	/*  on-off input of the GPS module */
+};
+#endif /* __LINUX_W2SG0004_H */
-- 
2.5.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 3/3] misc: Add w2sg0004 gps receiver driver
       [not found]   ` <c53ab7a3ca68f8b9f802c1ea799d72e1cb04a1eb.1445018913.git.hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>
@ 2015-10-16 18:15     ` H. Nikolaus Schaller
  0 siblings, 0 replies; 16+ messages in thread
From: H. Nikolaus Schaller @ 2015-10-16 18:15 UTC (permalink / raw)
  To: Jiri Slaby, Arnd Bergmann, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, Jonathan Corbet
  Cc: Sergei Zviagintsev, Peter Hurley, One Thousand Gnomes,
	Sebastian Reichel, NeilBrown, Grant Likely, LKML,
	linux-serial-u79uwXL29TY76Z2rM5mHXA, Marek Belisko,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-doc-u79uwXL29TY76Z2rM5mHXA


Am 16.10.2015 um 20:08 schrieb H. Nikolaus Schaller <hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>:

> Add driver for Wi2Wi W2SG0004/84 GPS module connected through uart.
> Use uart slave + notification hooks to glue with tty and turn on/off the
> module. Detect if the module is turned on (sends data) but should be off,
> e.g. if already turned on during boot.
> 
> Additionally, rfkill block/unblock can be used to control an external LNA
> (and power down the module if not needed).
> 
> The driver concept is based on code developed by NeilBrown <neilb-l3A5Bk7waGM@public.gmane.org>
> but simplified and adapted to use the serial slave API.
> 
> Signed-off-by: H. Nikolaus Schaller <hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>
> ---
> .../devicetree/bindings/misc/wi2wi,w2sg0004.txt    |  18 +
> .../devicetree/bindings/vendor-prefixes.txt        |   1 +
> drivers/misc/Kconfig                               |  18 +
> drivers/misc/Makefile                              |   1 +
> drivers/misc/w2sg0004.c                            | 443 +++++++++++++++++++++
> drivers/tty/serial/serial_core.c                   |  25 +-
^^^sorry this change is garbage from patch editing^^^
> include/linux/w2sg0004.h                           |  27 ++
> 7 files changed, 522 insertions(+), 11 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
> create mode 100644 drivers/misc/w2sg0004.c
> create mode 100644 include/linux/w2sg0004.h
> 
> diff --git a/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt b/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
> new file mode 100644
> index 0000000..ef0d6d5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
> @@ -0,0 +1,18 @@
> +Wi2Wi GPS module connected through UART
> +
> +Required properties:
> +- compatible: wi2wi,w2sg0004 or wi2wi,w2sg0084
> +- on-off-gpio: the GPIO that controls the module's on-off toggle input
> +- uart: the uart we are connected to (provides DTR for power control)
> +
> +Optional properties:
> +- lna-suppy: an (optional) LNA regulator that is enabled together with the GPS receiver
> +
> +example:
> +
> +        gps_receiver: w2sg0004 {
> +                compatible = "wi2wi,w2sg0004";
> +                lna-supply = <&vsim>;	/* LNA regulator */
> +                on-off-gpio = <&gpio5 17 0>;	/* GPIO_145: trigger for turning on/off w2sg0004 */
> +                uart = <&uart1>;	/* we are a slave of uart1 */
> +        }
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
> index 82d2ac9..a778eb5 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> @@ -230,6 +230,7 @@ via	VIA Technologies, Inc.
> virtio	Virtual I/O Device Specification, developed by the OASIS consortium
> voipac	Voipac Technologies s.r.o.
> wexler	Wexler
> +wi2wi	Wi2Wi, Inc.
> winbond Winbond Electronics corp.
> wlf	Wolfson Microelectronics
> wm	Wondermedia Technologies, Inc.
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index ccccc29..1279faf 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -537,4 +537,22 @@ source "drivers/misc/mic/Kconfig"
> source "drivers/misc/genwqe/Kconfig"
> source "drivers/misc/echo/Kconfig"
> source "drivers/misc/cxl/Kconfig"
> +
> +menu "GTA04 misc hardware support"
> +
> +config W2SG0004
> +	tristate "W2SG0004 on/off control"
> +	depends on GPIOLIB
> +	help
> +	  Enable on/off control of W2SG0004 GPS to allow powering up/down if
> +	  the /dev/tty$n is opened/closed.
> +	  It also provides a rfkill gps node to control the LNA power.
> +
> +config W2SG0004_DEBUG
> +	bool "W2SG0004 on/off debugging"
> +	depends on W2SG0004
> +	help
> +	  Enable driver debugging mode of W2SG0004 GPS.
> +
> +endmenu
> endmenu
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index 537d7f3..a153a89 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -54,5 +54,6 @@ obj-$(CONFIG_SRAM)		+= sram.o
> obj-y				+= mic/
> obj-$(CONFIG_GENWQE)		+= genwqe/
> obj-$(CONFIG_ECHO)		+= echo/
> +obj-$(CONFIG_W2SG0004)		+= w2sg0004.o
> obj-$(CONFIG_VEXPRESS_SYSCFG)	+= vexpress-syscfg.o
> obj-$(CONFIG_CXL_BASE)		+= cxl/
> diff --git a/drivers/misc/w2sg0004.c b/drivers/misc/w2sg0004.c
> new file mode 100644
> index 0000000..6aadf44
> --- /dev/null
> +++ b/drivers/misc/w2sg0004.c
> @@ -0,0 +1,443 @@
> +/*
> + * w2sg0004.c
> + * Driver for power controlling the w2sg0004/w2sg0084 GPS receiver.
> + *
> + * This receiver has an ON/OFF pin which must be toggled to
> + * turn the device 'on' of 'off'.  A high->low->high toggle
> + * will switch the device on if it is off, and off if it is on.
> + *
> + * To enable receiving on/off requests we register with the
> + * UART power management notifications.
> + *
> + * It is not possible to directly detect the state of the device.
> + * However when it is on it will send characters on a UART line
> + * regularly.
> + *
> + * To detect that the power state is out of sync (e.g. if GPS
> + * was enabled before a reboot), we register for UART data received
> + * notifications.
> + *
> + * In addition we register as a rfkill client so that we can
> + * control the LNA power.
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/interrupt.h>
> +#include <linux/sched.h>
> +#include <linux/irq.h>
> +#include <linux/slab.h>
> +#include <linux/err.h>
> +#include <linux/delay.h>
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_gpio.h>
> +#include <linux/platform_device.h>
> +#include <linux/w2sg0004.h>
> +#include <linux/workqueue.h>
> +#include <linux/rfkill.h>
> +#include <linux/serial_core.h>
> +
> +#ifdef CONFIG_W2SG0004_DEBUG
> +#undef pr_debug
> +#define pr_debug printk
> +#endif
> +
> +/*
> + * There seems to be restrictions on how quickly we can toggle the
> + * on/off line.  data sheets says "two rtc ticks", whatever that means.
> + * If we do it too soon it doesn't work.
> + * So we have a state machine which uses the common work queue to ensure
> + * clean transitions.
> + * When a change is requested we record that request and only act on it
> + * once the previous change has completed.
> + * A change involves a 10ms low pulse, and a 990ms raised level, so only
> + * one change per second.
> + */
> +
> +enum w2sg_state {
> +	W2SG_IDLE,	/* is not changing state */
> +	W2SG_PULSE,	/* activate on/off impulse */
> +	W2SG_NOPULSE	/* deactivate on/off impulse */
> +};
> +
> +struct w2sg_data {
> +	struct		rfkill *rf_kill;
> +	struct		regulator *lna_regulator;
> +	int		lna_blocked;	/* rfkill block gps active */
> +	int		lna_is_off;	/* LNA is currently off */
> +	int		is_on;		/* current state (0/1) */
> +	unsigned long	last_toggle;
> +	unsigned long	backoff;	/* time to wait since last_toggle */
> +	int		on_off_gpio;	/* the on-off gpio number */
> +	struct uart_port *uart;		/* the drvdata of the uart or tty */
> +	enum w2sg_state	state;
> +	int		requested;	/* requested state (0/1) */
> +	int		suspended;
> +	spinlock_t	lock;
> +	struct delayed_work work;
> +};
> +
> +static int w2sg_data_set_lna_power(struct w2sg_data *data)
> +{
> +	int ret = 0;
> +	int off = data->suspended || !data->requested || data->lna_blocked;
> +
> +	pr_debug("%s: %s\n", __func__, off ? "off" : "on");
> +
> +	if (off != data->lna_is_off) {
> +		data->lna_is_off = off;
> +		if (!IS_ERR_OR_NULL(data->lna_regulator)) {
> +			if (off)
> +				regulator_disable(data->lna_regulator);
> +			else
> +				ret = regulator_enable(data->lna_regulator);
> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +static void w2sg_data_set_power(void *pdata, int val)
> +{
> +	struct w2sg_data *data = (struct w2sg_data *) pdata;
> +	unsigned long flags;
> +
> +	pr_debug("%s to %d (%d)\n", __func__, val, data->requested);
> +
> +	spin_lock_irqsave(&data->lock, flags);
> +
> +	if (val && !data->requested) {
> +		data->requested = true;
> +	} else if (!val && data->requested) {
> +		data->backoff = HZ;
> +		data->requested = false;
> +	} else
> +		goto unlock;
> +
> +	pr_debug("w2sg scheduled for %d\n", data->requested);
> +
> +	if (!data->suspended)
> +		schedule_delayed_work(&data->work, 0);
> +unlock:
> +	spin_unlock_irqrestore(&data->lock, flags);
> +}
> +
> +/* called each time data is received by the host (i.e. sent by the w2sg0004) */
> +
> +static int rx_notification(void *pdata, unsigned int *c)
> +{
> +	struct w2sg_data *data = (struct w2sg_data *) pdata;
> +	unsigned long flags;
> +
> +	if (!data->requested && !data->is_on) {
> +		/* we have received a RX signal while GPS should be off */
> +		pr_debug("%02x!", *c);
> +
> +		if ((data->state == W2SG_IDLE) &&
> +		    time_after(jiffies,
> +		    data->last_toggle + data->backoff)) {
> +			/* Should be off by now, time to toggle again */
> +			pr_debug("w2sg has sent data although it should be off!\n");
> +			data->is_on = true;
> +			data->backoff *= 2;
> +			spin_lock_irqsave(&data->lock, flags);
> +			if (!data->suspended)
> +				schedule_delayed_work(&data->work, 0);
> +			spin_unlock_irqrestore(&data->lock, flags);
> +		}
> +	}
> +	return 0;	/* forward to tty */
> +}
> +
> +/* called by uart modem control line changes (DTR) */
> +
> +static int w2sg_mctrl(void *pdata, int val)
> +{
> +	pr_debug("%s(...,%x)\n", __func__, val);
> +	val = (val & TIOCM_DTR) != 0;	/* DTR controls power on/off */
> +	w2sg_data_set_power((struct w2sg_data *) pdata, val);
> +	return 0;
> +}
> +
> +/* try to toggle the power state by sending a pulse to the on-off GPIO */
> +
> +static void toggle_work(struct work_struct *work)
> +{
> +	struct w2sg_data *data = container_of(work, struct w2sg_data,
> +					      work.work);
> +
> +	switch (data->state) {
> +	case W2SG_IDLE:
> +		spin_lock_irq(&data->lock);
> +		if (data->requested == data->is_on) {
> +			spin_unlock_irq(&data->lock);
> +			return;
> +		}
> +		spin_unlock_irq(&data->lock);
> +		w2sg_data_set_lna_power(data);	/* update LNA power state */
> +		gpio_set_value_cansleep(data->on_off_gpio, 0);
> +		data->state = W2SG_PULSE;
> +
> +		pr_debug("w2sg: power gpio ON\n");
> +
> +		schedule_delayed_work(&data->work,
> +				      msecs_to_jiffies(10));
> +		break;
> +
> +	case W2SG_PULSE:
> +		gpio_set_value_cansleep(data->on_off_gpio, 1);
> +		data->last_toggle = jiffies;
> +		data->state = W2SG_NOPULSE;
> +		data->is_on = !data->is_on;
> +
> +		pr_debug("w2sg: power gpio OFF\n");
> +
> +		schedule_delayed_work(&data->work,
> +				      msecs_to_jiffies(10));
> +		break;
> +
> +	case W2SG_NOPULSE:
> +		data->state = W2SG_IDLE;
> +		pr_debug("w2sg: idle\n");
> +
> +		break;
> +
> +	}
> +}
> +
> +static int w2sg_data_rfkill_set_block(void *pdata, bool blocked)
> +{
> +	struct w2sg_data *data = pdata;
> +
> +	pr_debug("%s: blocked: %d\n", __func__, blocked);
> +
> +	data->lna_blocked = blocked;
> +
> +	return w2sg_data_set_lna_power(data);
> +}
> +
> +static struct rfkill_ops w2sg_data0004_rfkill_ops = {
> +	.set_block = w2sg_data_rfkill_set_block,
> +};
> +
> +static int w2sg_data_probe(struct platform_device *pdev)
> +{
> +	struct w2sg_pdata *pdata = dev_get_platdata(&pdev->dev);
> +	struct w2sg_data *data;
> +	struct rfkill *rf_kill;
> +	int err;
> +
> +	pr_debug("%s()\n", __func__);
> +
> +	if (pdev->dev.of_node) {
> +		struct device *dev = &pdev->dev;
> +		enum of_gpio_flags flags;
> +
> +		pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> +		if (!pdata)
> +			return -ENOMEM;
> +
> +		pdata->on_off_gpio = of_get_named_gpio_flags(dev->of_node,
> +							     "on-off-gpio", 0,
> +							     &flags);
> +
> +		if (pdata->on_off_gpio == -EPROBE_DEFER)
> +			return -EPROBE_DEFER;	/* defer until we have all gpios */
> +
> +		pdata->lna_regulator = devm_regulator_get_optional(dev, "lna");
> +
> +		pr_debug("%s() lna_regulator = %p\n", __func__, pdata->lna_regulator);
> +
> +		pdev->dev.platform_data = pdata;
> +	}
> +
> +	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> +	if (data == NULL)
> +		return -ENOMEM;
> +
> +	data->lna_regulator = pdata->lna_regulator;
> +	data->lna_blocked = true;
> +	data->lna_is_off = true;
> +
> +	data->on_off_gpio = pdata->on_off_gpio;
> +
> +	data->is_on = false;
> +	data->requested = false;
> +	data->state = W2SG_IDLE;
> +	data->last_toggle = jiffies;
> +	data->backoff = HZ;
> +
> +#ifdef CONFIG_OF
> +	data->uart = devm_serial_get_uart_by_phandle(&pdev->dev, "uart", 0);
> +	if (IS_ERR(data->uart)) {
> +		if (PTR_ERR(data->uart) == -EPROBE_DEFER)
> +			return -EPROBE_DEFER;	/* we can't probe yet */
> +		data->uart = NULL;	/* no UART */
> +		}
> +#endif
> +
> +	INIT_DELAYED_WORK(&data->work, toggle_work);
> +	spin_lock_init(&data->lock);
> +
> +	err = devm_gpio_request(&pdev->dev, data->on_off_gpio, "w2sg0004-on-off");
> +	if (err < 0)
> +		goto out;
> +
> +	gpio_direction_output(data->on_off_gpio, false);
> +
> +	if (data->uart) {
> +		struct ktermios termios = {
> +			.c_iflag = IGNBRK,
> +			.c_oflag = 0,
> +			.c_cflag = B9600 | CS8,
> +			.c_lflag = 0,
> +			.c_line = 0,
> +			.c_ispeed = 9600,
> +			.c_ospeed = 9600
> +		};
> +		uart_register_slave(data->uart, data);
> +		uart_register_mctrl_notification(data->uart, w2sg_mctrl);
> +		uart_register_rx_notification(data->uart, rx_notification,
> +					      &termios);
> +	}
> +
> +	rf_kill = rfkill_alloc("GPS", &pdev->dev, RFKILL_TYPE_GPS,
> +				&w2sg_data0004_rfkill_ops, data);
> +	if (rf_kill == NULL) {
> +		err = -ENOMEM;
> +		goto err_rfkill;
> +	}
> +
> +	err = rfkill_register(rf_kill);
> +	if (err) {
> +		dev_err(&pdev->dev, "Cannot register rfkill device\n");
> +		goto err_rfkill;
> +	}
> +
> +	data->rf_kill = rf_kill;
> +
> +	platform_set_drvdata(pdev, data);
> +
> +	pr_debug("w2sg0004 probed\n");
> +
> +#ifdef CONFIG_W2SG0004_DEBUG
> +	/* turn on for debugging rx notifications */
> +	pr_debug("w2sg power gpio ON\n");
> +	gpio_set_value_cansleep(data->on_off_gpio, 1);
> +	mdelay(100);
> +	pr_debug("w2sg power gpio OFF\n");
> +	gpio_set_value_cansleep(data->on_off_gpio, 0);
> +	mdelay(300);
> +#endif
> +
> +	/* if we won't receive mctrl notifications, turn on.
> +	 * otherwise keep off until DTR is asserted through mctrl.
> +	 */
> +
> +	w2sg_data_set_power(data, !data->uart);
> +
> +	return 0;
> +
> +err_rfkill:
> +	rfkill_destroy(rf_kill);
> +out:
> +	return err;
> +}
> +
> +static int w2sg_data_remove(struct platform_device *pdev)
> +{
> +	struct w2sg_data *data = platform_get_drvdata(pdev);
> +
> +	cancel_delayed_work_sync(&data->work);
> +
> +	if (data->uart) {
> +		uart_register_rx_notification(data->uart, NULL, NULL);
> +		uart_register_slave(data->uart, NULL);
> +	}
> +	return 0;
> +}
> +
> +static int w2sg_data_suspend(struct device *dev)
> +{
> +	struct w2sg_data *data = dev_get_drvdata(dev);
> +
> +	spin_lock_irq(&data->lock);
> +	data->suspended = true;
> +	spin_unlock_irq(&data->lock);
> +
> +	cancel_delayed_work_sync(&data->work);
> +
> +	w2sg_data_set_lna_power(data);	/* shuts down if needed */
> +
> +	if (data->state == W2SG_PULSE) {
> +		msleep(10);
> +		gpio_set_value_cansleep(data->on_off_gpio, 1);
> +		data->last_toggle = jiffies;
> +		data->is_on = !data->is_on;
> +		data->state = W2SG_NOPULSE;
> +	}
> +
> +	if (data->state == W2SG_NOPULSE) {
> +		msleep(10);
> +		data->state = W2SG_IDLE;
> +	}
> +
> +	if (data->is_on) {
> +		pr_info("GPS off for suspend %d %d %d\n", data->requested,
> +			data->is_on, data->lna_is_off);
> +
> +		gpio_set_value_cansleep(data->on_off_gpio, 0);
> +		msleep(10);
> +		gpio_set_value_cansleep(data->on_off_gpio, 1);
> +		data->is_on = 0;
> +	}
> +
> +	return 0;
> +}
> +
> +static int w2sg_data_resume(struct device *dev)
> +{
> +	struct w2sg_data *data = dev_get_drvdata(dev);
> +
> +	spin_lock_irq(&data->lock);
> +	data->suspended = false;
> +	spin_unlock_irq(&data->lock);
> +
> +	pr_info("GPS resuming %d %d %d\n", data->requested,
> +		data->is_on, data->lna_is_off);
> +
> +	schedule_delayed_work(&data->work, 0);	/* enables LNA if needed */
> +
> +	return 0;
> +}
> +
> +#if defined(CONFIG_OF)
> +static const struct of_device_id w2sg0004_of_match[] = {
> +	{ .compatible = "wi2wi,w2sg0004" },
> +	{ .compatible = "wi2wi,w2sg0084" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, w2sg0004_of_match);
> +#endif
> +
> +SIMPLE_DEV_PM_OPS(w2sg_pm_ops, w2sg_data_suspend, w2sg_data_resume);
> +
> +static struct platform_driver w2sg_data_driver = {
> +	.probe		= w2sg_data_probe,
> +	.remove		= w2sg_data_remove,
> +	.driver = {
> +		.name	= "w2sg0004",
> +		.owner	= THIS_MODULE,
> +		.pm	= &w2sg_pm_ops,
> +		.of_match_table = of_match_ptr(w2sg0004_of_match)
> +	},
> +};
> +
> +module_platform_driver(w2sg_data_driver);
> +
> +MODULE_ALIAS("w2sg0004");
> +
> +MODULE_AUTHOR("NeilBrown <neilb-l3A5Bk7waGM@public.gmane.org>");
> +MODULE_DESCRIPTION("w2sg0004 GPS power management driver");
> +MODULE_LICENSE("GPL v2");
VVVsorry this change is garbage from patch editingVVV
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index b731100..c8ae0dd 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -177,7 +177,7 @@ void uart_register_slave(struct uart_port *uport, void *slave)
> EXPORT_SYMBOL_GPL(uart_register_slave);
> 
> void uart_register_mctrl_notification(struct uart_port *uport,
> -		int (*function)(void *slave, int state))
> +			int (*function)(void *slave, int state))
> {
> 	uport->mctrl_notification = function;
> }
> @@ -208,14 +208,14 @@ void uart_register_rx_notification(struct uart_port *uport,
> 			retval = uport->ops->startup(uport);
> 			if (retval == 0 && termios) {
> 				int hw_stopped;
> -				/*
> -				 * Initialise the hardware port settings.
> -				 */
> +/*
> + * Initialise the hardware port settings.
> + */
> 				uport->ops->set_termios(uport, termios, NULL);
> 
> -				/*
> -				 * Set modem status enables based on termios cflag
> -				 */
> +/*
> + * Set modem status enables based on termios cflag
> + */
> 				spin_lock_irq(&uport->lock);
> 				if (termios->c_cflag & CRTSCTS)
> 					uport->status |= UPSTAT_CTS_ENABLE;
> @@ -227,11 +227,13 @@ void uart_register_rx_notification(struct uart_port *uport,
> 				else
> 					uport->status |= UPSTAT_DCD_ENABLE;
> 
> -				/* reset sw-assisted CTS flow control based on (possibly) new mode */
> +/*
> + * reset sw-assisted CTS flow control based on (possibly) new mode
> + */
> 				hw_stopped = uport->hw_stopped;
> 				uport->hw_stopped = uart_softcts_mode(uport) &&
> -					!(uport->ops->get_mctrl(uport)
> -						& TIOCM_CTS);
> +					!(uport->ops->get_mctrl(uport) &
> +						TIOCM_CTS);
> 				if (uport->hw_stopped) {
> 					if (!hw_stopped)
> 						uport->ops->stop_tx(uport);
> @@ -432,7 +434,8 @@ static void uart_shutdown(struct tty_struct *tty, struct uart_state *state)
> 			uart_clear_mctrl(uport, TIOCM_DTR | TIOCM_RTS);
> 		/*
> 		 * if we have a slave that has registered for rx_notifications
> -		 * we do not shut down the uart port to be able to monitor the device
> +		 * we do not shut down the uart port to be able to monitor the
> +		 * device
> 		 */
> 		if (!uport->rx_notification)
> 			uart_port_shutdown(port);
^^^sorry this change is garbage from patch editing^^^
> diff --git a/include/linux/w2sg0004.h b/include/linux/w2sg0004.h
> new file mode 100644
> index 0000000..ad0c4a1
> --- /dev/null
> +++ b/include/linux/w2sg0004.h
> @@ -0,0 +1,27 @@
> +/*
> + * UART slave to allow ON/OFF control of w2sg0004 GPS receiver.
> + *
> + * Copyright (C) 2011 Neil Brown <neil-+NVA1uvv1dVBDLzU/O5InQ@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + */
> +
> +
> +
> +#ifndef __LINUX_W2SG0004_H
> +#define __LINUX_W2SG0004_H
> +
> +#include <linux/regulator/consumer.h>
> +
> +struct w2sg_pdata {
> +	struct regulator *lna_regulator;	/* enable LNA power */
> +	int	on_off_gpio;	/*  on-off input of the GPS module */
> +};
> +#endif /* __LINUX_W2SG0004_H */
> -- 
> 2.5.1
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 1/3] tty: serial core: provide a method to search uart by phandle
  2015-10-16 18:08 ` [PATCH v3 1/3] tty: serial core: provide a method to search uart by phandle H. Nikolaus Schaller
@ 2015-10-16 18:39   ` Mark Rutland
  2015-10-16 19:24     ` H. Nikolaus Schaller
  2015-10-16 18:47   ` kbuild test robot
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 16+ messages in thread
From: Mark Rutland @ 2015-10-16 18:39 UTC (permalink / raw)
  To: H. Nikolaus Schaller
  Cc: Jiri Slaby, Arnd Bergmann, Rob Herring, Pawel Moll, Ian Campbell,
	Kumar Gala, Jonathan Corbet, Sergei Zviagintsev, Peter Hurley,
	One Thousand Gnomes, Sebastian Reichel, NeilBrown, Grant Likely,
	LKML, linux-serial, Marek Belisko, devicetree, linux-doc

On Fri, Oct 16, 2015 at 08:08:33PM +0200, H. Nikolaus Schaller wrote:
> 1. add uart_ports to a search list as soon as they are registered
> 2. provide a function to search an uart_port by phandle. This copies the
>    mechanism how devm_usb_get_phy_by_phandle() works
> 3. add a bindings document how serial slaves should use this feature
> 4. add Documentation how serla slaves work in general

I thought maintainers preferred the child node approach to the phandle
approach, and this series comes with no rationale (nor change log,
despite being 'v3').

I don't understand. What is going on here?

Mark.

> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> ---
>  .../devicetree/bindings/serial/slaves.txt          |  16 +++
>  Documentation/serial/slaves.txt                    |  36 +++++++
>  drivers/tty/serial/serial_core.c                   | 107 +++++++++++++++++++++
>  include/linux/serial_core.h                        |  10 ++
>  4 files changed, 169 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/serial/slaves.txt
>  create mode 100644 Documentation/serial/slaves.txt
> 
> diff --git a/Documentation/devicetree/bindings/serial/slaves.txt b/Documentation/devicetree/bindings/serial/slaves.txt
> new file mode 100644
> index 0000000..353b87f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serial/slaves.txt
> @@ -0,0 +1,16 @@
> +Device-Tree bindings for UART slave devices
> +
> +A node describing a slave device defines a phandle to reference the UART
> +the device is connected to. In the (unexpected) case of two or more UARTs
> +a list of phandles can be specified.
> +
> +properties:
> +	- uart: (list of) phandle(s) of UART(s) the device is connected to
> +
> +
> +example:
> +
> +	gps {
> +		compatible = "wi2wi,w2sg0004";
> +		uart = <&uart1>;
> +	};
> diff --git a/Documentation/serial/slaves.txt b/Documentation/serial/slaves.txt
> new file mode 100644
> index 0000000..6f8d44d
> --- /dev/null
> +++ b/Documentation/serial/slaves.txt
> @@ -0,0 +1,36 @@
> +UART slave device support
> +
> +A remote device connected to a RS232 interface is usually power controlled by the DTR line.
> +The DTR line is managed automatically by the UART driver for open() and close() syscalls
> +and on demand by tcsetattr().
> +
> +With embedded devices, the serial peripheral might be directly and always connected to the UART
> +and there might be no physical DTR line involved. Power control (on/off) has to be done by some
> +chip specific device driver (which we call "UART slave") through some mechanisms (I2C, GPIOs etc.)
> +not related to the serial interface. Some devices do not explicitly tell their power state except
> +by sending or not sending data to the UART. In such a case the device driver must be able to monitor
> +data activity. The role of the device driver is to encapsulate such power control in a single place.
> +
> +This patch series allows to support such drivers by providing:
> +* a mechanism that a slave driver can identify the UART instance it is connected to
> +* a mechanism that UART slave drivers can register to be notified
> +* notfications for DTR (and other modem control) state changes
> +* notifications that the UART has received some data from the UART
> +
> +A slave device simply adds a phandle reference to the UART it is connected to, e.g.
> +
> +	gps {
> +		compatible = "wi2wi,w2sg0004";
> +		uart = <&uart1>;
> +	};
> +
> +The slave driver calls devm_serial_get_uart_by_phandle() to identify the uart driver.
> +This API follows the concept of devm_usb_get_phy_by_phandle().
> +
> +A slave device driver registers itself with serial_register_slave() to receive notifications.
> +Notification handler callbacks can be registered by serial_register_mctrl_notification() and
> +serial_register_rx_notification(). If an UART has registered a NULL slave or a NULL handler,
> +no notifications are sent.
> +
> +RX notification handlers can define a ktermios during setup and the handler function can modify
> +or decide to throw away each character that is passed upwards.
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index 603d2cc..9caa33e 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -38,6 +38,36 @@
>  #include <asm/irq.h>
>  #include <asm/uaccess.h>
>  
> +static LIST_HEAD(uart_list);
> +static DEFINE_SPINLOCK(uart_lock);
> +
> +/* same concept as __of_usb_find_phy */
> +static struct uart_port *__of_serial_find_uart(struct device_node *node)
> +{
> +	struct uart_port  *uart;
> +
> +	if (!of_device_is_available(node))
> +		return ERR_PTR(-ENODEV);
> +
> +	list_for_each_entry(uart, &uart_list, head) {
> +		if (node != uart->dev->of_node)
> +			continue;
> +
> +		return uart;
> +	}
> +
> +	return ERR_PTR(-EPROBE_DEFER);
> +}
> +
> +static void devm_serial_uart_release(struct device *dev, void *res)
> +{
> +	struct uart_port *uart = *(struct uart_port **)res;
> +
> +	/* FIXME: I don't understand the serial subsystem well enough
> +	 * to know if we should call serial_put_uart(uart); here
> +	 */
> +}
> +
>  /*
>   * This is used to lock changes in serial line configuration.
>   */
> @@ -64,6 +94,79 @@ static int uart_dcd_enabled(struct uart_port *uport)
>  	return !!(uport->status & UPSTAT_DCD_ENABLE);
>  }
>  
> +/**
> + * devm_serial_get_uart_by_phandle - find the uart by phandle
> + * @dev - device that requests this uart
> + * @phandle - name of the property holding the uart phandle value
> + * @index - the index of the uart
> + *
> + * Returns the uart_port associated with the given phandle value,
> + * after getting a refcount to it, -ENODEV if there is no such uart or
> + * -EPROBE_DEFER if there is a phandle to the uart, but the device is
> + * not yet loaded. While at that, it also associates the device with
> + * the uart using devres. On driver detach, release function is invoked
> + * on the devres data, then, devres data is freed.
> + *
> + * For use by tty host and peripheral drivers.
> + */
> +
> +/* same concept as devm_usb_get_phy_by_phandle() */
> +
> +struct uart_port *devm_serial_get_uart_by_phandle(struct device *dev,
> +		const char *phandle, u8 index)
> +{
> +	struct uart_port  *uart = ERR_PTR(-ENOMEM), **ptr;
> +	unsigned long   flags;
> +	struct device_node *node;
> +
> +	if (!dev->of_node) {
> +		dev_err(dev, "device does not have a device node entry\n");
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	node = of_parse_phandle(dev->of_node, phandle, index);
> +	if (!node) {
> +		dev_err(dev, "failed to get %s phandle in %s node\n", phandle,
> +			dev->of_node->full_name);
> +		return ERR_PTR(-ENODEV);
> +	}
> +
> +	ptr = devres_alloc(devm_serial_uart_release, sizeof(*ptr), GFP_KERNEL);
> +	if (!ptr) {
> +		dev_err(dev, "failed to allocate memory for devres\n");
> +		goto err0;
> +	}
> +
> +	spin_lock_irqsave(&uart_lock, flags);
> +
> +	uart = __of_serial_find_uart(node);
> +	if (IS_ERR(uart)) {
> +		devres_free(ptr);
> +		goto err1;
> +	}
> +
> +	if (!try_module_get(uart->dev->driver->owner)) {
> +		uart = ERR_PTR(-ENODEV);
> +		devres_free(ptr);
> +		goto err1;
> +	}
> +
> +	*ptr = uart;
> +	devres_add(dev, ptr);
> +
> +	get_device(uart->dev);
> +
> +err1:
> +	spin_unlock_irqrestore(&uart_lock, flags);
> +
> +err0:
> +	of_node_put(node);
> +
> +	return uart;
> +}
> +EXPORT_SYMBOL_GPL(devm_serial_get_uart_by_phandle);
> +
> +
>  /*
>   * This routine is used by the interrupt handler to schedule processing in
>   * the software interrupt portion of the driver.
> @@ -2733,6 +2836,8 @@ int uart_add_one_port(struct uart_driver *drv, struct uart_port *uport)
>  	 */
>  	uport->flags &= ~UPF_DEAD;
>  
> +	list_add_tail(&uport->head, &uart_list);
> +
>   out:
>  	mutex_unlock(&port->mutex);
>  	mutex_unlock(&port_mutex);
> @@ -2764,6 +2869,8 @@ int uart_remove_one_port(struct uart_driver *drv, struct uart_port *uport)
>  
>  	mutex_lock(&port_mutex);
>  
> +	list_del(&uport->head);
> +
>  	/*
>  	 * Mark the port "dead" - this prevents any opens from
>  	 * succeeding while we shut down the port.
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index 297d4fa..d7a2e15 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -247,6 +247,7 @@ struct uart_port {
>  	const struct attribute_group **tty_groups;	/* all attributes (serial core use only) */
>  	struct serial_rs485     rs485;
>  	void			*private_data;		/* generic platform data pointer */
> +	struct list_head	head;			/* uarts list (lookup by phandle) */
>  };
>  
>  static inline int serial_port_in(struct uart_port *up, int offset)
> @@ -475,4 +476,13 @@ static inline int uart_handle_break(struct uart_port *port)
>  					 (cflag) & CRTSCTS || \
>  					 !((cflag) & CLOCAL))
>  
> +/*
> + * Helper functions for UART slave drivers
> + */
> +
> +/* find UART by phandle (e.g. with 'uart = <&uart2>;' then call as
> + * devm_serial_get_uart_by_phandle(dev, "uart", 0);
> + */
> +extern struct uart_port *devm_serial_get_uart_by_phandle(struct device *dev,
> +		const char *phandle, u8 index);
>  #endif /* LINUX_SERIAL_CORE_H */
> -- 
> 2.5.1
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 1/3] tty: serial core: provide a method to search uart by phandle
  2015-10-16 18:08 ` [PATCH v3 1/3] tty: serial core: provide a method to search uart by phandle H. Nikolaus Schaller
  2015-10-16 18:39   ` Mark Rutland
@ 2015-10-16 18:47   ` kbuild test robot
       [not found]   ` <c46694357c903d8791f07e2ec287c684832056a5.1445018913.git.hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>
  2015-10-16 19:29   ` kbuild test robot
  3 siblings, 0 replies; 16+ messages in thread
From: kbuild test robot @ 2015-10-16 18:47 UTC (permalink / raw)
  Cc: kbuild-all, Jiri Slaby, Arnd Bergmann, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala, Jonathan Corbet,
	Sergei Zviagintsev, Peter Hurley, One Thousand Gnomes,
	Sebastian Reichel, NeilBrown, Grant Likely, LKML, linux-serial,
	Marek Belisko, devicetree, linux-doc, H. Nikolaus Schaller

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

Hi Nikolaus,

[auto build test WARNING on tty/tty-next -- if it's inappropriate base, please suggest rules for selecting the more suitable base]

url:    https://github.com/0day-ci/linux/commits/H-Nikolaus-Schaller/UART-slave-device-support-goldelico-version/20151017-021238
config: i386-randconfig-s1-201541 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

   drivers/tty/serial/serial_core.c: In function 'devm_serial_uart_release':
>> drivers/tty/serial/serial_core.c:64:20: warning: unused variable 'uart' [-Wunused-variable]
     struct uart_port *uart = *(struct uart_port **)res;
                       ^

vim +/uart +64 drivers/tty/serial/serial_core.c

    48	
    49		if (!of_device_is_available(node))
    50			return ERR_PTR(-ENODEV);
    51	
    52		list_for_each_entry(uart, &uart_list, head) {
    53			if (node != uart->dev->of_node)
    54				continue;
    55	
    56			return uart;
    57		}
    58	
    59		return ERR_PTR(-EPROBE_DEFER);
    60	}
    61	
    62	static void devm_serial_uart_release(struct device *dev, void *res)
    63	{
  > 64		struct uart_port *uart = *(struct uart_port **)res;
    65	
    66		/* FIXME: I don't understand the serial subsystem well enough
    67		 * to know if we should call serial_put_uart(uart); here
    68		 */
    69	}
    70	
    71	/*
    72	 * This is used to lock changes in serial line configuration.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 24816 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 1/3] tty: serial core: provide a method to search uart by phandle
       [not found]   ` <c46694357c903d8791f07e2ec287c684832056a5.1445018913.git.hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>
@ 2015-10-16 19:05     ` Arnd Bergmann
  0 siblings, 0 replies; 16+ messages in thread
From: Arnd Bergmann @ 2015-10-16 19:05 UTC (permalink / raw)
  To: H. Nikolaus Schaller
  Cc: Jiri Slaby, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, Jonathan Corbet, Sergei Zviagintsev, Peter Hurley,
	One Thousand Gnomes, Sebastian Reichel, NeilBrown, Grant Likely,
	LKML, linux-serial-u79uwXL29TY76Z2rM5mHXA, Marek Belisko,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-doc-u79uwXL29TY76Z2rM5mHXA

On Friday 16 October 2015 20:08:33 H. Nikolaus Schaller wrote:
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serial/slaves.txt
> @@ -0,0 +1,16 @@
> +Device-Tree bindings for UART slave devices
> +
> +A node describing a slave device defines a phandle to reference the UART
> +the device is connected to. In the (unexpected) case of two or more UARTs
> +a list of phandles can be specified.
> +
> +properties:
> +       - uart: (list of) phandle(s) of UART(s) the device is connected to
> +
> +
> +example:
> +
> +       gps {
> +               compatible = "wi2wi,w2sg0004";
> +               uart = <&uart1>;
> +       };
> 


I would have expected the gps device here to be a child node of the
uart in DT. Can you explain why you chose differently?

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 3/3] misc: Add w2sg0004 gps receiver driver
  2015-10-16 18:08 ` [PATCH v3 3/3] misc: Add w2sg0004 gps receiver driver H. Nikolaus Schaller
       [not found]   ` <c53ab7a3ca68f8b9f802c1ea799d72e1cb04a1eb.1445018913.git.hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>
@ 2015-10-16 19:06   ` Arnd Bergmann
  2015-10-16 19:27     ` H. Nikolaus Schaller
  2015-10-16 23:45   ` kbuild test robot
  2 siblings, 1 reply; 16+ messages in thread
From: Arnd Bergmann @ 2015-10-16 19:06 UTC (permalink / raw)
  To: H. Nikolaus Schaller
  Cc: Jiri Slaby, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, Jonathan Corbet, Sergei Zviagintsev, Peter Hurley,
	One Thousand Gnomes, Sebastian Reichel, NeilBrown, Grant Likely,
	LKML, linux-serial, Marek Belisko, devicetree, linux-doc

On Friday 16 October 2015 20:08:35 H. Nikolaus Schaller wrote:
> +
> +static int w2sg_data_probe(struct platform_device *pdev)
> +{
> +       struct w2sg_pdata *pdata = dev_get_platdata(&pdev->dev);
> +       struct w2sg_data *data;
> +       struct rfkill *rf_kill;
> +       int err;
> +
> +       pr_debug("%s()\n", __func__);
> +
> +       if (pdev->dev.of_node) {
> +               struct device *dev = &pdev->dev;
> +               enum of_gpio_flags flags;
> +
> +               pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> +               if (!pdata)
> 

Why is this a platform_device and not a serio_device?

	Arnd

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 0/3] UART slave device support (goldelico version)
  2015-10-16 18:08 [PATCH v3 0/3] UART slave device support (goldelico version) H. Nikolaus Schaller
                   ` (2 preceding siblings ...)
  2015-10-16 18:08 ` [PATCH v3 3/3] misc: Add w2sg0004 gps receiver driver H. Nikolaus Schaller
@ 2015-10-16 19:11 ` H. Nikolaus Schaller
  3 siblings, 0 replies; 16+ messages in thread
From: H. Nikolaus Schaller @ 2015-10-16 19:11 UTC (permalink / raw)
  To: Jiri Slaby, Arnd Bergmann, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, Jonathan Corbet
  Cc: Sergei Zviagintsev, Peter Hurley, One Thousand Gnomes,
	Sebastian Reichel, NeilBrown, Grant Likely, LKML, linux-serial,
	Marek Belisko, devicetree, linux-doc

Changes V3:
- changed from RFC to PATCH
- added separate bindings and concept documentation documents
- worked in comments by Sergei Zviagintsev

Changes V2:
- fixed some formatting

This patch series is our proposal to add hooks so that the driver for a device connected to an UART can
monitor modem control lines and data activity of the connected chip.

It contains an example for such a device driver which needs such sophisticated power control: wi2wi,w2sg0004

A remote device connected to a RS232 interface is usually power controlled by the DTR line.
The DTR line is managed automatically by the UART driver for open() and close() syscalls
and on demand by tcsetattr().

With embedded devices, the serial peripheral might be directly and always connected to the UART
and there might be no physical DTR line involved. Power control (on/off) has to be done by some
chip specific device driver (which we call "UART slave") through some mechanisms (I2C, GPIOs etc.)
not related to the serial interface. Some devices do not tell their power state except by sending
or not sending data to the UART. In such a case the device driver must be able to monitor data
activity. The role of the device driver is to encapsulate such power control in a single place.

This patch series allows to support such UART slave drivers by providing:
* a mechanism that a slave driver can identify the UART instance it is connected to
* a mechanism that UART slave drivers can register to be notified
* notfications for DTR (and other modem control) state changes
* notifications that the device has sent some data to the UART

A slave device tree entry simply adds a phandle reference to the UART it is connected to, e.g.

       gps {
               compatible = "wi2wi,w2sg0004";
               uart = <&uart1>;
       };

The slave driver calls devm_serial_get_uart_by_phandle() to identify the uart driver.
devm_serial_get_uart_by_phandle() follows the concept of devm_usb_get_phy_by_phandle().

A slave device driver registers itself with serial_register_slave() to receive notifications.
Notification handlers can be registered by serial_register_mctrl_notification() and
serial_register_rx_notification(). If an UART has a NULL slave or a NULL handler registered,
no notifications are sent.

RX notification handlers can define a ktermios setup and modify or decide to throw away the
character that is passed upwards.

This all is a follow-up to the w2sg0004 driver submitted in 2014 that did want to add an optional
GPIO to DTR in omap-serial.c and required the w2sg0004 driver to present itself as a "virtual
GPIO". The idea of a "virtual GPIO"  is not compatible with the concept that DT must
describe hardware (and not virtual hardware). So in this new solution DT only describes that
the w2sg0004 is connected to some UART and how the power state signalling works is left
to the driver implementations.

The rx data notification also removes the concept of having two different pinmux states
and make the w2sg0004 driver intercept rx activities by switching the rx line to a GPIO
interrupt. This was very OMAP3 specific. The new solution is generic and might even be
extensible that the chip driver could filter or consume the rx data before it is passed
to the tty layer.

This patch works on 4.3-rc as intended except one major weakness: we have to call
uart_change_speed() each time we open the tty. This is the opposite of what we would like
to have: that the slave initializes the uart speed through some termios and the tty level just uses
this setting. We have not yet completely understood how to make this work and are happy
about help in this area.


Am 16.10.2015 um 20:08 schrieb H. Nikolaus Schaller <hns@goldelico.com>:

> H. Nikolaus Schaller (3):
>  tty: serial core: provide a method to search uart by phandle
>  tty: serial_core: add hooks for uart slave drivers
>  misc: Add w2sg0004 gps receiver driver
> 
> .../devicetree/bindings/misc/wi2wi,w2sg0004.txt    |  18 +
> .../devicetree/bindings/serial/slaves.txt          |  16 +
> .../devicetree/bindings/vendor-prefixes.txt        |   1 +
> Documentation/serial/slaves.txt                    |  36 ++
> drivers/misc/Kconfig                               |  18 +
> drivers/misc/Makefile                              |   1 +
> drivers/misc/w2sg0004.c                            | 443 +++++++++++++++++++++
> drivers/tty/serial/serial_core.c                   | 214 +++++++++-
> include/linux/serial_core.h                        |  25 +-
> include/linux/w2sg0004.h                           |  27 ++
> 10 files changed, 793 insertions(+), 6 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
> create mode 100644 Documentation/devicetree/bindings/serial/slaves.txt
> create mode 100644 Documentation/serial/slaves.txt
> create mode 100644 drivers/misc/w2sg0004.c
> create mode 100644 include/linux/w2sg0004.h
> 
> -- 
> 2.5.1
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 1/3] tty: serial core: provide a method to search uart by phandle
  2015-10-16 18:39   ` Mark Rutland
@ 2015-10-16 19:24     ` H. Nikolaus Schaller
  0 siblings, 0 replies; 16+ messages in thread
From: H. Nikolaus Schaller @ 2015-10-16 19:24 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Jiri Slaby, Arnd Bergmann, Rob Herring, Pawel Moll, Ian Campbell,
	Kumar Gala, Jonathan Corbet, Sergei Zviagintsev, Peter Hurley,
	One Thousand Gnomes, Sebastian Reichel, NeilBrown, Grant Likely,
	LKML, linux-serial, Marek Belisko, devicetree, linux-doc


Am 16.10.2015 um 20:39 schrieb Mark Rutland <mark.rutland@arm.com>:

> On Fri, Oct 16, 2015 at 08:08:33PM +0200, H. Nikolaus Schaller wrote:
>> 1. add uart_ports to a search list as soon as they are registered
>> 2. provide a function to search an uart_port by phandle. This copies the
>>   mechanism how devm_usb_get_phy_by_phandle() works
>> 3. add a bindings document how serial slaves should use this feature
>> 4. add Documentation how serla slaves work in general
> 
> I thought maintainers preferred the child node approach to the phandle
> approach,

> and this series comes with no rationale (nor change log,
> despite being 'v3').

For unknown reasons it was not part of the outgoing git send-email.

Today is not my day of operating command line git...

I have added it as a reply.

> 
> I don't understand. What is going on here?

There was a discussion about child vs. phandle and I came up with thousands
of technical arguments and examples from other subsystems where phandle
is common. Because I still don't believe that child node approach is the right one.

At some time of this discussion, I was asked to provide code because people
wanted to compare both ideas on code basis.

Therefore I wrote an implementation that works and shows all aspects.
I published V1 and V2 and got some comments, but not really much.

What I never got was a really convincing argumentation or principle or DT writer's
guideline why uart slaves must be subnodes (except that "uart" is a degenerate
variant of a "bus" and therefore must be prepared to have multiple child nodes).

The latest counter-example I have found is how iio adcs are accessed:
Documentation/devicetree/bindings/iio/iio-bindings.txt

One could as well argue that adcs are a "bus" (especially if they have multiple
inputs) and therefore all consumers of adc data must be their children... But they
are not.

Nothing has happened since I submittted RFC V2. I.e. there is no child node based
approach accepted. There was no significant comparison or discussion.

Therefore I took the freedom to resubmit my code and prevent it from bitrotting
in my local git repo.

Hope this explains.

BR and thanks,
Nikolaus


> Mark.
> 
>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>> ---
>> .../devicetree/bindings/serial/slaves.txt          |  16 +++
>> Documentation/serial/slaves.txt                    |  36 +++++++
>> drivers/tty/serial/serial_core.c                   | 107 +++++++++++++++++++++
>> include/linux/serial_core.h                        |  10 ++
>> 4 files changed, 169 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/serial/slaves.txt
>> create mode 100644 Documentation/serial/slaves.txt
>> 
>> diff --git a/Documentation/devicetree/bindings/serial/slaves.txt b/Documentation/devicetree/bindings/serial/slaves.txt
>> new file mode 100644
>> index 0000000..353b87f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/serial/slaves.txt
>> @@ -0,0 +1,16 @@
>> +Device-Tree bindings for UART slave devices
>> +
>> +A node describing a slave device defines a phandle to reference the UART
>> +the device is connected to. In the (unexpected) case of two or more UARTs
>> +a list of phandles can be specified.
>> +
>> +properties:
>> +	- uart: (list of) phandle(s) of UART(s) the device is connected to
>> +
>> +
>> +example:
>> +
>> +	gps {
>> +		compatible = "wi2wi,w2sg0004";
>> +		uart = <&uart1>;
>> +	};
>> diff --git a/Documentation/serial/slaves.txt b/Documentation/serial/slaves.txt
>> new file mode 100644
>> index 0000000..6f8d44d
>> --- /dev/null
>> +++ b/Documentation/serial/slaves.txt
>> @@ -0,0 +1,36 @@
>> +UART slave device support
>> +
>> +A remote device connected to a RS232 interface is usually power controlled by the DTR line.
>> +The DTR line is managed automatically by the UART driver for open() and close() syscalls
>> +and on demand by tcsetattr().
>> +
>> +With embedded devices, the serial peripheral might be directly and always connected to the UART
>> +and there might be no physical DTR line involved. Power control (on/off) has to be done by some
>> +chip specific device driver (which we call "UART slave") through some mechanisms (I2C, GPIOs etc.)
>> +not related to the serial interface. Some devices do not explicitly tell their power state except
>> +by sending or not sending data to the UART. In such a case the device driver must be able to monitor
>> +data activity. The role of the device driver is to encapsulate such power control in a single place.
>> +
>> +This patch series allows to support such drivers by providing:
>> +* a mechanism that a slave driver can identify the UART instance it is connected to
>> +* a mechanism that UART slave drivers can register to be notified
>> +* notfications for DTR (and other modem control) state changes
>> +* notifications that the UART has received some data from the UART
>> +
>> +A slave device simply adds a phandle reference to the UART it is connected to, e.g.
>> +
>> +	gps {
>> +		compatible = "wi2wi,w2sg0004";
>> +		uart = <&uart1>;
>> +	};
>> +
>> +The slave driver calls devm_serial_get_uart_by_phandle() to identify the uart driver.
>> +This API follows the concept of devm_usb_get_phy_by_phandle().
>> +
>> +A slave device driver registers itself with serial_register_slave() to receive notifications.
>> +Notification handler callbacks can be registered by serial_register_mctrl_notification() and
>> +serial_register_rx_notification(). If an UART has registered a NULL slave or a NULL handler,
>> +no notifications are sent.
>> +
>> +RX notification handlers can define a ktermios during setup and the handler function can modify
>> +or decide to throw away each character that is passed upwards.
>> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
>> index 603d2cc..9caa33e 100644
>> --- a/drivers/tty/serial/serial_core.c
>> +++ b/drivers/tty/serial/serial_core.c
>> @@ -38,6 +38,36 @@
>> #include <asm/irq.h>
>> #include <asm/uaccess.h>
>> 
>> +static LIST_HEAD(uart_list);
>> +static DEFINE_SPINLOCK(uart_lock);
>> +
>> +/* same concept as __of_usb_find_phy */
>> +static struct uart_port *__of_serial_find_uart(struct device_node *node)
>> +{
>> +	struct uart_port  *uart;
>> +
>> +	if (!of_device_is_available(node))
>> +		return ERR_PTR(-ENODEV);
>> +
>> +	list_for_each_entry(uart, &uart_list, head) {
>> +		if (node != uart->dev->of_node)
>> +			continue;
>> +
>> +		return uart;
>> +	}
>> +
>> +	return ERR_PTR(-EPROBE_DEFER);
>> +}
>> +
>> +static void devm_serial_uart_release(struct device *dev, void *res)
>> +{
>> +	struct uart_port *uart = *(struct uart_port **)res;
>> +
>> +	/* FIXME: I don't understand the serial subsystem well enough
>> +	 * to know if we should call serial_put_uart(uart); here
>> +	 */
>> +}
>> +
>> /*
>>  * This is used to lock changes in serial line configuration.
>>  */
>> @@ -64,6 +94,79 @@ static int uart_dcd_enabled(struct uart_port *uport)
>> 	return !!(uport->status & UPSTAT_DCD_ENABLE);
>> }
>> 
>> +/**
>> + * devm_serial_get_uart_by_phandle - find the uart by phandle
>> + * @dev - device that requests this uart
>> + * @phandle - name of the property holding the uart phandle value
>> + * @index - the index of the uart
>> + *
>> + * Returns the uart_port associated with the given phandle value,
>> + * after getting a refcount to it, -ENODEV if there is no such uart or
>> + * -EPROBE_DEFER if there is a phandle to the uart, but the device is
>> + * not yet loaded. While at that, it also associates the device with
>> + * the uart using devres. On driver detach, release function is invoked
>> + * on the devres data, then, devres data is freed.
>> + *
>> + * For use by tty host and peripheral drivers.
>> + */
>> +
>> +/* same concept as devm_usb_get_phy_by_phandle() */
>> +
>> +struct uart_port *devm_serial_get_uart_by_phandle(struct device *dev,
>> +		const char *phandle, u8 index)
>> +{
>> +	struct uart_port  *uart = ERR_PTR(-ENOMEM), **ptr;
>> +	unsigned long   flags;
>> +	struct device_node *node;
>> +
>> +	if (!dev->of_node) {
>> +		dev_err(dev, "device does not have a device node entry\n");
>> +		return ERR_PTR(-EINVAL);
>> +	}
>> +
>> +	node = of_parse_phandle(dev->of_node, phandle, index);
>> +	if (!node) {
>> +		dev_err(dev, "failed to get %s phandle in %s node\n", phandle,
>> +			dev->of_node->full_name);
>> +		return ERR_PTR(-ENODEV);
>> +	}
>> +
>> +	ptr = devres_alloc(devm_serial_uart_release, sizeof(*ptr), GFP_KERNEL);
>> +	if (!ptr) {
>> +		dev_err(dev, "failed to allocate memory for devres\n");
>> +		goto err0;
>> +	}
>> +
>> +	spin_lock_irqsave(&uart_lock, flags);
>> +
>> +	uart = __of_serial_find_uart(node);
>> +	if (IS_ERR(uart)) {
>> +		devres_free(ptr);
>> +		goto err1;
>> +	}
>> +
>> +	if (!try_module_get(uart->dev->driver->owner)) {
>> +		uart = ERR_PTR(-ENODEV);
>> +		devres_free(ptr);
>> +		goto err1;
>> +	}
>> +
>> +	*ptr = uart;
>> +	devres_add(dev, ptr);
>> +
>> +	get_device(uart->dev);
>> +
>> +err1:
>> +	spin_unlock_irqrestore(&uart_lock, flags);
>> +
>> +err0:
>> +	of_node_put(node);
>> +
>> +	return uart;
>> +}
>> +EXPORT_SYMBOL_GPL(devm_serial_get_uart_by_phandle);
>> +
>> +
>> /*
>>  * This routine is used by the interrupt handler to schedule processing in
>>  * the software interrupt portion of the driver.
>> @@ -2733,6 +2836,8 @@ int uart_add_one_port(struct uart_driver *drv, struct uart_port *uport)
>> 	 */
>> 	uport->flags &= ~UPF_DEAD;
>> 
>> +	list_add_tail(&uport->head, &uart_list);
>> +
>>  out:
>> 	mutex_unlock(&port->mutex);
>> 	mutex_unlock(&port_mutex);
>> @@ -2764,6 +2869,8 @@ int uart_remove_one_port(struct uart_driver *drv, struct uart_port *uport)
>> 
>> 	mutex_lock(&port_mutex);
>> 
>> +	list_del(&uport->head);
>> +
>> 	/*
>> 	 * Mark the port "dead" - this prevents any opens from
>> 	 * succeeding while we shut down the port.
>> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
>> index 297d4fa..d7a2e15 100644
>> --- a/include/linux/serial_core.h
>> +++ b/include/linux/serial_core.h
>> @@ -247,6 +247,7 @@ struct uart_port {
>> 	const struct attribute_group **tty_groups;	/* all attributes (serial core use only) */
>> 	struct serial_rs485     rs485;
>> 	void			*private_data;		/* generic platform data pointer */
>> +	struct list_head	head;			/* uarts list (lookup by phandle) */
>> };
>> 
>> static inline int serial_port_in(struct uart_port *up, int offset)
>> @@ -475,4 +476,13 @@ static inline int uart_handle_break(struct uart_port *port)
>> 					 (cflag) & CRTSCTS || \
>> 					 !((cflag) & CLOCAL))
>> 
>> +/*
>> + * Helper functions for UART slave drivers
>> + */
>> +
>> +/* find UART by phandle (e.g. with 'uart = <&uart2>;' then call as
>> + * devm_serial_get_uart_by_phandle(dev, "uart", 0);
>> + */
>> +extern struct uart_port *devm_serial_get_uart_by_phandle(struct device *dev,
>> +		const char *phandle, u8 index);
>> #endif /* LINUX_SERIAL_CORE_H */
>> -- 
>> 2.5.1
>> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 3/3] misc: Add w2sg0004 gps receiver driver
  2015-10-16 19:06   ` Arnd Bergmann
@ 2015-10-16 19:27     ` H. Nikolaus Schaller
  2015-10-16 19:38       ` Arnd Bergmann
  0 siblings, 1 reply; 16+ messages in thread
From: H. Nikolaus Schaller @ 2015-10-16 19:27 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Jiri Slaby, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, Jonathan Corbet, Sergei Zviagintsev, Peter Hurley,
	One Thousand Gnomes, Sebastian Reichel, NeilBrown, Grant Likely,
	LKML, linux-serial, Marek Belisko, devicetree, linux-doc


Am 16.10.2015 um 21:06 schrieb Arnd Bergmann <arnd@arndb.de>:

> On Friday 16 October 2015 20:08:35 H. Nikolaus Schaller wrote:
>> +
>> +static int w2sg_data_probe(struct platform_device *pdev)
>> +{
>> +       struct w2sg_pdata *pdata = dev_get_platdata(&pdev->dev);
>> +       struct w2sg_data *data;
>> +       struct rfkill *rf_kill;
>> +       int err;
>> +
>> +       pr_debug("%s()\n", __func__);
>> +
>> +       if (pdev->dev.of_node) {
>> +               struct device *dev = &pdev->dev;
>> +               enum of_gpio_flags flags;
>> +
>> +               pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
>> +               if (!pdata)
>> 
> 
> Why is this a platform_device and not a serio_device?

I can't find a struct serio_device. What is that?

BR and thanks,
Nikolaus

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 1/3] tty: serial core: provide a method to search uart by phandle
  2015-10-16 18:08 ` [PATCH v3 1/3] tty: serial core: provide a method to search uart by phandle H. Nikolaus Schaller
                     ` (2 preceding siblings ...)
       [not found]   ` <c46694357c903d8791f07e2ec287c684832056a5.1445018913.git.hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>
@ 2015-10-16 19:29   ` kbuild test robot
  3 siblings, 0 replies; 16+ messages in thread
From: kbuild test robot @ 2015-10-16 19:29 UTC (permalink / raw)
  Cc: kbuild-all, Jiri Slaby, Arnd Bergmann, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala, Jonathan Corbet,
	Sergei Zviagintsev, Peter Hurley, One Thousand Gnomes,
	Sebastian Reichel, NeilBrown, Grant Likely, LKML, linux-serial,
	Marek Belisko, devicetree, linux-doc, H. Nikolaus Schaller

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

Hi Nikolaus,

[auto build test WARNING on tty/tty-next -- if it's inappropriate base, please suggest rules for selecting the more suitable base]

url:    https://github.com/0day-ci/linux/commits/H-Nikolaus-Schaller/UART-slave-device-support-goldelico-version/20151017-021238
reproduce: make htmldocs

All warnings (new ones prefixed by >>):

   include/linux/init.h:1: warning: no structured comments found
   kernel/sys.c:1: warning: no structured comments found
   drivers/dma-buf/seqno-fence.c:1: warning: no structured comments found
   drivers/dma-buf/reservation.c:1: warning: no structured comments found
   include/linux/reservation.h:1: warning: no structured comments found
   include/media/v4l2-dv-timings.h:29: warning: cannot understand function prototype: 'const struct v4l2_dv_timings v4l2_dv_timings_presets[]; '
   include/media/v4l2-dv-timings.h:147: warning: No description found for parameter 'frame_height'
   include/media/v4l2-dv-timings.h:147: warning: No description found for parameter 'hfreq'
   include/media/v4l2-dv-timings.h:147: warning: No description found for parameter 'vsync'
   include/media/v4l2-dv-timings.h:147: warning: No description found for parameter 'active_width'
   include/media/v4l2-dv-timings.h:147: warning: No description found for parameter 'polarities'
   include/media/v4l2-dv-timings.h:147: warning: No description found for parameter 'interlaced'
   include/media/v4l2-dv-timings.h:147: warning: No description found for parameter 'fmt'
   include/media/v4l2-dv-timings.h:171: warning: No description found for parameter 'frame_height'
   include/media/v4l2-dv-timings.h:171: warning: No description found for parameter 'hfreq'
   include/media/v4l2-dv-timings.h:171: warning: No description found for parameter 'vsync'
   include/media/v4l2-dv-timings.h:171: warning: No description found for parameter 'polarities'
   include/media/v4l2-dv-timings.h:171: warning: No description found for parameter 'interlaced'
   include/media/v4l2-dv-timings.h:171: warning: No description found for parameter 'aspect'
   include/media/v4l2-dv-timings.h:171: warning: No description found for parameter 'fmt'
   include/media/v4l2-dv-timings.h:184: warning: No description found for parameter 'hor_landscape'
   include/media/v4l2-dv-timings.h:184: warning: No description found for parameter 'vert_portrait'
   include/media/videobuf2-core.h:112: warning: No description found for parameter 'get_dmabuf'
   include/media/videobuf2-core.h:233: warning: No description found for parameter 'cnt_mem_alloc'
   include/media/videobuf2-core.h:233: warning: No description found for parameter 'cnt_mem_put'
   include/media/videobuf2-core.h:233: warning: No description found for parameter 'cnt_mem_get_dmabuf'
   include/media/videobuf2-core.h:233: warning: No description found for parameter 'cnt_mem_get_userptr'
   include/media/videobuf2-core.h:233: warning: No description found for parameter 'cnt_mem_put_userptr'
   include/media/videobuf2-core.h:233: warning: No description found for parameter 'cnt_mem_prepare'
   include/media/videobuf2-core.h:233: warning: No description found for parameter 'cnt_mem_finish'
   include/media/videobuf2-core.h:233: warning: No description found for parameter 'cnt_mem_attach_dmabuf'
   include/media/videobuf2-core.h:233: warning: No description found for parameter 'cnt_mem_detach_dmabuf'
   include/media/videobuf2-core.h:233: warning: No description found for parameter 'cnt_mem_map_dmabuf'
   include/media/videobuf2-core.h:233: warning: No description found for parameter 'cnt_mem_unmap_dmabuf'
   include/media/videobuf2-core.h:233: warning: No description found for parameter 'cnt_mem_vaddr'
   include/media/videobuf2-core.h:233: warning: No description found for parameter 'cnt_mem_cookie'
   include/media/videobuf2-core.h:233: warning: No description found for parameter 'cnt_mem_num_users'
   include/media/videobuf2-core.h:233: warning: No description found for parameter 'cnt_mem_mmap'
   include/media/videobuf2-core.h:233: warning: No description found for parameter 'cnt_buf_init'
   include/media/videobuf2-core.h:233: warning: No description found for parameter 'cnt_buf_prepare'
   include/media/videobuf2-core.h:233: warning: No description found for parameter 'cnt_buf_finish'
   include/media/videobuf2-core.h:233: warning: No description found for parameter 'cnt_buf_cleanup'
   include/media/videobuf2-core.h:233: warning: No description found for parameter 'cnt_buf_queue'
   include/media/videobuf2-core.h:233: warning: No description found for parameter 'cnt_buf_done'
   drivers/media/dvb-core/dvbdev.h:199: warning: Excess function parameter 'device' description in 'dvb_register_device'
   drivers/media/dvb-core/dvbdev.h:199: warning: Excess function parameter 'adapter_nums' description in 'dvb_register_device'
>> drivers/tty/serial/serial_core.c:117: warning: No description found for parameter 'dev'
>> drivers/tty/serial/serial_core.c:117: warning: No description found for parameter 'phandle'
>> drivers/tty/serial/serial_core.c:117: warning: No description found for parameter 'index'
   include/linux/hsi/hsi.h:150: warning: Excess struct/union/enum/typedef member 'e_handler' description in 'hsi_client'
   include/linux/hsi/hsi.h:150: warning: Excess struct/union/enum/typedef member 'pclaimed' description in 'hsi_client'
   include/linux/hsi/hsi.h:150: warning: Excess struct/union/enum/typedef member 'nb' description in 'hsi_client'

vim +/dev +117 drivers/tty/serial/serial_core.c

   101	 * @index - the index of the uart
   102	 *
   103	 * Returns the uart_port associated with the given phandle value,
   104	 * after getting a refcount to it, -ENODEV if there is no such uart or
   105	 * -EPROBE_DEFER if there is a phandle to the uart, but the device is
   106	 * not yet loaded. While at that, it also associates the device with
   107	 * the uart using devres. On driver detach, release function is invoked
   108	 * on the devres data, then, devres data is freed.
   109	 *
   110	 * For use by tty host and peripheral drivers.
   111	 */
   112	
   113	/* same concept as devm_usb_get_phy_by_phandle() */
   114	
   115	struct uart_port *devm_serial_get_uart_by_phandle(struct device *dev,
   116			const char *phandle, u8 index)
 > 117	{
   118		struct uart_port  *uart = ERR_PTR(-ENOMEM), **ptr;
   119		unsigned long   flags;
   120		struct device_node *node;
   121	
   122		if (!dev->of_node) {
   123			dev_err(dev, "device does not have a device node entry\n");
   124			return ERR_PTR(-EINVAL);
   125		}

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 6062 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 3/3] misc: Add w2sg0004 gps receiver driver
  2015-10-16 19:27     ` H. Nikolaus Schaller
@ 2015-10-16 19:38       ` Arnd Bergmann
  2015-10-16 20:07         ` H. Nikolaus Schaller
  0 siblings, 1 reply; 16+ messages in thread
From: Arnd Bergmann @ 2015-10-16 19:38 UTC (permalink / raw)
  To: H. Nikolaus Schaller
  Cc: Jiri Slaby, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, Jonathan Corbet, Sergei Zviagintsev, Peter Hurley,
	One Thousand Gnomes, Sebastian Reichel, NeilBrown, Grant Likely,
	LKML, linux-serial, Marek Belisko, devicetree, linux-doc

On Friday 16 October 2015 21:27:11 H. Nikolaus Schaller wrote:
> Am 16.10.2015 um 21:06 schrieb Arnd Bergmann <arnd@arndb.de>:
> 
> > On Friday 16 October 2015 20:08:35 H. Nikolaus Schaller wrote:
> >> +
> >> +static int w2sg_data_probe(struct platform_device *pdev)
> >> +{
> >> +       struct w2sg_pdata *pdata = dev_get_platdata(&pdev->dev);
> >> +       struct w2sg_data *data;
> >> +       struct rfkill *rf_kill;
> >> +       int err;
> >> +
> >> +       pr_debug("%s()\n", __func__);
> >> +
> >> +       if (pdev->dev.of_node) {
> >> +               struct device *dev = &pdev->dev;
> >> +               enum of_gpio_flags flags;
> >> +
> >> +               pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> >> +               if (!pdata)
> >> 
> > 
> > Why is this a platform_device and not a serio_device?
> 
> I can't find a struct serio_device. What is that?
> 

Sorry, I meant 'struct serio', see drivers/input/serio/

This is an existing infrastructure that is used for devices attached
to a dumb serial device (rs232 or 8042/psaux usually). They have
a user interface for connecting a driver to a port, but you should
be able to do it all in the kernel as well if DT has the information
what device is connected.

	Arnd

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 3/3] misc: Add w2sg0004 gps receiver driver
  2015-10-16 19:38       ` Arnd Bergmann
@ 2015-10-16 20:07         ` H. Nikolaus Schaller
  0 siblings, 0 replies; 16+ messages in thread
From: H. Nikolaus Schaller @ 2015-10-16 20:07 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Jiri Slaby, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, Jonathan Corbet, Sergei Zviagintsev, Peter Hurley,
	One Thousand Gnomes, Sebastian Reichel, NeilBrown, Grant Likely,
	LKML, linux-serial, Marek Belisko, devicetree, linux-doc


Am 16.10.2015 um 21:38 schrieb Arnd Bergmann <arnd@arndb.de>:

> On Friday 16 October 2015 21:27:11 H. Nikolaus Schaller wrote:
>> Am 16.10.2015 um 21:06 schrieb Arnd Bergmann <arnd@arndb.de>:
>> 
>>> On Friday 16 October 2015 20:08:35 H. Nikolaus Schaller wrote:
>>>> +
>>>> +static int w2sg_data_probe(struct platform_device *pdev)
>>>> +{
>>>> +       struct w2sg_pdata *pdata = dev_get_platdata(&pdev->dev);
>>>> +       struct w2sg_data *data;
>>>> +       struct rfkill *rf_kill;
>>>> +       int err;
>>>> +
>>>> +       pr_debug("%s()\n", __func__);
>>>> +
>>>> +       if (pdev->dev.of_node) {
>>>> +               struct device *dev = &pdev->dev;
>>>> +               enum of_gpio_flags flags;
>>>> +
>>>> +               pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
>>>> +               if (!pdata)
>>>> 
>>> 
>>> Why is this a platform_device and not a serio_device?
>> 
>> I can't find a struct serio_device. What is that?
>> 
> 
> Sorry, I meant 'struct serio', see drivers/input/serio/
> 
> This is an existing infrastructure that is used for devices attached
> to a dumb serial device (rs232 or 8042/psaux usually). They have
> a user interface for connecting a driver to a port, but you should
> be able to do it all in the kernel as well if DT has the information
> what device is connected.

Ah, I understand. But it is for a different purpose. E.g. making a
serial device (mouse/touch) an input device. So it is a driver sitting
"on top" of tty/uart drivers.

The problem to be solved here is a different one. The only task for
the driver is to do power control of the device. I.e. turn it on by
open("/dev/ttyX") or asserting DTR.

So we are on a much lower level.

Please see also the patch 0/3 I have resent (the BLURB defined
by git --edit-description was apparently eaten by my git send-email).

BR and thanks,
Nikolaus


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 3/3] misc: Add w2sg0004 gps receiver driver
  2015-10-16 18:08 ` [PATCH v3 3/3] misc: Add w2sg0004 gps receiver driver H. Nikolaus Schaller
       [not found]   ` <c53ab7a3ca68f8b9f802c1ea799d72e1cb04a1eb.1445018913.git.hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>
  2015-10-16 19:06   ` Arnd Bergmann
@ 2015-10-16 23:45   ` kbuild test robot
  2 siblings, 0 replies; 16+ messages in thread
From: kbuild test robot @ 2015-10-16 23:45 UTC (permalink / raw)
  Cc: kbuild-all, Jiri Slaby, Arnd Bergmann, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala, Jonathan Corbet,
	Sergei Zviagintsev, Peter Hurley, One Thousand Gnomes,
	Sebastian Reichel, NeilBrown, Grant Likely, LKML, linux-serial,
	Marek Belisko, devicetree, linux-doc, H. Nikolaus Schaller

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

Hi Nikolaus,

[auto build test WARNING on tty/tty-next -- if it's inappropriate base, please suggest rules for selecting the more suitable base]

url:    https://github.com/0day-ci/linux/commits/H-Nikolaus-Schaller/UART-slave-device-support-goldelico-version/20151017-021238
config: xtensa-allmodconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=xtensa 

All warnings (new ones prefixed by >>):

>> drivers/misc/w2sg0004.c:361:12: warning: 'w2sg_data_suspend' defined but not used [-Wunused-function]
    static int w2sg_data_suspend(struct device *dev)
               ^
>> drivers/misc/w2sg0004.c:399:12: warning: 'w2sg_data_resume' defined but not used [-Wunused-function]
    static int w2sg_data_resume(struct device *dev)
               ^

vim +/w2sg_data_suspend +361 drivers/misc/w2sg0004.c

   355			uart_register_rx_notification(data->uart, NULL, NULL);
   356			uart_register_slave(data->uart, NULL);
   357		}
   358		return 0;
   359	}
   360	
 > 361	static int w2sg_data_suspend(struct device *dev)
   362	{
   363		struct w2sg_data *data = dev_get_drvdata(dev);
   364	
   365		spin_lock_irq(&data->lock);
   366		data->suspended = true;
   367		spin_unlock_irq(&data->lock);
   368	
   369		cancel_delayed_work_sync(&data->work);
   370	
   371		w2sg_data_set_lna_power(data);	/* shuts down if needed */
   372	
   373		if (data->state == W2SG_PULSE) {
   374			msleep(10);
   375			gpio_set_value_cansleep(data->on_off_gpio, 1);
   376			data->last_toggle = jiffies;
   377			data->is_on = !data->is_on;
   378			data->state = W2SG_NOPULSE;
   379		}
   380	
   381		if (data->state == W2SG_NOPULSE) {
   382			msleep(10);
   383			data->state = W2SG_IDLE;
   384		}
   385	
   386		if (data->is_on) {
   387			pr_info("GPS off for suspend %d %d %d\n", data->requested,
   388				data->is_on, data->lna_is_off);
   389	
   390			gpio_set_value_cansleep(data->on_off_gpio, 0);
   391			msleep(10);
   392			gpio_set_value_cansleep(data->on_off_gpio, 1);
   393			data->is_on = 0;
   394		}
   395	
   396		return 0;
   397	}
   398	
 > 399	static int w2sg_data_resume(struct device *dev)
   400	{
   401		struct w2sg_data *data = dev_get_drvdata(dev);
   402	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 42196 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2015-10-16 23:45 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-16 18:08 [PATCH v3 0/3] UART slave device support (goldelico version) H. Nikolaus Schaller
2015-10-16 18:08 ` [PATCH v3 1/3] tty: serial core: provide a method to search uart by phandle H. Nikolaus Schaller
2015-10-16 18:39   ` Mark Rutland
2015-10-16 19:24     ` H. Nikolaus Schaller
2015-10-16 18:47   ` kbuild test robot
     [not found]   ` <c46694357c903d8791f07e2ec287c684832056a5.1445018913.git.hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>
2015-10-16 19:05     ` Arnd Bergmann
2015-10-16 19:29   ` kbuild test robot
2015-10-16 18:08 ` [PATCH v3 2/3] tty: serial_core: add hooks for uart slave drivers H. Nikolaus Schaller
2015-10-16 18:08 ` [PATCH v3 3/3] misc: Add w2sg0004 gps receiver driver H. Nikolaus Schaller
     [not found]   ` <c53ab7a3ca68f8b9f802c1ea799d72e1cb04a1eb.1445018913.git.hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>
2015-10-16 18:15     ` H. Nikolaus Schaller
2015-10-16 19:06   ` Arnd Bergmann
2015-10-16 19:27     ` H. Nikolaus Schaller
2015-10-16 19:38       ` Arnd Bergmann
2015-10-16 20:07         ` H. Nikolaus Schaller
2015-10-16 23:45   ` kbuild test robot
2015-10-16 19:11 ` [PATCH v3 0/3] UART slave device support (goldelico version) H. Nikolaus Schaller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).