All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC 0/3] misc: new serdev based drivers for w2sg00x4 GPS module and w2cbw003 wifi/bluetooth
@ 2017-05-21 10:44 H. Nikolaus Schaller
  2017-05-21 10:44   ` H. Nikolaus Schaller
                   ` (3 more replies)
  0 siblings, 4 replies; 24+ messages in thread
From: H. Nikolaus Schaller @ 2017-05-21 10:44 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Benoît Cousson, Tony Lindgren,
	Russell King, H. Nikolaus Schaller, Thierry Reding,
	Jonathan Cameron, Maxime Ripard, Jarkko Sakkinen
  Cc: devicetree, linux-kernel, linux-omap, letux-kernel, kernel

Since our proposed API was not acceptable and the new serdev API has arrived in 4.11 kernels,
we finally took the challenge to update the w2sg and w2cbw drivers to use the serdev API.

The approach is to write a "man in the middle" driver which is on one side a serdev client
which directly controls the UART where the device is connected to and on the other side
presents a new tty port so that user-space software can talk to the chips as if they would
directly talk to the UART of the SoC (e.g. ttyO1). This is similar to connecting to a remote
serial device e.g. through USB (ttyACM) or Bluetooth UART profiles.

For example gpsd or hciattach expect a /dev/tty they can control (flow control, baud rate
etc.).

Here is the result of our first hack which is working as a demo on GTA04 devices (and the
w2cbw driver can also be used to control a GTA04 variant with WL1837).

Since it is just a demo hack, the code is not yet cleaned up, nor does it completely pass
check-patch, nor follows 100% the coding styles. And certainly has some bugs.

The most significant issue is that calling tty_port_register_device() inside of the
serdev probe() function makes the serdev probe() function to be entered a second
time. This does not lead to big problems since we currently have minor = 0
and this makes the second call assume the device is not available.

But we have no idea why this happens and how it can be prevented.

Another observation is that the man-in-the-middle approach means copying
and double buffering the data. This seems to add some delay, especially if
we run the w2cbw003 bluetooth interface with its max. speed of 3 Mbit/s.

So this is still work in progress and more a demo than quite some work away
ffrom upstreaming. Therefore, we are asking for comments if the general
direction is ok and a polished driver would be acceptable. And if there are
solutions for the duplicate probe() and double-buffering delays.


H. Nikolaus Schaller (3):
  DTS: gta04: add serdev nodes for w2sg00x4, w2cbw etc.
  misc: Add w2sg0004 (gps receiver) power control driver
  misc: Add w2cbw003 (wifi/bluetooth) power control driver

 .../devicetree/bindings/misc/wi2wi,w2sg0004.txt    |  20 +
 .../devicetree/bindings/vendor-prefixes.txt        |   1 +
 arch/arm/boot/dts/omap3-gta04.dtsi                 |  18 +
 drivers/misc/Kconfig                               |  23 +
 drivers/misc/Makefile                              |   2 +
 drivers/misc/w2cbw003-bluetooth.c                  | 390 +++++++++++++
 drivers/misc/w2sg0004.c                            | 646 +++++++++++++++++++++
 include/linux/w2sg0004.h                           |  27 +
 8 files changed, 1127 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
 create mode 100644 drivers/misc/w2cbw003-bluetooth.c
 create mode 100644 drivers/misc/w2sg0004.c
 create mode 100644 include/linux/w2sg0004.h

-- 
2.12.2

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

* [RFC 1/3] DTS: gta04: add serdev nodes for w2sg00x4, w2cbw etc.
@ 2017-05-21 10:44   ` H. Nikolaus Schaller
  0 siblings, 0 replies; 24+ messages in thread
From: H. Nikolaus Schaller @ 2017-05-21 10:44 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Benoît Cousson, Tony Lindgren,
	Russell King, H. Nikolaus Schaller, Thierry Reding,
	Jonathan Cameron, Maxime Ripard, Jarkko Sakkinen
  Cc: devicetree, linux-kernel, linux-omap, letux-kernel, kernel

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 arch/arm/boot/dts/omap3-gta04.dtsi | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/arch/arm/boot/dts/omap3-gta04.dtsi b/arch/arm/boot/dts/omap3-gta04.dtsi
index 430b438e0ee9..45f0c109960f 100644
--- a/arch/arm/boot/dts/omap3-gta04.dtsi
+++ b/arch/arm/boot/dts/omap3-gta04.dtsi
@@ -472,16 +472,34 @@
 &uart1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&uart1_pins>;
+
+	bluetooth: w2cbw003 {
+		compatible = "wi2wi,w2cbw003-bluetooth";
+		vdd-supply = <&vaux4>;
+	};
 };
 
 &uart2 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&uart2_pins>;
+
+	gps: w2sg0004 {
+		compatible = "wi2wi,w2sg0004";
+		lna-supply = <&vsim>;   /* LNA regulator */
+		on-off-gpios = <&gpio5 17 GPIO_ACTIVE_HIGH>;    /* GPIO_145: trigger for turning on/off w2sg0004 */
+	};
 };
 
 &uart3 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&uart3_pins>;
+
+	rs232: trs3386 {
+		status = "disabled";
+		compatible = "ti,trs3386-mctrl";
+		gpios = <0>,		/* ... */
+			<&gpio1 21 0>;	/* DTR: GPIO_21 */
+	};
 };
 
 &charger {
-- 
2.12.2

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

* [RFC 1/3] DTS: gta04: add serdev nodes for w2sg00x4, w2cbw etc.
@ 2017-05-21 10:44   ` H. Nikolaus Schaller
  0 siblings, 0 replies; 24+ messages in thread
From: H. Nikolaus Schaller @ 2017-05-21 10:44 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Benoît Cousson, Tony Lindgren,
	Russell King, H. Nikolaus Schaller, Thierry Reding,
	Jonathan Cameron, Maxime Ripard, Jarkko Sakkinen
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	letux-kernel-S0jZdbWzriLCfDggNXIi3w,
	kernel-Jl6IXVxNIMRxAtABVqVhTwC/G2K4zDHf

Signed-off-by: H. Nikolaus Schaller <hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>
---
 arch/arm/boot/dts/omap3-gta04.dtsi | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/arch/arm/boot/dts/omap3-gta04.dtsi b/arch/arm/boot/dts/omap3-gta04.dtsi
index 430b438e0ee9..45f0c109960f 100644
--- a/arch/arm/boot/dts/omap3-gta04.dtsi
+++ b/arch/arm/boot/dts/omap3-gta04.dtsi
@@ -472,16 +472,34 @@
 &uart1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&uart1_pins>;
+
+	bluetooth: w2cbw003 {
+		compatible = "wi2wi,w2cbw003-bluetooth";
+		vdd-supply = <&vaux4>;
+	};
 };
 
 &uart2 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&uart2_pins>;
+
+	gps: w2sg0004 {
+		compatible = "wi2wi,w2sg0004";
+		lna-supply = <&vsim>;   /* LNA regulator */
+		on-off-gpios = <&gpio5 17 GPIO_ACTIVE_HIGH>;    /* GPIO_145: trigger for turning on/off w2sg0004 */
+	};
 };
 
 &uart3 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&uart3_pins>;
