Hello,
Okash Khawaja, on jeu. 13 avril 2017 18:41:34 +0100, wrote:
> +static struct spk_synth *spk_ttyio_synth;
> +/* TODO: make this part of synth and remove the global */
Mmm, rather the other way round: add a synth pointer inside the
spk_ldisc_data structure, so you can get the synth from
> +static int spk_ttyio_receive_buf2(struct tty_struct *tty,
> - pr_err("Error registering line discipline.\n");
> + pr_err("speakup: Error registering line discipline.\n");
Do not mix with unrelated changes :)
> +static unsigned char ttyio_in(int timeout)
> +{
> + struct spk_ldisc_data *ldisc_data =
> + (struct spk_ldisc_data *)speakup_tty->disc_data;
> + char rv;
> +
> + if (!down_timeout(&ldisc_data->sem, usecs_to_jiffies(timeout))) {
Err, this should actually be if (down_timeout(...) != 0) (error case),
shouldn't it?
> + pr_warn("spk_ttyio: timeout (%d) while waiting for input\n",
> + timeout);
> + return 0xff;
> + }
> +
> + rv = ldisc_data->buf;
> + /* Make sure we have read buf before we set buf_free to let
> + * the producer overwrite it */
> + mb();
> + ldisc_data->buf_free = true;
> + tty_schedule_flip(speakup_tty->port);
Here, document that tty_schedule_flip lets the tty push more characters.
> + up(&ldisc_data->sem);
Err, no, it's spk_ttyio_receive_buf2 (the producer) which does the up(),
the consumer shouldn't do any up()! This is really a resource-counting
semaphore, not a mutex!
> Index: linux-staging/drivers/staging/speakup/spk_priv.h
> ===================================================================
> --- linux-staging.orig/drivers/staging/speakup/spk_priv.h
> +++ linux-staging/drivers/staging/speakup/spk_priv.h
> @@ -39,6 +39,7 @@
> #endif
>
> #define KT_SPKUP 15
> +#define SPK_SYNTH_TIMEOUT 100000
Please document the unit (us)
Samuel
_______________________________________________
Speakup mailing list
Speakup@xxxxxxxxxxxxxxxxx
http://linux-speakup.org/cgi-bin/mailman/listinfo/speakup
