public inbox for speakup@linux-speakup.org
 help / color / mirror / Atom feed
* [patch 3/7] staging: serial: add spk_io_ops struct to spk_synth
@  okash.khawaja
   ` Samuel Thibault
  0 siblings, 1 reply; 8+ messages in thread
From: okash.khawaja @  UTC (permalink / raw)
  To: samuel.thibault; +Cc: speakup, Okash Khawaja

[-- Attachment #1: 03_add_spk_io_ops --]
[-- Type: text/plain, Size: 15327 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>

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 serial_io_ops = {
+	.synth_out = spk_serial_out,
+};
+EXPORT_SYMBOL_GPL(serial_io_ops);
+
 const struct old_serial_port *spk_serial_init(int index)
 {
 	int baud = 9600, quot = 0;
@@ -130,6 +136,15 @@
 	outb(1, speakup_info.port_tts + UART_FCR);	/* Turn FIFO On */
 }
 
+static int spk_serial_out(struct spk_synth *in_synth, const char ch)
+{
+	if (in_synth->alive && spk_wait_for_xmitr(in_synth)) {
+		outb_p(ch, speakup_info.port_tts);
+		return 1;
+	}
+	return 0;
+}
+
 void spk_stop_serial_interrupt(void)
 {
 	if (speakup_info.port_tts == 0)
@@ -207,16 +222,6 @@
 }
 EXPORT_SYMBOL_GPL(spk_serial_in_nowait);
 
-int spk_serial_out(struct spk_synth *in_synth, const char ch)
-{
-	if (in_synth->alive && spk_wait_for_xmitr(in_synth)) {
-		outb_p(ch, speakup_info.port_tts);
-		return 1;
-	}
-	return 0;
-}
-EXPORT_SYMBOL_GPL(spk_serial_out);
-
 void spk_serial_release(void)
 {
 	if (speakup_info.port_tts == 0)
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 = &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 = &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 = &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 = &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 = &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 = &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 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 = &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 = &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 = &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 = &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 = &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 = &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 = &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 = &serial_io_ops,
 	.probe = spk_serial_synth_probe,
 	.release = spk_serial_release,
 	.synth_immediate = spk_synth_immediate,


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

* Re: [patch 3/7] staging: serial: add spk_io_ops struct to spk_synth
   [patch 3/7] staging: serial: add spk_io_ops struct to spk_synth okash.khawaja
@  ` Samuel Thibault
     ` Okash Khawaja
  0 siblings, 1 reply; 8+ messages in thread
From: Samuel Thibault @  UTC (permalink / raw)
  To: okash.khawaja; +Cc: speakup

okash.khawaja@gmail.com, on sam. 11 mars 2017 13:05:27 +0000, wrote:
> This patch adds spk_io_ops struct which contain those methods whose job is to
> +static int spk_serial_out(struct spk_synth *in_synth, const char ch);
> +struct spk_io_ops serial_io_ops = {
> +	.synth_out = spk_serial_out,
> +};
> +EXPORT_SYMBOL_GPL(serial_io_ops);

Prefix it with spk_, to avoid naming clashes with other drivers.

> @@ -130,6 +136,15 @@
>  	outb(1, speakup_info.port_tts + UART_FCR);	/* Turn FIFO On */
>  }
>  
> +static int spk_serial_out(struct spk_synth *in_synth, const char ch)
> +{
> +	if (in_synth->alive && spk_wait_for_xmitr(in_synth)) {
> +		outb_p(ch, speakup_info.port_tts);
> +		return 1;
> +	}
> +	return 0;
> +}

Don't unnecessarily move the code, only drop the export symbol.

> -EXPORT_SYMBOL_GPL(spk_serial_out);



Apart from that, it's clean and nice :)

Samuel

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

* Re: [patch 3/7] staging: serial: add spk_io_ops struct to spk_synth
   ` Samuel Thibault
@    ` Okash Khawaja
       ` Samuel Thibault
  0 siblings, 1 reply; 8+ messages in thread
From: Okash Khawaja @  UTC (permalink / raw)
  To: Samuel Thibault; +Cc: speakup

Hi,

On Sun, Mar 12, 2017 at 03:12:41AM +0100, Samuel Thibault wrote:
> okash.khawaja@gmail.com, on sam. 11 mars 2017 13:05:27 +0000, wrote:
> > This patch adds spk_io_ops struct which contain those methods whose job is to
> > +static int spk_serial_out(struct spk_synth *in_synth, const char ch);
> > +struct spk_io_ops serial_io_ops = {
> > +	.synth_out = spk_serial_out,
> > +};
> > +EXPORT_SYMBOL_GPL(serial_io_ops);
> 
> Prefix it with spk_, to avoid naming clashes with other drivers.
> 
> > @@ -130,6 +136,15 @@
> >  	outb(1, speakup_info.port_tts + UART_FCR);	/* Turn FIFO On */
> >  }
> >  
> > +static int spk_serial_out(struct spk_synth *in_synth, const char ch)
> > +{
> > +	if (in_synth->alive && spk_wait_for_xmitr(in_synth)) {
> > +		outb_p(ch, speakup_info.port_tts);
> > +		return 1;
> > +	}
> > +	return 0;
> > +}
> 
> Don't unnecessarily move the code, only drop the export symbol.

