On Wed, Nov 15, 2017 at 4:19 PM, H. Nikolaus Schaller <hns@xxxxxxxxxxxxx> 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 <neilb@xxxxxxx>
> but simplified and adapted to use the new serdev API introduced in 4.11.
>
> Signed-off-by: H. Nikolaus Schaller <hns@xxxxxxxxxxxxx>
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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
