[RFC PATCH 0/3] USB: serial: add device tree (and serdev) support

[RFC PATCH 0/3] USB: serial: add device tree (and serdev) support

[Johan Hovold]

These are some patches I've been using to test out using serdev with USB
serial. Adding device-tree support to USB serial is really a
separate matter and could be merged before the remaining issues related
to hotplug are addressed for serdev.

Note that this has been a low-intensity on-going effort where most
prerequisites are already upstream including USB device tree support
(4.15), device-tree node sharing (4.13), musb device-node propagation
(linux-next) and various fixes along the way (driver core, usb core,
musb).

What left to be decided is how to deal with multi-port devices. This
series uses child nodes to represent each port, which may be a little
counter-intuitive for devices (or rather interfaces) with just a single
port:

	&usb_interface {
		#address-cells = <1>;
		#size-cells = <0>;

		serial@0 {
			reg = <0>;
		};
	};

but I still think this it how it needs to be implemented.

Another thing that's currently lacking is binding documentation.

For completeness (and per request), the second patch enables serdev
support and can be used for testing purposes. The third patch can be
used as a base for testing this on a BBB and describes two USB serial
devices attached to an external hub.

Note that this series depends on a couple of patches (for usb-serial and
musb) that are still in linux-next. For convenience, I've prepared a
branch here:

	https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git/log/?h=usb-serial-of

Johan


Johan Hovold (3):
  USB: serial: add device-tree support
  USB: serial: enable serdev support
  dbg: ARM: dts: boneblack: add USB topology and serdev nodes

 arch/arm/boot/dts/am335x-boneblack.dts | 57 ++++++++++++++++++++++++++
 drivers/usb/serial/bus.c               |  7 ++--
 drivers/usb/serial/usb-serial.c        | 28 ++++++++++++-
 3 files changed, 88 insertions(+), 4 deletions(-)

-- 
2.17.0

[RFC PATCH 1/3] USB: serial: add device-tree support

[Johan Hovold]

Lookup and associate serial-port device-tree nodes given a parent
USB-interface node during probe.

Note that a serial-port node must be named "serial" and have a "reg"
property so that ports on multi-port interfaces can be distinguished.

	&usb_interface {
		#address-cells = <1>;
		#size-cells = <0>;

		serial@0 {
			reg = <0>;
		};
	};

FIXME: binding doc

Not-signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/usb-serial.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
index 44ecf0e2be9d..5a7ebe1e9fd6 100644
--- a/drivers/usb/serial/usb-serial.c
+++ b/drivers/usb/serial/usb-serial.c
@@ -35,6 +35,7 @@
 #include <linux/usb/serial.h>
 #include <linux/kfifo.h>
 #include <linux/idr.h>
+#include <linux/of.h>
 
 #define DRIVER_AUTHOR "Greg Kroah-Hartman <gregkh@linuxfoundation.org>"
 #define DRIVER_DESC "USB Serial Driver core"
@@ -97,7 +98,6 @@ static int allocate_minors(struct usb_serial *serial, int num_ports)
 		if (minor < 0)
 			goto error;
 		port->minor = minor;
-		port->port_number = i;
 	}
 	serial->minors_reserved = 1;
 	mutex_unlock(&table_lock);
@@ -589,6 +589,7 @@ static void usb_serial_port_release(struct device *dev)
 	kfifo_free(&port->write_fifo);
 	kfree(port->interrupt_in_buffer);
 	kfree(port->interrupt_out_buffer);
+	of_node_put(dev->of_node);
 	tty_port_destroy(&port->port);
 	kfree(port);
 }
@@ -857,6 +858,29 @@ static int setup_port_interrupt_out(struct usb_serial_port *port,
 	return 0;
 }
 
