[PATCH 00/19] Dynamically load/remove serdev devices via sysfs*

[PATCH 00/19] Dynamically load/remove serdev devices via sysfs*

[Ricardo Ribalda Delgado]

There are some situations where it is interesting to load/remove serdev
devices dynamically, like during board bring-up or when we are
developing a new driver or for devices that are neither described via
ACPI or device tree.

This implementation allows the creation of serdev devices via sysfs,
in a similar way as the i2c bus allows sysfs instantiation [1].

It also opens the door to create platform drivers for specific
platforms, such us notebooks or industrial computers, when their serial
devices are not described via DT or ACPI.

This patchset also supports automatic module load via udev/modprobe,
simplifying its use by the final user.

Currently, serdev does not manage ports that do not have a serdev device
defined at boot time. This implementation adds a new serdev module,
ttydev, that provides the same functionality of tty (it calls the
original one), but can be unloaded.

TL/DR:

When we want to create a new device, we just run:
root@qt5022:~# echo ttydev > /sys/bus/serial/devices/serial0/new_device

This will create a new device:
root@qt5022:~# ls /sys/bus/serial/devices/serial0-0/
driver	modalias  power  subsystem  tty  uevent

And load the required driver to use it:
root@qt5022:~# lsmod | grep serdev_ttydev
serdev_ttydev          16384  0

The device can be removed:
root@qt5022:~#
	echo serial0-0 > /sys/bus/serial/devices/serial0/delete_device

And now we can connect a new device:
root@qt5022:~# echo hci-ti > /sys/bus/serial/devices/serial0/new_device


[1] https://www.kernel.org/doc/Documentation/i2c/instantiating-devices

Ricardo Ribalda Delgado (19):
  serdev: Add id_table to serdev_device_driver
  Bluetooth: hci_bcm: Add serdev_id_table
  Bluetooth: hci_ll: Add serdev_id_table
  Bluetooth: hci_nokia: Add serdev_id_table
  serdev: Introduce modalias field
  serdev: Support bus matching with modalias field
  serdev: Allows dynamic creation of devices via sysfs
  serdev: Provide modalias attribute for modalias devices
  serdev: Provide modalias uevent for modalias devices
  file2alias: Support for serdev devices
  Bluetooth: hci_bcm: MODULE_DEVICE_TABLE(serdev)
  Bluetooth: hci_ll: MODULE_DEVICE_TABLE(serdev)
  Bluetooth: hci_nokia: MODULE_DEVICE_TABLE(serdev)
  mfd: rave-sp: MODULE_DEVICE_TABLE(serdev)
  net: qualcomm: MODULE_DEVICE_TABLE(serdev)
  serdev: ttyport: Move serport structure to its own header
  serdev: Mark controllers compatible with ttyport
  serdev: ttydev: Serdev driver that creates an standard TTY port
  serdev: Instantiate a ttydev serdev if acpi and of fails

 drivers/bluetooth/hci_bcm.c              |   8 ++
 drivers/bluetooth/hci_ll.c               |  19 ++++
 drivers/bluetooth/hci_nokia.c            |   8 ++
 drivers/mfd/rave-sp.c                    |   7 ++
 drivers/net/ethernet/qualcomm/qca_uart.c |   7 ++
 drivers/tty/serdev/Kconfig               |  11 ++
 drivers/tty/serdev/Makefile              |   2 +
 drivers/tty/serdev/core.c                | 133 +++++++++++++++++++++--
 drivers/tty/serdev/serdev-ttydev.c       |  70 ++++++++++++
 drivers/tty/serdev/serdev-ttyport.c      |  10 +-
 drivers/tty/serdev/serport.h             |  16 +++
 include/linux/mod_devicetable.h          |  10 ++
 include/linux/serdev.h                   |   4 +
 scripts/mod/devicetable-offsets.c        |   3 +
 scripts/mod/file2alias.c                 |  11 ++
 15 files changed, 304 insertions(+), 15 deletions(-)
 create mode 100644 drivers/tty/serdev/serdev-ttydev.c
 create mode 100644 drivers/tty/serdev/serport.h

-- 
2.17.0

[PATCH 01/19] serdev: Add id_table to serdev_device_driver

[Ricardo Ribalda Delgado]

Currently, serdev device driver can only be used with devices that are
nodes of a device tree, or are part of the ACPI table.

Id_table will be used for devices that are created at runtime or that
are not part of the device tree nor the ACPI table.

Cc: Rob Herring <robh@kernel.org>
Cc: Johan Hovold <johan@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 include/linux/mod_devicetable.h | 10 ++++++++++
 include/linux/serdev.h          |  2 ++
 2 files changed, 12 insertions(+)

diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index 7d361be2e24f..1877a4e43f1b 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -448,6 +448,16 @@ struct pci_epf_device_id {
 	kernel_ulong_t driver_data;
 };
 
+/* serdev */
+
+#define SERDEV_NAME_SIZE	32
+#define SERDEV_MODULE_PREFIX	"serdev:"
+
+struct serdev_device_id {
+	char name[SERDEV_NAME_SIZE];
+	kernel_ulong_t driver_data;	/* Data private to the driver */
+};
+
 /* spi */
 
 #define SPI_NAME_SIZE	32
diff --git a/include/linux/serdev.h b/include/linux/serdev.h
index f153b2c7f0cd..62f1b085a794 100644
--- a/include/linux/serdev.h
+++ b/include/linux/serdev.h
@@ -15,6 +15,7 @@
 
 #include <linux/types.h>
 #include <linux/device.h>
+#include <linux/mod_devicetable.h>
 #include <linux/termios.h>
 #include <linux/delay.h>
 
