Skip to contentSkip to conversation

staging: speakup: introduce tty-based comms

PATCH In review [email protected] 5 participants 24 messages Mar 13, 2017 – Apr 9, 2017

Subsystems (2)
Staging TTY layer and serial drivers
lore
Share & tools
lore

[email protected]

author
#0 lore raw

cover staging: speakup: introduce tty-based comms

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

author
#1 lore raw

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

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 <[email protected]>
Reviewed-by Samuel Thibault <[email protected]>
drivers/tty/tty_io.c +8 -4
Index: 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
drivers/tty/tty_port.c +2 -2
Index: 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);
include/linux/tty.h +2 -0
Index: 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)
 { }
maintainer
#2 lore raw

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

Quoted 30 lines · On Mon, Mar 13, 2017 at 10:05:52PM +0000, [email protected] 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)."

Quoted 3 lines
Signed-off-by Okash Khawaja <[email protected]>
Reviewed-by Samuel Thibault <[email protected]>

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

author
#3 lore raw

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

Quoted 31 lines · Greg Kroah-Hartman
Quoted 30 lines · On Mon, Mar 13, 2017 at 10:05:52PM +0000, [email protected] 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)."

Quoted 6 lines
Quoted 3 lines
Signed-off-by Okash Khawaja <[email protected]>
Reviewed-by Samuel Thibault <[email protected]>

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

author
#5 lore raw

PATCH 2/7 staging: speakup: spk_serial_out and spk_wait_for_xmitr to take synth arg

In reply to [email protected]

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 <[email protected]>
Reviewed-by Samuel Thibault <[email protected]>
drivers/staging/speakup/serialio.c +7 -7
Index: 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;
 	}
drivers/staging/speakup/speakup_apollo.c +3 -3
Index: 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);
drivers/staging/speakup/speakup_audptr.c +1 -1
Index: 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)
drivers/staging/speakup/speakup_decext.c +4 -4
Index: 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)
drivers/staging/speakup/speakup_dectlk.c +6 -6
Index: 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);
drivers/staging/speakup/spk_priv.h +2 -2
Index: 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);
drivers/staging/speakup/synth.c +6 -6
Index: 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);
author
#6 lore raw

PATCH 3/7 staging: serial: add spk_io_ops struct to spk_synth

In reply to [email protected]

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 <[email protected]>
Reviewed-by Samuel Thibault <[email protected]>
drivers/staging/speakup/serialio.c +6 -1
Index: 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)
 {
drivers/staging/speakup/speakup_acntpc.c +1 -0
Index: 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,
drivers/staging/speakup/speakup_acntsa.c +1 -0
Index: 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,
drivers/staging/speakup/speakup_apollo.c +4 -3
Index: 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);
drivers/staging/speakup/speakup_audptr.c +2 -1
Index: 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)
drivers/staging/speakup/speakup_decext.c +5 -4
Index: 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)
drivers/staging/speakup/speakup_dectlk.c +7 -6
Index: 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);
drivers/staging/speakup/spk_priv.h +2 -1
Index: 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
drivers/staging/speakup/spk_types.h +7 -0
Index: 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,
drivers/staging/speakup/synth.c +4 -4
Index: 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);
drivers/staging/speakup/speakup_bns.c +1 -0
Index: 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,
drivers/staging/speakup/speakup_decpc.c +1 -0
Index: 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,
drivers/staging/speakup/speakup_dtlk.c +1 -0
Index: 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,
drivers/staging/speakup/speakup_dummy.c +1 -0
Index: 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,
drivers/staging/speakup/speakup_keypc.c +1 -0
Index: 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,
drivers/staging/speakup/speakup_ltlk.c +1 -0
Index: 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,
drivers/staging/speakup/speakup_soft.c +1 -0
Index: 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,
drivers/staging/speakup/speakup_spkout.c +1 -0
Index: 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,
drivers/staging/speakup/speakup_txprt.c +1 -0
Index: 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,
author
#7 lore raw

PATCH 4/7 staging: speakup: move spk_stop_serial_interrupt into synth-specific release function

