Unused synth_immediate method

[Samuel Thibault]

Hello,

Okash Khawaja, on lun. 20 févr. 2017 12:23:45 +0000, wrote:
> > On 19 Feb 2017, at 23:06, Samuel Thibault <samuel.thibault at ens-lyon.org> wrote:
> > 
> > Okash Khawaja, on dim. 19 févr. 2017 10:44:36 +0000, wrote:
> >> Some drivers like apollo and bns only assign synth_immediate method in
> >> their spk_synth structs but don't seem to use it. For those drivers,
> >> can it be set to NULL?
> > 
> > I guess this method could be useful for some screen reading features,
> > just not used yet.
> 
> I see. Now because they are assigned but not used, they may cause confusion. For example, spk_synth_immediate uses outb.

Oh, I didn't realize this.

> When migrating a synth to ttyio, if synth_immediate is left assigned, it may look like that synth isn't fully migrated yet.

Indeed, and that can only lead to problems later.

> We can leave as it is although I would prefer it to be set to NULL
> until it is actually used.

Indeed.

Ideally however we'd have a ttyio variant to assign here. It shouldn't
be hard, something like this (untested):

const char *spk_ttyio_synth_immediate(struct spk_synth *synth, const char *buff)
{
	u_char ch;

	while ((ch = *buff)) {
		if (ch == '\n')
			ch = synth->procspeech;
		if (tty_write_room(speakup_tty) < 1 || !spk_ttyio_out(ch))
			return buff;
		buff++;
	}
	return NULL;
}

And the existing spk_synth_immediate should be renamed to
spk_serial_synth_immediate to make it clear it uses I/O ports.

Thinking about this, spk_ttyio_out should check the result of the write
operation, something like this:

int spk_ttyio_out(const char ch)
{
        if (synth->alive && speakup_tty && speakup_tty->ops->write) {
                int ret = speakup_tty->ops->write(speakup_tty, &ch, 1);
		if (ret == 0)
			/* No room */
			return 0;
		if (ret < 0) {
			pr_warn("%s: I/O error, deactivating speakup\n", synth->long_name);
			/* 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
			 */
			synth->alive = 0;
			speakup_start_ttys();
			return 0;
		}
                return 1;
        }
        return 0;
}

BTW, I can see that spk_serial_synth_probe uses outb too, it should be
moved to serialio.c too. In the end, we should have a situation where
only serialio.c functions use in[bwl]/out[bwl], and these functions be
prefixed with spk_serial_, so that it's easy to know which code parts
still use them.

Samuel