+/* FIXME: move to separate compilation unit? */
+static struct device_node *find_port_node(struct usb_interface *intf, int port)
+{
+	struct device_node *node;
+	u32 reg;
+
+	for_each_child_of_node(intf->dev.of_node, node) {
+		if (!node->name || of_node_cmp(node->name, "serial") != 0)
+			continue;
+
+		if (of_property_read_u32(node, "reg", &reg))
+			continue;
+
+		if (reg == port)
+			break;
+	}
+
+	dev_dbg(&intf->dev, "node %pOF, port %d: %pOFP\n", intf->dev.of_node,
+			port, node);
+
+	return node;
+}
+
 static int usb_serial_probe(struct usb_interface *interface,
 			       const struct usb_device_id *id)
 {
@@ -963,6 +987,7 @@ static int usb_serial_probe(struct usb_interface *interface,
 			retval = -ENOMEM;
 			goto err_free_epds;
 		}
+		port->port_number = i;
 		tty_port_init(&port->port);
 		port->port.ops = &serial_port_ops;
 		port->serial = serial;
@@ -976,6 +1001,7 @@ static int usb_serial_probe(struct usb_interface *interface,
 		port->dev.bus = &usb_serial_bus_type;
 		port->dev.release = &usb_serial_port_release;
 		port->dev.groups = usb_serial_port_groups;
+		port->dev.of_node = find_port_node(interface, port->port_number);
 		device_initialize(&port->dev);
 	}
 
-- 
2.17.0

[RFC,1/3] USB: serial: add device-tree support

[Johan Hovold]

Lookup and associate serial-port device-tree nodes given a parent
USB-interface node during probe.

Note that a serial-port node must be named "serial" and have a "reg"
property so that ports on multi-port interfaces can be distinguished.

	&usb_interface {
		#address-cells = <1>;
		#size-cells = <0>;

		serial@0 {
			reg = <0>;
		};
	};

FIXME: binding doc

Not-signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/usb-serial.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
index 44ecf0e2be9d..5a7ebe1e9fd6 100644
--- a/drivers/usb/serial/usb-serial.c
+++ b/drivers/usb/serial/usb-serial.c
@@ -35,6 +35,7 @@
 #include <linux/usb/serial.h>
 #include <linux/kfifo.h>
 #include <linux/idr.h>
+#include <linux/of.h>
 
 #define DRIVER_AUTHOR "Greg Kroah-Hartman <gregkh@linuxfoundation.org>"
 #define DRIVER_DESC "USB Serial Driver core"
@@ -97,7 +98,6 @@ static int allocate_minors(struct usb_serial *serial, int num_ports)
 		if (minor < 0)
 			goto error;
 		port->minor = minor;
-		port->port_number = i;
 	}
 	serial->minors_reserved = 1;
 	mutex_unlock(&table_lock);
@@ -589,6 +589,7 @@ static void usb_serial_port_release(struct device *dev)
 	kfifo_free(&port->write_fifo);
 	kfree(port->interrupt_in_buffer);
 	kfree(port->interrupt_out_buffer);
+	of_node_put(dev->of_node);
 	tty_port_destroy(&port->port);
 	kfree(port);
 }
@@ -857,6 +858,29 @@ static int setup_port_interrupt_out(struct usb_serial_port *port,
 	return 0;
 }
 
