speakup crashes with kernel 5.4.69

Samuel Thibault samuel.thibault at ens-lyon.org
Sun Nov 1 15:59:09 EST 2020


John Covici, le mar. 13 oct. 2020 04:02:09 -0400, a ecrit:
> Hi.  When I just tried to boot with kernel 5.4.69, it did come up, but
> soon crashed with a lot of kernel: basd schedule from idle thread.

Ok, I see the concern. Would you be able to try the attached patch?

Samuel
-------------- next part --------------
speakup ttyio: Do not buffer by default

The old serialio code was assuming that the inb would return the
last speech index.  We should thus not actually buffer characters by
default, and just always keep the last character.  When a driver is
reading a ROM version, however, we need to buffer characters.  We just
need to make drivers explicitly tell when they need buffering or not.

Signed-off-by: Samuel Thibault <samuel.thibault at ens-lyon.org>

---
 drivers/staging/speakup/serialio.c       |    7 +++
 drivers/staging/speakup/speakup_audptr.c |    2 
 drivers/staging/speakup/speakup_ltlk.c   |    2 
 drivers/staging/speakup/spk_ttyio.c      |   53 +++++++++++++++++++------
 drivers/staging/speakup/spk_types.h      |   14 ++++++
 5 files changed, 67 insertions(+), 11 deletions(-)

