From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by befuddled.reisers.ca (Postfix, from userid 65534) id 9ECA01F0C72; Thu, 30 Mar 2017 10:45:18 -0400 (EDT) Received: from mail-io0-x229.google.com (mail-io0-x229.google.com [IPv6:2607:f8b0:4001:c06::229]) by befuddled.reisers.ca (Postfix) with ESMTPS id C86171F0C6F for ; Thu, 30 Mar 2017 10:45:15 -0400 (EDT) Received: by mail-io0-x229.google.com with SMTP id z13so20589251iof.2 for ; Thu, 30 Mar 2017 07:45:15 -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:content-transfer-encoding; bh=7WcFgsWWIoiq1grULIZNriKXgIlHcGnB8AOgmJeVBKM=; b=hpJbWCy2sapCfXiss27JblMwnzNO7aPr0XWQHB97vKyRxY4DgiQTuUegH+hZFR+pgN 8TzwC9W6do1NtAUmxFz4nfjWaWXcxhQLbAJvWvKhNxtRQUYHJY7tWsaQol4CdXJs7FjU 31PSq9IUZqPeIFSqcBrhZx8odmx5k1MBiZixxn/030NqMQsFRIxPp8ufjIzkRfqJE30T 2KQ7x3AjpgabpntNJRUJmIzMzfwOrEzoI1CB9k8L3AvfG4VjYe0w0h83dQjMx4gDiC6h fIaBwvqMh/NYh5N/32qzdGBdiK7wW56c4aDeSrbGwsJpSrUtV7D3s4kSVDZFQC6KcfRd Pp9A== 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:content-transfer-encoding; bh=7WcFgsWWIoiq1grULIZNriKXgIlHcGnB8AOgmJeVBKM=; b=uDZWJ5S8Brgb6IkM+wk12FbNLwHWu6lYTHcmUr2XqS9A5eei8jMhCQ7xGJ00wStlTk 9wnqVc6RUoj/xUL3/GVq73lE/lDxUMOjRok4vbBf8qIkplTrBf/Ktr/+V9XrsELr563n bPKQuInaWV2PM3qSl4EOb7Lj8BfJdq12leWixk1GcFde/maRVMSoYEeJ90VvKOEIgJ6D FmopI5F01uBtepr7pwaa8zHxLO18JzW9elec6jrxSRD/T/Kq70bHw5GyicStFdgkOBS5 HufyCQEzjR4jfFMjuYFqL4nGr48bSrwKTRXKUQz29ywfgl+BeyhUVI53wNbXCh05xyd7 jpnQ== X-Gm-Message-State: AFeK/H26skRe2/FbdkjmQOdpARgOUdrV2MhMA2ySEjIhXvm2TzFMupqs4/HUycKvW2hdCGVeQrXFUM06wPS9Pw== X-Received: by 10.107.149.7 with SMTP id x7mr777569iod.167.1490885108497; Thu, 30 Mar 2017 07:45:08 -0700 (PDT) MIME-Version: 1.0 Received: by 10.79.158.81 with HTTP; Thu, 30 Mar 2017 07:45:08 -0700 (PDT) In-Reply-To: <20170226110748.puaws6f5q5zankyc@var.youpi.perso.aquilenet.fr> References: <20170225192842.GA4519@sanghar> <20170225193031.GA4525@sanghar> <20170226015005.25u3ea7ag5ub5ulc@var.youpi.perso.aquilenet.fr> <20170226024832.ly3mogxxrktjehzf@var.youpi.perso.aquilenet.fr> <20170226110748.puaws6f5q5zankyc@var.youpi.perso.aquilenet.fr> From: Okash Khawaja Date: Thu, 30 Mar 2017 15:45:08 +0100 Message-ID: Subject: Re: [patch 6/6] staging: speakup: Migrate acntsa, bns, dectlk and txprt to ttyio To: Samuel Thibault Cc: "Speakup is a screen review system for Linux." Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable 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: Thu, 30 Mar 2017 14:45:19 -0000 Hi, On Sun, Feb 26, 2017 at 11:07 AM, Samuel Thibault wrote: > Hello, > > Samuel Thibault, on dim. 26 f=C3=A9vr. 2017 03:48:32 +0100, wrote: >> Samuel Thibault, on dim. 26 f=C3=A9vr. 2017 02:50:05 +0100, wrote: >> > Please note in your todo for the input part, that ttyio will have to >> > call read_buff_add for each received character, when that method is >> > not NULL (that's actually the only driver using it, so we will really >> > need a tester for this exact driver). >> >> More precisely, it's the spk_ttyio_ldisc_ops->receive_buf2 method which >> should call read_buff_add for each received character. >> >> >> The step further will be implementing spk_ttyio_in and >> spk_ttyio_in_nowait. I believe the easiest way is the following: > > I forgot the part that stops the tty layer from sending characters: > >> - define an spk_ldisc_data structure containing just one character (buf)= , >> and a semaphore. > > and one int (buf_free). > >> - on ldisc_open, allocate a pointer to such structure, set >> tty->disc_data to point to it, and initialized the semaphore to 0 >> tokens. > > and initialize buf_free to 1. > >> - in the receive_buf2 method, >> - if read_buff_add is defined, just call it for each character and be >> done >> - otherwise, store the first received character in >> ((struct spk_ldisc_data *)tty->disc_data)->buf >> , call up() on the semaphore, and return 1 (to tell that you ate the >> character). > > Here, *just before* storing the character in buf, check buf_free: if > it is 0, return 0. otherwise, call mb(), and continue with what I wrote > above. > >> - in spk_serial_in, call down_timeout(usecs_to_jiffies(SPK_SERIAL_TIMEOU= T)), >> - on success, copy the character stored in buf, then call tty_schedule= _flip() and return the copy >> - on failure (timed out), return 0xff. >> >> - in spk_serial_in_nowait, call down_trylock(), >> - on success, copy the character stored in buf, then call tty_schedule= _flip() and return the copy >> - on failure, return 0. > > In each case, right after the copy, call mb() and set buf_free to 1. > > > Actually, these two function could be factorized to just one, which > takes a long timeout parameter (in jiffies) and returns an int > instead of a char, and uses down_trylock when the timeout is 0 and > down_timeout otherwise, and returns -1 instead of 0 on failure. And then > make spk_serial_in use it and return 0xff when getting -1, and make > spk_serial_in_nowait return 0 when getting -1. Cleaning that returned > value convention can be another patch later. > Here's another approach using atomic_t instead of semaphore but still employing same logic. - spk_ldisc_data contains two members: char buf and atomic_t buf_free - ldisc_open initialises buf_free to 1 - in receive_buf2, if read_buff_add is not defined: - check if buf_free is zero then return zero and finish - call mb() - copy first character to buf in spk_ldisc_data - call atomic_dec(&buf_free) - return 1 - in spk_ttyio_in: - countdown for SPK_SERIAL_TIMEOUT usecs - similar to spk_serial_in - and check for atomic_read(&buf_free) =3D=3D 0 - if timed out, return 0xff - otherwise call mb() - read the char stored in buf and call atomic_inc(&buf_free) - call tty_schedule_flip() - return the char spk_ttyio_in_nowait will be similar and can be factorised into spk_ttyio_in as you suggested. Using this approach we can avoid using down_timeout() which may need justification when merging upstream. I might be wrong here but there seems to be a somewhat less acceptance for timeout while acquiring locks. For example in case of mutex_lock_timeout: http://lkml.iu.edu/hypermail/linux/kernel/0611.3/0254.html. If however there are plans of making buf bigger than one char, then this will require reworking as the above scheme overloads buf_free both for mutual exclusion and to count free bytes in buffer. Okash