Skip to contentSkip to conversation

UART slave devices using serio

RFC PATCH In review Rob Herring 4 participants 21 messages Aug 24, 2016 – Nov 2, 2016

Subsystems (3)
Bluetooth drivers TTY layer and serial drivers Input drivers
lore
Share & tools
lore

Rob Herring

author
#0 lore raw

cover UART slave devices using serio

This is a new approach to supporting UART slave devices using the existing serio bus. After Arnd's proding, I took another look at serio and decided extending it does make sense. Using serio primarily requires adding DT based device matching and supporting buffer based write and receive.

Currently, I'm using the existing serio serport ldisc for testing. This requires using inputattach to open the tty and set the ldisc which in turn registers a serio port with the serio core:

inputattach -bare /dev/ttyAMA1

Once a tty_port based serio port driver is in place, this step will not be needed. Supporting cases like a USB UART will also work if the USB UART is described in DT. If not described in DT, I'm not sure if the existing serio manual binding is sufficient (Need to figure out how that works). Slave drivers also need other ways to specify additional data using module params perhaps. Getting DT overlays to work for non-discoverable devices behind discoverable buses (i.e. detached from a base DT) is another option, but that's not yet supported in general.

I've done all the serio changes in place, but ultimately I think at least the core of serio should be moved out of drivers/input/. I don't think it belongs under drivers/tty/ or drivers/tty/serial/, so drivers/serio/?

BT is working under QEMU to the point a slave driver can bind to a serio port device via DT, register as a BT device, start sending out initial packets and receive data (typed at a terminal). Now I need to find a real device.

A git branch is available here[1]. Note it will get rebased.

Rob

[1] git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git serial-bus-serio

Rob Herring (6):
serio: add DT driver binding
serio: serport: hacks to get DT probe to work
serio: add buffer receive and write functions
serio: serport: add support for buffered write and receive
serio: add serial configuration functions
bluetooth: hack up ldisc to use serio

 drivers/bluetooth/hci_ldisc.c | 261 +++++++++++++++++-------------------------
 drivers/bluetooth/hci_uart.h  |   3 +
 drivers/input/serio/serio.c   |  34 +++++-
 drivers/input/serio/serport.c |  49 ++++----
 include/linux/serio.h         |  59 +++++++++-
 5 files changed, 223 insertions(+), 183 deletions(-)

--
2.9.3

author
#1 lore raw

RFC PATCH 1/6 serio: add DT driver binding

This adds DT driver binding support to the serio bus. The parent of the serio port device must have a device_node associated with it. Typically, this would be the UART device node. The slave device must be a child node of the UART device node.

Signed-off-by Rob Herring <[email protected]>
---
 drivers/input/serio/serio.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
drivers/input/serio/serio.c +10 -1
index 1ca7f55..9e8eb7a 100644
@@ -36,6 +36,7 @@
 #include <linux/slab.h>
 #include <linux/workqueue.h>
 #include <linux/mutex.h>
+#include <linux/of_device.h>
 
 MODULE_AUTHOR("Vojtech Pavlik <[email protected]>");
 MODULE_DESCRIPTION("Serio abstraction core");

