linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 0/6] UART slave devices using serio
@ 2016-08-24 23:24 Rob Herring
  2016-08-24 23:24 ` [RFC PATCH 1/6] serio: add DT driver binding Rob Herring
                   ` (8 more replies)
  0 siblings, 9 replies; 20+ messages in thread
From: Rob Herring @ 2016-08-24 23:24 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Marcel Holtmann, Jiri Slaby,
	Sebastian Reichel, Arnd Bergmann, Dr . H . Nikolaus Schaller,
	Alan Cox
  Cc: Loic Poulain, Pavel Machek, Peter Hurley, NeilBrown,
	Linus Walleij, linux-bluetooth, linux-serial, linux-kernel

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [RFC PATCH 1/6] serio: add DT driver binding
  2016-08-24 23:24 [RFC PATCH 0/6] UART slave devices using serio Rob Herring
@ 2016-08-24 23:24 ` Rob Herring
  2016-08-29  9:57   ` Pavel Machek
  2016-08-24 23:24 ` [RFC PATCH 1/6] serio: add OF " Rob Herring
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: Rob Herring @ 2016-08-24 23:24 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Marcel Holtmann, Jiri Slaby,
	Sebastian Reichel, Arnd Bergmann, Dr . H . Nikolaus Schaller,
	Alan Cox
  Cc: Loic Poulain, Pavel Machek, Peter Hurley, NeilBrown,
	Linus Walleij, linux-bluetooth, linux-serial, linux-kernel

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 <robh@kernel.org>
---
 drivers/input/serio/serio.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/input/serio/serio.c b/drivers/input/serio/serio.c
index 1ca7f55..9e8eb7a 100644
--- a/drivers/input/serio/serio.c
+++ b/drivers/input/serio/serio.c
@@ -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 <vojtech@ucw.cz>");
 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;
 
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [RFC PATCH 1/6] serio: add OF driver binding
  2016-08-24 23:24 [RFC PATCH 0/6] UART slave devices using serio Rob Herring
  2016-08-24 23:24 ` [RFC PATCH 1/6] serio: add DT driver binding Rob Herring
@ 2016-08-24 23:24 ` Rob Herring
  2016-08-24 23:24 ` [RFC PATCH 2/6] serio: serport: hacks to get DT probe to work Rob Herring
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 20+ messages in thread
From: Rob Herring @ 2016-08-24 23:24 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Marcel Holtmann, Jiri Slaby,
	Sebastian Reichel, Arnd Bergmann, Dr . H . Nikolaus Schaller,
	Alan Cox
  Cc: Loic Poulain, Pavel Machek, Peter Hurley, NeilBrown,
	Linus Walleij, linux-bluetooth, linux-serial, linux-kernel

Signed-off-by: Rob Herring <robh@kernel.org>
---
 drivers/input/serio/serio.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/input/serio/serio.c b/drivers/input/serio/serio.c
index 1ca7f55..6cfa22f 100644
--- a/drivers/input/serio/serio.c
+++ b/drivers/input/serio/serio.c
@@ -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 <vojtech@ucw.cz>");
 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;
 
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [RFC PATCH 2/6] serio: serport: hacks to get DT probe to work
  2016-08-24 23:24 [RFC PATCH 0/6] UART slave devices using serio Rob Herring
  2016-08-24 23:24 ` [RFC PATCH 1/6] serio: add DT driver binding Rob Herring
  2016-08-24 23:24 ` [RFC PATCH 1/6] serio: add OF " Rob Herring
@ 2016-08-24 23:24 ` Rob Herring
  2016-08-24 23:24 ` [RFC PATCH 3/6] serio: add buffer receive and write functions Rob Herring
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 20+ messages in thread
From: Rob Herring @ 2016-08-24 23:24 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Marcel Holtmann, Jiri Slaby,
	Sebastian Reichel, Arnd Bergmann, Dr . H . Nikolaus Schaller,
	Alan Cox
  Cc: Loic Poulain, Pavel Machek, Peter Hurley, NeilBrown,
	Linus Walleij, linux-bluetooth, linux-serial, linux-kernel

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 <robh@kernel.org>
---
 drivers/input/serio/serport.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/input/serio/serport.c b/drivers/input/serio/serport.c