In reply to [email protected]

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 <[email protected]>
Reviewed-by Samuel Thibault <[email protected]>
drivers/staging/speakup/serialio.c +2 -0
Index: 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);
drivers/staging/speakup/speakup_acntpc.c +1 -0
Index: 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;
drivers/staging/speakup/synth.c +0 -1
Index: 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;
 }
drivers/staging/speakup/speakup_decpc.c +1 -0
Index: 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;
drivers/staging/speakup/speakup_dtlk.c +1 -0
Index: 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;
drivers/staging/speakup/speakup_keypc.c +1 -0
Index: 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;
author
#8 lore raw

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

In reply to [email protected]

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 <[email protected]>
Reviewed-by Samuel Thibault <[email protected]>
drivers/staging/speakup/serialio.c +46 -0
Index: 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();
drivers/staging/speakup/speakup_acntsa.c +2 -2
Index: 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;
drivers/staging/speakup/speakup_apollo.c +1 -1
Index: 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,
drivers/staging/speakup/speakup_audptr.c +2 -2
Index: 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 {
drivers/staging/speakup/speakup_bns.c +1 -1
Index: 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,
drivers/staging/speakup/speakup_decext.c +2 -2
Index: 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);
drivers/staging/speakup/speakup_dectlk.c +1 -1
Index: 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,
drivers/staging/speakup/speakup_dummy.c +1 -1
Index: 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,
drivers/staging/speakup/speakup_ltlk.c +2 -2
Index: 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)
drivers/staging/speakup/speakup_spkout.c +1 -1
Index: 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,
drivers/staging/speakup/speakup_txprt.c +1 -1
Index: 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,
drivers/staging/speakup/spk_priv.h +1 -1
Index: 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);
drivers/staging/speakup/synth.c +0 -46

58 lines not on this page · raw message

author
#9 lore raw

PATCH 6/7 staging: speakup: add tty-based comms functions

In reply to [email protected]

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 <[email protected]>
Reviewed-by Samuel Thibault <[email protected]>
drivers/staging/speakup/Makefile +1 -0
Index: 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
drivers/staging/speakup/spk_priv.h +4 -0
Index: 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
drivers/staging/speakup/spk_ttyio.c new file +143 -0

143 lines not on this page · raw message

drivers/tty/tty_ldisc.c +2 -0
Index: 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
include/uapi/linux/tty.h +1 -0
Index: 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 */
author
#10 lore raw

PATCH 7/7 staging: speakup: migrate acntsa, bns, dummy and txprt to ttyio

In reply to [email protected]

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 <[email protected]>
Reviewed-by Samuel Thibault <[email protected]>
drivers/staging/speakup/speakup_dummy.c +4 -4
Index: 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,
drivers/staging/speakup/speakup_acntsa.c +4 -4
Index: 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);
drivers/staging/speakup/speakup_txprt.c +4 -4
Index: 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,
maintainer
#11 lore raw
In reply to [email protected]
Quoted 7 lines · On Mon, Mar 13, 2017 at 10:05:51PM +0000, [email protected] 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

#12 lore raw

Hello,

Quoted 8 lines · Greg KH, on mar. 14 mars 2017 06:14:04 +0800, wrote:
Quoted 5 lines · On Mon, Mar 13, 2017 at 10:05:51PM +0000, [email protected] 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 :)

Quoted 2 lines

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

maintainer
#13 lore raw
Quoted 10 lines · Samuel Thibault

Hello,

Quoted 7 lines · Greg KH, on mar. 14 mars 2017 06:14:04 +0800, wrote:
Quoted 5 lines · On Mon, Mar 13, 2017 at 10:05:51PM +0000, [email protected] 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.

Quoted 3 lines
Quoted 1 line

What about using the serbus code that is now in the tree?

I guess you mean serdev?

Sorry, yes.

Quoted 1 line

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.

Quoted 5 lines
Quoted 2 lines

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

#14 lore raw
Quoted 4 lines · Greg KH, on mar. 14 mars 2017 07:43:03 +0800, wrote:
Quoted 1 line

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).

Quoted 7 lines
Quoted 5 lines
Quoted 2 lines

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

#15 lore raw
In reply to Samuel Thibault
Quoted 4 lines · Samuel Thibault, on lun. 13 mars 2017 23:26:08 +0100, wrote:
Quoted 2 lines

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

