Discussion:
[patch 3/6] staging: speakup: refactor spk_stop_serial_interrupt
Okash Khawaja
2017-02-25 19:25:26 UTC
Permalink
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 woudn't call it.

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

Index: linux-stable/drivers/staging/speakup/serialio.c
===================================================================
--- linux-stable.orig/drivers/staging/speakup/serialio.c
+++ linux-stable/drivers/staging/speakup/serialio.c
@@ -219,6 +219,7 @@

void spk_serial_release(void)
{
+ spk_stop_serial_interrupt();
if (speakup_info.port_tts == 0)
return;
synth_release_region(speakup_info.port_tts, 8);
Index: linux-stable/drivers/staging/speakup/speakup_acntpc.c
===================================================================
--- linux-stable.orig/drivers/staging/speakup/speakup_acntpc.c
+++ linux-stable/drivers/staging/speakup/speakup_acntpc.c
@@ -303,6 +303,7 @@

static void accent_release(void)
{
+ spk_stop_serial_interrupt();
if (speakup_info.port_tts)
synth_release_region(speakup_info.port_tts-1, SYNTH_IO_EXTENT);
speakup_info.port_tts = 0;
Index: linux-stable/drivers/staging/speakup/speakup_decpc.c
===================================================================
--- linux-stable.orig/drivers/staging/speakup/speakup_decpc.c
+++ linux-stable/drivers/staging/speakup/speakup_decpc.c
@@ -482,6 +482,7 @@

static void dtpc_release(void)
{
+ spk_stop_serial_interrupt();
if (speakup_info.port_tts)
synth_release_region(speakup_info.port_tts, SYNTH_IO_EXTENT);
speakup_info.port_tts = 0;
Index: linux-stable/drivers/staging/speakup/speakup_dtlk.c
===================================================================
--- linux-stable.orig/drivers/staging/speakup/speakup_dtlk.c
+++ linux-stable/drivers/staging/speakup/speakup_dtlk.c
@@ -374,6 +374,7 @@

static void dtlk_release(void)
{
+ spk_stop_serial_interrupt();
if (speakup_info.port_tts)
synth_release_region(speakup_info.port_tts-1, SYNTH_IO_EXTENT);
speakup_info.port_tts = 0;
Index: linux-stable/drivers/staging/speakup/speakup_keypc.c
===================================================================
--- linux-stable.orig/drivers/staging/speakup/speakup_keypc.c
+++ linux-stable/drivers/staging/speakup/speakup_keypc.c
@@ -305,6 +305,7 @@

static void keynote_release(void)
{
+ spk_stop_serial_interrupt();
if (synth_port)
synth_release_region(synth_port, SYNTH_IO_EXTENT);
synth_port = 0;
Index: linux-stable/drivers/staging/speakup/synth.c
===================================================================
--- linux-stable.orig/drivers/staging/speakup/synth.c
+++ linux-stable/drivers/staging/speakup/synth.c
@@ -432,7 +432,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;
}
Samuel Thibault
2017-02-25 23:03:34 UTC
Permalink
Post by Okash Khawaja
This moves call to spk_stop_serial_interrupt() function out of synth_release()
and into release() method of specific spk_synth instances.
Did you try to compile this as a module? It seems this is missing an
EXPORT_SYMBOL_GPL line for modules to be able to call it.

I'm now having a try, will report later.

Samuel
Okash Khawaja
2017-02-26 15:11:54 UTC
Permalink
Hi,
Post by Samuel Thibault
Post by Okash Khawaja
This moves call to spk_stop_serial_interrupt() function out of synth_release()
and into release() method of specific spk_synth instances.
Did you try to compile this as a module? It seems this is missing an
EXPORT_SYMBOL_GPL line for modules to be able to call it.
It should need EXPORT_SYMBOL_GPL for other synths to compile as
modules but I don't get a linker warning when compiling them as
modules. Am I missing something here?
Post by Samuel Thibault
I'm now having a try, will report later.
Samuel Thibault
2017-02-26 15:25:30 UTC
Permalink
Post by Okash Khawaja
Post by Samuel Thibault
Post by Okash Khawaja
This moves call to spk_stop_serial_interrupt() function out of synth_release()
and into release() method of specific spk_synth instances.
Did you try to compile this as a module? It seems this is missing an
EXPORT_SYMBOL_GPL line for modules to be able to call it.
It should need EXPORT_SYMBOL_GPL for other synths to compile as
modules
Yes.
Post by Okash Khawaja
but I don't get a linker warning when compiling them as modules. Am I
missing something here?
I don't know, perhaps some config option about modules.

What I'm sure of is that it's needed at least for some configurations :)

Samuel

Samuel Thibault
2017-02-26 00:38:09 UTC
Permalink
+static int spk_ttyio_initialise_ldisc(void)
Add an "int ser" parameter here.
+{
+ int ret = 0;
+ struct tty_struct *tty;
+
+ ret = tty_register_ldisc(N_SPEAKUP, &spk_ttyio_ldisc_ops);
+ if (ret) {
+ pr_err("speakup_test_init(): Error registering line discipline.\n");
+ return ret;
+ }
+
+ tty = tty_open_by_driver(MKDEV(4, 64), NULL, NULL);
Here, check that ser is between 0 and (255-64), and add ser to 64.
+int spk_ttyio_synth_probe(struct spk_synth *synth)
+{
+ int rv = spk_ttyio_initialise_ldisc();
Here, pass synth->ser to spk_ttyio_initialise_ldisc()


That way, people want easily configure the serial port with the existing
ser parameter, e.g. speakup_dummy.ser=1

and AIUI we should be getting the same support as before, and thus get
this tested and pushed upstream.

Samuel
Samuel Thibault
2017-02-26 01:19:46 UTC
Permalink
+const char *spk_ttyio_immediate(struct spk_synth *synth, const char *buff)
Rather call it spk_ttyio_synth_immediate, for coherency.

Samuel
Loading...