+
+	rs232: trs3386 {
+		status = "disabled";
+		compatible = "ti,trs3386-mctrl";
+		gpios = <0>,		/* ... */
+			<&gpio1 21 0>;	/* DTR: GPIO_21 */
+	};
 };
 
 &charger {
-- 
2.12.2

--
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 related	[flat|nested] 24+ messages in thread

* [RFC 2/3] misc: Add w2sg0004 (gps receiver) power control driver
  2017-05-21 10:44 [RFC 0/3] misc: new serdev based drivers for w2sg00x4 GPS module and w2cbw003 wifi/bluetooth H. Nikolaus Schaller
  2017-05-21 10:44   ` H. Nikolaus Schaller
@ 2017-05-21 10:44 ` H. Nikolaus Schaller
  2017-05-21 10:48     ` H. Nikolaus Schaller
                     ` (2 more replies)
  2017-05-21 10:44 ` [RFC 3/3] misc: Add w2cbw003 (wifi/bluetooth) " H. Nikolaus Schaller
  2017-05-23  2:26   ` Rob Herring
  3 siblings, 3 replies; 24+ messages in thread
From: H. Nikolaus Schaller @ 2017-05-21 10:44 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Benoît Cousson, Tony Lindgren,
	Russell King, H. Nikolaus Schaller, Thierry Reding,
	Jonathan Cameron, Maxime Ripard, Jarkko Sakkinen
  Cc: devicetree, linux-kernel, linux-omap, letux-kernel, kernel

Add driver for Wi2Wi W2SG0004/84 GPS module connected through uart.

Use serdev API hooks to monitor and forward the UART traffic to /dev/BTn
and turn on/off the module. It also detects if the module is turned on (sends data)
but should be off, e.g. if it was already turned on during boot or power-on-reset.

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 new serdev API introduced in 4.11.

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 .../devicetree/bindings/misc/wi2wi,w2sg0004.txt    |  20 +
 .../devicetree/bindings/vendor-prefixes.txt        |   1 +
 drivers/misc/Kconfig                               |  16 +
 drivers/misc/Makefile                              |   1 +
 drivers/misc/w2sg0004.c                            | 646 +++++++++++++++++++++
 include/linux/w2sg0004.h                           |  27 +
 6 files changed, 711 insertions(+)
 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 000000000000..b7125c7a598c
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
@@ -0,0 +1,20 @@
+Wi2Wi GPS module connected through UART
+
+Should be a subnode of the SoC UART it is connected to (serdev).
+
+Required properties:
+- compatible:	wi2wi,w2sg0004 or wi2wi,w2sg0084
+- on-off-gpio:	the GPIO that controls the module's on-off toggle input
+
+Optional properties:
+- lna-suppy:	an (optional) LNA regulator that is enabled together with the GPS receiver
+
+Example:
+
+&uart2 {
+	gps: w2sg0004 {
+		compatible = "wi2wi,w2sg0004";
+		lna-supply = <&vsim>;   /* LNA regulator */
+		on-off-gpios = <&gpio5 17 GPIO_ACTIVE_HIGH>;    /* GPIO_145: trigger for turning on/off w2sg0004 */
+        };
+};
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index c03d20140366..c56b3181b266 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -345,6 +345,7 @@ voipac	Voipac Technologies s.r.o.
 wd	Western Digital Corp.
 wetek	WeTek Electronics, limited.
 wexler	Wexler
+wi2wi	Wi2Wi, Inc.
 winbond Winbond Electronics corp.
 winstar	Winstar Display Corp.
 wlf	Wolfson Microelectronics
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 60f876b03586..7f97ef8fb6cd 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -509,4 +509,20 @@ source "drivers/misc/mic/Kconfig"
 source "drivers/misc/genwqe/Kconfig"
 source "drivers/misc/echo/Kconfig"
 source "drivers/misc/cxl/Kconfig"
+
+config W2SG0004
+	tristate "W2SG00x4 on/off control"
+	depends on GPIOLIB && SERIAL_DEV_BUS
+	help
+          Enable on/off control of W2SG00x4 GPS moduled connected
+	  to some SoC UART to allow powering up/down if the /dev/ttyGPSn
+	  is opened/closed.
+	  It also provides a rfkill gps name 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
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 81ef3e67acc9..0e88e06e5ee0 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -50,6 +50,7 @@ obj-$(CONFIG_SRAM_EXEC)		+= sram-exec.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/
 obj-$(CONFIG_ASPEED_LPC_CTRL)	+= aspeed-lpc-ctrl.o
diff --git a/drivers/misc/w2sg0004.c b/drivers/misc/w2sg0004.c
new file mode 100644
index 000000000000..1b335317c8ac
--- /dev/null
+++ b/drivers/misc/w2sg0004.c
@@ -0,0 +1,646 @@
+/*
+ * 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/delay.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/of_gpio.h>
+#include <linux/platform_device.h>
+#include <linux/rfkill.h>
+#include <linux/serdev.h>
+#include <linux/sched.h>
+#include <linux/slab.h>
+#include <linux/tty.h>
+#include <linux/tty_flip.h>
+#include <linux/w2sg0004.h>
+#include <linux/workqueue.h>
+
+#ifdef CONFIG_W2SG0004_DEBUG	// not for upstreaming
+#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		serdev_device *uart;	/* the uart connected to the chip */
+	struct		tty_driver *tty_drv;	/* this is the user space tty */
+	struct		device *dev;	/* returned by tty_port_register_device() */
+	struct		tty_port port;
+	int		open_count;	/* how often we were opened */
+	enum		w2sg_state state;
+	int		requested;	/* requested state (0/1) */
+	int		suspended;
+	spinlock_t	lock;
+	struct delayed_work work;
+	int		discard_count;
+};
+
+static struct w2sg_data *w2sg_by_minor[1];
+
+static int w2sg_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_set_power(void *pdata, int val)
+{
+	struct w2sg_data *data = (struct w2sg_data *) pdata;
+
+	pr_debug("%s to state=%d (requested=%d)\n", __func__, val, data->requested);
+
+	if (val && !data->requested) {
+		data->requested = true;
+	} else if (!val && data->requested) {
+		data->backoff = HZ;
+		data->requested = false;
+	} else
+		return;
+
+	pr_debug("w2sg00x4 scheduled for %d\n", data->requested);
+
+	if (!data->suspended)
+		schedule_delayed_work(&data->work, 0);
+}
+
+/* called each time data is received by the UART (i.e. sent by the w2sg0004) */
+
+static int w2sg_uart_receive_buf(struct serdev_device *serdev, const unsigned char *rxdata,
+				size_t count)
+{
+	struct w2sg_data *data = (struct w2sg_data *) serdev_device_get_drvdata(serdev);
+//	unsigned long flags;
+
+//	pr_debug("w2sg: %d characters\n", count);
+
+	if (!data->requested && !data->is_on) {
+		/* we have received characters while the w2sg should have been be turned off */
+		data->discard_count += count;
+		if ((data->state == W2SG_IDLE) &&
+		    time_after(jiffies,
+		    data->last_toggle + data->backoff)) {
+			/* Should be off by now, time to toggle again */
+			pr_debug("w2sg00x4 has sent %d characters data although it should be off!\n", data->discard_count);
+			data->discard_count = 0;
+
+			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);
+		}
+	} else if (data->open_count > 0) {
+		int n;
+
+//		pr_debug("w2sg00x4: push %d chars to tty port\n", count);
+		n = tty_insert_flip_string(&data->port, rxdata, count);	/* pass to user-space */
+		if (n != count)
+			pr_debug("w2sg00x4: did loose %d characters\n", count - n);
+		tty_flip_buffer_push(&data->port);
+		return n;
+	}
+
+	/* assume we have processed everything */
+	return count;
+}
+
+/* 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_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_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_set_lna_power(data);
+}
+
+static struct rfkill_ops w2sg0004_rfkill_ops = {
+	.set_block = w2sg_rfkill_set_block,
+};
+
+static struct serdev_device_ops serdev_ops = {
+	.receive_buf = w2sg_uart_receive_buf,
+#if 0
+	.write_wakeup = w2sg_uart_wakeup,
+#endif
+};
+
+/*
+ * we are a man-in the middle between the user-space visible tty port
+ * and the serdev tty where the chip is connected.
+ * This allows us to recognise when the device should be powered on
+ * or off and handle the "false" state that data arrives while no
+ * users-space tty client exists.
+ */
+
+static struct w2sg_data *w2sg_get_by_minor(unsigned int minor)
+{
+	return w2sg_by_minor[minor];
+}
+
+static int w2sg_tty_install(struct tty_driver *driver, struct tty_struct *tty)
+{
+	struct w2sg_data *data;
+	int retval;
+
+	pr_debug("%s() tty = %p\n", __func__, tty);
+
+	data = w2sg_get_by_minor(tty->index);
+	pr_debug("%s() data = %p\n", __func__, data);
+
+	if (!data)
+		return -ENODEV;
+
+	retval = tty_standard_install(driver, tty);
+	if (retval)
+		goto error_init_termios;
+
+	tty->driver_data = data;
+
+	return 0;
+
+error_init_termios:
+	tty_port_put(&data->port);
+	return retval;
+}
+
+static int w2sg_tty_open(struct tty_struct *tty, struct file *file)
+{
+	struct w2sg_data *data = tty->driver_data;
+
+	pr_debug("%s() data = %p open_count = ++%d\n", __func__, data, data->open_count);
+//	val = (val & TIOCM_DTR) != 0;	/* DTR controls power on/off */
+
+	w2sg_set_power(data, ++data->open_count > 0);
+
+// we could/should return -Esomething if already open...
+
+	return tty_port_open(&data->port, tty, file);
+}
+
+static void w2sg_tty_close(struct tty_struct *tty, struct file *file)
+{
+	struct w2sg_data *data = tty->driver_data;
+
+	pr_debug("%s()\n", __func__);
+//	val = (val & TIOCM_DTR) != 0;	/* DTR controls power on/off */
+	w2sg_set_power(data, --data->open_count > 0);
+
+	tty_port_close(&data->port, tty, file);
+}
+
+static int w2sg_tty_write(struct tty_struct *tty,
+		const unsigned char *buffer, int count)
+{
+	struct w2sg_data *data = tty->driver_data;
+
+	/* simply pass down to UART */
+	return serdev_device_write_buf(data->uart, buffer, count);
+}
+
+#if 0
+static void w2sg_tty_tiocmget(...)
+{
+	int val;
+
+	pr_debug("%s(...,%x)\n", __func__, val);
+	val = (val & TIOCM_DTR) != 0;	/* DTR controls power on/off */
+	w2sg_set_power((struct w2sg_data *) pdata, val);
+}
+#endif
+
+
+static const struct tty_operations w2sg_serial_ops = {
+	.install = w2sg_tty_install,
+	.open = w2sg_tty_open,
+	.close = w2sg_tty_close,
+	.write = w2sg_tty_write,
+#if 0
+	.write_room = w2sg_tty_write_room,
+	.cleanup = w2sg_tty_cleanup,
+	.ioctl = w2sg_tty_ioctl,
+	.set_termios = w2sg_tty_set_termios,
+	.chars_in_buffer = w2sg_tty_chars_in_buffer,
+	.tiocmget = w2sg_tty_tiocmget,
+	.tiocmset = w2sg_tty_tiocmset,
+	.get_icount = w2sg_tty_get_count,
+	.unthrottle = w2sg_tty_unthrottle
+#endif
+};
+
+static const struct tty_port_operations w2sg_port_ops = {
+};
+
+static int w2sg_probe(struct serdev_device *serdev)
+{
+	struct w2sg_pdata *pdata = NULL;
+	struct w2sg_data *data;
+	struct rfkill *rf_kill;
+	int err;
+	int minor;
+
+	pr_debug("%s()\n", __func__);
+
+	minor = 0;
+	if (w2sg_by_minor[minor]) {
+		pr_err("w2sg minor is already in use!\n");
+		return -ENODEV;
+	}
+
+// can be simplified if we require OF
+
+	if (serdev->dev.of_node) {
+		struct device *dev = &serdev->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-gpios", 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");
+// shouldn't we defer probing as well???
+
+		pr_debug("%s() lna_regulator = %p\n", __func__, pdata->lna_regulator);
+
+		serdev->dev.platform_data = pdata;
+	}
+
+	data = devm_kzalloc(&serdev->dev, sizeof(*data), GFP_KERNEL);
+	if (data == NULL)
+		return -ENOMEM;
+
+	w2sg_by_minor[minor] = data;
+
+#if 1
+	pr_debug("w2sg serdev_device_set_drvdata\n");
+#endif
+	serdev_device_set_drvdata(serdev, data);
+
+	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;
+
+	data->uart = serdev;
+
+	INIT_DELAYED_WORK(&data->work, toggle_work);
+//	spin_lock_init(&data->lock);
+
+#if 1
+	pr_debug("w2sg devm_gpio_request\n");
+#endif
+	err = devm_gpio_request(&serdev->dev, data->on_off_gpio, "w2sg0004-on-off");
+	if (err < 0)
+		goto out;
+
+	gpio_direction_output(data->on_off_gpio, false);
+
+	serdev_device_set_client_ops(data->uart, &serdev_ops);
+	serdev_device_open(data->uart);
+
+	serdev_device_set_baudrate(data->uart, 9600);
+	serdev_device_set_flow_control(data->uart, false);
+
+#if 1
+	pr_debug("w2sg rfkill_alloc\n");
+#endif
+	rf_kill = rfkill_alloc("GPS", &serdev->dev, RFKILL_TYPE_GPS,
+				&w2sg0004_rfkill_ops, data);
+	if (rf_kill == NULL) {
+		err = -ENOMEM;
+		goto err_rfkill;
+	}
+
+#if 1
+	pr_debug("w2sg register rfkill\n");
+#endif
+	err = rfkill_register(rf_kill);
+	if (err) {
+		dev_err(&serdev->dev, "Cannot register rfkill device\n");
+		goto err_rfkill;
+	}
+
+	data->rf_kill = rf_kill;
+
+#if 1
+	pr_debug("w2sg alloc_tty_driver\n");
+#endif
+	/* allocate the tty driver */
+	data->tty_drv = alloc_tty_driver(1);
+	if (!data->tty_drv)
+		return -ENOMEM;
+
+	/* initialize the tty driver */
+	data->tty_drv->owner = THIS_MODULE;
+	data->tty_drv->driver_name = "w2sg0004";
+	data->tty_drv->name = "ttyGPS";
+	data->tty_drv->major = 0;
+	data->tty_drv->minor_start = 0;
+	data->tty_drv->type = TTY_DRIVER_TYPE_SERIAL;
+	data->tty_drv->subtype = SERIAL_TYPE_NORMAL;
+	data->tty_drv->flags = TTY_DRIVER_REAL_RAW | TTY_DRIVER_DYNAMIC_DEV;
+	data->tty_drv->init_termios = tty_std_termios;
+	data->tty_drv->init_termios.c_cflag = B9600 | CS8 | CREAD | HUPCL | CLOCAL;
+	/*
+	 * tty_termios_encode_baud_rate(&data->tty_drv->init_termios, 115200, 115200);
+	 * w2sg_tty_termios(&data->tty_drv->init_termios);
+	 */
+	tty_set_operations(data->tty_drv, &w2sg_serial_ops);
+
+#if 1
+	pr_debug("w2sg tty_register_driver\n");
+#endif
+	/* register the tty driver */
+	err = tty_register_driver(data->tty_drv);
+	if (err) {
+		pr_err("%s - tty_register_driver failed(%d)\n",
+			__func__, err);
+		put_tty_driver(data->tty_drv);
+		goto err_rfkill;
+	}
+
+#if 1
+	pr_debug("w2sg call tty_port_init\n");
+#endif
+	tty_port_init(&data->port);
+	data->port.ops = &w2sg_port_ops;
+
+#if 1
+	pr_debug("w2sg call tty_port_register_device\n");
+#endif
+/*
+ * FIXME: this appears to reenter this probe() function a second time
+ * which only fails because the gpio is already assigned
+ */
+
+	data->dev = tty_port_register_device(&data->port,
+			data->tty_drv, minor, &serdev->dev);
+
+#if 1
+	pr_debug("w2sg tty_port_register_device -> %p\n", data->dev);
+	pr_debug("w2sg port.tty = %p\n", data->port.tty);
+#endif
+//	data->port.tty->driver_data = data;	/* make us known in tty_struct */
+
+	pr_debug("w2sg probed\n");
+
+#ifdef CONFIG_W2SG0004_DEBUG
+	pr_debug("w2sg DEBUGGING MODE enabled\n");
+	/* 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
+
+	/* keep off until user space requests the device */
+	w2sg_set_power(data, false);
+
+#if 0	// more debugging - not for upstreaming
+	w2sg_set_power(data, true);
+#endif
+
+	return 0;
+
+err_rfkill:
+	rfkill_destroy(rf_kill);
+	serdev_device_close(data->uart);
+out:
+#if 0
+	if (err == -EBUSY)
+		err = -EPROBE_DEFER;
+#endif
+#if 1
+	pr_debug("w2sg error %d\n", err);
+#endif
+	return err;
+}
+
+static void w2sg_remove(struct serdev_device *serdev)
+{
+	struct w2sg_data *data = serdev_device_get_drvdata(serdev);
+	int minor;
+
+	cancel_delayed_work_sync(&data->work);
+
+	/* what is the right sequence to avoid problems? */
+	serdev_device_close(data->uart);
+
+	// get minor from searching for data == w2sg_by_minor[minor]
+	minor = 0;
+	tty_unregister_device(data->tty_drv, minor);
+
+	tty_unregister_driver(data->tty_drv);
+}
+
+static int w2sg_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_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_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_suspend, w2sg_resume);
+
+static struct serdev_device_driver w2sg_driver = {
+	.probe		= w2sg_probe,
+	.remove		= w2sg_remove,
+	.driver = {
+		.name	= "w2sg0004",
+		.owner	= THIS_MODULE,
+		.pm	= &w2sg_pm_ops,
+		.of_match_table = of_match_ptr(w2sg0004_of_match)
+	},
+};
+
+module_serdev_device_driver(w2sg_driver);
+
+MODULE_ALIAS("w2sg0004");
+
+MODULE_AUTHOR("NeilBrown <neilb@suse.de>");
+MODULE_DESCRIPTION("w2sg0004 GPS power management driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/w2sg0004.h b/include/linux/w2sg0004.h
new file mode 100644
index 000000000000..ad0c4a18e01d
--- /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.12.2

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

* [RFC 3/3] misc: Add w2cbw003 (wifi/bluetooth) power control driver
  2017-05-21 10:44 [RFC 0/3] misc: new serdev based drivers for w2sg00x4 GPS module and w2cbw003 wifi/bluetooth H. Nikolaus Schaller
  2017-05-21 10:44   ` H. Nikolaus Schaller
  2017-05-21 10:44 ` [RFC 2/3] misc: Add w2sg0004 (gps receiver) power control driver H. Nikolaus Schaller
@ 2017-05-21 10:44 ` H. Nikolaus Schaller
  2017-05-23  2:26   ` Rob Herring
  3 siblings, 0 replies; 24+ messages in thread
From: H. Nikolaus Schaller @ 2017-05-21 10:44 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Benoît Cousson, Tony Lindgren,
	Russell King, H. Nikolaus Schaller, Thierry Reding,
	Jonathan Cameron, Maxime Ripard, Jarkko Sakkinen
  Cc: devicetree, linux-kernel, linux-omap, letux-kernel, kernel

Add driver for Wi2Wi W2CBW003 WiFi and Bluetooth module
where the Bluetooth interface is connected through uart.

Uses the new serdev API to glue with tty and turn on/off the
module if the tty port (/dev/ttyBTn) is opened.

Note that this is only for the Bluetooth side. The WLAN
(libertas) sdio driver should be abe to enable the same vdd
regulator. So that the device is powered on if either WiFi or
Bluetooth is activated (or both) and powered down if neither
is in use.

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 drivers/misc/Kconfig              |   7 +
 drivers/misc/Makefile             |   1 +
 drivers/misc/w2cbw003-bluetooth.c | 390 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 398 insertions(+)
 create mode 100644 drivers/misc/w2cbw003-bluetooth.c

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 7f97ef8fb6cd..90ce23bef77b 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -525,4 +525,11 @@ config W2SG0004_DEBUG
 	help
 	  Enable driver debugging mode of W2SG0004 GPS.
 
+config W2CBW003
+	tristate "W2CBW003 power on/off control"
+	depends on GPIOLIB
+	help
+	  Enable on/off power control of W2CBW003 if the /dev/tty$n for
+	  Bluetooth is opened.
+
 endmenu
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 0e88e06e5ee0..f6d3f096c5e0 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -51,6 +51,7 @@ obj-y				+= mic/
 obj-$(CONFIG_GENWQE)		+= genwqe/
 obj-$(CONFIG_ECHO)		+= echo/
 obj-$(CONFIG_W2SG0004)		+= w2sg0004.o
+obj-$(CONFIG_W2CBW003)		+= w2cbw003-bluetooth.o
 obj-$(CONFIG_VEXPRESS_SYSCFG)	+= vexpress-syscfg.o
 obj-$(CONFIG_CXL_BASE)		+= cxl/
 obj-$(CONFIG_ASPEED_LPC_CTRL)	+= aspeed-lpc-ctrl.o
diff --git a/drivers/misc/w2cbw003-bluetooth.c b/drivers/misc/w2cbw003-bluetooth.c
new file mode 100644
index 000000000000..3bcfd51bbf09
--- /dev/null
+++ b/drivers/misc/w2cbw003-bluetooth.c
@@ -0,0 +1,390 @@
+/*
+ * w2scbw003.c
+ * Driver for power controlling the w2cbw003 WiFi/Bluetooth chip.
+ *
+ * powers on the chip if the tty port associated/connected
+ * to the bluetooth subsystem is opened (e.g. hciattach /dev/ttyBT0)
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/err.h>
+#include <linux/of.h>
+#include <linux/regulator/consumer.h>
+#include <linux/serdev.h>
+#include <linux/tty.h>
+#include <linux/tty_flip.h>
+
+#undef pr_debug
+#define pr_debug printk
+
+struct w2cbw_data {
+	struct		regulator *vdd_regulator;
+	struct		serdev_device *uart;	/* the uart connected to the chip */
+	struct		tty_driver *tty_drv;	/* this is the user space tty */
+	struct		device *dev;	/* returned by tty_port_register_device() */
+	struct		tty_port port;
+	int		open_count;	/* how often we were opened */
+};
+
+static struct w2cbw_data *w2cbw_by_minor[1];
+
+static void w2cbw_set_power(void *pdata, int val)
+{
+	struct w2cbw_data *data = (struct w2cbw_data *) pdata;
+
+	pr_debug("%s(...,%x)\n", __func__, val);
+
+	if (val != 0)
+		WARN_ON(regulator_enable(data->vdd_regulator));
+	else
+		regulator_disable(data->vdd_regulator);
+}
+
+/* called each time data is received by the UART (i.e. sent by the w2cbw003) */
+
+static int w2cbw_uart_receive_buf(struct serdev_device *serdev, const unsigned char *rxdata,
+				size_t count)
+{
+	struct w2cbw_data *data = (struct w2cbw_data *) serdev_device_get_drvdata(serdev);
+
+//	pr_debug("%s() characters\n", __func__, count);
+
+	if (data->open_count > 0) {
+		int n;
+
+		pr_debug("w2cbw003: uart->tty %d chars\n", count);
+		n = tty_insert_flip_string(&data->port, rxdata, count);	/* pass to user-space */
+		if (n != count)
+			pr_debug("w2cbw003: did loose %d characters\n", count - n);
+		tty_flip_buffer_push(&data->port);
+		return n;
+	}
+
+	/* nobody is listenig - ignore incoming data */
+	return count;
+}
+
+static struct serdev_device_ops serdev_ops = {
+	.receive_buf = w2cbw_uart_receive_buf,
+#if 0
+	.write_wakeup = w2cbw_uart_wakeup,
+#endif
+};
+
+static struct w2cbw_data *w2cbw_get_by_minor(unsigned int minor)
+{
+	return w2cbw_by_minor[minor];
+}
+
+static int w2cbw_tty_install(struct tty_driver *driver, struct tty_struct *tty)
+{
+	struct w2cbw_data *data;
+	int retval;
+
+	pr_debug("%s() tty = %p\n", __func__, tty);
+
+	data = w2cbw_get_by_minor(tty->index);
+	pr_debug("%s() data = %p\n", __func__, data);
+
+	if (!data)
+		return -ENODEV;
+
+	retval = tty_standard_install(driver, tty);
+	if (retval)
+		goto error_init_termios;
+
+	tty->driver_data = data;
+
+	return 0;
+
+error_init_termios:
+	tty_port_put(&data->port);
+	return retval;
+}
+
+static int w2cbw_tty_open(struct tty_struct *tty, struct file *file)
+{
+	struct w2cbw_data *data = tty->driver_data;
+
+	pr_debug("%s() data = %p open_count = ++%d\n", __func__, data, data->open_count);
+//	val = (val & TIOCM_DTR) != 0;	/* DTR controls power on/off */
+
+	w2cbw_set_power(data, ++data->open_count > 0);
+
+// we could/should return -Esomething if already open...
+
+	return tty_port_open(&data->port, tty, file);
+}
+
+static void w2cbw_tty_close(struct tty_struct *tty, struct file *file)
+{
+	struct w2cbw_data *data = tty->driver_data;
+
+	pr_debug("%s()\n", __func__);
+//	val = (val & TIOCM_DTR) != 0;	/* DTR controls power on/off */
+	w2cbw_set_power(data, --data->open_count > 0);
+
+	tty_port_close(&data->port, tty, file);
+}
+
+static int w2cbw_tty_write(struct tty_struct *tty,
+		const unsigned char *buffer, int count)
+{
+	struct w2cbw_data *data = tty->driver_data;
+
+	pr_debug("w2cbw003: tty->uart %d chars\n", count);
+
+	return serdev_device_write_buf(data->uart, buffer, count);	/* simply pass down to UART */
+}
+
+static void w2cbw_tty_set_termios(struct tty_struct *tty,
+		struct ktermios *old)
+{ /* copy from tty settings */
+	struct w2cbw_data *data = tty->driver_data;
+
+pr_debug("%s() flow = %d\n", __func__, tty_port_cts_enabled(tty->port));
+pr_debug("%s() baud rate = %d\n", __func__, tty_termios_baud_rate(&tty->termios));
+
+	serdev_device_set_flow_control(data->uart, tty_port_cts_enabled(tty->port));
+	serdev_device_set_baudrate(data->uart, tty_termios_baud_rate(&tty->termios));
+/* this may trigger
+[  295.607781]
+[  295.609349] =============================================
+[  295.615024] [ INFO: possible recursive locking detected ]
+[  295.620707] 4.11.0-rc8-letux+ #1026 Tainted: G        W
+[  295.626745] ---------------------------------------------
+[  295.632421] hciattach/2633 is trying to acquire lock:
+[  295.637737]  (&tty->termios_rwsem){++++.+}, at: [<c04bb400>] tty_set_termios+0x40/0x1b0
+[  295.646190]
+[  295.646190] but task is already holding lock:
+[  295.652329]  (&tty->termios_rwsem){++++.+}, at: [<c04bb400>] tty_set_termios+0x40/0x1b0
+[  295.660780]
+[  295.660780] other info that might help us debug this:
+[  295.667646]  Possible unsafe locking scenario:
+[  295.667646]
+[  295.673873]        CPU0
+[  295.676444]        ----
+[  295.679017]   lock(&tty->termios_rwsem);
+[  295.683151]   lock(&tty->termios_rwsem);
+[  295.687288]
+[  295.687288]  *** DEADLOCK ***
+[  295.687288]
+[  295.693517]  May be due to missing lock nesting notation
+[  295.693517]
+[  295.700666] 2 locks held by hciattach/2633:
+[  295.705063]  #0:  (&tty->ldisc_sem){++++++}, at: [<c04bc790>] tty_ldisc_ref_wait+0x18/0x34
+[  295.713789]  #1:  (&tty->termios_rwsem){++++.+}, at: [<c04bb400>] tty_set_termios+0x40/0x1b0
+[  295.722691]
+[  295.722691] stack backtrace:
+[  295.727285] CPU: 1 PID: 2633 Comm: hciattach Tainted: G        W       4.11.0-rc8-letux+ #1026
+[  295.736342] Hardware name: Generic OMAP5 (Flattened Device Tree)
+[  295.742679] [<c010f378>] (unwind_backtrace) from [<c010b8fc>] (show_stack+0x10/0x14)
+[  295.750841] [<c010b8fc>] (show_stack) from [<c043e4a0>] (dump_stack+0x98/0xd0)
+[  295.758450] [<c043e4a0>] (dump_stack) from [<c0186ff0>] (validate_chain+0x760/0x1010)
+[  295.766694] [<c0186ff0>] (validate_chain) from [<c0188e58>] (__lock_acquire+0x690/0x760)
+[  295.775214] [<c0188e58>] (__lock_acquire) from [<c0189428>] (lock_acquire+0x1d0/0x29c)
+[  295.783558] [<c0189428>] (lock_acquire) from [<c0754a88>] (down_write+0x28/0x78)
+[  295.791354] [<c0754a88>] (down_write) from [<c04bb400>] (tty_set_termios+0x40/0x1b0)
+[  295.799508] [<c04bb400>] (tty_set_termios) from [<c04d50dc>] (ttyport_set_flow_control+0x58/0x60)
+[  295.808856] [<c04d50dc>] (ttyport_set_flow_control) from [<bf0a4080>] (w2cbw_tty_set_termios+0x58/0x80 [w2cbw003_bluetooth])
+[  295.820675] [<bf0a4080>] (w2cbw_tty_set_termios [w2cbw003_bluetooth]) from [<c04bb514>] (tty_set_termios+0x154/0x1b0)
+[  295.831859] [<c04bb514>] (tty_set_termios) from [<c04bbd18>] (set_termios.part.1+0x3b4/0x42c)
+[  295.840841] [<c04bbd18>] (set_termios.part.1) from [<c04bbff4>] (tty_mode_ioctl+0x1e8/0x598)
+[  295.849721] [<c04bbff4>] (tty_mode_ioctl) from [<c04b7080>] (tty_ioctl+0xe84/0xedc)
+[  295.857784] [<c04b7080>] (tty_ioctl) from [<c0282e80>] (vfs_ioctl+0x20/0x34)
+[  295.865215] [<c0282e80>] (vfs_ioctl) from [<c0283844>] (do_vfs_ioctl+0x82c/0x93c)
+[  295.873098] [<c0283844>] (do_vfs_ioctl) from [<c02839a0>] (SyS_ioctl+0x4c/0x74)
+[  295.880801] [<c02839a0>] (SyS_ioctl) from [<c0107040>] (ret_fast_syscall+0x0/0x1c)
+*/
+}
+
+static const struct tty_operations w2cbw_serial_ops = {
+	.install = w2cbw_tty_install,
+	.open = w2cbw_tty_open,
+	.close = w2cbw_tty_close,
+	.write = w2cbw_tty_write,
+	.set_termios = w2cbw_tty_set_termios,
+#if 0
+	/* sometimes: [  300.314968] ttyBT ttyBT0: missing write_room method */
+	.write_room = w2cbw_tty_write_room,
+	.cleanup = w2cbw_tty_cleanup,
+	.ioctl = w2cbw_tty_ioctl,
+	.chars_in_buffer = w2cbw_tty_chars_in_buffer,
+	.tiocmget = w2cbw_tty_tiocmget,
+	.tiocmset = w2cbw_tty_tiocmset,
+	.get_icount = w2cbw_tty_get_count,
+	.unthrottle = w2cbw_tty_unthrottle
+#endif
+};
+
+static const struct tty_port_operations w2cbw_port_ops = {
+};
+
+static int w2cbw_probe(struct serdev_device *serdev)
+{
+	struct device *dev = &serdev->dev;
+	struct w2cbw_data *data;
+	int err;
+	int minor;
+
+	pr_debug("%s()\n", __func__);
+
+	minor = 0;
+	if (w2cbw_by_minor[minor]) {
+		pr_err("w2cbw minor is already in use!\n");
+		return -ENODEV;
+	}
+
+	if (!serdev->dev.of_node) {
+		dev_err(&serdev->dev, "No device tree data\n");
+		return -EINVAL;
+	}
+
+	data = devm_kzalloc(&serdev->dev, sizeof(*data), GFP_KERNEL);
+	if (data == NULL)
+		return -ENOMEM;
+
+	data->vdd_regulator = devm_regulator_get_optional(dev, "vdd");
+
+	if (IS_ERR(data->vdd_regulator)) {
+		if (PTR_ERR(data->vdd_regulator) == -EPROBE_DEFER)
+			return -EPROBE_DEFER;	/* we can't probe yet */
+		data->uart = NULL;	/* no regulator yet */
+	}
+
+	pr_debug("%s() vdd_regulator = %p\n", __func__, data->vdd_regulator);
+
+	w2cbw_by_minor[minor] = data;
+
+	serdev_device_set_drvdata(serdev, data);
+
+	pr_debug("w2cbw003 probed\n");
+
+	data->uart = serdev;
+
+	serdev_device_set_client_ops(data->uart, &serdev_ops);
+	serdev_device_open(data->uart);
+
+	serdev_device_set_baudrate(data->uart, 9600);
+	serdev_device_set_flow_control(data->uart, false);
+#if 1
+	pr_debug("w2cbw alloc_tty_driver\n");
+#endif
+	/* allocate the tty driver */
+	data->tty_drv = alloc_tty_driver(1);
+	if (!data->tty_drv)
+		return -ENOMEM;
+
+	/* initialize the tty driver */
+	data->tty_drv->owner = THIS_MODULE;
+	data->tty_drv->driver_name = "w2cbw003";
+	data->tty_drv->name = "ttyBT";
+	data->tty_drv->major = 0;
+	data->tty_drv->minor_start = 0;
+	data->tty_drv->type = TTY_DRIVER_TYPE_SERIAL;
+	data->tty_drv->subtype = SERIAL_TYPE_NORMAL;
+	data->tty_drv->flags = TTY_DRIVER_REAL_RAW | TTY_DRIVER_DYNAMIC_DEV;
+	data->tty_drv->init_termios = tty_std_termios;
+	data->tty_drv->init_termios.c_cflag = B9600 | CS8 | CREAD | HUPCL | CLOCAL;
+	/*
+	 * tty_termios_encode_baud_rate(&data->tty_drv->init_termios, 115200, 115200);
+	 * w2cbw_tty_termios(&data->tty_drv->init_termios);
+	 */
+	tty_set_operations(data->tty_drv, &w2cbw_serial_ops);
+
+#if 1
+	pr_debug("w2cbw tty_register_driver\n");
+#endif
+	/* register the tty driver */
+	err = tty_register_driver(data->tty_drv);
+	if (err) {
+		pr_err("%s - tty_register_driver failed(%d)\n",
+			__func__, err);
+		put_tty_driver(data->tty_drv);
+		goto err;
+	}
+
+#if 1
+	pr_debug("w2cbw call tty_port_init\n");
+#endif
+	tty_port_init(&data->port);
+	data->port.ops = &w2cbw_port_ops;
+
+#if 1
+	pr_debug("w2cbw call tty_port_register_device\n");
+#endif
+/*
+ * FIXME: this appears to reenter this probe() function a second time
+ * which only fails because the minor number is already assigned
+ */
+
+	data->dev = tty_port_register_device(&data->port,
+			data->tty_drv, minor, &serdev->dev);
+
+#if 1
+	pr_debug("w2cbw tty_port_register_device -> %p\n", data->dev);
+	pr_debug("w2cbw port.tty = %p\n", data->port.tty);
+#endif
+//	data->port.tty->driver_data = data;	/* make us known in tty_struct */
+
+	/* keep off until user space requests the device */
+	if (regulator_is_enabled(data->vdd_regulator))
+		w2cbw_set_power(data, false);
+
+	pr_debug("w2cbw probed\n");
+
+	return 0;
+
+err:
+	serdev_device_close(data->uart);
+#if 0
+	if (err == -EBUSY)
+		err = -EPROBE_DEFER;
+#endif
+#if 1
+	pr_debug("w2cbw error %d\n", err);
+#endif
+	return err;
+}
+
+static void w2cbw_remove(struct serdev_device *serdev)
+{
+	struct w2cbw_data *data = serdev_device_get_drvdata(serdev);
+	int minor;
+
+	/* what is the right sequence to avoid problems? */
+	serdev_device_close(data->uart);
+
+	// should get minor from searching for data == w2cbw_by_minor[minor]
+	minor = 0;
+	tty_unregister_device(data->tty_drv, minor);
+
+	tty_unregister_driver(data->tty_drv);
+}
+
+// suspend/resume? handle by regulator
+
+static const struct of_device_id w2cbw_of_match[] = {
+	{ .compatible = "wi2wi,w2cbw003-bluetooth" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, w2cbw_of_match);
+
+static struct serdev_device_driver w2cbw_driver = {
+	.probe		= w2cbw_probe,
+	.remove		= w2cbw_remove,
+	.driver = {
+		.name	= "w2cbw003",
+		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(w2cbw_of_match)
+	},
+};
+
+module_serdev_device_driver(w2cbw_driver);
+
+MODULE_ALIAS("w2cbw003-bluetooth");
+
+MODULE_AUTHOR("H. Nikolaus Schaller <hns@goldelico.com>");
+MODULE_DESCRIPTION("w2cbw003 power management driver");
+MODULE_LICENSE("GPL v2");
-- 
2.12.2

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

* Re: [RFC 2/3] misc: Add w2sg0004 (gps receiver) power control driver
@ 2017-05-21 10:48     ` H. Nikolaus Schaller
  0 siblings, 0 replies; 24+ messages in thread
From: H. Nikolaus Schaller @ 2017-05-21 10:48 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Benoît Cousson, Tony Lindgren,
	Russell King, Nikolaus Schaller, Thierry Reding,
	Jonathan Cameron, Maxime Ripard, Jarkko Sakkinen
  Cc: devicetree, linux-kernel, linux-omap, letux-kernel, kernel


> Am 21.05.2017 um 12:44 schrieb H. Nikolaus Schaller <hns@goldelico.com>:
> 
> Add driver for Wi2Wi W2SG0004/84 GPS module connected through uart.
> 
> Use serdev API hooks to monitor and forward the UART traffic to /dev/BTn

s/BTn/GPSn/

> and turn on/off the module. It also detects if the module is turned on (sends data)
> but should be off, e.g. if it was already turned on during boot or power-on-reset.
> 
> 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 new serdev API introduced in 4.11.
> 
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> ---
> .../devicetree/bindings/misc/wi2wi,w2sg0004.txt    |  20 +
> .../devicetree/bindings/vendor-prefixes.txt        |   1 +
> drivers/misc/Kconfig                               |  16 +
> drivers/misc/Makefile                              |   1 +
> drivers/misc/w2sg0004.c                            | 646 +++++++++++++++++++++
> include/linux/w2sg0004.h                           |  27 +
> 6 files changed, 711 insertions(+)
> 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 000000000000..b7125c7a598c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
> @@ -0,0 +1,20 @@
> +Wi2Wi GPS module connected through UART
> +
> +Should be a subnode of the SoC UART it is connected to (serdev).
> +
> +Required properties:
> +- compatible:	wi2wi,w2sg0004 or wi2wi,w2sg0084
> +- on-off-gpio:	the GPIO that controls the module's on-off toggle input
> +
> +Optional properties:
> +- lna-suppy:	an (optional) LNA regulator that is enabled together with the GPS receiver
> +
> +Example:
> +
> +&uart2 {
> +	gps: w2sg0004 {
> +		compatible = "wi2wi,w2sg0004";
> +		lna-supply = <&vsim>;   /* LNA regulator */
> +		on-off-gpios = <&gpio5 17 GPIO_ACTIVE_HIGH>;    /* GPIO_145: trigger for turning on/off w2sg0004 */
> +        };
> +};
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
> index c03d20140366..c56b3181b266 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> @@ -345,6 +345,7 @@ voipac	Voipac Technologies s.r.o.
> wd	Western Digital Corp.
> wetek	WeTek Electronics, limited.
> wexler	Wexler
> +wi2wi	Wi2Wi, Inc.
> winbond Winbond Electronics corp.
> winstar	Winstar Display Corp.
> wlf	Wolfson Microelectronics
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 60f876b03586..7f97ef8fb6cd 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -509,4 +509,20 @@ source "drivers/misc/mic/Kconfig"
> source "drivers/misc/genwqe/Kconfig"
> source "drivers/misc/echo/Kconfig"
> source "drivers/misc/cxl/Kconfig"
> +
> +config W2SG0004
> +	tristate "W2SG00x4 on/off control"
> +	depends on GPIOLIB && SERIAL_DEV_BUS
> +	help
> +          Enable on/off control of W2SG00x4 GPS moduled connected
> +	  to some SoC UART to allow powering up/down if the /dev/ttyGPSn
> +	  is opened/closed.
> +	  It also provides a rfkill gps name 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
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index 81ef3e67acc9..0e88e06e5ee0 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -50,6 +50,7 @@ obj-$(CONFIG_SRAM_EXEC)		+= sram-exec.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/
> obj-$(CONFIG_ASPEED_LPC_CTRL)	+= aspeed-lpc-ctrl.o
> diff --git a/drivers/misc/w2sg0004.c b/drivers/misc/w2sg0004.c
> new file mode 100644
> index 000000000000..1b335317c8ac
> --- /dev/null
> +++ b/drivers/misc/w2sg0004.c
> @@ -0,0 +1,646 @@
> +/*
> + * 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/delay.h>
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_gpio.h>
> +#include <linux/platform_device.h>
> +#include <linux/rfkill.h>
> +#include <linux/serdev.h>
> +#include <linux/sched.h>
> +#include <linux/slab.h>
> +#include <linux/tty.h>
> +#include <linux/tty_flip.h>
> +#include <linux/w2sg0004.h>
> +#include <linux/workqueue.h>
> +
> +#ifdef CONFIG_W2SG0004_DEBUG	// not for upstreaming
> +#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		serdev_device *uart;	/* the uart connected to the chip */
> +	struct		tty_driver *tty_drv;	/* this is the user space tty */
> +	struct		device *dev;	/* returned by tty_port_register_device() */
> +	struct		tty_port port;
> +	int		open_count;	/* how often we were opened */
> +	enum		w2sg_state state;
> +	int		requested;	/* requested state (0/1) */
> +	int		suspended;
> +	spinlock_t	lock;
> +	struct delayed_work work;
> +	int		discard_count;
> +};
> +
> +static struct w2sg_data *w2sg_by_minor[1];
> +
> +static int w2sg_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_set_power(void *pdata, int val)
> +{
> +	struct w2sg_data *data = (struct w2sg_data *) pdata;
> +
> +	pr_debug("%s to state=%d (requested=%d)\n", __func__, val, data->requested);
> +
> +	if (val && !data->requested) {
> +		data->requested = true;
> +	} else if (!val && data->requested) {
> +		data->backoff = HZ;
> +		data->requested = false;
> +	} else
> +		return;
> +
> +	pr_debug("w2sg00x4 scheduled for %d\n", data->requested);
> +
> +	if (!data->suspended)
> +		schedule_delayed_work(&data->work, 0);
> +}
> +
> +/* called each time data is received by the UART (i.e. sent by the w2sg0004) */
> +
> +static int w2sg_uart_receive_buf(struct serdev_device *serdev, const unsigned char *rxdata,
> +				size_t count)
> +{
> +	struct w2sg_data *data = (struct w2sg_data *) serdev_device_get_drvdata(serdev);
> +//	unsigned long flags;
> +
> +//	pr_debug("w2sg: %d characters\n", count);
> +
> +	if (!data->requested && !data->is_on) {
> +		/* we have received characters while the w2sg should have been be turned off */
> +		data->discard_count += count;
> +		if ((data->state == W2SG_IDLE) &&
> +		    time_after(jiffies,
> +		    data->last_toggle + data->backoff)) {
> +			/* Should be off by now, time to toggle again */
> +			pr_debug("w2sg00x4 has sent %d characters data although it should be off!\n", data->discard_count);
> +			data->discard_count = 0;
> +
> +			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);
> +		}
> +	} else if (data->open_count > 0) {
> +		int n;
> +
> +//		pr_debug("w2sg00x4: push %d chars to tty port\n", count);
> +		n = tty_insert_flip_string(&data->port, rxdata, count);	/* pass to user-space */
> +		if (n != count)
> +			pr_debug("w2sg00x4: did loose %d characters\n", count - n);
> +		tty_flip_buffer_push(&data->port);
> +		return n;
> +	}
> +
> +	/* assume we have processed everything */
> +	return count;
> +}
> +
> +/* 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_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_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_set_lna_power(data);
> +}
> +
> +static struct rfkill_ops w2sg0004_rfkill_ops = {
> +	.set_block = w2sg_rfkill_set_block,
> +};
> +
> +static struct serdev_device_ops serdev_ops = {
> +	.receive_buf = w2sg_uart_receive_buf,
> +#if 0
> +	.write_wakeup = w2sg_uart_wakeup,
> +#endif
> +};
> +
> +/*
> + * we are a man-in the middle between the user-space visible tty port
> + * and the serdev tty where the chip is connected.
> + * This allows us to recognise when the device should be powered on
> + * or off and handle the "false" state that data arrives while no
> + * users-space tty client exists.
> + */
> +
> +static struct w2sg_data *w2sg_get_by_minor(unsigned int minor)
> +{
> +	return w2sg_by_minor[minor];
> +}
> +
> +static int w2sg_tty_install(struct tty_driver *driver, struct tty_struct *tty)
> +{
> +	struct w2sg_data *data;
> +	int retval;
> +
> +	pr_debug("%s() tty = %p\n", __func__, tty);
> +
> +	data = w2sg_get_by_minor(tty->index);
> +	pr_debug("%s() data = %p\n", __func__, data);
> +
> +	if (!data)
> +		return -ENODEV;
> +
> +	retval = tty_standard_install(driver, tty);
> +	if (retval)
> +		goto error_init_termios;
> +
> +	tty->driver_data = data;
> +
> +	return 0;
> +
> +error_init_termios:
> +	tty_port_put(&data->port);
> +	return retval;
> +}
> +
> +static int w2sg_tty_open(struct tty_struct *tty, struct file *file)
> +{
> +	struct w2sg_data *data = tty->driver_data;
> +
> +	pr_debug("%s() data = %p open_count = ++%d\n", __func__, data, data->open_count);
> +//	val = (val & TIOCM_DTR) != 0;	/* DTR controls power on/off */
> +
> +	w2sg_set_power(data, ++data->open_count > 0);
> +
> +// we could/should return -Esomething if already open...
> +
> +	return tty_port_open(&data->port, tty, file);
> +}
> +
> +static void w2sg_tty_close(struct tty_struct *tty, struct file *file)
> +{
> +	struct w2sg_data *data = tty->driver_data;
> +
> +	pr_debug("%s()\n", __func__);
> +//	val = (val & TIOCM_DTR) != 0;	/* DTR controls power on/off */
> +	w2sg_set_power(data, --data->open_count > 0);
> +
> +	tty_port_close(&data->port, tty, file);
> +}
> +
> +static int w2sg_tty_write(struct tty_struct *tty,
> +		const unsigned char *buffer, int count)
> +{
> +	struct w2sg_data *data = tty->driver_data;
> +
> +	/* simply pass down to UART */
> +	return serdev_device_write_buf(data->uart, buffer, count);
> +}
> +
> +#if 0
> +static void w2sg_tty_tiocmget(...)
> +{
> +	int val;
> +
> +	pr_debug("%s(...,%x)\n", __func__, val);
> +	val = (val & TIOCM_DTR) != 0;	/* DTR controls power on/off */
> +	w2sg_set_power((struct w2sg_data *) pdata, val);
> +}
> +#endif
> +
> +
> +static const struct tty_operations w2sg_serial_ops = {
> +	.install = w2sg_tty_install,
> +	.open = w2sg_tty_open,
> +	.close = w2sg_tty_close,
> +	.write = w2sg_tty_write,
> +#if 0
> +	.write_room = w2sg_tty_write_room,
> +	.cleanup = w2sg_tty_cleanup,
> +	.ioctl = w2sg_tty_ioctl,
> +	.set_termios = w2sg_tty_set_termios,
> +	.chars_in_buffer = w2sg_tty_chars_in_buffer,
> +	.tiocmget = w2sg_tty_tiocmget,
> +	.tiocmset = w2sg_tty_tiocmset,
> +	.get_icount = w2sg_tty_get_count,
> +	.unthrottle = w2sg_tty_unthrottle
> +#endif
> +};
> +
> +static const struct tty_port_operations w2sg_port_ops = {
> +};
> +
> +static int w2sg_probe(struct serdev_device *serdev)
> +{
> +	struct w2sg_pdata *pdata = NULL;
> +	struct w2sg_data *data;
> +	struct rfkill *rf_kill;
> +	int err;
> +	int minor;
> +
> +	pr_debug("%s()\n", __func__);
> +
> +	minor = 0;
> +	if (w2sg_by_minor[minor]) {
> +		pr_err("w2sg minor is already in use!\n");
> +		return -ENODEV;
> +	}
> +
> +// can be simplified if we require OF
> +
> +	if (serdev->dev.of_node) {
> +		struct device *dev = &serdev->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-gpios", 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");
> +// shouldn't we defer probing as well???
> +
> +		pr_debug("%s() lna_regulator = %p\n", __func__, pdata->lna_regulator);
> +
> +		serdev->dev.platform_data = pdata;
> +	}
> +
> +	data = devm_kzalloc(&serdev->dev, sizeof(*data), GFP_KERNEL);
> +	if (data == NULL)
> +		return -ENOMEM;
> +
> +	w2sg_by_minor[minor] = data;
> +
> +#if 1
> +	pr_debug("w2sg serdev_device_set_drvdata\n");
> +#endif
> +	serdev_device_set_drvdata(serdev, data);
> +
> +	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;
> +
> +	data->uart = serdev;
> +
> +	INIT_DELAYED_WORK(&data->work, toggle_work);
> +//	spin_lock_init(&data->lock);
> +
> +#if 1
> +	pr_debug("w2sg devm_gpio_request\n");
> +#endif
> +	err = devm_gpio_request(&serdev->dev, data->on_off_gpio, "w2sg0004-on-off");
> +	if (err < 0)
> +		goto out;
> +
> +	gpio_direction_output(data->on_off_gpio, false);
> +
> +	serdev_device_set_client_ops(data->uart, &serdev_ops);
> +	serdev_device_open(data->uart);
> +
> +	serdev_device_set_baudrate(data->uart, 9600);
> +	serdev_device_set_flow_control(data->uart, false);
> +
> +#if 1
> +	pr_debug("w2sg rfkill_alloc\n");
> +#endif
> +	rf_kill = rfkill_alloc("GPS", &serdev->dev, RFKILL_TYPE_GPS,
> +				&w2sg0004_rfkill_ops, data);
> +	if (rf_kill == NULL) {
> +		err = -ENOMEM;
> +		goto err_rfkill;
> +	}
> +
> +#if 1
> +	pr_debug("w2sg register rfkill\n");
> +#endif
> +	err = rfkill_register(rf_kill);
> +	if (err) {
> +		dev_err(&serdev->dev, "Cannot register rfkill device\n");
> +		goto err_rfkill;
> +	}
> +
> +	data->rf_kill = rf_kill;
> +
> +#if 1
> +	pr_debug("w2sg alloc_tty_driver\n");
> +#endif
> +	/* allocate the tty driver */
> +	data->tty_drv = alloc_tty_driver(1);
> +	if (!data->tty_drv)
> +		return -ENOMEM;
> +
> +	/* initialize the tty driver */
> +	data->tty_drv->owner = THIS_MODULE;
> +	data->tty_drv->driver_name = "w2sg0004";
> +	data->tty_drv->name = "ttyGPS";
> +	data->tty_drv->major = 0;
> +	data->tty_drv->minor_start = 0;
> +	data->tty_drv->type = TTY_DRIVER_TYPE_SERIAL;
> +	data->tty_drv->subtype = SERIAL_TYPE_NORMAL;
> +	data->tty_drv->flags = TTY_DRIVER_REAL_RAW | TTY_DRIVER_DYNAMIC_DEV;
> +	data->tty_drv->init_termios = tty_std_termios;
> +	data->tty_drv->init_termios.c_cflag = B9600 | CS8 | CREAD | HUPCL | CLOCAL;
> +	/*
> +	 * tty_termios_encode_baud_rate(&data->tty_drv->init_termios, 115200, 115200);
> +	 * w2sg_tty_termios(&data->tty_drv->init_termios);
> +	 */
> +	tty_set_operations(data->tty_drv, &w2sg_serial_ops);
> +
> +#if 1
> +	pr_debug("w2sg tty_register_driver\n");
> +#endif
> +	/* register the tty driver */
> +	err = tty_register_driver(data->tty_drv);
> +	if (err) {
> +		pr_err("%s - tty_register_driver failed(%d)\n",
> +			__func__, err);
> +		put_tty_driver(data->tty_drv);
> +		goto err_rfkill;
> +	}
> +
> +#if 1
> +	pr_debug("w2sg call tty_port_init\n");
> +#endif
> +	tty_port_init(&data->port);
> +	data->port.ops = &w2sg_port_ops;
> +
> +#if 1
> +	pr_debug("w2sg call tty_port_register_device\n");
> +#endif
> +/*
> + * FIXME: this appears to reenter this probe() function a second time
> + * which only fails because the gpio is already assigned
> + */
> +
> +	data->dev = tty_port_register_device(&data->port,
> +			data->tty_drv, minor, &serdev->dev);
> +
> +#if 1
> +	pr_debug("w2sg tty_port_register_device -> %p\n", data->dev);
> +	pr_debug("w2sg port.tty = %p\n", data->port.tty);
> +#endif
> +//	data->port.tty->driver_data = data;	/* make us known in tty_struct */
> +
> +	pr_debug("w2sg probed\n");
> +
> +#ifdef CONFIG_W2SG0004_DEBUG
> +	pr_debug("w2sg DEBUGGING MODE enabled\n");
> +	/* 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
> +
> +	/* keep off until user space requests the device */
> +	w2sg_set_power(data, false);
> +
> +#if 0	// more debugging - not for upstreaming
> +	w2sg_set_power(data, true);
> +#endif
> +
> +	return 0;
> +
> +err_rfkill:
> +	rfkill_destroy(rf_kill);
> +	serdev_device_close(data->uart);
> +out:
> +#if 0
> +	if (err == -EBUSY)
> +		err = -EPROBE_DEFER;
> +#endif
> +#if 1
> +	pr_debug("w2sg error %d\n", err);
> +#endif
> +	return err;
> +}
> +
> +static void w2sg_remove(struct serdev_device *serdev)
> +{
> +	struct w2sg_data *data = serdev_device_get_drvdata(serdev);
> +	int minor;
> +
> +	cancel_delayed_work_sync(&data->work);
> +
> +	/* what is the right sequence to avoid problems? */
> +	serdev_device_close(data->uart);
> +
> +	// get minor from searching for data == w2sg_by_minor[minor]
> +	minor = 0;
> +	tty_unregister_device(data->tty_drv, minor);
> +
> +	tty_unregister_driver(data->tty_drv);
> +}
> +
> +static int w2sg_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_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_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_suspend, w2sg_resume);
> +
> +static struct serdev_device_driver w2sg_driver = {
> +	.probe		= w2sg_probe,
> +	.remove		= w2sg_remove,
> +	.driver = {
> +		.name	= "w2sg0004",
> +		.owner	= THIS_MODULE,
> +		.pm	= &w2sg_pm_ops,
> +		.of_match_table = of_match_ptr(w2sg0004_of_match)
> +	},
> +};
> +
> +module_serdev_device_driver(w2sg_driver);
> +
> +MODULE_ALIAS("w2sg0004");
> +
> +MODULE_AUTHOR("NeilBrown <neilb@suse.de>");
> +MODULE_DESCRIPTION("w2sg0004 GPS power management driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/w2sg0004.h b/include/linux/w2sg0004.h
> new file mode 100644
> index 000000000000..ad0c4a18e01d
> --- /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.12.2
> 

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

* Re: [RFC 2/3] misc: Add w2sg0004 (gps receiver) power control driver
@ 2017-05-21 10:48     ` H. Nikolaus Schaller
  0 siblings, 0 replies; 24+ messages in thread
From: H. Nikolaus Schaller @ 2017-05-21 10:48 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Benoît Cousson, Tony Lindgren,
	Russell King, Nikolaus Schaller, Thierry Reding,
	Jonathan Cameron, Maxime Ripard, Jarkko Sakkinen
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	letux-kernel-S0jZdbWzriLCfDggNXIi3w,
	kernel-Jl6IXVxNIMRxAtABVqVhTwC/G2K4zDHf


> Am 21.05.2017 um 12:44 schrieb H. Nikolaus Schaller <hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>:
> 
> Add driver for Wi2Wi W2SG0004/84 GPS module connected through uart.
> 
> Use serdev API hooks to monitor and forward the UART traffic to /dev/BTn

s/BTn/GPSn/

> and turn on/off the module. It also detects if the module is turned on (sends data)
> but should be off, e.g. if it was already turned on during boot or power-on-reset.
> 
> 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 new serdev API introduced in 4.11.
> 
> Signed-off-by: H. Nikolaus Schaller <hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>
> ---
> .../devicetree/bindings/misc/wi2wi,w2sg0004.txt    |  20 +
> .../devicetree/bindings/vendor-prefixes.txt        |   1 +
> drivers/misc/Kconfig                               |  16 +
> drivers/misc/Makefile                              |   1 +
> drivers/misc/w2sg0004.c                            | 646 +++++++++++++++++++++
> include/linux/w2sg0004.h                           |  27 +
> 6 files changed, 711 insertions(+)
> 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 000000000000..b7125c7a598c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
> @@ -0,0 +1,20 @@
> +Wi2Wi GPS module connected through UART
> +
> +Should be a subnode of the SoC UART it is connected to (serdev).
> +
> +Required properties:
> +- compatible:	wi2wi,w2sg0004 or wi2wi,w2sg0084
> +- on-off-gpio:	the GPIO that controls the module's on-off toggle input
> +
> +Optional properties:
> +- lna-suppy:	an (optional) LNA regulator that is enabled together with the GPS receiver
> +
> +Example:
> +
> +&uart2 {
> +	gps: w2sg0004 {
> +		compatible = "wi2wi,w2sg0004";
> +		lna-supply = <&vsim>;   /* LNA regulator */
> +		on-off-gpios = <&gpio5 17 GPIO_ACTIVE_HIGH>;    /* GPIO_145: trigger for turning on/off w2sg0004 */
> +        };
> +};
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
> index c03d20140366..c56b3181b266 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> @@ -345,6 +345,7 @@ voipac	Voipac Technologies s.r.o.
> wd	Western Digital Corp.
> wetek	WeTek Electronics, limited.
> wexler	Wexler
> +wi2wi	Wi2Wi, Inc.
> winbond Winbond Electronics corp.
> winstar	Winstar Display Corp.
> wlf	Wolfson Microelectronics
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 60f876b03586..7f97ef8fb6cd 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -509,4 +509,20 @@ source "drivers/misc/mic/Kconfig"
> source "drivers/misc/genwqe/Kconfig"
> source "drivers/misc/echo/Kconfig"
> source "drivers/misc/cxl/Kconfig"
> +
> +config W2SG0004
> +	tristate "W2SG00x4 on/off control"
> +	depends on GPIOLIB && SERIAL_DEV_BUS
> +	help
> +          Enable on/off control of W2SG00x4 GPS moduled connected
> +	  to some SoC UART to allow powering up/down if the /dev/ttyGPSn
> +	  is opened/closed.
> +	  It also provides a rfkill gps name 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
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index 81ef3e67acc9..0e88e06e5ee0 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -50,6 +50,7 @@ obj-$(CONFIG_SRAM_EXEC)		+= sram-exec.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/
> obj-$(CONFIG_ASPEED_LPC_CTRL)	+= aspeed-lpc-ctrl.o
> diff --git a/drivers/misc/w2sg0004.c b/drivers/misc/w2sg0004.c
> new file mode 100644
> index 000000000000..1b335317c8ac
> --- /dev/null
> +++ b/drivers/misc/w2sg0004.c
> @@ -0,0 +1,646 @@
> +/*
> + * 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/delay.h>
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_gpio.h>
> +#include <linux/platform_device.h>
> +#include <linux/rfkill.h>
> +#include <linux/serdev.h>
> +#include <linux/sched.h>
> +#include <linux/slab.h>
> +#include <linux/tty.h>
> +#include <linux/tty_flip.h>
> +#include <linux/w2sg0004.h>
> +#include <linux/workqueue.h>
> +
> +#ifdef CONFIG_W2SG0004_DEBUG	// not for upstreaming
> +#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		serdev_device *uart;	/* the uart connected to the chip */
> +	struct		tty_driver *tty_drv;	/* this is the user space tty */
> +	struct		device *dev;	/* returned by tty_port_register_device() */
> +	struct		tty_port port;
> +	int		open_count;	/* how often we were opened */
> +	enum		w2sg_state state;
> +	int		requested;	/* requested state (0/1) */
> +	int		suspended;
> +	spinlock_t	lock;
> +	struct delayed_work work;
> +	int		discard_count;
> +};
> +
> +static struct w2sg_data *w2sg_by_minor[1];
> +
> +static int w2sg_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_set_power(void *pdata, int val)
> +{
> +	struct w2sg_data *data = (struct w2sg_data *) pdata;
> +
> +	pr_debug("%s to state=%d (requested=%d)\n", __func__, val, data->requested);
> +
> +	if (val && !data->requested) {
> +		data->requested = true;
> +	} else if (!val && data->requested) {
> +		data->backoff = HZ;
> +		data->requested = false;
> +	} else
> +		return;
> +
> +	pr_debug("w2sg00x4 scheduled for %d\n", data->requested);
> +
> +	if (!data->suspended)
> +		schedule_delayed_work(&data->work, 0);
> +}
> +
> +/* called each time data is received by the UART (i.e. sent by the w2sg0004) */
> +
> +static int w2sg_uart_receive_buf(struct serdev_device *serdev, const unsigned char *rxdata,
> +				size_t count)
> +{
> +	struct w2sg_data *data = (struct w2sg_data *) serdev_device_get_drvdata(serdev);
> +//	unsigned long flags;
> +
> +//	pr_debug("w2sg: %d characters\n", count);
> +
> +	if (!data->requested && !data->is_on) {
> +		/* we have received characters while the w2sg should have been be turned off */
> +		data->discard_count += count;
> +		if ((data->state == W2SG_IDLE) &&
> +		    time_after(jiffies,
> +		    data->last_toggle + data->backoff)) {
> +			/* Should be off by now, time to toggle again */
> +			pr_debug("w2sg00x4 has sent %d characters data although it should be off!\n", data->discard_count);
> +			data->discard_count = 0;
> +
> +			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);
> +		}
> +	} else if (data->open_count > 0) {
> +		int n;
> +
> +//		pr_debug("w2sg00x4: push %d chars to tty port\n", count);
> +		n = tty_insert_flip_string(&data->port, rxdata, count);	/* pass to user-space */
> +		if (n != count)
> +			pr_debug("w2sg00x4: did loose %d characters\n", count - n);
> +		tty_flip_buffer_push(&data->port);
> +		return n;
> +	}
> +
> +	/* assume we have processed everything */
> +	return count;
> +}
> +
> +/* 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_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_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_set_lna_power(data);
> +}
> +
> +static struct rfkill_ops w2sg0004_rfkill_ops = {
> +	.set_block = w2sg_rfkill_set_block,
> +};
> +
> +static struct serdev_device_ops serdev_ops = {
> +	.receive_buf = w2sg_uart_receive_buf,
> +#if 0
> +	.write_wakeup = w2sg_uart_wakeup,
> +#endif
> +};
> +
> +/*
> + * we are a man-in the middle between the user-space visible tty port
> + * and the serdev tty where the chip is connected.
> + * This allows us to recognise when the device should be powered on
> + * or off and handle the "false" state that data arrives while no
> + * users-space tty client exists.
> + */
> +
> +static struct w2sg_data *w2sg_get_by_minor(unsigned int minor)
> +{
> +	return w2sg_by_minor[minor];
> +}
> +
> +static int w2sg_tty_install(struct tty_driver *driver, struct tty_struct *tty)
> +{
> +	struct w2sg_data *data;
> +	int retval;
> +
> +	pr_debug("%s() tty = %p\n", __func__, tty);
> +
> +	data = w2sg_get_by_minor(tty->index);
> +	pr_debug("%s() data = %p\n", __func__, data);
> +
> +	if (!data)
> +		return -ENODEV;
> +
> +	retval = tty_standard_install(driver, tty);
> +	if (retval)
> +		goto error_init_termios;
> +
> +	tty->driver_data = data;
> +
> +	return 0;
> +
> +error_init_termios:
> +	tty_port_put(&data->port);
> +	return retval;
> +}
> +
> +static int w2sg_tty_open(struct tty_struct *tty, struct file *file)
> +{
> +	struct w2sg_data *data = tty->driver_data;
> +
> +	pr_debug("%s() data = %p open_count = ++%d\n", __func__, data, data->open_count);
> +//	val = (val & TIOCM_DTR) != 0;	/* DTR controls power on/off */
> +
> +	w2sg_set_power(data, ++data->open_count > 0);
> +
> +// we could/should return -Esomething if already open...
> +
> +	return tty_port_open(&data->port, tty, file);
> +}
> +
> +static void w2sg_tty_close(struct tty_struct *tty, struct file *file)
> +{
> +	struct w2sg_data *data = tty->driver_data;
> +
> +	pr_debug("%s()\n", __func__);
> +//	val = (val & TIOCM_DTR) != 0;	/* DTR controls power on/off */
> +	w2sg_set_power(data, --data->open_count > 0);
> +
> +	tty_port_close(&data->port, tty, file);
> +}
> +
> +static int w2sg_tty_write(struct tty_struct *tty,
> +		const unsigned char *buffer, int count)
> +{
> +	struct w2sg_data *data = tty->driver_data;
> +
> +	/* simply pass down to UART */
> +	return serdev_device_write_buf(data->uart, buffer, count);
> +}
> +
> +#if 0
> +static void w2sg_tty_tiocmget(...)
> +{
> +	int val;
> +
> +	pr_debug("%s(...,%x)\n", __func__, val);
> +	val = (val & TIOCM_DTR) != 0;	/* DTR controls power on/off */
> +	w2sg_set_power((struct w2sg_data *) pdata, val);
> +}
> +#endif
> +
> +
> +static const struct tty_operations w2sg_serial_ops = {
> +	.install = w2sg_tty_install,
> +	.open = w2sg_tty_open,
> +	.close = w2sg_tty_close,
> +	.write = w2sg_tty_write,
> +#if 0
> +	.write_room = w2sg_tty_write_room,
> +	.cleanup = w2sg_tty_cleanup,
> +	.ioctl = w2sg_tty_ioctl,
> +	.set_termios = w2sg_tty_set_termios,
> +	.chars_in_buffer = w2sg_tty_chars_in_buffer,
> +	.tiocmget = w2sg_tty_tiocmget,
> +	.tiocmset = w2sg_tty_tiocmset,
> +	.get_icount = w2sg_tty_get_count,
> +	.unthrottle = w2sg_tty_unthrottle
> +#endif
> +};
> +
> +static const struct tty_port_operations w2sg_port_ops = {
> +};
> +
> +static int w2sg_probe(struct serdev_device *serdev)
> +{
> +	struct w2sg_pdata *pdata = NULL;
> +	struct w2sg_data *data;
> +	struct rfkill *rf_kill;
> +	int err;
> +	int minor;
> +
> +	pr_debug("%s()\n", __func__);
> +
> +	minor = 0;
> +	if (w2sg_by_minor[minor]) {
> +		pr_err("w2sg minor is already in use!\n");
> +		return -ENODEV;
> +	}
> +
> +// can be simplified if we require OF
> +
> +	if (serdev->dev.of_node) {
> +		struct device *dev = &serdev->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-gpios", 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");
> +// shouldn't we defer probing as well???
> +
> +		pr_debug("%s() lna_regulator = %p\n", __func__, pdata->lna_regulator);
> +
> +		serdev->dev.platform_data = pdata;
> +	}
> +
> +	data = devm_kzalloc(&serdev->dev, sizeof(*data), GFP_KERNEL);
> +	if (data == NULL)
> +		return -ENOMEM;
> +
> +	w2sg_by_minor[minor] = data;
> +
> +#if 1
> +	pr_debug("w2sg serdev_device_set_drvdata\n");
> +#endif
> +	serdev_device_set_drvdata(serdev, data);
> +
> +	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;
> +
> +	data->uart = serdev;
> +
> +	INIT_DELAYED_WORK(&data->work, toggle_work);
> +//	spin_lock_init(&data->lock);
> +
> +#if 1
> +	pr_debug("w2sg devm_gpio_request\n");
> +#endif
> +	err = devm_gpio_request(&serdev->dev, data->on_off_gpio, "w2sg0004-on-off");
> +	if (err < 0)
> +		goto out;
> +
> +	gpio_direction_output(data->on_off_gpio, false);
> +
> +	serdev_device_set_client_ops(data->uart, &serdev_ops);
> +	serdev_device_open(data->uart);
> +
> +	serdev_device_set_baudrate(data->uart, 9600);
> +	serdev_device_set_flow_control(data->uart, false);
> +
> +#if 1
> +	pr_debug("w2sg rfkill_alloc\n");
> +#endif
> +	rf_kill = rfkill_alloc("GPS", &serdev->dev, RFKILL_TYPE_GPS,
> +				&w2sg0004_rfkill_ops, data);
> +	if (rf_kill == NULL) {
> +		err = -ENOMEM;
> +		goto err_rfkill;
> +	}
> +
> +#if 1
> +	pr_debug("w2sg register rfkill\n");
> +#endif
> +	err = rfkill_register(rf_kill);
> +	if (err) {
> +		dev_err(&serdev->dev, "Cannot register rfkill device\n");
> +		goto err_rfkill;
> +	}
> +
> +	data->rf_kill = rf_kill;
> +
> +#if 1
> +	pr_debug("w2sg alloc_tty_driver\n");
> +#endif
> +	/* allocate the tty driver */
> +	data->tty_drv = alloc_tty_driver(1);
> +	if (!data->tty_drv)
> +		return -ENOMEM;
> +
> +	/* initialize the tty driver */
> +	data->tty_drv->owner = THIS_MODULE;
> +	data->tty_drv->driver_name = "w2sg0004";
> +	data->tty_drv->name = "ttyGPS";
> +	data->tty_drv->major = 0;
> +	data->tty_drv->minor_start = 0;
> +	data->tty_drv->type = TTY_DRIVER_TYPE_SERIAL;
> +	data->tty_drv->subtype = SERIAL_TYPE_NORMAL;
> +	data->tty_drv->flags = TTY_DRIVER_REAL_RAW | TTY_DRIVER_DYNAMIC_DEV;
> +	data->tty_drv->init_termios = tty_std_termios;
> +	data->tty_drv->init_termios.c_cflag = B9600 | CS8 | CREAD | HUPCL | CLOCAL;
> +	/*
> +	 * tty_termios_encode_baud_rate(&data->tty_drv->init_termios, 115200, 115200);
> +	 * w2sg_tty_termios(&data->tty_drv->init_termios);
> +	 */
> +	tty_set_operations(data->tty_drv, &w2sg_serial_ops);
> +
> +#if 1
> +	pr_debug("w2sg tty_register_driver\n");
> +#endif
> +	/* register the tty driver */
> +	err = tty_register_driver(data->tty_drv);
> +	if (err) {
> +		pr_err("%s - tty_register_driver failed(%d)\n",
> +			__func__, err);
> +		put_tty_driver(data->tty_drv);
> +		goto err_rfkill;
> +	}
> +
> +#if 1
> +	pr_debug("w2sg call tty_port_init\n");
> +#endif
> +	tty_port_init(&data->port);
> +	data->port.ops = &w2sg_port_ops;
> +
> +#if 1
> +	pr_debug("w2sg call tty_port_register_device\n");
> +#endif
> +/*
> + * FIXME: this appears to reenter this probe() function a second time
> + * which only fails because the gpio is already assigned
> + */
> +
> +	data->dev = tty_port_register_device(&data->port,
> +			data->tty_drv, minor, &serdev->dev);
> +
> +#if 1
> +	pr_debug("w2sg tty_port_register_device -> %p\n", data->dev);
> +	pr_debug("w2sg port.tty = %p\n", data->port.tty);
> +#endif
> +//	data->port.tty->driver_data = data;	/* make us known in tty_struct */
> +
> +	pr_debug("w2sg probed\n");
> +
> +#ifdef CONFIG_W2SG0004_DEBUG
> +	pr_debug("w2sg DEBUGGING MODE enabled\n");
> +	/* 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
> +
> +	/* keep off until user space requests the device */
> +	w2sg_set_power(data, false);
> +
> +#if 0	// more debugging - not for upstreaming
> +	w2sg_set_power(data, true);
> +#endif
> +
> +	return 0;
> +
> +err_rfkill:
> +	rfkill_destroy(rf_kill);
> +	serdev_device_close(data->uart);
> +out:
> +#if 0
> +	if (err == -EBUSY)
> +		err = -EPROBE_DEFER;
> +#endif
> +#if 1
> +	pr_debug("w2sg error %d\n", err);
> +#endif
> +	return err;
> +}
> +
> +static void w2sg_remove(struct serdev_device *serdev)
> +{
> +	struct w2sg_data *data = serdev_device_get_drvdata(serdev);
> +	int minor;
> +
> +	cancel_delayed_work_sync(&data->work);
> +
> +	/* what is the right sequence to avoid problems? */
> +	serdev_device_close(data->uart);
> +
> +	// get minor from searching for data == w2sg_by_minor[minor]
> +	minor = 0;
> +	tty_unregister_device(data->tty_drv, minor);
> +
> +	tty_unregister_driver(data->tty_drv);
> +}
> +
> +static int w2sg_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_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_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_suspend, w2sg_resume);
> +
> +static struct serdev_device_driver w2sg_driver = {
> +	.probe		= w2sg_probe,
> +	.remove		= w2sg_remove,
> +	.driver = {
> +		.name	= "w2sg0004",
> +		.owner	= THIS_MODULE,
> +		.pm	= &w2sg_pm_ops,
> +		.of_match_table = of_match_ptr(w2sg0004_of_match)
> +	},
> +};
> +
> +module_serdev_device_driver(w2sg_driver);
> +
> +MODULE_ALIAS("w2sg0004");
> +
> +MODULE_AUTHOR("NeilBrown <neilb-l3A5Bk7waGM@public.gmane.org>");
> +MODULE_DESCRIPTION("w2sg0004 GPS power management driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/w2sg0004.h b/include/linux/w2sg0004.h
> new file mode 100644
> index 000000000000..ad0c4a18e01d
> --- /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.12.2
> 

--
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] 24+ messages in thread