+/* FIXME: move to separate compilation unit? */
+static struct device_node *find_port_node(struct usb_interface *intf, int port)
+{
+	struct device_node *node;
+	u32 reg;
+
+	for_each_child_of_node(intf->dev.of_node, node) {
+		if (!node->name || of_node_cmp(node->name, "serial") != 0)
+			continue;
+
+		if (of_property_read_u32(node, "reg", &reg))
+			continue;
+
+		if (reg == port)
+			break;
+	}
+
+	dev_dbg(&intf->dev, "node %pOF, port %d: %pOFP\n", intf->dev.of_node,
+			port, node);
+
+	return node;
+}
+
 static int usb_serial_probe(struct usb_interface *interface,
 			       const struct usb_device_id *id)
 {
@@ -963,6 +987,7 @@ static int usb_serial_probe(struct usb_interface *interface,
 			retval = -ENOMEM;
 			goto err_free_epds;
 		}
+		port->port_number = i;
 		tty_port_init(&port->port);
 		port->port.ops = &serial_port_ops;
 		port->serial = serial;
@@ -976,6 +1001,7 @@ static int usb_serial_probe(struct usb_interface *interface,
 		port->dev.bus = &usb_serial_bus_type;
 		port->dev.release = &usb_serial_port_release;
 		port->dev.groups = usb_serial_port_groups;
+		port->dev.of_node = find_port_node(interface, port->port_number);
 		device_initialize(&port->dev);
 	}
 

[RFC PATCH 2/3] USB: serial: enable serdev support

[Johan Hovold]

Enable serdev support by using the serdev opt-in tty-port registration
helpers.

FIXME: serdev core always allocates and registers a serdev controller
during port registration only to immediately roll back in the common
case when there is no serdev slave defined in firmware

FIXME: serdev does not support hotplugging (e.g. tty port hangups)

Not-signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/bus.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/serial/bus.c b/drivers/usb/serial/bus.c
index eb0195cf37dd..5f574a418c52 100644
--- a/drivers/usb/serial/bus.c
+++ b/drivers/usb/serial/bus.c
@@ -60,8 +60,9 @@ static int usb_serial_device_probe(struct device *dev)
 	}
 
 	minor = port->minor;
-	tty_dev = tty_port_register_device(&port->port, usb_serial_tty_driver,
-					   minor, dev);
+	tty_dev = tty_port_register_device_serdev(&port->port,
+							usb_serial_tty_driver,
+							minor, dev);
 	if (IS_ERR(tty_dev)) {
 		retval = PTR_ERR(tty_dev);
 		goto err_port_remove;
@@ -105,7 +106,7 @@ static int usb_serial_device_remove(struct device *dev)
 	autopm_err = usb_autopm_get_interface(port->serial->interface);
 
 	minor = port->minor;
-	tty_unregister_device(usb_serial_tty_driver, minor);
+	tty_port_unregister_device(&port->port, usb_serial_tty_driver, minor);
 
 	driver = port->serial->type;
 	if (driver->port_remove)
-- 
2.17.0

[RFC,2/3] USB: serial: enable serdev support

[Johan Hovold]

Enable serdev support by using the serdev opt-in tty-port registration
helpers.

FIXME: serdev core always allocates and registers a serdev controller
during port registration only to immediately roll back in the common
case when there is no serdev slave defined in firmware

FIXME: serdev does not support hotplugging (e.g. tty port hangups)

Not-signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/bus.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/serial/bus.c b/drivers/usb/serial/bus.c
index eb0195cf37dd..5f574a418c52 100644
--- a/drivers/usb/serial/bus.c
+++ b/drivers/usb/serial/bus.c
@@ -60,8 +60,9 @@ static int usb_serial_device_probe(struct device *dev)
 	}
 
 	minor = port->minor;
