[PATCH v2 0/4] staging: speakup: refactor to make room for serial comms changes

[Okash Khawaja]

Hi,

This set of patches introduces serial_out() method which will point to
spk_serial_out() for other drivers and spk_serial_out_tty() for speakup_dummy.
Intention is to allow changing implementation of serial_out() for speakup_dummy
while allowing other drivers to continue unaffected. spk_serial_out_tty() will
use tty_struct instance instead of outb.

Thanks,
Okash

[Okash Khawaja]

This adds serial_out() method to spk_synth struct.

Signed-off-by: Okash Khawaja <***@gmail.com>
---
drivers/staging/speakup/spk_types.h | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/staging/speakup/spk_types.h b/drivers/staging/speakup/spk_types.h
index e8ff5d7..56dc7df 100644
--- a/drivers/staging/speakup/spk_types.h
+++ b/drivers/staging/speakup/spk_types.h
@@ -168,6 +168,7 @@ struct spk_synth {
int *default_vol;
int (*probe)(struct spk_synth *synth);
void (*release)(void);
+ int (*serial_out)(const char ch);
const char *(*synth_immediate)(struct spk_synth *synth,
const char *buff);
void (*catch_up)(struct spk_synth *synth);
--
2.10.2

[Okash Khawaja]

spk_serial_out_tty() simply delegates to spk_serial_out() for now.

Signed-off-by: Okash Khawaja <***@gmail.com>
---
drivers/staging/speakup/serialio.c | 6 ++++++
drivers/staging/speakup/spk_priv.h | 1 +
2 files changed, 7 insertions(+)

diff --git a/drivers/staging/speakup/serialio.c b/drivers/staging/speakup/serialio.c
index c2c435c..5110822 100644
--- a/drivers/staging/speakup/serialio.c
+++ b/drivers/staging/speakup/serialio.c
@@ -217,6 +217,12 @@ int spk_serial_out(const char ch)
}
EXPORT_SYMBOL_GPL(spk_serial_out);