* Re: [RFC 0/3] misc: new serdev based drivers for w2sg00x4 GPS module and w2cbw003 wifi/bluetooth
@ 2017-05-23  2:26   ` Rob Herring
  0 siblings, 0 replies; 24+ messages in thread
From: Rob Herring @ 2017-05-23  2:26 UTC (permalink / raw)
  To: H. Nikolaus Schaller
  Cc: Mark Rutland, Benoît Cousson, Tony Lindgren, Russell King,
	Thierry Reding, Jonathan Cameron, Maxime Ripard, Jarkko Sakkinen,
	devicetree, linux-kernel, linux-omap,
	Discussions about the Letux Kernel, kernel

On Sun, May 21, 2017 at 5:44 AM, H. Nikolaus Schaller <hns@goldelico.com> wrote:
> Since our proposed API was not acceptable and the new serdev API has arrived in 4.11 kernels,
> we finally took the challenge to update the w2sg and w2cbw drivers to use the serdev API.
>
> The approach is to write a "man in the middle" driver which is on one side a serdev client
> which directly controls the UART where the device is connected to and on the other side
> presents a new tty port so that user-space software can talk to the chips as if they would
> directly talk to the UART of the SoC (e.g. ttyO1). This is similar to connecting to a remote
> serial device e.g. through USB (ttyACM) or Bluetooth UART profiles.
>
> For example gpsd or hciattach expect a /dev/tty they can control (flow control, baud rate
> etc.).

I understand from the prior discussion why you want to pass the data
thru for gps, but why do you need to do that for BT?

> Here is the result of our first hack which is working as a demo on GTA04 devices (and the
> w2cbw driver can also be used to control a GTA04 variant with WL1837).
>
> Since it is just a demo hack, the code is not yet cleaned up, nor does it completely pass
> check-patch, nor follows 100% the coding styles. And certainly has some bugs.
>
> The most significant issue is that calling tty_port_register_device() inside of the
> serdev probe() function makes the serdev probe() function to be entered a second
> time. This does not lead to big problems since we currently have minor = 0
> and this makes the second call assume the device is not available.
>
> But we have no idea why this happens and how it can be prevented.

Johan's fixes may help there, but it is intended to be temporary to
have a separate API for registering tty ports with or without serdev.

Rob

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

* Re: [RFC 0/3] misc: new serdev based drivers for w2sg00x4 GPS module and w2cbw003 wifi/bluetooth
@ 2017-05-23  2:26   ` Rob Herring
  0 siblings, 0 replies; 24+ messages in thread