@@ -68,6 +69,7 @@ static inline struct serdev_device *to_serdev_device(struct device *d)
  * @remove:	unbinds this driver from the serdev device.
  */
 struct serdev_device_driver {
+	const struct serdev_device_id *id_table;
 	struct device_driver driver;
 	int	(*probe)(struct serdev_device *);
 	void	(*remove)(struct serdev_device *);
-- 
2.17.0

[PATCH 02/19] Bluetooth: hci_bcm: Add serdev_id_table

[Ricardo Ribalda Delgado]

Describe which hardware is supported by the current driver.

Cc: Marcel Holtmann <marcel@holtmann.org>
Cc: Johan Hedberg <johan.hedberg@gmail.com>
Cc: Rob Herring <robh@kernel.org>
Cc: Johan Hovold <johan@kernel.org>
Cc: linux-bluetooth@vger.kernel.org
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/bluetooth/hci_bcm.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index 441f5e1deb11..f4d7846c06b8 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -1325,6 +1325,11 @@ static const struct of_device_id bcm_bluetooth_of_match[] = {
 MODULE_DEVICE_TABLE(of, bcm_bluetooth_of_match);
 #endif
 
+static const struct serdev_device_id bcm_serdev_id[] = {
+	{ "bcm43438-bt", },
+	{}
+};
+
 static struct serdev_device_driver bcm_serdev_driver = {
 	.probe = bcm_serdev_probe,
 	.remove = bcm_serdev_remove,
@@ -1334,6 +1339,7 @@ static struct serdev_device_driver bcm_serdev_driver = {
 		.acpi_match_table = ACPI_PTR(bcm_acpi_match),
 		.pm = &bcm_pm_ops,
 	},
+	.id_table = bcm_serdev_id,
 };
 
 int __init bcm_init(void)
-- 
2.17.0

[PATCH 03/19] Bluetooth: hci_ll: Add serdev_id_table

[Ricardo Ribalda Delgado]

Describe which hardware is supported by the current driver.

Cc: Marcel Holtmann <marcel@holtmann.org>
Cc: Johan Hedberg <johan.hedberg@gmail.com>
Cc: Rob Herring <robh@kernel.org>
Cc: Johan Hovold <johan@kernel.org>
Cc: linux-bluetooth@vger.kernel.org
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/bluetooth/hci_ll.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/bluetooth/hci_ll.c b/drivers/bluetooth/hci_ll.c
index 27e414b4e3a2..c31942c9b466 100644
--- a/drivers/bluetooth/hci_ll.c
+++ b/drivers/bluetooth/hci_ll.c
@@ -776,6 +776,22 @@ static const struct of_device_id hci_ti_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, hci_ti_of_match);
 
+static struct serdev_device_id hci_ti_id[] = {
+	{ "cc2560", },
+	{ "wl1271-st", },
+	{ "wl1273-st", },
+	{ "wl1281-st", },
+	{ "wl1283-st", },
+	{ "wl1285-st", },
+	{ "wl1801-st", },
+	{ "wl1805-st", },
+	{ "wl1807-st", },
+	{ "wl1831-st", },
+	{ "wl1835-st", },
+	{ "wl1837-st", },
+	{},
+};
+
 static struct serdev_device_driver hci_ti_drv = {
 	.driver		= {
 		.name	= "hci-ti",
@@ -783,6 +799,7 @@ static struct serdev_device_driver hci_ti_drv = {
 	},
 	.probe	= hci_ti_probe,
 	.remove	= hci_ti_remove,
+	.id_table = hci_ti_id,
 };
 #else
 #define ll_setup NULL
-- 
2.17.0

[PATCH 04/19] Bluetooth: hci_nokia: Add serdev_id_table

[Ricardo Ribalda Delgado]

Describe which hardware is supported by the current driver.

Cc: Marcel Holtmann <marcel@holtmann.org>
Cc: Johan Hedberg <johan.hedberg@gmail.com>
Cc: Rob Herring <robh@kernel.org>
Cc: Johan Hovold <johan@kernel.org>
Cc: linux-bluetooth@vger.kernel.org
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/bluetooth/hci_nokia.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/bluetooth/hci_nokia.c b/drivers/bluetooth/hci_nokia.c
index 3539fd03f47e..e32dfcd56b8d 100644
--- a/drivers/bluetooth/hci_nokia.c
+++ b/drivers/bluetooth/hci_nokia.c
@@ -801,6 +801,11 @@ static const struct of_device_id nokia_bluetooth_of_match[] = {
 MODULE_DEVICE_TABLE(of, nokia_bluetooth_of_match);
 #endif
 
+static struct serdev_device_id nokia_bluetooth_serdev_id[] = {
+	{ "hp4-bluetooth", },
+	{},
+};
+
 static struct serdev_device_driver nokia_bluetooth_serdev_driver = {
 	.probe = nokia_bluetooth_serdev_probe,
 	.remove = nokia_bluetooth_serdev_remove,
@@ -809,6 +814,7 @@ static struct serdev_device_driver nokia_bluetooth_serdev_driver = {
 		.pm = &nokia_bluetooth_pm_ops,
 		.of_match_table = of_match_ptr(nokia_bluetooth_of_match),
 	},
+	.id_table = nokia_bluetooth_serdev_id,
 };
 
 module_serdev_device_driver(nokia_bluetooth_serdev_driver);
-- 
2.17.0

[PATCH 05/19] serdev: Introduce modalias field

[Ricardo Ribalda Delgado]

Name of the driver to use with this device, or an alias for that name,
or an alias for the part.
Required for hardware that is neither an of_node nor part of the ACPI
table.

Cc: Rob Herring <robh@kernel.org>
Cc: Johan Hovold <johan@kernel.org>
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 include/linux/serdev.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/serdev.h b/include/linux/serdev.h
index 62f1b085a794..bb3b9599c652 100644
--- a/include/linux/serdev.h
+++ b/include/linux/serdev.h
@@ -54,6 +54,7 @@ struct serdev_device {
 	const struct serdev_device_ops *ops;
 	struct completion write_comp;
 	struct mutex write_lock;
+	char modalias[SERDEV_NAME_SIZE];
 };
 
 static inline struct serdev_device *to_serdev_device(struct device *d)
-- 
2.17.0

[PATCH 06/19] serdev: Support bus matching with modalias field

[Ricardo Ribalda Delgado]

Match devices to drivers by their modalias when the ACPI and the OF
match fails.

Cc: Rob Herring <robh@kernel.org>
Cc: Johan Hovold <johan@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.com>
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/tty/serdev/core.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index df93b727e984..2c79f47fc0db 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -79,8 +79,23 @@ static const struct device_type serdev_ctrl_type = {
 	.release	= serdev_ctrl_release,
 };
 
+static int serdev_match_id(const struct serdev_device_id *id,
+			   const struct serdev_device *sdev)
+{
+	while (id->name[0]) {
+		if (!strcmp(sdev->modalias, id->name))
+			return 1;
+		id++;
+	}
+
+	return 0;
+}
+
 static int serdev_device_match(struct device *dev, struct device_driver *drv)
 {
+	const struct serdev_device *sdev = to_serdev_device(dev);
+	const struct serdev_device_driver *sdrv = to_serdev_device_driver(drv);
+
 	if (!is_serdev_device(dev))
 		return 0;
 
@@ -88,7 +103,13 @@ static int serdev_device_match(struct device *dev, struct device_driver *drv)
 	if (acpi_driver_match_device(dev, drv))
 		return 1;
 
-	return of_driver_match_device(dev, drv);
+	if (of_driver_match_device(dev, drv))
+		return 1;
+
+	if (sdrv->id_table)
+		return serdev_match_id(sdrv->id_table, sdev);
+
+	return strcmp(sdev->modalias, drv->name) == 0;
 }
 
 /**
-- 
2.17.0

[PATCH 07/19] serdev: Allows dynamic creation of devices via sysfs

[Ricardo Ribalda Delgado]

Allow creating and deleting devices via sysfs. Devices created will be
matched to serdev drivers via modalias (the string provided by the user)
and deleted via their name. Eg:

 # Create device
root@qt5022:~# echo ttydev > /sys/bus/serial/devices/serial0/new_device

 # Delete device
root@qt5022:~#
	echo serial0-0 > /sys/bus/serial/devices/serial0/delete_device

Cc: Rob Herring <robh@kernel.org>
Cc: Johan Hovold <johan@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.com>
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/tty/serdev/core.c | 60 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index 2c79f47fc0db..e695fa649a6d 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -75,7 +75,67 @@ static void serdev_ctrl_release(struct device *dev)
 	kfree(ctrl);
 }
 
+static ssize_t
+new_device_store(struct device *dev, struct device_attribute *attr,
+			const char *buf, size_t count)
+{
+	struct serdev_controller *ctrl = to_serdev_controller(dev);
+	struct serdev_device *serdev;
+	int err;
+	char *nline;
+
+	serdev = serdev_device_alloc(ctrl);
+	if (!serdev)
+		return -ENOMEM;
+
+	nline = strchr(buf, '\n');
+	if (nline)
+		*nline = '\0';
+
+	strncpy(serdev->modalias, buf, SERDEV_NAME_SIZE);
+
+	err = serdev_device_add(serdev);
+	if (err) {
+		serdev_device_put(serdev);
+		return err;
+	}
+
+	return count;
+}
+static DEVICE_ATTR_WO(new_device);
+
+static ssize_t
+delete_device_store(struct device *dev, struct device_attribute *attr,
+			   const char *buf, size_t count)
+{
+	struct serdev_controller *ctrl = to_serdev_controller(dev);
+	struct serdev_device *serdev = ctrl->serdev;
+	char *nline;
+
+	nline = strchr(buf, '\n');
+	if (nline)
+		*nline = '\0';
+
+	if (!ctrl->serdev ||
+	    strncmp(dev_name(&serdev->dev), buf, SERDEV_NAME_SIZE))
+		return -ENODEV;
+
+	serdev_device_remove(serdev);
+
+	return count;
+}
+static DEVICE_ATTR_IGNORE_LOCKDEP(delete_device, 0200, NULL,
+				  delete_device_store);
+
+static struct attribute *serdev_ctrl_attrs[] = {
+	&dev_attr_new_device.attr,
+	&dev_attr_delete_device.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(serdev_ctrl);
+
 static const struct device_type serdev_ctrl_type = {
+	.groups		= serdev_ctrl_groups,
 	.release	= serdev_ctrl_release,
 };
 
-- 
2.17.0

[PATCH 08/19] serdev: Provide modalias attribute for modalias devices

[Ricardo Ribalda Delgado]

Create modalias sysfs attribute for modalias devices.
This is required by modprobe/udev to autoload the serdev driver.

Eg:
root@qt5022:~# cat /sys/bus/serial/devices/serial1-0/modalias
serdev:ttydev

Cc: Rob Herring <robh@kernel.org>
Cc: Johan Hovold <johan@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.com>
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/tty/serdev/core.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index e695fa649a6d..a9c935f68076 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -23,12 +23,17 @@ static ssize_t modalias_show(struct device *dev,
 			     struct device_attribute *attr, char *buf)
 {
 	int len;
+	struct serdev_device *serdev = to_serdev_device(dev);
 
 	len = acpi_device_modalias(dev, buf, PAGE_SIZE - 1);
 	if (len != -ENODEV)
 		return len;
 
-	return of_device_modalias(dev, buf, PAGE_SIZE);
+	len = of_device_modalias(dev, buf, PAGE_SIZE);
+	if (len != -ENODEV)
+		return len;
+
+	return sprintf(buf, "%s%s\n", SERDEV_MODULE_PREFIX, serdev->modalias);
 }
 static DEVICE_ATTR_RO(modalias);
 
-- 
2.17.0

[PATCH 09/19] serdev: Provide modalias uevent for modalias devices

[Ricardo Ribalda Delgado]

Create the sysfs uevent for modalias devices. This is required by newer
versions of udev for autoload modules.

Cc: Rob Herring <robh@kernel.org>
Cc: Johan Hovold <johan@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.com>
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/tty/serdev/core.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index a9c935f68076..9414700e6442 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -46,6 +46,7 @@ ATTRIBUTE_GROUPS(serdev_device);
 static int serdev_device_uevent(struct device *dev, struct kobj_uevent_env *env)
 {
 	int rc;
+	struct serdev_device *serdev = to_serdev_device(dev);
 
 	/* TODO: platform modalias */
 
@@ -53,7 +54,11 @@ static int serdev_device_uevent(struct device *dev, struct kobj_uevent_env *env)
 	if (rc != -ENODEV)
 		return rc;
 
-	return of_device_uevent_modalias(dev, env);
+	if (rc != of_device_uevent_modalias(dev, env))
+		return rc;
+
+	return add_uevent_var(env, "MODALIAS=%s%s", SERDEV_MODULE_PREFIX,
+						    serdev->modalias);
 }
 
 static void serdev_device_release(struct device *dev)
-- 
2.17.0

[PATCH 10/19] file2alias: Support for serdev devices

[Ricardo Ribalda Delgado]

This patch allows file2alias to generate the proper module headers to
support serdev modalias drivers.

Eg:

root@qt5022:~# modinfo serdev:ttydev | grep alias
alias:          serdev:ttydev_serdev
alias:          serdev:ttydev

root@qt5022:~#
	cat /lib/modules/4.16.0-qtec-standard/modules.alias | grep serdev
alias serdev:ttydev_serdev serdev_ttydev
alias serdev:ttydev serdev_ttydev

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Philippe Ombredanne <pombredanne@nexb.com>
Cc: Rob Herring <robh@kernel.org>
Cc: Johan Hovold <johan@kernel.org>
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 scripts/mod/devicetable-offsets.c |  3 +++
 scripts/mod/file2alias.c          | 11 +++++++++++
 2 files changed, 14 insertions(+)

diff --git a/scripts/mod/devicetable-offsets.c b/scripts/mod/devicetable-offsets.c
index 9fad6afe4c41..6082c41b7ad7 100644
--- a/scripts/mod/devicetable-offsets.c
+++ b/scripts/mod/devicetable-offsets.c
@@ -142,6 +142,9 @@ int main(void)
 	DEVID(i2c_device_id);
 	DEVID_FIELD(i2c_device_id, name);
 
+	DEVID(serdev_device_id);
+	DEVID_FIELD(serdev_device_id, name);
+
 	DEVID(spi_device_id);
 	DEVID_FIELD(spi_device_id, name);
 
diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index b9beeaa4695b..dce6df3a159a 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -955,6 +955,17 @@ static int do_i2c_entry(const char *filename, void *symval,
 }
 ADD_TO_DEVTABLE("i2c", i2c_device_id, do_i2c_entry);
 
+/* Looks like: serdev:S */
+static int do_serdev_entry(const char *filename, void *symval,
+			   char *alias)
+{
+	DEF_FIELD_ADDR(symval, serdev_device_id, name);
+	sprintf(alias, SERDEV_MODULE_PREFIX "%s", *name);
+
+	return 1;
+}
+ADD_TO_DEVTABLE("serdev", serdev_device_id, do_serdev_entry);
+
 /* Looks like: spi:S */
 static int do_spi_entry(const char *filename, void *symval,
 			char *alias)
-- 
2.17.0

[PATCH 11/19] Bluetooth: hci_bcm: MODULE_DEVICE_TABLE(serdev)

[Ricardo Ribalda Delgado]

Export serdev table to the module header, allowing module autoload via
udev/modprobe.

Cc: Marcel Holtmann <marcel@holtmann.org>
Cc: Johan Hedberg <johan.hedberg@gmail.com>
Cc: Rob Herring <robh@kernel.org>
Cc: Johan Hovold <johan@kernel.org>
Cc: linux-bluetooth@vger.kernel.org
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/bluetooth/hci_bcm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index f4d7846c06b8..ff0fd3502a90 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -1327,8 +1327,10 @@ MODULE_DEVICE_TABLE(of, bcm_bluetooth_of_match);
 
 static const struct serdev_device_id bcm_serdev_id[] = {
 	{ "bcm43438-bt", },
+	{ "hci_uart_bcm", },
 	{}
 };
+MODULE_DEVICE_TABLE(serdev, bcm_serdev_id);
 
 static struct serdev_device_driver bcm_serdev_driver = {
 	.probe = bcm_serdev_probe,
-- 
2.17.0

[PATCH 12/19] Bluetooth: hci_ll: MODULE_DEVICE_TABLE(serdev)

[Ricardo Ribalda Delgado]

Export serdev table to the module header, allowing module autoload via
udev/modprobe.

Cc: Marcel Holtmann <marcel@holtmann.org>
Cc: Johan Hedberg <johan.hedberg@gmail.com>
Cc: Rob Herring <robh@kernel.org>
Cc: Johan Hovold <johan@kernel.org>
Cc: linux-bluetooth@vger.kernel.org
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/bluetooth/hci_ll.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/bluetooth/hci_ll.c b/drivers/bluetooth/hci_ll.c
index c31942c9b466..5c36d8967a00 100644
--- a/drivers/bluetooth/hci_ll.c
+++ b/drivers/bluetooth/hci_ll.c
@@ -789,8 +789,10 @@ static struct serdev_device_id hci_ti_id[] = {
 	{ "wl1831-st", },
 	{ "wl1835-st", },
 	{ "wl1837-st", },
+	{ "hci-ti", },
 	{},
 };
+MODULE_DEVICE_TABLE(serdev, hci_ti_id);
 
 static struct serdev_device_driver hci_ti_drv = {
 	.driver		= {
-- 
2.17.0

[PATCH 13/19] Bluetooth: hci_nokia: MODULE_DEVICE_TABLE(serdev)

[Ricardo Ribalda Delgado]

Export serdev table to the module header, allowing module autoload via
udev/modprobe.

Cc: Marcel Holtmann <marcel@holtmann.org>
Cc: Johan Hedberg <johan.hedberg@gmail.com>
Cc: Rob Herring <robh@kernel.org>
Cc: Johan Hovold <johan@kernel.org>
Cc: linux-bluetooth@vger.kernel.org
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/bluetooth/hci_nokia.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/bluetooth/hci_nokia.c b/drivers/bluetooth/hci_nokia.c
index e32dfcd56b8d..c283e1ca6064 100644
--- a/drivers/bluetooth/hci_nokia.c
+++ b/drivers/bluetooth/hci_nokia.c
@@ -803,8 +803,10 @@ MODULE_DEVICE_TABLE(of, nokia_bluetooth_of_match);
 
 static struct serdev_device_id nokia_bluetooth_serdev_id[] = {
 	{ "hp4-bluetooth", },
+	{ "nokia-bluetooth", },
 	{},
 };
+MODULE_DEVICE_TABLE(serdev, nokia_bluetooth_serdev_id);
 
 static struct serdev_device_driver nokia_bluetooth_serdev_driver = {
 	.probe = nokia_bluetooth_serdev_probe,
-- 
2.17.0

[PATCH 14/19] mfd: rave-sp: MODULE_DEVICE_TABLE(serdev)

[Ricardo Ribalda Delgado]

Export serdev table to the module header, allowing module autoload via
udev/modprobe.

Cc: Lee Jones <lee.jones@linaro.org>
Cc: Rob Herring <robh@kernel.org>
Cc: Johan Hovold <johan@kernel.org>
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/mfd/rave-sp.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/mfd/rave-sp.c b/drivers/mfd/rave-sp.c
index 5c858e784a89..807c237e061b 100644
--- a/drivers/mfd/rave-sp.c
+++ b/drivers/mfd/rave-sp.c
@@ -694,12 +694,19 @@ static int rave_sp_probe(struct serdev_device *serdev)
 
 MODULE_DEVICE_TABLE(of, rave_sp_dt_ids);
 
+static struct serdev_device_id rave_sp_serdev_id[] = {
+	{ "rave-sp", },
+	{},
+};
+MODULE_DEVICE_TABLE(serdev, rave_sp_serdev_id);
+
 static struct serdev_device_driver rave_sp_drv = {
 	.probe			= rave_sp_probe,
 	.driver = {
 		.name		= "rave-sp",
 		.of_match_table	= rave_sp_dt_ids,
 	},
+	.id_table = rave_sp_serdev_id,
 };
 module_serdev_device_driver(rave_sp_drv);
 
-- 
2.17.0

[PATCH 15/19] net: qualcomm: MODULE_DEVICE_TABLE(serdev)

[Ricardo Ribalda Delgado]

Export serdev table to the module header, allowing module autoload via
udev/modprobe.

Cc: Lino Sanfilippo <LinoSanfilippo@gmx.de>
Cc: David S. Miller <davem@davemloft.net>
Cc: Stefan Wahren <stefan.wahren@i2se.com>
Cc: Rob Herring <robh@kernel.org>
Cc: Johan Hovold <johan@kernel.org>
Cc: netdev@vger.kernel.org
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/net/ethernet/qualcomm/qca_uart.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/ethernet/qualcomm/qca_uart.c b/drivers/net/ethernet/qualcomm/qca_uart.c
index db6068cd7a1f..bb7aed805083 100644
--- a/drivers/net/ethernet/qualcomm/qca_uart.c
+++ b/drivers/net/ethernet/qualcomm/qca_uart.c
@@ -405,6 +405,12 @@ static void qca_uart_remove(struct serdev_device *serdev)
 	free_netdev(qca->net_dev);
 }
 
+static struct serdev_device_id qca_uart_serdev_id[] = {
+	{ QCAUART_DRV_NAME, },
+	{},
+};
+MODULE_DEVICE_TABLE(serdev, qca_uart_serdev_id);
+
 static struct serdev_device_driver qca_uart_driver = {
 	.probe = qca_uart_probe,
 	.remove = qca_uart_remove,
@@ -412,6 +418,7 @@ static struct serdev_device_driver qca_uart_driver = {
 		.name = QCAUART_DRV_NAME,
 		.of_match_table = of_match_ptr(qca_uart_of_match),
 	},
+	.id_table = qca_uart_serdev_id,
 };
 
 module_serdev_device_driver(qca_uart_driver);
-- 
2.17.0

[PATCH 16/19] serdev: ttyport: Move serport structure to its own header

[Ricardo Ribalda Delgado]

This way we can reuse this structure in other modules.

Cc: Rob Herring <robh@kernel.org>
Cc: Johan Hovold <johan@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.com>
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/tty/serdev/serdev-ttyport.c |  9 +--------
 drivers/tty/serdev/serport.h        | 16 ++++++++++++++++
 2 files changed, 17 insertions(+), 8 deletions(-)
 create mode 100644 drivers/tty/serdev/serport.h

diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c
index fa1672993b4c..4acc5f41dc67 100644
--- a/drivers/tty/serdev/serdev-ttyport.c
+++ b/drivers/tty/serdev/serdev-ttyport.c
@@ -7,17 +7,10 @@
 #include <linux/tty.h>
 #include <linux/tty_driver.h>
 #include <linux/poll.h>
+#include "serport.h"
 
 #define SERPORT_ACTIVE		1
 
-struct serport {
-	struct tty_port *port;
-	struct tty_struct *tty;
-	struct tty_driver *tty_drv;
-	int tty_idx;
-	unsigned long flags;
-};
-
 /*
  * Callback functions from the tty port.
  */
diff --git a/drivers/tty/serdev/serport.h b/drivers/tty/serdev/serport.h
new file mode 100644
index 000000000000..15bc1ff6326e
--- /dev/null
+++ b/drivers/tty/serdev/serport.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2016-2017 Linaro Ltd., Rob Herring <robh@kernel.org>
+ */
+#ifndef _SERPORT_H
+#define _SERPORT_H
+
+struct serport {
+	struct tty_port *port;
+	struct tty_struct *tty;
+	struct tty_driver *tty_drv;
+	int tty_idx;
+	unsigned long flags;
+};
+
+#endif
-- 
2.17.0

[PATCH 17/19] serdev: Mark controllers compatible with ttyport

[Ricardo Ribalda Delgado]

This allows us to treat differently this controllers, by creating a tty
compatibility layer.

Cc: Rob Herring <robh@kernel.org>
Cc: Johan Hovold <johan@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.com>
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/tty/serdev/serdev-ttyport.c | 1 +
 include/linux/serdev.h              | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c
index 4acc5f41dc67..ae961260e4a4 100644
--- a/drivers/tty/serdev/serdev-ttyport.c
+++ b/drivers/tty/serdev/serdev-ttyport.c
@@ -276,6 +276,7 @@ struct device *serdev_tty_port_register(struct tty_port *port,
 	serport->tty_drv = drv;
 
 	ctrl->ops = &ctrl_ops;
+	ctrl->is_ttyport = true;
 
 	old_ops = port->client_ops;
 	port->client_ops = &client_ops;
diff --git a/include/linux/serdev.h b/include/linux/serdev.h
index bb3b9599c652..07d63933bdb9 100644
--- a/include/linux/serdev.h
+++ b/include/linux/serdev.h
@@ -116,6 +116,7 @@ struct serdev_controller {
 	unsigned int		nr;
 	struct serdev_device	*serdev;
 	const struct serdev_controller_ops *ops;
+	bool			is_ttyport;
 };
 
 static inline struct serdev_controller *to_serdev_controller(struct device *d)
-- 
2.17.0

[PATCH 18/19] 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 <robh@kernel.org>
Cc: Johan Hovold <johan@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.com>
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/tty/serdev/Kconfig         | 11 +++++
 drivers/tty/serdev/Makefile        |  2 +
 drivers/tty/serdev/serdev-ttydev.c | 70 ++++++++++++++++++++++++++++++
 3 files changed, 83 insertions(+)
 create mode 100644 drivers/tty/serdev/serdev-ttydev.c

diff --git a/drivers/tty/serdev/Kconfig b/drivers/tty/serdev/Kconfig
index 1dbc8352e027..d19bf689a424 100644
--- a/drivers/tty/serdev/Kconfig
+++ b/drivers/tty/serdev/Kconfig
@@ -21,4 +21,15 @@ 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 Y.
+	depends on SERIAL_DEV_CTRL_TTYPORT
+	default m
+
 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 000000000000..66479d6534dd
--- /dev/null
+++ b/drivers/tty/serdev/serdev-ttydev.c
@@ -0,0 +1,70 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Ricardo Ribalda <ricardo.ribalda@gmail.com>
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/module.h>
+#include <linux/serdev.h>
+#include <linux/tty.h>
+#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,
+};
+
+static int __init ttydev_serdev_init(void)
+{
+	return serdev_device_driver_register(&ttydev_serdev_driver);
+}
+module_init(ttydev_serdev_init);
+
+static void __exit ttydev_serdev_exit(void)
+{
+	return serdev_device_driver_unregister(&ttydev_serdev_driver);
+}
+module_exit(ttydev_serdev_exit);
+
+MODULE_AUTHOR("Ricardo Ribalda <ricardo.ribalda@gmail.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Serdev to ttydev module");
-- 
2.17.0

[PATCH 19/19] serdev: Instantiate a ttydev serdev if acpi and of fails

[Ricardo Ribalda Delgado]

If a serdev ttyport controller does not have an acpi nor an of child,
create a ttydev as a child of that controller.

Doing this allows the removal, addition and replacement of ttydev devices
at runtime.

Cc: Rob Herring <robh@kernel.org>
Cc: Johan Hovold <johan@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.com>
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/tty/serdev/core.c | 36 ++++++++++++++++++++++++++++++++----
 1 file changed, 32 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index 9414700e6442..34295dacfb84 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -619,6 +619,27 @@ static inline int acpi_serdev_register_devices(struct serdev_controller *ctrl)
 }
 #endif /* CONFIG_ACPI */
 
+
+#if IS_ENABLED(CONFIG_SERIAL_DEV_CTRL_TTYDEV)
+static int serdev_controller_add_ttydev(struct serdev_controller *ctrl)
+{
+	struct serdev_device *serdev;
+	int err;
+
+	serdev = serdev_device_alloc(ctrl);
+	if (!serdev)
+		return -ENOMEM;
+
+	strcpy(serdev->modalias, "ttydev");
+
+	err = serdev_device_add(serdev);
+	if (err)
+		serdev_device_put(serdev);
+
+	return err;
+}
+#endif
+
 /**
  * serdev_controller_add() - Add an serdev controller
  * @ctrl:	controller to be registered.
@@ -628,7 +649,7 @@ static inline int acpi_serdev_register_devices(struct serdev_controller *ctrl)
  */
 int serdev_controller_add(struct serdev_controller *ctrl)
 {
-	int ret_of, ret_acpi, ret;
+	int ret_of, ret_acpi, ret, ret_tty = -ENODEV;
 
 	/* Can't register until after driver model init */
 	if (WARN_ON(!is_registered))
@@ -640,9 +661,16 @@ int serdev_controller_add(struct serdev_controller *ctrl)
 
 	ret_of = of_serdev_register_devices(ctrl);
 	ret_acpi = acpi_serdev_register_devices(ctrl);
-	if (ret_of && ret_acpi) {
-		dev_dbg(&ctrl->dev, "no devices registered: of:%d acpi:%d\n",
-			ret_of, ret_acpi);
+
+#if IS_ENABLED(CONFIG_SERIAL_DEV_CTRL_TTYDEV)
+	if (ret_of && ret_acpi && ctrl->is_ttyport)
+		ret_tty = serdev_controller_add_ttydev(ctrl);
+#endif
+
+	if (ret_of && ret_acpi && ret_tty) {
+		dev_dbg(&ctrl->dev,
+			"no devices registered: of:%d acpi:%d tty:%d\n",
+			ret_of, ret_acpi, ret_tty);
 		ret = -ENODEV;
 		goto out_dev_del;
 	}
-- 
2.17.0

Re: [PATCH 07/19] serdev: Allows dynamic creation of devices via sysfs

[Rob Herring]

On Tue, May 29, 2018 at 8:10 AM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:
> Allow creating and deleting devices via sysfs. Devices created will be
> matched to serdev drivers via modalias (the string provided by the user)
> and deleted via their name. Eg:
>
>  # Create device
> root@qt5022:~# echo ttydev > /sys/bus/serial/devices/serial0/new_device
>
>  # Delete device
> root@qt5022:~#
>         echo serial0-0 > /sys/bus/serial/devices/serial0/delete_device

I think the model here should be the kernel provides dummy slave
device for each serial port and then you can use bind and unbind to
bind to a particular driver.

Rob

Re: [PATCH 07/19] serdev: Allows dynamic creation of devices via sysfs

[Ricardo Ribalda Delgado]

Hi Rob

On Tue, May 29, 2018 at 5:38 PM Rob Herring <robh@kernel.org> wrote:

> On Tue, May 29, 2018 at 8:10 AM, Ricardo Ribalda Delgado
> <ricardo.ribalda@gmail.com> wrote:
> > Allow creating and deleting devices via sysfs. Devices created will be
> > matched to serdev drivers via modalias (the string provided by the user)
> > and deleted via their name. Eg:
> >
> >  # Create device
> > root@qt5022:~# echo ttydev > /sys/bus/serial/devices/serial0/new_device
> >
> >  # Delete device
> > root@qt5022:~#
> >         echo serial0-0 > /sys/bus/serial/devices/serial0/delete_device

> I think the model here should be the kernel provides dummy slave
> device for each serial port and then you can use bind and unbind to
> bind to a particular driver.


I have been researching a bit that approach, but I found a couple of issues:

- With the bind/unbind you need to modprobe manually the module. Something
like

modprobe myserdev
echo myserdev > bind

- You need one module per part_number, with modalias you can have a
different alias per module

- I guess that the final user will appreciate that the serdev has the same
API as other serial slow bus (i2c).

ccing Wolfram becase maybe he has some feedback to same from his experience
with the i2c bus.

Thanks!


> Rob



-- 
Ricardo Ribalda

Re: [PATCH 07/19] serdev: Allows dynamic creation of devices via sysfs

[Ricardo Ribalda Delgado]

Using the right address for Wolfram
On Tue, May 29, 2018 at 6:30 PM Ricardo Ribalda Delgado <
ricardo.ribalda@gmail.com> wrote:

> Hi Rob

> On Tue, May 29, 2018 at 5:38 PM Rob Herring <robh@kernel.org> wrote:

> > On Tue, May 29, 2018 at 8:10 AM, Ricardo Ribalda Delgado
> > <ricardo.ribalda@gmail.com> wrote:
> > > Allow creating and deleting devices via sysfs. Devices created will be
> > > matched to serdev drivers via modalias (the string provided by the
user)
> > > and deleted via their name. Eg:
> > >
> > >  # Create device
> > > root@qt5022:~# echo ttydev >
/sys/bus/serial/devices/serial0/new_device
> > >
> > >  # Delete device
> > > root@qt5022:~#
> > >         echo serial0-0 > /sys/bus/serial/devices/serial0/delete_device

> > I think the model here should be the kernel provides dummy slave
> > device for each serial port and then you can use bind and unbind to
> > bind to a particular driver.


> I have been researching a bit that approach, but I found a couple of
issues:

> - With the bind/unbind you need to modprobe manually the module. Something
> like

> modprobe myserdev
> echo myserdev > bind

> - You need one module per part_number, with modalias you can have a
> different alias per module

> - I guess that the final user will appreciate that the serdev has the same
> API as other serial slow bus (i2c).

> ccing Wolfram becase maybe he has some feedback to same from his
experience
> with the i2c bus.

> Thanks!


> > Rob



> --
> Ricardo Ribalda



-- 
Ricardo Ribalda

Re: [PATCH 07/19] serdev: Allows dynamic creation of devices via sysfs

[Andy Shevchenko]

On Tue, May 29, 2018 at 4:10 PM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:
> Allow creating and deleting devices via sysfs. Devices created will be
> matched to serdev drivers via modalias (the string provided by the user)
> and deleted via their name.

> +       int err;
> +       char *nline;

Better to read in reversed order.

> +       nline = strchr(buf, '\n');
> +       if (nline)
> +               *nline = '\0';

strim() / strstrip() ?

> +       nline = strchr(buf, '\n');
> +       if (nline)
> +               *nline = '\0';

Ditto.

> +static DEVICE_ATTR_IGNORE_LOCKDEP(delete_device, 0200, NULL,
> +                                 delete_device_store);

Perhaps leave it on one line?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 07/19] serdev: Allows dynamic creation of devices via sysfs

[Ricardo Ribalda Delgado]

Hi Andy,

On Tue, May 29, 2018 at 10:35 PM Andy Shevchenko <andy.shevchenko@gmail.com>
wrote:

> On Tue, May 29, 2018 at 4:10 PM, Ricardo Ribalda Delgado
> <ricardo.ribalda@gmail.com> wrote:
> > Allow creating and deleting devices via sysfs. Devices created will be
> > matched to serdev drivers via modalias (the string provided by the user)
> > and deleted via their name.

> > +       int err;
> > +       char *nline;

> Better to read in reversed order.

> > +       nline = strchr(buf, '\n');
> > +       if (nline)
> > +               *nline = '\0';

> strim() / strstrip() ?

> > +       nline = strchr(buf, '\n');
> > +       if (nline)
> > +               *nline = '\0';

> Ditto.

> > +static DEVICE_ATTR_IGNORE_LOCKDEP(delete_device, 0200, NULL,
> > +                                 delete_device_store);

> Perhaps leave it on one line?

> --
> With Best Regards,
> Andy Shevchenko

Thanks for your review. I am working on v2 at
https://github.com/ribalda/linux/tree/serdev2 it fixes your comments except
the line cut for DEVICE_ATTR, I want also to make checkpatch happy ;)

Will send v2  to the list after I got more feedback, you can of course ping
me if there is something in my tree that it is not fixed properly.

Cheers


-- 
Ricardo Ribalda

Re: [PATCH 15/19] net: qualcomm: MODULE_DEVICE_TABLE(serdev)

[Stefan Wahren]

Hi Ricardo,


Am 29.05.2018 um 15:10 schrieb Ricardo Ribalda Delgado:
> Export serdev table to the module header, allowing module autoload via
> udev/modprobe.
>
> Cc: Lino Sanfilippo <LinoSanfilippo@gmx.de>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Stefan Wahren <stefan.wahren@i2se.com>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Johan Hovold <johan@kernel.org>
> Cc: netdev@vger.kernel.org
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> ---
>   drivers/net/ethernet/qualcomm/qca_uart.c | 7 +++++++
>   1 file changed, 7 insertions(+)
>
> diff --git a/drivers/net/ethernet/qualcomm/qca_uart.c b/drivers/net/ethernet/qualcomm/qca_uart.c
> index db6068cd7a1f..bb7aed805083 100644
> --- a/drivers/net/ethernet/qualcomm/qca_uart.c
> +++ b/drivers/net/ethernet/qualcomm/qca_uart.c
> @@ -405,6 +405,12 @@ static void qca_uart_remove(struct serdev_device *serdev)
>   	free_netdev(qca->net_dev);
>   }
>   
> +static struct serdev_device_id qca_uart_serdev_id[] = {
> +	{ QCAUART_DRV_NAME, },

i guess the id must be stable, so in case someone changes the driver 
name this has unexpected side effects?

Apart from that i'm fine with this patch.

Thanks
Stefan

> +	{},
> +};
> +MODULE_DEVICE_TABLE(serdev, qca_uart_serdev_id);
> +
>   static struct serdev_device_driver qca_uart_driver = {
>   	.probe = qca_uart_probe,
>   	.remove = qca_uart_remove,
> @@ -412,6 +418,7 @@ static struct serdev_device_driver qca_uart_driver = {
>   		.name = QCAUART_DRV_NAME,
>   		.of_match_table = of_match_ptr(qca_uart_of_match),
>   	},
> +	.id_table = qca_uart_serdev_id,
>   };
>   
>   module_serdev_device_driver(qca_uart_driver);

Re: [PATCH 04/19] Bluetooth: hci_nokia: Add serdev_id_table

[Andy Shevchenko]

On Tue, May 29, 2018 at 4:09 PM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:
> Describe which hardware is supported by the current driver.

> +static struct serdev_device_id nokia_bluetooth_serdev_id[] = {
> +       { "hp4-bluetooth", },

> +       {},

Terminator line better w/o comma.

> +};



-- 
With Best Regards,
Andy Shevchenko

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

[Andy Shevchenko]

On Tue, May 29, 2018 at 4:10 PM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> 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.

> +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 Y.
> +       depends on SERIAL_DEV_CTRL_TTYPORT

> +       default m

Hmm... Can't we survive w/o this by default?

> +static int __init ttydev_serdev_init(void)
> +{
> +       return serdev_device_driver_register(&ttydev_serdev_driver);
> +}
> +module_init(ttydev_serdev_init);
> +
> +static void __exit ttydev_serdev_exit(void)
> +{
> +       return serdev_device_driver_unregister(&ttydev_serdev_driver);
> +}
> +module_exit(ttydev_serdev_exit);

Isn't above is just a macro in serdev.h?
I.e. module_serdev_device_driver().

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 19/19] serdev: Instantiate a ttydev serdev if acpi and of fails

[Andy Shevchenko]

On Tue, May 29, 2018 at 4:10 PM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:
> If a serdev ttyport controller does not have an acpi nor an of child,
> create a ttydev as a child of that controller.
>
> Doing this allows the removal, addition and replacement of ttydev devices
> at runtime.

> @@ -619,6 +619,27 @@ static inline int acpi_serdev_register_devices(struct serdev_controller *ctrl)
>  }
>  #endif /* CONFIG_ACPI */
>
> +

Redundant blank line.

> +#if IS_ENABLED(CONFIG_SERIAL_DEV_CTRL_TTYDEV)

> -       if (ret_of && ret_acpi) {
> -               dev_dbg(&ctrl->dev, "no devices registered: of:%d acpi:%d\n",
> -                       ret_of, ret_acpi);
> +
> +#if IS_ENABLED(CONFIG_SERIAL_DEV_CTRL_TTYDEV)
> +       if (ret_of && ret_acpi && ctrl->is_ttyport)
> +               ret_tty = serdev_controller_add_ttydev(ctrl);
> +#endif
> +
> +       if (ret_of && ret_acpi && ret_tty) {
> +               dev_dbg(&ctrl->dev,
> +                       "no devices registered: of:%d acpi:%d tty:%d\n",
> +                       ret_of, ret_acpi, ret_tty);
>                 ret = -ENODEV;
>                 goto out_dev_del;
>         }

Wouldn't be better to leave above if-condition and introduce your
stuff inside it?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 04/19] Bluetooth: hci_nokia: Add serdev_id_table

[Marcel Holtmann]

Hi Ricardo,

> Describe which hardware is supported by the current driver.
> 
> Cc: Marcel Holtmann <marcel@holtmann.org>
> Cc: Johan Hedberg <johan.hedberg@gmail.com>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Johan Hovold <johan@kernel.org>
> Cc: linux-bluetooth@vger.kernel.org
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> ---
> drivers/bluetooth/hci_nokia.c | 6 ++++++
> 1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/bluetooth/hci_nokia.c b/drivers/bluetooth/hci_nokia.c
> index 3539fd03f47e..e32dfcd56b8d 100644
> --- a/drivers/bluetooth/hci_nokia.c
> +++ b/drivers/bluetooth/hci_nokia.c
> @@ -801,6 +801,11 @@ static const struct of_device_id nokia_bluetooth_of_match[] = {
> MODULE_DEVICE_TABLE(of, nokia_bluetooth_of_match);
> #endif
> 
> +static struct serdev_device_id nokia_bluetooth_serdev_id[] = {
> +	{ "hp4-bluetooth", },
> +	{},
> +};
> +
> static struct serdev_device_driver nokia_bluetooth_serdev_driver = {
> 	.probe = nokia_bluetooth_serdev_probe,
> 	.remove = nokia_bluetooth_serdev_remove,
> @@ -809,6 +814,7 @@ static struct serdev_device_driver nokia_bluetooth_serdev_driver = {
> 		.pm = &nokia_bluetooth_pm_ops,
> 		.of_match_table = of_match_ptr(nokia_bluetooth_of_match),
> 	},
> +	.id_table = nokia_bluetooth_serdev_id,
> };

I would actually skip this hardware. First of all it is such a dedicated custom Nokia transport and hardware, and secondly it is no longer produced anyway.

Regards

Marcel

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

[Ricardo Ribalda Delgado]

Hi Andy
On Tue, Jun 5, 2018 at 3:42 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Tue, May 29, 2018 at 4:10 PM, Ricardo Ribalda Delgado
> <ricardo.ribalda@gmail.com> 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.
>
> > +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 Y.
> > +       depends on SERIAL_DEV_CTRL_TTYPORT
>
> > +       default m
>
> Hmm... Can't we survive w/o this by default?

If this module is not available and serdev is enabled the user would
miss his /dev/ttyS* nodes, so I rather leave this on.

>
> > +static int __init ttydev_serdev_init(void)
> > +{
> > +       return serdev_device_driver_register(&ttydev_serdev_driver);
> > +}
> > +module_init(ttydev_serdev_init);
> > +
> > +static void __exit ttydev_serdev_exit(void)
> > +{
> > +       return serdev_device_driver_unregister(&ttydev_serdev_driver);
> > +}
> > +module_exit(ttydev_serdev_exit);
>
> Isn't above is just a macro in serdev.h?
> I.e. module_serdev_device_driver().

ACK, thanks!
>
> --
> With Best Regards,
> Andy Shevchenko

Best regards

-- 
Ricardo Ribalda

Re: [PATCH 19/19] serdev: Instantiate a ttydev serdev if acpi and of fails

[Ricardo Ribalda Delgado]

Hi Andy,

On Tue, Jun 5, 2018 at 3:44 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Tue, May 29, 2018 at 4:10 PM, Ricardo Ribalda Delgado
> <ricardo.ribalda@gmail.com> wrote:
> > If a serdev ttyport controller does not have an acpi nor an of child,
> > create a ttydev as a child of that controller.
> >
> > Doing this allows the removal, addition and replacement of ttydev devices
> > at runtime.
>
> > @@ -619,6 +619,27 @@ static inline int acpi_serdev_register_devices(struct serdev_controller *ctrl)
> >  }
> >  #endif /* CONFIG_ACPI */
> >
> > +
>
> Redundant blank line.

ACK
>
> > +#if IS_ENABLED(CONFIG_SERIAL_DEV_CTRL_TTYDEV)
>
> > -       if (ret_of && ret_acpi) {
> > -               dev_dbg(&ctrl->dev, "no devices registered: of:%d acpi:%d\n",
> > -                       ret_of, ret_acpi);
> > +
> > +#if IS_ENABLED(CONFIG_SERIAL_DEV_CTRL_TTYDEV)
> > +       if (ret_of && ret_acpi && ctrl->is_ttyport)
> > +               ret_tty = serdev_controller_add_ttydev(ctrl);
> > +#endif
> > +
> > +       if (ret_of && ret_acpi && ret_tty) {
> > +               dev_dbg(&ctrl->dev,
> > +                       "no devices registered: of:%d acpi:%d tty:%d\n",
> > +                       ret_of, ret_acpi, ret_tty);
> >                 ret = -ENODEV;
> >                 goto out_dev_del;
> >         }
>
> Wouldn't be better to leave above if-condition and introduce your
> stuff inside it?

I have redesign the logic in the function on the next version

Thanks


-- 
Ricardo Ribalda

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

[Ricardo Ribalda Delgado]

Hi Andy,
On Wed, Jun 6, 2018 at 8:58 AM Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:
>
> Hi Andy
> On Tue, Jun 5, 2018 at 3:42 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> >
> > On Tue, May 29, 2018 at 4:10 PM, Ricardo Ribalda Delgado
> > <ricardo.ribalda@gmail.com> 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.
> >
> > > +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 Y.
> > > +       depends on SERIAL_DEV_CTRL_TTYPORT
> >
> > > +       default m
> >
> > Hmm... Can't we survive w/o this by default?
>
> If this module is not available and serdev is enabled the user would
> miss his /dev/ttyS* nodes, so I rather leave this on.

Sorry, brain fart. This is exactly what #if
IS_ENABLED(CONFIG_SERIAL_DEV_CTRL_TTYDEV) prevents.

I have defaulted the module to n as you suggested. You can take a look
to the patches that I plan to submit tomorrow at:

https://github.com/ribalda/linux/tree/serdev2

>
> >
> > > +static int __init ttydev_serdev_init(void)
> > > +{
> > > +       return serdev_device_driver_register(&ttydev_serdev_driver);
> > > +}
> > > +module_init(ttydev_serdev_init);
> > > +
> > > +static void __exit ttydev_serdev_exit(void)
> > > +{
> > > +       return serdev_device_driver_unregister(&ttydev_serdev_driver);
> > > +}
> > > +module_exit(ttydev_serdev_exit);
> >
> > Isn't above is just a macro in serdev.h?
> > I.e. module_serdev_device_driver().
>
> ACK, thanks!
> >
> > --
> > With Best Regards,
> > Andy Shevchenko
>
> Best regards
>
> --
> Ricardo Ribalda



-- 
Ricardo Ribalda

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

[Andy Shevchenko]

On Wed, Jun 6, 2018 at 10:47 AM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:
> Hi Andy,
> On Wed, Jun 6, 2018 at 8:58 AM Ricardo Ribalda Delgado
> <ricardo.ribalda@gmail.com> wrote:

> I have defaulted the module to n as you suggested. You can take a look
> to the patches that I plan to submit tomorrow at:
>
> https://github.com/ribalda/linux/tree/serdev2

n _is_ default. So, just remove a line.

-- 
With Best Regards,
Andy Shevchenko

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

[Ricardo Ribalda Delgado]

On Wed, Jun 6, 2018 at 11:55 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> >
> > https://github.com/ribalda/linux/tree/serdev2
>
> n _is_ default. So, just remove a line.
>

Done. Thanks!

> --
> With Best Regards,
> Andy Shevchenko



-- 
Ricardo Ribalda

Re: [PATCH 04/19] Bluetooth: hci_nokia: Add serdev_id_table

[Pavel Machek]

Hi!

> > Describe which hardware is supported by the current driver.
> > 
> > Cc: Marcel Holtmann <marcel@holtmann.org>
> > Cc: Johan Hedberg <johan.hedberg@gmail.com>
> > Cc: Rob Herring <robh@kernel.org>
> > Cc: Johan Hovold <johan@kernel.org>
> > Cc: linux-bluetooth@vger.kernel.org
> > Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> > ---
> > drivers/bluetooth/hci_nokia.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> > 
> > diff --git a/drivers/bluetooth/hci_nokia.c b/drivers/bluetooth/hci_nokia.c
> > index 3539fd03f47e..e32dfcd56b8d 100644
> > --- a/drivers/bluetooth/hci_nokia.c
> > +++ b/drivers/bluetooth/hci_nokia.c
> > @@ -801,6 +801,11 @@ static const struct of_device_id nokia_bluetooth_of_match[] = {
> > MODULE_DEVICE_TABLE(of, nokia_bluetooth_of_match);
> > #endif
> > 
> > +static struct serdev_device_id nokia_bluetooth_serdev_id[] = {
> > +	{ "hp4-bluetooth", },
> > +	{},
> > +};
> > +
> > static struct serdev_device_driver nokia_bluetooth_serdev_driver = {
> > 	.probe = nokia_bluetooth_serdev_probe,
> > 	.remove = nokia_bluetooth_serdev_remove,
> > @@ -809,6 +814,7 @@ static struct serdev_device_driver nokia_bluetooth_serdev_driver = {
> > 		.pm = &nokia_bluetooth_pm_ops,
> > 		.of_match_table = of_match_ptr(nokia_bluetooth_of_match),
> > 	},
> > +	.id_table = nokia_bluetooth_serdev_id,
> > };
> 
> I would actually skip this hardware. First of all it is such a dedicated custom Nokia transport and hardware, and secondly it is no longer produced anyway.
> 

Would it make sense to cc: sre here? We want good support even for old
hardware, and this is n9/n950, it is still on "top ten supported
phones" list... Probably even top 5.

Thanks,
								 Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 04/19] Bluetooth: hci_nokia: Add serdev_id_table

[Marcel Holtmann]

Hi Pavel,

>>> Describe which hardware is supported by the current driver.
>>> 
>>> Cc: Marcel Holtmann <marcel@holtmann.org>
>>> Cc: Johan Hedberg <johan.hedberg@gmail.com>
>>> Cc: Rob Herring <robh@kernel.org>
>>> Cc: Johan Hovold <johan@kernel.org>
>>> Cc: linux-bluetooth@vger.kernel.org
>>> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
>>> ---
>>> drivers/bluetooth/hci_nokia.c | 6 ++++++
>>> 1 file changed, 6 insertions(+)
>>> 
>>> diff --git a/drivers/bluetooth/hci_nokia.c b/drivers/bluetooth/hci_nokia.c
>>> index 3539fd03f47e..e32dfcd56b8d 100644
>>> --- a/drivers/bluetooth/hci_nokia.c
>>> +++ b/drivers/bluetooth/hci_nokia.c
>>> @@ -801,6 +801,11 @@ static const struct of_device_id nokia_bluetooth_of_match[] = {
>>> MODULE_DEVICE_TABLE(of, nokia_bluetooth_of_match);
>>> #endif
>>> 
>>> +static struct serdev_device_id nokia_bluetooth_serdev_id[] = {
>>> +	{ "hp4-bluetooth", },
>>> +	{},
>>> +};
>>> +
>>> static struct serdev_device_driver nokia_bluetooth_serdev_driver = {
>>> 	.probe = nokia_bluetooth_serdev_probe,
>>> 	.remove = nokia_bluetooth_serdev_remove,
>>> @@ -809,6 +814,7 @@ static struct serdev_device_driver nokia_bluetooth_serdev_driver = {
>>> 		.pm = &nokia_bluetooth_pm_ops,
>>> 		.of_match_table = of_match_ptr(nokia_bluetooth_of_match),
>>> 	},
>>> +	.id_table = nokia_bluetooth_serdev_id,
>>> };
>> 
>> I would actually skip this hardware. First of all it is such a dedicated custom Nokia transport and hardware, and secondly it is no longer produced anyway.
>> 
> 
> Would it make sense to cc: sre here? We want good support even for old
> hardware, and this is n9/n950, it is still on "top ten supported
> phones" list... Probably even top 5.

but that is not what this patch series is about. We do not need new_id kinda support for the existing hardware. My point is there will be no newly designed hardware using this driver.

Regards

Marcel

Re: [PATCH 04/19] Bluetooth: hci_nokia: Add serdev_id_table

[Pavel Machek]

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

Hi!


> >>> +	{ "hp4-bluetooth", },
> >>> +	{},
> >>> +};
> >>> +
> >>> static struct serdev_device_driver nokia_bluetooth_serdev_driver = {
> >>> 	.probe = nokia_bluetooth_serdev_probe,
> >>> 	.remove = nokia_bluetooth_serdev_remove,
> >>> @@ -809,6 +814,7 @@ static struct serdev_device_driver nokia_bluetooth_serdev_driver = {
> >>> 		.pm = &nokia_bluetooth_pm_ops,
> >>> 		.of_match_table = of_match_ptr(nokia_bluetooth_of_match),
> >>> 	},
> >>> +	.id_table = nokia_bluetooth_serdev_id,
> >>> };
> >> 
> >> I would actually skip this hardware. First of all it is such a dedicated custom Nokia transport and hardware, and secondly it is no longer produced anyway.
> >> 
> > 
> > Would it make sense to cc: sre here? We want good support even for old
> > hardware, and this is n9/n950, it is still on "top ten supported
> > phones" list... Probably even top 5.
> 
> but that is not what this patch series is about. We do not need new_id kinda support for the existing hardware. My point is there will be no newly designed hardware using this driver.
> 

You are right, new hardware with that protocol sounds unlikely at this
point.

Sorry for the noise,
									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 --]