[patch 0/7] staging: speakup: introduce tty-based comms

[patch 0/7] staging: speakup: introduce tty-based comms

[okash.khawaja]

Hi,

This patchset introduces a TTY-based way for the synths to communicate
with devices as an alternate for direct serial comms used by the synths
at the moment. It then migrates some of the synths to the TTY-based
comms. Synths migrated in this patchset are dummy, acntsa, bns and
txprt.

As such, this series is meant to start a discussion on the changes in the
first patch: "tty_port: allow a port to be opened with a tty that has no
file handle". This allows speakup to access tty port early on during boot,
making it possible to start emiting messages as early as possible. This is
demonstrated in patch 6, where it calls tty_open_by_driver, passing in NULL
for inode and filp params.

To wrap up, here is a summary of the patches that follow. Patch 1 enables
kernel access to TTY device. Patches 2 - 5 prepare the code to accomodate
TTY-based comms as an alternate to raw serial I/O for outputting data to
external device. Patch 6 builds upon the changes made in patch 1 and utilises
TTY-subsystem to communicate with external device - currently just over ttyS*
but with plans to expand it to other TTY devices soon. Finally, patch 7 does
actual migration of some of the simple synths from raw serial comms to TTY.

Okash

[patch 1/7] tty_port: allow a port to be opened with a tty that has no file handle

[okash.khawaja]