From: Rob Herring @ 2017-05-23  2:26 UTC (permalink / raw)
  To: H. Nikolaus Schaller
  Cc: Mark Rutland, Benoît Cousson, Tony Lindgren, Russell King,
	Thierry Reding, Jonathan Cameron, Maxime Ripard, Jarkko Sakkinen,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-omap,
	Discussions about the Letux Kernel,
	kernel-Jl6IXVxNIMRxAtABVqVhTwC/G2K4zDHf

On Sun, May 21, 2017 at 5:44 AM, H. Nikolaus Schaller <hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org> wrote:
> Since our proposed API was not acceptable and the new serdev API has arrived in 4.11 kernels,
> we finally took the challenge to update the w2sg and w2cbw drivers to use the serdev API.
>
> The approach is to write a "man in the middle" driver which is on one side a serdev client
> which directly controls the UART where the device is connected to and on the other side
> presents a new tty port so that user-space software can talk to the chips as if they would
> directly talk to the UART of the SoC (e.g. ttyO1). This is similar to connecting to a remote
> serial device e.g. through USB (ttyACM) or Bluetooth UART profiles.
>
> For example gpsd or hciattach expect a /dev/tty they can control (flow control, baud rate
> etc.).

I understand from the prior discussion why you want to pass the data
thru for gps, but why do you need to do that for BT?

