From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by befuddled.reisers.ca (Postfix, from userid 65534) id C04281F0383; Fri, 28 Apr 2017 03:45:52 -0400 (EDT) Received: from mail-it0-x242.google.com (mail-it0-x242.google.com [IPv6:2607:f8b0:4001:c0b::242]) by befuddled.reisers.ca (Postfix) with ESMTPS id 385741F0380 for ; Fri, 28 Apr 2017 03:45:50 -0400 (EDT) Received: by mail-it0-x242.google.com with SMTP id z67so4797304itb.0 for ; Fri, 28 Apr 2017 00:45:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=m784Lr83gkp9kZm1yok1bauw+esJDk0O8zoGGrR0Dbk=; b=P0wPcHcph7lK1O70HQsftO8tXI4LtzzL7BqxVrP2M9+EW0EFG8PU3rjn8ehKRUaUQU JeOnrWNOeDF3FcoagG2IOojauMKWx7dGzAXr+m9qQFeZb58bWHk7j8gvZ39ULAm6mzQC r0avPaCgZQwZFRnUmq+DotfOEsan0PVJfyOSBkSlVpKnUtgAO+uCVGch7nWpYwnB9De3 cW7N5+zUw0rE3f30lKuTknrquMdsoYsHDpJA56iepmhCcNyIAm+xqg6cM2VQuJWJFHC/ lXYmV+MrV+uhtwXk/Pgy1o9IF0wzuuYEnZwFMJsF9Ja/8sq52erWbvL3Fl8kzs5z2Z6c m4Qw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=m784Lr83gkp9kZm1yok1bauw+esJDk0O8zoGGrR0Dbk=; b=kx3/k45+YKaa/oyxIH0x7j1+Icot3nWDTLI40zfCibF2jGBPsjqEvepOOOV5ODDHr2 IUOP9iBQ35n/21PIKOgwBmFTRHcx7FLp+tqv1zvAKQr02BkRoYyYE0fSIpU41HoyZJTA RCqdLTsDP7dYXpNkF7AFeHRusquW8pIliR7hvsxoIJG14caxLYb1Ht7a2843cEAQShVq +kQRTFA9RQOPRoHKV7cdqfMZRaRRQSLD0zBeyFispdXy69+31WRUBYB0BV1Rmdxf7jVV 7oOMwDM1SEhu0rh81JC24qMlj4VqioV/DGL9Eiwi093+obFlsqxVxUOhNoBCMcxAI+Tk +Gtw== X-Gm-Message-State: AN3rC/49b03Z9ktTCRGITPWd6GVDh6oLcnxmUY6nNCmmZAYeZfYNa675 y6YNRuH7xT1rQ84FlrecEBHHW1oAnw== X-Received: by 10.36.89.200 with SMTP id p191mr7649214itb.36.1493365540708; Fri, 28 Apr 2017 00:45:40 -0700 (PDT) MIME-Version: 1.0 Received: by 10.79.65.130 with HTTP; Fri, 28 Apr 2017 00:45:40 -0700 (PDT) In-Reply-To: <20170427140655.22u372ajrtlk37xw@var.youpi.perso.aquilenet.fr> References: <20170413174128.794011516@gmail.com> <20170413174733.472443648@gmail.com> <20170427140655.22u372ajrtlk37xw@var.youpi.perso.aquilenet.fr> From: Okash Khawaja Date: Fri, 28 Apr 2017 08:45:40 +0100 Message-ID: Subject: Re: [patch 6/7] staging: speakup: add send_xchar, tiocmset and input functionality for tty To: Samuel Thibault Cc: "Speakup is a screen review system for Linux." Content-Type: text/plain; charset=UTF-8 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 X-BeenThere: speakup@linux-speakup.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "Speakup is a screen review system for Linux." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 28 Apr 2017 07:45:52 -0000 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 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