linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Life is hard, and then you die" <ronald@innovation.ch>
To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Mao Wenan <maowenan@huawei.com>,
	Federico Lorenzi <federico@travelground.com>,
	linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] Input: applespi - register touchpad device synchronously in probe
Date: Mon, 29 Jul 2019 23:56:48 -0700	[thread overview]
Message-ID: <20190730065648.GA20206@innovation.ch> (raw)
In-Reply-To: <20190729132203.GB1201@penguin>


  Hi Dmitry,

On Mon, Jul 29, 2019 at 03:22:03PM +0200, Dmitry Torokhov wrote:
> Hi Ronald,
> 
> On Sun, Jul 21, 2019 at 12:05:23AM -0700, Ronald Tschalär wrote:
> > This allows errors during registration to properly fail the probe
> > function.
> > 
> > Doing this requires waiting for a response from the device inside the
> > probe function. While this generally takes about 15ms, in case of errors
> > it could be arbitrarily long, and hence a 3 second timeout is used.
> > 
> > This also adds 3 second timeouts to the drain functions to avoid the
> > potential for suspend or remove hanging forever.
> 
> Question: is it possible to read command response synchronously as well?
> I.e. I was wondering if we could add 2 (or 1?) more read xfers for the
> actual result that is coming after the status response, and then we
> could use spi_sync() to send the command and read the whole thing.

Yes'ish. But you still need to wait for the GPE to know when to read
the response, and while you're doing so any number of keyboard and
trackpad events may arrive (i.e. you may need to do any number of read
xfers). I suppose those events could all just be discarded, though. So
something like this:

    assemble-info-cmd(write_msg)
    spi_sync(write_msg)
    
    while (1) {
        wait_event_timeout(wait_queue, gpe_received, timeout)
        spi_sync(read_msg)
        if (is-info-cmd-response(read_msg))
            break
    }

and also modify the gpe-handler to wake the wait_queue instead of
issuing an spy_async() while doing the above.

I guess the advantage would certainly be the need to avoid the
spi-flushing in case of a timeout, at the expense of some slight
duplication of some of the received-message handling logic (would
refactor make most re-usable, of course).

So would this be the preferred approach then?


  Cheers,

  Ronald

  reply	other threads:[~2019-07-30  6:56 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-21  7:05 [PATCH v2] Input: applespi - register touchpad device synchronously in probe Ronald Tschalär
2019-07-29 13:22 ` Dmitry Torokhov
2019-07-30  6:56   ` Life is hard, and then you die [this message]
2019-07-30 12:39     ` Andy Shevchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190730065648.GA20206@innovation.ch \
    --to=ronald@innovation.ch \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=dmitry.torokhov@gmail.com \
    --cc=federico@travelground.com \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maowenan@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).