> Here is the result of our first hack which is working as a demo on GTA04 devices (and the
> w2cbw driver can also be used to control a GTA04 variant with WL1837).
>
> Since it is just a demo hack, the code is not yet cleaned up, nor does it completely pass
> check-patch, nor follows 100% the coding styles. And certainly has some bugs.
>
> The most significant issue is that calling tty_port_register_device() inside of the
> serdev probe() function makes the serdev probe() function to be entered a second
> time. This does not lead to big problems since we currently have minor = 0
> and this makes the second call assume the device is not available.
>
> But we have no idea why this happens and how it can be prevented.

Johan's fixes may help there, but it is intended to be temporary to
have a separate API for registering tty ports with or without serdev.

Rob
--
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] 24+ messages in thread

* Re: [RFC 0/3] misc: new serdev based drivers for w2sg00x4 GPS module and w2cbw003 wifi/bluetooth
@ 2017-05-23  5:43     ` H. Nikolaus Schaller
  0 siblings, 0 replies; 24+ messages in thread
From: H. Nikolaus Schaller @ 2017-05-23  5:43 UTC (permalink / raw)
  To: Rob Herring
  Cc: Mark Rutland, Benoît Cousson, Tony Lindgren, Russell King,
	Thierry Reding, Jonathan Cameron, Maxime Ripard, Jarkko Sakkinen,
	devicetree, linux-kernel, linux-omap,
	Discussions about the Letux Kernel, kernel

Hi Rob,

> Am 23.05.2017 um 04:26 schrieb Rob Herring <robh+dt@kernel.org>:
> 
> On Sun, May 21, 2017 at 5:44 AM, H. Nikolaus Schaller <hns@goldelico.com> wrote:
>> Since our proposed API was not acceptable and the new serdev API has arrived in 4.11 kernels,
>> we finally took the challenge to update the w2sg and w2cbw drivers to use the serdev API.
>> 
>> The approach is to write a "man in the middle" driver which is on one side a serdev client
>> which directly controls the UART where the device is connected to and on the other side
>> presents a new tty port so that user-space software can talk to the chips as if they would
>> directly talk to the UART of the SoC (e.g. ttyO1). This is similar to connecting to a remote
>> serial device e.g. through USB (ttyACM) or Bluetooth UART profiles.
>> 
>> For example gpsd or hciattach expect a /dev/tty they can control (flow control, baud rate
>> etc.).
> 
> I understand from the prior discussion why you want to pass the data
> thru for gps, but why do you need to do that for BT?

Because we otherwise can't turn on power when /dev/ttyBT0 is opened and turn off when it
is closed. I.e. it should not be powered unless someone does a hciattach /dev/ttyBT0. And it
should be turned off by a killall hciattach.

Basically we would like to have a power control automatic like it exists for many other devices.

Since the BT chip is described as a serdev by DT, we see no other means than to pass data
through the serdev driver.

We had looked into the line discipline approach but it makes a lot of problems. The first one
is that registering a new system-wide ldesc number is required. Next we do not see how to make
a serdev driver (as it seems to be required by the DT) to register a different ldesc.

> 
>> Here is the result of our first hack which is working as a demo on GTA04 devices (and the
>> w2cbw driver can also be used to control a GTA04 variant with WL1837).
>> 
>> Since it is just a demo hack, the code is not yet cleaned up, nor does it completely pass
>> check-patch, nor follows 100% the coding styles. And certainly has some bugs.
>> 
>> The most significant issue is that calling tty_port_register_device() inside of the
>> serdev probe() function makes the serdev probe() function to be entered a second
>> time. This does not lead to big problems since we currently have minor = 0
>> and this makes the second call assume the device is not available.
>> 
>> But we have no idea why this happens and how it can be prevented.
> 
> Johan's fixes may help there, but it is intended to be temporary to
> have a separate API for registering tty ports with or without serdev.

Ah, would that mean something like a tty_port_register_device_without_serdev()?

Do you have a reference to his fixes?

BR and thanks,
Nikolaus

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

* Re: [RFC 0/3] misc: new serdev based drivers for w2sg00x4 GPS module and w2cbw003 wifi/bluetooth
@ 2017-05-23  5:43     ` H. Nikolaus Schaller
  0 siblings, 0 replies; 24+ messages in thread
