Hi,
Two of the mistakes are frankly embarassing (down_timeout and up
inside spk_ttyio_in) . At some point during testing with apollo and
ltlk harwdare, I had to import and modify the patches to suit 4.10
kernel as 4.11 was still not stable and giving problems with USB on my
computer. I remember fixing them during tests, which is why those
synths worked. But didn't merge them back.
I will fix this patch today.
Thanks,
Okash
On Thu, Apr 27, 2017 at 3:06 PM, Samuel Thibault
<samuel.thibault@xxxxxxxxxxxx> wrote:
> 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
