On Wed, Nov 15, 2017 at 4:19 PM, H. Nikolaus Schaller [off-list ref] 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/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 [off-list ref]
but simplified and adapted to use the new serdev API introduced in 4.11.

Signed-off-by: H. Nikolaus Schaller <redacted>

I'm a bit confused by the concept here. Did I understand it right that this
attaches to a tty_port and then registers another tty_driver with one
tty_port for the first port?

+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;
+       }
+
+       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;

This looks like it's a leftover from pre-DT days, but it doesn't
actually work without DT in the current form. How about
merging the contents of w2sg_pdata into w2sg_data?

+
+       /* 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;
+       /*
+        * optional:
+        * 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);

While I'm still not sure about why we want nested tty port, it
seems that we should have only one tty_driver that gets initialized
at module load time, rather than one driver structure per port.

+       /* 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;
+       }
+
+       tty_port_init(&data->port);
+       data->port.ops = &w2sg_port_ops;
+
+/*
+ * 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);

This seems to be a result of having nested tty ports, and both
ports point to the same device.

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