From: H. Nikolaus Schaller @ 2017-05-23  5:43 UTC (permalink / raw)
  To: Rob Herring
  Cc: Mark Rutland, Benoît Cousson, Tony Lindgren, Russell King,
	Thierry Reding, Jonathan Cameron, Maxime Ripard, Jarkko Sakkinen,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-omap,
	Discussions about the Letux Kernel,
	kernel-Jl6IXVxNIMRxAtABVqVhTwC/G2K4zDHf

Hi Rob,

> Am 23.05.2017 um 04:26 schrieb Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>:
> 
> On Sun, May 21, 2017 at 5:44 AM, H. Nikolaus Schaller <hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org> wrote:
>> Since our proposed API was not acceptable and the new serdev API has arrived in 4.11 kernels,
>> we finally took the challenge to update the w2sg and w2cbw drivers to use the serdev API.
>> 
>> The approach is to write a "man in the middle" driver which is on one side a serdev client
>> which directly controls the UART where the device is connected to and on the other side
>> presents a new tty port so that user-space software can talk to the chips as if they would
>> directly talk to the UART of the SoC (e.g. ttyO1). This is similar to connecting to a remote
>> serial device e.g. through USB (ttyACM) or Bluetooth UART profiles.
>> 
>> For example gpsd or hciattach expect a /dev/tty they can control (flow control, baud rate
>> etc.).
> 
> I understand from the prior discussion why you want to pass the data
> thru for gps, but why do you need to do that for BT?

Because we otherwise can't turn on power when /dev/ttyBT0 is opened and turn off when it
is closed. I.e. it should not be powered unless someone does a hciattach /dev/ttyBT0. And it
should be turned off by a killall hciattach.

Basically we would like to have a power control automatic like it exists for many other devices.

Since the BT chip is described as a serdev by DT, we see no other means than to pass data
through the serdev driver.

We had looked into the line discipline approach but it makes a lot of problems. The first one
is that registering a new system-wide ldesc number is required. Next we do not see how to make
a serdev driver (as it seems to be required by the DT) to register a different ldesc.

> 
>> Here is the result of our first hack which is working as a demo on GTA04 devices (and the
>> w2cbw driver can also be used to control a GTA04 variant with WL1837).
>> 
>> Since it is just a demo hack, the code is not yet cleaned up, nor does it completely pass
>> check-patch, nor follows 100% the coding styles. And certainly has some bugs.
>> 
>> The most significant issue is that calling tty_port_register_device() inside of the
>> serdev probe() function makes the serdev probe() function to be entered a second
>> time. This does not lead to big problems since we currently have minor = 0
>> and this makes the second call assume the device is not available.
>> 
>> But we have no idea why this happens and how it can be prevented.
> 
> Johan's fixes may help there, but it is intended to be temporary to
> have a separate API for registering tty ports with or without serdev.

Ah, would that mean something like a tty_port_register_device_without_serdev()?

Do you have a reference to his fixes?

BR and thanks,
Nikolaus

--
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] 24+ messages in thread

* Re: [RFC 0/3] misc: new serdev based drivers for w2sg00x4 GPS module and w2cbw003 wifi/bluetooth
  2017-05-23  5:43     ` H. Nikolaus Schaller
  (?)
@ 2017-05-23 12:28     ` Rob Herring
  2017-05-23 12:48       ` H. Nikolaus Schaller
  -1 siblings, 1 reply; 24+ messages in thread
From: Rob Herring @ 2017-05-23 12:28 UTC (permalink / raw)
  To: H. Nikolaus Schaller
  Cc: Mark Rutland, Benoît Cousson, Tony Lindgren, Russell King,
	Thierry Reding, Jonathan Cameron, Maxime Ripard, Jarkko Sakkinen,
	devicetree, linux-kernel, linux-omap,
	Discussions about the Letux Kernel, kernel

On Tue, May 23, 2017 at 12:43 AM, H. Nikolaus Schaller
<hns@goldelico.com> wrote:
> Hi Rob,
>
>> Am 23.05.2017 um 04:26 schrieb Rob Herring <robh+dt@kernel.org>:
>>
>> On Sun, May 21, 2017 at 5:44 AM, H. Nikolaus Schaller <hns@goldelico.com> wrote:
>>> Since our proposed API was not acceptable and the new serdev API has arrived in 4.11 kernels,
>>> we finally took the challenge to update the w2sg and w2cbw drivers to use the serdev API.
>>>
>>> The approach is to write a "man in the middle" driver which is on one side a serdev client
>>> which directly controls the UART where the device is connected to and on the other side
>>> presents a new tty port so that user-space software can talk to the chips as if they would
>>> directly talk to the UART of the SoC (e.g. ttyO1). This is similar to connecting to a remote
>>> serial device e.g. through USB (ttyACM) or Bluetooth UART profiles.
>>>
>>> For example gpsd or hciattach expect a /dev/tty they can control (flow control, baud rate
>>> etc.).
>>
>> I understand from the prior discussion why you want to pass the data
>> thru for gps, but why do you need to do that for BT?
>
> Because we otherwise can't turn on power when /dev/ttyBT0 is opened and turn off when it
> is closed. I.e. it should not be powered unless someone does a hciattach /dev/ttyBT0. And it
> should be turned off by a killall hciattach.

Still, you can do power control within BT HCI drivers. You wouldn't be
limited to just open/close, but can handle suspend/resume as well.

> Basically we would like to have a power control automatic like it exists for many other devices.
>
> Since the BT chip is described as a serdev by DT, we see no other means than to pass data
> through the serdev driver.

We could have a blacklist if we need to have serdev not create a
device and create a tty device instead.

> We had looked into the line discipline approach but it makes a lot of problems. The first one
> is that registering a new system-wide ldesc number is required. Next we do not see how to make
> a serdev driver (as it seems to be required by the DT) to register a different ldesc.
>
>>
>>> Here is the result of our first hack which is working as a demo on GTA04 devices (and the
>>> w2cbw driver can also be used to control a GTA04 variant with WL1837).
>>>
>>> Since it is just a demo hack, the code is not yet cleaned up, nor does it completely pass
>>> check-patch, nor follows 100% the coding styles. And certainly has some bugs.
>>>
>>> The most significant issue is that calling tty_port_register_device() inside of the
>>> serdev probe() function makes the serdev probe() function to be entered a second
>>> time. This does not lead to big problems since we currently have minor = 0
>>> and this makes the second call assume the device is not available.
>>>
>>> But we have no idea why this happens and how it can be prevented.
>>
>> Johan's fixes may help there, but it is intended to be temporary to
>> have a separate API for registering tty ports with or without serdev.
>
> Ah, would that mean something like a tty_port_register_device_without_serdev()?

Yes, but other way around. The old function doesn't register with
serdev and there's a new function that will.

> Do you have a reference to his fixes?

They are in Greg's tty-linus branch if not Linus' tree now.

Rob

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

* Re: [RFC 0/3] misc: new serdev based drivers for w2sg00x4 GPS module and w2cbw003 wifi/bluetooth
  2017-05-23 12:28     ` Rob Herring
@ 2017-05-23 12:48       ` H. Nikolaus Schaller
  2017-05-23 13:10         ` Rob Herring
  0 siblings, 1 reply; 24+ messages in thread
From: H. Nikolaus Schaller @ 2017-05-23 12:48 UTC (permalink / raw)
  To: Rob Herring
  Cc: Mark Rutland, Benoît Cousson, Tony Lindgren, Russell King,
	Thierry Reding, Jonathan Cameron, Maxime Ripard, Jarkko Sakkinen,
	devicetree, linux-kernel, linux-omap,
	Discussions about the Letux Kernel, kernel

Hi Rob,

> Am 23.05.2017 um 14:28 schrieb Rob Herring <robh+dt@kernel.org>:
> 
> On Tue, May 23, 2017 at 12:43 AM, H. Nikolaus Schaller
> <hns@goldelico.com> wrote:
>> Hi Rob,
>> 
>>> Am 23.05.2017 um 04:26 schrieb Rob Herring <robh+dt@kernel.org>:
>>> 
>>> On Sun, May 21, 2017 at 5:44 AM, H. Nikolaus Schaller <hns@goldelico.com> wrote:
>>>> Since our proposed API was not acceptable and the new serdev API has arrived in 4.11 kernels,
>>>> we finally took the challenge to update the w2sg and w2cbw drivers to use the serdev API.
>>>> 
>>>> The approach is to write a "man in the middle" driver which is on one side a serdev client
>>>> which directly controls the UART where the device is connected to and on the other side
>>>> presents a new tty port so that user-space software can talk to the chips as if they would
>>>> directly talk to the UART of the SoC (e.g. ttyO1). This is similar to connecting to a remote
>>>> serial device e.g. through USB (ttyACM) or Bluetooth UART profiles.
>>>> 
>>>> For example gpsd or hciattach expect a /dev/tty they can control (flow control, baud rate
>>>> etc.).
>>> 
>>> I understand from the prior discussion why you want to pass the data
>>> thru for gps, but why do you need to do that for BT?
>> 
>> Because we otherwise can't turn on power when /dev/ttyBT0 is opened and turn off when it
>> is closed. I.e. it should not be powered unless someone does a hciattach /dev/ttyBT0. And it
>> should be turned off by a killall hciattach.
> 
> Still, you can do power control within BT HCI drivers.

We do not use any driver for bluetooth. We just start hciattach on demand.
And afaik there is no plugin mechanism for adding power control to hciattach.

Or do you have a link to what you think about?

> You wouldn't be
> limited to just open/close, but can handle suspend/resume as well.

Well, it does not look as if we need more than open/close since suspend/resume
is already handled by the regulator driver. We just need to keep it powered off
if there is no user-space client.

> 
>> Basically we would like to have a power control automatic like it exists for many other devices.
>> 
>> Since the BT chip is described as a serdev by DT, we see no other means than to pass data
>> through the serdev driver.
> 
> We could have a blacklist if we need to have serdev not create a
> device and create a tty device instead.

Interesting idea.

> 
>> We had looked into the line discipline approach but it makes a lot of problems. The first one
>> is that registering a new system-wide ldesc number is required. Next we do not see how to make
>> a serdev driver (as it seems to be required by the DT) to register a different ldesc.
>> 
>>> 
>>>> Here is the result of our first hack which is working as a demo on GTA04 devices (and the
>>>> w2cbw driver can also be used to control a GTA04 variant with WL1837).
>>>> 
>>>> Since it is just a demo hack, the code is not yet cleaned up, nor does it completely pass
>>>> check-patch, nor follows 100% the coding styles. And certainly has some bugs.
>>>> 
>>>> The most significant issue is that calling tty_port_register_device() inside of the
>>>> serdev probe() function makes the serdev probe() function to be entered a second
>>>> time. This does not lead to big problems since we currently have minor = 0
>>>> and this makes the second call assume the device is not available.
>>>> 
>>>> But we have no idea why this happens and how it can be prevented.
>>> 
>>> Johan's fixes may help there, but it is intended to be temporary to
>>> have a separate API for registering tty ports with or without serdev.
>> 
>> Ah, would that mean something like a tty_port_register_device_without_serdev()?
> 
> Yes, but other way around. The old function doesn't register with
> serdev and there's a new function that will.

Ah, ok.

> 
>> Do you have a reference to his fixes?
> 
> They are in Greg's tty-linus branch if not Linus' tree now.

Have found them. Not yet in Linus' tree but likely in one of the next rc

Looks good. This would mean that this problem will simply go away soon.

BR and thanks,
Nikolaus

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

* Re: [RFC 0/3] misc: new serdev based drivers for w2sg00x4 GPS module and w2cbw003 wifi/bluetooth
  2017-05-23 12:48       ` H. Nikolaus Schaller
@ 2017-05-23 13:10         ` Rob Herring
  2017-05-23 13:49           ` H. Nikolaus Schaller
  0 siblings, 1 reply; 24+ messages in thread
From: Rob Herring @ 2017-05-23 13:10 UTC (permalink / raw)
  To: H. Nikolaus Schaller
  Cc: Mark Rutland, Benoît Cousson, Tony Lindgren, Russell King,
	Thierry Reding, Jonathan Cameron, Maxime Ripard, Jarkko Sakkinen,
	devicetree, linux-kernel, linux-omap,
	Discussions about the Letux Kernel, kernel, Marcel Holtmann

+Marcel

On Tue, May 23, 2017 at 7:48 AM, H. Nikolaus Schaller <hns@goldelico.com> wrote:
> Hi Rob,
>
>> Am 23.05.2017 um 14:28 schrieb Rob Herring <robh+dt@kernel.org>:
>>
>> On Tue, May 23, 2017 at 12:43 AM, H. Nikolaus Schaller
>> <hns@goldelico.com> wrote:
>>> Hi Rob,
>>>
>>>> Am 23.05.2017 um 04:26 schrieb Rob Herring <robh+dt@kernel.org>:
>>>>
>>>> On Sun, May 21, 2017 at 5:44 AM, H. Nikolaus Schaller <hns@goldelico.com> wrote:
>>>>> Since our proposed API was not acceptable and the new serdev API has arrived in 4.11 kernels,
>>>>> we finally took the challenge to update the w2sg and w2cbw drivers to use the serdev API.
>>>>>
>>>>> The approach is to write a "man in the middle" driver which is on one side a serdev client
>>>>> which directly controls the UART where the device is connected to and on the other side
>>>>> presents a new tty port so that user-space software can talk to the chips as if they would
>>>>> directly talk to the UART of the SoC (e.g. ttyO1). This is similar to connecting to a remote
>>>>> serial device e.g. through USB (ttyACM) or Bluetooth UART profiles.
>>>>>
>>>>> For example gpsd or hciattach expect a /dev/tty they can control (flow control, baud rate
>>>>> etc.).
>>>>
>>>> I understand from the prior discussion why you want to pass the data
>>>> thru for gps, but why do you need to do that for BT?
>>>
>>> Because we otherwise can't turn on power when /dev/ttyBT0 is opened and turn off when it
>>> is closed. I.e. it should not be powered unless someone does a hciattach /dev/ttyBT0. And it
>>> should be turned off by a killall hciattach.
>>
>> Still, you can do power control within BT HCI drivers.
>
> We do not use any driver for bluetooth. We just start hciattach on demand.
> And afaik there is no plugin mechanism for adding power control to hciattach.

You don't need hciattach. All userspace has to do for kernel BT
drivers is "hciconfig hci0 up|down".

> Or do you have a link to what you think about?

Look at the nokia BT or TI (HCI_LL) BT drivers. Those both have f/w
downloading and some GPIO controls. Given that this module is based on
Marvell chipset, I'd expect you need to add serdev support to
hci_mrvl.c.

>> You wouldn't be
>> limited to just open/close, but can handle suspend/resume as well.
>
> Well, it does not look as if we need more than open/close since suspend/resume
> is already handled by the regulator driver. We just need to keep it powered off
> if there is no user-space client.

Okay.

Rob

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

* Re: [RFC 0/3] misc: new serdev based drivers for w2sg00x4 GPS module and w2cbw003 wifi/bluetooth
  2017-05-23 13:10         ` Rob Herring