Since spk_serial_out is not more exported, I made it static and moved it
closer to other static functions which are all towards the top of the
file. Perhaps I should put this in description?

> 
> > -EXPORT_SYMBOL_GPL(spk_serial_out);
> 
> 
> 
> Apart from that, it's clean and nice :)
> 
> Samuel

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

* Re: [patch 3/7] staging: serial: add spk_io_ops struct to spk_synth
     ` Okash Khawaja
@      ` Samuel Thibault
         ` Okash Khawaja
         ` Okash Khawaja
  0 siblings, 2 replies; 8+ messages in thread
From: Samuel Thibault @  UTC (permalink / raw)
  To: Okash Khawaja; +Cc: speakup

Okash Khawaja, on dim. 12 mars 2017 09:32:25 +0000, wrote:
> > Don't unnecessarily move the code, only drop the export symbol.
> 
> Since spk_serial_out is not more exported, I made it static and moved it
> closer to other static functions which are all towards the top of the
> file.

There is no need for this :)

Samuel

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

* Re: [patch 3/7] staging: serial: add spk_io_ops struct to spk_synth
       ` Samuel Thibault
@        ` Okash Khawaja
         ` Okash Khawaja
  1 sibling, 0 replies; 8+ messages in thread
From: Okash Khawaja @  UTC (permalink / raw)
  To: Samuel Thibault; +Cc: speakup

On Sun, Mar 12, 2017 at 11:33:32AM +0100, Samuel Thibault wrote:
> Okash Khawaja, on dim. 12 mars 2017 09:32:25 +0000, wrote:
> > > Don't unnecessarily move the code, only drop the export symbol.
> > 
> > Since spk_serial_out is not more exported, I made it static and moved it
> > closer to other static functions which are all towards the top of the
> > file.
> 
> There is no need for this :)

Okay, I'll leave it as it is and just remove the export.

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

* Re: [patch 3/7] staging: serial: add spk_io_ops struct to spk_synth
       ` Samuel Thibault
         ` Okash Khawaja
@        ` Okash Khawaja
           ` Samuel Thibault
  1 sibling, 1 reply; 8+ messages in thread
From: Okash Khawaja @  UTC (permalink / raw)
  To: Samuel Thibault; +Cc: speakup

On Sun, Mar 12, 2017 at 11:33:32AM +0100, Samuel Thibault wrote:
> Okash Khawaja, on dim. 12 mars 2017 09:32:25 +0000, wrote:
> > > Don't unnecessarily move the code, only drop the export symbol.
> > 
> > Since spk_serial_out is not more exported, I made it static and moved it
> > closer to other static functions which are all towards the top of the
> > file.
> 
> There is no need for this :)

Ah I see, utlimately we plan to completely replace serialio with ttyio.

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

* Re: [patch 3/7] staging: serial: add spk_io_ops struct to spk_synth
         ` Okash Khawaja
@          ` Samuel Thibault
  0 siblings, 0 replies; 8+ messages in thread
From: Samuel Thibault @  UTC (permalink / raw)
  To: Okash Khawaja; +Cc: speakup

Okash Khawaja, on dim. 12 mars 2017 11:52:02 +0000, wrote:
> On Sun, Mar 12, 2017 at 11:33:32AM +0100, Samuel Thibault wrote:
> > Okash Khawaja, on dim. 12 mars 2017 09:32:25 +0000, wrote:
> > > > Don't unnecessarily move the code, only drop the export symbol.
> > > 
> > > Since spk_serial_out is not more exported, I made it static and moved it
> > > closer to other static functions which are all towards the top of the
> > > file.
> > 
> > There is no need for this :)
> 
> Ah I see, utlimately we plan to completely replace serialio with ttyio.

Well, serialio might remain for some time, but anyway shuffling code
around in a sensitive patch series doesn't help getting it accepted :)

Samuel

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

* [patch 3/7] staging: serial: add spk_io_ops struct to spk_synth
   [patch 0/7] staging: speakup: introduce tty-based comms okash.khawaja
@  ` okash.khawaja
  0 siblings, 0 replies; 8+ messages in thread
From: okash.khawaja @  UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby, Samuel Thibault, linux-kernel
  Cc: William Hubbs, Chris Brannon, Kirk Reiser, speakup, devel, Okash Khawaja

[-- Attachment #1: 03_add_spk_io_ops --]
[-- Type: text/plain, Size: 14833 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,


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

end of thread, other threads:[~ UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
 [patch 3/7] staging: serial: add spk_io_ops struct to spk_synth okash.khawaja
 ` Samuel Thibault
   ` Okash Khawaja
     ` Samuel Thibault
       ` Okash Khawaja
       ` Okash Khawaja
         ` Samuel Thibault
 [patch 0/7] staging: speakup: introduce tty-based comms okash.khawaja
 ` [patch 3/7] staging: serial: add spk_io_ops struct to spk_synth okash.khawaja

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