[-- Attachment #1: 01_enable_kernel_access_to_tty_device --]
[-- Type: text/plain, Size: 4317 bytes --]

Allow access to TTY device from kernel. This is based on Alan Cox's patch
(http://www.mail-archive.com/linux-kernel at vger.kernel.org/msg1215095.html),
with description quoted below.

"tty_port: allow a port to be opened with a tty that has no file handle

Let us create tty objects entirely in kernel space.

With this a kernel created non file backed tty object could be used to handle
data, and set terminal modes. Not all ldiscs can cope with this as N_TTY in
particular has to work back to the fs/tty layer.

The tty_port code is however otherwise clean of file handles as far as I can
tell as is the low level tty port write path used by the ldisc, the
configuration low level interfaces and most of the ldiscs.

Currently you don't have any exposure to see tty hangups because those are
built around the file layer. However a) it's a fixed port so you probably
don't care about that b) if you do we can add a callback and c) you almost
certainly don't want the userspace tear down/rebuild behaviour anyway.

This should however be sufficient if we wanted for example to enumerate all
the bluetooth bound fixed ports via ACPI and make them directly available.

It doesn't deal with the case of a user opening a port that's also kernel
opened and that would need some locking out (so it returned EBUSY if bound
to a kernel device of some kind). That needs resolving along with how you
"up" or "down" your new bluetooth device, or enumerate it while providing
the existing tty API to avoid regressions (and to debug)."

Signed-off-by: Okash Khawaja <okash.khawaja@gmail.com>

Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

Index: linux-4.10.1/drivers/tty/tty_io.c
===================================================================
--- linux-4.10.1.orig/drivers/tty/tty_io.c
+++ linux-4.10.1/drivers/tty/tty_io.c
@@ -855,7 +855,7 @@
 
 int tty_hung_up_p(struct file *filp)
 {
-	return (filp->f_op == &hung_up_tty_fops);
+	return (filp && filp->f_op == &hung_up_tty_fops);
 }
 
 EXPORT_SYMBOL(tty_hung_up_p);
@@ -1368,7 +1368,10 @@
 	struct tty_struct *tty;
 
 	if (driver->ops->lookup)
-		tty = driver->ops->lookup(driver, file, idx);
+		if (!file)
+			tty = ERR_PTR(-EIO);
+		else
+			tty = driver->ops->lookup(driver, file, idx);
 	else
 		tty = driver->ttys[idx];
 
@@ -1986,7 +1989,7 @@
 		struct tty_driver *console_driver = console_device(index);
 		if (console_driver) {
 			driver = tty_driver_kref_get(console_driver);
-			if (driver) {
+			if (driver && filp) {
 				/* Don't let /dev/console block */
 				filp->f_flags |= O_NONBLOCK;
 				break;
@@ -2019,7 +2022,7 @@
  *	  - concurrent tty driver removal w/ lookup
  *	  - concurrent tty removal from driver table
  */
-static struct tty_struct *tty_open_by_driver(dev_t device, struct inode *inode,
+struct tty_struct *tty_open_by_driver(dev_t device, struct inode *inode,
 					     struct file *filp)
 {
 	struct tty_struct *tty;
@@ -2064,6 +2067,7 @@
 	tty_driver_kref_put(driver);
 	return tty;
 }
+EXPORT_SYMBOL(tty_open_by_driver);
 
 /**
  *	tty_open		-	open a tty device
Index: linux-4.10.1/drivers/tty/tty_port.c
===================================================================
--- linux-4.10.1.orig/drivers/tty/tty_port.c
+++ linux-4.10.1/drivers/tty/tty_port.c
@@ -335,7 +335,7 @@
  *	tty_port_block_til_ready	-	Waiting logic for tty open
  *	@port: the tty port being opened
  *	@tty: the tty device being bound
- *	@filp: the file pointer of the opener
+ *	@filp: the file pointer of the opener or NULL
  *
  *	Implement the core POSIX/SuS tty behaviour when opening a tty device.
  *	Handles:
@@ -369,7 +369,7 @@
 		tty_port_set_active(port, 1);
 		return 0;
 	}
-	if (filp->f_flags & O_NONBLOCK) {
+	if (filp ==  NULL || filp->f_flags & O_NONBLOCK) {
 		/* Indicate we are open */
 		if (C_BAUD(tty))
 			tty_port_raise_dtr_rts(port);
Index: linux-4.10.1/include/linux/tty.h
===================================================================
--- linux-4.10.1.orig/include/linux/tty.h
+++ linux-4.10.1/include/linux/tty.h
@@ -394,6 +394,8 @@
 /* tty_io.c */
 extern int __init tty_init(void);
 extern const char *tty_name(const struct tty_struct *tty);
+extern struct tty_struct *tty_open_by_driver(dev_t device, struct inode *inode,
+					     struct file *filp);
 #else
 static inline void console_init(void)
 { }

[patch 2/7] staging: speakup: spk_serial_out and spk_wait_for_xmitr to take synth arg

[okash.khawaja]

[-- Attachment #1: 02_spk_serial_out_and_xmitr_to_take_synth --]
[-- Type: text/plain, Size: 8461 bytes --]

These two functions are always called from a context where spk_synth instance
is available. They also use the spk_synth instance but instead of taking it
as an argument, they rely on a global spk_synth instance inside synth.c which
points to the same synth as the one being passed in as argument.

Signed-off-by: Okash Khawaja <okash.khawaja@gmail.com>

Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

Index: linux-4.10.1/drivers/staging/speakup/serialio.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/serialio.c
+++ linux-4.10.1/drivers/staging/speakup/serialio.c
@@ -144,14 +144,14 @@
 	free_irq(serstate->irq, (void *)synth_readbuf_handler);
 }
 
-int spk_wait_for_xmitr(void)
+int spk_wait_for_xmitr(struct spk_synth *in_synth)
 {
 	int tmout = SPK_XMITR_TIMEOUT;
 
-	if ((synth->alive) && (timeouts >= NUM_DISABLE_TIMEOUTS)) {
+	if ((in_synth->alive) && (timeouts >= NUM_DISABLE_TIMEOUTS)) {
 		pr_warn("%s: too many timeouts, deactivating speakup\n",
-			synth->long_name);
-		synth->alive = 0;
+			in_synth->long_name);
+		in_synth->alive = 0;
 		/* No synth any more, so nobody will restart TTYs, and we thus
 		 * need to do it ourselves.  Now that there is no synth we can
 		 * let application flood anyway
@@ -162,7 +162,7 @@
 	}
 	while (spk_serial_tx_busy()) {
 		if (--tmout == 0) {
-			pr_warn("%s: timed out (tx busy)\n", synth->long_name);
+			pr_warn("%s: timed out (tx busy)\n", in_synth->long_name);
 			timeouts++;
 			return 0;
 		}
@@ -207,9 +207,9 @@
 }
 EXPORT_SYMBOL_GPL(spk_serial_in_nowait);
 
-int spk_serial_out(const char ch)
+int spk_serial_out(struct spk_synth *in_synth, const char ch)
 {
-	if (synth->alive && spk_wait_for_xmitr()) {
+	if (in_synth->alive && spk_wait_for_xmitr(in_synth)) {
 		outb_p(ch, speakup_info.port_tts);
 		return 1;
 	}
Index: linux-4.10.1/drivers/staging/speakup/speakup_apollo.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_apollo.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_apollo.c
@@ -169,7 +169,7 @@
 		set_current_state(TASK_INTERRUPTIBLE);
 		full_time_val = full_time->u.n.value;
 		spin_unlock_irqrestore(&speakup_info.spinlock, flags);
-		if (!spk_serial_out(ch)) {
+		if (!spk_serial_out(synth, ch)) {
 			outb(UART_MCR_DTR, speakup_info.port_tts + UART_MCR);
 			outb(UART_MCR_DTR | UART_MCR_RTS,
 					speakup_info.port_tts + UART_MCR);
@@ -182,7 +182,7 @@
 			full_time_val = full_time->u.n.value;
 			delay_time_val = delay_time->u.n.value;
 			spin_unlock_irqrestore(&speakup_info.spinlock, flags);
-			if (spk_serial_out(synth->procspeech))
+			if (spk_serial_out(synth, synth->procspeech))
 				schedule_timeout(msecs_to_jiffies
 						 (delay_time_val));
 			else
@@ -195,7 +195,7 @@
 		synth_buffer_getc();
 		spin_unlock_irqrestore(&speakup_info.spinlock, flags);
 	}
-	spk_serial_out(PROCSPEECH);
+	spk_serial_out(synth, PROCSPEECH);
 }
 
 module_param_named(ser, synth_apollo.ser, int, S_IRUGO);
Index: linux-4.10.1/drivers/staging/speakup/speakup_audptr.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_audptr.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_audptr.c
@@ -135,7 +135,7 @@
 		udelay(1);
 	}
 	outb(SYNTH_CLEAR, speakup_info.port_tts);
-	spk_serial_out(PROCSPEECH);
+	spk_serial_out(synth, PROCSPEECH);
 }
 
 static void synth_version(struct spk_synth *synth)
Index: linux-4.10.1/drivers/staging/speakup/speakup_decext.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_decext.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_decext.c
@@ -186,7 +186,7 @@
 		spin_unlock_irqrestore(&speakup_info.spinlock, flags);
 		if (ch == '\n')
 			ch = 0x0D;
-		if (synth_full() || !spk_serial_out(ch)) {
+		if (synth_full() || !spk_serial_out(synth, ch)) {
 			schedule_timeout(msecs_to_jiffies(delay_time_val));
 			continue;
 		}
@@ -200,10 +200,10 @@
 			in_escape = 0;
 		else if (ch <= SPACE) {
 			if (!in_escape && strchr(",.!?;:", last))
-				spk_serial_out(PROCSPEECH);
+				spk_serial_out(synth, PROCSPEECH);
 			if (time_after_eq(jiffies, jiff_max)) {
 				if (!in_escape)
-					spk_serial_out(PROCSPEECH);
+					spk_serial_out(synth, PROCSPEECH);
 				spin_lock_irqsave(&speakup_info.spinlock,
 							flags);
 				jiffy_delta_val = jiffy_delta->u.n.value;
@@ -218,7 +218,7 @@
 		last = ch;
 	}
 	if (!in_escape)
-		spk_serial_out(PROCSPEECH);
+		spk_serial_out(synth, PROCSPEECH);
 }
 
 static void synth_flush(struct spk_synth *synth)
Index: linux-4.10.1/drivers/staging/speakup/speakup_dectlk.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_dectlk.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_dectlk.c
@@ -251,7 +251,7 @@
 		spin_unlock_irqrestore(&speakup_info.spinlock, flags);
 		if (ch == '\n')
 			ch = 0x0D;
-		if (synth_full_val || !spk_serial_out(ch)) {
+		if (synth_full_val || !spk_serial_out(synth, ch)) {
 			schedule_timeout(msecs_to_jiffies(delay_time_val));
 			continue;
 		}
@@ -265,10 +265,10 @@
 			in_escape = 0;
 		else if (ch <= SPACE) {
 			if (!in_escape && strchr(",.!?;:", last))
-				spk_serial_out(PROCSPEECH);
+				spk_serial_out(synth, PROCSPEECH);
 			if (time_after_eq(jiffies, jiff_max)) {
 				if (!in_escape)
-					spk_serial_out(PROCSPEECH);
+					spk_serial_out(synth, PROCSPEECH);
 				spin_lock_irqsave(&speakup_info.spinlock,
 						flags);
 				jiffy_delta_val = jiffy_delta->u.n.value;
@@ -283,17 +283,17 @@
 		last = ch;
 	}
 	if (!in_escape)
-		spk_serial_out(PROCSPEECH);
+		spk_serial_out(synth, PROCSPEECH);
 }
 
 static void synth_flush(struct spk_synth *synth)
 {
 	if (in_escape)
 		/* if in command output ']' so we don't get an error */
-		spk_serial_out(']');
+		spk_serial_out(synth, ']');
 	in_escape = 0;
 	is_flushing = 1;
-	spk_serial_out(SYNTH_CLEAR);
+	spk_serial_out(synth, SYNTH_CLEAR);
 }
 
 module_param_named(ser, synth_dectlk.ser, int, S_IRUGO);
Index: linux-4.10.1/drivers/staging/speakup/spk_priv.h
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/spk_priv.h
+++ linux-4.10.1/drivers/staging/speakup/spk_priv.h
@@ -42,10 +42,10 @@
 
 const struct old_serial_port *spk_serial_init(int index);
 void spk_stop_serial_interrupt(void);
-int spk_wait_for_xmitr(void);
+int spk_wait_for_xmitr(struct spk_synth *in_synth);
 unsigned char spk_serial_in(void);
 unsigned char spk_serial_in_nowait(void);
-int spk_serial_out(const char ch);
+int spk_serial_out(struct spk_synth *in_synth, const char ch);
 void spk_serial_release(void);
 
 void synth_buffer_skip_nonlatin1(void);
Index: linux-4.10.1/drivers/staging/speakup/synth.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/synth.c
+++ linux-4.10.1/drivers/staging/speakup/synth.c
@@ -120,7 +120,7 @@
 		spin_unlock_irqrestore(&speakup_info.spinlock, flags);
 		if (ch == '\n')
 			ch = synth->procspeech;
-		if (!spk_serial_out(ch)) {
+		if (!spk_serial_out(synth, ch)) {
 			schedule_timeout(msecs_to_jiffies(full_time_val));
 			continue;
 		}
@@ -130,7 +130,7 @@
 			delay_time_val = delay_time->u.n.value;
 			full_time_val = full_time->u.n.value;
 			spin_unlock_irqrestore(&speakup_info.spinlock, flags);
-			if (spk_serial_out(synth->procspeech))
+			if (spk_serial_out(synth, synth->procspeech))
 				schedule_timeout(
 					msecs_to_jiffies(delay_time_val));
 			else
@@ -143,7 +143,7 @@
 		synth_buffer_getc();
 		spin_unlock_irqrestore(&speakup_info.spinlock, flags);
 	}
-	spk_serial_out(synth->procspeech);
+	spk_serial_out(synth, synth->procspeech);
 }
 EXPORT_SYMBOL_GPL(spk_do_catch_up);
 
@@ -154,7 +154,7 @@
 	while ((ch = *buff)) {
 		if (ch == '\n')
 			ch = synth->procspeech;
-		if (spk_wait_for_xmitr())
+		if (spk_wait_for_xmitr(synth))
 			outb(ch, speakup_info.port_tts);
 		else
 			return buff;
@@ -166,7 +166,7 @@
 
 void spk_synth_flush(struct spk_synth *synth)
 {
-	spk_serial_out(synth->clear);
+	spk_serial_out(synth, synth->clear);
 }
 EXPORT_SYMBOL_GPL(spk_synth_flush);
 
@@ -181,7 +181,7 @@
 {
 	if (synth->alive)
 		return 1;
-	if (spk_wait_for_xmitr() > 0) {
+	if (spk_wait_for_xmitr(synth) > 0) {
 		/* restart */
 		synth->alive = 1;
 		synth_printf("%s", synth->init);

[patch 3/7] staging: serial: add spk_io_ops struct to spk_synth

[okash.khawaja]

[-- Attachment #1: 03_add_spk_io_ops --]
[-- Type: text/plain, Size: 14832 bytes --]

This patch adds spk_io_ops struct which contain those methods whose job is to
communicate with synth device. Currently, all comms with external synth
device use raw serial i/o. Starting with this patch set, an alternative
tty-based way of communication is being introduced. The idea is to group all
methods which do the actual communication with external device into this new
struct. Then migrating a serial-based synth over to tty will mean swapping
serial spk_io_ops instance with tty spk_io_ops instance, making the migration
simpler.

At the moment, this struct only contains one method, synth_out but more will
be added in future when incorporating inputs in the tty-based comms. Also at
the moment, synth_out method has one implementation which uses serial i/o.
Later in this patch set, an alternate tty-based implementation will be added.

Signed-off-by: Okash Khawaja <okash.khawaja@gmail.com>

Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

Index: linux-4.10.1/drivers/staging/speakup/serialio.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/serialio.c
+++ linux-4.10.1/drivers/staging/speakup/serialio.c
@@ -24,6 +24,12 @@
 static const struct old_serial_port *serstate;
 static int timeouts;
 
+static int spk_serial_out(struct spk_synth *in_synth, const char ch);
+struct spk_io_ops spk_serial_io_ops = {
+	.synth_out = spk_serial_out,
+};
+EXPORT_SYMBOL_GPL(spk_serial_io_ops);
+
 const struct old_serial_port *spk_serial_init(int index)
 {
 	int baud = 9600, quot = 0;
@@ -215,7 +221,6 @@
 	}
 	return 0;
 }
-EXPORT_SYMBOL_GPL(spk_serial_out);
 
 void spk_serial_release(void)
 {
Index: linux-4.10.1/drivers/staging/speakup/speakup_acntpc.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_acntpc.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_acntpc.c
@@ -113,6 +113,7 @@
 	.startup = SYNTH_START,
 	.checkval = SYNTH_CHECK,
 	.vars = vars,
+	.io_ops = &spk_serial_io_ops,
 	.probe = synth_probe,
 	.release = accent_release,
 	.synth_immediate = synth_immediate,
Index: linux-4.10.1/drivers/staging/speakup/speakup_acntsa.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_acntsa.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_acntsa.c
@@ -99,6 +99,7 @@
 	.startup = SYNTH_START,
 	.checkval = SYNTH_CHECK,
 	.vars = vars,
+	.io_ops = &spk_serial_io_ops,
 	.probe = synth_probe,
 	.release = spk_serial_release,
 	.synth_immediate = spk_synth_immediate,
Index: linux-4.10.1/drivers/staging/speakup/speakup_apollo.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_apollo.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_apollo.c
@@ -108,6 +108,7 @@
 	.startup = SYNTH_START,
 	.checkval = SYNTH_CHECK,
 	.vars = vars,
+	.io_ops = &spk_serial_io_ops,
 	.probe = spk_serial_synth_probe,
 	.release = spk_serial_release,
 	.synth_immediate = spk_synth_immediate,
@@ -169,7 +170,7 @@
 		set_current_state(TASK_INTERRUPTIBLE);
 		full_time_val = full_time->u.n.value;
 		spin_unlock_irqrestore(&speakup_info.spinlock, flags);
-		if (!spk_serial_out(synth, ch)) {
+		if (!synth->io_ops->synth_out(synth, ch)) {
 			outb(UART_MCR_DTR, speakup_info.port_tts + UART_MCR);
 			outb(UART_MCR_DTR | UART_MCR_RTS,
 					speakup_info.port_tts + UART_MCR);
@@ -182,7 +183,7 @@
 			full_time_val = full_time->u.n.value;
 			delay_time_val = delay_time->u.n.value;
 			spin_unlock_irqrestore(&speakup_info.spinlock, flags);
-			if (spk_serial_out(synth, synth->procspeech))
+			if (synth->io_ops->synth_out(synth, synth->procspeech))
 				schedule_timeout(msecs_to_jiffies
 						 (delay_time_val));
 			else
@@ -195,7 +196,7 @@
 		synth_buffer_getc();
 		spin_unlock_irqrestore(&speakup_info.spinlock, flags);
 	}
-	spk_serial_out(synth, PROCSPEECH);
+	synth->io_ops->synth_out(synth, PROCSPEECH);
 }
 
 module_param_named(ser, synth_apollo.ser, int, S_IRUGO);
Index: linux-4.10.1/drivers/staging/speakup/speakup_audptr.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_audptr.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_audptr.c
@@ -104,6 +104,7 @@
 	.startup = SYNTH_START,
 	.checkval = SYNTH_CHECK,
 	.vars = vars,
+	.io_ops = &spk_serial_io_ops,
 	.probe = synth_probe,
 	.release = spk_serial_release,
 	.synth_immediate = spk_synth_immediate,
@@ -135,7 +136,7 @@
 		udelay(1);
 	}
 	outb(SYNTH_CLEAR, speakup_info.port_tts);
-	spk_serial_out(synth, PROCSPEECH);
+	synth->io_ops->synth_out(synth, PROCSPEECH);
 }
 
 static void synth_version(struct spk_synth *synth)
Index: linux-4.10.1/drivers/staging/speakup/speakup_decext.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_decext.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_decext.c
@@ -127,6 +127,7 @@
 	.startup = SYNTH_START,
 	.checkval = SYNTH_CHECK,
 	.vars = vars,
+	.io_ops = &spk_serial_io_ops,
 	.probe = spk_serial_synth_probe,
 	.release = spk_serial_release,
 	.synth_immediate = spk_synth_immediate,
@@ -186,7 +187,7 @@
 		spin_unlock_irqrestore(&speakup_info.spinlock, flags);
 		if (ch == '\n')
 			ch = 0x0D;
-		if (synth_full() || !spk_serial_out(synth, ch)) {
+		if (synth_full() || !synth->io_ops->synth_out(synth, ch)) {
 			schedule_timeout(msecs_to_jiffies(delay_time_val));
 			continue;
 		}
@@ -200,10 +201,10 @@
 			in_escape = 0;
 		else if (ch <= SPACE) {
 			if (!in_escape && strchr(",.!?;:", last))
-				spk_serial_out(synth, PROCSPEECH);
+				synth->io_ops->synth_out(synth, PROCSPEECH);
 			if (time_after_eq(jiffies, jiff_max)) {
 				if (!in_escape)
-					spk_serial_out(synth, PROCSPEECH);
+					synth->io_ops->synth_out(synth, PROCSPEECH);
 				spin_lock_irqsave(&speakup_info.spinlock,
 							flags);
 				jiffy_delta_val = jiffy_delta->u.n.value;
@@ -218,7 +219,7 @@
 		last = ch;
 	}
 	if (!in_escape)
-		spk_serial_out(synth, PROCSPEECH);
+		synth->io_ops->synth_out(synth, PROCSPEECH);
 }
 
 static void synth_flush(struct spk_synth *synth)
Index: linux-4.10.1/drivers/staging/speakup/speakup_dectlk.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_dectlk.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_dectlk.c
@@ -130,6 +130,7 @@
 	.vars = vars,
 	.default_pitch = ap_defaults,
 	.default_vol = g5_defaults,
+	.io_ops = &spk_serial_io_ops,
 	.probe = spk_serial_synth_probe,
 	.release = spk_serial_release,
 	.synth_immediate = spk_synth_immediate,
@@ -251,7 +252,7 @@
 		spin_unlock_irqrestore(&speakup_info.spinlock, flags);
 		if (ch == '\n')
 			ch = 0x0D;
-		if (synth_full_val || !spk_serial_out(synth, ch)) {
+		if (synth_full_val || !synth->io_ops->synth_out(synth, ch)) {
 			schedule_timeout(msecs_to_jiffies(delay_time_val));
 			continue;
 		}
@@ -265,10 +266,10 @@
 			in_escape = 0;
 		else if (ch <= SPACE) {
 			if (!in_escape && strchr(",.!?;:", last))
-				spk_serial_out(synth, PROCSPEECH);
+				synth->io_ops->synth_out(synth, PROCSPEECH);
 			if (time_after_eq(jiffies, jiff_max)) {
 				if (!in_escape)
-					spk_serial_out(synth, PROCSPEECH);
+					synth->io_ops->synth_out(synth, PROCSPEECH);
 				spin_lock_irqsave(&speakup_info.spinlock,
 						flags);
 				jiffy_delta_val = jiffy_delta->u.n.value;
@@ -283,17 +284,17 @@
 		last = ch;
 	}
 	if (!in_escape)
-		spk_serial_out(synth, PROCSPEECH);
+		synth->io_ops->synth_out(synth, PROCSPEECH);
 }
 
 static void synth_flush(struct spk_synth *synth)
 {
 	if (in_escape)
 		/* if in command output ']' so we don't get an error */
-		spk_serial_out(synth, ']');
+		synth->io_ops->synth_out(synth, ']');
 	in_escape = 0;
 	is_flushing = 1;
-	spk_serial_out(synth, SYNTH_CLEAR);
+	synth->io_ops->synth_out(synth, SYNTH_CLEAR);
 }
 
 module_param_named(ser, synth_dectlk.ser, int, S_IRUGO);
Index: linux-4.10.1/drivers/staging/speakup/spk_priv.h
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/spk_priv.h
+++ linux-4.10.1/drivers/staging/speakup/spk_priv.h
@@ -45,7 +45,6 @@
 int spk_wait_for_xmitr(struct spk_synth *in_synth);
 unsigned char spk_serial_in(void);
 unsigned char spk_serial_in_nowait(void);
-int spk_serial_out(struct spk_synth *in_synth, const char ch);
 void spk_serial_release(void);
 
 void synth_buffer_skip_nonlatin1(void);
@@ -78,4 +77,6 @@
 
 extern struct var_t synth_time_vars[];
 
+extern struct spk_io_ops spk_serial_io_ops;
+
 #endif
Index: linux-4.10.1/drivers/staging/speakup/spk_types.h
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/spk_types.h
+++ linux-4.10.1/drivers/staging/speakup/spk_types.h
@@ -146,6 +146,12 @@
 	unsigned char currindex;
 };
 
+struct spk_synth;
+
+struct spk_io_ops {
+	int (*synth_out)(struct spk_synth *synth, const char ch);
+};
+
 struct spk_synth {
 	const char *name;
 	const char *version;
@@ -164,6 +170,7 @@
 	struct var_t *vars;
 	int *default_pitch;
 	int *default_vol;
+	struct spk_io_ops *io_ops;
 	int (*probe)(struct spk_synth *synth);
 	void (*release)(void);
 	const char *(*synth_immediate)(struct spk_synth *synth,
Index: linux-4.10.1/drivers/staging/speakup/synth.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/synth.c
+++ linux-4.10.1/drivers/staging/speakup/synth.c
@@ -120,7 +120,7 @@
 		spin_unlock_irqrestore(&speakup_info.spinlock, flags);
 		if (ch == '\n')
 			ch = synth->procspeech;
-		if (!spk_serial_out(synth, ch)) {
+		if (!synth->io_ops->synth_out(synth, ch)) {
 			schedule_timeout(msecs_to_jiffies(full_time_val));
 			continue;
 		}
@@ -130,7 +130,7 @@
 			delay_time_val = delay_time->u.n.value;
 			full_time_val = full_time->u.n.value;
 			spin_unlock_irqrestore(&speakup_info.spinlock, flags);
-			if (spk_serial_out(synth, synth->procspeech))
+			if (synth->io_ops->synth_out(synth, synth->procspeech))
 				schedule_timeout(
 					msecs_to_jiffies(delay_time_val));
 			else
@@ -143,7 +143,7 @@
 		synth_buffer_getc();
 		spin_unlock_irqrestore(&speakup_info.spinlock, flags);
 	}
-	spk_serial_out(synth, synth->procspeech);
+	synth->io_ops->synth_out(synth, synth->procspeech);
 }
 EXPORT_SYMBOL_GPL(spk_do_catch_up);
 
@@ -166,7 +166,7 @@
 
 void spk_synth_flush(struct spk_synth *synth)
 {
-	spk_serial_out(synth, synth->clear);
+	synth->io_ops->synth_out(synth, synth->clear);
 }
 EXPORT_SYMBOL_GPL(spk_synth_flush);
 
Index: linux-4.10.1/drivers/staging/speakup/speakup_bns.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_bns.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_bns.c
@@ -96,6 +96,7 @@
 	.startup = SYNTH_START,
 	.checkval = SYNTH_CHECK,
 	.vars = vars,
+	.io_ops = &spk_serial_io_ops,
 	.probe = spk_serial_synth_probe,
 	.release = spk_serial_release,
 	.synth_immediate = spk_synth_immediate,
Index: linux-4.10.1/drivers/staging/speakup/speakup_decpc.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_decpc.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_decpc.c
@@ -220,6 +220,7 @@
 	.startup = SYNTH_START,
 	.checkval = SYNTH_CHECK,
 	.vars = vars,
+	.io_ops = &spk_serial_io_ops,
 	.probe = synth_probe,
 	.release = dtpc_release,
 	.synth_immediate = synth_immediate,
Index: linux-4.10.1/drivers/staging/speakup/speakup_dtlk.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_dtlk.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_dtlk.c
@@ -128,6 +128,7 @@
 	.startup = SYNTH_START,
 	.checkval = SYNTH_CHECK,
 	.vars = vars,
+	.io_ops = &spk_serial_io_ops,
 	.probe = synth_probe,
 	.release = dtlk_release,
 	.synth_immediate = synth_immediate,
Index: linux-4.10.1/drivers/staging/speakup/speakup_dummy.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_dummy.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_dummy.c
@@ -98,6 +98,7 @@
 	.startup = SYNTH_START,
 	.checkval = SYNTH_CHECK,
 	.vars = vars,
+	.io_ops = &spk_serial_io_ops,
 	.probe = spk_serial_synth_probe,
 	.release = spk_serial_release,
 	.synth_immediate = spk_synth_immediate,
Index: linux-4.10.1/drivers/staging/speakup/speakup_keypc.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_keypc.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_keypc.c
@@ -105,6 +105,7 @@
 	.startup = SYNTH_START,
 	.checkval = SYNTH_CHECK,
 	.vars = vars,
+	.io_ops = &spk_serial_io_ops,
 	.probe = synth_probe,
 	.release = keynote_release,
 	.synth_immediate = synth_immediate,
Index: linux-4.10.1/drivers/staging/speakup/speakup_ltlk.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_ltlk.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_ltlk.c
@@ -111,6 +111,7 @@
 	.startup = SYNTH_START,
 	.checkval = SYNTH_CHECK,
 	.vars = vars,
+	.io_ops = &spk_serial_io_ops,
 	.probe = synth_probe,
 	.release = spk_serial_release,
 	.synth_immediate = spk_synth_immediate,
Index: linux-4.10.1/drivers/staging/speakup/speakup_soft.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_soft.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_soft.c
@@ -131,6 +131,7 @@
 	.startup = SYNTH_START,
 	.checkval = SYNTH_CHECK,
 	.vars = vars,
+	.io_ops = NULL,
 	.probe = softsynth_probe,
 	.release = softsynth_release,
 	.synth_immediate = NULL,
Index: linux-4.10.1/drivers/staging/speakup/speakup_spkout.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_spkout.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_spkout.c
@@ -102,6 +102,7 @@
 	.startup = SYNTH_START,
 	.checkval = SYNTH_CHECK,
 	.vars = vars,
+	.io_ops = &spk_serial_io_ops,
 	.probe = spk_serial_synth_probe,
 	.release = spk_serial_release,
 	.synth_immediate = spk_synth_immediate,
Index: linux-4.10.1/drivers/staging/speakup/speakup_txprt.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_txprt.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_txprt.c
@@ -95,6 +95,7 @@
 	.startup = SYNTH_START,
 	.checkval = SYNTH_CHECK,
 	.vars = vars,
+	.io_ops = &spk_serial_io_ops,
 	.probe = spk_serial_synth_probe,
 	.release = spk_serial_release,
 	.synth_immediate = spk_synth_immediate,

[patch 4/7] staging: speakup: move spk_stop_serial_interrupt into synth-specific release function

[okash.khawaja]

[-- Attachment #1: 04_refactor_spk_stop_serial_interrupt --]
[-- Type: text/plain, Size: 3435 bytes --]

This moves call to spk_stop_serial_interrupt() function out of synth_release()
and into release() method of specific spk_synth instances. This is because
a TTY-based synth implementation wouldn't need spk_stop_serial_interrupt()
call. Moving it into each synth's release() method gives the decision of
calling  spk_stop_serial_interrupt() to that synth. TTY-based synths which
follow in this patchset simply wouldn't call it.

Signed-off-by: Okash Khawaja <okash.khawaja@gmail.com>

Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

Index: linux-4.10.1/drivers/staging/speakup/serialio.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/serialio.c
+++ linux-4.10.1/drivers/staging/speakup/serialio.c
@@ -149,6 +149,7 @@
 	/* Free IRQ */
 	free_irq(serstate->irq, (void *)synth_readbuf_handler);
 }
+EXPORT_SYMBOL_GPL(spk_stop_serial_interrupt);
 
 int spk_wait_for_xmitr(struct spk_synth *in_synth)
 {
@@ -224,6 +225,7 @@
 
 void spk_serial_release(void)
 {
+	spk_stop_serial_interrupt();
 	if (speakup_info.port_tts == 0)
 		return;
 	synth_release_region(speakup_info.port_tts, 8);
Index: linux-4.10.1/drivers/staging/speakup/speakup_acntpc.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_acntpc.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_acntpc.c
@@ -304,6 +304,7 @@
 
 static void accent_release(void)
 {
+	spk_stop_serial_interrupt();
 	if (speakup_info.port_tts)
 		synth_release_region(speakup_info.port_tts-1, SYNTH_IO_EXTENT);
 	speakup_info.port_tts = 0;
Index: linux-4.10.1/drivers/staging/speakup/synth.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/synth.c
+++ linux-4.10.1/drivers/staging/speakup/synth.c
@@ -462,7 +462,6 @@
 		sysfs_remove_group(speakup_kobj, &synth->attributes);
 	for (var = synth->vars; var->var_id != MAXVARS; var++)
 		speakup_unregister_var(var->var_id);
-	spk_stop_serial_interrupt();
 	synth->release();
 	synth = NULL;
 }
Index: linux-4.10.1/drivers/staging/speakup/speakup_decpc.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_decpc.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_decpc.c
@@ -483,6 +483,7 @@
 
 static void dtpc_release(void)
 {
+	spk_stop_serial_interrupt();
 	if (speakup_info.port_tts)
 		synth_release_region(speakup_info.port_tts, SYNTH_IO_EXTENT);
 	speakup_info.port_tts = 0;
Index: linux-4.10.1/drivers/staging/speakup/speakup_dtlk.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_dtlk.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_dtlk.c
@@ -375,6 +375,7 @@
 
 static void dtlk_release(void)
 {
+	spk_stop_serial_interrupt();
 	if (speakup_info.port_tts)
 		synth_release_region(speakup_info.port_tts-1, SYNTH_IO_EXTENT);
 	speakup_info.port_tts = 0;
Index: linux-4.10.1/drivers/staging/speakup/speakup_keypc.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_keypc.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_keypc.c
@@ -306,6 +306,7 @@
 
 static void keynote_release(void)
 {
+	spk_stop_serial_interrupt();
 	if (synth_port)
 		synth_release_region(synth_port, SYNTH_IO_EXTENT);
 	synth_port = 0;

[patch 5/7] staging: speakup: move those functions which do outgoing serial comms, into serialio.c

[okash.khawaja]

[-- Attachment #1: 05_move_spk_synth_immediate_and_spk_serial_synth_probe_into_serialio --]
[-- Type: text/plain, Size: 10916 bytes --]

This moves spk_synth_immediate and spk_serial_synth_probe functions into
serialio.c. These functions do outgoing serial comms. The move is a step
towards collecting all serial comms in serialio.c. This also renames
spk_synth_immediate to spk_serial_synth_immediate.

Code inside those functions has not been changed. Along the way, this patch
also fixes a couple of spots which were calling spk_synth_immediate directly,
so that the calls now happen via the spk_syth struct.

Signed-off-by: Okash Khawaja <okash.khawaja@gmail.com>

Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

Index: linux-4.10.1/drivers/staging/speakup/serialio.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/serialio.c
+++ linux-4.10.1/drivers/staging/speakup/serialio.c
@@ -136,6 +136,35 @@
 	outb(1, speakup_info.port_tts + UART_FCR);	/* Turn FIFO On */
 }
 
+int spk_serial_synth_probe(struct spk_synth *synth)
+{
+	const struct old_serial_port *ser;
+	int failed = 0;
+
+	if ((synth->ser >= SPK_LO_TTY) && (synth->ser <= SPK_HI_TTY)) {
+		ser = spk_serial_init(synth->ser);
+		if (ser == NULL) {
+			failed = -1;
+		} else {
+			outb_p(0, ser->port);
+			mdelay(1);
+			outb_p('\r', ser->port);
+		}
+	} else {
+		failed = -1;
+		pr_warn("ttyS%i is an invalid port\n", synth->ser);
+	}
+	if (failed) {
+		pr_info("%s: not found\n", synth->long_name);
+		return -ENODEV;
+	}
+	pr_info("%s: ttyS%i, Driver Version %s\n",
+		synth->long_name, synth->ser, synth->version);
+	synth->alive = 1;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(spk_serial_synth_probe);
+
 void spk_stop_serial_interrupt(void)
 {
 	if (speakup_info.port_tts == 0)
@@ -223,6 +252,23 @@
 	return 0;
 }
 
+const char *spk_serial_synth_immediate(struct spk_synth *synth, const char *buff)
+{
+	u_char ch;
+
+	while ((ch = *buff)) {
+		if (ch == '\n')
+			ch = synth->procspeech;
+		if (spk_wait_for_xmitr(synth))
+			outb(ch, speakup_info.port_tts);
+		else
+			return buff;
+		buff++;
+	}
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(spk_serial_synth_immediate);
+
 void spk_serial_release(void)
 {
 	spk_stop_serial_interrupt();
Index: linux-4.10.1/drivers/staging/speakup/speakup_acntsa.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_acntsa.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_acntsa.c
@@ -102,7 +102,7 @@
 	.io_ops = &spk_serial_io_ops,
 	.probe = synth_probe,
 	.release = spk_serial_release,
-	.synth_immediate = spk_synth_immediate,
+	.synth_immediate = spk_serial_synth_immediate,
 	.catch_up = spk_do_catch_up,
 	.flush = spk_synth_flush,
 	.is_alive = spk_synth_is_alive_restart,
@@ -127,7 +127,7 @@
 
 	failed = spk_serial_synth_probe(synth);
 	if (failed == 0) {
-		spk_synth_immediate(synth, "\033=R\r");
+		synth->synth_immediate(synth, "\033=R\r");
 		mdelay(100);
 	}
 	synth->alive = !failed;
Index: linux-4.10.1/drivers/staging/speakup/speakup_apollo.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_apollo.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_apollo.c
@@ -111,7 +111,7 @@
 	.io_ops = &spk_serial_io_ops,
 	.probe = spk_serial_synth_probe,
 	.release = spk_serial_release,
-	.synth_immediate = spk_synth_immediate,
+	.synth_immediate = spk_serial_synth_immediate,
 	.catch_up = do_catch_up,
 	.flush = spk_synth_flush,
 	.is_alive = spk_synth_is_alive_restart,
Index: linux-4.10.1/drivers/staging/speakup/speakup_audptr.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_audptr.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_audptr.c
@@ -107,7 +107,7 @@
 	.io_ops = &spk_serial_io_ops,
 	.probe = synth_probe,
 	.release = spk_serial_release,
-	.synth_immediate = spk_synth_immediate,
+	.synth_immediate = spk_serial_synth_immediate,
 	.catch_up = spk_do_catch_up,
 	.flush = synth_flush,
 	.is_alive = spk_synth_is_alive_restart,
@@ -144,7 +144,7 @@
 	unsigned char test = 0;
 	char synth_id[40] = "";
 
-	spk_synth_immediate(synth, "\x05[Q]");
+	synth->synth_immediate(synth, "\x05[Q]");
 	synth_id[test] = spk_serial_in();
 	if (synth_id[test] == 'A') {
 		do {
Index: linux-4.10.1/drivers/staging/speakup/speakup_bns.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_bns.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_bns.c
@@ -99,7 +99,7 @@
 	.io_ops = &spk_serial_io_ops,
 	.probe = spk_serial_synth_probe,
 	.release = spk_serial_release,
-	.synth_immediate = spk_synth_immediate,
+	.synth_immediate = spk_serial_synth_immediate,
 	.catch_up = spk_do_catch_up,
 	.flush = spk_synth_flush,
 	.is_alive = spk_synth_is_alive_restart,
Index: linux-4.10.1/drivers/staging/speakup/speakup_decext.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_decext.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_decext.c
@@ -130,7 +130,7 @@
 	.io_ops = &spk_serial_io_ops,
 	.probe = spk_serial_synth_probe,
 	.release = spk_serial_release,
-	.synth_immediate = spk_synth_immediate,
+	.synth_immediate = spk_serial_synth_immediate,
 	.catch_up = do_catch_up,
 	.flush = synth_flush,
 	.is_alive = spk_synth_is_alive_restart,
@@ -225,7 +225,7 @@
 static void synth_flush(struct spk_synth *synth)
 {
 	in_escape = 0;
-	spk_synth_immediate(synth, "\033P;10z\033\\");
+	synth->synth_immediate(synth, "\033P;10z\033\\");
 }
 
 module_param_named(ser, synth_decext.ser, int, S_IRUGO);
Index: linux-4.10.1/drivers/staging/speakup/speakup_dectlk.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_dectlk.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_dectlk.c
@@ -133,7 +133,7 @@
 	.io_ops = &spk_serial_io_ops,
 	.probe = spk_serial_synth_probe,
 	.release = spk_serial_release,
-	.synth_immediate = spk_synth_immediate,
+	.synth_immediate = spk_serial_synth_immediate,
 	.catch_up = do_catch_up,
 	.flush = synth_flush,
 	.is_alive = spk_synth_is_alive_restart,
Index: linux-4.10.1/drivers/staging/speakup/speakup_dummy.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_dummy.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_dummy.c
@@ -101,7 +101,7 @@
 	.io_ops = &spk_serial_io_ops,
 	.probe = spk_serial_synth_probe,
 	.release = spk_serial_release,
-	.synth_immediate = spk_synth_immediate,
+	.synth_immediate = spk_serial_synth_immediate,
 	.catch_up = spk_do_catch_up,
 	.flush = spk_synth_flush,
 	.is_alive = spk_synth_is_alive_restart,
Index: linux-4.10.1/drivers/staging/speakup/speakup_ltlk.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_ltlk.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_ltlk.c
@@ -114,7 +114,7 @@
 	.io_ops = &spk_serial_io_ops,
 	.probe = synth_probe,
 	.release = spk_serial_release,
-	.synth_immediate = spk_synth_immediate,
+	.synth_immediate = spk_serial_synth_immediate,
 	.catch_up = spk_do_catch_up,
 	.flush = spk_synth_flush,
 	.is_alive = spk_synth_is_alive_restart,
@@ -139,7 +139,7 @@
 	unsigned char *t, i;
 	unsigned char buf[50], rom_v[20];
 
-	spk_synth_immediate(synth, "\x18\x01?");
+	synth->synth_immediate(synth, "\x18\x01?");
 	for (i = 0; i < 50; i++) {
 		buf[i] = spk_serial_in();
 		if (i > 2 && buf[i] == 0x7f)
Index: linux-4.10.1/drivers/staging/speakup/speakup_spkout.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_spkout.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_spkout.c
@@ -105,7 +105,7 @@
 	.io_ops = &spk_serial_io_ops,
 	.probe = spk_serial_synth_probe,
 	.release = spk_serial_release,
-	.synth_immediate = spk_synth_immediate,
+	.synth_immediate = spk_serial_synth_immediate,
 	.catch_up = spk_do_catch_up,
 	.flush = synth_flush,
 	.is_alive = spk_synth_is_alive_restart,
Index: linux-4.10.1/drivers/staging/speakup/speakup_txprt.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_txprt.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_txprt.c
@@ -98,7 +98,7 @@
 	.io_ops = &spk_serial_io_ops,
 	.probe = spk_serial_synth_probe,
 	.release = spk_serial_release,
-	.synth_immediate = spk_synth_immediate,
+	.synth_immediate = spk_serial_synth_immediate,
 	.catch_up = spk_do_catch_up,
 	.flush = spk_synth_flush,
 	.is_alive = spk_synth_is_alive_restart,
Index: linux-4.10.1/drivers/staging/speakup/spk_priv.h
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/spk_priv.h
+++ linux-4.10.1/drivers/staging/speakup/spk_priv.h
@@ -58,7 +58,7 @@
 		      const char *buf, size_t count);
 
 int spk_serial_synth_probe(struct spk_synth *synth);
-const char *spk_synth_immediate(struct spk_synth *synth, const char *buff);
+const char *spk_serial_synth_immediate(struct spk_synth *synth, const char *buff);
 void spk_do_catch_up(struct spk_synth *synth);
 void spk_synth_flush(struct spk_synth *synth);
 int spk_synth_is_alive_nop(struct spk_synth *synth);
Index: linux-4.10.1/drivers/staging/speakup/synth.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/synth.c
+++ linux-4.10.1/drivers/staging/speakup/synth.c
@@ -44,35 +44,6 @@
 
 static int do_synth_init(struct spk_synth *in_synth);
 
-int spk_serial_synth_probe(struct spk_synth *synth)
-{
-	const struct old_serial_port *ser;
-	int failed = 0;
-
-	if ((synth->ser >= SPK_LO_TTY) && (synth->ser <= SPK_HI_TTY)) {
-		ser = spk_serial_init(synth->ser);
-		if (ser == NULL) {
-			failed = -1;
-		} else {
-			outb_p(0, ser->port);
-			mdelay(1);
-			outb_p('\r', ser->port);
-		}
-	} else {
-		failed = -1;
-		pr_warn("ttyS%i is an invalid port\n", synth->ser);
-	}
-	if (failed) {
-		pr_info("%s: not found\n", synth->long_name);
-		return -ENODEV;
-	}
-	pr_info("%s: ttyS%i, Driver Version %s\n",
-		synth->long_name, synth->ser, synth->version);
-	synth->alive = 1;
-	return 0;
-}
-EXPORT_SYMBOL_GPL(spk_serial_synth_probe);
-
 /*
  * Main loop of the progression thread: keep eating from the buffer
  * and push to the serial port, waiting as needed
@@ -147,23 +118,6 @@
 }
 EXPORT_SYMBOL_GPL(spk_do_catch_up);
 
-const char *spk_synth_immediate(struct spk_synth *synth, const char *buff)
-{
-	u_char ch;
-
-	while ((ch = *buff)) {
-		if (ch == '\n')
-			ch = synth->procspeech;
-		if (spk_wait_for_xmitr(synth))
-			outb(ch, speakup_info.port_tts);
-		else
-			return buff;
-		buff++;
-	}
-	return NULL;
-}
-EXPORT_SYMBOL_GPL(spk_synth_immediate);
-
 void spk_synth_flush(struct spk_synth *synth)
 {
 	synth->io_ops->synth_out(synth, synth->clear);

[patch 6/7] staging: speakup: add tty-based comms functions

[okash.khawaja]

[-- Attachment #1: 06_add_spk_ttyio --]
[-- Type: text/plain, Size: 6562 bytes --]

This adds spk_ttyio.c file. It contains a set of functions which implement
those methods in spk_synth struct which relate to sending bytes out using
serial comms. Implementations in this file perform the same function but
using TTY subsystem instead. Currently synths access serial ports, directly
poking standard ISA ports by trying to steal them from serial driver. Some ISA
cards actually need this way of doing it, but most other synthesizers don't,
and can actually work by using the proper TTY subsystem through a new N_SPEAKUP
line discipline. So this adds the methods for drivers to switch to accessing
serial ports through the TTY subsystem, whenever appropriate.

Signed-off-by: Okash Khawaja <okash.khawaja@gmail.com>

Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

Index: linux-4.10.1/drivers/staging/speakup/Makefile
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/Makefile
+++ linux-4.10.1/drivers/staging/speakup/Makefile
@@ -25,6 +25,7 @@
 	kobjects.o \
 	selection.o \
 	serialio.o \
+	spk_ttyio.o \
 	synth.o \
 	thread.o \
 	varhandlers.o
Index: linux-4.10.1/drivers/staging/speakup/spk_priv.h
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/spk_priv.h
+++ linux-4.10.1/drivers/staging/speakup/spk_priv.h
@@ -46,6 +46,7 @@
 unsigned char spk_serial_in(void);
 unsigned char spk_serial_in_nowait(void);
 void spk_serial_release(void);
+void spk_ttyio_release(void);
 
 void synth_buffer_skip_nonlatin1(void);
 u16 synth_buffer_getc(void);
@@ -58,7 +59,9 @@
 		      const char *buf, size_t count);
 
 int spk_serial_synth_probe(struct spk_synth *synth);
+int spk_ttyio_synth_probe(struct spk_synth *synth);
 const char *spk_serial_synth_immediate(struct spk_synth *synth, const char *buff);
+const char *spk_ttyio_synth_immediate(struct spk_synth *synth, const char *buff);
 void spk_do_catch_up(struct spk_synth *synth);
 void spk_synth_flush(struct spk_synth *synth);
 int spk_synth_is_alive_nop(struct spk_synth *synth);
@@ -78,5 +81,6 @@
 extern struct var_t synth_time_vars[];
 
 extern struct spk_io_ops spk_serial_io_ops;
+extern struct spk_io_ops spk_ttyio_ops;
 
 #endif
Index: linux-4.10.1/drivers/staging/speakup/spk_ttyio.c
===================================================================
--- /dev/null
+++ linux-4.10.1/drivers/staging/speakup/spk_ttyio.c
@@ -0,0 +1,143 @@
+#include <linux/types.h>
+#include <linux/tty.h>
+
+#include "speakup.h"
+#include "spk_types.h"
+
+static struct tty_struct *speakup_tty;
+
+static int spk_ttyio_ldisc_open(struct tty_struct *tty)
+{
+	if (tty->ops->write == NULL)
+		return -EOPNOTSUPP;
+	speakup_tty = tty;
+
+	return 0;
+}
+
+static void spk_ttyio_ldisc_close(struct tty_struct *tty)
+{
+	speakup_tty = NULL;
+}
+
+static struct tty_ldisc_ops spk_ttyio_ldisc_ops = {
+	.owner          = THIS_MODULE,
+	.magic          = TTY_LDISC_MAGIC,
+	.name           = "speakup_ldisc",
+	.open           = spk_ttyio_ldisc_open,
+	.close          = spk_ttyio_ldisc_close,
+};
+
+static int spk_ttyio_out(struct spk_synth *in_synth, const char ch);
+struct spk_io_ops spk_ttyio_ops = {
+	.synth_out = spk_ttyio_out,
+};
+EXPORT_SYMBOL_GPL(spk_ttyio_ops);
+
+static int spk_ttyio_initialise_ldisc(int ser)
+{
+	int ret = 0;
+	struct tty_struct *tty;
+
+	ret = tty_register_ldisc(N_SPEAKUP, &spk_ttyio_ldisc_ops);
+	if (ret) {
+		pr_err("Error registering line discipline.\n");
+		return ret;
+	}
+
+	if (ser < 0 || ser > (255 - 64)) {
+		pr_err("speakup: Invalid ser param. Must be between 0 and 191 inclusive.\n");
+		return -EINVAL;
+	}
+
+	/* TODO: support more than ttyS* */
+	tty = tty_open_by_driver(MKDEV(4, (ser +  64)), NULL, NULL);
+	if (IS_ERR(tty))
+		return PTR_ERR(tty);
+
+	if (tty->ops->open)
+		ret = tty->ops->open(tty, NULL);
+	else
+		ret = -ENODEV;
+
+	if (ret) {
+		tty_unlock(tty);
+		return ret;
+	}
+
+	clear_bit(TTY_HUPPED, &tty->flags);
+	tty_unlock(tty);
+
+	ret = tty_set_ldisc(tty, N_SPEAKUP);
+
+	return ret;
+}
+
+static int spk_ttyio_out(struct spk_synth *in_synth, const char ch)
+{
+	if (in_synth->alive && speakup_tty && speakup_tty->ops->write) {
+		int ret = speakup_tty->ops->write(speakup_tty, &ch, 1);
+		if (ret == 0)
+			/* No room */
+			return 0;
+		if (ret < 0) {
+			pr_warn("%s: I/O error, deactivating speakup\n", in_synth->long_name);
+			/* No synth any more, so nobody will restart TTYs, and we thus
+			 * need to do it ourselves.  Now that there is no synth we can
+			 * let application flood anyway
+			 */
+			in_synth->alive = 0;
+			speakup_start_ttys();
+			return 0;
+		}
+		return 1;
+	}
+	return 0;
+}
+
+int spk_ttyio_synth_probe(struct spk_synth *synth)
+{
+	int rv = spk_ttyio_initialise_ldisc(synth->ser);
+
+	if (rv)
+		return rv;
+
+	synth->alive = 1;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(spk_ttyio_synth_probe);
+
+void spk_ttyio_release(void)
+{
+	int idx;
+
+	if (!speakup_tty)
+		return;
+
+	tty_lock(speakup_tty);
+	idx = speakup_tty->index;
+
+	if (speakup_tty->ops->close)
+		speakup_tty->ops->close(speakup_tty, NULL);
+
+	tty_ldisc_flush(speakup_tty);
+	tty_unlock(speakup_tty);
+	tty_ldisc_release(speakup_tty);
+}
+EXPORT_SYMBOL_GPL(spk_ttyio_release);
+
+const char *spk_ttyio_synth_immediate(struct spk_synth *synth, const char *buff)
+{
+	u_char ch;
+
+	while ((ch = *buff)) {
+		if (ch == '\n')
+			ch = synth->procspeech;
+		if (tty_write_room(speakup_tty) < 1 || !synth->io_ops->synth_out(synth, ch))
+			return buff;
+		buff++;
+	}
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(spk_ttyio_synth_immediate);
Index: linux-4.10.1/drivers/tty/tty_ldisc.c
===================================================================
--- linux-4.10.1.orig/drivers/tty/tty_ldisc.c
+++ linux-4.10.1/drivers/tty/tty_ldisc.c
@@ -602,6 +602,7 @@
 	tty_unlock(tty);
 	return retval;
 }
+EXPORT_SYMBOL(tty_set_ldisc);
 
 /**
  *	tty_ldisc_kill	-	teardown ldisc
@@ -794,6 +795,7 @@
 
 	tty_ldisc_debug(tty, "released\n");
 }
+EXPORT_SYMBOL(tty_ldisc_release);
 
 /**
  *	tty_ldisc_init		-	ldisc setup for new tty
Index: linux-4.10.1/include/uapi/linux/tty.h
===================================================================
--- linux-4.10.1.orig/include/uapi/linux/tty.h
+++ linux-4.10.1/include/uapi/linux/tty.h
@@ -35,5 +35,6 @@
 #define N_TRACESINK	23	/* Trace data routing for MIPI P1149.7 */
 #define N_TRACEROUTER	24	/* Trace data routing for MIPI P1149.7 */
 #define N_NCI		25	/* NFC NCI UART */
+#define N_SPEAKUP	26	/* Speakup communication with synths*/
 
 #endif /* _UAPI_LINUX_TTY_H */

[patch 7/7] staging: speakup: migrate acntsa, bns, dummy and txprt to ttyio

[okash.khawaja]

[-- Attachment #1: 07_migrate_dummy_acntsa_bns_txprt_to_ttyio --]
[-- Type: text/plain, Size: 2671 bytes --]

This changes the above five synths to TTY-based comms. They were chosen as a
first pass because their serial comms are straightforward, i.e. they don't use
serial input and don't do internal port knocking.

Signed-off-by: Okash Khawaja <okash.khawaja@gmail.com>

Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

Index: linux-4.10.1/drivers/staging/speakup/speakup_dummy.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_dummy.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_dummy.c
@@ -98,10 +98,10 @@
 	.startup = SYNTH_START,
 	.checkval = SYNTH_CHECK,
 	.vars = vars,
-	.io_ops = &spk_serial_io_ops,
-	.probe = spk_serial_synth_probe,
-	.release = spk_serial_release,
-	.synth_immediate = spk_serial_synth_immediate,
+	.io_ops = &spk_ttyio_ops,
+	.probe = spk_ttyio_synth_probe,
+	.release = spk_ttyio_release,
+	.synth_immediate = spk_ttyio_synth_immediate,
 	.catch_up = spk_do_catch_up,
 	.flush = spk_synth_flush,
 	.is_alive = spk_synth_is_alive_restart,
Index: linux-4.10.1/drivers/staging/speakup/speakup_acntsa.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_acntsa.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_acntsa.c
@@ -99,10 +99,10 @@
 	.startup = SYNTH_START,
 	.checkval = SYNTH_CHECK,
 	.vars = vars,
-	.io_ops = &spk_serial_io_ops,
+	.io_ops = &spk_ttyio_ops,
 	.probe = synth_probe,
-	.release = spk_serial_release,
-	.synth_immediate = spk_serial_synth_immediate,
+	.release = spk_ttyio_release,
+	.synth_immediate = spk_ttyio_synth_immediate,
 	.catch_up = spk_do_catch_up,
 	.flush = spk_synth_flush,
 	.is_alive = spk_synth_is_alive_restart,
@@ -125,7 +125,7 @@
 {
 	int failed;
 
-	failed = spk_serial_synth_probe(synth);
+	failed = spk_ttyio_synth_probe(synth);
 	if (failed == 0) {
 		synth->synth_immediate(synth, "\033=R\r");
 		mdelay(100);
Index: linux-4.10.1/drivers/staging/speakup/speakup_txprt.c
===================================================================
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_txprt.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_txprt.c
@@ -95,10 +95,10 @@
 	.startup = SYNTH_START,
 	.checkval = SYNTH_CHECK,
 	.vars = vars,
-	.io_ops = &spk_serial_io_ops,
-	.probe = spk_serial_synth_probe,
-	.release = spk_serial_release,
-	.synth_immediate = spk_serial_synth_immediate,
+	.io_ops = &spk_ttyio_ops,
+	.probe = spk_ttyio_synth_probe,
+	.release = spk_ttyio_release,
+	.synth_immediate = spk_ttyio_synth_immediate,
 	.catch_up = spk_do_catch_up,
 	.flush = spk_synth_flush,
 	.is_alive = spk_synth_is_alive_restart,

Re: [patch 1/7] tty_port: allow a port to be opened with a tty that has no file handle

[Greg Kroah-Hartman]

On Mon, Mar 13, 2017 at 10:05:52PM +0000, okash.khawaja@gmail.com wrote:
> Allow access to TTY device from kernel. This is based on Alan Cox's patch
> (http://www.mail-archive.com/linux-kernel at vger.kernel.org/msg1215095.html),
> with description quoted below.
> 
> "tty_port: allow a port to be opened with a tty that has no file handle
> 
> Let us create tty objects entirely in kernel space.
> 
> With this a kernel created non file backed tty object could be used to handle
> data, and set terminal modes. Not all ldiscs can cope with this as N_TTY in
> particular has to work back to the fs/tty layer.
> 
> The tty_port code is however otherwise clean of file handles as far as I can
> tell as is the low level tty port write path used by the ldisc, the
> configuration low level interfaces and most of the ldiscs.
> 
> Currently you don't have any exposure to see tty hangups because those are
> built around the file layer. However a) it's a fixed port so you probably
> don't care about that b) if you do we can add a callback and c) you almost
> certainly don't want the userspace tear down/rebuild behaviour anyway.
> 
> This should however be sufficient if we wanted for example to enumerate all
> the bluetooth bound fixed ports via ACPI and make them directly available.
> 
> It doesn't deal with the case of a user opening a port that's also kernel
> opened and that would need some locking out (so it returned EBUSY if bound
> to a kernel device of some kind). That needs resolving along with how you
> "up" or "down" your new bluetooth device, or enumerate it while providing
> the existing tty API to avoid regressions (and to debug)."
> 
> Signed-off-by: Okash Khawaja <okash.khawaja@gmail.com>
> 
> Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

You do know this is already in 4.11-rc1, right?  Please rebase your
patch set on 4.11-rc2 at the least and resend.

thanks,

greg k-h

Re: [patch 0/7] staging: speakup: introduce tty-based comms

[Greg Kroah-Hartman]

On Mon, Mar 13, 2017 at 10:05:51PM +0000, okash.khawaja@gmail.com wrote:
> Hi,
> 
> This patchset introduces a TTY-based way for the synths to communicate
> with devices as an alternate for direct serial comms used by the synths
> at the moment. It then migrates some of the synths to the TTY-based
> comms. Synths migrated in this patchset are dummy, acntsa, bns and
> txprt.

What about using the serbus code that is now in the tree?  That should
make this a lot easier than your patchset from what I can see.

thanks,

greg k-h

Re: [patch 0/7] staging: speakup: introduce tty-based comms

[Samuel Thibault]

Hello,

Greg KH, on mar. 14 mars 2017 06:14:04 +0800, wrote:
> On Mon, Mar 13, 2017 at 10:05:51PM +0000, okash.khawaja@gmail.com wrote:
> > This patchset introduces a TTY-based way for the synths to communicate
> > with devices as an alternate for direct serial comms used by the synths
> > at the moment. It then migrates some of the synths to the TTY-based
> > comms. Synths migrated in this patchset are dummy, acntsa, bns and
> > txprt.
> 
> What about using the serbus code that is now in the tree?

I guess you mean serdev?

Well, for a start that didn't exist when Okash worked on the TTY part :)

> That should make this a lot easier than your patchset from what I can
> see.

Will serdev support modem line control?  We need this for some devices.
How is the serial port chosen with serdev?

Samuel

Re: [patch 1/7] tty_port: allow a port to be opened with a tty that has no file handle

[Okash Khawaja]

On Tue, Mar 14, 2017 at 06:12:47AM +0800, Greg Kroah-Hartman wrote:
> On Mon, Mar 13, 2017 at 10:05:52PM +0000, okash.khawaja@gmail.com wrote:
> > Allow access to TTY device from kernel. This is based on Alan Cox's patch
> > (http://www.mail-archive.com/linux-kernel at vger.kernel.org/msg1215095.html),
> > with description quoted below.
> > 
> > "tty_port: allow a port to be opened with a tty that has no file handle
> > 
> > Let us create tty objects entirely in kernel space.
> > 
> > With this a kernel created non file backed tty object could be used to handle
> > data, and set terminal modes. Not all ldiscs can cope with this as N_TTY in
> > particular has to work back to the fs/tty layer.
> > 
> > The tty_port code is however otherwise clean of file handles as far as I can
> > tell as is the low level tty port write path used by the ldisc, the
> > configuration low level interfaces and most of the ldiscs.
> > 
> > Currently you don't have any exposure to see tty hangups because those are
> > built around the file layer. However a) it's a fixed port so you probably
> > don't care about that b) if you do we can add a callback and c) you almost
> > certainly don't want the userspace tear down/rebuild behaviour anyway.
> > 
> > This should however be sufficient if we wanted for example to enumerate all
> > the bluetooth bound fixed ports via ACPI and make them directly available.
> > 
> > It doesn't deal with the case of a user opening a port that's also kernel
> > opened and that would need some locking out (so it returned EBUSY if bound
> > to a kernel device of some kind). That needs resolving along with how you
> > "up" or "down" your new bluetooth device, or enumerate it while providing
> > the existing tty API to avoid regressions (and to debug)."
> > 
> > Signed-off-by: Okash Khawaja <okash.khawaja@gmail.com>
> > 
> > Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> 
> You do know this is already in 4.11-rc1, right?  Please rebase your
> patch set on 4.11-rc2 at the least and resend.

Didn't realise that! Will resend

Re: [patch 0/7] staging: speakup: introduce tty-based comms

[Greg Kroah-Hartman]

On Mon, Mar 13, 2017 at 11:26:08PM +0100, Samuel Thibault wrote:
> Hello,
> 
> Greg KH, on mar. 14 mars 2017 06:14:04 +0800, wrote:
> > On Mon, Mar 13, 2017 at 10:05:51PM +0000, okash.khawaja@gmail.com wrote:
> > > This patchset introduces a TTY-based way for the synths to communicate
> > > with devices as an alternate for direct serial comms used by the synths
> > > at the moment. It then migrates some of the synths to the TTY-based
> > > comms. Synths migrated in this patchset are dummy, acntsa, bns and
> > > txprt.
> > 
> > What about using the serbus code that is now in the tree?
> 
> I guess you mean serdev?

Sorry, yes.

> Well, for a start that didn't exist when Okash worked on the TTY part :)

Fair enough, but it has been discussed for many months now on the
linux-serial mailing list.

> > That should make this a lot easier than your patchset from what I can
> > see.
> 
> Will serdev support modem line control?  We need this for some devices.
> How is the serial port chosen with serdev?

Best asked on the linux-serial mailing list :)

thanks,

greg k-h

Re: [patch 0/7] staging: speakup: introduce tty-based comms

[Samuel Thibault]

Greg KH, on mar. 14 mars 2017 07:43:03 +0800, wrote:
> > Well, for a start that didn't exist when Okash worked on the TTY part :)
> 
> Fair enough, but it has been discussed for many months now on the
> linux-serial mailing list.

I have not had time for years to actually work on this subject, so I've
had even less time to follow linux-serial :) (and Okash, who actually
got the time to do the work, is very new to all this).

> > > That should make this a lot easier than your patchset from what I can
> > > see.
> > 
> > Will serdev support modem line control?  We need this for some devices.
> > How is the serial port chosen with serdev?
> 
> Best asked on the linux-serial mailing list :)

Let's discuss there, then.

Samuel

Re: [patch 0/7] staging: speakup: introduce tty-based comms

[Samuel Thibault]

Samuel Thibault, on lun. 13 mars 2017 23:26:08 +0100, wrote:
> > That should make this a lot easier than your patchset from what I can
> > see.
> 
> Will serdev support modem line control?  We need this for some devices.

"serdev: add serdev_device_set_rts" seems to be saying yes :)

Samuel

Re: [patch 0/7] staging: speakup: introduce tty-based comms

[Samuel Thibault]

Hello Okash,

I'd say for a start already, rebase and submit patches 2-5, since they
are refactoring that makes sense anyway. And we can discuss about serdev
while that gets integrated, and thus serdev-equivalents of patches 1 and
6 will then be immediatetly applicable.

Samuel

Re: [patch 0/7] staging: speakup: introduce tty-based comms

[Samuel Thibault]

Hello,

Greg KH, on mar. 14 mars 2017 06:14:04 +0800, wrote:
> On Mon, Mar 13, 2017 at 10:05:51PM +0000, okash.khawaja@gmail.com wrote:
> > This patchset introduces a TTY-based way for the synths to communicate
> > with devices as an alternate for direct serial comms used by the synths
> > at the moment. It then migrates some of the synths to the TTY-based
> > comms. Synths migrated in this patchset are dummy, acntsa, bns and
> > txprt.
> 
> What about using the serbus code that is now in the tree?  That should
> make this a lot easier than your patchset from what I can see.

Mmm... AIUI from reading tty_port_register_device_attr, one
would have to have registered a speakup serdev device driver
*before* tty_port_register_device_attr gets called, so that
serdev_tty_port_register matches the driver in the loop of
of_serdev_register_devices, and no TTY cdev is created?

That would mean that speakup can not be loaded as a module after ttyS0
initialization, that won't fly for our use needs. The line discipline
mechanism allows us to attach ourself to an existing tty.  Could we
imagine a tty_port function which removes the cdev and tries to register
the tty port again to serdev?

What we basically need to be able to say on speakup module load is
e.g. "I'm now attaching a device to ttyS0, use this serdev_device_ops to
discuss with it".

Samuel

Re: [patch 0/7] staging: speakup: introduce tty-based comms

[Samuel Thibault]

Samuel Thibault, on mar. 14 mars 2017 01:47:01 +0100, wrote:
> Greg KH, on mar. 14 mars 2017 06:14:04 +0800, wrote:
> > On Mon, Mar 13, 2017 at 10:05:51PM +0000, okash.khawaja@gmail.com wrote:
> > > This patchset introduces a TTY-based way for the synths to communicate
> > > with devices as an alternate for direct serial comms used by the synths
> > > at the moment. It then migrates some of the synths to the TTY-based
> > > comms. Synths migrated in this patchset are dummy, acntsa, bns and
> > > txprt.
> > 
> > What about using the serbus code that is now in the tree?  That should
> > make this a lot easier than your patchset from what I can see.
> 
> Mmm... AIUI from reading tty_port_register_device_attr, one
> would have to have registered a speakup serdev device driver
> *before* tty_port_register_device_attr gets called, so that
> serdev_tty_port_register matches the driver in the loop of
> of_serdev_register_devices, and no TTY cdev is created?
> 
> That would mean that speakup can not be loaded as a module after ttyS0
> initialization, that won't fly for our use needs. The line discipline
> mechanism allows us to attach ourself to an existing tty.  Could we
> imagine a tty_port function which removes the cdev and tries to register
> the tty port again to serdev?
> 
> What we basically need to be able to say on speakup module load is
> e.g. "I'm now attaching a device to ttyS0, use this serdev_device_ops to
> discuss with it".

That for_each_available_child_of_node loop is really way more complex
than what we need.  And what's more, it's not working without CONFIG_OF
(!)

It would really make sense to me to have a

serdev_device *tty_port_register_serdev_device(tty, device)

which unregisters the character device of the tty, and creates instead a
controler with the given device plugged to it. Really much like a line
discipline, but way simpler :)

The issue that remains is the "tty" part: we'd need there the tty_port,
the parent device, the tty driver, the idx. All we know when we load
speakup is "ttyS0". That's where going through the cdev to attach a line
discipline was simpler :)

Samuel

Re: [patch 1/7] tty_port: allow a port to be opened with a tty that has no file handle

[Dan Carpenter]

Could you fix your From header in your email client?

regards,
dan carpenter

Re: [patch 0/7] staging: speakup: introduce tty-based comms

[Rob Herring]

On Mon, Mar 13, 2017 at 8:18 PM, Samuel Thibault
<samuel.thibault@ens-lyon.org> wrote:
> Samuel Thibault, on mar. 14 mars 2017 01:47:01 +0100, wrote:
>> Greg KH, on mar. 14 mars 2017 06:14:04 +0800, wrote:
>> > On Mon, Mar 13, 2017 at 10:05:51PM +0000, okash.khawaja@gmail.com wrote:
>> > > This patchset introduces a TTY-based way for the synths to communicate
>> > > with devices as an alternate for direct serial comms used by the synths
>> > > at the moment. It then migrates some of the synths to the TTY-based
>> > > comms. Synths migrated in this patchset are dummy, acntsa, bns and
>> > > txprt.
>> >
>> > What about using the serbus code that is now in the tree?  That should
>> > make this a lot easier than your patchset from what I can see.
>>
>> Mmm... AIUI from reading tty_port_register_device_attr, one
>> would have to have registered a speakup serdev device driver
>> *before* tty_port_register_device_attr gets called, so that
>> serdev_tty_port_register matches the driver in the loop of
>> of_serdev_register_devices, and no TTY cdev is created?

Not exactly. The driver doesn't have to be registered nor loaded, but
the device does have to be present so no tty cdev is created. The only
way a device is present currently is when a node is in the DT. I
expect the x86 folks will be adding ACPI support soon.

>> That would mean that speakup can not be loaded as a module after ttyS0
>> initialization, that won't fly for our use needs. The line discipline
>> mechanism allows us to attach ourself to an existing tty.  Could we
>> imagine a tty_port function which removes the cdev and tries to register
>> the tty port again to serdev?
>>
>> What we basically need to be able to say on speakup module load is
>> e.g. "I'm now attaching a device to ttyS0, use this serdev_device_ops to
>> discuss with it".
>
> That for_each_available_child_of_node loop is really way more complex
> than what we need.  And what's more, it's not working without CONFIG_OF
> (!)
>
> It would really make sense to me to have a
>
> serdev_device *tty_port_register_serdev_device(tty, device)
>
> which unregisters the character device of the tty, and creates instead a
> controler with the given device plugged to it. Really much like a line
> discipline, but way simpler :)

What would trigger calling this function? I could imagine people
wanting to do DT overlays to add a device which could trigger it. Or
did you have some other mechanism in mind.

>
> The issue that remains is the "tty" part: we'd need there the tty_port,
> the parent device, the tty driver, the idx. All we know when we load
> speakup is "ttyS0". That's where going through the cdev to attach a line
> discipline was simpler :)
>
> Samuel

Re: [patch 0/7] staging: speakup: introduce tty-based comms

[Samuel Thibault]

Rob Herring, on mer. 15 mars 2017 09:45:59 -0500, wrote:
> On Mon, Mar 13, 2017 at 8:18 PM, Samuel Thibault
> <samuel.thibault@ens-lyon.org> wrote:
> > Samuel Thibault, on mar. 14 mars 2017 01:47:01 +0100, wrote:
> >> Greg KH, on mar. 14 mars 2017 06:14:04 +0800, wrote:
> >> > On Mon, Mar 13, 2017 at 10:05:51PM +0000, okash.khawaja@gmail.com wrote:
> >> > > This patchset introduces a TTY-based way for the synths to communicate
> >> > > with devices as an alternate for direct serial comms used by the synths
> >> > > at the moment. It then migrates some of the synths to the TTY-based
> >> > > comms. Synths migrated in this patchset are dummy, acntsa, bns and
> >> > > txprt.
> >> >
> >> > What about using the serbus code that is now in the tree?  That should
> >> > make this a lot easier than your patchset from what I can see.
> >>
> >> Mmm... AIUI from reading tty_port_register_device_attr, one
> >> would have to have registered a speakup serdev device driver
> >> *before* tty_port_register_device_attr gets called, so that
> >> serdev_tty_port_register matches the driver in the loop of
> >> of_serdev_register_devices, and no TTY cdev is created?
> 
> Not exactly. The driver doesn't have to be registered nor loaded, but
> the device does have to be present so no tty cdev is created. The only
> way a device is present currently is when a node is in the DT. I
> expect the x86 folks will be adding ACPI support soon.

Ok, but in our case there is no hardware device that the system can
see/probe, it's just plugged externally.

> >> That would mean that speakup can not be loaded as a module after ttyS0
> >> initialization, that won't fly for our use needs. The line discipline
> >> mechanism allows us to attach ourself to an existing tty.  Could we
> >> imagine a tty_port function which removes the cdev and tries to register
> >> the tty port again to serdev?
> >>
> >> What we basically need to be able to say on speakup module load is
> >> e.g. "I'm now attaching a device to ttyS0, use this serdev_device_ops to
> >> discuss with it".
> >
> > That for_each_available_child_of_node loop is really way more complex
> > than what we need.  And what's more, it's not working without CONFIG_OF
> > (!)
> >
> > It would really make sense to me to have a
> >
> > serdev_device *tty_port_register_serdev_device(tty, device)
> >
> > which unregisters the character device of the tty, and creates instead a
> > controler with the given device plugged to it. Really much like a line
> > discipline, but way simpler :)
> 
> What would trigger calling this function?

>From the user point of view, loading the speakup module for the external
device, typically, with "ttyS0" as module parameter. Then the speakup
init function can do whatever it needs to achieve this :)

Samuel

Re: [patch 0/7] staging: speakup: introduce tty-based comms

[Samuel Thibault]

Hello,

Also, could serdev also provide an send_xchar function similar to the
send_xchar tty driver function?  We need this to send high-priority
characters.

Samuel

Re: [patch 0/7] staging: speakup: introduce tty-based comms

[Samuel Thibault]

Hello,

So Rob, how do you see this going?  Shall we introduce a serdev_device
*tty_port_register_serdev_device(tty, device)? Will you work on it or
should I give it a try?

Samuel

Samuel Thibault, on mer. 15 mars 2017 16:03:23 +0100, wrote:
> Rob Herring, on mer. 15 mars 2017 09:45:59 -0500, wrote:
> > On Mon, Mar 13, 2017 at 8:18 PM, Samuel Thibault
> > <samuel.thibault@ens-lyon.org> wrote:
> > > Samuel Thibault, on mar. 14 mars 2017 01:47:01 +0100, wrote:
> > >> Greg KH, on mar. 14 mars 2017 06:14:04 +0800, wrote:
> > >> > On Mon, Mar 13, 2017 at 10:05:51PM +0000, okash.khawaja@gmail.com wrote:
> > >> > > This patchset introduces a TTY-based way for the synths to communicate
> > >> > > with devices as an alternate for direct serial comms used by the synths
> > >> > > at the moment. It then migrates some of the synths to the TTY-based
> > >> > > comms. Synths migrated in this patchset are dummy, acntsa, bns and
> > >> > > txprt.
> > >> >
> > >> > What about using the serbus code that is now in the tree?  That should
> > >> > make this a lot easier than your patchset from what I can see.
> > >>
> > >> Mmm... AIUI from reading tty_port_register_device_attr, one
> > >> would have to have registered a speakup serdev device driver
> > >> *before* tty_port_register_device_attr gets called, so that
> > >> serdev_tty_port_register matches the driver in the loop of
> > >> of_serdev_register_devices, and no TTY cdev is created?
> > 
> > Not exactly. The driver doesn't have to be registered nor loaded, but
> > the device does have to be present so no tty cdev is created. The only
> > way a device is present currently is when a node is in the DT. I
> > expect the x86 folks will be adding ACPI support soon.
> 
> Ok, but in our case there is no hardware device that the system can
> see/probe, it's just plugged externally.
> 
> > >> That would mean that speakup can not be loaded as a module after ttyS0
> > >> initialization, that won't fly for our use needs. The line discipline
> > >> mechanism allows us to attach ourself to an existing tty.  Could we
> > >> imagine a tty_port function which removes the cdev and tries to register
> > >> the tty port again to serdev?
> > >>
> > >> What we basically need to be able to say on speakup module load is
> > >> e.g. "I'm now attaching a device to ttyS0, use this serdev_device_ops to
> > >> discuss with it".
> > >
> > > That for_each_available_child_of_node loop is really way more complex
> > > than what we need.  And what's more, it's not working without CONFIG_OF
> > > (!)
> > >
> > > It would really make sense to me to have a
> > >
> > > serdev_device *tty_port_register_serdev_device(tty, device)
> > >
> > > which unregisters the character device of the tty, and creates instead a
> > > controler with the given device plugged to it. Really much like a line
> > > discipline, but way simpler :)
> > 
> > What would trigger calling this function?
> 
> From the user point of view, loading the speakup module for the external
> device, typically, with "ttyS0" as module parameter. Then the speakup
> init function can do whatever it needs to achieve this :)

Re: [patch 0/7] staging: speakup: introduce tty-based comms

[Okash Khawaja]

Hi,

Any updates on this? We can start working in
tty_port_register_serdev_device() if that's okay?

Thanks,
Okash

On Wed, Mar 22, 2017 at 01:05:24AM +0100, Samuel Thibault wrote:
> Hello,
> 
> So Rob, how do you see this going?  Shall we introduce a serdev_device
> *tty_port_register_serdev_device(tty, device)? Will you work on it or
> should I give it a try?
> 
> Samuel
> 
> Samuel Thibault, on mer. 15 mars 2017 16:03:23 +0100, wrote:
> > Rob Herring, on mer. 15 mars 2017 09:45:59 -0500, wrote:
> > > On Mon, Mar 13, 2017 at 8:18 PM, Samuel Thibault
> > > <samuel.thibault@ens-lyon.org> wrote:
> > > > Samuel Thibault, on mar. 14 mars 2017 01:47:01 +0100, wrote:
> > > >> Greg KH, on mar. 14 mars 2017 06:14:04 +0800, wrote:
> > > >> > On Mon, Mar 13, 2017 at 10:05:51PM +0000, okash.khawaja@gmail.com wrote:
> > > >> > > This patchset introduces a TTY-based way for the synths to communicate
> > > >> > > with devices as an alternate for direct serial comms used by the synths
> > > >> > > at the moment. It then migrates some of the synths to the TTY-based
> > > >> > > comms. Synths migrated in this patchset are dummy, acntsa, bns and
> > > >> > > txprt.
> > > >> >
> > > >> > What about using the serbus code that is now in the tree?  That should
> > > >> > make this a lot easier than your patchset from what I can see.
> > > >>
> > > >> Mmm... AIUI from reading tty_port_register_device_attr, one
> > > >> would have to have registered a speakup serdev device driver
> > > >> *before* tty_port_register_device_attr gets called, so that
> > > >> serdev_tty_port_register matches the driver in the loop of
> > > >> of_serdev_register_devices, and no TTY cdev is created?
> > > 
> > > Not exactly. The driver doesn't have to be registered nor loaded, but
> > > the device does have to be present so no tty cdev is created. The only
> > > way a device is present currently is when a node is in the DT. I
> > > expect the x86 folks will be adding ACPI support soon.
> > 
> > Ok, but in our case there is no hardware device that the system can
> > see/probe, it's just plugged externally.
> > 
> > > >> That would mean that speakup can not be loaded as a module after ttyS0
> > > >> initialization, that won't fly for our use needs. The line discipline
> > > >> mechanism allows us to attach ourself to an existing tty.  Could we
> > > >> imagine a tty_port function which removes the cdev and tries to register
> > > >> the tty port again to serdev?
> > > >>
> > > >> What we basically need to be able to say on speakup module load is
> > > >> e.g. "I'm now attaching a device to ttyS0, use this serdev_device_ops to
> > > >> discuss with it".
> > > >
> > > > That for_each_available_child_of_node loop is really way more complex
> > > > than what we need.  And what's more, it's not working without CONFIG_OF
> > > > (!)
> > > >
> > > > It would really make sense to me to have a
> > > >
> > > > serdev_device *tty_port_register_serdev_device(tty, device)
> > > >
> > > > which unregisters the character device of the tty, and creates instead a
> > > > controler with the given device plugged to it. Really much like a line
> > > > discipline, but way simpler :)
> > > 
> > > What would trigger calling this function?
> > 
> > From the user point of view, loading the speakup module for the external
> > device, typically, with "ttyS0" as module parameter. Then the speakup
> > init function can do whatever it needs to achieve this :)