+int spk_serial_out_tty(const char ch)
+{
+ return spk_serial_out(ch);
+}
+EXPORT_SYMBOL_GPL(spk_serial_out_tty);
+
void spk_serial_release(void)
{
if (speakup_info.port_tts == 0)
diff --git a/drivers/staging/speakup/spk_priv.h b/drivers/staging/speakup/spk_priv.h
index 98c4b6f..bd85135 100644
--- a/drivers/staging/speakup/spk_priv.h
+++ b/drivers/staging/speakup/spk_priv.h
@@ -46,6 +46,7 @@ int spk_wait_for_xmitr(void);
unsigned char spk_serial_in(void);
unsigned char spk_serial_in_nowait(void);
int spk_serial_out(const char ch);
+int spk_serial_out_tty(const char ch);
void spk_serial_release(void);

char synth_buffer_getc(void);
--
2.10.2

[Okash Khawaja]

Initialise serial_out() to spk_serial_out() in all drivers except
speakup_dummy which uses spk_serial_out_tty() instead.

Signed-off-by: Okash Khawaja <***@gmail.com>
---
drivers/staging/speakup/speakup_acntpc.c | 1 +
drivers/staging/speakup/speakup_acntsa.c | 1 +
drivers/staging/speakup/speakup_apollo.c | 1 +
drivers/staging/speakup/speakup_audptr.c | 1 +
drivers/staging/speakup/speakup_bns.c | 1 +
drivers/staging/speakup/speakup_decext.c | 1 +
drivers/staging/speakup/speakup_decpc.c | 1 +
drivers/staging/speakup/speakup_dectlk.c | 1 +
drivers/staging/speakup/speakup_dtlk.c | 1 +
drivers/staging/speakup/speakup_dummy.c | 1 +
drivers/staging/speakup/speakup_keypc.c | 1 +
drivers/staging/speakup/speakup_ltlk.c | 1 +
drivers/staging/speakup/speakup_soft.c | 1 +
drivers/staging/speakup/speakup_spkout.c | 1 +
drivers/staging/speakup/speakup_txprt.c | 1 +
15 files changed, 15 insertions(+)

diff --git a/drivers/staging/speakup/speakup_acntpc.c b/drivers/staging/speakup/speakup_acntpc.c
index efb791b..22557e4 100644
--- a/drivers/staging/speakup/speakup_acntpc.c
+++ b/drivers/staging/speakup/speakup_acntpc.c
@@ -115,6 +115,7 @@ static struct spk_synth synth_acntpc = {
.vars = vars,
.probe = synth_probe,
.release = accent_release,
+ .serial_out = spk_serial_out,
.synth_immediate = synth_immediate,
.catch_up = do_catch_up,
.flush = synth_flush,
diff --git a/drivers/staging/speakup/speakup_acntsa.c b/drivers/staging/speakup/speakup_acntsa.c
index 34f45d3..3b18cb9 100644
--- a/drivers/staging/speakup/speakup_acntsa.c
+++ b/drivers/staging/speakup/speakup_acntsa.c
@@ -101,6 +101,7 @@ static struct spk_synth synth_acntsa = {
.vars = vars,
.probe = synth_probe,
.release = spk_serial_release,
+ .serial_out = spk_serial_out,
.synth_immediate = spk_synth_immediate,
.catch_up = spk_do_catch_up,
.flush = spk_synth_flush,
diff --git a/drivers/staging/speakup/speakup_apollo.c b/drivers/staging/speakup/speakup_apollo.c
index 3cbc8a7..47db548 100644
--- a/drivers/staging/speakup/speakup_apollo.c
+++ b/drivers/staging/speakup/speakup_apollo.c
@@ -110,6 +110,7 @@ static struct spk_synth synth_apollo = {
.vars = vars,
.probe = spk_serial_synth_probe,
.release = spk_serial_release,
+ .serial_out = spk_serial_out,
.synth_immediate = spk_synth_immediate,
.catch_up = do_catch_up,
.flush = spk_synth_flush,
diff --git a/drivers/staging/speakup/speakup_audptr.c b/drivers/staging/speakup/speakup_audptr.c
index 7a12b84..1714b7d 100644
--- a/drivers/staging/speakup/speakup_audptr.c
+++ b/drivers/staging/speakup/speakup_audptr.c
@@ -106,6 +106,7 @@ static struct spk_synth synth_audptr = {
.vars = vars,
.probe = synth_probe,
.release = spk_serial_release,
+ .serial_out = spk_serial_out,
.synth_immediate = spk_synth_immediate,
.catch_up = spk_do_catch_up,
.flush = synth_flush,
diff --git a/drivers/staging/speakup/speakup_bns.c b/drivers/staging/speakup/speakup_bns.c
index 570f0c2..e9cbcfc 100644
--- a/drivers/staging/speakup/speakup_bns.c
+++ b/drivers/staging/speakup/speakup_bns.c
@@ -98,6 +98,7 @@ static struct spk_synth synth_bns = {
.vars = vars,
.probe = spk_serial_synth_probe,
.release = spk_serial_release,
+ .serial_out = spk_serial_out,
.synth_immediate = spk_synth_immediate,
.catch_up = spk_do_catch_up,
.flush = spk_synth_flush,
diff --git a/drivers/staging/speakup/speakup_decext.c b/drivers/staging/speakup/speakup_decext.c
index 1a5cf3d..098d079 100644
--- a/drivers/staging/speakup/speakup_decext.c
+++ b/drivers/staging/speakup/speakup_decext.c
@@ -129,6 +129,7 @@ static struct spk_synth synth_decext = {
.vars = vars,
.probe = spk_serial_synth_probe,
.release = spk_serial_release,
+ .serial_out = spk_serial_out,
.synth_immediate = spk_synth_immediate,
.catch_up = do_catch_up,
.flush = synth_flush,
diff --git a/drivers/staging/speakup/speakup_decpc.c b/drivers/staging/speakup/speakup_decpc.c
index d6479bd..7fc956a 100644
--- a/drivers/staging/speakup/speakup_decpc.c
+++ b/drivers/staging/speakup/speakup_decpc.c
@@ -222,6 +222,7 @@ static struct spk_synth synth_dec_pc = {
.vars = vars,
.probe = synth_probe,
.release = dtpc_release,
+ .serial_out = spk_serial_out,
.synth_immediate = synth_immediate,
.catch_up = do_catch_up,
.flush = synth_flush,
diff --git a/drivers/staging/speakup/speakup_dectlk.c b/drivers/staging/speakup/speakup_dectlk.c
index 7646567..9bdf2ea 100644
--- a/drivers/staging/speakup/speakup_dectlk.c
+++ b/drivers/staging/speakup/speakup_dectlk.c
@@ -132,6 +132,7 @@ static struct spk_synth synth_dectlk = {
.default_vol = g5_defaults,
.probe = spk_serial_synth_probe,
.release = spk_serial_release,
+ .serial_out = spk_serial_out,
.synth_immediate = spk_synth_immediate,
.catch_up = do_catch_up,
.flush = synth_flush,
diff --git a/drivers/staging/speakup/speakup_dtlk.c b/drivers/staging/speakup/speakup_dtlk.c
index 38aa401..47768cd 100644
--- a/drivers/staging/speakup/speakup_dtlk.c
+++ b/drivers/staging/speakup/speakup_dtlk.c
@@ -130,6 +130,7 @@ static struct spk_synth synth_dtlk = {
.vars = vars,
.probe = synth_probe,
.release = dtlk_release,
+ .serial_out = spk_serial_out,
.synth_immediate = synth_immediate,
.catch_up = do_catch_up,
.flush = synth_flush,
diff --git a/drivers/staging/speakup/speakup_dummy.c b/drivers/staging/speakup/speakup_dummy.c
index 87d2a80..37e03d0 100644
--- a/drivers/staging/speakup/speakup_dummy.c
+++ b/drivers/staging/speakup/speakup_dummy.c
@@ -100,6 +100,7 @@ static struct spk_synth synth_dummy = {
.vars = vars,
.probe = spk_serial_synth_probe,
.release = spk_serial_release,
+ .serial_out = spk_serial_out_tty,
.synth_immediate = spk_synth_immediate,
.catch_up = spk_do_catch_up,
.flush = spk_synth_flush,
diff --git a/drivers/staging/speakup/speakup_keypc.c b/drivers/staging/speakup/speakup_keypc.c
index 5e2170b..e7c5c1c 100644
--- a/drivers/staging/speakup/speakup_keypc.c
+++ b/drivers/staging/speakup/speakup_keypc.c
@@ -107,6 +107,7 @@ static struct spk_synth synth_keypc = {
.vars = vars,
.probe = synth_probe,
.release = keynote_release,
+ .serial_out = spk_serial_out,
.synth_immediate = synth_immediate,
.catch_up = do_catch_up,
.flush = synth_flush,
diff --git a/drivers/staging/speakup/speakup_ltlk.c b/drivers/staging/speakup/speakup_ltlk.c
index b474e8b..82a995c 100644
--- a/drivers/staging/speakup/speakup_ltlk.c
+++ b/drivers/staging/speakup/speakup_ltlk.c
@@ -113,6 +113,7 @@ static struct spk_synth synth_ltlk = {
.vars = vars,
.probe = synth_probe,
.release = spk_serial_release,
+ .serial_out = spk_serial_out,
.synth_immediate = spk_synth_immediate,
.catch_up = spk_do_catch_up,
.flush = spk_synth_flush,
diff --git a/drivers/staging/speakup/speakup_soft.c b/drivers/staging/speakup/speakup_soft.c
index 6b1d0f5..f364ac1 100644
--- a/drivers/staging/speakup/speakup_soft.c
+++ b/drivers/staging/speakup/speakup_soft.c
@@ -133,6 +133,7 @@ static struct spk_synth synth_soft = {
.vars = vars,
.probe = softsynth_probe,
.release = softsynth_release,
+ .serial_out = NULL,
.synth_immediate = NULL,
.catch_up = NULL,
.flush = NULL,
diff --git a/drivers/staging/speakup/speakup_spkout.c b/drivers/staging/speakup/speakup_spkout.c
index e449f27..90384dd 100644
--- a/drivers/staging/speakup/speakup_spkout.c
+++ b/drivers/staging/speakup/speakup_spkout.c
@@ -105,6 +105,7 @@ static struct spk_synth synth_spkout = {
.vars = vars,
.probe = spk_serial_synth_probe,
.release = spk_serial_release,
+ .serial_out = spk_serial_out,
.synth_immediate = spk_synth_immediate,
.catch_up = spk_do_catch_up,
.flush = synth_flush,
diff --git a/drivers/staging/speakup/speakup_txprt.c b/drivers/staging/speakup/speakup_txprt.c
index fd98d4f..2c322f7 100644
--- a/drivers/staging/speakup/speakup_txprt.c
+++ b/drivers/staging/speakup/speakup_txprt.c
@@ -98,6 +98,7 @@ static struct spk_synth synth_txprt = {
.vars = vars,
.probe = spk_serial_synth_probe,
.release = spk_serial_release,
+ .serial_out = spk_serial_out,
.synth_immediate = spk_synth_immediate,
.catch_up = spk_do_catch_up,
.flush = spk_synth_flush,
--
2.10.2

[Okash Khawaja]

This replaces calls to spk_serial_out() in spk_do_catch_up() and spk_synth_flush()
with calls to serial_out(). spk_do_catch_up() and spk_synth_flush() are the only
functions through which speakup_dummy calls into spk_serial_out(). Calls to
spk_serial_out() in other drivers can stay for now.

Signed-off-by: Okash Khawaja <***@gmail.com>
---
drivers/staging/speakup/synth.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/speakup/synth.c b/drivers/staging/speakup/synth.c
index 4f462c3..cef6591 100644
--- a/drivers/staging/speakup/synth.c
+++ b/drivers/staging/speakup/synth.c
@@ -118,7 +118,7 @@ void spk_do_catch_up(struct spk_synth *synth)
spin_unlock_irqrestore(&speakup_info.spinlock, flags);
if (ch == '\n')
ch = synth->procspeech;
- if (!spk_serial_out(ch)) {
+ if (!synth->serial_out(ch)) {
schedule_timeout(msecs_to_jiffies(full_time_val));
continue;
}
@@ -128,7 +128,7 @@ void spk_do_catch_up(struct spk_synth *synth)
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 (synth->serial_out(synth->procspeech))
schedule_timeout(
msecs_to_jiffies(delay_time_val));
else
@@ -141,7 +141,7 @@ void spk_do_catch_up(struct spk_synth *synth)
synth_buffer_getc();
spin_unlock_irqrestore(&speakup_info.spinlock, flags);
}
- spk_serial_out(synth->procspeech);
+ synth->serial_out(synth->procspeech);
}
EXPORT_SYMBOL_GPL(spk_do_catch_up);

@@ -164,7 +164,7 @@ EXPORT_SYMBOL_GPL(spk_synth_immediate);

void spk_synth_flush(struct spk_synth *synth)
{
- spk_serial_out(synth->clear);
+ synth->serial_out(synth->clear);
}
EXPORT_SYMBOL_GPL(spk_synth_flush);

--
2.10.2

[Samuel Thibault]

Hello,

Just looking over the patch, it looks good. I don't think it needs to be
split in 4 pieces though, I'd say merge patches 1, 3, and 4 together,
where patch 3 would be modified to make dummy use spk_serial_out too, to
get a patch which does just one complete thing: make serial_out a
method.

Then you'll have other patches introducing spk_serial_out_tty and making
some drivers use it.

Also note that as Greg said, the patches you have just submitted do
not really make sense alone. I don't know if staging people feel like
applying them while it's only pavement for future work. If so, then
good; otherwise, then fine too: it's good that you have sent them so
that we could check how they look like, I just wanted to let you know
that they might not get applied yet, just because one may want to see
the future patches before applying what you have done so far.

BTW, I have been looking at functions again, we'll also need
a spk_synth_immediate_tty that drivers can use instead of
spk_synth_immediate, in addition to the _in, and _out functions. For now
(i.e. to make the dummy driver work, and probably a few simple more),
the tty versions for _out and _synth_immediate will probably enough to
get something working and commitable to the main kernel.

Also, as mentioned, now the hard work is getting to open the tty from
the kernel :) That'd be in a spk_tty_synth_probe function that drivers
would use instead of spk_serial_synth_probe.

Samuel

[Okash Khawaja]

On Sat, Nov 19, 2016 at 10:03 PM, Samuel Thibault <

Post by Samuel Thibault
Hello,
Just looking over the patch, it looks good. I don't think it needs to be
split in 4 pieces though, I'd say merge patches 1, 3, and 4 together,
where patch 3 would be modified to make dummy use spk_serial_out too, to
get a patch which does just one complete thing: make serial_out a
method.
Then you'll have other patches introducing spk_serial_out_tty and making
some drivers use it.
Also note that as Greg said, the patches you have just submitted do
not really make sense alone. I don't know if staging people feel like
applying them while it's only pavement for future work. If so, then
good; otherwise, then fine too: it's good that you have sent them so
that we could check how they look like, I just wanted to let you know
that they might not get applied yet, just because one may want to see
the future patches before applying what you have done so far.

Yeah makes sense. Perhaps should have asked around beforehand, as my
experience for anything other than trivial patches is non-existent.

Post by Samuel Thibault
BTW, I have been looking at functions again, we'll also need
a spk_synth_immediate_tty that drivers can use instead of
spk_synth_immediate, in addition to the _in, and _out functions. For now
(i.e. to make the dummy driver work, and probably a few simple more),
the tty versions for _out and _synth_immediate will probably enough to
get something working and commitable to the main kernel.

Good point, will add tty version of spk_synth_immediate.

Post by Samuel Thibault
Also, as mentioned, now the hard work is getting to open the tty from
the kernel :) That'd be in a spk_tty_synth_probe function that drivers
would use instead of spk_serial_synth_probe.
Samuel

Thanks,
Okash

[Okash Khawaja]

Post by Samuel Thibault
Hello,
Just looking over the patch, it looks good. I don't think it needs to be
split in 4 pieces though, I'd say merge patches 1, 3, and 4 together,
where patch 3 would be modified to make dummy use spk_serial_out too, to
get a patch which does just one complete thing: make serial_out a
method.
Then you'll have other patches introducing spk_serial_out_tty and making
some drivers use it.
Also note that as Greg said, the patches you have just submitted do
not really make sense alone. I don't know if staging people feel like
applying them while it's only pavement for future work. If so, then
good; otherwise, then fine too: it's good that you have sent them so
that we could check how they look like, I just wanted to let you know
that they might not get applied yet, just because one may want to see
the future patches before applying what you have done so far.

I would like to see the full patch series that uses these changes as
right now these patches do nothing on their own, so I don't want to
apply them.
thanks,
greg k-h

Sure, will send complete patch set.
Thanks,
Okash