@ 2017-05-23 13:49           ` H. Nikolaus Schaller
  2017-05-25 12:48             ` Rob Herring
  2017-05-25 17:05             ` Pavel Machek
  0 siblings, 2 replies; 24+ messages in thread
From: H. Nikolaus Schaller @ 2017-05-23 13:49 UTC (permalink / raw)
  To: Rob Herring
  Cc: Mark Rutland, Benoît Cousson, Tony Lindgren, Russell King,
	Thierry Reding, Jonathan Cameron, Maxime Ripard, Jarkko Sakkinen,
	devicetree, linux-kernel, linux-omap,
	Discussions about the Letux Kernel, kernel, Marcel Holtmann

Hi Rob,

> Am 23.05.2017 um 15:10 schrieb Rob Herring <robh+dt@kernel.org>:
> 
> +Marcel

Good!

> 
> On Tue, May 23, 2017 at 7:48 AM, H. Nikolaus Schaller <hns@goldelico.com> wrote:
>> Hi Rob,
>> 
>>> Am 23.05.2017 um 14:28 schrieb Rob Herring <robh+dt@kernel.org>:
>>> 
>>> On Tue, May 23, 2017 at 12:43 AM, H. Nikolaus Schaller
>>> <hns@goldelico.com> wrote:
>>>> Hi Rob,
>>>> 
>>>>> Am 23.05.2017 um 04:26 schrieb Rob Herring <robh+dt@kernel.org>:
>>>>> 
>>>>> On Sun, May 21, 2017 at 5:44 AM, H. Nikolaus Schaller <hns@goldelico.com> wrote:
>>>>>> Since our proposed API was not acceptable and the new serdev API has arrived in 4.11 kernels,
>>>>>> we finally took the challenge to update the w2sg and w2cbw drivers to use the serdev API.
>>>>>> 
>>>>>> The approach is to write a "man in the middle" driver which is on one side a serdev client
>>>>>> which directly controls the UART where the device is connected to and on the other side
>>>>>> presents a new tty port so that user-space software can talk to the chips as if they would
>>>>>> directly talk to the UART of the SoC (e.g. ttyO1). This is similar to connecting to a remote
>>>>>> serial device e.g. through USB (ttyACM) or Bluetooth UART profiles.
>>>>>> 
>>>>>> For example gpsd or hciattach expect a /dev/tty they can control (flow control, baud rate
>>>>>> etc.).
>>>>> 
>>>>> I understand from the prior discussion why you want to pass the data
>>>>> thru for gps, but why do you need to do that for BT?
>>>> 
>>>> Because we otherwise can't turn on power when /dev/ttyBT0 is opened and turn off when it
>>>> is closed. I.e. it should not be powered unless someone does a hciattach /dev/ttyBT0. And it
>>>> should be turned off by a killall hciattach.
>>> 
>>> Still, you can do power control within BT HCI drivers.
>> 
>> We do not use any driver for bluetooth. We just start hciattach on demand.
>> And afaik there is no plugin mechanism for adding power control to hciattach.
> 
> You don't need hciattach. All userspace has to do for kernel BT
> drivers is "hciconfig hci0 up|down".

Hm. Well:

root@letux:~# hciconfig hci0 up
Can't get device info: No such device
root@letux:~#

I wonder how I can tell hciconfig about the UART port if not by running hciattach /dev/ttyBT0?

> 
>> Or do you have a link to what you think about?
> 
> Look at the nokia BT or TI (HCI_LL) BT drivers. Those both have f/w
> downloading and some GPIO controls. Given that this module is based on
> Marvell chipset, I'd expect you need to add serdev support to
> hci_mrvl.c.

The w2cb003 has a Marvell WiFi (libertas) but a CSR Bluetooth side.

It has built-in firmware and already talks serial HCI over simple UART right
after power-on. This is why our serdev driver has no firmware download.

> 
>>> You wouldn't be
>>> limited to just open/close, but can handle suspend/resume as well.
>> 
>> Well, it does not look as if we need more than open/close since suspend/resume
>> is already handled by the regulator driver. We just need to keep it powered off
>> if there is no user-space client.
> 
> Okay.
> 
> Rob

BR,
Nikolaus

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

* Re: [RFC 0/3] misc: new serdev based drivers for w2sg00x4 GPS module and w2cbw003 wifi/bluetooth
  2017-05-23 13:49           ` H. Nikolaus Schaller
@ 2017-05-25 12:48             ` Rob Herring
  2017-06-06 18:50               ` H. Nikolaus Schaller
  2017-05-25 17:05             ` Pavel Machek
  1 sibling, 1 reply; 24+ messages in thread
From: Rob Herring @ 2017-05-25 12:48 UTC (permalink / raw)
  To: H. Nikolaus Schaller
  Cc: Mark Rutland, Benoît Cousson, Tony Lindgren, Russell King,
	Thierry Reding, Jonathan Cameron, Maxime Ripard, Jarkko Sakkinen,
	devicetree, linux-kernel, linux-omap,
	Discussions about the Letux Kernel, kernel, Marcel Holtmann

On Tue, May 23, 2017 at 8:49 AM, H. Nikolaus Schaller <hns@goldelico.com> wrote:
> Hi Rob,
>
>> Am 23.05.2017 um 15:10 schrieb Rob Herring <robh+dt@kernel.org>:
>>
>> +Marcel
>
> Good!
>
>>
>> On Tue, May 23, 2017 at 7:48 AM, H. Nikolaus Schaller <hns@goldelico.com> wrote:
>>> Hi Rob,
>>>
>>>> Am 23.05.2017 um 14:28 schrieb Rob Herring <robh+dt@kernel.org>:
>>>>
>>>> On Tue, May 23, 2017 at 12:43 AM, H. Nikolaus Schaller
>>>> <hns@goldelico.com> wrote:
>>>>> Hi Rob,
>>>>>
>>>>>> Am 23.05.2017 um 04:26 schrieb Rob Herring <robh+dt@kernel.org>:
>>>>>>
>>>>>> On Sun, May 21, 2017 at 5:44 AM, H. Nikolaus Schaller <hns@goldelico.com> wrote:
>>>>>>> Since our proposed API was not acceptable and the new serdev API has arrived in 4.11 kernels,
>>>>>>> we finally took the challenge to update the w2sg and w2cbw drivers to use the serdev API.
>>>>>>>
>>>>>>> The approach is to write a "man in the middle" driver which is on one side a serdev client
>>>>>>> which directly controls the UART where the device is connected to and on the other side
>>>>>>> presents a new tty port so that user-space software can talk to the chips as if they would
>>>>>>> directly talk to the UART of the SoC (e.g. ttyO1). This is similar to connecting to a remote
>>>>>>> serial device e.g. through USB (ttyACM) or Bluetooth UART profiles.
>>>>>>>
>>>>>>> For example gpsd or hciattach expect a /dev/tty they can control (flow control, baud rate
>>>>>>> etc.).
>>>>>>
>>>>>> I understand from the prior discussion why you want to pass the data
>>>>>> thru for gps, but why do you need to do that for BT?
>>>>>
>>>>> Because we otherwise can't turn on power when /dev/ttyBT0 is opened and turn off when it
>>>>> is closed. I.e. it should not be powered unless someone does a hciattach /dev/ttyBT0. And it
>>>>> should be turned off by a killall hciattach.
>>>>
>>>> Still, you can do power control within BT HCI drivers.
>>>
>>> We do not use any driver for bluetooth. We just start hciattach on demand.
>>> And afaik there is no plugin mechanism for adding power control to hciattach.
>>
>> You don't need hciattach. All userspace has to do for kernel BT
>> drivers is "hciconfig hci0 up|down".
>
> Hm. Well:
>
> root@letux:~# hciconfig hci0 up
> Can't get device info: No such device
> root@letux:~#
>
> I wonder how I can tell hciconfig about the UART port if not by running hciattach /dev/ttyBT0?

You don't create a tty device. Instead you call
hci_uart_register_device from the serdev probe.

>>> Or do you have a link to what you think about?
>>
>> Look at the nokia BT or TI (HCI_LL) BT drivers. Those both have f/w
>> downloading and some GPIO controls. Given that this module is based on
>> Marvell chipset, I'd expect you need to add serdev support to
>> hci_mrvl.c.
>
> The w2cb003 has a Marvell WiFi (libertas) but a CSR Bluetooth side.
>
> It has built-in firmware and already talks serial HCI over simple UART right
> after power-on. This is why our serdev driver has no firmware download.

Okay, then probably add serdev support to hci_bcsp. In any case,
hci_uart_register_device just needs to register the correct protocol.

Rob

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

* Re: [RFC 0/3] misc: new serdev based drivers for w2sg00x4 GPS module and w2cbw003 wifi/bluetooth
  2017-05-23 13:49           ` H. Nikolaus Schaller
  2017-05-25 12:48             ` Rob Herring
@ 2017-05-25 17:05             ` Pavel Machek
  1 sibling, 0 replies; 24+ messages in thread
From: Pavel Machek @ 2017-05-25 17:05 UTC (permalink / raw)
  To: H. Nikolaus Schaller
  Cc: Rob Herring, Mark Rutland, Benoît Cousson, Tony Lindgren,
	Russell King, Thierry Reding, Jonathan Cameron, Maxime Ripard,
	Jarkko Sakkinen, devicetree, linux-kernel, linux-omap,
	Discussions about the Letux Kernel, kernel, Marcel Holtmann

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

Hi!

> > You don't need hciattach. All userspace has to do for kernel BT
> > drivers is "hciconfig hci0 up|down".
> 
> Hm. Well:
> 
> root@letux:~# hciconfig hci0 up
> Can't get device info: No such device
> root@letux:~#
> 
> I wonder how I can tell hciconfig about the UART port if not by running hciattach /dev/ttyBT0?

You should not have to tell hciconfig about ttyBT0. Kernel should
automatically setup hci0<->ttyBT0 connection during boot, based on the
device tree.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

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

* Re: [RFC 2/3] misc: Add w2sg0004 (gps receiver) power control driver
@ 2017-05-30 23:09     ` Rob Herring
  0 siblings, 0 replies; 24+ messages in thread
From: Rob Herring @ 2017-05-30 23:09 UTC (permalink / raw)
  To: H. Nikolaus Schaller
  Cc: Mark Rutland, Benoît Cousson, Tony Lindgren, Russell King,
	Thierry Reding, Jonathan Cameron, Maxime Ripard, Jarkko Sakkinen,
	devicetree, linux-kernel, linux-omap, letux-kernel, kernel

On Sun, May 21, 2017 at 12:44:03PM +0200, H. Nikolaus Schaller wrote:
> Add driver for Wi2Wi W2SG0004/84 GPS module connected through uart.
> 
> Use serdev API hooks to monitor and forward the UART traffic to /dev/BTn
> and turn on/off the module. It also detects if the module is turned on (sends data)
> but should be off, e.g. if it was already turned on during boot or power-on-reset.
> 
> 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 new serdev API introduced in 4.11.
> 
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> ---
>  .../devicetree/bindings/misc/wi2wi,w2sg0004.txt    |  20 +
>  .../devicetree/bindings/vendor-prefixes.txt        |   1 +

Please split binding patches.

>  drivers/misc/Kconfig                               |  16 +
>  drivers/misc/Makefile                              |   1 +
>  drivers/misc/w2sg0004.c                            | 646 +++++++++++++++++++++
>  include/linux/w2sg0004.h                           |  27 +
>  6 files changed, 711 insertions(+)
>  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 000000000000..b7125c7a598c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
> @@ -0,0 +1,20 @@
> +Wi2Wi GPS module connected through UART
> +
> +Should be a subnode of the SoC UART it is connected to (serdev).
> +
> +Required properties:
> +- compatible:	wi2wi,w2sg0004 or wi2wi,w2sg0084

Reformat to one per line.

> +- on-off-gpio:	the GPIO that controls the module's on-off toggle input

Does enable-gpios or powerdown-gpios work as those are semi-standard 
names. Also, need to state the active state.

> +
> +Optional properties:
> +- lna-suppy:	an (optional) LNA regulator that is enabled together with the GPS receiver

typo

> +
> +Example:
> +
> +&uart2 {
> +	gps: w2sg0004 {

w2sg0004: gps {

The node name should be generic. The label can be whatever you like.

> +		compatible = "wi2wi,w2sg0004";
> +		lna-supply = <&vsim>;   /* LNA regulator */
> +		on-off-gpios = <&gpio5 17 GPIO_ACTIVE_HIGH>;    /* GPIO_145: trigger for turning on/off w2sg0004 */
> +        };
> +};
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
> index c03d20140366..c56b3181b266 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> @@ -345,6 +345,7 @@ voipac	Voipac Technologies s.r.o.
>  wd	Western Digital Corp.
>  wetek	WeTek Electronics, limited.
>  wexler	Wexler
> +wi2wi	Wi2Wi, Inc.
>  winbond Winbond Electronics corp.
>  winstar	Winstar Display Corp.
>  wlf	Wolfson Microelectronics

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

* Re: [RFC 2/3] misc: Add w2sg0004 (gps receiver) power control driver
@ 2017-05-30 23:09     ` Rob Herring
  0 siblings, 0 replies; 24+ messages in thread
From: Rob Herring @ 2017-05-30 23:09 UTC (permalink / raw)
  To: H. Nikolaus Schaller
  Cc: Mark Rutland, Benoît Cousson, Tony Lindgren, Russell King,
	Thierry Reding, Jonathan Cameron, Maxime Ripard, Jarkko Sakkinen,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	letux-kernel-S0jZdbWzriLCfDggNXIi3w,
	kernel-Jl6IXVxNIMRxAtABVqVhTwC/G2K4zDHf

On Sun, May 21, 2017 at 12:44:03PM +0200, H. Nikolaus Schaller wrote:
> Add driver for Wi2Wi W2SG0004/84 GPS module connected through uart.
> 
> Use serdev API hooks to monitor and forward the UART traffic to /dev/BTn
> and turn on/off the module. It also detects if the module is turned on (sends data)
> but should be off, e.g. if it was already turned on during boot or power-on-reset.
> 
> 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 new serdev API introduced in 4.11.
> 
> Signed-off-by: H. Nikolaus Schaller <hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>
> ---
>  .../devicetree/bindings/misc/wi2wi,w2sg0004.txt    |  20 +
>  .../devicetree/bindings/vendor-prefixes.txt        |   1 +

Please split binding patches.

>  drivers/misc/Kconfig                               |  16 +
>  drivers/misc/Makefile                              |   1 +
>  drivers/misc/w2sg0004.c                            | 646 +++++++++++++++++++++
>  include/linux/w2sg0004.h                           |  27 +
>  6 files changed, 711 insertions(+)
>  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 000000000000..b7125c7a598c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
> @@ -0,0 +1,20 @@
> +Wi2Wi GPS module connected through UART
> +
> +Should be a subnode of the SoC UART it is connected to (serdev).
> +
> +Required properties:
> +- compatible:	wi2wi,w2sg0004 or wi2wi,w2sg0084

Reformat to one per line.

> +- on-off-gpio:	the GPIO that controls the module's on-off toggle input

Does enable-gpios or powerdown-gpios work as those are semi-standard 
names. Also, need to state the active state.

> +
> +Optional properties:
> +- lna-suppy:	an (optional) LNA regulator that is enabled together with the GPS receiver

typo

> +
> +Example:
> +
> +&uart2 {
> +	gps: w2sg0004 {

w2sg0004: gps {

The node name should be generic. The label can be whatever you like.

> +		compatible = "wi2wi,w2sg0004";
> +		lna-supply = <&vsim>;   /* LNA regulator */
> +		on-off-gpios = <&gpio5 17 GPIO_ACTIVE_HIGH>;    /* GPIO_145: trigger for turning on/off w2sg0004 */
> +        };
> +};
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
> index c03d20140366..c56b3181b266 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> @@ -345,6 +345,7 @@ voipac	Voipac Technologies s.r.o.
>  wd	Western Digital Corp.
>  wetek	WeTek Electronics, limited.
>  wexler	Wexler
> +wi2wi	Wi2Wi, Inc.
>  winbond Winbond Electronics corp.
>  winstar	Winstar Display Corp.
>  wlf	Wolfson Microelectronics
--
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] 24+ messages in thread

* Re: [RFC 2/3] misc: Add w2sg0004 (gps receiver) power control driver
  2017-05-30 23:09     ` Rob Herring
  (?)
@ 2017-06-06 18:50     ` H. Nikolaus Schaller
  -1 siblings, 0 replies; 24+ messages in thread
From: H. Nikolaus Schaller @ 2017-06-06 18:50 UTC (permalink / raw)
  To: Rob Herring
  Cc: Mark Rutland, Benoît Cousson, Tony Lindgren, Russell King,
	Thierry Reding, Jonathan Cameron, Maxime Ripard, Jarkko Sakkinen,
	devicetree, linux-kernel, linux-omap, letux-kernel, kernel


> Am 31.05.2017 um 01:09 schrieb Rob Herring <robh@kernel.org>:
> 
> On Sun, May 21, 2017 at 12:44:03PM +0200, H. Nikolaus Schaller wrote:
>> Add driver for Wi2Wi W2SG0004/84 GPS module connected through uart.
>> 
>> Use serdev API hooks to monitor and forward the UART traffic to /dev/BTn
>> and turn on/off the module. It also detects if the module is turned on (sends data)
>> but should be off, e.g. if it was already turned on during boot or power-on-reset.
>> 
>> 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 new serdev API introduced in 4.11.
>> 
>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>> ---
>> .../devicetree/bindings/misc/wi2wi,w2sg0004.txt    |  20 +
>> .../devicetree/bindings/vendor-prefixes.txt        |   1 +
> 
> Please split binding patches.

Yes, will do.

We thought that it gives better understanding if kept together with the code in a single patch for this RFC.

> 
>> drivers/misc/Kconfig                               |  16 +
>> drivers/misc/Makefile                              |   1 +
>> drivers/misc/w2sg0004.c                            | 646 +++++++++++++++++++++
>> include/linux/w2sg0004.h                           |  27 +
>> 6 files changed, 711 insertions(+)
>> 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 000000000000..b7125c7a598c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
>> @@ -0,0 +1,20 @@
>> +Wi2Wi GPS module connected through UART
>> +
>> +Should be a subnode of the SoC UART it is connected to (serdev).
>> +
>> +Required properties:
>> +- compatible:	wi2wi,w2sg0004 or wi2wi,w2sg0084
> 
> Reformat to one per line.

Ok.