--- a/drivers/staging/speakup/speakup_audptr.c
+++ b/drivers/staging/speakup/speakup_audptr.c
@@ -129,6 +129,7 @@ static void synth_version(struct spk_syn
 	unsigned char test = 0;
 	char synth_id[40] = "";
 
+	synth->io_ops->bufferize(1);
 	synth->synth_immediate(synth, "\x05[Q]");
 	synth_id[test] = synth->io_ops->synth_in();
 	if (synth_id[test] == 'A') {
@@ -140,6 +141,7 @@ static void synth_version(struct spk_syn
 	}
 	if (synth_id[0] == 'A')
 		pr_info("%s version: %s", synth->long_name, synth_id);
+	synth->io_ops->bufferize(0);
 }
 
 static int synth_probe(struct spk_synth *synth)
--- a/drivers/staging/speakup/speakup_ltlk.c
+++ b/drivers/staging/speakup/speakup_ltlk.c
@@ -130,6 +130,7 @@ static void synth_interrogate(struct spk
 	unsigned char *t, i;
 	unsigned char buf[50], rom_v[20];
 
+	synth->io_ops->bufferize(1);
 	synth->synth_immediate(synth, "\x18\x01?");
 	for (i = 0; i < 50; i++) {
 		buf[i] = synth->io_ops->synth_in();
@@ -144,6 +145,7 @@ static void synth_interrogate(struct spk
 	}
 	rom_v[i] = 0;
 	pr_info("%s: ROM version: %s\n", synth->long_name, rom_v);
+	synth->io_ops->bufferize(0);
 }
 
 static int synth_probe(struct spk_synth *synth)
--- a/drivers/staging/speakup/spk_ttyio.c
+++ b/drivers/staging/speakup/spk_ttyio.c
@@ -11,6 +11,7 @@
 struct spk_ldisc_data {
 	char buf;
 	struct completion completion;
+	bool bufferize;
 	bool buf_free;
 };
 
@@ -56,7 +57,7 @@ static int spk_ttyio_ldisc_open(struct t
 		return -ENOMEM;
 
 	init_completion(&ldisc_data->completion);
-	ldisc_data->buf_free = true;
+	ldisc_data->bufferize = false;
 	speakup_tty->disc_data = ldisc_data;
 
 	return 0;
@@ -84,6 +85,12 @@ static int spk_ttyio_receive_buf2(struct
 		return count;
 	}
 
+	if (!ldisc_data->bufferize) {
+		/* Just keep the last character */
+		ldisc_data->buf = cp[count-1];
+		return count;
+	}
+
 	if (!ldisc_data->buf_free)
 		/* ttyio_in will tty_schedule_flip */
 		return 0;
@@ -113,6 +120,7 @@ static int spk_ttyio_out(struct spk_synt
 static int spk_ttyio_out_unicode(struct spk_synth *in_synth, u16 ch);
 static void spk_ttyio_send_xchar(char ch);
 static void spk_ttyio_tiocmset(unsigned int set, unsigned int clear);
+static void spk_ttyio_bufferize(bool enable);
 static unsigned char spk_ttyio_in(void);
 static unsigned char spk_ttyio_in_nowait(void);
 static void spk_ttyio_flush_buffer(void);
@@ -123,6 +131,7 @@ struct spk_io_ops spk_ttyio_ops = {
 	.synth_out_unicode = spk_ttyio_out_unicode,
 	.send_xchar = spk_ttyio_send_xchar,
 	.tiocmset = spk_ttyio_tiocmset,
+	.bufferize = spk_ttyio_bufferize,
 	.synth_in = spk_ttyio_in,
 	.synth_in_nowait = spk_ttyio_in_nowait,
 	.flush_buffer = spk_ttyio_flush_buffer,
@@ -293,16 +302,38 @@ static int spk_ttyio_wait_for_xmitr(stru
 	return 1;
 }
 
-static unsigned char ttyio_in(int timeout)
+static void spk_ttyio_bufferize(bool enable)
+{
+	struct spk_ldisc_data *ldisc_data = speakup_tty->disc_data;
+	bool previous;
+
+	previous = ldisc_data->bufferize;
+	ldisc_data->bufferize = enable;
+	mb();
+
+	if (!previous && enable) {
+		/* Prepare buffering */
+		reinit_completion(&ldisc_data->completion);
+		ldisc_data->buf_free = true;
+	} else if (previous && !enable) {
+		/* Stop buffering */
+		tty_schedule_flip(speakup_tty->port);
+		ldisc_data->buf = 0xff;
+	}
+}
+
+static unsigned char spk_ttyio_in(void)
 {
+	int timeout = SPK_SYNTH_TIMEOUT;
 	struct spk_ldisc_data *ldisc_data = speakup_tty->disc_data;
 	char rv;
 
+	BUG_ON(!ldisc_data->bufferize);
+
 	if (wait_for_completion_timeout(&ldisc_data->completion,
 					usecs_to_jiffies(timeout)) == 0) {
-		if (timeout)
-			pr_warn("spk_ttyio: timeout (%d)  while waiting for input\n",
-				timeout);
+		pr_warn("spk_ttyio: timeout (%d)  while waiting for input\n",
+			timeout);
 		return 0xff;
 	}
 
@@ -318,14 +349,14 @@ static unsigned char ttyio_in(int timeou
 	return rv;
 }
 
-static unsigned char spk_ttyio_in(void)
-{
-	return ttyio_in(SPK_SYNTH_TIMEOUT);
-}
-
 static unsigned char spk_ttyio_in_nowait(void)
 {
-	u8 rv = ttyio_in(0);
+	struct spk_ldisc_data *ldisc_data = speakup_tty->disc_data;
+	u8 rv;
+
+	BUG_ON(ldisc_data->bufferize);
+
+	rv = ldisc_data->buf;
 
 	return (rv == 0xff) ? 0 : rv;
 }
--- a/drivers/staging/speakup/spk_types.h
+++ b/drivers/staging/speakup/spk_types.h
@@ -155,8 +155,22 @@ struct spk_io_ops {
 	int (*synth_out_unicode)(struct spk_synth *synth, u16 ch);
 	void (*send_xchar)(char ch);
 	void (*tiocmset)(unsigned int set, unsigned int clear);
+
+	/*
+	 * Drivers may either
+	 * - define read_buff_add, and thus spk_io does not buffer at all and
+	 *   synth_in and synth_in_nowait must not be called.
+	 * - or not define read_buff_add, and use synth_in_nowait to read the
+	 *   last characters that was received (this is typically what
+	 *   spk_synth_get_index does).
+	 * - or not define read_buff_add, but call bufferize(1), then call
+	 *   synth_in several times to get characters one after the other, then
+	 *   call bufferize(0) to revert back to using synth_in_nowait.
+	 */
+	void (*bufferize)(bool enable);
 	unsigned char (*synth_in)(void);
 	unsigned char (*synth_in_nowait)(void);
+
 	void (*flush_buffer)(void);
 	int (*wait_for_xmitr)(struct spk_synth *synth);
 };
--- a/drivers/staging/speakup/serialio.c
+++ b/drivers/staging/speakup/serialio.c
@@ -29,6 +29,7 @@ static int timeouts;
 static int spk_serial_out(struct spk_synth *in_synth, const char ch);
 static void spk_serial_send_xchar(char ch);
 static void spk_serial_tiocmset(unsigned int set, unsigned int clear);
+static void spk_serial_bufferize(bool enable);
 static unsigned char spk_serial_in(void);
 static unsigned char spk_serial_in_nowait(void);
 static void spk_serial_flush_buffer(void);
@@ -38,6 +39,7 @@ struct spk_io_ops spk_serial_io_ops = {
 	.synth_out = spk_serial_out,
 	.send_xchar = spk_serial_send_xchar,
 	.tiocmset = spk_serial_tiocmset,
+	.bufferize = spk_serial_bufferize,
 	.synth_in = spk_serial_in,
 	.synth_in_nowait = spk_serial_in_nowait,
 	.flush_buffer = spk_serial_flush_buffer,
@@ -251,6 +253,11 @@ static int spk_serial_wait_for_xmitr(str
 	return 1;
 }
 
+static void spk_serial_bufferize(bool enable)
+{
+	/* We let the UART buffer characters */
+}
+
 static unsigned char spk_serial_in(void)
 {
 	int tmout = SPK_SERIAL_TIMEOUT;


More information about the Speakup mailing list