[patch 6/7] staging: speakup: add TTY-based input functionality

Samuel Thibault samuel.thibault at ens-lyon.org
Sun Apr 9 18:53:37 EDT 2017


Hello,

We need to add comments about the synchronization, added inline:

Okash Khawaja, on lun. 03 avril 2017 21:21:29 +0100, wrote:
> +static int spk_ttyio_receive_buf2(struct tty_struct *tty,
> +		const unsigned char *cp, char *fp, int count)
> +{
> +	struct spk_ldisc_data *ldisc_data = (struct spk_ldisc_data *)tty->disc_data;
> +
> +	if (spk_ttyio_synth->read_buff_add) {
> +		int i;
> +		for (i = 0; i < count; i++)
> +			spk_ttyio_synth->read_buff_add(cp[i]);
> +
> +		return count;
> +	}
> +
> +	if (ldisc_data->buf_free == 0)
                /* ttyio_in will tty_schedule_flip */
> +		return 0;
> +
        /* Make sure the consumer has read buf before we have seen
         * buf_free == 1 and overwrite buf */
> +	mb();
> +
> +	ldisc_data->buf = cp[0];
> +	ldisc_data->buf_free--;

Don't decrement, set to 0. Decrementing would make the reader think
that there is potential for multi-character support, but with such a
lightweight synchronization there can't be. The fact that buf_free is 1
allows the tty part to set it to 0, but not more.

> +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))) {
> +		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 = 1;
> +	tty_schedule_flip(speakup_tty->port);
> +	up(&ldisc_data->sem);

> 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.

Samuel


More information about the Speakup mailing list