Re: [PATCH v2 18/24] serdev: ttydev: Serdev driver that creates an standard TTY port

[Ricardo Ribalda Delgado]

Hi Rob,
On Wed, Jun 13, 2018 at 3:20 AM Rob Herring  wrote:
>
> On Mon, Jun 11, 2018 at 5:52 AM, Ricardo Ribalda Delgado
>  wrote:
> > Standard TTY port that can be loaded/unloaded via serdev sysfs. This
> > serdev driver can only be used by serdev controllers that are compatible
> > with ttyport.
>
> I'm hesitant to expose a tty device on top of serdev to userspace that
> we then have to support forever unless that's the only way tty devices
> (for serial ports) are created.

My concern is that, with the current implementation, serdev does have
a tiny collection of usecases:

-It cannot be used for board bring up
-It cannot be used as a replacement of hciattach and friends

It can only be used on embedded devices or platforms where the
developer has control of the ACPI table.

This hack, allows its use in almost any scenario and I have been
happily using it for two weeks with no issue.
It is also a very simple solution that does not have the issues of
cdev/serdev coexistence that you mention.

I am not very convinced about all the ttydev being serdevs. Adding 1K
lines of code to people that do not plan to use serdev seems like a
high expense. If in the future we can get rid of all the hciattach
programs then we can redesign the port probing.

>
> I did a similar patch which just registered both serdev and a tty
> allowing them to coexist. It suffered from warnings about open counts
> though and felt hacky.
>
> Rob



-- 
Ricardo Ribalda

Re: [PATCH v2 18/24] serdev: ttydev: Serdev driver that creates an standard TTY port

[Rob Herring]

On Mon, Jun 11, 2018 at 5:52 AM, Ricardo Ribalda Delgado
 wrote:
> Standard TTY port that can be loaded/unloaded via serdev sysfs. This
> serdev driver can only be used by serdev controllers that are compatible
> with ttyport.

I'm hesitant to expose a tty device on top of serdev to userspace that
we then have to support forever unless that's the only way tty devices
(for serial ports) are created.

I did a similar patch which just registered both serdev and a tty
allowing them to coexist. It suffered from warnings about open counts
though and felt hacky.

Rob

[PATCH v2 18/24] serdev: ttydev: Serdev driver that creates an standard TTY port

[Ricardo Ribalda Delgado]

Standard TTY port that can be loaded/unloaded via serdev sysfs. This
serdev driver can only be used by serdev controllers that are compatible
with ttyport.

Cc: Rob Herring 
Cc: Johan Hovold 
Cc: Greg Kroah-Hartman 
Cc: Jiri Slaby 
Cc: Andy Shevchenko 
Signed-off-by: Ricardo Ribalda Delgado 
---
 drivers/tty/serdev/Kconfig | 10 +
 drivers/tty/serdev/Makefile|  2 +
 drivers/tty/serdev/serdev-ttydev.c | 60 ++
 3 files changed, 72 insertions(+)
 create mode 100644 drivers/tty/serdev/serdev-ttydev.c

diff --git a/drivers/tty/serdev/Kconfig b/drivers/tty/serdev/Kconfig
index 1dbc8352e027..848692caabd6 100644
--- a/drivers/tty/serdev/Kconfig
+++ b/drivers/tty/serdev/Kconfig
@@ -21,4 +21,14 @@ config SERIAL_DEV_CTRL_TTYPORT
depends on SERIAL_DEV_BUS != m
default y
 
+config SERIAL_DEV_CTRL_TTYDEV
+   tristate "TTY port dynamically loaded by the Serial Device Bus"
+   help
+ Say Y here if you want to create a bridge driver between the Serial
+ device bus and the TTY chardevice. This driver can be dynamically
+ loaded/unloaded by the Serial Device Bus.
+
+ If unsure, say N.
+   depends on SERIAL_DEV_CTRL_TTYPORT
+
 endif
diff --git a/drivers/tty/serdev/Makefile b/drivers/tty/serdev/Makefile
index 0cbdb9444d9d..5c807b77d12d 100644
--- a/drivers/tty/serdev/Makefile
+++ b/drivers/tty/serdev/Makefile
@@ -3,3 +3,5 @@ serdev-objs := core.o
 obj-$(CONFIG_SERIAL_DEV_BUS) += serdev.o
 
 obj-$(CONFIG_SERIAL_DEV_CTRL_TTYPORT) += serdev-ttyport.o
+
+obj-$(CONFIG_SERIAL_DEV_CTRL_TTYDEV) += serdev-ttydev.o
diff --git a/drivers/tty/serdev/serdev-ttydev.c 
b/drivers/tty/serdev/serdev-ttydev.c
new file mode 100644
index ..180035e101dc
--- /dev/null
+++ b/drivers/tty/serdev/serdev-ttydev.c
@@ -0,0 +1,60 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Ricardo Ribalda 
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "serport.h"
+
+static int ttydev_serdev_probe(struct serdev_device *serdev)
+{
+   struct serdev_controller *ctrl = serdev->ctrl;
+   struct serport *serport;
+   struct device *dev;
+
+   if (!ctrl->is_ttyport)
+   return -ENODEV;
+
+   serport = serdev_controller_get_drvdata(ctrl);
+
+   dev = tty_register_device_attr(serport->tty_drv, serport->tty_idx,
+   &serdev->dev, NULL, NULL);
+
+   return dev ? 0 : PTR_ERR(dev);
+}
+
+static void ttydev_serdev_remove(struct serdev_device *serdev)
+{
+   struct serdev_controller *ctrl = serdev->ctrl;
+   struct serport *serport;
+
+   serport = serdev_controller_get_drvdata(ctrl);
+   tty_unregister_device(serport->tty_drv, serport->tty_idx);
+}
+
+static const struct serdev_device_id ttydev_serdev_id[] = {
+   { "ttydev", },
+   { "ttydev_serdev", },
+   {}
+};
+MODULE_DEVICE_TABLE(serdev, ttydev_serdev_id);
+
+static struct serdev_device_driver ttydev_serdev_driver = {
+   .probe = ttydev_serdev_probe,
+   .remove = ttydev_serdev_remove,
+   .driver = {
+   .name = "ttydev_serdev",
+   },
+   .id_table = ttydev_serdev_id,
+};
+
+module_serdev_device_driver(ttydev_serdev_driver);
+
+MODULE_AUTHOR("Ricardo Ribalda ");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Serdev to ttydev module");
-- 
2.17.1