index 9c927d3..a938c2b 100644
--- a/drivers/input/serio/serport.c
+++ b/drivers/input/serio/serport.c
@@ -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);
 }
 
 /*
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [RFC PATCH 3/6] serio: add buffer receive and write functions
  2016-08-24 23:24 [RFC PATCH 0/6] UART slave devices using serio Rob Herring
                   ` (2 preceding siblings ...)
  2016-08-24 23:24 ` [RFC PATCH 2/6] serio: serport: hacks to get DT probe to work Rob Herring
@ 2016-08-24 23:24 ` Rob Herring
  2016-08-24 23:24 ` [RFC PATCH 4/6] serio: serport: add support for buffered write and receive Rob Herring
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 20+ messages in thread
From: Rob Herring @ 2016-08-24 23:24 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Marcel Holtmann, Jiri Slaby,
	Sebastian Reichel, Arnd Bergmann, Dr . H . Nikolaus Schaller,
	Alan Cox
  Cc: Loic Poulain, Pavel Machek, Peter Hurley, NeilBrown,
	Linus Walleij, linux-bluetooth, linux-serial, linux-kernel

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 <robh@kernel.org>
---
 drivers/input/serio/serio.c | 23 +++++++++++++++++++++++
 include/linux/serio.h       | 43 +++++++++++++++++++++++++++++++++++++++----
 2 files changed, 62 insertions(+), 4 deletions(-)

diff --git a/drivers/input/serio/serio.c b/drivers/input/serio/serio.c
index 9e8eb7a..c994581 100644
--- a/drivers/input/serio/serio.c
+++ b/drivers/input/serio/serio.c
@@ -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,
diff --git a/include/linux/serio.h b/include/linux/serio.h
index c733cff..5d0b69f 100644
--- a/include/linux/serio.h
+++ b/include/linux/serio.h
@@ -35,6 +35,8 @@ struct serio {
 	spinlock_t lock;
 
 	int (*write)(struct serio *, unsigned char);
+	int (*write_buf)(struct serio *, const unsigned char *, size_t);
+	void (*write_flush)(struct serio *);
 	int (*open)(struct serio *);
 	void (*close)(struct serio *);
 	int (*start)(struct serio *);
@@ -69,12 +71,16 @@ struct serio {
 
 struct serio_driver {
 	const char *description;
+	unsigned long flags;
+
+#define SERIO_MODE_BUFFERED	1
 
 	const struct serio_device_id *id_table;
 	bool manual_bind;
 
 	void (*write_wakeup)(struct serio *);
 	irqreturn_t (*interrupt)(struct serio *, unsigned char, unsigned int);
+	int (*receive_buf)(struct serio *, const unsigned char *, size_t);
 	int  (*connect)(struct serio *, struct serio_driver *drv);
 	int  (*reconnect)(struct serio *);
 	void (*disconnect)(struct serio *);
@@ -89,6 +95,12 @@ void serio_close(struct serio *serio);
 void serio_rescan(struct serio *serio);
 void serio_reconnect(struct serio *serio);
 irqreturn_t serio_interrupt(struct serio *serio, unsigned char data, unsigned int flags);
+int serio_receive_buf(struct serio *serio, const unsigned char *data, size_t len);
+
+static inline bool serio_buffered_mode_enabled(struct serio *serio)
+{
+	return test_bit(SERIO_MODE_BUFFERED, &serio->drv->flags);
+}
 
 void __serio_register_port(struct serio *serio, struct module *owner);
 
@@ -121,12 +133,35 @@ void serio_unregister_driver(struct serio_driver *drv);
 	module_driver(__serio_driver, serio_register_driver, \
 		       serio_unregister_driver)
 
+static inline int serio_write_buf(struct serio *serio, const unsigned char *data, size_t len)
+{
+	int ret;
+
+	if (serio->write_buf)
+		return serio->write_buf(serio, data, len);
+	else if (serio->write) {
+		int i;
+		for (i = 0; i < len; i++) {
+			ret = serio->write(serio, data[i]);
+			if (ret)
+				break;
+		}
+		return i;
+	}
+
+	return -1;
+}
+
 static inline int serio_write(struct serio *serio, unsigned char data)
 {
-	if (serio->write)
-		return serio->write(serio, data);
-	else
-		return -1;
+	int ret = serio_write_buf(serio, &data, 1);
+	return ret == 1 ? 0 : ret;
+}
+
+static inline void serio_write_flush(struct serio *serio)
+{
+	if (serio->write_flush)
+		serio->write_flush(serio);
 }
 
 static inline void serio_drv_write_wakeup(struct serio *serio)
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [RFC PATCH 4/6] serio: serport: add support for buffered write and receive
  2016-08-24 23:24 [RFC PATCH 0/6] UART slave devices using serio Rob Herring
                   ` (3 preceding siblings ...)
  2016-08-24 23:24 ` [RFC PATCH 3/6] serio: add buffer receive and write functions Rob Herring
@ 2016-08-24 23:24 ` Rob Herring
  2016-08-26 20:12   ` Pavel Machek
  2016-08-24 23:24 ` [RFC PATCH 5/6] serio: add serial configuration functions Rob Herring
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: Rob Herring @ 2016-08-24 23:24 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Marcel Holtmann, Jiri Slaby,
	Sebastian Reichel, Arnd Bergmann, Dr . H . Nikolaus Schaller,
	Alan Cox
  Cc: Loic Poulain, Pavel Machek, Peter Hurley, NeilBrown,
	Linus Walleij, linux-bluetooth, linux-serial, linux-kernel

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 <robh@kernel.org>
---
 drivers/input/serio/serport.c | 43 +++++++++++++++++++++++--------------------
 1 file changed, 23 insertions(+), 20 deletions(-)

diff --git a/drivers/input/serio/serport.c b/drivers/input/serio/serport.c
index a938c2b..5572c28 100644
--- a/drivers/input/serio/serport.c
+++ b/drivers/input/serio/serport.c
@@ -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;
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [RFC PATCH 5/6] serio: add serial configuration functions
  2016-08-24 23:24 [RFC PATCH 0/6] UART slave devices using serio Rob Herring
                   ` (4 preceding siblings ...)
  2016-08-24 23:24 ` [RFC PATCH 4/6] serio: serport: add support for buffered write and receive Rob Herring
@ 2016-08-24 23:24 ` Rob Herring
  2016-08-24 23:24 ` [RFC PATCH 6/6] bluetooth: hack up ldisc to use serio Rob Herring
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 20+ messages in thread
From: Rob Herring @ 2016-08-24 23:24 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Marcel Holtmann, Jiri Slaby,
	Sebastian Reichel, Arnd Bergmann, Dr . H . Nikolaus Schaller,
	Alan Cox
  Cc: Loic Poulain, Pavel Machek, Peter Hurley, NeilBrown,
	Linus Walleij, linux-bluetooth, linux-serial, linux-kernel

Just stub functions ATM.

Signed-off-by: Rob Herring <robh@kernel.org>
---
 include/linux/serio.h | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/include/linux/serio.h b/include/linux/serio.h
index 5d0b69f..5bf1754 100644
--- a/include/linux/serio.h
+++ b/include/linux/serio.h
@@ -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.
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [RFC PATCH 6/6] bluetooth: hack up ldisc to use serio
  2016-08-24 23:24 [RFC PATCH 0/6] UART slave devices using serio Rob Herring
                   ` (5 preceding siblings ...)
  2016-08-24 23:24 ` [RFC PATCH 5/6] serio: add serial configuration functions Rob Herring
@ 2016-08-24 23:24 ` Rob Herring
  2016-08-26 20:05 ` [RFC PATCH 0/6] UART slave devices using serio Pavel Machek
  2016-10-25 21:55 ` Sebastian Reichel
  8 siblings, 0 replies; 20+ messages in thread
From: Rob Herring @ 2016-08-24 23:24 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Marcel Holtmann, Jiri Slaby,
	Sebastian Reichel, Arnd Bergmann, Dr . H . Nikolaus Schaller,
	Alan Cox
  Cc: Loic Poulain, Pavel Machek, Peter Hurley, NeilBrown,
	Linus Walleij, linux-bluetooth, linux-serial, linux-kernel

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 <robh@kernel.org>
---
 drivers/bluetooth/hci_ldisc.c | 261 +++++++++++++++++-------------------------
 drivers/bluetooth/hci_uart.h  |   3 +
 2 files changed, 109 insertions(+), 155 deletions(-)

diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index dda9739..8149952 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -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 <marcel@holtmann.org>");
 MODULE_DESCRIPTION("Bluetooth HCI UART driver ver " VERSION);
 MODULE_VERSION(VERSION);
 MODULE_LICENSE("GPL");
-MODULE_ALIAS_LDISC(N_HCI);
diff --git a/drivers/bluetooth/hci_uart.h b/drivers/bluetooth/hci_uart.h
index 839bad1..c48dddc 100644
--- a/drivers/bluetooth/hci_uart.h
+++ b/drivers/bluetooth/hci_uart.h
@@ -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;
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 0/6] UART slave devices using serio
  2016-08-24 23:24 [RFC PATCH 0/6] UART slave devices using serio Rob Herring
                   ` (6 preceding siblings ...)
  2016-08-24 23:24 ` [RFC PATCH 6/6] bluetooth: hack up ldisc to use serio Rob Herring
@ 2016-08-26 20:05 ` Pavel Machek
  2016-08-26 21:29   ` Rob Herring
  2016-10-25 21:55 ` Sebastian Reichel
  8 siblings, 1 reply; 20+ messages in thread
From: Pavel Machek @ 2016-08-26 20:05 UTC (permalink / raw)
  To: Rob Herring
  Cc: Greg Kroah-Hartman, Marcel Holtmann, Jiri Slaby,
	Sebastian Reichel, Arnd Bergmann, Dr . H . Nikolaus Schaller,
	Alan Cox, Loic Poulain, Peter Hurley, NeilBrown, Linus Walleij,
	linux-bluetooth, linux-serial, linux-kernel

On Wed 2016-08-24 18:24:30, Rob Herring wrote:
> 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.

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

There's one saying "add OF driver binding" on the mailinglist, too...
Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 4/6] serio: serport: add support for buffered write and receive
  2016-08-24 23:24 ` [RFC PATCH 4/6] serio: serport: add support for buffered write and receive Rob Herring
@ 2016-08-26 20:12   ` Pavel Machek
  2016-08-26 21:27     ` Rob Herring
  0 siblings, 1 reply; 20+ messages in thread
From: Pavel Machek @ 2016-08-26 20:12 UTC (permalink / raw)
  To: Rob Herring
  Cc: Greg Kroah-Hartman, Marcel Holtmann, Jiri Slaby,
	Sebastian Reichel, Arnd Bergmann, Dr . H . Nikolaus Schaller,
	Alan Cox, Loic Poulain, Peter Hurley, NeilBrown, Linus Walleij,
	linux-bluetooth, linux-serial, linux-kernel

Hi!

> @@ -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);

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
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 4/6] serio: serport: add support for buffered write and receive
  2016-08-26 20:12   ` Pavel Machek
@ 2016-08-26 21:27     ` Rob Herring
  2016-08-26 22:24       ` Pavel Machek
  0 siblings, 1 reply; 20+ messages in thread
From: Rob Herring @ 2016-08-26 21:27 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Greg Kroah-Hartman, Marcel Holtmann, Jiri Slaby,
	Sebastian Reichel, Arnd Bergmann, Dr . H . Nikolaus Schaller,
	Alan Cox, Loic Poulain, Peter Hurley, NeilBrown, Linus Walleij,
	open list:BLUETOOTH DRIVERS, linux-serial, linux-kernel

On Fri, Aug 26, 2016 at 3:12 PM, Pavel Machek <pavel@ucw.cz> wrote:
> Hi!
>
>> @@ -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);
>
> 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?

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 0/6] UART slave devices using serio
  2016-08-26 20:05 ` [RFC PATCH 0/6] UART slave devices using serio Pavel Machek
@ 2016-08-26 21:29   ` Rob Herring
  0 siblings, 0 replies; 20+ messages in thread
From: Rob Herring @ 2016-08-26 21:29 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Greg Kroah-Hartman, Marcel Holtmann, Jiri Slaby,
	Sebastian Reichel, Arnd Bergmann, Dr . H . Nikolaus Schaller,
	Alan Cox, Loic Poulain, Peter Hurley, NeilBrown, Linus Walleij,
	open list:BLUETOOTH DRIVERS, linux-serial, linux-kernel

On Fri, Aug 26, 2016 at 3:05 PM, Pavel Machek <pavel@ucw.cz> wrote:
> On Wed 2016-08-24 18:24:30, Rob Herring wrote:
>> 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.
>
>> 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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 4/6] serio: serport: add support for buffered write and receive
  2016-08-26 21:27     ` Rob Herring
@ 2016-08-26 22:24       ` Pavel Machek
  0 siblings, 0 replies; 20+ messages in thread
From: Pavel Machek @ 2016-08-26 22:24 UTC (permalink / raw)
  To: Rob Herring
  Cc: Greg Kroah-Hartman, Marcel Holtmann, Jiri Slaby,
	Sebastian Reichel, Arnd Bergmann, Dr . H . Nikolaus Schaller,
	Alan Cox, Loic Poulain, Peter Hurley, NeilBrown, Linus Walleij,
	open list:BLUETOOTH DRIVERS, linux-serial, linux-kernel

On Fri 2016-08-26 16:27:35, Rob Herring wrote:
> On Fri, Aug 26, 2016 at 3:12 PM, Pavel Machek <pavel@ucw.cz> wrote:
> > Hi!
> >
> >> @@ -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);
> >
> > 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?
> 
> 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

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 1/6] serio: add DT driver binding
  2016-08-24 23:24 ` [RFC PATCH 1/6] serio: add DT driver binding Rob Herring
@ 2016-08-29  9:57   ` Pavel Machek
  0 siblings, 0 replies; 20+ messages in thread
From: Pavel Machek @ 2016-08-29  9:57 UTC (permalink / raw)
  To: Rob Herring, vojtech
  Cc: Greg Kroah-Hartman, Marcel Holtmann, Jiri Slaby,
	Sebastian Reichel, Arnd Bergmann, Dr . H . Nikolaus Schaller,
	Alan Cox, Loic Poulain, Peter Hurley, NeilBrown, Linus Walleij,
	linux-bluetooth, linux-serial, linux-kernel

On Wed 2016-08-24 18:24:31, Rob Herring wrote:
> 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 <robh@kernel.org>

Acked-by: Pavel Machek <pavel@ucw.cz>

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

> diff --git a/drivers/input/serio/serio.c b/drivers/input/serio/serio.c
> index 1ca7f55..9e8eb7a 100644
> --- a/drivers/input/serio/serio.c
> +++ b/drivers/input/serio/serio.c
> @@ -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 <vojtech@ucw.cz>");
>  MODULE_DESCRIPTION("Serio abstraction core");
> @@ -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
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 0/6] UART slave devices using serio
  2016-08-24 23:24 [RFC PATCH 0/6] UART slave devices using serio Rob Herring
                   ` (7 preceding siblings ...)
  2016-08-26 20:05 ` [RFC PATCH 0/6] UART slave devices using serio Pavel Machek
@ 2016-10-25 21:55 ` Sebastian Reichel
  2016-10-25 22:02   ` Rob Herring
  8 siblings, 1 reply; 20+ messages in thread
From: Sebastian Reichel @ 2016-10-25 21:55 UTC (permalink / raw)
  To: Rob Herring
  Cc: Greg Kroah-Hartman, Marcel Holtmann, Jiri Slaby, Arnd Bergmann,
	Dr . H . Nikolaus Schaller, Alan Cox, Loic Poulain, Pavel Machek,
	Peter Hurley, NeilBrown, Linus Walleij, linux-bluetooth,
	linux-serial, linux-kernel

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

Hi,

On Wed, Aug 24, 2016 at 06:24:30PM -0500, Rob Herring wrote:
> 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.

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 <sre@kernel.org>

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 0/6] UART slave devices using serio
  2016-10-25 21:55 ` Sebastian Reichel
@ 2016-10-25 22:02   ` Rob Herring
  2016-10-26  2:51     ` Sebastian Reichel
       [not found]     ` <CAKU3ayV7vLSaNJGzV2MoCyT2GopReS55DCH4MPtSmGiHEUKdXw@mail.gmail.com>
  0 siblings, 2 replies; 20+ messages in thread
From: Rob Herring @ 2016-10-25 22:02 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: Greg Kroah-Hartman, Marcel Holtmann, Jiri Slaby, Arnd Bergmann,
	Dr . H . Nikolaus Schaller, Alan Cox, Loic Poulain, Pavel Machek,
	Peter Hurley, NeilBrown, Linus Walleij,
	open list:BLUETOOTH DRIVERS, linux-serial, linux-kernel

On Tue, Oct 25, 2016 at 4:55 PM, Sebastian Reichel <sre@kernel.org> wrote:
> Hi,
>
> On Wed, Aug 24, 2016 at 06:24:30PM -0500, Rob Herring wrote:
>> 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.
>
> 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.

>  * 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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 0/6] UART slave devices using serio
  2016-10-25 22:02   ` Rob Herring
@ 2016-10-26  2:51     ` Sebastian Reichel
       [not found]     ` <CAKU3ayV7vLSaNJGzV2MoCyT2GopReS55DCH4MPtSmGiHEUKdXw@mail.gmail.com>
  1 sibling, 0 replies; 20+ messages in thread
From: Sebastian Reichel @ 2016-10-26  2:51 UTC (permalink / raw)
  To: Rob Herring
  Cc: Greg Kroah-Hartman, Marcel Holtmann, Jiri Slaby, Arnd Bergmann,
	Dr . H . Nikolaus Schaller, Alan Cox, Loic Poulain, Pavel Machek,
	Peter Hurley, NeilBrown, Linus Walleij,
	open list:BLUETOOTH DRIVERS, linux-serial, linux-kernel

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

Hi,

On Tue, Oct 25, 2016 at 05:02:23PM -0500, Rob Herring wrote:
> On Tue, Oct 25, 2016 at 4:55 PM, Sebastian Reichel wrote:
> > On Wed, Aug 24, 2016 at 06:24:30PM -0500, Rob Herring wrote:
> >> [...]
> > 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.

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

Ok, I will have a look at that.

> >  * 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 0/6] UART slave devices using serio
       [not found]     ` <CAKU3ayV7vLSaNJGzV2MoCyT2GopReS55DCH4MPtSmGiHEUKdXw@mail.gmail.com>
@ 2016-10-31 20:08       ` Peter Hurley
  2016-11-01  3:40       ` Rob Herring
  1 sibling, 0 replies; 20+ messages in thread
From: Peter Hurley @ 2016-10-31 20:08 UTC (permalink / raw)
  To: Rob Herring
  Cc: Sebastian Reichel, Greg Kroah-Hartman, Marcel Holtmann,
	Jiri Slaby, Arnd Bergmann, Dr . H . Nikolaus Schaller, Alan Cox,
	Loic Poulain, Pavel Machek, NeilBrown, Linus Walleij,
	open list:BLUETOOTH DRIVERS, linux-serial, linux-kernel

[ this time w/o the html ]

On Tue, Oct 25, 2016 at 4:02 PM, Rob Herring <robh@kernel.org> wrote:
> > 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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 0/6] UART slave devices using serio
       [not found]     ` <CAKU3ayV7vLSaNJGzV2MoCyT2GopReS55DCH4MPtSmGiHEUKdXw@mail.gmail.com>
  2016-10-31 20:08       ` Peter Hurley
@ 2016-11-01  3:40       ` Rob Herring
  2016-11-02  3:49         ` Rob Herring
  1 sibling, 1 reply; 20+ messages in thread
From: Rob Herring @ 2016-11-01  3:40 UTC (permalink / raw)
  To: Peter Hurley
  Cc: Sebastian Reichel, Greg Kroah-Hartman, Marcel Holtmann,
	Jiri Slaby, Arnd Bergmann, Dr . H . Nikolaus Schaller, Alan Cox,
	Loic Poulain, Pavel Machek, NeilBrown, Linus Walleij,
	open list:BLUETOOTH DRIVERS, linux-serial, linux-kernel

On Mon, Oct 31, 2016 at 3:00 PM, Peter Hurley <peter@hurleysoftware.com> wrote:
> On Tue, Oct 25, 2016 at 4:02 PM, Rob Herring <robh@kernel.org> wrote:
>>
>> > 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.

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

Rob

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 0/6] UART slave devices using serio
  2016-11-01  3:40       ` Rob Herring
@ 2016-11-02  3:49         ` Rob Herring
  0 siblings, 0 replies; 20+ messages in thread
From: Rob Herring @ 2016-11-02  3:49 UTC (permalink / raw)
  To: Peter Hurley, Andy Shevchenko, Arnd Bergmann
  Cc: Sebastian Reichel, Greg Kroah-Hartman, Marcel Holtmann,
	Jiri Slaby, Dr . H . Nikolaus Schaller, Alan Cox, Loic Poulain,
	Pavel Machek, NeilBrown, Linus Walleij,
	open list:BLUETOOTH DRIVERS, linux-serial, linux-kernel

On Mon, Oct 31, 2016 at 10:40 PM, Rob Herring <robh@kernel.org> wrote:
> On Mon, Oct 31, 2016 at 3:00 PM, Peter Hurley <peter@hurleysoftware.com> wrote:
>> On Tue, Oct 25, 2016 at 4:02 PM, Rob Herring <robh@kernel.org> wrote:
>>>
>>> > 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.
>
> 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

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2016-11-02  3:49 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-24 23:24 [RFC PATCH 0/6] UART slave devices using serio Rob Herring
2016-08-24 23:24 ` [RFC PATCH 1/6] serio: add DT driver binding Rob Herring
2016-08-29  9:57   ` Pavel Machek
2016-08-24 23:24 ` [RFC PATCH 1/6] serio: add OF " Rob Herring
2016-08-24 23:24 ` [RFC PATCH 2/6] serio: serport: hacks to get DT probe to work Rob Herring
2016-08-24 23:24 ` [RFC PATCH 3/6] serio: add buffer receive and write functions Rob Herring
2016-08-24 23:24 ` [RFC PATCH 4/6] serio: serport: add support for buffered write and receive Rob Herring
2016-08-26 20:12   ` Pavel Machek
2016-08-26 21:27     ` Rob Herring
2016-08-26 22:24       ` Pavel Machek
2016-08-24 23:24 ` [RFC PATCH 5/6] serio: add serial configuration functions Rob Herring
2016-08-24 23:24 ` [RFC PATCH 6/6] bluetooth: hack up ldisc to use serio Rob Herring
2016-08-26 20:05 ` [RFC PATCH 0/6] UART slave devices using serio Pavel Machek
2016-08-26 21:29   ` Rob Herring
2016-10-25 21:55 ` Sebastian Reichel
2016-10-25 22:02   ` Rob Herring
2016-10-26  2:51     ` Sebastian Reichel
     [not found]     ` <CAKU3ayV7vLSaNJGzV2MoCyT2GopReS55DCH4MPtSmGiHEUKdXw@mail.gmail.com>
2016-10-31 20:08       ` Peter Hurley
2016-11-01  3:40       ` Rob Herring
2016-11-02  3:49         ` Rob Herring

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).