> 
>> +- on-off-gpio:	the GPIO that controls the module's on-off toggle input
> 
> Does enable-gpios or powerdown-gpios work as those are semi-standard 
> names. Also, need to state the active state.

Ok.

> 
>> +
>> +Optional properties:
>> +- lna-suppy:	an (optional) LNA regulator that is enabled together with the GPS receiver
> 
> typo

tnx :)

> 
>> +
>> +Example:
>> +
>> +&uart2 {
>> +	gps: w2sg0004 {
> 
> w2sg0004: gps {
> 
> The node name should be generic. The label can be whatever you like.

Ok. That comes from thoughtless copying very old stuff (I think we started with trying to get this chip DT based around 3.14 kernels...)

> 
>> +		compatible = "wi2wi,w2sg0004";
>> +		lna-supply = <&vsim>;   /* LNA regulator */
>> +		on-off-gpios = <&gpio5 17 GPIO_ACTIVE_HIGH>;    /* GPIO_145: trigger for turning on/off w2sg0004 */
>> +        };
>> +};
>> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
>> index c03d20140366..c56b3181b266 100644
>> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
>> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
>> @@ -345,6 +345,7 @@ voipac	Voipac Technologies s.r.o.
>> wd	Western Digital Corp.
>> wetek	WeTek Electronics, limited.
>> wexler	Wexler
>> +wi2wi	Wi2Wi, Inc.
>> winbond Winbond Electronics corp.
>> winstar	Winstar Display Corp.
>> wlf	Wolfson Microelectronics

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

* Re: [RFC 0/3] misc: new serdev based drivers for w2sg00x4 GPS module and w2cbw003 wifi/bluetooth
  2017-05-25 12:48             ` Rob Herring
@ 2017-06-06 18:50               ` H. Nikolaus Schaller
  0 siblings, 0 replies; 24+ messages in thread
From: H. Nikolaus Schaller @ 2017-06-06 18:50 UTC (permalink / raw)
  To: Rob Herring
  Cc: Mark Rutland, Benoît Cousson, Tony Lindgren, Russell King,
	Thierry Reding, Jonathan Cameron, Maxime Ripard, Jarkko Sakkinen,
	devicetree, linux-kernel, linux-omap,
	Discussions about the Letux Kernel, kernel, Marcel Holtmann

Hi Rob,
thanks for all the comments.

> Am 25.05.2017 um 14:48 schrieb Rob Herring <robh+dt@kernel.org>:
> 
> On Tue, May 23, 2017 at 8:49 AM, H. Nikolaus Schaller <hns@goldelico.com> wrote:
>> Hi Rob,
>> 
>>> Am 23.05.2017 um 15:10 schrieb Rob Herring <robh+dt@kernel.org>:
>>> 
>>> +Marcel
>> 
>> Good!
>> 
>> Hm. Well:
>> 
>> root@letux:~# hciconfig hci0 up
>> Can't get device info: No such device
>> root@letux:~#
>> 
>> I wonder how I can tell hciconfig about the UART port if not by running hciattach /dev/ttyBT0?
> 
> You don't create a tty device. Instead you call
> hci_uart_register_device from the serdev probe.

I see. That is also brand new API in 4.12.

> 
>>>> Or do you have a link to what you think about?
>>> 
>>> Look at the nokia BT or TI (HCI_LL) BT drivers. Those both have f/w
>>> downloading and some GPIO controls. Given that this module is based on
>>> Marvell chipset, I'd expect you need to add serdev support to
>>> hci_mrvl.c.
>> 
>> The w2cb003 has a Marvell WiFi (libertas) but a CSR Bluetooth side.
>> 
>> It has built-in firmware and already talks serial HCI over simple UART right
>> after power-on. This is why our serdev driver has no firmware download.
> 
> Okay, then probably add serdev support to hci_bcsp. In any case,
> hci_uart_register_device just needs to register the correct protocol.

Hm. I have tried to understand the hci_bcsp stuff but I must admit that we lack
quick enough understanding of such magic, to be able to add serdev support with
reasonable efforts. Is there a good description how hci_bcsp it works?

And I found:

	  BCSP (BlueCore Serial Protocol) is serial protocol for communication 
	  between Bluetooth device and host. This protocol is required for non
	  USB Bluetooth devices based on CSR BlueCore chip, including PCMCIA and
	  CF cards.

It appears as if the w2cbw003 bluetooth is really based on a BlueCore chip
(there are 2 references in http://www.wi2wi.com/mgr/docs/W2CBW003_Datasheet_Rev1.8.pdf ).

Therefore it might be possible to use BCSP but that are completely new horizons.
But so far we have simply used "hciattach any" mode for years.

We also know that the Gumstix Storm uses the same w2cbw wifi/bluetooth combo
(omap3-overo-base.dtsi) but there also appears to be no BCSP support in the kernel.

BTW: it looks as if they simply turn on bluetooth as soon as WiFi is configured up. So they
could be interested in a more precise power management driver like what we propose.

Maybe someone who owns an older Gumstix with the same chip (they were phased out in 2015
and replaced by TI modules) can help or jump into this discussion.

Thanks and BR,
Nikolaus

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

* Re: [RFC 2/3] misc: Add w2sg0004 (gps receiver) power control driver
  2017-05-21 10:44 ` [RFC 2/3] misc: Add w2sg0004 (gps receiver) power control driver H. Nikolaus Schaller
  2017-05-21 10:48     ` H. Nikolaus Schaller
  2017-05-30 23:09     ` Rob Herring
@ 2017-06-07  7:34   ` Ladislav Michl
  2017-06-07 10:08       ` H. Nikolaus Schaller
  2 siblings, 1 reply; 24+ messages in thread
From: Ladislav Michl @ 2017-06-07  7:34 UTC (permalink / raw)
  To: H. Nikolaus Schaller
  Cc: Rob Herring, Mark Rutland, Benoît Cousson, Tony Lindgren,
	Russell King, Thierry Reding, Jonathan Cameron, Maxime Ripard,
	Jarkko Sakkinen, devicetree, linux-kernel, linux-omap,
	letux-kernel, kernel

Dear Nikolaus,

On Sun, May 21, 2017 at 12:44:03PM +0200, H. Nikolaus Schaller wrote:
> Add driver for Wi2Wi W2SG0004/84 GPS module connected through uart.
> 
> Use serdev API hooks to monitor and forward the UART traffic to /dev/BTn
> and turn on/off the module. It also detects if the module is turned on (sends data)
> but should be off, e.g. if it was already turned on during boot or power-on-reset.
> 
> 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 new serdev API introduced in 4.11.
[snip]
> diff --git a/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt b/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
> new file mode 100644
> index 000000000000..b7125c7a598c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
> @@ -0,0 +1,20 @@
> +Wi2Wi GPS module connected through UART
> +
> +Should be a subnode of the SoC UART it is connected to (serdev).
> +
> +Required properties:
> +- compatible:	wi2wi,w2sg0004 or wi2wi,w2sg0084
> +- on-off-gpio:	the GPIO that controls the module's on-off toggle input
> +
> +Optional properties:
> +- lna-suppy:	an (optional) LNA regulator that is enabled together with the GPS receiver
> +
> +Example:
> +
> +&uart2 {
> +	gps: w2sg0004 {
> +		compatible = "wi2wi,w2sg0004";
> +		lna-supply = <&vsim>;   /* LNA regulator */
> +		on-off-gpios = <&gpio5 17 GPIO_ACTIVE_HIGH>;    /* GPIO_145: trigger for turning on/off w2sg0004 */
> +        };
> +};

Here it seems is nothing specific to GPS receiver in this driver and
basically UART connected GSM modules could benefit from such functionality
too - as they need power supply, had reset and ignition pin and often
status pin. See for example:
http://simcom.ee/documents/SIM5300E/SIM5300E_Hardware_Design_V1.08.pdf
page 14 or
http://www.robotshop.com/media/files/pdf/datasheet-gsm-tc35.pdf
page 19 for block diagrams.
So I wonder if you would accept making this driver a bit more generic
to cover also GSM modems use case.

Best regards,
	ladis

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

* Re: [RFC 2/3] misc: Add w2sg0004 (gps receiver) power control driver
  2017-06-07  7:34   ` Ladislav Michl
@ 2017-06-07 10:08       ` H. Nikolaus Schaller
  0 siblings, 0 replies; 24+ messages in thread
From: H. Nikolaus Schaller @ 2017-06-07 10:08 UTC (permalink / raw)
  To: Ladislav Michl
  Cc: Rob Herring, Mark Rutland, Benoît Cousson, Tony Lindgren,
	Russell King, Thierry Reding, Jonathan Cameron, Maxime Ripard,
	Jarkko Sakkinen, devicetree, linux-kernel, linux-omap,
	letux-kernel, kernel

Hi Ladislav,

> Am 07.06.2017 um 09:34 schrieb Ladislav Michl <ladis@linux-mips.org>:
> 
> Dear Nikolaus,
> 
> On Sun, May 21, 2017 at 12:44:03PM +0200, H. Nikolaus Schaller wrote:
>> Add driver for Wi2Wi W2SG0004/84 GPS module connected through uart.
>> 
>> Use serdev API hooks to monitor and forward the UART traffic to /dev/BTn
>> and turn on/off the module. It also detects if the module is turned on (sends data)
>> but should be off, e.g. if it was already turned on during boot or power-on-reset.
>> 
>> 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 new serdev API introduced in 4.11.
> [snip]
>> diff --git a/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt b/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
>> new file mode 100644
>> index 000000000000..b7125c7a598c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
>> @@ -0,0 +1,20 @@
>> +Wi2Wi GPS module connected through UART
>> +
>> +Should be a subnode of the SoC UART it is connected to (serdev).
>> +
>> +Required properties:
>> +- compatible:	wi2wi,w2sg0004 or wi2wi,w2sg0084
>> +- on-off-gpio:	the GPIO that controls the module's on-off toggle input
>> +
>> +Optional properties:
>> +- lna-suppy:	an (optional) LNA regulator that is enabled together with the GPS receiver
>> +
>> +Example:
>> +
>> +&uart2 {
>> +	gps: w2sg0004 {
>> +		compatible = "wi2wi,w2sg0004";
>> +		lna-supply = <&vsim>;   /* LNA regulator */
>> +		on-off-gpios = <&gpio5 17 GPIO_ACTIVE_HIGH>;    /* GPIO_145: trigger for turning on/off w2sg0004 */
>> +        };
>> +};
> 
> Here it seems is nothing specific to GPS receiver in this driver

Well, most serdev subnodes will look similar to this but still will
need different drivers (timing, pulses, other logic, special functions,
firmware download etc.).

> and
> basically UART connected GSM modules could benefit from such functionality
> too - as they need power supply, had reset and ignition pin and often
> status pin. See for example:
> http://simcom.ee/documents/SIM5300E/SIM5300E_Hardware_Design_V1.08.pdf
> page 14 or
> http://www.robotshop.com/media/files/pdf/datasheet-gsm-tc35.pdf
> page 19 for block diagrams.
> So I wonder if you would accept making this driver a bit more generic
> to cover also GSM modems use case.

We also have such a 2G/3G/4G module in the GTA04 and Pyra devices but there
we have significantly different power control needs because they are connected
through USB and not as UART / serdev.

But you could simply try to use and extend our proposed w2cbw003 bluetooth
driver [RFC 3/3] for that purpose.

It already has most of what you need: provides a user-space /dev/tty for the
serdev and controls some power-control gpio or regulator.

This w2sg0004 gps chip is very specific for this chip since it has no simple
power-enable to be controlled. This also differs too much from GSM modules
with ignition. Especially it has no status feedback.

Another aspect is that there are plans to have some gps infrastructure in the
kernel. Then, this driver wouldn't be the right solution for gsm.

Therefore it is and should remain a special purpose driver for this type of
gps chips.

The common denominator of all these things is the serdev and gpio infrastructure.

Feel free to take both drivers as blueprints to propose some new driver
for your chips.

BR,
Nikolaus

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

* Re: [RFC 2/3] misc: Add w2sg0004 (gps receiver) power control driver
@ 2017-06-07 10:08       ` H. Nikolaus Schaller
  0 siblings, 0 replies; 24+ messages in thread
From: H. Nikolaus Schaller @ 2017-06-07 10:08 UTC (permalink / raw)
  To: Ladislav Michl
  Cc: Rob Herring, Mark Rutland, Benoît Cousson, Tony Lindgren,
	Russell King, Thierry Reding, Jonathan Cameron, Maxime Ripard,
	Jarkko Sakkinen, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	letux-kernel-S0jZdbWzriLCfDggNXIi3w,
	kernel-Jl6IXVxNIMRxAtABVqVhTwC/G2K4zDHf

Hi Ladislav,

> Am 07.06.2017 um 09:34 schrieb Ladislav Michl <ladis-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org>:
> 
> Dear Nikolaus,
> 
> On Sun, May 21, 2017 at 12:44:03PM +0200, H. Nikolaus Schaller wrote:
>> Add driver for Wi2Wi W2SG0004/84 GPS module connected through uart.
>> 
>> Use serdev API hooks to monitor and forward the UART traffic to /dev/BTn
>> and turn on/off the module. It also detects if the module is turned on (sends data)
>> but should be off, e.g. if it was already turned on during boot or power-on-reset.
>> 
>> 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 new serdev API introduced in 4.11.
> [snip]
>> diff --git a/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt b/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
>> new file mode 100644
>> index 000000000000..b7125c7a598c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
>> @@ -0,0 +1,20 @@
>> +Wi2Wi GPS module connected through UART
>> +
>> +Should be a subnode of the SoC UART it is connected to (serdev).
>> +
>> +Required properties:
>> +- compatible:	wi2wi,w2sg0004 or wi2wi,w2sg0084
>> +- on-off-gpio:	the GPIO that controls the module's on-off toggle input
>> +
>> +Optional properties:
>> +- lna-suppy:	an (optional) LNA regulator that is enabled together with the GPS receiver
>> +
>> +Example:
>> +
>> +&uart2 {
>> +	gps: w2sg0004 {
>> +		compatible = "wi2wi,w2sg0004";
>> +		lna-supply = <&vsim>;   /* LNA regulator */
>> +		on-off-gpios = <&gpio5 17 GPIO_ACTIVE_HIGH>;    /* GPIO_145: trigger for turning on/off w2sg0004 */
>> +        };
>> +};
> 
> Here it seems is nothing specific to GPS receiver in this driver

Well, most serdev subnodes will look similar to this but still will
need different drivers (timing, pulses, other logic, special functions,
firmware download etc.).

> and
> basically UART connected GSM modules could benefit from such functionality
> too - as they need power supply, had reset and ignition pin and often
> status pin. See for example:
> http://simcom.ee/documents/SIM5300E/SIM5300E_Hardware_Design_V1.08.pdf
> page 14 or
> http://www.robotshop.com/media/files/pdf/datasheet-gsm-tc35.pdf
> page 19 for block diagrams.
> So I wonder if you would accept making this driver a bit more generic
> to cover also GSM modems use case.

We also have such a 2G/3G/4G module in the GTA04 and Pyra devices but there
we have significantly different power control needs because they are connected
through USB and not as UART / serdev.

But you could simply try to use and extend our proposed w2cbw003 bluetooth
driver [RFC 3/3] for that purpose.

It already has most of what you need: provides a user-space /dev/tty for the
serdev and controls some power-control gpio or regulator.

This w2sg0004 gps chip is very specific for this chip since it has no simple
power-enable to be controlled. This also differs too much from GSM modules
with ignition. Especially it has no status feedback.

Another aspect is that there are plans to have some gps infrastructure in the
kernel. Then, this driver wouldn't be the right solution for gsm.

Therefore it is and should remain a special purpose driver for this type of
gps chips.

The common denominator of all these things is the serdev and gpio infrastructure.

Feel free to take both drivers as blueprints to propose some new driver
for your chips.

BR,
Nikolaus

--
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] 24+ messages in thread

end of thread, other threads:[~2017-06-07 10:08 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-21 10:44 [RFC 0/3] misc: new serdev based drivers for w2sg00x4 GPS module and w2cbw003 wifi/bluetooth H. Nikolaus Schaller
2017-05-21 10:44 ` [RFC 1/3] DTS: gta04: add serdev nodes for w2sg00x4, w2cbw etc H. Nikolaus Schaller
2017-05-21 10:44   ` H. Nikolaus Schaller
2017-05-21 10:44 ` [RFC 2/3] misc: Add w2sg0004 (gps receiver) power control driver H. Nikolaus Schaller
2017-05-21 10:48   ` H. Nikolaus Schaller
2017-05-21 10:48     ` H. Nikolaus Schaller
2017-05-30 23:09   ` Rob Herring
2017-05-30 23:09     ` Rob Herring
2017-06-06 18:50     ` H. Nikolaus Schaller
2017-06-07  7:34   ` Ladislav Michl
2017-06-07 10:08     ` H. Nikolaus Schaller
2017-06-07 10:08       ` H. Nikolaus Schaller
2017-05-21 10:44 ` [RFC 3/3] misc: Add w2cbw003 (wifi/bluetooth) " H. Nikolaus Schaller
2017-05-23  2:26 ` [RFC 0/3] misc: new serdev based drivers for w2sg00x4 GPS module and w2cbw003 wifi/bluetooth Rob Herring
2017-05-23  2:26   ` Rob Herring
2017-05-23  5:43   ` H. Nikolaus Schaller
2017-05-23  5:43     ` H. Nikolaus Schaller
2017-05-23 12:28     ` Rob Herring
2017-05-23 12:48       ` H. Nikolaus Schaller
2017-05-23 13:10         ` Rob Herring
2017-05-23 13:49           ` H. Nikolaus Schaller
2017-05-25 12:48             ` Rob Herring
2017-06-06 18:50               ` H. Nikolaus Schaller
2017-05-25 17:05             ` Pavel Machek

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.