#16 lore raw
In reply to Greg Kroah-Hartman

Hello,

Quoted 9 lines · Greg KH, on mar. 14 mars 2017 06:14:04 +0800, wrote:
Quoted 5 lines · On Mon, Mar 13, 2017 at 10:05:51PM +0000, [email protected] 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

#17 lore raw
Quoted 23 lines · Samuel Thibault, on mar. 14 mars 2017 01:47:01 +0100, wrote:
Quoted 9 lines · Greg KH, on mar. 14 mars 2017 06:14:04 +0800, wrote:
Quoted 5 lines · On Mon, Mar 13, 2017 at 10:05:51PM +0000, [email protected] 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?

Quoted 3 lines

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

#18 lore raw
Quoted 9 lines · Samuel Thibault
Quoted 8 lines · Samuel Thibault, on mar. 14 mars 2017 01:47:01 +0100, wrote:
Quoted 7 lines · Greg KH, on mar. 14 mars 2017 06:14:04 +0800, wrote:
Quoted 5 lines · On Mon, Mar 13, 2017 at 10:05:51PM +0000, [email protected] 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.

Quoted 8 lines
Quoted 8 lines
Quoted 2 lines

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.

Quoted 14 lines
Quoted 9 lines

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 (!)

Quoted 7 lines

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.

Quoted 7 lines

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

#19 lore raw
Quoted 20 lines · Rob Herring, on mer. 15 mars 2017 09:45:59 -0500, wrote:
Quoted 9 lines · Samuel Thibault
Quoted 8 lines · Samuel Thibault, on mar. 14 mars 2017 01:47:01 +0100, wrote:
Quoted 7 lines · Greg KH, on mar. 14 mars 2017 06:14:04 +0800, wrote:
Quoted 5 lines · On Mon, Mar 13, 2017 at 10:05:51PM +0000, [email protected] 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.

Quoted 8 lines
Quoted 8 lines
Quoted 2 lines

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?

Quoted 4 lines

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.

Quoted 18 lines
Quoted 18 lines
Quoted 9 lines

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)

Quoted 7 lines
Quoted 3 lines

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

#20 lore raw

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

Quoted 56 lines · Samuel Thibault, on mer. 15 mars 2017 16:03:23 +0100, wrote:
Quoted 20 lines · Rob Herring, on mer. 15 mars 2017 09:45:59 -0500, wrote:
Quoted 9 lines · Samuel Thibault
Quoted 8 lines · Samuel Thibault, on mar. 14 mars 2017 01:47:01 +0100, wrote:
Quoted 7 lines · Greg KH, on mar. 14 mars 2017 06:14:04 +0800, wrote:
Quoted 5 lines · On Mon, Mar 13, 2017 at 10:05:51PM +0000, [email protected] 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.

Quoted 8 lines
Quoted 8 lines
Quoted 2 lines

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?

Quoted 4 lines

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.

Quoted 16 lines
Quoted 16 lines
Quoted 9 lines

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

Quoted 7 lines
Quoted 5 lines

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 :)

author
#21 lore raw

Hi,

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

Thanks,
Okash

Quoted 65 lines · 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

Quoted 56 lines · Samuel Thibault, on mer. 15 mars 2017 16:03:23 +0100, wrote:
Quoted 20 lines · Rob Herring, on mer. 15 mars 2017 09:45:59 -0500, wrote:
Quoted 9 lines · Samuel Thibault
Quoted 8 lines · Samuel Thibault, on mar. 14 mars 2017 01:47:01 +0100, wrote:
Quoted 7 lines · Greg KH, on mar. 14 mars 2017 06:14:04 +0800, wrote:
Quoted 5 lines · On Mon, Mar 13, 2017 at 10:05:51PM +0000, [email protected] 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.

Quoted 8 lines
Quoted 8 lines
Quoted 2 lines

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?

Quoted 4 lines

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.

Quoted 16 lines
Quoted 16 lines
Quoted 9 lines

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

Quoted 7 lines
Quoted 5 lines

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 :)

#23 lore raw
In reply to [email protected]

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