Discussion:
[PATCH 1/2] misc: Add Wi2Wi w2sc0004 gps driver
Marek Belisko
2014-10-16 20:26:22 UTC
Permalink
This is a driver for the Wi2Wi GPS modules connected through an UART.
The tricky part is that the module is turned on or off by an impulse
on the control line - but it is only possible to get the real state by monitoring
the UART RX input. Since the kernel can't know in which status the module
is brought e.g. by a boot loader or after suspend, we need some logic to handle
this.

The driver allows two different methods to enable (and power up) GPS:
a) through rfkill
b) as a GPIO

The GPIO enable can be plumbed to other drivers that expect to be able to control
a GPIO. On the GTA04 board this is the DTR-gpio of the connected UART so that
opening the UART device enables the receiver and closing it shuts the receiver down.

Original implementation by Neil Brown, fixes + DT bindings by H. Nikolaus Schaller

Signed-off-by: NeilBrown <***@suse.de>
Signed-off-by: H. Nikolaus Schaller <***@goldelico.com>
Signed-off-by: Marek Belisko <***@goldelico.com>
---
drivers/misc/Kconfig | 10 +
drivers/misc/Makefile | 1 +
drivers/misc/w2sg0004.c | 512 +++++++++++++++++++++++++++++++++++++++++++++++
include/linux/w2sg0004.h | 23 +++
4 files changed, 546 insertions(+)
create mode 100644 drivers/misc/w2sg0004.c
create mode 100644 include/linux/w2sg0004.h

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index bbeb451..03af633 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -515,6 +515,16 @@ config VEXPRESS_SYSCFG
bus. System Configuration interface is one of the possible means
of generating transactions on this bus.

