All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
To: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cc: Bastien Nocera <hadess@hadess.net>,
	linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] Input - surface3_spi: add new driver for the Surface 3
Date: Tue, 17 May 2016 11:04:28 -0700	[thread overview]
Message-ID: <20160517180428.GA9007@dtor-ws> (raw)
In-Reply-To: <1463480179-11974-1-git-send-email-benjamin.tissoires@redhat.com>

Hi Benjamin,

On Tue, May 17, 2016 at 12:16:19PM +0200, Benjamin Tissoires wrote:
> This is a basic driver for the Surface 3. I am not so sure it will work
> with any firmwares as most values are encoded, but given that I only have
> access to my current device with its firmware and I don't have the
> datasheet, it should be OK for now.
> 
> The Surface Pen is not supported (if it is supposed to be). I'll work on
> this when I get one.
> 
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
> Changes in v2:
> 
> - module renamed from ntrig_spi to surface3_spi
> - took into account Dmitry's remarks
> - kept the retrieval of the GPIO as mandatory as otherwise the device fails to work
> 
> Changes in v3:
> 
> - asm include put at the end
> - used proper types in struct surfacae3_ts_data_finger
> - dropped temps in surface3_spi_report_touch()
> - inlined surface3_spi_request_irq()
> - detect IRQ type (falling/rising) depending on the gpiod active low parameter

I do not think that this should be done in the driver. The device either
has interrupt descriptor or GpioInt decriptor in DSDT and SPI core/ACPI
should do the right thing and configure GPIO as input and set interrupt
polarity accordingly, so when you do request_threaded_irq() you only
need to specify IRQF_ONESHOT.

See drivers/spi/spi.c - in case there is no interrupt specified we call
acpi_dev_gpio_irq_get(). Do you see it called in your case? Or your DSDT
has interrupt for the touchscreen device and acpi_spi_add_resource() and
acpi_dev_resource_interrupt() are called? You need to trace it and
figure out if there is an issue there with setting up interrupt
properly.

Thanks.

-- 
Dmitry

  parent reply	other threads:[~2016-05-17 18:04 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-17 10:16 [PATCH v3] Input - surface3_spi: add new driver for the Surface 3 Benjamin Tissoires
2016-05-17 17:45 ` Bastien Nocera
2016-05-17 18:04 ` Dmitry Torokhov [this message]
2016-05-17 19:13   ` Benjamin Tissoires
2016-05-18 13:31     ` Benjamin Tissoires

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=20160517180428.GA9007@dtor-ws \
    --to=dmitry.torokhov@gmail.com \
    --cc=benjamin.tissoires@redhat.com \
    --cc=hadess@hadess.net \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.