[patch 5/6] staging: speakup: Make speakup_dummy use ttyio

[Okash Khawaja]

This changes speakup_dummy to use spk_ttyio implementations.

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

Index: linux-stable/drivers/staging/speakup/speakup_dummy.c
===================================================================
--- linux-stable.orig/drivers/staging/speakup/speakup_dummy.c
+++ linux-stable/drivers/staging/speakup/speakup_dummy.c
@@ -98,10 +98,10 @@
.startup = SYNTH_START,
.checkval = SYNTH_CHECK,
.vars = vars,
- .probe = spk_serial_synth_probe,
- .release = spk_serial_release,
- .serial_out = spk_serial_out,
- .synth_immediate = spk_synth_immediate,
+ .probe = spk_ttyio_synth_probe,
+ .release = spk_ttyio_release,
+ .serial_out = spk_ttyio_out,
+ .synth_immediate = spk_ttyio_immediate,
.catch_up = spk_do_catch_up,
.flush = spk_synth_flush,
.is_alive = spk_synth_is_alive_restart,

[Okash Khawaja]

This changes the above four 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. This also moves
spk_serial_synth_probe() into serialio.c in order to segregate those functions
which directly use serial comms into serialio.c.

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

Index: linux-stable/drivers/staging/speakup/speakup_acntsa.c
===================================================================
--- linux-stable.orig/drivers/staging/speakup/speakup_acntsa.c
+++ linux-stable/drivers/staging/speakup/speakup_acntsa.c
@@ -100,9 +100,9 @@
.checkval = SYNTH_CHECK,
.vars = vars,
.probe = synth_probe,
- .release = spk_serial_release,
- .serial_out = spk_serial_out,
- .synth_immediate = spk_synth_immediate,
+ .release = spk_ttyio_release,
+ .serial_out = spk_ttyio_out,
+ .synth_immediate = spk_ttyio_immediate,
.catch_up = spk_do_catch_up,
.flush = spk_synth_flush,
.is_alive = spk_synth_is_alive_restart,
@@ -125,9 +125,9 @@
{
int failed;

- failed = spk_serial_synth_probe(synth);
+ failed = spk_ttyio_synth_probe(synth);
if (failed == 0) {
- spk_synth_immediate(synth, "\033=R\r");
+ synth->synth_immediate(synth, "\033=R\r");
mdelay(100);
}
synth->alive = !failed;
Index: linux-stable/drivers/staging/speakup/speakup_bns.c
===================================================================
--- linux-stable.orig/drivers/staging/speakup/speakup_bns.c
+++ linux-stable/drivers/staging/speakup/speakup_bns.c
@@ -96,10 +96,10 @@
.startup = SYNTH_START,
.checkval = SYNTH_CHECK,
.vars = vars,
- .probe = spk_serial_synth_probe,
- .release = spk_serial_release,
- .serial_out = spk_serial_out,
- .synth_immediate = spk_synth_immediate,
+ .probe = spk_ttyio_synth_probe,
+ .release = spk_ttyio_release,
+ .serial_out = spk_ttyio_out,
+ .synth_immediate = spk_ttyio_immediate,
.catch_up = spk_do_catch_up,
.flush = spk_synth_flush,
.is_alive = spk_synth_is_alive_restart,
Index: linux-stable/drivers/staging/speakup/speakup_txprt.c
===================================================================
--- linux-stable.orig/drivers/staging/speakup/speakup_txprt.c
+++ linux-stable/drivers/staging/speakup/speakup_txprt.c
@@ -95,10 +95,10 @@
.startup = SYNTH_START,
.checkval = SYNTH_CHECK,
.vars = vars,
- .probe = spk_serial_synth_probe,
- .release = spk_serial_release,
- .serial_out = spk_serial_out,
- .synth_immediate = spk_synth_immediate,
+ .probe = spk_ttyio_synth_probe,
+ .release = spk_ttyio_release,
+ .serial_out = spk_ttyio_out,
+ .synth_immediate = spk_ttyio_immediate,
.catch_up = spk_do_catch_up,
.flush = spk_synth_flush,
.is_alive = spk_synth_is_alive_restart,
Index: linux-stable/drivers/staging/speakup/speakup_dectlk.c
===================================================================
--- linux-stable.orig/drivers/staging/speakup/speakup_dectlk.c
+++ linux-stable/drivers/staging/speakup/speakup_dectlk.c
@@ -130,10 +130,10 @@
.vars = vars,
.default_pitch = ap_defaults,
.default_vol = g5_defaults,
- .probe = spk_serial_synth_probe,
- .serial_out = spk_serial_out,
- .release = spk_serial_release,
- .synth_immediate = spk_synth_immediate,
+ .probe = spk_ttyio_synth_probe,
+ .serial_out = spk_ttyio_out,
+ .release = spk_ttyio_release,
+ .synth_immediate = spk_ttyio_immediate,
.catch_up = do_catch_up,
.flush = synth_flush,
.is_alive = spk_synth_is_alive_restart,
Index: linux-stable/drivers/staging/speakup/serialio.c
===================================================================
--- linux-stable.orig/drivers/staging/speakup/serialio.c
+++ linux-stable/drivers/staging/speakup/serialio.c
@@ -181,6 +181,36 @@
return 1;
}