+config W2SG0004
+ tristate "W2SG0004 on/off control"
+ depends on GPIOLIB
+ help
+ Enable on/off control of W2SG0004 GPS using a virtual GPIO.
+ The virtual GPIO can be connected to a DTR line of a serial
+ interface to allow powering up if the /dev/tty$n is opened.
+ It also provides a rfkill gps node to control the LNA power.
+ NOTE: can't currently be compiled as module, so please choose Y.
+
source "drivers/misc/c2port/Kconfig"
source "drivers/misc/eeprom/Kconfig"
source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 7d5c4cd..2f83270 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -56,3 +56,4 @@ obj-$(CONFIG_GENWQE) += genwqe/
obj-$(CONFIG_ECHO) += echo/
obj-$(CONFIG_VEXPRESS_SYSCFG) += vexpress-syscfg.o
obj-$(CONFIG_CXL_BASE) += cxl/
+obj-${CONFIG_W2SG0004} += w2sg0004.o
diff --git a/drivers/misc/w2sg0004.c b/drivers/misc/w2sg0004.c
new file mode 100644
index 0000000..8f0afcf
--- /dev/null
+++ b/drivers/misc/w2sg0004.c
@@ -0,0 +1,512 @@
+/*
+ * w2sg0004.c
+ * Virtual GPIO of controlling the w2sg0004 GPS receiver.
+ *
+ * Copyright (C) 2011 Neil Brown <***@brown.name>
+ *
+ * This receiver has an ON/OFF pin which must be toggled to
+ * turn the device 'on' or 'off'. A high->low->high toggle
+ * will switch the device on if it is off, and off if it is on.
+ * 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.
+ * On the OMAP3, the UART line can also be programmed as a GPIO
+ * on which we can receive interrupts.
+ * So when we want the device to be 'off' we can reprogram
+ * the line, toggle the ON/OFF pin and hope that it is off.
+ * However if an interrupt arrives we know that it is really on
+ * and can toggle again.
+ *
+ * To enable receiving on/off requests we create a gpio_chip
+ * with a single 'output' GPIO. When it is low, the
+ * GPS is turned off. When it is high, it is turned on.
+ * This can be configured as the DTR GPIO on the UART which
+ * connects the GPS. Then whenever the tty is open, the GPS
+ * will be switched on, and whenever it is closed, the GPS will
+ * be switched off.
+ *
+ * In addition we register as a rfkill client so that we can
+ * control the LNA power.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/sched.h>
+#include <linux/irq.h>
+#include <linux/slab.h>
+#include <linux/err.h>
+#include <linux/delay.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/gpio.h>
+#include <linux/of_gpio.h>
+#include <linux/platform_device.h>
+#include <linux/w2sg0004.h>
+#include <linux/workqueue.h>
+#include <linux/rfkill.h>
+#include <linux/pinctrl/consumer.h>
+
+/*
+ * There seems to 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 /* desctivate on/off impulse */
+};
+
+struct gpio_w2sg {
+ 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;
+ int rx_irq;
+
+ struct pinctrl *p;
+ struct pinctrl_state *default_state; /* should be UART mode */
+ struct pinctrl_state *monitor_state; /* monitor RX as GPIO */
+ enum w2sg_state state;
+ int requested; /* requested state (0/1) */
+ int suspended;
+ int rx_redirected;
+ spinlock_t lock;
+#ifdef CONFIG_GPIOLIB
+ struct gpio_chip gpio;
+ const char *gpio_name[1];
+#endif
+ struct delayed_work work;
+};
+
+static int gpio_w2sg_set_lna_power(struct gpio_w2sg *gw2sg)
+{
+ int ret = 0;
+ int off = gw2sg->suspended || !gw2sg->requested || gw2sg->lna_blocked;
+
+ pr_debug("gpio_w2sg_set_lna_power: %s\n", off ? "off" : "on");
+
+ if (off != gw2sg->lna_is_off) {
+ gw2sg->lna_is_off = off;
+ if (!IS_ERR_OR_NULL(gw2sg->lna_regulator)) {
+ if (off)
+ regulator_disable(gw2sg->lna_regulator);
+ else
+ ret = regulator_enable(gw2sg->lna_regulator);
+ }
+ }
+
+ return ret;
+}
+
+static void toggle_work(struct work_struct *work)
+{
+ struct gpio_w2sg *gw2sg = container_of(work, struct gpio_w2sg,
+ work.work);
+
+ gpio_w2sg_set_lna_power(gw2sg); /* update LNA power state */
+
+ switch (gw2sg->state) {
+ case W2SG_NOPULSE:
+ gw2sg->state = W2SG_IDLE;
+ pr_debug("GPS idle\n");
+ break;
+
+ case W2SG_IDLE:
+ spin_lock_irq(&gw2sg->lock);
+ if (gw2sg->requested == gw2sg->is_on) {
+ if (!gw2sg->is_on && !gw2sg->rx_redirected) {
+ /* not yet redirected in off state */
+ gw2sg->rx_redirected = 1;
+ pinctrl_select_state(gw2sg->p,
+ gw2sg->monitor_state);
+ enable_irq(gw2sg->rx_irq);
+ }
+ spin_unlock_irq(&gw2sg->lock);
+ return;
+ }
+ spin_unlock_irq(&gw2sg->lock);
+ gpio_set_value_cansleep(gw2sg->on_off_gpio, 0);
+ gw2sg->state = W2SG_PULSE;
+
+ pr_debug("GPS pulse\n");
+
+ schedule_delayed_work(&gw2sg->work,
+ msecs_to_jiffies(10));
+ break;
+
+ case W2SG_PULSE:
+ gpio_set_value_cansleep(gw2sg->on_off_gpio, 1);
+ gw2sg->last_toggle = jiffies;
+ gw2sg->state = W2SG_NOPULSE;
+
+ pr_debug("GPS nopulse\n");
+
+ gw2sg->is_on = !gw2sg->is_on;
+ schedule_delayed_work(&gw2sg->work,
+ msecs_to_jiffies(10));
+ break;
+ }
+}
+
+static irqreturn_t gpio_w2sg_isr(int irq, void *dev_id)
+{
+ struct gpio_w2sg *gw2sg = dev_id;
+ unsigned long flags;
+
+ /* we have received a RX signal while GPS should be off */
+ pr_debug("!");
+
+ if (!gw2sg->requested && !gw2sg->is_on &&
+ (gw2sg->state == W2SG_IDLE) &&
+ time_after(jiffies,
+ gw2sg->last_toggle + gw2sg->backoff)) {
+ /* Should be off by now, time to toggle again */
+ gw2sg->is_on = 1;
+ gw2sg->backoff *= 2;
+ spin_lock_irqsave(&gw2sg->lock, flags);
+ if (!gw2sg->suspended)
+ schedule_delayed_work(&gw2sg->work, 0);
+ spin_unlock_irqrestore(&gw2sg->lock, flags);
+ }
+
+ return IRQ_HANDLED;
+}
+
+static void gpio_w2sg_set_power(struct gpio_w2sg *gw2sg, int val)
+{
+ unsigned long flags;
+
+ pr_debug("GPS SET to %d\n", val);
+
+ spin_lock_irqsave(&gw2sg->lock, flags);
+
+ if (val && !gw2sg->requested) {
+ if (gw2sg->rx_redirected) {
+ gw2sg->rx_redirected = 0;
+ disable_irq(gw2sg->rx_irq);
+ pinctrl_select_state(gw2sg->p, gw2sg->default_state);
+ }
+ gw2sg->requested = 1;
+ } else if (!val && gw2sg->requested) {
+ gw2sg->backoff = HZ;
+ gw2sg->requested = 0;
+ } else
+ goto unlock;
+
+ if (!gw2sg->suspended)
+ schedule_delayed_work(&gw2sg->work, 0);
+unlock:
+ spin_unlock_irqrestore(&gw2sg->lock, flags);
+}
+
+static int gpio_w2sg_get_value(struct gpio_chip *gc, unsigned offset)
+{
+ struct gpio_w2sg *gw2sg = container_of(gc, struct gpio_w2sg, gpio);
+
+ return gw2sg->is_on;
+}
+
+static void gpio_w2sg_set_value(struct gpio_chip *gc, unsigned offset, int val)
+{
+ struct gpio_w2sg *gw2sg = container_of(gc, struct gpio_w2sg, gpio);
+
+ gpio_w2sg_set_power(gw2sg, val);
+}
+
+static int gpio_w2sg_direction_output(struct gpio_chip *gc,
+ unsigned offset, int val)
+{
+ gpio_w2sg_set_value(gc, offset, val);
+
+ return 0;
+}
+
+static int gpio_w2sg_rfkill_set_block(void *data, bool blocked)
+{
+ struct gpio_w2sg *gw2sg = data;
+
+ pr_debug("%s: blocked: %d\n", __func__, blocked);
+
+ gw2sg->lna_blocked = blocked;
+
+ return gpio_w2sg_set_lna_power(gw2sg);
+}
+
+static struct rfkill_ops gpio_w2sg0004_rfkill_ops = {
+ .set_block = gpio_w2sg_rfkill_set_block,
+};
+
+static int gpio_w2sg_probe(struct platform_device *pdev)
+{
+ struct gpio_w2sg_data *pdata = dev_get_platdata(&pdev->dev);
+ struct gpio_w2sg *gw2sg;
+ struct rfkill *rf_kill;
+ int err;
+
+ pr_debug("gpio_w2sg_probe()\n");
+
+#ifdef CONFIG_OF
+ if (pdev->dev.of_node) {
+ struct device *dev = &pdev->dev;
+ enum of_gpio_flags flags;
+
+ pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata)
+ return -ENOMEM;
+
+ pdata->on_off_gpio = of_get_named_gpio_flags(dev->of_node,
+ "on-off-gpio", 0, &flags);
+
+ pdata->rx_irq = irq_of_parse_and_map(dev->of_node, 0);
+ if (!pdata->rx_irq) {
+ dev_err(dev, "no IRQ found\n");
+ return -ENODEV;
+ }
+
+ if (pdata->on_off_gpio == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+
+ pdata->lna_regulator = devm_regulator_get_optional(dev, "lna");
+
+ pr_debug("lna_regulator = %p\n", pdata->lna_regulator);
+
+ if (IS_ERR(pdata->lna_regulator))
+ return PTR_ERR(pdata->lna_regulator);
+
+ pdata->gpio_base = -1;
+ pdev->dev.platform_data = pdata;
+ }
+#endif
+
+ gw2sg = devm_kzalloc(&pdev->dev, sizeof(*gw2sg), GFP_KERNEL);
+ if (gw2sg == NULL)
+ return -ENOMEM;
+
+ gw2sg->lna_regulator = pdata->lna_regulator;
+ gw2sg->lna_blocked = true;
+ gw2sg->lna_is_off = true;
+
+ gw2sg->on_off_gpio = pdata->on_off_gpio;
+
+ gw2sg->is_on = false;
+ gw2sg->requested = true;
+ gw2sg->state = W2SG_IDLE;
+ gw2sg->last_toggle = jiffies;
+ gw2sg->backoff = HZ;
+
+ /* label of controlling GPIO */
+ gw2sg->gpio_name[0] = "gpio-w2sg0004-enable";
+ gw2sg->gpio.label = "w2sg0004";
+ gw2sg->gpio.names = gw2sg->gpio_name;
+ gw2sg->gpio.ngpio = 1;
+ gw2sg->gpio.base = pdata->gpio_base;
+ gw2sg->gpio.owner = THIS_MODULE;
+ gw2sg->gpio.direction_output = gpio_w2sg_direction_output;
+ gw2sg->gpio.get = gpio_w2sg_get_value;
+ gw2sg->gpio.set = gpio_w2sg_set_value;
+ gw2sg->gpio.can_sleep = 0;
+ gw2sg->rx_irq = pdata->rx_irq;
+
+#ifdef CONFIG_OF_GPIO
+ gw2sg->gpio.of_node = pdev->dev.of_node;
+#endif
+
+#ifdef CONFIG_OF
+ if (pdev->dev.of_node) {
+ gw2sg->p = devm_pinctrl_get(&pdev->dev);
+ if (IS_ERR(gw2sg->p)) {
+ err = PTR_ERR(gw2sg->p);
+ dev_err(&pdev->dev, "Cannot get pinctrl: %d\n", err);
+ goto out;
+ }
+
+ gw2sg->default_state = pinctrl_lookup_state(gw2sg->p,
+ PINCTRL_STATE_DEFAULT);
+ if (IS_ERR(gw2sg->default_state)) {
+ err = PTR_ERR(gw2sg->default_state);
+ dev_err(&pdev->dev, "Cannot look up pinctrl state %s: %d\n",
+ PINCTRL_STATE_DEFAULT, err);
+ goto out;
+ }
+
+ gw2sg->monitor_state = pinctrl_lookup_state(gw2sg->p,
+ "monitor");
+ if (IS_ERR(gw2sg->monitor_state)) {
+ err = PTR_ERR(gw2sg->monitor_state);
+ dev_err(&pdev->dev,
+ "Cannot look up pinctrl state %s: %d\n",
+ "monitor", err);
+ goto out;
+ }
+ /* choose UART state as default */
+ err = pinctrl_select_state(gw2sg->p, gw2sg->default_state);
+ if (err < 0)
+ goto out;
+ }
+#endif
+
+ INIT_DELAYED_WORK(&gw2sg->work, toggle_work);
+ spin_lock_init(&gw2sg->lock);
+
+ err = devm_gpio_request(&pdev->dev, gw2sg->on_off_gpio,
+ "w2sg0004-on-off");
+ if (err < 0)
+ goto out;
+
+ gpio_direction_output(gw2sg->on_off_gpio, false);
+
+ err = devm_request_threaded_irq(&pdev->dev, gw2sg->rx_irq, NULL,
+ gpio_w2sg_isr,
+ IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+ "w2sg0004-rx",
+ gw2sg);
+ if (err < 0) {
+ dev_err(&pdev->dev, "Unable to claim irq %d; error %d\n",
+ gw2sg->rx_irq, err);
+ goto out;
+ }
+
+ disable_irq(gw2sg->rx_irq);
+
+ err = gpiochip_add(&gw2sg->gpio);
+ if (err)
+ goto out;
+
+ rf_kill = rfkill_alloc("GPS", &pdev->dev, RFKILL_TYPE_GPS,
+ &gpio_w2sg0004_rfkill_ops, gw2sg);
+ if (rf_kill == NULL) {
+ err = -ENOMEM;
+ goto err_rfkill;
+ }
+
+ err = rfkill_register(rf_kill);
+ if (err) {
+ dev_err(&pdev->dev, "Cannot register rfkill device\n");
+ goto err_rfkill;
+ }
+
+ gw2sg->rf_kill = rf_kill;
+
+ platform_set_drvdata(pdev, gw2sg);
+
+ pr_debug("w2sg0004 probed\n");
+
+ return 0;
+
+err_rfkill:
+ rfkill_destroy(rf_kill);
+ gpiochip_remove(&gw2sg->gpio);
+out:
+ return err;
+}
+
+static int gpio_w2sg_remove(struct platform_device *pdev)
+{
+ struct gpio_w2sg *gw2sg = platform_get_drvdata(pdev);
+
+ cancel_delayed_work_sync(&gw2sg->work);
+ gpiochip_remove(&gw2sg->gpio);
+
+ return 0;
+}
+
+static int gpio_w2sg_suspend(struct device *dev)
+{
+ /* Ignore the GPIO and just turn device off.
+ * we cannot really wait for a separate thread to
+ * do things, so we disable that and do it all
+ * here
+ */
+ struct gpio_w2sg *gw2sg = dev_get_drvdata(dev);
+
+ spin_lock_irq(&gw2sg->lock);
+ gw2sg->suspended = 1;
+ spin_unlock_irq(&gw2sg->lock);
+
+ cancel_delayed_work_sync(&gw2sg->work);
+
+ gpio_w2sg_set_lna_power(gw2sg); /* shut down if needed */
+
+ if (gw2sg->state == W2SG_PULSE) {
+ usleep_range(10000, 15000);
+ gpio_set_value_cansleep(gw2sg->on_off_gpio, 1);
+ gw2sg->last_toggle = jiffies;
+ gw2sg->is_on = !gw2sg->is_on;
+ gw2sg->state = W2SG_NOPULSE;
+ }
+
+ if (gw2sg->state == W2SG_NOPULSE) {
+ usleep_range(10000, 15000);
+ gw2sg->state = W2SG_IDLE;
+ }
+
+ if (gw2sg->is_on) {
+ pr_info("GPS off for suspend %d %d %d\n",
+ gw2sg->requested, gw2sg->is_on, gw2sg->lna_is_off);
+
+ gpio_set_value_cansleep(gw2sg->on_off_gpio, 0);
+ usleep_range(10000, 15000);
+ gpio_set_value_cansleep(gw2sg->on_off_gpio, 1);
+ gw2sg->is_on = 0;
+ }
+
+ return 0;
+}
+
+static int gpio_w2sg_resume(struct device *dev)
+{
+ struct gpio_w2sg *gw2sg = dev_get_drvdata(dev);
+
+ spin_lock_irq(&gw2sg->lock);
+ gw2sg->suspended = 0;
+ spin_unlock_irq(&gw2sg->lock);
+
+ pr_info("GPS resuming %d %d %d\n",
+ gw2sg->requested, gw2sg->is_on, gw2sg->lna_is_off);
+
+ schedule_delayed_work(&gw2sg->work, 0); /* enables LNA if needed */
+
+ return 0;
+}
+
+#if defined(CONFIG_OF)
+static const struct of_device_id w2sg0004_of_match[] = {
+ { .compatible = "wi2wi,w2sg0004" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, w2sg0004_of_match);
+#endif
+
+SIMPLE_DEV_PM_OPS(w2sg_pm_ops, gpio_w2sg_suspend, gpio_w2sg_resume);
+
+static struct platform_driver gpio_w2sg_driver = {
+ .probe = gpio_w2sg_probe,
+ .remove = gpio_w2sg_remove,
+ .driver = {
+ .name = "w2sg0004",
+ .owner = THIS_MODULE,
+ .pm = &w2sg_pm_ops,
+ .of_match_table = of_match_ptr(w2sg0004_of_match)
+ },
+};
+
+module_platform_driver(gpio_w2sg_driver);
+
+MODULE_ALIAS("w2sg0004");
+
+MODULE_AUTHOR("NeilBrown <***@suse.de>");
+MODULE_DESCRIPTION("w2sg0004 GPS virtual GPIO driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/w2sg0004.h b/include/linux/w2sg0004.h
new file mode 100644
index 0000000..f20e90c
--- /dev/null
+++ b/include/linux/w2sg0004.h
@@ -0,0 +1,23 @@
+/*
+ * Virtual gpio to allow ON/OFF control of w2sg0004 GPS receiver.
+ *
+ * Copyright (C) 2011 Neil Brown <***@brown.name>
+ *
+ */
+
+
+#ifndef __LINUX_W2SG0004_H
+#define __LINUX_W2SG0004_H
+
+#include <linux/regulator/consumer.h>
+
+struct gpio_w2sg_data {
+ int gpio_base; /* (not used by DT) - defines the gpio.base */
+ struct regulator *lna_regulator; /* enable LNA power */
+ int on_off_gpio; /* connected to the on-off input of the GPS module */
+ int rx_irq; /* the rx irq - we track to check for module activity */
+ unsigned short on_state; /* Mux state when GPS is on */
+ unsigned short off_state; /* Mux state when GPS is off */
+};
+
+#endif /* __LINUX_W2SG0004_H */
--
1.9.1
Marek Belisko
2014-10-16 20:26:23 UTC
Permalink
Signed-off-by: H. Nikolaus Schaller <***@goldelico.com>
Signed-off-by: Marek Belisko <***@goldelico.com>
---
.../devicetree/bindings/misc/wi2wi,w2sg0004.txt | 44 ++++++++++++++++++++++
1 file changed, 44 insertions(+)
create mode 100644 Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt

diff --git a/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt b/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
new file mode 100644
index 0000000..e144441
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
@@ -0,0 +1,44 @@
+Wi2Wi GPS module connected through UART
+
+Required properties:
+- compatible: wi2wi,w2sg0004 or wi2wi,w2sg0084
+- pinctrl: specify two states (default and monitor). One is the default (UART) mode
+ and the other is for monitoring the RX line by an interrupt
+- 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:
+
+ gps_receiver: w2sg0004 {
+ compatible = "wi2wi,w2sg0004";
+ gpio-controller;
+ #gpio-cells = <2>;
+
+ pinctrl-names = "default", "monitor";
+ pinctrl-0 = <&uart2_pins>;
+ pinctrl-1 = <&uart2_rx_irq_pins>;
+
+ interrupt-parent = <&gpio5>;
+ interrupts = <19 IRQ_TYPE_EDGE_FALLING>; /* GPIO_147: RX - trigger on arrival of start bit */
+ lna-supply = <&vsim>; /* LNA regulator */
+ on-off-gpio = <&gpio5 17 0>; /* GPIO_145: trigger for turning on/off w2sg0004 */
+
+&pinmux {
+
+ uart2_pins: pinmux_uart2_pins {
+ pinctrl-single,pins = <
+ 0x14a (PIN_INPUT | MUX_MODE0) /* uart2_tx.uart2_rx */
+ 0x148 (PIN_OUTPUT | MUX_MODE0) /* uart2_tx.uart2_tx */
+ >;
+ };
+
+ uart2_rx_irq_pins: pinmux_uart2_rx_irq_pins {
+ pinctrl-single,pins = <
+ /* switch RX to GPIO so that we can get interrupts by the start bit */
+ 0x14a (PIN_INPUT | MUX_MODE4) /* uart2_tx.uart2_rx */
+ >;
+ };
+
+}
--
1.9.1
Mark Rutland
2014-10-17 09:37:14 UTC
Permalink
Post by Marek Belisko
---
.../devicetree/bindings/misc/wi2wi,w2sg0004.txt | 44 ++++++++++++++++++++++
1 file changed, 44 insertions(+)
create mode 100644 Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
diff --git a/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt b/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
new file mode 100644
index 0000000..e144441
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
@@ -0,0 +1,44 @@
+Wi2Wi GPS module connected through UART
+
+- compatible: wi2wi,w2sg0004 or wi2wi,w2sg0084
+- pinctrl: specify two states (default and monitor). One is the default (UART) mode
+ and the other is for monitoring the RX line by an interrupt
+- on-off-gpio: the GPIO that controls the module's on-off toggle input
+
+- lna-suppy: an (optional) LNA regulator that is enabled together with the GPS receiver
+
+
+ gps_receiver: w2sg0004 {
+ compatible = "wi2wi,w2sg0004";
I couldn't spot "wi2wi" in
Documentation/devicetree/bindings/vendor-prefixes.txt (in mainline).

Could you please add it?
Post by Marek Belisko
+ gpio-controller;
+ #gpio-cells = <2>;
As far as I can see, these properties aren't necessary. This only
consumes a GPIO, it doesn't provide any.
Post by Marek Belisko
+
+ pinctrl-names = "default", "monitor";
+ pinctrl-0 = <&uart2_pins>;
+ pinctrl-1 = <&uart2_rx_irq_pins>;
+
+ interrupt-parent = <&gpio5>;
+ interrupts = <19 IRQ_TYPE_EDGE_FALLING>; /* GPIO_147: RX - trigger on arrival of start bit */
While interrupts is a standard property, please describe above how many
you expect and what their logical function is.

The only part I'm confused about is how the link to the UART is
described. I assume I'm just ignorant of some existing pattern.

Otherwise this looks ok.

Thanks,
Mark.
Post by Marek Belisko
+ lna-supply = <&vsim>; /* LNA regulator */
+ on-off-gpio = <&gpio5 17 0>; /* GPIO_145: trigger for turning on/off w2sg0004 */
+
+&pinmux {
+
+ uart2_pins: pinmux_uart2_pins {
+ pinctrl-single,pins = <
+ 0x14a (PIN_INPUT | MUX_MODE0) /* uart2_tx.uart2_rx */
+ 0x148 (PIN_OUTPUT | MUX_MODE0) /* uart2_tx.uart2_tx */
+ >;
+ };
+
+ uart2_rx_irq_pins: pinmux_uart2_rx_irq_pins {
+ pinctrl-single,pins = <
+ /* switch RX to GPIO so that we can get interrupts by the start bit */
+ 0x14a (PIN_INPUT | MUX_MODE4) /* uart2_tx.uart2_rx */
+ >;
+ };
+
+}
--
1.9.1
Dr. H. Nikolaus Schaller
2014-10-17 10:16:42 UTC
Permalink
Post by Mark Rutland
---
.../devicetree/bindings/misc/wi2wi,w2sg0004.txt | 44 ++++++++++++=
++++++++++
Post by Mark Rutland
1 file changed, 44 insertions(+)
create mode 100644 Documentation/devicetree/bindings/misc/wi2wi,w2sg=
0004.txt
Post by Mark Rutland
=20
diff --git a/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.t=
xt b/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
Post by Mark Rutland
new file mode 100644
index 0000000..e144441
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
@@ -0,0 +1,44 @@
+Wi2Wi GPS module connected through UART
+
+- compatible: wi2wi,w2sg0004 or wi2wi,w2sg0084
+- pinctrl: specify two states (default and monitor). One is the def=
ault (UART) mode
Post by Mark Rutland
+ and the other is for monitoring the RX line by an interrupt
+- on-off-gpio: the GPIO that controls the module's on-off toggle in=
put
Post by Mark Rutland
+
+- lna-suppy: an (optional) LNA regulator that is enabled together w=
ith the GPS receiver
Post by Mark Rutland
+
+
+ gps_receiver: w2sg0004 {
+ compatible =3D "wi2wi,w2sg0004";
=20
I couldn't spot "wi2wi" in
Documentation/devicetree/bindings/vendor-prefixes.txt (in mainline).
=20
Could you please add it?
=20
+ gpio-controller;
+ #gpio-cells =3D <2>;
=20
As far as I can see, these properties aren't necessary. This only
consumes a GPIO, it doesn't provide any.
Well, it provides one GPIO. Sort of a "virtual=93 GPIO. It is needed so=
that=20
it can be wired up to the DTR gpio of the UART driver (unfortunately th=
is
patch was reverted some months ago from mainline and we will reintroduc=
e
it soon).

The reason to solve it that way is that we did not want to have a direc=
t link
between this driver and any serial drivers or other mechanisms how driv=
ers
can detect that their serial port (/dev/tty*) is opened.

It is used to power down the w2sg GPS chip if no user space process is
accessing its serial port (or de-asserts DTR through tcsetattr/ioctl).
Post by Mark Rutland
=20
+
+ pinctrl-names =3D "default", "monitor";
+ pinctrl-0 =3D <&uart2_pins>;
+ pinctrl-1 =3D <&uart2_rx_irq_pins>;
+
+ interrupt-parent =3D <&gpio5>;
+ interrupts =3D <19 IRQ_TYPE_EDGE_FALLING>; /* GPIO=
_147: RX - trigger on arrival of start bit */
Post by Mark Rutland
=20
While interrupts is a standard property, please describe above how ma=
ny
Post by Mark Rutland
you expect and what their logical function is.
=20
The only part I'm confused about is how the link to the UART is
described. I assume I'm just ignorant of some existing pattern.
The serial link itself is not described at all because it is assumed to=
be a =84must have=93.
The driver only needs to monitor the RX line and needs to switch it bet=
ween UART and
GPIO/IRQ mode. So this monitoring switch is described (with two differe=
nt pinctrl states).

We know that it is a little tricky to control this chip correctly - and=
we think this solution
is the most general (no direct dependency on the serial line, and just =
to pinmux states
and an interrupt).
Post by Mark Rutland
=20
Otherwise this looks ok.
=20
Thanks,
Mark.
Thanks as well,
Nikolaus
Post by Mark Rutland
=20
+ lna-supply =3D <&vsim>; /* LNA regulator */
+ on-off-gpio =3D <&gpio5 17 0>; /* GPIO_145: trigger=
for turning on/off w2sg0004 */
Post by Mark Rutland
+
+&pinmux {
+
+ uart2_pins: pinmux_uart2_pins {
+ pinctrl-single,pins =3D <
+ 0x14a (PIN_INPUT | MUX_MODE0) /* uart2_tx.uart2_rx */
+ 0x148 (PIN_OUTPUT | MUX_MODE0) /* uart2_tx.uart2_tx */
+ >;
+ };
+
+ uart2_rx_irq_pins: pinmux_uart2_rx_irq_pins {
+ pinctrl-single,pins =3D <
+ /* switch RX to GPIO so that we can get interrupts by the start =
bit */
Post by Mark Rutland
+ 0x14a (PIN_INPUT | MUX_MODE4) /* uart2_tx.uart2_rx */
+ >;
+ };
+
+}
--=20
1.9.1
Mark Rutland
2014-10-17 11:00:44 UTC
Permalink
On Fri, Oct 17, 2014 at 11:16:42AM +0100, Dr. H. Nikolaus Schaller wrot=
=20
=20
Post by Mark Rutland
---
.../devicetree/bindings/misc/wi2wi,w2sg0004.txt | 44 ++++++++++=
++++++++++++
Post by Mark Rutland
1 file changed, 44 insertions(+)
create mode 100644 Documentation/devicetree/bindings/misc/wi2wi,w2=
sg0004.txt
Post by Mark Rutland
=20
diff --git a/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004=
=2Etxt b/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
Post by Mark Rutland
new file mode 100644
index 0000000..e144441
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
@@ -0,0 +1,44 @@
+Wi2Wi GPS module connected through UART
+
+- compatible: wi2wi,w2sg0004 or wi2wi,w2sg0084
+- pinctrl: specify two states (default and monitor). One is the d=
efault (UART) mode
Post by Mark Rutland
+ and the other is for monitoring the RX line by an interrupt
+- on-off-gpio: the GPIO that controls the module's on-off toggle =
input
Post by Mark Rutland
+
+- lna-suppy: an (optional) LNA regulator that is enabled together=
with the GPS receiver
Post by Mark Rutland
+
+
+ gps_receiver: w2sg0004 {
+ compatible =3D "wi2wi,w2sg0004";
=20
I couldn't spot "wi2wi" in
Documentation/devicetree/bindings/vendor-prefixes.txt (in mainline)=
=2E
Post by Mark Rutland
=20
Could you please add it?
=20
+ gpio-controller;
+ #gpio-cells =3D <2>;
=20
As far as I can see, these properties aren't necessary. This only
consumes a GPIO, it doesn't provide any.
=20
Well, it provides one GPIO. Sort of a "virtual=E2=80=9C GPIO. It is n=
eeded so that=20
it can be wired up to the DTR gpio of the UART driver (unfortunately =
this
patch was reverted some months ago from mainline and we will reintrod=
uce
it soon).
If this GPIO doesn't really exist, then it's a Linux internal
implementation detail rather than a description of the hardware, and
doesn't really belong in the DT.

It sounds like what we actually need is the ability to describe devices
attached to UARTs. Then you could have a mechanism whereby the UART
driver can notify the other device driver regarding events (e.g. the
UART being opened for access), or the other driver could claim ownershi=
p
of the UART and expose its own interface to userspace.

That would be independent of the particular UART or other device, and
the only description necessary in the DT would be an accurate
representation of the way the hardware is wired.

There are a few ways that could be done, but I suspect the simplest is
to just have the device as a sub-node of the UART, like we do for SPI o=
r
I2C buses:

***@f00 {
compatible =3D "vendor,uart";
reg =3D <0xf00 0x100>;
...

gps {
compatible =3D "wi2wi,w2sg0004";
...
};
};

That wouldn't work for devices with multiple UART connections. Do those
exist, and are they common in configurations where out-of-band
management is necessary (e.g. regulators, clocks)?
The reason to solve it that way is that we did not want to have a dir=
ect link
between this driver and any serial drivers or other mechanisms how dr=
ivers
can detect that their serial port (/dev/tty*) is opened.
It is used to power down the w2sg GPS chip if no user space process i=
s
accessing its serial port (or de-asserts DTR through tcsetattr/ioctl)=
=2E
=20
Post by Mark Rutland
=20
+
+ pinctrl-names =3D "default", "monitor";
+ pinctrl-0 =3D <&uart2_pins>;
+ pinctrl-1 =3D <&uart2_rx_irq_pins>;
+
+ interrupt-parent =3D <&gpio5>;
+ interrupts =3D <19 IRQ_TYPE_EDGE_FALLING>; /* GP=
IO_147: RX - trigger on arrival of start bit */
Post by Mark Rutland
=20
While interrupts is a standard property, please describe above how =
many
Post by Mark Rutland
you expect and what their logical function is.
=20
The only part I'm confused about is how the link to the UART is
described. I assume I'm just ignorant of some existing pattern.
=20
The serial link itself is not described at all because it is assumed =
to be a =E2=80=9Emust have=E2=80=9C.

Huh? So it's a "must have" that you "don't have" in the DT?

I think that the relationship is being described incorrectly in the DT,
and I think that there is a more general problem that needs to be
addressed in order to make this case work.
The driver only needs to monitor the RX line and needs to switch it b=
etween UART and
GPIO/IRQ mode. So this monitoring switch is described (with two diffe=
rent pinctrl states).

While this particular driver only needs that at this point in time,
that's not necessarily true of drivers for similar devices, nor is it
necessarily true if we need to add additional features to this driver.

Describing the relationship leaves a lot more freedom to improve things
without having to update every DTB.
=20
We know that it is a little tricky to control this chip correctly - a=
nd we think this solution
is the most general (no direct dependency on the serial line, and jus=
t to pinmux states
and an interrupt).
I think that the rough approach I sketched out above is more general,
and I think that you must describe the relationship with the serial
line.

It's not clear to me whether the interrupt you describe is attached to
the GPS, or if this is logically part of the UART.

Thanks,
Mark.
Dr. H. Nikolaus Schaller
2014-10-17 19:55:50 UTC
Permalink
On Fri, Oct 17, 2014 at 11:16:42AM +0100, Dr. H. Nikolaus Schaller wr=
=20
=20
Post by Mark Rutland
---
.../devicetree/bindings/misc/wi2wi,w2sg0004.txt | 44 ++++++++++=
++++++++++++
Post by Mark Rutland
1 file changed, 44 insertions(+)
create mode 100644 Documentation/devicetree/bindings/misc/wi2wi,w2=
sg0004.txt
Post by Mark Rutland
=20
diff --git a/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004=
=2Etxt b/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
Post by Mark Rutland
new file mode 100644
index 0000000..e144441
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
@@ -0,0 +1,44 @@
+Wi2Wi GPS module connected through UART
+
+- compatible: wi2wi,w2sg0004 or wi2wi,w2sg0084
+- pinctrl: specify two states (default and monitor). One is the d=
efault (UART) mode
Post by Mark Rutland
+ and the other is for monitoring the RX line by an interrupt
+- on-off-gpio: the GPIO that controls the module's on-off toggle =
input
Post by Mark Rutland
+
+- lna-suppy: an (optional) LNA regulator that is enabled together=
with the GPS receiver
Post by Mark Rutland
+
+
+ gps_receiver: w2sg0004 {
+ compatible =3D "wi2wi,w2sg0004";
=20
I couldn't spot "wi2wi" in
Documentation/devicetree/bindings/vendor-prefixes.txt (in mainline)=
=2E
Post by Mark Rutland
=20
Could you please add it?
=20
+ gpio-controller;
+ #gpio-cells =3D <2>;
=20
As far as I can see, these properties aren't necessary. This only
consumes a GPIO, it doesn't provide any.
=20
Well, it provides one GPIO. Sort of a "virtual=93 GPIO. It is needed=
so that=20
it can be wired up to the DTR gpio of the UART driver (unfortunately=
this
patch was reverted some months ago from mainline and we will reintro=
duce
it soon).
=20
If this GPIO doesn't really exist, then it's a Linux internal
implementation detail rather than a description of the hardware, and
doesn't really belong in the DT.
Hm.=20

Let=92s describe it differently.

I can see the Linux driver module as a simple software simulation for a
piece of hardware that could have been connected between the UART
and the GPS chip.

Basically it is a pulse-generator and a flip-flop to detect data flow o=
n the RX
wire. This could be implemented by an external FPGA or uController. Or =
as
it is done on our board for saving hardware by a mix of the main CPU ha=
rdware
(Pinmux + GPIO + IRQ) and a kernel driver.

The best of course would have been if the w2sg0004 would have a physica=
l
=84enable=93 GPIO (instead of that on-off control input).

Then we would hook up that enable to some physical GPIO of the CPU
and simply refer to it as e.g. <&gpio4 12>. And would not even need a d=
river
for it (unless we want to make rfkill gps work).

Therefore the driver we suggest provides an additional gpio controller =
with a
single GPIO so that we can write <&w2sg 0> to refer to this virtual gpi=
o.

So in fact we try to wrap a non-optimal chip design by the driver and m=
ake it
appear as a standard GPIO interface to the DT and user space and whoeve=
r
needs simply to enable/disable the GPS chip.
=20
It sounds like what we actually need is the ability to describe devic=
es
attached to UARTs.
Hm. The purpose of the driver is power control of the chip. Not the ser=
ial
interface.
Then you could have a mechanism whereby the UART
driver can notify the other device driver regarding events (e.g. the
UART being opened for access), or the other driver could claim owners=
hip
of the UART and expose its own interface to userspace.
=20
That would be independent of the particular UART or other device, and
the only description necessary in the DT would be an accurate
representation of the way the hardware is wired.
=20
There are a few ways that could be done, but I suspect the simplest i=
s
to just have the device as a sub-node of the UART, like we do for SPI=
or
=20
compatible =3D "vendor,uart";
reg =3D <0xf00 0x100>;
...
=20
gps {
compatible =3D "wi2wi,w2sg0004";
...
};
};
=20
That wouldn't work for devices with multiple UART connections. Do tho=
se
exist, and are they common in configurations where out-of-band
management is necessary (e.g. regulators, clocks)?
UARTs are usually point to point interfaces and not busses. So there is
no need to describe the interface. And I would speculate that in most c=
ases
they simply go to some connector and therefore no connected chip that n=
eeds
to be described in the DT at all. Because it has a user-space driver (e=
=2Eg. AT
commands) and no kernel driver.

But we have no idea how such a solution could be implemented or tested.

If someone adds that to the serial drivers, we would be happy to use it=
,
but unless such a thing exists, I think our solution is quite simple an=
d isolated
into this single driver and also uses existing standard interfaces (gpi=
os, pinmux).
=20
The reason to solve it that way is that we did not want to have a di=
rect link
between this driver and any serial drivers or other mechanisms how d=
rivers
can detect that their serial port (/dev/tty*) is opened.
=20
It is used to power down the w2sg GPS chip if no user space process =
is
accessing its serial port (or de-asserts DTR through tcsetattr/ioctl=
).
=20
Post by Mark Rutland
=20
+
+ pinctrl-names =3D "default", "monitor";
+ pinctrl-0 =3D <&uart2_pins>;
+ pinctrl-1 =3D <&uart2_rx_irq_pins>;
+
+ interrupt-parent =3D <&gpio5>;
+ interrupts =3D <19 IRQ_TYPE_EDGE_FALLING>; /* GP=
IO_147: RX - trigger on arrival of start bit */
Post by Mark Rutland
=20
While interrupts is a standard property, please describe above how =
many
Post by Mark Rutland
you expect and what their logical function is.
=20
The only part I'm confused about is how the link to the UART is
described. I assume I'm just ignorant of some existing pattern.
=20
The serial link itself is not described at all because it is assumed=
to be a =84must have=93.
=20
Huh? So it's a "must have" that you "don't have" in the DT?
Yes. The DT does not describe everything. Only those things that need
a kernel driver. And UARTs usually have user-space drivers (e.g. gpsd,
gsmd, pppd) and ioctl/tcsetattr.
=20
I think that the relationship is being described incorrectly in the D=
T,
and I think that there is a more general problem that needs to be
addressed in order to make this case work.
=20
The driver only needs to monitor the RX line and needs to switch it =
between UART and
GPIO/IRQ mode. So this monitoring switch is described (with two diff=
erent pinctrl states).
=20
While this particular driver only needs that at this point in time,
that's not necessarily true of drivers for similar devices, nor is it
necessarily true if we need to add additional features to this driver=
=2E

Which features are you thinking of to add to this driver? And do
similar devices exist at all? Since we have not found any, we have
declared it as a "misc=93 driver.
=20
Describing the relationship leaves a lot more freedom to improve thin=
gs
without having to update every DTB.
=20
We know that it is a little tricky to control this chip correctly - =
and we think this solution
is the most general (no direct dependency on the serial line, and ju=
st to pinmux states
and an interrupt).
=20
I think that the rough approach I sketched out above is more general,
and I think that you must describe the relationship with the serial
line.
=20
It's not clear to me whether the interrupt you describe is attached t=
o
the GPS, or if this is logically part of the UART.
The interrupt is needed to simulate the glue logic connected between
UART and GPS.

The output signal comes from the GPS module and goes to some pad
of the OMAP3 SoC. This pad can be either multiplexed into the UART RX
input or into a GPIO bank of the OMAP. That GPIO controller can generat=
e
the interrupt on incoming data (when none is expected).

Therefore it is a GPS-generated interrupt and has nothing to do with
the UART.
=20
Thanks,
Mark.
BR,
Nikolaus
Mark Rutland
2014-10-20 09:35:36 UTC
Permalink
Hi,

On Fri, Oct 17, 2014 at 08:55:50PM +0100, Dr. H. Nikolaus Schaller wrot=
Post by Dr. H. Nikolaus Schaller
=20
=20
On Fri, Oct 17, 2014 at 11:16:42AM +0100, Dr. H. Nikolaus Schaller =
=20
=20
Post by Mark Rutland
---
.../devicetree/bindings/misc/wi2wi,w2sg0004.txt | 44 ++++++++=
++++++++++++++
Post by Dr. H. Nikolaus Schaller
Post by Mark Rutland
1 file changed, 44 insertions(+)
create mode 100644 Documentation/devicetree/bindings/misc/wi2wi,=
w2sg0004.txt
Post by Dr. H. Nikolaus Schaller
Post by Mark Rutland
=20
diff --git a/Documentation/devicetree/bindings/misc/wi2wi,w2sg00=
04.txt b/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
Post by Dr. H. Nikolaus Schaller
Post by Mark Rutland
new file mode 100644
index 0000000..e144441
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
@@ -0,0 +1,44 @@
+Wi2Wi GPS module connected through UART
+
+- compatible: wi2wi,w2sg0004 or wi2wi,w2sg0084
+- pinctrl: specify two states (default and monitor). One is the=
default (UART) mode
Post by Dr. H. Nikolaus Schaller
Post by Mark Rutland
+ and the other is for monitoring the RX line by an interrupt
+- on-off-gpio: the GPIO that controls the module's on-off toggl=
e input
Post by Dr. H. Nikolaus Schaller
Post by Mark Rutland
+
+- lna-suppy: an (optional) LNA regulator that is enabled togeth=
er with the GPS receiver
Post by Dr. H. Nikolaus Schaller
Post by Mark Rutland
+
+
+ gps_receiver: w2sg0004 {
+ compatible =3D "wi2wi,w2sg0004";
=20
I couldn't spot "wi2wi" in
Documentation/devicetree/bindings/vendor-prefixes.txt (in mainlin=
e).
Post by Dr. H. Nikolaus Schaller
Post by Mark Rutland
=20
Could you please add it?
=20
+ gpio-controller;
+ #gpio-cells =3D <2>;
=20
As far as I can see, these properties aren't necessary. This only
consumes a GPIO, it doesn't provide any.
=20
Well, it provides one GPIO. Sort of a "virtual=E2=80=9C GPIO. It i=
s needed so that=20
Post by Dr. H. Nikolaus Schaller
it can be wired up to the DTR gpio of the UART driver (unfortunate=
ly this
Post by Dr. H. Nikolaus Schaller
patch was reverted some months ago from mainline and we will reint=
roduce
Post by Dr. H. Nikolaus Schaller
it soon).
=20
If this GPIO doesn't really exist, then it's a Linux internal
implementation detail rather than a description of the hardware, an=
d
Post by Dr. H. Nikolaus Schaller
doesn't really belong in the DT.
=20
Hm.=20
=20
Let=E2=80=99s describe it differently.
=20
I can see the Linux driver module as a simple software simulation for=
a
Post by Dr. H. Nikolaus Schaller
piece of hardware that could have been connected between the UART
and the GPS chip.
=20
Basically it is a pulse-generator and a flip-flop to detect data flow=
on the RX
Post by Dr. H. Nikolaus Schaller
wire. This could be implemented by an external FPGA or uController. O=
r as
Post by Dr. H. Nikolaus Schaller
it is done on our board for saving hardware by a mix of the main CPU =
hardware
Post by Dr. H. Nikolaus Schaller
(Pinmux + GPIO + IRQ) and a kernel driver.
=20
The best of course would have been if the w2sg0004 would have a physi=
cal
Post by Dr. H. Nikolaus Schaller
=E2=80=9Eenable=E2=80=9C GPIO (instead of that on-off control input).
=20
Then we would hook up that enable to some physical GPIO of the CPU
and simply refer to it as e.g. <&gpio4 12>. And would not even need a=
driver
Post by Dr. H. Nikolaus Schaller
for it (unless we want to make rfkill gps work).
=20
Therefore the driver we suggest provides an additional gpio controlle=
r with a
Post by Dr. H. Nikolaus Schaller
single GPIO so that we can write <&w2sg 0> to refer to this virtual g=
pio.
Post by Dr. H. Nikolaus Schaller
=20
So in fact we try to wrap a non-optimal chip design by the driver and=
make it
Post by Dr. H. Nikolaus Schaller
appear as a standard GPIO interface to the DT and user space and whoe=
ver
Post by Dr. H. Nikolaus Schaller
needs simply to enable/disable the GPS chip.
The fact remains that this does not accurately represent the hardware,
and is unnecessarily strongly tied to a particular UART design (where
the DTR line is a separate UART).
Post by Dr. H. Nikolaus Schaller
It sounds like what we actually need is the ability to describe dev=
ices
Post by Dr. H. Nikolaus Schaller
attached to UARTs.
=20
Hm. The purpose of the driver is power control of the chip. Not the s=
erial
Post by Dr. H. Nikolaus Schaller
interface.
I'm not sure I follow your point. By describing the device as attached
to the UART, the kernel can figure out that when said UART is accessed
the attached device needs to be on (and must be poked as necessary).

The power management logic for the device can stay in the device driver=
,
and the power management logic for the UART can stay in the UART driver=
=2E
Neither would need to know about each other's internal details
(e.g.GPIOs, for DTR or otherwise).
Post by Dr. H. Nikolaus Schaller
Then you could have a mechanism whereby the UART
driver can notify the other device driver regarding events (e.g. th=
e
Post by Dr. H. Nikolaus Schaller
UART being opened for access), or the other driver could claim owne=
rship
Post by Dr. H. Nikolaus Schaller
of the UART and expose its own interface to userspace.
=20
That would be independent of the particular UART or other device, a=
nd
Post by Dr. H. Nikolaus Schaller
the only description necessary in the DT would be an accurate
representation of the way the hardware is wired.
=20
There are a few ways that could be done, but I suspect the simplest=
is
Post by Dr. H. Nikolaus Schaller
to just have the device as a sub-node of the UART, like we do for S=
PI or
Post by Dr. H. Nikolaus Schaller
=20
compatible =3D "vendor,uart";
reg =3D <0xf00 0x100>;
...
=20
gps {
compatible =3D "wi2wi,w2sg0004";
...
};
};
=20
That wouldn't work for devices with multiple UART connections. Do t=
hose
Post by Dr. H. Nikolaus Schaller
exist, and are they common in configurations where out-of-band
management is necessary (e.g. regulators, clocks)?
=20
UARTs are usually point to point interfaces and not busses. So there =
is
Post by Dr. H. Nikolaus Schaller
no need to describe the interface.
I don't follow. You have a device which seems to require management
kernel-side. Rather than describing the interface, you've described a
fictitious relationship between the GPS device and the UART's DTR GPIO,
and you've described a fictitious GPIO to hand to the UART driver. This
is how you have linked the two in order to get the behaviour you want.

So it _is_ necessary to describe the interface. Rather than describing
that interface at a high level you've chosen to hack together a set of
fake relationships to work around the lack of ability to describe said
interface.
Post by Dr. H. Nikolaus Schaller
And I would speculate that in most cases they simply go to some
connector and therefore no connected chip that needs to be described
in the DT at all. Because it has a user-space driver (e.g. AT
commands) and no kernel driver.
In the case where no driver is necessary I agree that no description is
necessary, though the description could be exposed in a helpful way to
userspace to describe what's attached to which UARTs.

However, in this case you do have a kernel driver (even if basic), and
it requires some knowledge of the relationship between the device and
the UART in order to function.
Post by Dr. H. Nikolaus Schaller
But we have no idea how such a solution could be implemented or teste=
d.

I would disagree on that point, given I provided a high level
description of how this could be implemented.
Post by Dr. H. Nikolaus Schaller
If someone adds that to the serial drivers, we would be happy to use =
it,
Post by Dr. H. Nikolaus Schaller
but unless such a thing exists, I think our solution is quite simple =
and isolated
Post by Dr. H. Nikolaus Schaller
into this single driver and also uses existing standard interfaces (g=
pios, pinmux).

I would argue that this _abuses_ standard interfaces, as you have one
device driver fiddling with resources logically owned by another.
Post by Dr. H. Nikolaus Schaller
The reason to solve it that way is that we did not want to have a =
direct link
Post by Dr. H. Nikolaus Schaller
between this driver and any serial drivers or other mechanisms how=
drivers
Post by Dr. H. Nikolaus Schaller
can detect that their serial port (/dev/tty*) is opened.
=20
It is used to power down the w2sg GPS chip if no user space proces=
s is
Post by Dr. H. Nikolaus Schaller
accessing its serial port (or de-asserts DTR through tcsetattr/ioc=
tl).
Post by Dr. H. Nikolaus Schaller
=20
Post by Mark Rutland
=20
+
+ pinctrl-names =3D "default", "monitor";
+ pinctrl-0 =3D <&uart2_pins>;
+ pinctrl-1 =3D <&uart2_rx_irq_pins>;
+
+ interrupt-parent =3D <&gpio5>;
+ interrupts =3D <19 IRQ_TYPE_EDGE_FALLING>; /* =
GPIO_147: RX - trigger on arrival of start bit */
Post by Dr. H. Nikolaus Schaller
Post by Mark Rutland
=20
While interrupts is a standard property, please describe above ho=
w many
Post by Dr. H. Nikolaus Schaller
Post by Mark Rutland
you expect and what their logical function is.
=20
The only part I'm confused about is how the link to the UART is
described. I assume I'm just ignorant of some existing pattern.
=20
The serial link itself is not described at all because it is assum=
ed to be a =E2=80=9Emust have=E2=80=9C.
Post by Dr. H. Nikolaus Schaller
=20
Huh? So it's a "must have" that you "don't have" in the DT?
=20
Yes. The DT does not describe everything. Only those things that need
a kernel driver. And UARTs usually have user-space drivers (e.g. gpsd=
,
Post by Dr. H. Nikolaus Schaller
gsmd, pppd) and ioctl/tcsetattr.
The DT should describe the static portions of the hardware. Typically w=
e
only have devices with kernelspace drivers described simply because
that's the way people have built DTs. Whether or not you have a
kernelspace driver can change over time, the organisation of the
hardware cannot.
Post by Dr. H. Nikolaus Schaller
I think that the relationship is being described incorrectly in the=
DT,
Post by Dr. H. Nikolaus Schaller
and I think that there is a more general problem that needs to be
addressed in order to make this case work.
=20
The driver only needs to monitor the RX line and needs to switch i=
t between UART and
Post by Dr. H. Nikolaus Schaller
GPIO/IRQ mode. So this monitoring switch is described (with two di=
fferent pinctrl states).
Post by Dr. H. Nikolaus Schaller
=20
While this particular driver only needs that at this point in time,
that's not necessarily true of drivers for similar devices, nor is =
it
Post by Dr. H. Nikolaus Schaller
necessarily true if we need to add additional features to this driv=
er.
Post by Dr. H. Nikolaus Schaller
=20
Which features are you thinking of to add to this driver? And do
similar devices exist at all? Since we have not found any, we have
declared it as a "misc=E2=80=9C driver.
I don't have any particular feature in mind.

I am not immediately aware of other serial devices which require
out-of-band management in the same way, though we have a vaguely simila=
r
case with SDIO devices which must be powered up before they appear on
the bus. In that case I believe the intent is to describe them in the D=
T
under the bus.
Post by Dr. H. Nikolaus Schaller
Describing the relationship leaves a lot more freedom to improve th=
ings
Post by Dr. H. Nikolaus Schaller
without having to update every DTB.
=20
We know that it is a little tricky to control this chip correctly =
- and we think this solution
Post by Dr. H. Nikolaus Schaller
is the most general (no direct dependency on the serial line, and =
just to pinmux states
Post by Dr. H. Nikolaus Schaller
and an interrupt).
=20
I think that the rough approach I sketched out above is more genera=
l,
Post by Dr. H. Nikolaus Schaller
and I think that you must describe the relationship with the serial
line.
=20
It's not clear to me whether the interrupt you describe is attached=
to
Post by Dr. H. Nikolaus Schaller
the GPS, or if this is logically part of the UART.
=20
The interrupt is needed to simulate the glue logic connected between
UART and GPS.
=20
The output signal comes from the GPS module and goes to some pad
of the OMAP3 SoC. This pad can be either multiplexed into the UART RX
input or into a GPIO bank of the OMAP. That GPIO controller can gener=
ate
Post by Dr. H. Nikolaus Schaller
the interrupt on incoming data (when none is expected).
=20
Therefore it is a GPS-generated interrupt and has nothing to do with
the UART.
Ok. When does the GPS device raise this interrupt?

Thanks,
Mark.
Dr. H. Nikolaus Schaller
2014-10-20 17:26:27 UTC
Permalink
Hi,
Hi,
=20
On Fri, Oct 17, 2014 at 08:55:50PM +0100, Dr. H. Nikolaus Schaller wr=
Post by Dr. H. Nikolaus Schaller
=20
=20
On Fri, Oct 17, 2014 at 11:16:42AM +0100, Dr. H. Nikolaus Schaller =
=20
=20
Post by Mark Rutland
---
.../devicetree/bindings/misc/wi2wi,w2sg0004.txt | 44 ++++++++=
++++++++++++++
Post by Dr. H. Nikolaus Schaller
Post by Mark Rutland
1 file changed, 44 insertions(+)
create mode 100644 Documentation/devicetree/bindings/misc/wi2wi,=
w2sg0004.txt
Post by Dr. H. Nikolaus Schaller
Post by Mark Rutland
=20
diff --git a/Documentation/devicetree/bindings/misc/wi2wi,w2sg00=
04.txt b/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
Post by Dr. H. Nikolaus Schaller
Post by Mark Rutland
new file mode 100644
index 0000000..e144441
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
@@ -0,0 +1,44 @@
+Wi2Wi GPS module connected through UART
+
+- compatible: wi2wi,w2sg0004 or wi2wi,w2sg0084
+- pinctrl: specify two states (default and monitor). One is the=
default (UART) mode
Post by Dr. H. Nikolaus Schaller
Post by Mark Rutland
+ and the other is for monitoring the RX line by an interrupt
+- on-off-gpio: the GPIO that controls the module's on-off toggl=
e input
Post by Dr. H. Nikolaus Schaller
Post by Mark Rutland
+
+- lna-suppy: an (optional) LNA regulator that is enabled togeth=
er with the GPS receiver
Post by Dr. H. Nikolaus Schaller
Post by Mark Rutland
+
+
+ gps_receiver: w2sg0004 {
+ compatible =3D "wi2wi,w2sg0004";
=20
I couldn't spot "wi2wi" in
Documentation/devicetree/bindings/vendor-prefixes.txt (in mainlin=
e).
Post by Dr. H. Nikolaus Schaller
Post by Mark Rutland
=20
Could you please add it?
=20
+ gpio-controller;
+ #gpio-cells =3D <2>;
=20
As far as I can see, these properties aren't necessary. This only
consumes a GPIO, it doesn't provide any.
=20
Well, it provides one GPIO. Sort of a "virtual=93 GPIO. It is need=
ed so that=20
Post by Dr. H. Nikolaus Schaller
it can be wired up to the DTR gpio of the UART driver (unfortunate=
ly this
Post by Dr. H. Nikolaus Schaller
patch was reverted some months ago from mainline and we will reint=
roduce
Post by Dr. H. Nikolaus Schaller
it soon).
=20
If this GPIO doesn't really exist, then it's a Linux internal
implementation detail rather than a description of the hardware, an=
d
Post by Dr. H. Nikolaus Schaller
doesn't really belong in the DT.
=20
Hm.=20
=20
Let=92s describe it differently.
=20
I can see the Linux driver module as a simple software simulation fo=
r a
Post by Dr. H. Nikolaus Schaller
piece of hardware that could have been connected between the UART
and the GPS chip.
=20
Basically it is a pulse-generator and a flip-flop to detect data flo=
w on the RX
Post by Dr. H. Nikolaus Schaller
wire. This could be implemented by an external FPGA or uController. =
Or as
Post by Dr. H. Nikolaus Schaller
it is done on our board for saving hardware by a mix of the main CPU=
hardware
Post by Dr. H. Nikolaus Schaller
(Pinmux + GPIO + IRQ) and a kernel driver.
=20
The best of course would have been if the w2sg0004 would have a phys=
ical
Post by Dr. H. Nikolaus Schaller
=84enable=93 GPIO (instead of that on-off control input).
=20
Then we would hook up that enable to some physical GPIO of the CPU
and simply refer to it as e.g. <&gpio4 12>. And would not even need =
a driver
Post by Dr. H. Nikolaus Schaller
for it (unless we want to make rfkill gps work).
=20
Therefore the driver we suggest provides an additional gpio controll=
er with a
Post by Dr. H. Nikolaus Schaller
single GPIO so that we can write <&w2sg 0> to refer to this virtual =
gpio.
Post by Dr. H. Nikolaus Schaller
=20
So in fact we try to wrap a non-optimal chip design by the driver an=
d make it
Post by Dr. H. Nikolaus Schaller
appear as a standard GPIO interface to the DT and user space and who=
ever
Post by Dr. H. Nikolaus Schaller
needs simply to enable/disable the GPS chip.
=20
The fact remains that this does not accurately represent the hardware=
,
and is unnecessarily strongly tied to a particular UART design (where
the DTR line is a separate UART).
I don=92t get that it is tied to a particular UART design (except that =
our DTR
patch affects to omap-serial only).

Let=92s describe the facts:

The chip has this interface:

GPS-TX (output)
GPS-RX (input)
ON/OFF (input) - to toggle the power state of the chip

They are connected to an OMAP3 UART2 as

UART2-TX > GPS-RX
UART2-RX <- GPS-TX
GPIO145 -> ON/OFF

That=92s it.

If the chip (or any other serial GPS receiver like a Garmin) were conne=
cted
through real RS232 we would have

UART2-TX -> level shifter -> cable -> level shifter -> GPS-RX
UART2-RX <- level shifter <- cable <- level shifter <- GPS-TX
DTR-GPIO -> level shifter (DTR line) -> cable -> level shifter -> power=
-management logic -> ON/OFF

But because it is connected directly, we need to implement the power-ma=
nagement
logic between the DTR-GPIO and GPIO145:

DTR-GPIO -> driver -> GPIO145 -> ON/OFF

To correctly determine the state we need to tap the RX signal before
it enters the UART2-RX (it is pinmuxed with GPIO147):

UART2-RX <=97=97+
OMAP3 pinmux <- OMAP3 pad <- GPS-TX
GPIO147 <=97=97+

So if we describe the driver as a box we have

inputs from user space:
* rfkill for GPS
* a control that is activated by opening /dev/tty

outputs to system:
* a control to switch the pinmux between RX and GPIO (interrupt)
* a control to external hardware
=20
Post by Dr. H. Nikolaus Schaller
It sounds like what we actually need is the ability to describe dev=
ices
Post by Dr. H. Nikolaus Schaller
attached to UARTs.
=20
Hm. The purpose of the driver is power control of the chip. Not the =
serial
Post by Dr. H. Nikolaus Schaller
interface.
=20
I'm not sure I follow your point. By describing the device as attache=
d
to the UART, the kernel can figure out that when said UART is accesse=
d
the attached device needs to be on (and must be poked as necessary).
Why do we need to describe this? It is all about controlling the GPIO14=
5,
GPIO147 and pinmux. But not RX or TX.

So the driver does not need to know anything about UARTs (and if we
connect it to UART3 we only have to specify different GPIOs).

Only our board specific dtb connects the UART to the =93virtual=94 GPIO
provided by the driver.

This also brings up a minor problem: on OMAP3 some UART RX lines can
be pinmuxed with different GPIOs. So putting the driver under some UART=
2
node doesn=92t uniquely define the GPIO number to monitor RX and might
become a mess.
=20
The power management logic for the device can stay in the device driv=
er,
and the power management logic for the UART can stay in the UART driv=
er.

Here I can=92t follow you. What power management does an UART driver pr=
ovide?
Neither would need to know about each other's internal details
(e.g.GPIOs, for DTR or otherwise).
Yes, that is our goal and in our solution they don=92t need to make ass=
umptions
about each other. We just define in the DT: this GTA04 board has UART2-=
DTR
connected to the W2SG-GPIO# - instead of OMAP3-GPIO145.

In our solution the UART driver knows that there could be a DTR GPIO (s=
imilar to
a RTS GPIO which is already provided by the omap-serial RS484 bindings)=
=2E Which
could be connected to some GPIO which drives the real DTR wire of a RS2=
32 level
shifter.
=20
Post by Dr. H. Nikolaus Schaller
Then you could have a mechanism whereby the UART
driver can notify the other device driver regarding events (e.g. th=
e
Post by Dr. H. Nikolaus Schaller
UART being opened for access), or the other driver could claim owne=
rship
Post by Dr. H. Nikolaus Schaller
of the UART and expose its own interface to userspace.
=20
That would be independent of the particular UART or other device, a=
nd
Post by Dr. H. Nikolaus Schaller
the only description necessary in the DT would be an accurate
representation of the way the hardware is wired.
=20
There are a few ways that could be done, but I suspect the simplest=
is
Post by Dr. H. Nikolaus Schaller
to just have the device as a sub-node of the UART, like we do for S=
PI or
Post by Dr. H. Nikolaus Schaller
=20
compatible =3D "vendor,uart";
reg =3D <0xf00 0x100>;
...
=20
gps {
compatible =3D "wi2wi,w2sg0004";
...
};
};
=20
That wouldn't work for devices with multiple UART connections. Do t=
hose
Post by Dr. H. Nikolaus Schaller
exist, and are they common in configurations where out-of-band
management is necessary (e.g. regulators, clocks)?
=20
UARTs are usually point to point interfaces and not busses. So there=
is
Post by Dr. H. Nikolaus Schaller
no need to describe the interface.
=20
I don't follow. You have a device which seems to require management
kernel-side.
Yes, power on/off.
Rather than describing the interface, you've described a
fictitious relationship between the GPS device and the UART's DTR GPI=
O,
and you=92ve described a fictitious GPIO to hand to the UART driver.
Yes. Because the UART driver would generally expect a GPIO (if a GPIO d=
riven
DTR hardware line is available on the connector).
This
is how you have linked the two in order to get the behaviour you want=
=2E

Exactly.
=20
So it _is_ necessary to describe the interface. Rather than describin=
g
that interface at a high level you've chosen to hack together a set o=
f
fake relationships to work around the lack of ability to describe sai=
d
interface.
The power control interface is a single GPIO from UART to the driver.
The other end is an interface from the driver to the pinmux of the OMAP
and the chip. So it is a =93middleman=94. But I think this is already c=
lear.
=20
Post by Dr. H. Nikolaus Schaller
And I would speculate that in most cases they simply go to some
connector and therefore no connected chip that needs to be described
in the DT at all. Because it has a user-space driver (e.g. AT
commands) and no kernel driver.
=20
In the case where no driver is necessary I agree that no description =
is
necessary, though the description could be exposed in a helpful way t=
o
userspace to describe what=92s attached to which UARTs.
That is usually done by configuring the gpsd process.
=20
However, in this case you do have a kernel driver (even if basic), an=
d
it requires some knowledge of the relationship between the device and
the UART in order to function.
Yes. And that is what we want to provide by the dtb file. That one shou=
ld describe
the relationship.
=20
Post by Dr. H. Nikolaus Schaller
But we have no idea how such a solution could be implemented or test=
ed.
=20
I would disagree on that point, given I provided a high level
description of how this could be implemented.
We do understand how you suggest the bindings should look like, but we =
have
no idea how that translates into code.

And we do not want to touch the general serial drivers, just the omap-s=
erial
(because the solutions does not work without an UART behind a pinmux wi=
th a
GPIO with interrupt capabilities).

We simply have no experience modifying serial drivers (Linux is too big=
that anyone
can know all areas).

Our experience is limited to re-submitting the DTR-GPIO-control patch b=
y Neil Brown
and making it read DT properties instead of board file.

So your approach needs a lot of help to really implement it. The questi=
on is who
will be doing it.
=20
Post by Dr. H. Nikolaus Schaller
If someone adds that to the serial drivers, we would be happy to use=
it,
Post by Dr. H. Nikolaus Schaller
but unless such a thing exists, I think our solution is quite simple=
and isolated
Post by Dr. H. Nikolaus Schaller
into this single driver and also uses existing standard interfaces (=
gpios, pinmux).
=20
I would argue that this _abuses_ standard interfaces, as you have one
device driver fiddling with resources logically owned by another.
=20
Post by Dr. H. Nikolaus Schaller
The reason to solve it that way is that we did not want to have a =
direct link
Post by Dr. H. Nikolaus Schaller
between this driver and any serial drivers or other mechanisms how=
drivers
Post by Dr. H. Nikolaus Schaller
can detect that their serial port (/dev/tty*) is opened.
=20
It is used to power down the w2sg GPS chip if no user space proces=
s is
Post by Dr. H. Nikolaus Schaller
accessing its serial port (or de-asserts DTR through tcsetattr/ioc=
tl).
Post by Dr. H. Nikolaus Schaller
=20
Post by Mark Rutland
=20
+
+ pinctrl-names =3D "default", "monitor";
+ pinctrl-0 =3D <&uart2_pins>;
+ pinctrl-1 =3D <&uart2_rx_irq_pins>;
+
+ interrupt-parent =3D <&gpio5>;
+ interrupts =3D <19 IRQ_TYPE_EDGE_FALLING>; /* =
GPIO_147: RX - trigger on arrival of start bit */
Post by Dr. H. Nikolaus Schaller
Post by Mark Rutland
=20
While interrupts is a standard property, please describe above ho=
w many
Post by Dr. H. Nikolaus Schaller
Post by Mark Rutland
you expect and what their logical function is.
=20
The only part I'm confused about is how the link to the UART is
described. I assume I'm just ignorant of some existing pattern.
=20
The serial link itself is not described at all because it is assum=
ed to be a =84must have=93.
Post by Dr. H. Nikolaus Schaller
=20
Huh? So it's a "must have" that you "don't have" in the DT?
=20
Yes. The DT does not describe everything. Only those things that nee=
d
Post by Dr. H. Nikolaus Schaller
a kernel driver. And UARTs usually have user-space drivers (e.g. gps=
d,
Post by Dr. H. Nikolaus Schaller
gsmd, pppd) and ioctl/tcsetattr.
=20
The DT should describe the static portions of the hardware. Typically=
we
only have devices with kernelspace drivers described simply because
that's the way people have built DTs. Whether or not you have a
kernelspace driver can change over time, the organisation of the
hardware cannot.
So far none of the DT I have seen describe what is connected to the UAR=
T.
Even for boards which use HCI to communicate with a Bluetooth module.
=20
Post by Dr. H. Nikolaus Schaller
I think that the relationship is being described incorrectly in the=
DT,
Post by Dr. H. Nikolaus Schaller
and I think that there is a more general problem that needs to be
addressed in order to make this case work.
=20
The driver only needs to monitor the RX line and needs to switch i=
t between UART and
Post by Dr. H. Nikolaus Schaller
GPIO/IRQ mode. So this monitoring switch is described (with two di=
fferent pinctrl states).
Post by Dr. H. Nikolaus Schaller
=20
While this particular driver only needs that at this point in time,
that's not necessarily true of drivers for similar devices, nor is =
it
Post by Dr. H. Nikolaus Schaller
necessarily true if we need to add additional features to this driv=
er.
Post by Dr. H. Nikolaus Schaller
=20
Which features are you thinking of to add to this driver? And do
similar devices exist at all? Since we have not found any, we have
declared it as a "misc=93 driver.
=20
I don't have any particular feature in mind.
=20
I am not immediately aware of other serial devices which require
out-of-band management in the same way, though we have a vaguely simi=
lar
case with SDIO devices which must be powered up before they appear on
the bus.
AFAIK, they are usually descibed by a regulator that is enabled/disable=
d by
the SDIO driver. And the regulator is usually defined as a gpio-regulat=
or to
drive an GPIO.

And I think the SDIO driver has a reset-gpio (which can also be interpr=
eted
as a disable/power-down).

So such interface drivers simply expect that they can power-control dep=
endent
devices through a regulator or a simple gpio.
In that case I believe the intent is to describe them in the DT
under the bus.
Or would it be ok to allow a regulator property for the serial interfac=
e? Then
our driver would become a w2sg0004-regulator driver similar to a gpio-r=
egulator
but with a state machine that knows how to pulse the gpio until power i=
s applied
to the GPS receiver internals.
=20
Post by Dr. H. Nikolaus Schaller
Describing the relationship leaves a lot more freedom to improve th=
ings
Post by Dr. H. Nikolaus Schaller
without having to update every DTB.
=20
We know that it is a little tricky to control this chip correctly =
- and we think this solution
Post by Dr. H. Nikolaus Schaller
is the most general (no direct dependency on the serial line, and =
just to pinmux states
Post by Dr. H. Nikolaus Schaller
and an interrupt).
=20
I think that the rough approach I sketched out above is more genera=
l,
Post by Dr. H. Nikolaus Schaller
and I think that you must describe the relationship with the serial
line.
=20
It's not clear to me whether the interrupt you describe is attached=
to
Post by Dr. H. Nikolaus Schaller
the GPS, or if this is logically part of the UART.
=20
The interrupt is needed to simulate the glue logic connected between
UART and GPS.
=20
The output signal comes from the GPS module and goes to some pad
of the OMAP3 SoC. This pad can be either multiplexed into the UART R=
X
Post by Dr. H. Nikolaus Schaller
input or into a GPIO bank of the OMAP. That GPIO controller can gene=
rate
Post by Dr. H. Nikolaus Schaller
the interrupt on incoming data (when none is expected).
=20
Therefore it is a GPS-generated interrupt and has nothing to do with
the UART.
=20
Ok. When does the GPS device raise this interrupt?
If the driver assumes the GPS receiver is turned off, it disconnedts th=
e RX
wire from the UART to a GPIO by using two different pinmux states.

If the GPIO raises an interrupt, the driver knows that the GPS module i=
s not
turned off, because the driver has lost knowledge about its state. This=
is not
an UART related function but OMAP pinmux and GPIO.

I am not sure if that could even be implemented at all by a UART depend=
ent
driver (since the UART is shut down and does not interrupt).
=20
Thanks,
Mark.
Thanks as well - it is a fruitful discussion (even if it becomes length=
y because
I might have repeated a lot of things that are already clear. Please ac=
cept an apology).

I think we just disagree about implementing a version that =93works=94 =
in a quite specific
setup (there are necessary assumptions about OMAP pinmux and omap-seria=
l) with
existing APIs versus a general one that might need a lot of changes out=
side the driver
and introduce new APIs.

BR,
Nikolaus

Arnd Bergmann
2014-10-19 19:51:27 UTC
Permalink
Post by Marek Belisko
This is a driver for the Wi2Wi GPS modules connected through an UART.
The tricky part is that the module is turned on or off by an impulse
on the control line - but it is only possible to get the real state by monitoring
the UART RX input. Since the kernel can't know in which status the module
is brought e.g. by a boot loader or after suspend, we need some logic to handle
this.
a) through rfkill
b) as a GPIO
The GPIO enable can be plumbed to other drivers that expect to be able to control
a GPIO. On the GTA04 board this is the DTR-gpio of the connected UART so that
opening the UART device enables the receiver and closing it shuts the receiver down.
Original implementation by Neil Brown, fixes + DT bindings by H. Nikolaus Schaller
I'm not sure if this is a good way to model the device. You say that it's
connected to a UART, but the code itself has no reference to the tty layer
at all. I assume the actual driver is in user space and it would open the
UART and this control device as separate instances handles and then try
to coordinate them. Is that right?

What is the protocol for the GPS driver itself? Is it standardized?
Would it help to have a TTY line discipline for the GPS mode instead
so the power management and startup could be integrated into the serial
port management instead?

Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Dr. H. Nikolaus Schaller
2014-10-19 20:29:27 UTC
Permalink
Hi,
This is a driver for the Wi2Wi GPS modules connected through an UART=
=2E
The tricky part is that the module is turned on or off by an impulse
on the control line - but it is only possible to get the real state =
by monitoring
the UART RX input. Since the kernel can't know in which status the m=
odule
is brought e.g. by a boot loader or after suspend, we need some logi=
c to handle
this.
=20
The driver allows two different methods to enable (and power up) GPS=
a) through rfkill
b) as a GPIO
=20
The GPIO enable can be plumbed to other drivers that expect to be ab=
le to control
a GPIO. On the GTA04 board this is the DTR-gpio of the connected UAR=
T so that
opening the UART device enables the receiver and closing it shuts th=
e receiver down.
=20
Original implementation by Neil Brown, fixes + DT bindings by H. Nik=
olaus Schaller
=20
I=92m not sure if this is a good way to model the device.
It is the easiest way we have found after ca. 2 years of working on it =
:)
You say that it's
connected to a UART, but the code itself has no reference to the tty =
layer
at all.
Yes.
I assume the actual driver is in user space and it would open the
UART and this control device as separate instances handles and then t=
ry
to coordinate them. Is that right?
Yes, it is called gpsd.
=20
What is the protocol for the GPS driver itself? Is it standardized?
Yes, NMEA records. Like most GPS receivers provide them.
Would it help to have a TTY line discipline for the GPS mode instead
so the power management and startup could be integrated into the seri=
al
port management instead?
I don=92t think line disciplines (as far as I understand them) are help=
ful and
this chip is not special at all regarding the serial interface data. So=
it needs no
=93GPS mode=94.

It is very similar to connecting some external handheld GPS receiver by=
a
RS232 cable or RS232-USB adapter. Or GPS =93mouse=94 receivers through
bluetooth.

They all create/show some /dev/tty that you simply open/read/close. And
there is no special processing of the serial data involved.

The only thing this driver needs to solve is to properly power up/down
the chip on demand.

The DTR line of the tty can enable/disable power of a connected device
(being an external modem or this GPS chip). This is what we have made
the driver compatible to by providing a virtual GPIO to enable/disable.

This is done by a patch to the tty driver that already was in the kerne=
l
but removed in 3.15 because the w2sg driver wasn=92t submitted at that
time.

Commit: 985bfd54c826c0ba873ca0adfd5589263e0c6ee2

Basically, if our CPU would have a real RS232 interface and we would
put the chip into an external device, it should look exactly the same i=
f
we look at the serial interface part. Since soldering it on the same PC=
B
or connecting through a connector should not need different serial driv=
ers.
Just different device tree files.

Therefore we have decided not to touch any serial driver code at all,
because it is not needed (except allowing DTR to control some GPIO)
and keeps it simple and manageable.

BR,
Nikolaus


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