@@ -88,7 +89,7 @@ static void serio_disconnect_driver(struct serio *serio)
 
 static int serio_match_port(const struct serio_device_id *ids, struct serio *serio)
 {
-	while (ids->type || ids->proto) {
+	while (ids && (ids->type || ids->proto)) {
 		if ((ids->type == SERIO_ANY || ids->type == serio->id.type) &&
 		    (ids->proto == SERIO_ANY || ids->proto == serio->id.proto) &&
 		    (ids->extra == SERIO_ANY || ids->extra == serio->id.extra) &&

@@ -542,6 +543,8 @@ static void serio_init_port(struct serio *serio)
 static void serio_add_port(struct serio *serio)
 {
 	struct serio *parent = serio->parent;
+	struct device_node *parent_node =
+		serio->dev.parent ? serio->dev.parent->of_node : NULL;
 	int error;
 
 	if (parent) {

@@ -555,6 +558,8 @@ static void serio_add_port(struct serio *serio)
 	if (serio->start)
 		serio->start(serio);
 
+	serio->dev.of_node = of_get_next_available_child(parent_node, NULL);
+
 	error = device_add(&serio->dev);
 	if (error)
 		dev_err(&serio->dev,

@@ -902,6 +907,10 @@ static int serio_bus_match(struct device *dev, struct device_driver *drv)
 	struct serio *serio = to_serio_port(dev);
 	struct serio_driver *serio_drv = to_serio_driver(drv);
 
+	if (of_driver_match_device(dev, drv)) {
+		printk("matched %s\n", dev->of_node->name);
+		return 1;
+	}
 	if (serio->manual_bind || serio_drv->manual_bind)
 		return 0;
Signature · 1 line
2.9.3
Acked-by on v1 1/6
#2 lore raw

Re: [RFC PATCH 1/6] serio: add DT driver binding

Quoted 6 lines · Rob Herring

This adds DT driver binding support to the serio bus. The parent of the serio port device must have a device_node associated with it. Typically, this would be the UART device node. The slave device must be a child node of the UART device node.

Signed-off-by Rob Herring <[email protected]>
Acked-by Pavel Machek <[email protected]>

Added vojtech to cc list, as he's original author.

Quoted 13 lines
drivers/input/serio/serio.c +1 -0
index 1ca7f55..9e8eb7a 100644
@@ -36,6 +36,7 @@
 #include <linux/slab.h>
 #include <linux/workqueue.h>
 #include <linux/mutex.h>
+#include <linux/of_device.h>
 
 MODULE_AUTHOR("Vojtech Pavlik <[email protected]>");
 MODULE_DESCRIPTION("Serio abstraction core");

drivers/input/serio/serio.c static int serio_bus_match(struct device *dev, struct device_driver *drv) · line 911

Quoted 5 lines
drivers/input/serio/serio.c +2 -0
@@ -902,6 +907,10 @@ static int serio_bus_match(struct device *dev, struct device_driver *drv)
 	struct serio *serio = to_serio_port(dev);
 	struct serio_driver *serio_drv = to_serio_driver(drv);
 
+	if (of_driver_match_device(dev, drv)) {
+		printk("matched %s\n", dev->of_node->name);

printk probalby should be removed when moving out of RFC phase.

Thanks,
Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
author
#3 lore raw

RFC PATCH 1/6 serio: add OF driver binding

In reply to Rob Herring
Signed-off-by Rob Herring <[email protected]>
---
 drivers/input/serio/serio.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)
drivers/input/serio/serio.c +19 -1
index 1ca7f55..6cfa22f 100644
@@ -36,6 +36,7 @@
 #include <linux/slab.h>
 #include <linux/workqueue.h>
 #include <linux/mutex.h>
+#include <linux/of_device.h>
 
 MODULE_AUTHOR("Vojtech Pavlik <[email protected]>");
 MODULE_DESCRIPTION("Serio abstraction core");

@@ -88,7 +89,7 @@ static void serio_disconnect_driver(struct serio *serio)
 
 static int serio_match_port(const struct serio_device_id *ids, struct serio *serio)
 {
-	while (ids->type || ids->proto) {
+	while (ids && (ids->type || ids->proto)) {
 		if ((ids->type == SERIO_ANY || ids->type == serio->id.type) &&
 		    (ids->proto == SERIO_ANY || ids->proto == serio->id.proto) &&
 		    (ids->extra == SERIO_ANY || ids->extra == serio->id.extra) &&

@@ -107,6 +108,7 @@ static int serio_bind_driver(struct serio *serio, struct serio_driver *drv)
 {
 	int error;
 
+	printk("%s\n", __func__);
 	if (serio_match_port(drv->id_table, serio)) {
 
 		serio->dev.driver = &drv->driver;

@@ -133,6 +135,7 @@ static void serio_find_driver(struct serio *serio)
 {
 	int error;
 
+	printk("%s\n", __func__);
 	error = device_attach(&serio->dev);
 	if (error < 0 && error != -EPROBE_DEFER)
 		dev_warn(&serio->dev,

@@ -542,8 +545,12 @@ static void serio_init_port(struct serio *serio)
 static void serio_add_port(struct serio *serio)
 {
 	struct serio *parent = serio->parent;
+	struct device_node *parent_node =
+		serio->dev.parent ? serio->dev.parent->of_node : NULL;
 	int error;
 
+	printk("%s\n", __func__);
+
 	if (parent) {
 		serio_pause_rx(parent);
 		list_add_tail(&serio->child_node, &parent->children);

@@ -555,6 +562,8 @@ static void serio_add_port(struct serio *serio)
 	if (serio->start)
 		serio->start(serio);
 
+	serio->dev.of_node = of_get_next_available_child(parent_node, NULL);
+
 	error = device_add(&serio->dev);
 	if (error)
 		dev_err(&serio->dev,

@@ -570,6 +579,8 @@ static void serio_destroy_port(struct serio *serio)
 {
 	struct serio *child;
 
+	printk("%s\n", __func__);
+
 	while ((child = serio_get_pending_child(serio)) != NULL) {
 		serio_remove_pending_events(child);
 		put_device(&child->dev);

@@ -791,6 +802,7 @@ static int serio_driver_probe(struct device *dev)
 	struct serio *serio = to_serio_port(dev);
 	struct serio_driver *drv = to_serio_driver(dev->driver);
 
+	printk("%s\n", __func__);
 	return serio_connect_driver(serio, drv);
 }
 

@@ -902,6 +914,12 @@ static int serio_bus_match(struct device *dev, struct device_driver *drv)
 	struct serio *serio = to_serio_port(dev);
 	struct serio_driver *serio_drv = to_serio_driver(drv);
 
+	printk("%s\n", __func__);
+
+	if (of_driver_match_device(dev, drv)) {
+		printk("matched %s\n", dev->of_node->name);
+		return 1;
+	}
 	if (serio->manual_bind || serio_drv->manual_bind)
 		return 0;
Signature · 1 line
2.9.3
author
#4 lore raw

RFC PATCH 2/6 serio: serport: hacks to get DT probe to work

In reply to Rob Herring

For DT matching, the serio->dev.parent is expected to be the UART device which is the tty's parent device. Not sure if this change has any side effects, but this is all just for testing.

The spinlock causes deadlocks if serio_write is called from interrupt handler as write_wakeup gets called. This could be fixed by clearing TTY_DO_WRITE_WAKEUP if the spinlock is locked, but just doing quick hack for now.

Signed-off-by Rob Herring <[email protected]>
---
 drivers/input/serio/serport.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/input/serio/serport.c +3 -3
index 9c927d3..a938c2b 100644
@@ -183,7 +183,7 @@ static ssize_t serport_ldisc_read(struct tty_struct * tty, struct file * file, u
 	serio->open = serport_serio_open;
 	serio->close = serport_serio_close;
 	serio->port_data = serport;
-	serio->dev.parent = tty->dev;
+	serio->dev.parent = tty->dev->parent;
 
 	serio_register_port(serport->serio);
 	printk(KERN_INFO "serio: Serial port %s\n", tty_name(tty));

@@ -253,10 +253,10 @@ static void serport_ldisc_write_wakeup(struct tty_struct * tty)
 	struct serport *serport = (struct serport *) tty->disc_data;
 	unsigned long flags;
 
-	spin_lock_irqsave(&serport->lock, flags);
+//	spin_lock_irqsave(&serport->lock, flags);
 	if (test_bit(SERPORT_ACTIVE, &serport->flags))
 		serio_drv_write_wakeup(serport->serio);
-	spin_unlock_irqrestore(&serport->lock, flags);
+//	spin_unlock_irqrestore(&serport->lock, flags);
 }
 
 /*
Signature · 1 line
2.9.3
author
#5 lore raw

RFC PATCH 3/6 serio: add buffer receive and write functions

In reply to Rob Herring

Currently, serio only supports a character at a time receive and write functions. In order to support higher speed protocols like bluetooth, buffer at a time functions are needed.

This mirrors the tty ldisc interface for write and receive_buf. Now that a whole buffer can be queued, we need to be able to flush the write queue as well.

While the write path doesn't require any updates to serio port drivers as buffered write can be emulated with the existing write() function, the receive path for port drivers must be updated to support the buffered receive.

Signed-off-by Rob Herring <[email protected]>
---
 drivers/input/serio/serio.c | 23 +++++++++++++++++++++++
 include/linux/serio.h       | 43 +++++++++++++++++++++++++++++++++++++++----
 2 files changed, 62 insertions(+), 4 deletions(-)
drivers/input/serio/serio.c +23 -0
index 9e8eb7a..c994581 100644
@@ -989,6 +989,11 @@ int serio_open(struct serio *serio, struct serio_driver *drv)
 		serio_set_drv(serio, NULL);
 		return -1;
 	}
+
+	/* Use buffer receive if the driver provides a callback */
+	if (drv->receive_buf)
+		set_bit(SERIO_MODE_BUFFERED, &drv->flags);
+
 	return 0;
 }
 EXPORT_SYMBOL(serio_open);

@@ -1024,6 +1029,24 @@ irqreturn_t serio_interrupt(struct serio *serio,
 }
 EXPORT_SYMBOL(serio_interrupt);
 
+int serio_receive_buf(struct serio *serio, const unsigned char *data, size_t len)
+{
+	unsigned long flags;
+	int ret = 0;
+
+	spin_lock_irqsave(&serio->lock, flags);
+
+        if (likely(serio->drv))
+                ret = serio->drv->receive_buf(serio, data, len);
+	else if (device_is_registered(&serio->dev))
+		serio_rescan(serio);
+
+	spin_unlock_irqrestore(&serio->lock, flags);
+
+	return ret;
+}
+EXPORT_SYMBOL(serio_receive_buf);
+
 struct bus_type serio_bus = {
 	.name		= "serio",
 	.drv_groups	= serio_driver_groups,
include/linux/serio.h +39 -4

75 lines not on this page · raw message

Signature · 1 line
2.9.3
author
#6 lore raw

RFC PATCH 4/6 serio: serport: add support for buffered write and receive

In reply to Rob Herring

A tty ldisc naturally supports writing a buffer at a time, so convert the serport driver to use the serio write_buf() function. For clients using the existing interfaces, there is no change in functionality.

Signed-off-by Rob Herring <[email protected]>
---
 drivers/input/serio/serport.c | 43 +++++++++++++++++++++++--------------------
 1 file changed, 23 insertions(+), 20 deletions(-)
drivers/input/serio/serport.c +23 -20
index a938c2b..5572c28 100644
@@ -45,10 +45,10 @@ struct serport {
  * Callback functions from the serio code.
  */
 
-static int serport_serio_write(struct serio *serio, unsigned char data)
+static int serport_serio_write_buf(struct serio *serio, const unsigned char *data, size_t len)
 {
 	struct serport *serport = serio->port_data;
-	return -(serport->tty->ops->write(serport->tty, &data, 1) != 1);
+	return serport->tty->ops->write(serport->tty, data, len);
 }
 
 static int serport_serio_open(struct serio *serio)

@@ -133,26 +133,29 @@ static void serport_ldisc_receive(struct tty_struct *tty, const unsigned char *c
 	if (!test_bit(SERPORT_ACTIVE, &serport->flags))
 		goto out;
 
-	for (i = 0; i < count; i++) {
-		if (fp) {
-			switch (fp[i]) {
-			case TTY_FRAME:
-				ch_flags = SERIO_FRAME;
-				break;
-
-			case TTY_PARITY:
-				ch_flags = SERIO_PARITY;
-				break;
-
-			default:
-				ch_flags = 0;
-				break;
+	if (serio_buffered_mode_enabled(serport->serio)) {
+		serio_receive_buf(serport->serio, cp, count);
+	} else {
+		for (i = 0; i < count; i++) {
+			if (fp) {
+				switch (fp[i]) {
+				case TTY_FRAME:
+					ch_flags = SERIO_FRAME;
+					break;
+
+				case TTY_PARITY:
+					ch_flags = SERIO_PARITY;
+					break;
+
+				default:
+					ch_flags = 0;
+					break;
+				}
 			}
-		}
 
-		serio_interrupt(serport->serio, cp[i], ch_flags);
+			serio_interrupt(serport->serio, cp[i], ch_flags);
+		}
 	}
-
 out:
 	spin_unlock_irqrestore(&serport->lock, flags);
 }

@@ -179,7 +182,7 @@ static ssize_t serport_ldisc_read(struct tty_struct * tty, struct file * file, u
 	snprintf(serio->phys, sizeof(serio->phys), "%s/serio0", tty_name(tty));
 	serio->id = serport->id;
 	serio->id.type = SERIO_RS232;
-	serio->write = serport_serio_write;
+	serio->write_buf = serport_serio_write_buf;
 	serio->open = serport_serio_open;
 	serio->close = serport_serio_close;
 	serio->port_data = serport;
Signature · 1 line
2.9.3
#7 lore raw

Re: [RFC PATCH 4/6] serio: serport: add support for buffered write and receive

Hi!

Quoted 10 lines
drivers/input/serio/serport.c +0 -6
@@ -133,26 +133,29 @@ static void serport_ldisc_receive(struct tty_struct *tty, const unsigned char *c
 	if (!test_bit(SERPORT_ACTIVE, &serport->flags))
 		goto out;
 
-	for (i = 0; i < count; i++) {
-		if (fp) {
-			switch (fp[i]) {
-			case TTY_FRAME:
-				ch_flags = SERIO_FRAME;
-				break;

drivers/input/serio/serport.c static void serport_ldisc_receive(struct tty_struct *tty, const unsigned char *c · line 137

Quoted 10 lines
drivers/input/serio/serport.c +2 -8
-
-			case TTY_PARITY:
-				ch_flags = SERIO_PARITY;
-				break;
-
-			default:
-				ch_flags = 0;
-				break;
+	if (serio_buffered_mode_enabled(serport->serio)) {
+		serio_receive_buf(serport->serio, cp, count);

Elsewhere:
+ /* Use buffer receive if the driver provides a callback */
+ if (drv->receive_buf)
+ set_bit(SERIO_MODE_BUFFERED, &drv->flags);

Could we use if (drv->receive_buf) above directly, and not require the bitfield?

Thanks,
Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
author
#8 lore raw

Re: [RFC PATCH 4/6] serio: serport: add support for buffered write and receive

Quoted 28 lines · Pavel Machek

Hi!

Quoted 10 lines
drivers/input/serio/serport.c +0 -6
@@ -133,26 +133,29 @@ static void serport_ldisc_receive(struct tty_struct *tty, const unsigned char *c
      if (!test_bit(SERPORT_ACTIVE, &serport->flags))
              goto out;
 
-     for (i = 0; i < count; i++) {
-             if (fp) {
-                     switch (fp[i]) {
-                     case TTY_FRAME:
-                             ch_flags = SERIO_FRAME;
-                             break;

drivers/input/serio/serport.c static void serport_ldisc_receive(struct tty_struct *tty, const unsigned char *c · line 137

Quoted 10 lines
drivers/input/serio/serport.c +2 -8
-
-                     case TTY_PARITY:
-                             ch_flags = SERIO_PARITY;
-                             break;
-
-                     default:
-                             ch_flags = 0;
-                             break;
+     if (serio_buffered_mode_enabled(serport->serio)) {
+             serio_receive_buf(serport->serio, cp, count);

Elsewhere:

(quoted diff) +3 -0
+       /* Use buffer receive if the driver provides a callback */
+       if (drv->receive_buf)
+               set_bit(SERIO_MODE_BUFFERED, &drv->flags);
Quoted 2 lines

Could we use if (drv->receive_buf) above directly, and not require the bitfield?

I'm allowing for allowing drivers to provide both functions and be able to switch between them. However, I didn't provide any mechanism to do so yet.

Rob

#9 lore raw

Re: [RFC PATCH 4/6] serio: serport: add support for buffered write and receive

Quoted 29 lines · Rob Herring
Quoted 28 lines · Pavel Machek

Hi!

Quoted 10 lines
drivers/input/serio/serport.c +0 -6
@@ -133,26 +133,29 @@ static void serport_ldisc_receive(struct tty_struct *tty, const unsigned char *c
      if (!test_bit(SERPORT_ACTIVE, &serport->flags))
              goto out;
 
-     for (i = 0; i < count; i++) {
-             if (fp) {
-                     switch (fp[i]) {
-                     case TTY_FRAME:
-                             ch_flags = SERIO_FRAME;
-                             break;

drivers/input/serio/serport.c static void serport_ldisc_receive(struct tty_struct *tty, const unsigned char *c · line 137

Quoted 10 lines
drivers/input/serio/serport.c +2 -8
-
-                     case TTY_PARITY:
-                             ch_flags = SERIO_PARITY;
-                             break;
-
-                     default:
-                             ch_flags = 0;
-                             break;
+     if (serio_buffered_mode_enabled(serport->serio)) {
+             serio_receive_buf(serport->serio, cp, count);

Elsewhere:

(quoted diff) +3 -0
+       /* Use buffer receive if the driver provides a callback */
+       if (drv->receive_buf)
+               set_bit(SERIO_MODE_BUFFERED, &drv->flags);
Quoted 6 lines
Quoted 2 lines

Could we use if (drv->receive_buf) above directly, and not require the bitfield?

I'm allowing for allowing drivers to provide both functions and be able to switch between them. However, I didn't provide any mechanism to do so yet.

Dunno -- does switching make sense? IMO we'd want to migrate all the drivers to the "blocks" interface... no need to switch between the two.

Best regards,
Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
author
#10 lore raw

RFC PATCH 5/6 serio: add serial configuration functions

In reply to Rob Herring

Just stub functions ATM.

Signed-off-by Rob Herring <[email protected]>
---
 include/linux/serio.h | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
include/linux/serio.h +16 -0
index 5d0b69f..5bf1754 100644
@@ -41,6 +41,8 @@ struct serio {
 	void (*close)(struct serio *);
 	int (*start)(struct serio *);
 	void (*stop)(struct serio *);
+	void (*set_flow_control)(struct serio *, bool);
+	unsigned int (*set_baudrate)(struct serio *, unsigned int);
 
 	struct serio *parent;
 	/* Entry in parent->children list */

@@ -170,6 +172,20 @@ static inline void serio_drv_write_wakeup(struct serio *serio)
 		serio->drv->write_wakeup(serio);
 }
 
+static inline void serio_set_flow_control(struct serio *serio, bool enable)
+{
+	if (serio->set_flow_control)
+		serio->set_flow_control(serio, enable);
+}
+
+static inline unsigned int serio_set_baudrate(struct serio *serio, unsigned int speed)
+{
+	if (serio->set_baudrate)
+		return serio->set_baudrate(serio, speed);
+
+	return 0;
+}
+
 /*
  * Use the following functions to manipulate serio's per-port
  * driver-specific data.
Signature · 1 line
2.9.3
author
#11 lore raw

RFC PATCH 6/6 bluetooth: hack up ldisc to use serio

In reply to Rob Herring

This hacks up the BT ldisc in place to work as a serio driver. It will need refactoring into common, ldisc, and serio parts.

It is working under QEMU to the point the driver can bind to a serio device via DT, register as a BT device, start sending out initial packets and receive data (typed at a terminal). Now I need to find a real device.

Still need to figure out how to plumb a few tty things like TTY_DO_WRITE_WAKEUP bit handling and tty_unthrottle.

Signed-off-by Rob Herring <[email protected]>
---
 drivers/bluetooth/hci_ldisc.c | 261 +++++++++++++++++-------------------------
 drivers/bluetooth/hci_uart.h  |   3 +
 2 files changed, 109 insertions(+), 155 deletions(-)
drivers/bluetooth/hci_ldisc.c +106 -155
index dda9739..8149952 100644
@@ -34,7 +34,9 @@
 #include <linux/poll.h>
 
 #include <linux/slab.h>
-#include <linux/tty.h>
+#include <linux/serio.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/errno.h>
 #include <linux/string.h>
 #include <linux/signal.h>

@@ -138,7 +140,7 @@ int hci_uart_tx_wakeup(struct hci_uart *hu)
 static void hci_uart_write_work(struct work_struct *work)
 {
 	struct hci_uart *hu = container_of(work, struct hci_uart, write_work);
-	struct tty_struct *tty = hu->tty;
+	struct serio *serio = hu->serio;
 	struct hci_dev *hdev = hu->hdev;
 	struct sk_buff *skb;
 

@@ -152,8 +154,8 @@ restart:
 	while ((skb = hci_uart_dequeue(hu))) {
 		int len;
 
-		set_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
-		len = tty->ops->write(tty, skb->data, skb->len);
+//		set_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
+		len = serio_write_buf(serio, skb->data, skb->len);
 		hdev->stat.byte_tx += len;
 
 		skb_pull(skb, len);

@@ -215,17 +217,15 @@ static int hci_uart_open(struct hci_dev *hdev)
 static int hci_uart_flush(struct hci_dev *hdev)
 {
 	struct hci_uart *hu  = hci_get_drvdata(hdev);
-	struct tty_struct *tty = hu->tty;
 
-	BT_DBG("hdev %p tty %p", hdev, tty);
+	BT_DBG("hdev %p serio %p", hdev, hu->serio);
 
 	if (hu->tx_skb) {
 		kfree_skb(hu->tx_skb); hu->tx_skb = NULL;
 	}
 
 	/* Flush any pending characters in the driver and discipline. */
-	tty_ldisc_flush(tty);
-	tty_driver_flush_buffer(tty);
+	serio_write_flush(hu->serio);
 
 	if (test_bit(HCI_UART_PROTO_READY, &hu->flags))
 		hu->proto->flush(hu);

@@ -261,6 +261,8 @@ static int hci_uart_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
 /* Flow control or un-flow control the device */
 void hci_uart_set_flow_control(struct hci_uart *hu, bool enable)
 {
+	serio_set_flow_control(hu->serio, enable);
+#if 0
 	struct tty_struct *tty = hu->tty;
 	struct ktermios ktermios;
 	int status;

@@ -309,6 +311,7 @@ void hci_uart_set_flow_control(struct hci_uart *hu, bool enable)
 		BT_DBG("Enabling hardware flow control: %s",
 		       status ? "failed" : "success");
 	}
+#endif
 }
 
 void hci_uart_set_speeds(struct hci_uart *hu, unsigned int init_speed,

@@ -317,7 +320,7 @@ void hci_uart_set_speeds(struct hci_uart *hu, unsigned int init_speed,
 	hu->init_speed = init_speed;
 	hu->oper_speed = oper_speed;
 }
-
+#if 0
 void hci_uart_init_tty(struct hci_uart *hu)
 {
 	struct tty_struct *tty = hu->tty;

@@ -336,21 +339,13 @@ void hci_uart_init_tty(struct hci_uart *hu)
 	/* tty_set_termios() return not checked as it is always 0 */
 	tty_set_termios(tty, &ktermios);
 }
-
+#endif
 void hci_uart_set_baudrate(struct hci_uart *hu, unsigned int speed)
 {
-	struct tty_struct *tty = hu->tty;
-	struct ktermios ktermios;
-
-	ktermios = tty->termios;
-	ktermios.c_cflag &= ~CBAUD;
-	tty_termios_encode_baud_rate(&ktermios, speed, speed);
-
-	/* tty_set_termios() return not checked as it is always 0 */
-	tty_set_termios(tty, &ktermios);
+	int out_speed = serio_set_baudrate(hu->serio, speed);
 
 	BT_DBG("%s: New tty speeds: %d/%d", hu->hdev->name,
-	       tty->termios.c_ispeed, tty->termios.c_ospeed);
+	       speed, out_speed);
 }
 
 static int hci_uart_setup(struct hci_dev *hdev)

@@ -428,83 +423,6 @@ done:
 	return 0;
 }
 
-/* ------ LDISC part ------ */
-/* hci_uart_tty_open
- *
- *     Called when line discipline changed to HCI_UART.
- *
- * Arguments:
- *     tty    pointer to tty info structure
- * Return Value:
- *     0 if success, otherwise error code
- */
-static int hci_uart_tty_open(struct tty_struct *tty)
-{
-	struct hci_uart *hu;
-
-	BT_DBG("tty %p", tty);
-
-	/* Error if the tty has no write op instead of leaving an exploitable
-	   hole */
-	if (tty->ops->write == NULL)
-		return -EOPNOTSUPP;
-
-	hu = kzalloc(sizeof(struct hci_uart), GFP_KERNEL);
-	if (!hu) {
-		BT_ERR("Can't allocate control structure");
-		return -ENFILE;
-	}
-
-	tty->disc_data = hu;
-	hu->tty = tty;
-	tty->receive_room = 65536;
-
-	INIT_WORK(&hu->init_ready, hci_uart_init_work);
-	INIT_WORK(&hu->write_work, hci_uart_write_work);
-
-	/* Flush any pending characters in the driver */
-	tty_driver_flush_buffer(tty);
-
-	return 0;
-}
-
-/* hci_uart_tty_close()
- *
- *    Called when the line discipline is changed to something
- *    else, the tty is closed, or the tty detects a hangup.
- */
-static void hci_uart_tty_close(struct tty_struct *tty)
-{
-	struct hci_uart *hu = tty->disc_data;
-	struct hci_dev *hdev;
-
-	BT_DBG("tty %p", tty);
-
-	/* Detach from the tty */
-	tty->disc_data = NULL;
-
-	if (!hu)
-		return;
-
-	hdev = hu->hdev;
-	if (hdev)
-		hci_uart_close(hdev);
-
-	cancel_work_sync(&hu->write_work);
-
-	if (test_and_clear_bit(HCI_UART_PROTO_READY, &hu->flags)) {
-		if (hdev) {
-			if (test_bit(HCI_UART_REGISTERED, &hu->flags))
-				hci_unregister_dev(hdev);
-			hci_free_dev(hdev);
-		}
-		hu->proto->close(hu);
-	}
-	clear_bit(HCI_UART_PROTO_SET, &hu->flags);
-
-	kfree(hu);
-}
-
 /* hci_uart_tty_wakeup()
  *
  *    Callback for transmit wakeup. Called when low level

@@ -513,18 +431,18 @@ static void hci_uart_tty_close(struct tty_struct *tty)
  * Arguments:        tty    pointer to associated tty instance data
  * Return Value:    None
  */
-static void hci_uart_tty_wakeup(struct tty_struct *tty)
+static void hci_uart_serio_wakeup(struct serio *serio)
 {
-	struct hci_uart *hu = tty->disc_data;
+	struct hci_uart *hu = serio_get_drvdata(serio);
 
 	BT_DBG("");
 
 	if (!hu)
 		return;
 
-	clear_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
+//	clear_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
 
-	if (tty != hu->tty)
+	if (serio != hu->serio)
 		return;
 
 	if (test_bit(HCI_UART_PROTO_READY, &hu->flags))

@@ -543,16 +461,16 @@ static void hci_uart_tty_wakeup(struct tty_struct *tty)
  *
  * Return Value:    None
  */
-static void hci_uart_tty_receive(struct tty_struct *tty, const u8 *data,
-				 char *flags, int count)
+static int hci_uart_serio_receive(struct serio *serio, const u8 *data,
+				   size_t count)
 {
-	struct hci_uart *hu = tty->disc_data;
+	struct hci_uart *hu = serio_get_drvdata(serio);
 
-	if (!hu || tty != hu->tty)
-		return;
+	if (!hu || serio != hu->serio)
+		return 0;
 
 	if (!test_bit(HCI_UART_PROTO_READY, &hu->flags))
-		return;
+		return 0;
 
 	/* It does not need a lock here as it is already protected by a mutex in
 	 * tty caller

@@ -562,7 +480,8 @@ static void hci_uart_tty_receive(struct tty_struct *tty, const u8 *data,
 	if (hu->hdev)
 		hu->hdev->stat.byte_rx += count;
 
-	tty_unthrottle(tty);
+//	tty_unthrottle(tty);
+	return count;
 }
 
 static int hci_uart_register_dev(struct hci_uart *hu)

@@ -595,7 +514,7 @@ static int hci_uart_register_dev(struct hci_uart *hu)
 	hdev->flush = hci_uart_flush;
 	hdev->send  = hci_uart_send_frame;
 	hdev->setup = hci_uart_setup;
-	SET_HCIDEV_DEV(hdev, hu->tty->dev);
+	SET_HCIDEV_DEV(hdev, &hu->serio->dev);
 
 	if (test_bit(HCI_UART_RAW_DEVICE, &hu->hdev_flags))
 		set_bit(HCI_QUIRK_RAW_DEVICE, &hdev->quirks);

@@ -650,6 +569,7 @@ static int hci_uart_set_proto(struct hci_uart *hu, int id)
 
 	return 0;
 }
+#if 0
 
 static int hci_uart_set_flags(struct hci_uart *hu, unsigned long flags)
 {

@@ -733,56 +653,95 @@ static int hci_uart_tty_ioctl(struct tty_struct *tty, struct file *file,
 
 	return err;
 }
+#endif
 
-/*
- * We don't provide read/write/poll interface for user space.
- */
-static ssize_t hci_uart_tty_read(struct tty_struct *tty, struct file *file,
-				 unsigned char __user *buf, size_t nr)
+static int hci_uart_connect(struct serio *serio, struct serio_driver *drv)
 {
-	return 0;
-}
+	int id, ret;
+	struct hci_uart *hu;
+
+	BT_INFO("HCI UART driver ver %s", VERSION);
+
+	id = (int)of_device_get_match_data(&serio->dev);
+
+	hu = devm_kzalloc(&serio->dev, sizeof(struct hci_uart), GFP_KERNEL);
+	if (!hu)
+		return -ENFILE;
+
+	serio_set_drvdata(serio, hu);
+	hu->serio = serio;
+
+//	tty->receive_room = 65536;
+
+	INIT_WORK(&hu->init_ready, hci_uart_init_work);
+	INIT_WORK(&hu->write_work, hci_uart_write_work);
+
+	ret = serio_open(serio, drv);
+	if (ret)
+		return ret;
+
+	set_bit(HCI_UART_PROTO_SET, &hu->flags);
+	ret = hci_uart_set_proto(hu, id);
+	if (ret) {
+		serio_close(serio);
+		return ret;
+	}
+
+	/* Flush any pending characters in the driver */
+//	tty_driver_flush_buffer(tty);
 
-static ssize_t hci_uart_tty_write(struct tty_struct *tty, struct file *file,
-				  const unsigned char *data, size_t count)
-{
-	return 0;
-}
 
-static unsigned int hci_uart_tty_poll(struct tty_struct *tty,
-				      struct file *filp, poll_table *wait)
-{
 	return 0;
 }
 
-static int __init hci_uart_init(void)
+static void hci_uart_disconnect(struct serio *serio)
 {
-	static struct tty_ldisc_ops hci_uart_ldisc;
-	int err;
+	struct hci_dev *hdev;
+	struct hci_uart *hu = serio_get_drvdata(serio);
 
-	BT_INFO("HCI UART driver ver %s", VERSION);
+	hdev = hu->hdev;
+	if (hdev)
+		hci_uart_close(hdev);
 
-	/* Register the tty discipline */
-
-	memset(&hci_uart_ldisc, 0, sizeof(hci_uart_ldisc));
-	hci_uart_ldisc.magic		= TTY_LDISC_MAGIC;
-	hci_uart_ldisc.name		= "n_hci";
-	hci_uart_ldisc.open		= hci_uart_tty_open;
-	hci_uart_ldisc.close		= hci_uart_tty_close;
-	hci_uart_ldisc.read		= hci_uart_tty_read;
-	hci_uart_ldisc.write		= hci_uart_tty_write;
-	hci_uart_ldisc.ioctl		= hci_uart_tty_ioctl;
-	hci_uart_ldisc.poll		= hci_uart_tty_poll;
-	hci_uart_ldisc.receive_buf	= hci_uart_tty_receive;
-	hci_uart_ldisc.write_wakeup	= hci_uart_tty_wakeup;
-	hci_uart_ldisc.owner		= THIS_MODULE;
-
-	err = tty_register_ldisc(N_HCI, &hci_uart_ldisc);
-	if (err) {
-		BT_ERR("HCI line discipline registration failed. (%d)", err);
-		return err;
+	cancel_work_sync(&hu->write_work);
+
+	if (test_and_clear_bit(HCI_UART_PROTO_READY, &hu->flags)) {
+		if (hdev) {
+			if (test_bit(HCI_UART_REGISTERED, &hu->flags))
+				hci_unregister_dev(hdev);
+			hci_free_dev(hdev);
+		}
+		hu->proto->close(hu);
 	}
+	clear_bit(HCI_UART_PROTO_SET, &hu->flags);
+
+	pr_info("hci_uart disconnect!!!\n");
+	serio_close(serio);
+}
+
+
+static const struct of_device_id hci_uart_of_match[] = {
+	{ .compatible = "loopback-uart", .data = (void *)HCI_UART_BCSP },
+	{},
+};
+MODULE_DEVICE_TABLE(of, hci_uart_of_match);
+
+static struct serio_driver serio_hci_uart_drv = {
+	.driver		= {
+		.name	= "hci-uart",
+		.of_match_table = of_match_ptr(hci_uart_of_match),
+	},
+	.description	= "hci uart",
+	.write_wakeup	= hci_uart_serio_wakeup,
+	.receive_buf	= hci_uart_serio_receive,
+	.connect	= hci_uart_connect,
+	.disconnect	= hci_uart_disconnect,
+};
 
+module_serio_driver(serio_hci_uart_drv);
+
+static int __init hci_uart_init(void)
+{
 #ifdef CONFIG_BT_HCIUART_H4
 	h4_init();
 #endif

@@ -816,8 +775,6 @@ static int __init hci_uart_init(void)
 
 static void __exit hci_uart_exit(void)
 {
-	int err;
-
 #ifdef CONFIG_BT_HCIUART_H4
 	h4_deinit();
 #endif

@@ -845,11 +802,6 @@ static void __exit hci_uart_exit(void)
 #ifdef CONFIG_BT_HCIUART_AG6XX
 	ag6xx_deinit();
 #endif
-
-	/* Release tty registration of line discipline */
-	err = tty_unregister_ldisc(N_HCI);
-	if (err)
-		BT_ERR("Can't unregister HCI line discipline (%d)", err);
 }
 
 module_init(hci_uart_init);

@@ -859,4 +811,3 @@ MODULE_AUTHOR("Marcel Holtmann <[email protected]>");
 MODULE_DESCRIPTION("Bluetooth HCI UART driver ver " VERSION);
 MODULE_VERSION(VERSION);
 MODULE_LICENSE("GPL");
-MODULE_ALIAS_LDISC(N_HCI);
drivers/bluetooth/hci_uart.h +3 -0
index 839bad1..c48dddc 100644
@@ -73,8 +73,11 @@ struct hci_uart_proto {
 	struct sk_buff *(*dequeue)(struct hci_uart *hu);
 };
 
+struct serio;
+
 struct hci_uart {
 	struct tty_struct	*tty;
+	struct serio		*serio;
 	struct hci_dev		*hdev;
 	unsigned long		flags;
 	unsigned long		hdev_flags;
Signature · 1 line
2.9.3
#12 lore raw
In reply to Rob Herring
Quoted 5 lines · Rob Herring

This is a new approach to supporting UART slave devices using the existing serio bus. After Arnd's proding, I took another look at serio and decided extending it does make sense. Using serio primarily requires adding DT based device matching and supporting buffer based write and receive.

Strange, the series has _two_ 1/6 patches.

Quoted 2 lines

Rob Herring (6):
serio: add DT driver binding

There's one saying "add OF driver binding" on the mailinglist, too... Best regards,
Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
author
#13 lore raw
Quoted 7 lines · Pavel Machek
Quoted 5 lines · Rob Herring

This is a new approach to supporting UART slave devices using the existing serio bus. After Arnd's proding, I took another look at serio and decided extending it does make sense. Using serio primarily requires adding DT based device matching and supporting buffer based write and receive.

Quoted 6 lines

Strange, the series has _two_ 1/6 patches.

Quoted 2 lines

Rob Herring (6):
serio: add DT driver binding

There's one saying "add OF driver binding" on the mailinglist, too...

Ugg, ignore that one. That's what I get for not cleaning out patches before refreshing them with git-format-patch.

Rob

Tested-by on the v1 series
#14 lore raw
In reply to Rob Herring

Hi,

Quoted 26 lines · Rob Herring

This is a new approach to supporting UART slave devices using the existing serio bus. After Arnd's proding, I took another look at serio and decided extending it does make sense. Using serio primarily requires adding DT based device matching and supporting buffer based write and receive.

Currently, I'm using the existing serio serport ldisc for testing. This requires using inputattach to open the tty and set the ldisc which in turn registers a serio port with the serio core:

inputattach -bare /dev/ttyAMA1

Once a tty_port based serio port driver is in place, this step will not be needed. Supporting cases like a USB UART will also work if the USB UART is described in DT. If not described in DT, I'm not sure if the existing serio manual binding is sufficient (Need to figure out how that works). Slave drivers also need other ways to specify additional data using module params perhaps. Getting DT overlays to work for non-discoverable devices behind discoverable buses (i.e. detached from a base DT) is another option, but that's not yet supported in general.

I've done all the serio changes in place, but ultimately I think at least the core of serio should be moved out of drivers/input/. I don't think it belongs under drivers/tty/ or drivers/tty/serial/, so drivers/serio/?

Quoted 4 lines

BT is working under QEMU to the point a slave driver can bind to a serio port device via DT, register as a BT device, start sending out initial packets and receive data (typed at a terminal). Now I need to find a real device.

I had a more detailed look at the series during the last two weeks. For me the approach looks ok and it should work for the nokia bluetooth use case. Actually my work on that driver is more or less stalled until this is solved, so it would be nice to get this forward. Whose feedback is this waiting from? I guess

* Alan & Greg for the serial parts
* Marcel for the bluetooth parts
* Dmitry for the serio parts

Maybe you can try to find some minutes at the Kernel Summit to talk about this?

Anyways, for the series:

Tested-by Sebastian Reichel <[email protected]>

-- Sebastian

author
#15 lore raw
Quoted 34 lines · Sebastian Reichel

Hi,

Quoted 26 lines · Rob Herring

This is a new approach to supporting UART slave devices using the existing serio bus. After Arnd's proding, I took another look at serio and decided extending it does make sense. Using serio primarily requires adding DT based device matching and supporting buffer based write and receive.

Currently, I'm using the existing serio serport ldisc for testing. This requires using inputattach to open the tty and set the ldisc which in turn registers a serio port with the serio core:

inputattach -bare /dev/ttyAMA1

Once a tty_port based serio port driver is in place, this step will not be needed. Supporting cases like a USB UART will also work if the USB UART is described in DT. If not described in DT, I'm not sure if the existing serio manual binding is sufficient (Need to figure out how that works). Slave drivers also need other ways to specify additional data using module params perhaps. Getting DT overlays to work for non-discoverable devices behind discoverable buses (i.e. detached from a base DT) is another option, but that's not yet supported in general.

I've done all the serio changes in place, but ultimately I think at least the core of serio should be moved out of drivers/input/. I don't think it belongs under drivers/tty/ or drivers/tty/serial/, so drivers/serio/?

Quoted 4 lines

BT is working under QEMU to the point a slave driver can bind to a serio port device via DT, register as a BT device, start sending out initial packets and receive data (typed at a terminal). Now I need to find a real device.

Quoted 5 lines

I had a more detailed look at the series during the last two weeks. For me the approach looks ok and it should work for the nokia bluetooth use case. Actually my work on that driver is more or less stalled until this is solved, so it would be nice to get this forward. Whose feedback is this waiting from? I guess

I think it is mainly waiting for me to spend more time on it and get the tty port part done. I could use help especially for converting the BT part properly.

Quoted 6 lines

* Alan & Greg for the serial parts
* Marcel for the bluetooth parts
* Dmitry for the serio parts

Maybe you can try to find some minutes at the Kernel Summit to talk about this?

Still waiting for my invite...

But I will be at Plumbers if folks want to discuss this.

Rob

#16 lore raw

Hi,

Quoted 11 lines · Rob Herring
Quoted 7 lines · Sebastian Reichel
Quoted 1 line · Rob Herring

[...]

I had a more detailed look at the series during the last two weeks. For me the approach looks ok and it should work for the nokia bluetooth use case. Actually my work on that driver is more or less stalled until this is solved, so it would be nice to get this forward. Whose feedback is this waiting from? I guess

I think it is mainly waiting for me to spend more time on it and get the tty port part done.

The general approach could already be commented on.

Quoted 1 line

I could use help especially for converting the BT part properly.

Ok, I will have a look at that.

Quoted 9 lines
Quoted 6 lines

* Alan & Greg for the serial parts
* Marcel for the bluetooth parts
* Dmitry for the serio parts

Maybe you can try to find some minutes at the Kernel Summit to talk about this?

Still waiting for my invite...
But I will be at Plumbers if folks want to discuss this.

Ok. I obviously assumed invites have already been sent and that you would be invited.

-- Sebastian

#17 lore raw
In reply to Rob Herring
Quoted 7 lines · Rob Herring
Quoted 3 lines

Maybe you can try to find some minutes at the Kernel Summit to talk about this?

Still waiting for my invite...

But I will be at Plumbers if folks want to discuss this.

Hey Rob,

I'm here so let's make time to discuss this.

Regards,
Peter Hurley

#18 lore raw

[ this time w/o the html ]

Quoted 6 lines · Rob Herring
Quoted 2 lines

Maybe you can try to find some minutes at the Kernel Summit to talk about this?

Still waiting for my invite...

But I will be at Plumbers if folks want to discuss this.

Hey Rob,

I'm here so let's make time to discuss this.

Regards,
Peter Hurley

author
#19 lore raw
In reply to Peter Hurley
Quoted 5 lines · Peter Hurley
Quoted 4 lines · Rob Herring
Quoted 3 lines

Maybe you can try to find some minutes at the Kernel Summit to talk about this?

Quoted 8 lines
Quoted 3 lines

Still waiting for my invite...

But I will be at Plumbers if folks want to discuss this.

Hey Rob,

I'm here so let's make time to discuss this.

Tomorrow should be good for me after the complex device dependencies discussion.

Rob

author
#20 lore raw
Quoted 8 lines · Rob Herring
Quoted 7 lines · Peter Hurley
Quoted 6 lines · Rob Herring
Quoted 3 lines

Maybe you can try to find some minutes at the Kernel Summit to talk about this?

Still waiting for my invite...

Quoted 8 lines
Quoted 6 lines
Quoted 1 line

But I will be at Plumbers if folks want to discuss this.

Hey Rob,

I'm here so let's make time to discuss this.

Tomorrow should be good for me after the complex device dependencies discussion.

I have a somewhat working branch using ttyport and serio here[1]. It works with a BT device to the level the device works with my PC which is only identifying the BCM device and loading the firmware.

I'm still not sure using serio really buys us much. We probably want to hide some of the sysfs files when using DT binding. It also provides a file to reconnect devices. I suppose this is because there is typically no hotplug detection on PS/2 ports? Serio also provides its own async binding mechanism. Perhaps this can be updated to use async probing?

The registration of serio ports is a bit problematic. I had the tty port registration call into the serio ttyport driver to register, but this broke serio working as a module. While serial drivers can be modules, the tty_port code is always built-in. So the serio ttyport driver will request a callback for all tty ports when the serio driver is loaded. This means any tty driver hotplugged after serio will not be registered with serio. The only other solution I see is serio would have to be built-in.

Rob

[1] git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git serio-tty-slave