-	tty_dev = tty_port_register_device(&port->port, usb_serial_tty_driver,
-					   minor, dev);
+	tty_dev = tty_port_register_device_serdev(&port->port,
+							usb_serial_tty_driver,
+							minor, dev);
 	if (IS_ERR(tty_dev)) {
 		retval = PTR_ERR(tty_dev);
 		goto err_port_remove;
@@ -105,7 +106,7 @@ static int usb_serial_device_remove(struct device *dev)
 	autopm_err = usb_autopm_get_interface(port->serial->interface);
 
 	minor = port->minor;
-	tty_unregister_device(usb_serial_tty_driver, minor);
+	tty_port_unregister_device(&port->port, usb_serial_tty_driver, minor);
 
 	driver = port->serial->type;
 	if (driver->port_remove)

[RFC PATCH 3/3] dbg: ARM: dts: boneblack: add USB topology and serdev nodes

[Johan Hovold]

Add a hub device and two USB devices, of which one has a combined node.

Note that we need to represent the serial ports as well -- consider
devices with multiple ports per interface; which one should serdev
use? Sibling devices can also be described this way (e.g. gpio@0), and
would need to use the same address size.

Also note that serial ports have a standardised node name in ePAPR.

Not-signed-off-by: Johan Hovold <johan@kernel.org>
---
 arch/arm/boot/dts/am335x-boneblack.dts | 57 ++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/arch/arm/boot/dts/am335x-boneblack.dts b/arch/arm/boot/dts/am335x-boneblack.dts
index d154d3133c16..d5f4c78efa53 100644
--- a/arch/arm/boot/dts/am335x-boneblack.dts
+++ b/arch/arm/boot/dts/am335x-boneblack.dts
@@ -26,3 +26,60 @@
 		opp-supported-hw = <0x06 0x0100>;
 	};
 };
+
+&usb1 {
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	dlink_hub: hub@1 {
+		compatible = "usb2101,8501";
+		reg = <1>;
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		ft232r: device@3 {
+			compatible = "usb403,6001";
+			reg = <3>;
+
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			serial@0 {
+				reg = <0>;
+
+				serdev {
+					compatible = "none,serdev-mockup";
+				};
+			};
+		};
+
+		mos7820: device@5 {
+			compatible = "usb9710,7840";
+			reg = <5>;
+
+			#address-cells = <2>;
+			#size-cells = <0>;
+
+			interface@0 {
+				compatible = "usbif9710,7840.config1.0";
+				reg = <0 1>;
+
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				serial@0 {
+					reg = <0>;
+
+					gnss {
+						compatible = "u-blox,neo-8";
+					};
+				};
+
+				serial@1 {
+					reg = <1>;
+				};
+			};
+		};
+	};
+};
-- 
2.17.0

[RFC,3/3] dbg: ARM: dts: boneblack: add USB topology and serdev nodes

[Johan Hovold]

Add a hub device and two USB devices, of which one has a combined node.

Note that we need to represent the serial ports as well -- consider
devices with multiple ports per interface; which one should serdev
use? Sibling devices can also be described this way (e.g. gpio@0), and
would need to use the same address size.

Also note that serial ports have a standardised node name in ePAPR.

Not-signed-off-by: Johan Hovold <johan@kernel.org>
---
 arch/arm/boot/dts/am335x-boneblack.dts | 57 ++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/arch/arm/boot/dts/am335x-boneblack.dts b/arch/arm/boot/dts/am335x-boneblack.dts
index d154d3133c16..d5f4c78efa53 100644
--- a/arch/arm/boot/dts/am335x-boneblack.dts
+++ b/arch/arm/boot/dts/am335x-boneblack.dts
@@ -26,3 +26,60 @@
 		opp-supported-hw = <0x06 0x0100>;
 	};
 };
+
+&usb1 {
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	dlink_hub: hub@1 {
+		compatible = "usb2101,8501";
+		reg = <1>;
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		ft232r: device@3 {
+			compatible = "usb403,6001";
+			reg = <3>;
+
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			serial@0 {
+				reg = <0>;
+
+				serdev {
+					compatible = "none,serdev-mockup";
+				};
+			};
+		};
+
+		mos7820: device@5 {
+			compatible = "usb9710,7840";
+			reg = <5>;
+
+			#address-cells = <2>;
+			#size-cells = <0>;
+
+			interface@0 {
+				compatible = "usbif9710,7840.config1.0";
+				reg = <0 1>;
+
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				serial@0 {
+					reg = <0>;
+
+					gnss {
+						compatible = "u-blox,neo-8";
+					};
+				};
+
+				serial@1 {
+					reg = <1>;
+				};
+			};
+		};
+	};
+};