+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);
+
+
unsigned char spk_serial_in(void)
{
int tmout = SPK_SERIAL_TIMEOUT;
Index: linux-stable/drivers/staging/speakup/synth.c
===================================================================
--- linux-stable.orig/drivers/staging/speakup/synth.c
+++ linux-stable/drivers/staging/speakup/synth.c
@@ -44,35 +44,6 @@

static int do_synth_init(struct spk_synth *in_synth);

-int spk_serial_synth_probe(struct spk_synth *synth)
-{
- const struct old_serial_port *ser;
- int failed = 0;
-
- if ((synth->ser >= SPK_LO_TTY) && (synth->ser <= SPK_HI_TTY)) {
- ser = spk_serial_init(synth->ser);
- if (ser == NULL) {
- failed = -1;
- } else {
- outb_p(0, ser->port);
- mdelay(1);
- outb_p('\r', ser->port);
- }
- } else {
- failed = -1;
- pr_warn("ttyS%i is an invalid port\n", synth->ser);
- }
- if (failed) {
- pr_info("%s: not found\n", synth->long_name);
- return -ENODEV;
- }
- pr_info("%s: ttyS%i, Driver Version %s\n",
- synth->long_name, synth->ser, synth->version);
- synth->alive = 1;
- return 0;
-}
-EXPORT_SYMBOL_GPL(spk_serial_synth_probe);
-
/*
* Main loop of the progression thread: keep eating from the buffer
* and push to the serial port, waiting as needed

[Samuel Thibault]

This also moves spk_serial_synth_probe() into serialio.c in order
to segregate those functions which directly use serial comms into
serialio.c.

Please take this into another patch too, it is unrelated.

Samuel

[Samuel Thibault]

Post by Okash Khawaja
Index: linux-stable/drivers/staging/speakup/speakup_dectlk.c
===================================================================
--- linux-stable.orig/drivers/staging/speakup/speakup_dectlk.c
+++ linux-stable/drivers/staging/speakup/speakup_dectlk.c

Actually, this one does use input, even if not explicitly, and it's
quite important actually since it's the flow control (XON/XOFF), so it
should not be included in our first round of conversion.

This is done through the read_buff_add method. Please note in your todo
for the input part, that ttyio will have to call read_buff_add for each
received character, when that method is not NULL (that's actually the
only driver using it, so we will really need a tester for this exact
driver).

Samuel

[Samuel Thibault]

Please note in your todo for the input part, that ttyio will have to
call read_buff_add for each received character, when that method is
not NULL (that's actually the only driver using it, so we will really
need a tester for this exact driver).

More precisely, it's the spk_ttyio_ldisc_ops->receive_buf2 method which
should call read_buff_add for each received character.


The step further will be implementing spk_ttyio_in and
spk_ttyio_in_nowait. I believe the easiest way is the following:

- define an spk_ldisc_data structure containing just one character (buf),
and a semaphore.

- on ldisc_open, allocate a pointer to such structure, set
tty->disc_data to point to it, and initialized the semaphore to 0
tokens.

- in the receive_buf2 method,
- if read_buff_add is defined, just call it for each character and be
done
- otherwise, store the first received character in
((struct spk_ldisc_data *)tty->disc_data)->buf
, call up() on the semaphore, and return 1 (to tell that you ate the
character).

- in spk_serial_in, call down_timeout(usecs_to_jiffies(SPK_SERIAL_TIMEOUT)),
- on success, copy the character stored in buf, then call tty_schedule_flip() and return the copy
- on failure (timed out), return 0xff.

- in spk_serial_in_nowait, call down_trylock(),
- on success, copy the character stored in buf, then call tty_schedule_flip() and return the copy
- on failure, return 0.

Samuel

[Samuel Thibault]

Hello,

Post by Samuel Thibault

Please note in your todo for the input part, that ttyio will have to
call read_buff_add for each received character, when that method is
not NULL (that's actually the only driver using it, so we will really
need a tester for this exact driver).

More precisely, it's the spk_ttyio_ldisc_ops->receive_buf2 method which
should call read_buff_add for each received character.
The step further will be implementing spk_ttyio_in and
- define an spk_ldisc_data structure containing just one character (buf),
and a semaphore.

and one int (buf_free).

Post by Samuel Thibault
- on ldisc_open, allocate a pointer to such structure, set
tty->disc_data to point to it, and initialized the semaphore to 0
tokens.

and initialize buf_free to 1.

Post by Samuel Thibault
- in the receive_buf2 method,
- if read_buff_add is defined, just call it for each character and be
done
- otherwise, store the first received character in
((struct spk_ldisc_data *)tty->disc_data)->buf
, call up() on the semaphore, and return 1 (to tell that you ate the
character).

Here, *just before* storing the character in buf, check buf_free: if
it is 0, return 0. otherwise, call mb(), and continue with what I wrote
above.

Post by Samuel Thibault
- in spk_serial_in, call down_timeout(usecs_to_jiffies(SPK_SERIAL_TIMEOUT)),
- on success, copy the character stored in buf, then call tty_schedule_flip() and return the copy
- on failure (timed out), return 0xff.
- in spk_serial_in_nowait, call down_trylock(),
- on success, copy the character stored in buf, then call tty_schedule_flip() and return the copy
- on failure, return 0.

In each case, right after the copy, call mb() and set buf_free to 1.


Actually, these two function could be factorized to just one, which
takes a long timeout parameter (in jiffies) and returns an int
instead of a char, and uses down_trylock when the timeout is 0 and
down_timeout otherwise, and returns -1 instead of 0 on failure. And then
make spk_serial_in use it and return 0xff when getting -1, and make
spk_serial_in_nowait return 0 when getting -1. Cleaning that returned
value convention can be another patch later.

Samuel

[Okash Khawaja]

Hi,

On Sun, Feb 26, 2017 at 11:07 AM, Samuel Thibault

Post by Samuel Thibault
Hello,

Post by Samuel Thibault

Please note in your todo for the input part, that ttyio will have to
call read_buff_add for each received character, when that method is
not NULL (that's actually the only driver using it, so we will really
need a tester for this exact driver).

More precisely, it's the spk_ttyio_ldisc_ops->receive_buf2 method which
should call read_buff_add for each received character.
The step further will be implementing spk_ttyio_in and
- define an spk_ldisc_data structure containing just one character (buf),
and a semaphore.

and one int (buf_free).

Post by Samuel Thibault
- on ldisc_open, allocate a pointer to such structure, set
tty->disc_data to point to it, and initialized the semaphore to 0
tokens.

and initialize buf_free to 1.

Post by Samuel Thibault
- in the receive_buf2 method,
- if read_buff_add is defined, just call it for each character and be
done
- otherwise, store the first received character in
((struct spk_ldisc_data *)tty->disc_data)->buf
, call up() on the semaphore, and return 1 (to tell that you ate the
character).

Here, *just before* storing the character in buf, check buf_free: if
above.

Post by Samuel Thibault
- in spk_serial_in, call down_timeout(usecs_to_jiffies(SPK_SERIAL_TIMEOUT)),
- on success, copy the character stored in buf, then call tty_schedule_flip() and return the copy
- on failure (timed out), return 0xff.
- in spk_serial_in_nowait, call down_trylock(),
- on success, copy the character stored in buf, then call tty_schedule_flip() and return the copy
- on failure, return 0.

In each case, right after the copy, call mb() and set buf_free to 1.
Actually, these two function could be factorized to just one, which
takes a long timeout parameter (in jiffies) and returns an int
instead of a char, and uses down_trylock when the timeout is 0 and
down_timeout otherwise, and returns -1 instead of 0 on failure. And then
make spk_serial_in use it and return 0xff when getting -1, and make
spk_serial_in_nowait return 0 when getting -1. Cleaning that returned
value convention can be another patch later.

Here's another approach using atomic_t instead of semaphore but still
employing same logic.

- spk_ldisc_data contains two members: char buf and atomic_t buf_free
- ldisc_open initialises buf_free to 1
- in receive_buf2, if read_buff_add is not defined:
- check if buf_free is zero then return zero and finish
- call mb()
- copy first character to buf in spk_ldisc_data
- call atomic_dec(&buf_free)
- return 1
- in spk_ttyio_in:
- countdown for SPK_SERIAL_TIMEOUT usecs - similar to
spk_serial_in - and check for atomic_read(&buf_free) == 0
- if timed out, return 0xff
- otherwise call mb()
- read the char stored in buf and call atomic_inc(&buf_free)
- call tty_schedule_flip()
- return the char

spk_ttyio_in_nowait will be similar and can be factorised into
spk_ttyio_in as you suggested.

Using this approach we can avoid using down_timeout() which may need
justification when merging upstream. I might be wrong here but there
seems to be a somewhat less acceptance for timeout while acquiring
locks. For example in case of mutex_lock_timeout:
http://lkml.iu.edu/hypermail/linux/kernel/0611.3/0254.html.

If however there are plans of making buf bigger than one char, then
this will require reworking as the above scheme overloads buf_free
both for mutual exclusion and to count free bytes in buffer.

Okash

[Samuel Thibault]

Hello,

Post by Okash Khawaja
- countdown for SPK_SERIAL_TIMEOUT usecs - similar to
spk_serial_in - and check for atomic_read(&buf_free) == 0

Such busy polling will be way less acceptable than down_timeout :)
And all the more so since it does not actually try to give CPU to the
part of the kernel which will provide the character, while down_timeout
exactly does that.

Post by Okash Khawaja
seems to be a somewhat less acceptance for timeout while acquiring
http://lkml.iu.edu/hypermail/linux/kernel/0611.3/0254.html.

Here we are not acquiring a lock: the down_timeout call is used to
try to consume a character, that's a very different thing than a
mutex_lock_timeout, which is indeed a very questionable thing.

Samuel

[Okash Khawaja]

On Thu, Mar 30, 2017 at 4:19 PM, Samuel Thibault

Post by Samuel Thibault
Hello,

Post by Okash Khawaja
- countdown for SPK_SERIAL_TIMEOUT usecs - similar to
spk_serial_in - and check for atomic_read(&buf_free) == 0

Such busy polling will be way less acceptable than down_timeout :)
And all the more so since it does not actually try to give CPU to the
part of the kernel which will provide the character, while down_timeout
exactly does that.

Of course. I misread the code in down_timeout() which uses spinlock
which made me think it's busy waiting.

Post by Samuel Thibault

Post by Okash Khawaja
seems to be a somewhat less acceptance for timeout while acquiring
http://lkml.iu.edu/hypermail/linux/kernel/0611.3/0254.html.

Here we are not acquiring a lock: the down_timeout call is used to
try to consume a character

Yes I can see now that we utilise waiting feature of semaphore here.

Thanks,
Okash

[Okash Khawaja]

Hi,

On Sun, Feb 26, 2017 at 2:48 AM, Samuel Thibault

Post by Samuel Thibault

Please note in your todo for the input part, that ttyio will have to
call read_buff_add for each received character, when that method is
not NULL (that's actually the only driver using it, so we will really
need a tester for this exact driver).

More precisely, it's the spk_ttyio_ldisc_ops->receive_buf2 method which
should call read_buff_add for each received character.
The step further will be implementing spk_ttyio_in and
- define an spk_ldisc_data structure containing just one character (buf),
and a semaphore.
- on ldisc_open, allocate a pointer to such structure, set
tty->disc_data to point to it, and initialized the semaphore to 0
tokens.
- in the receive_buf2 method,
- if read_buff_add is defined, just call it for each character and be
done
- otherwise, store the first received character in
((struct spk_ldisc_data *)tty->disc_data)->buf
, call up() on the semaphore, and return 1 (to tell that you ate the
character).

I don't fully understand how the return value of receive_buf2 is used
in flow control. According to Documentation/serial/tty.txt it is the
number of bytes processed by it, whereas comments on top of
tty_ldisc_receive_buf function's definition - which returns value
returned by receive_buf2 - say it is the number of bytes not
processed.

Also, is the call to tty_schedule_flip in spk_serial_in because we
returned 1 receive_buf2 so we have to manually tell it to flip buffer?

Thanks,
Okash

[Samuel Thibault]

Hello,

Post by Okash Khawaja
On Sun, Feb 26, 2017 at 2:48 AM, Samuel Thibault

Post by Samuel Thibault
- in the receive_buf2 method,
- if read_buff_add is defined, just call it for each character and be
done
- otherwise, store the first received character in
((struct spk_ldisc_data *)tty->disc_data)->buf
, call up() on the semaphore, and return 1 (to tell that you ate the
character).

I don't fully understand how the return value of receive_buf2 is used
in flow control. According to Documentation/serial/tty.txt it is the
number of bytes processed by it, whereas comments on top of
tty_ldisc_receive_buf function's definition - which returns value
returned by receive_buf2 - say it is the number of bytes not
processed.

I believe the comment is wrong: for instance n_tty_receive_buf_common
clearly returns the number of processed bytes, and users (such as
paste_selection, receive_buf from flush_to_ldisc) use it that way. You
can probably submit a patch that fixes the comment already.

Post by Okash Khawaja
Also, is the call to tty_schedule_flip in spk_serial_in because we
returned 1 receive_buf2 so we have to manually tell it to flip buffer?

(note: I meant spk_ttyio_in, of course)

It's because we only returned 1, yes, i.e. we only ate 1 character,
while there probably were more to read, but we didn't eat them, so we
have to tell the tty layer when we are ready to eat more.

Samuel

[Okash Khawaja]

On Tue, Mar 28, 2017 at 4:11 PM, Samuel Thibault

Post by Samuel Thibault
Hello,

Post by Okash Khawaja
On Sun, Feb 26, 2017 at 2:48 AM, Samuel Thibault

Post by Samuel Thibault
- in the receive_buf2 method,
- if read_buff_add is defined, just call it for each character and be
done
- otherwise, store the first received character in
((struct spk_ldisc_data *)tty->disc_data)->buf
, call up() on the semaphore, and return 1 (to tell that you ate the
character).

I don't fully understand how the return value of receive_buf2 is used
in flow control. According to Documentation/serial/tty.txt it is the
number of bytes processed by it, whereas comments on top of
tty_ldisc_receive_buf function's definition - which returns value
returned by receive_buf2 - say it is the number of bytes not
processed.

I believe the comment is wrong: for instance n_tty_receive_buf_common
clearly returns the number of processed bytes, and users (such as
paste_selection, receive_buf from flush_to_ldisc) use it that way. You
can probably submit a patch that fixes the comment already.

Post by Okash Khawaja
Also, is the call to tty_schedule_flip in spk_serial_in because we
returned 1 receive_buf2 so we have to manually tell it to flip buffer?

(note: I meant spk_ttyio_in, of course)
It's because we only returned 1, yes, i.e. we only ate 1 character,
while there probably were more to read, but we didn't eat them, so we
have to tell the tty layer when we are ready to eat more.

Right, so either we process all characters, return count (the argument
to receive_buf2) and don't bother with tty_schedule_flip, or we
process less than count and call tty_schedule_flip.

Thanks very much for explaining
Okash

[Samuel Thibault]

Post by Okash Khawaja
Right, so either we process all characters, return count (the argument
to receive_buf2) and don't bother with tty_schedule_flip, or we
process less than count and call tty_schedule_flip.

Yes.

And we don't want the former, because that'd mean buffering in our own
layer, while tty already does so for us.

Samuel

[Samuel Thibault]

You can also migrate ltlk, you just need to comment the call to
synth_interrogate() from synth_probe(). That'll disable getting the rom
version, but that's all, so it should be good enough for testing.

Samuel

[Samuel Thibault]

Concerning speakup_apollo.c, it seems its only use of outb is to control
the modem RTS line. To support that, yet another step will be:

- introduce spk_serial_tiocmset(unsigned int set, unsigned int clear),
which does:

int old = inb(speakup_info.port_tts + UART_MCR);
outb((old & ~clear) | set, speakup_info.port_tts + UART_MCR);

- introduce spk_ttyio_tiocmset(unsigned int set, unsigned int clear),
which does:

speakup_tty->ops->tiocmset(speakup_tty, set, clear);

and make them a tiocmset method of synths.

and then in speakup_apollo.c, instead of

outb(UART_MCR_DTR, speakup_info.port_tts + UART_MCR);
outb(UART_MCR_DTR | UART_MCR_RTS,
speakup_info.port_tts + UART_MCR);

rather use

synth->tiocmset(0, UART_MCR_RTS);
synth->tiocmset(UART_MCR_RTS, 0);

Samuel

[Okash Khawaja]

Hi,

Post by Samuel Thibault
Concerning speakup_apollo.c, it seems its only use of outb is to control
- introduce spk_serial_tiocmset(unsigned int set, unsigned int clear),
int old = inb(speakup_info.port_tts + UART_MCR);
outb((old & ~clear) | set, speakup_info.port_tts + UART_MCR);
- introduce spk_ttyio_tiocmset(unsigned int set, unsigned int clear),
speakup_tty->ops->tiocmset(speakup_tty, set, clear);
and make them a tiocmset method of synths.
and then in speakup_apollo.c, instead of
outb(UART_MCR_DTR, speakup_info.port_tts + UART_MCR);
outb(UART_MCR_DTR | UART_MCR_RTS,
speakup_info.port_tts + UART_MCR);

Is there a specific reason for keeping DTR set all the time? Or is it
just a defensive measure?

Post by Samuel Thibault
rather use
synth->tiocmset(0, UART_MCR_RTS);
synth->tiocmset(UART_MCR_RTS, 0);
Samuel

[Samuel Thibault]

Post by Okash Khawaja

Post by Samuel Thibault
and then in speakup_apollo.c, instead of
outb(UART_MCR_DTR, speakup_info.port_tts + UART_MCR);
outb(UART_MCR_DTR | UART_MCR_RTS,
speakup_info.port_tts + UART_MCR);

Is there a specific reason for keeping DTR set all the time? Or is it
just a defensive measure?

I guess the device requires it. That's normal RS232 signaling anyway.
With tiocmset, however, you won't have to care: the generic code will
already have enabled DTR, and you will only want to clear and set RTS.

Samuel

[Okash Khawaja]

Post by Samuel Thibault

Post by Okash Khawaja

Post by Samuel Thibault
and then in speakup_apollo.c, instead of
outb(UART_MCR_DTR, speakup_info.port_tts + UART_MCR);
outb(UART_MCR_DTR | UART_MCR_RTS,
speakup_info.port_tts + UART_MCR);

Is there a specific reason for keeping DTR set all the time? Or is it
just a defensive measure?

I guess the device requires it. That's normal RS232 signaling anyway.
With tiocmset, however, you won't have to care: the generic code will
already have enabled DTR, and you will only want to clear and set RTS.

I noticed that spk_serial_init sets it and other places keep it set. By
generic code, do you mean that when migrated to TTY, we don't have to
explicitly set DTR?

Post by Samuel Thibault
Samuel

[Samuel Thibault]

Post by Okash Khawaja

Post by Samuel Thibault

Post by Okash Khawaja

Post by Samuel Thibault
and then in speakup_apollo.c, instead of
outb(UART_MCR_DTR, speakup_info.port_tts + UART_MCR);
outb(UART_MCR_DTR | UART_MCR_RTS,
speakup_info.port_tts + UART_MCR);

Is there a specific reason for keeping DTR set all the time? Or is it
just a defensive measure?

I guess the device requires it. That's normal RS232 signaling anyway.
With tiocmset, however, you won't have to care: the generic code will
already have enabled DTR, and you will only want to clear and set RTS.

I noticed that spk_serial_init sets it and other places keep it set. By
generic code, do you mean that when migrated to TTY, we don't have to
explicitly set DTR?

Yes, it'll already be enabled by the layers below.

Samuel

[Samuel Thibault]

Concerning speakup_decext.c, I believe it should be turned into defining
a read_buff_add method, which simply sets last_char to the received
character. get_last_char() can then simply be dropped, and synth_full()
just directly read last_char.

Samuel

[Samuel Thibault]

Last but not least, there are the outb(CLEAR) in speakup_audptr.c and
speakup_spkout. I'd say the following:

- add spk_serial_send_xchar(char ch) which does

outb(ch, speakup_info.port_tts);

- add spk_ttyio_send_xchar(char ch) which does

speakup_tty->ops->send_xchar(speakup_tty, ch)

and make these a send_xchar method of synths.

Then, in speakup_audptr.c, replace the content of synth_flush() with
synth->send_xchar(SYNTH_CLEAR) + the last line unmodified (synth->serial_out(PROCSPEECH);)

and in speakup_spkout.c, replace the content of synth_flush() with just
synth->send_xchar(SYNTH_CLEAR);

and we should be done with replacing in[bwl]/out[bwl]!
(except the internal synth (acntpc, decpc, keypc, dtlk))

Samuel