* [PATCH v3] input: bcm5974 - Add driver for Apple Magic Trackpad 2
From: Stephan Müller @ 2018-01-21 22:06 UTC (permalink / raw)
To: Jiri Kosina, Dmitry Torokhov, Henrik Rydberg
Cc: linux-input, marek.wyborski, linux-kernel
In-Reply-To: <40764439.8xGR6LbaWR@positron.chronox.de>
Hi,
Changes v3:
* port to 4.15-rc8
* small code cleanups (isolation of type casts to functions pertaining
to the Apple Magic Trackpad 2
* clean up all checkpatch.pl errors and warnings (except those
where the patch uses the structure of existing code fragments)
* updated horizontal and vertical limits to capture start of movements
in the outer areas of the pad
---8<---
Add support for Apple Magic Trackpad 2 in bcm5974 (MacBook Tochpad) driver.
The Magic Trackpad 2 needs to be switched into the finger-reporting-mode,
just like the other macbook touchpads as well. But the format is different
to the ones before. The Header is 12 Bytes long and each reported finger
is additional 9 Bytes. The data order reported by the hardware is
different as well.
Signed-off-by: Marek Wyborski <marek.wyborski@emwesoft.com>
Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
drivers/input/mouse/bcm5974.c | 127 +++++++++++++++++++++++++++++++++++++
+----
1 file changed, 116 insertions(+), 11 deletions(-)
diff --git a/drivers/input/mouse/bcm5974.c b/drivers/input/mouse/bcm5974.c
index d0122134f320..11868321992c 100644
--- a/drivers/input/mouse/bcm5974.c
+++ b/drivers/input/mouse/bcm5974.c
@@ -96,6 +96,8 @@
#define USB_DEVICE_ID_APPLE_WELLSPRING9_ANSI 0x0272
#define USB_DEVICE_ID_APPLE_WELLSPRING9_ISO 0x0273
#define USB_DEVICE_ID_APPLE_WELLSPRING9_JIS 0x0274
+/* MagicTrackpad2 (2015) */
+#define USB_DEVICE_ID_APPLE_MAGICTRACKPAD2 0x0265
#define BCM5974_DEVICE(prod) { \
.match_flags = (USB_DEVICE_ID_MATCH_DEVICE | \
@@ -161,6 +163,8 @@ static const struct usb_device_id bcm5974_table[] = {
BCM5974_DEVICE(USB_DEVICE_ID_APPLE_WELLSPRING9_ANSI),
BCM5974_DEVICE(USB_DEVICE_ID_APPLE_WELLSPRING9_ISO),
BCM5974_DEVICE(USB_DEVICE_ID_APPLE_WELLSPRING9_JIS),
+ /* MagicTrackpad2 */
+ BCM5974_DEVICE(USB_DEVICE_ID_APPLE_MAGICTRACKPAD2),
/* Terminating entry */
{}
};
@@ -190,7 +194,8 @@ enum tp_type {
TYPE1, /* plain trackpad */
TYPE2, /* button integrated in trackpad */
TYPE3, /* additional header fields since June 2013 */
- TYPE4 /* additional header field for pressure data */
+ TYPE4, /* additional header field for pressure data */
+ TYPE5 /* format for magic trackpad 2 */
};
/* trackpad finger data offsets, le16-aligned */
@@ -198,12 +203,14 @@ enum tp_type {
#define HEADER_TYPE2 (15 * sizeof(__le16))
#define HEADER_TYPE3 (19 * sizeof(__le16))
#define HEADER_TYPE4 (23 * sizeof(__le16))
+#define HEADER_TYPE5 (6 * sizeof(__le16))
/* trackpad button data offsets */
#define BUTTON_TYPE1 0
#define BUTTON_TYPE2 15
#define BUTTON_TYPE3 23
#define BUTTON_TYPE4 31
+#define BUTTON_TYPE5 1
/* list of device capability bits */
#define HAS_INTEGRATED_BUTTON 1
@@ -213,18 +220,21 @@ enum tp_type {
#define FSIZE_TYPE2 (14 * sizeof(__le16))
#define FSIZE_TYPE3 (14 * sizeof(__le16))
#define FSIZE_TYPE4 (15 * sizeof(__le16))
+#define FSIZE_TYPE5 (9)
/* offset from header to finger struct */
#define DELTA_TYPE1 (0 * sizeof(__le16))
#define DELTA_TYPE2 (0 * sizeof(__le16))
#define DELTA_TYPE3 (0 * sizeof(__le16))
#define DELTA_TYPE4 (1 * sizeof(__le16))
+#define DELTA_TYPE5 (0 * sizeof(__le16))
/* usb control message mode switch data */
#define USBMSG_TYPE1 8, 0x300, 0, 0, 0x1, 0x8
#define USBMSG_TYPE2 8, 0x300, 0, 0, 0x1, 0x8
#define USBMSG_TYPE3 8, 0x300, 0, 0, 0x1, 0x8
#define USBMSG_TYPE4 2, 0x302, 2, 1, 0x1, 0x0
+#define USBMSG_TYPE5 2, 0x302, 1, 1, 0x1, 0x0
/* Wellspring initialization constants */
#define BCM5974_WELLSPRING_MODE_READ_REQUEST_ID 1
@@ -247,6 +257,18 @@ struct tp_finger {
__le16 multi; /* one finger: varies, more fingers: constant */
} __attribute__((packed,aligned(2)));
+/* trackpad finger structure for type5 (magic trackpad), le16-aligned */
+struct tp_finger_type5 {
+ u8 abs_x; /* absolute x coodinate */
+ u8 abs_x_y; /* absolute x,y coodinate */
+ u8 abs_y[2]; /* absolute y coodinate */
+ u8 touch_major; /* touch area, major axis */
+ u8 touch_minor; /* touch area, minor axis */
+ u8 size; /* tool area, size */
+ u8 pressure; /* pressure on forcetouch touchpad */
+ u8 orientation_origin; /* orientation and id */
+} __attribute__((packed,aligned(2)));
+
/* trackpad finger data size, empirically at least ten fingers */
#define MAX_FINGERS 16
#define MAX_FINGER_ORIENTATION 16384
@@ -497,6 +519,19 @@ static const struct bcm5974_config bcm5974_config_table[]
= {
{ SN_COORD, -203, 6803 },
{ SN_ORIENT, -MAX_FINGER_ORIENTATION, MAX_FINGER_ORIENTATION }
},
+ {
+ USB_DEVICE_ID_APPLE_MAGICTRACKPAD2,
+ USB_DEVICE_ID_APPLE_MAGICTRACKPAD2,
+ USB_DEVICE_ID_APPLE_MAGICTRACKPAD2,
+ HAS_INTEGRATED_BUTTON,
+ 0, sizeof(struct bt_data),
+ 0x83, DATAFORMAT(TYPE5),
+ { SN_PRESSURE, 0, 300 },
+ { SN_WIDTH, 0, 2048 },
+ { SN_COORD, -9000, 9000 },
+ { SN_COORD, -6803, 6803 },
+ { SN_ORIENT, -MAX_FINGER_ORIENTATION, MAX_FINGER_ORIENTATION }
+ },
{}
};
@@ -539,9 +574,13 @@ static void setup_events_to_report(struct input_dev
*input_dev,
/* finger touch area */
set_abs(input_dev, ABS_MT_TOUCH_MAJOR, &cfg->w);
set_abs(input_dev, ABS_MT_TOUCH_MINOR, &cfg->w);
+
/* finger approach area */
- set_abs(input_dev, ABS_MT_WIDTH_MAJOR, &cfg->w);
- set_abs(input_dev, ABS_MT_WIDTH_MINOR, &cfg->w);
+ if (cfg->tp_type != TYPE5) {
+ set_abs(input_dev, ABS_MT_WIDTH_MAJOR, &cfg->w);
+ set_abs(input_dev, ABS_MT_WIDTH_MINOR, &cfg->w);
+ }
+
/* finger orientation */
set_abs(input_dev, ABS_MT_ORIENTATION, &cfg->o);
/* finger position */
@@ -596,6 +635,25 @@ static void report_finger_data(struct input_dev *input,
int slot,
input_report_abs(input, ABS_MT_POSITION_Y, pos->y);
}
+static void report_finger_data_type5(struct input_dev *input, int slot,
+ const struct input_mt_pos *pos,
+ const struct tp_finger *f)
+{
+ const struct tp_finger_type5 *f5 = (const struct tp_finger_type5 *)f;
+
+ input_mt_slot(input, slot);
+ input_mt_report_slot_state(input, MT_TOOL_FINGER, true);
+
+ input_report_abs(input, ABS_MT_TOUCH_MAJOR,
+ raw2int(f5->touch_major) << 2);
+ input_report_abs(input, ABS_MT_TOUCH_MINOR,
+ raw2int(f5->touch_minor) << 2);
+ input_report_abs(input, ABS_MT_ORIENTATION,
+ MAX_FINGER_ORIENTATION - ((f5->orientation_origin & 0xf0) << 6));
+ input_report_abs(input, ABS_MT_POSITION_X, pos->x << 1);
+ input_report_abs(input, ABS_MT_POSITION_Y, pos->y << 1);
+}
+
static void report_synaptics_data(struct input_dev *input,
const struct bcm5974_config *cfg,
const struct tp_finger *f, int raw_n)
@@ -615,11 +673,34 @@ static void report_synaptics_data(struct input_dev
*input,
input_report_abs(input, ABS_TOOL_WIDTH, abs_w);
}
+static void report_synaptics_data_type5(struct input_dev *input,
+ const struct bcm5974_config *cfg,
+ const struct tp_finger *f,
+ int raw_n)
+{
+ const struct tp_finger_type5 *f5 = (const struct tp_finger_type5 *)f;
+ int abs_p = 0, abs_w = 0;
+
+ if (raw_n) {
+ int p = f5->pressure;
+ int w = f5->size;
+
+ if (p && w) {
+ abs_p = p;
+ abs_w = w;
+ }
+ }
+
+ input_report_abs(input, ABS_PRESSURE, abs_p);
+ input_report_abs(input, ABS_TOOL_WIDTH, abs_w);
+}
+
/* report trackpad data as logical trackpad state */
static int report_tp_state(struct bcm5974 *dev, int size)
{
const struct bcm5974_config *c = &dev->cfg;
const struct tp_finger *f;
+ const struct tp_finger_type5 *f_type5;
struct input_dev *input = dev->input;
int raw_n, i, n = 0;
@@ -629,23 +710,47 @@ static int report_tp_state(struct bcm5974 *dev, int
size)
raw_n = (size - c->tp_header) / c->tp_fsize;
for (i = 0; i < raw_n; i++) {
+
f = get_tp_finger(dev, i);
- if (raw2int(f->touch_major) == 0)
- continue;
- dev->pos[n].x = raw2int(f->abs_x);
- dev->pos[n].y = c->y.min + c->y.max - raw2int(f->abs_y);
+
+ if (c->tp_type != TYPE5) {
+ if (raw2int(f->touch_major) == 0)
+ continue;
+ dev->pos[n].x = raw2int(f->abs_x);
+ dev->pos[n].y = c->y.min + c->y.max - raw2int(f->abs_y);
+ } else {
+ u16 tmp_x;
+ u32 tmp_y;
+
+ f_type5 = (const struct tp_finger_type5 *) f;
+ if (f_type5->pressure == 0)
+ continue;
+ tmp_x = le16_to_cpu(*((__le16 *)f_type5)) & 0x1fff;
+ dev->pos[n].x = (s16) (tmp_x << 3) >> 3;
+ tmp_y = (s32) le32_to_cpu(*((__le32 *)f_type5));
+ dev->pos[n].y = -(s32) (tmp_y << 6) >> 19;
+ }
dev->index[n++] = f;
}
input_mt_assign_slots(input, dev->slots, dev->pos, n, 0);
- for (i = 0; i < n; i++)
- report_finger_data(input, dev->slots[i],
- &dev->pos[i], dev->index[i]);
+ for (i = 0; i < n; i++) {
+ if (c->tp_type != TYPE5)
+ report_finger_data(input, dev->slots[i],
+ &dev->pos[i], dev->index[i]);
+ else
+ report_finger_data_type5(input, dev->slots[i],
+ &dev->pos[i], dev->index[i]);
+ }
input_mt_sync_frame(input);
- report_synaptics_data(input, c, get_tp_finger(dev, 0), raw_n);
+ if (c->tp_type != TYPE5)
+ report_synaptics_data(input, c, get_tp_finger(dev, 0), raw_n);
+ else
+ report_synaptics_data_type5(input, c, get_tp_finger(dev, 0),
+ raw_n);
/* later types report button events via integrated button only */
if (c->caps & HAS_INTEGRATED_BUTTON) {
--
2.14.3
^ permalink raw reply related
* Darlehen
From: defina @ 2018-01-21 17:05 UTC (permalink / raw)
To: Recipients
Benötigen Sie Privat- oder Geschäftskredite ohne Stress und schnelle Zustimmung? Wenn ja, kontaktieren Sie uns bitte alexgrtz5@gmail.com
^ permalink raw reply
* Re: [PATCH 1/2] Input: sur40: Delete an error message for a failed memory allocation in sur40_probe()
From: Martin Kepplinger @ 2018-01-22 7:00 UTC (permalink / raw)
To: SF Markus Elfring, linux-input, Dmitry Torokhov, Florian Echtler,
Johan Hovold, Martin Kaltenbrunner
Cc: LKML, kernel-janitors
In-Reply-To: <122986b6-b7f2-9b34-b11d-4d63557a2b97@users.sourceforge.net>
On 2018-01-20 22:28, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 20 Jan 2018 22:11:24 +0100
>
> Omit an extra message for a memory allocation failure in this function.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
Acked-by: Martin Kepplinger <martink@posteo.de>
> ---
> drivers/input/touchscreen/sur40.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
> index f16f8358c70a..c7a0a92b2044 100644
> --- a/drivers/input/touchscreen/sur40.c
> +++ b/drivers/input/touchscreen/sur40.c
> @@ -591,7 +591,6 @@ static int sur40_probe(struct usb_interface *interface,
> sur40->bulk_in_epaddr = endpoint->bEndpointAddress;
> sur40->bulk_in_buffer = kmalloc(sur40->bulk_in_size, GFP_KERNEL);
> if (!sur40->bulk_in_buffer) {
> - dev_err(&interface->dev, "Unable to allocate input buffer.");
> error = -ENOMEM;
> goto err_free_polldev;
> }
>
^ permalink raw reply
* Re: [PATCH] Input: edt-ft5x06: Delete an error message for a failed memory allocation in edt_ft5x06_ts_probe()
From: Martin Kepplinger @ 2018-01-22 7:00 UTC (permalink / raw)
To: SF Markus Elfring, linux-input, Andi Shyti, Dmitry Torokhov,
Günter Röck, Rob Herring, Simon Budig,
Stefan Schöfegger
Cc: LKML, kernel-janitors
In-Reply-To: <45f8a6c2-423b-5bc1-6512-c5b954c5620b@users.sourceforge.net>
On 2018-01-21 20:19, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 21 Jan 2018 20:10:03 +0100
>
> Omit an extra message for a memory allocation failure in this function.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
Acked-by: Martin Kepplinger <martink@posteo.de>
> ---
> drivers/input/touchscreen/edt-ft5x06.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> index c53a3d7239e7..9d2799c90150 100644
> --- a/drivers/input/touchscreen/edt-ft5x06.c
> +++ b/drivers/input/touchscreen/edt-ft5x06.c
> @@ -978,10 +978,8 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
> dev_dbg(&client->dev, "probing for EDT FT5x06 I2C\n");
>
> tsdata = devm_kzalloc(&client->dev, sizeof(*tsdata), GFP_KERNEL);
> - if (!tsdata) {
> - dev_err(&client->dev, "failed to allocate driver data.\n");
> + if (!tsdata)
> return -ENOMEM;
> - }
>
> chip_data = of_device_get_match_data(&client->dev);
> if (!chip_data)
>
^ permalink raw reply
* Re: [PATCH 2/2] Input: sur40: Improve a size determination in sur40_probe()
From: Martin Kepplinger @ 2018-01-22 7:02 UTC (permalink / raw)
To: SF Markus Elfring, linux-input, Dmitry Torokhov, Florian Echtler,
Johan Hovold, Martin Kaltenbrunner
Cc: LKML, kernel-janitors
In-Reply-To: <11e0b313-f8c2-93ec-0324-24b30053a586@users.sourceforge.net>
On 2018-01-20 22:30, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 20 Jan 2018 22:16:14 +0100
>
> Replace the specification of a data structure by a pointer dereference
> as the parameter for the operator "sizeof" to make the corresponding size
> determination a bit safer according to the Linux coding style convention.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
Acked-by: Martin Kepplinger <martink@posteo.de>
> ---
> drivers/input/touchscreen/sur40.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
> index c7a0a92b2044..946e1a0328b4 100644
> --- a/drivers/input/touchscreen/sur40.c
> +++ b/drivers/input/touchscreen/sur40.c
> @@ -550,7 +550,7 @@ static int sur40_probe(struct usb_interface *interface,
> return -ENODEV;
>
> /* Allocate memory for our device state and initialize it. */
> - sur40 = kzalloc(sizeof(struct sur40_state), GFP_KERNEL);
> + sur40 = kzalloc(sizeof(*sur40), GFP_KERNEL);
> if (!sur40)
> return -ENOMEM;
>
>
^ permalink raw reply
* Re: [PATCH] Input: edt-ft5x06: Delete an error message for a failed memory allocation in edt_ft5x06_ts_probe()
From: Andi Shyti @ 2018-01-22 7:43 UTC (permalink / raw)
To: SF Markus Elfring
Cc: linux-input, Andi Shyti, Dmitry Torokhov, Günter Röck,
Martin Kepplinger, Rob Herring, Simon Budig,
Stefan Schöfegger, LKML, kernel-janitors
In-Reply-To: <45f8a6c2-423b-5bc1-6512-c5b954c5620b@users.sourceforge.net>
Hi Markus,
On Sun, Jan 21, 2018 at 08:19:00PM +0100, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 21 Jan 2018 20:10:03 +0100
>
> Omit an extra message for a memory allocation failure in this function.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
Acked-by: Andi Shyti <andi@etezian.org>
Thanks,
Andi
> ---
> drivers/input/touchscreen/edt-ft5x06.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> index c53a3d7239e7..9d2799c90150 100644
> --- a/drivers/input/touchscreen/edt-ft5x06.c
> +++ b/drivers/input/touchscreen/edt-ft5x06.c
> @@ -978,10 +978,8 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
> dev_dbg(&client->dev, "probing for EDT FT5x06 I2C\n");
>
> tsdata = devm_kzalloc(&client->dev, sizeof(*tsdata), GFP_KERNEL);
> - if (!tsdata) {
> - dev_err(&client->dev, "failed to allocate driver data.\n");
> + if (!tsdata)
> return -ENOMEM;
> - }
>
> chip_data = of_device_get_match_data(&client->dev);
> if (!chip_data)
> --
> 2.16.0
^ permalink raw reply
* Re: [PATCH v4 1/2] Input: Add driver for Cypress Generation 5 touchscreen
From: Maxime Ripard @ 2018-01-22 12:50 UTC (permalink / raw)
To: dmitry.torokhov, Mylène Josserand
Cc: robh+dt, mark.rutland, linux-kernel, linux-input, devicetree,
thomas.petazzoni
In-Reply-To: <20171201153957.13053-2-mylene.josserand@free-electrons.com>
[-- Attachment #1: Type: text/plain, Size: 885 bytes --]
Dmitry,
On Fri, Dec 01, 2017 at 04:39:56PM +0100, Mylène Josserand wrote:
> This is the basic driver for the Cypress TrueTouch Gen5 touchscreen
> controllers. This driver supports only the I2C bus but it uses regmap
> so SPI support could be added later.
> The touchscreen can retrieve some defined zone that are handled as
> buttons (according to the hardware). That is why it handles
> button and multitouch events.
>
> Reviewed-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> Signed-off-by: Mylène Josserand <mylene.josserand@free-electrons.com>
Can we have some review here? This is the fourth iteration, with the
first having been sent back in May, and as far as I know we never got
any review from you. Can we please move forward on this?
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v2 1/2] Input: edt-ft5x06 - Add support for regulator
From: Maxime Ripard @ 2018-01-22 12:51 UTC (permalink / raw)
To: dmitry.torokhov, Mylène Josserand
Cc: robh+dt, mark.rutland, linux, wens, linux-arm-kernel, linux-input,
devicetree, linux-kernel, thomas.petazzoni, quentin.schulz
In-Reply-To: <20171228163336.28131-2-mylene.josserand@free-electrons.com>
[-- Attachment #1: Type: text/plain, Size: 325 bytes --]
Dmitry,
On Thu, Dec 28, 2017 at 05:33:35PM +0100, Mylène Josserand wrote:
> Add the support of regulator to use it as VCC source.
>
> Signed-off-by: Mylène Josserand <mylene.josserand@free-electrons.com>
Ping?
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH v1] Input: docs - use PROPERTY_ENTRY_U32() directly
From: Andy Shevchenko @ 2018-01-22 12:54 UTC (permalink / raw)
To: Dmitry Torokhov, linux-input, linux-doc, Jonathan Corbet; +Cc: Andy Shevchenko
Instead of using PROPERTY_ENTRY_INTEGER() with explicitly supplied type,
use PROPERTY_ENTRY_U32() dedicated macro.
It will help modify internals of built-in device properties API.
No functional change intended.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
Documentation/input/devices/rotary-encoder.rst | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/input/devices/rotary-encoder.rst b/Documentation/input/devices/rotary-encoder.rst
index b07b20a295ac..810ae02bdaa0 100644
--- a/Documentation/input/devices/rotary-encoder.rst
+++ b/Documentation/input/devices/rotary-encoder.rst
@@ -108,9 +108,9 @@ example below:
};
static const struct property_entry rotary_encoder_properties[] __initconst = {
- PROPERTY_ENTRY_INTEGER("rotary-encoder,steps-per-period", u32, 24),
- PROPERTY_ENTRY_INTEGER("linux,axis", u32, ABS_X),
- PROPERTY_ENTRY_INTEGER("rotary-encoder,relative_axis", u32, 0),
+ PROPERTY_ENTRY_U32("rotary-encoder,steps-per-period", 24),
+ PROPERTY_ENTRY_U32("linux,axis", ABS_X),
+ PROPERTY_ENTRY_U32("rotary-encoder,relative_axis", 0),
{ },
};
--
2.15.1
^ permalink raw reply related
* [PATCH] input: da9052_tsi: remove unused mutex
From: Marcus Folkesson @ 2018-01-22 15:52 UTC (permalink / raw)
To: Support Opensource, Dmitry Torokhov
Cc: Marcus Folkesson, linux-input, linux-kernel
Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---
drivers/input/touchscreen/da9052_tsi.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/input/touchscreen/da9052_tsi.c b/drivers/input/touchscreen/da9052_tsi.c
index 5a013bb7bcad..b5dfd5944cc3 100644
--- a/drivers/input/touchscreen/da9052_tsi.c
+++ b/drivers/input/touchscreen/da9052_tsi.c
@@ -26,7 +26,6 @@ struct da9052_tsi {
struct da9052 *da9052;
struct input_dev *dev;
struct delayed_work ts_pen_work;
- struct mutex mutex;
bool stopped;
bool adc_on;
};
--
2.15.1
^ permalink raw reply related
* Re: [PATCH v1] Input: docs - use PROPERTY_ENTRY_U32() directly
From: Dmitry Torokhov @ 2018-01-22 17:28 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: linux-input, linux-doc, Jonathan Corbet
In-Reply-To: <20180122125457.6226-1-andriy.shevchenko@linux.intel.com>
On Mon, Jan 22, 2018 at 02:54:57PM +0200, Andy Shevchenko wrote:
> Instead of using PROPERTY_ENTRY_INTEGER() with explicitly supplied type,
> use PROPERTY_ENTRY_U32() dedicated macro.
>
> It will help modify internals of built-in device properties API.
>
> No functional change intended.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Applied, thank you.
> ---
> Documentation/input/devices/rotary-encoder.rst | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/input/devices/rotary-encoder.rst b/Documentation/input/devices/rotary-encoder.rst
> index b07b20a295ac..810ae02bdaa0 100644
> --- a/Documentation/input/devices/rotary-encoder.rst
> +++ b/Documentation/input/devices/rotary-encoder.rst
> @@ -108,9 +108,9 @@ example below:
> };
>
> static const struct property_entry rotary_encoder_properties[] __initconst = {
> - PROPERTY_ENTRY_INTEGER("rotary-encoder,steps-per-period", u32, 24),
> - PROPERTY_ENTRY_INTEGER("linux,axis", u32, ABS_X),
> - PROPERTY_ENTRY_INTEGER("rotary-encoder,relative_axis", u32, 0),
> + PROPERTY_ENTRY_U32("rotary-encoder,steps-per-period", 24),
> + PROPERTY_ENTRY_U32("linux,axis", ABS_X),
> + PROPERTY_ENTRY_U32("rotary-encoder,relative_axis", 0),
> { },
> };
>
> --
> 2.15.1
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH] input: da9052_tsi: remove unused mutex
From: Dmitry Torokhov @ 2018-01-22 17:28 UTC (permalink / raw)
To: Marcus Folkesson; +Cc: Support Opensource, linux-input, linux-kernel
In-Reply-To: <20180122155254.9255-1-marcus.folkesson@gmail.com>
On Mon, Jan 22, 2018 at 04:52:53PM +0100, Marcus Folkesson wrote:
> Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
Applied, thank you.
> ---
> drivers/input/touchscreen/da9052_tsi.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/input/touchscreen/da9052_tsi.c b/drivers/input/touchscreen/da9052_tsi.c
> index 5a013bb7bcad..b5dfd5944cc3 100644
> --- a/drivers/input/touchscreen/da9052_tsi.c
> +++ b/drivers/input/touchscreen/da9052_tsi.c
> @@ -26,7 +26,6 @@ struct da9052_tsi {
> struct da9052 *da9052;
> struct input_dev *dev;
> struct delayed_work ts_pen_work;
> - struct mutex mutex;
> bool stopped;
> bool adc_on;
> };
> --
> 2.15.1
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2 1/2] Input: edt-ft5x06 - Add support for regulator
From: Dmitry Torokhov @ 2018-01-22 17:42 UTC (permalink / raw)
To: Mylène Josserand
Cc: Rob Herring, Mark Rutland, Russell King - ARM Linux,
Maxime Ripard, Chen-Yu Tsai,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-input-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA, lkml, Thomas Petazzoni,
Quentin Schulz
In-Reply-To: <20171228163336.28131-2-mylene.josserand-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Hi Mylène,
On Thu, Dec 28, 2017 at 8:33 AM, Mylène Josserand
<mylene.josserand-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> Add the support of regulator to use it as VCC source.
>
> Signed-off-by: Mylène Josserand <mylene.josserand-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
> .../bindings/input/touchscreen/edt-ft5x06.txt | 1 +
> drivers/input/touchscreen/edt-ft5x06.c | 33 ++++++++++++++++++++++
> 2 files changed, 34 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
> index 025cf8c9324a..48e975b9c1aa 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
> +++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
> @@ -30,6 +30,7 @@ Required properties:
> Optional properties:
> - reset-gpios: GPIO specification for the RESET input
> - wake-gpios: GPIO specification for the WAKE input
> + - vcc-supply: Regulator that supplies the touchscreen
>
> - pinctrl-names: should be "default"
> - pinctrl-0: a phandle pointing to the pin settings for the
> diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> index c53a3d7239e7..5ee14a25a382 100644
> --- a/drivers/input/touchscreen/edt-ft5x06.c
> +++ b/drivers/input/touchscreen/edt-ft5x06.c
> @@ -39,6 +39,7 @@
> #include <linux/input/mt.h>
> #include <linux/input/touchscreen.h>
> #include <linux/of_device.h>
> +#include <linux/regulator/consumer.h>
>
> #define WORK_REGISTER_THRESHOLD 0x00
> #define WORK_REGISTER_REPORT_RATE 0x08
> @@ -91,6 +92,7 @@ struct edt_ft5x06_ts_data {
> struct touchscreen_properties prop;
> u16 num_x;
> u16 num_y;
> + struct regulator *vcc;
>
> struct gpio_desc *reset_gpio;
> struct gpio_desc *wake_gpio;
> @@ -993,6 +995,23 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
>
> tsdata->max_support_points = chip_data->max_support_points;
>
> + tsdata->vcc = devm_regulator_get(&client->dev, "vcc");
> + if (IS_ERR(tsdata->vcc)) {
> + error = PTR_ERR(tsdata->vcc);
> + dev_err(&client->dev, "failed to request regulator: %d\n",
> + error);
> + return error;
> + };
As 0-day pounted out, this semicolon is not needed.
> +
> + if (tsdata->vcc) {
You do not need to check for non-NULL here, devm_regulator_get() wil
lnever give you a NULL. If regulator is not defined in DT/board
mappings, then dummy regulator will be provided. You can call
regulator_enable() and regulator_disable() and other regulator APIs
with dummy regulator.
> + error = regulator_enable(tsdata->vcc);
> + if (error < 0) {
> + dev_err(&client->dev, "failed to enable vcc: %d\n",
> + error);
> + return error;
> + }
> + }
> +
> tsdata->reset_gpio = devm_gpiod_get_optional(&client->dev,
> "reset", GPIOD_OUT_HIGH);
> if (IS_ERR(tsdata->reset_gpio)) {
> @@ -1122,20 +1141,34 @@ static int edt_ft5x06_ts_remove(struct i2c_client *client)
> static int __maybe_unused edt_ft5x06_ts_suspend(struct device *dev)
> {
> struct i2c_client *client = to_i2c_client(dev);
> + struct edt_ft5x06_ts_data *tsdata = i2c_get_clientdata(client);
>
> if (device_may_wakeup(dev))
> enable_irq_wake(client->irq);
>
> + if (tsdata->vcc)
Same here.
> + regulator_disable(tsdata->vcc);
> +
> return 0;
> }
>
> static int __maybe_unused edt_ft5x06_ts_resume(struct device *dev)
> {
> struct i2c_client *client = to_i2c_client(dev);
> + struct edt_ft5x06_ts_data *tsdata = i2c_get_clientdata(client);
> + int ret;
>
> if (device_may_wakeup(dev))
> disable_irq_wake(client->irq);
>
> + if (tsdata->vcc) {
And here.
> + ret = regulator_enable(tsdata->vcc);
> + if (ret < 0) {
> + dev_err(dev, "failed to enable vcc: %d\n", ret);
> + return ret;
> + }
Since power to the device may have been cut, I think you need to
restore the register settings to whatever it was (factory vs work
mode, threshold, gain and offset registers, etc, etc).
> + }
> +
> return 0;
> }
>
> --
> 2.11.0
>
Thanks.
--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 5/7] Input: libps2 - add debugging statements
From: Dmitry Torokhov @ 2018-01-22 18:33 UTC (permalink / raw)
To: ulrik.debie-os
Cc: Benjamin Tissoires, Hans de Goede, Lyude Paul, linux-input,
linux-kernel, Peter Hutterer
In-Reply-To: <20180121202230.GB23310@beacon.debie>
H Ulrik,
On Sun, Jan 21, 2018 at 09:22:31PM +0100, ulrik.debie-os@e2big.org wrote:
>
> Hi,
>
> the patch series looks nice, but best to also include the procedure
> described here in Documentation/input somewhere. I would not expect
> everyone to dig into git commit logs to find the howto :)
Right now we do not have any notes in kernel on debugging PS/2 devices,
although we have a bunch of tools (ps2emu), etc. I wonder if I can get
Benjamin or Lyude or Hans or Peter write something up as they've been
doing a lot of touchpad bringups...
Thanks.
>
> Best regards,
> Ulrik
>
>
> On Fri, Jan 19, 2018 at 11:41:09AM -0800, Dmitry Torokhov wrote:
> > Date: Fri, 19 Jan 2018 11:41:09 -0800
> > From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > To: Benjamin Tissoires <benjamin.tissoires@redhat.com>, Hans de Goede
> > <hdegoede@redhat.com>, Lyude Paul <lyude@redhat.com>
> > Cc: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
> > Subject: [PATCH 5/7] Input: libps2 - add debugging statements
> > X-Mailer: git-send-email 2.16.0.rc1.238.g530d649a79-goog
> > X-Mailing-List: linux-input@vger.kernel.org
> >
> > Debugging via i8042.debug and analyzing raw PS/2 data stream may be
> > cumbersome as you need to locate the boundaries of commands, decipher the
> > sliced commands, etc, etc. Let's add a bit more high level debug statements
> > for ps2_sendbyte(), ps2_command(), and ps2_sliced_command().
> >
> > We do not introduce a new module parameter, but rater rely on the kernel
> > having dynamic debug facility enabled (which most everyone has nowadays).
> > Enable with:
> >
> > echo "file libps2.c +pf" > /sys/kernel/debug/dynamic_debug/control
> >
> > or add "libps2.dyndbg=+pf" to the kernel command line.
> >
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> > drivers/input/serio/libps2.c | 52 +++++++++++++++++++++++++++++++++-----------
> > 1 file changed, 39 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/input/serio/libps2.c b/drivers/input/serio/libps2.c
> > index e96ae477f0b56..82befae4dab04 100644
> > --- a/drivers/input/serio/libps2.c
> > +++ b/drivers/input/serio/libps2.c
> > @@ -26,22 +26,20 @@ MODULE_AUTHOR("Dmitry Torokhov <dtor@mail.ru>");
> > MODULE_DESCRIPTION("PS/2 driver library");
> > MODULE_LICENSE("GPL");
> >
> > -/*
> > - * ps2_sendbyte() sends a byte to the device and waits for acknowledge.
> > - * It doesn't handle retransmission, though it could - because if there
> > - * is a need for retransmissions device has to be replaced anyway.
> > - *
> > - * ps2_sendbyte() can only be called from a process context.
> > - */
> > -
> > -int ps2_sendbyte(struct ps2dev *ps2dev, u8 byte, unsigned int timeout)
> > +static int ps2_do_sendbyte(struct ps2dev *ps2dev, u8 byte, unsigned int timeout)
> > {
> > + int error;
> > +
> > serio_pause_rx(ps2dev->serio);
> > ps2dev->nak = 1;
> > ps2dev->flags |= PS2_FLAG_ACK;
> > serio_continue_rx(ps2dev->serio);
> >
> > - if (serio_write(ps2dev->serio, byte) == 0)
> > + error = serio_write(ps2dev->serio, byte);
> > + if (error)
> > + dev_dbg(&ps2dev->serio->dev,
> > + "failed to write %#02x: %d\n", byte, error);
> > + else
> > wait_event_timeout(ps2dev->wait,
> > !(ps2dev->flags & PS2_FLAG_ACK),
> > msecs_to_jiffies(timeout));
> > @@ -52,6 +50,24 @@ int ps2_sendbyte(struct ps2dev *ps2dev, u8 byte, unsigned int timeout)
> >
> > return -ps2dev->nak;
> > }
> > +
> > +/*
> > + * ps2_sendbyte() sends a byte to the device and waits for acknowledge.
> > + * It doesn't handle retransmission, though it could - because if there
> > + * is a need for retransmissions device has to be replaced anyway.
> > + *
> > + * ps2_sendbyte() can only be called from a process context.
> > + */
> > +
> > +int ps2_sendbyte(struct ps2dev *ps2dev, u8 byte, unsigned int timeout)
> > +{
> > + int retval;
> > +
> > + retval = ps2_do_sendbyte(ps2dev, byte, timeout);
> > + dev_dbg(&ps2dev->serio->dev, "%02x - %x\n", byte, ps2dev->nak);
> > +
> > + return retval;
> > +}
> > EXPORT_SYMBOL(ps2_sendbyte);
> >
> > void ps2_begin_command(struct ps2dev *ps2dev)
> > @@ -186,6 +202,7 @@ int __ps2_command(struct ps2dev *ps2dev, u8 *param, unsigned int command)
> > unsigned int receive = (command >> 8) & 0xf;
> > int rc = -1;
> > int i;
> > + u8 send_param[16];
> >
> > if (receive > sizeof(ps2dev->cmdbuf)) {
> > WARN_ON(1);
> > @@ -197,6 +214,8 @@ int __ps2_command(struct ps2dev *ps2dev, u8 *param, unsigned int command)
> > return -1;
> > }
> >
> > + memcpy(send_param, param, send);
> > +
> > serio_pause_rx(ps2dev->serio);
> > ps2dev->flags = command == PS2_CMD_GETID ? PS2_FLAG_WAITID : 0;
> > ps2dev->cmdcnt = receive;
> > @@ -210,14 +229,14 @@ int __ps2_command(struct ps2dev *ps2dev, u8 *param, unsigned int command)
> > * ACKing the reset command, and so it can take a long
> > * time before the ACK arrives.
> > */
> > - if (ps2_sendbyte(ps2dev, command & 0xff,
> > - command == PS2_CMD_RESET_BAT ? 1000 : 200)) {
> > + if (ps2_do_sendbyte(ps2dev, command & 0xff,
> > + command == PS2_CMD_RESET_BAT ? 1000 : 200)) {
> > serio_pause_rx(ps2dev->serio);
> > goto out_reset_flags;
> > }
> >
> > for (i = 0; i < send; i++) {
> > - if (ps2_sendbyte(ps2dev, param[i], 200)) {
> > + if (ps2_do_sendbyte(ps2dev, param[i], 200)) {
> > serio_pause_rx(ps2dev->serio);
> > goto out_reset_flags;
> > }
> > @@ -253,6 +272,12 @@ int __ps2_command(struct ps2dev *ps2dev, u8 *param, unsigned int command)
> > ps2dev->flags = 0;
> > serio_continue_rx(ps2dev->serio);
> >
> > + dev_dbg(&ps2dev->serio->dev,
> > + "%02x [%*ph] - %x/%08lx [%*ph]\n",
> > + command & 0xff, send, send_param,
> > + ps2dev->nak, ps2dev->flags,
> > + receive, param ?: send_param);
> > +
> > return rc;
> > }
> > EXPORT_SYMBOL(__ps2_command);
> > @@ -296,6 +321,7 @@ int ps2_sliced_command(struct ps2dev *ps2dev, u8 command)
> > }
> >
> > out:
> > + dev_dbg(&ps2dev->serio->dev, "%02x - %d\n", command, retval);
> > ps2_end_command(ps2dev);
> > return retval;
> > }
> > --
> > 2.16.0.rc1.238.g530d649a79-goog
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-input" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Dmitry
^ permalink raw reply
* Re: [PATCH 5/7] Input: psmouse - add support for 2nd wheel on A4Tech Dual-Scroll wheel mice
From: Dmitry Torokhov @ 2018-01-22 18:39 UTC (permalink / raw)
To: Michel Hermier
Cc: Benjamin Tissoires, Hans de Goede, Lyude Paul, Stephen Lyons,
linux-input, LKML
In-Reply-To: <CAAZ5spDzNfrosHfCYFtPcQ2bZiE5iSn8Ac40yNNHR-mdjzt83w@mail.gmail.com>
Hi Michel,
On Sat, Jan 20, 2018 at 11:02:50AM +0100, Michel Hermier wrote:
> Hi,
> I think this expose a little problem in the driver: The lack of a
> feature/quirk flags in the struct psmouse_protocol, to replace the 4 bools.
> Maybe I'm mistaken but this driver is legacy, so I doubt your patch would
> be accepted, or a more proper refactoring.
> My 2 cents as a powerless reviewer.
Flags are more compact, separate module parameters are more user
friendly. If we see a need for more quirks yet we may revisit the
situation and add flags.
As far as the dirver being legacy - yes, this is somewhat true, we are
slowly abandoning PS/2 in favor of I2C (and HID). Most of the PS/2
support goes into advanced protocols like ALPS and trackpoint, not basic
PS/2 handling.
> Cheers
>
> Le 20 janv. 2018 12:09 AM, "Dmitry Torokhov" <dmitry.torokhov@gmail.com> a
> écrit :
>
> > From: Stephen Lyons <slysven@virginmedia.com>
> >
> > This Far-Eastern company's PS/2 mice use a deviant format for the data
> > relating to movement of the scroll wheels for, at least, their dual wheel
> > mice, such as their "Optical GreatEye Wheelmouse" model "WOP-35". This
> > product has five "buttons" (one of which is the click action on the first
> > wheel) and TWO scroll wheels. However for a byte comprising d0-d7 instead
> > of setting one of d6-7 in the forth byte of the mouse data packet and a
> > twos complement number of scroll steps in the remaining d5-d0 (or d3-d0
> > should there be a fourth (BTN_SIDE - d4) or fifth (BTN_EXTRA - d5) button
> > to report; they only report a single +/- event for each wheel and use a bit
> > pattern that corresponds to +/-1 for the first wheel and +/- 2 for the
> > second in the lower nibble of the fourth byte.
> >
> > The effect with existing code is that the second mouse wheel merely repeats
> > the effect of the first but providing two steps per click rather than the
> > one of the first wheel - so there is no HORIZONTAL scroll wheel movement
> > detected from the device as far as the rest of the kernel sees it.
> >
> > This patch, if enabled by the "a4tech_workaround" module parameter modifies
> > the handling just for mice of type PSMOUSE_IMEX so that the second scroll
> > wheel movement gets correctly reported as REL_HWHEEL events. Should this
> > module parameter be activated for other mice of the same PSMOUSE_IMEX type
> > then it is possible that at the point where the mouse reports more than a
> > single movement step the user may start seeing horizontal rather than
> > vertical wheel events, but should the movement steps get to be more than
> > two at a time the hack will get immediately deactivated and the behaviour
> > will revert to the past code.
> >
> > This was discussed around *fifteen* *years* *ago* on the LKML and the best
> > summary is in post https://lkml.org/lkml/2002/7/18/111 "Re: PS2 Input Core
> > Support" by Vojtech Pavlik. I was not able to locate any discussion later
> > than this on this topic.
> >
> > Given that most users of the "psmouse" module will NOT want this additional
> > feature enabled I have taken the apparently erroneous step of defaulting
> > the module parameter that enables it to be "disabled" - this functionality
> > may interfere with the operation of "normal" mice of this type (until a
> > large enough scroll wheel movement is detected) so I cannot see how it
> > would want to be enabled for "normal" users - i.e. everyone without this
> > brand of mouse.
> >
> > I am using this patch at the moment and I can confirm that it is working
> > for me as both a module and compiled into the kernel for my mouse that is
> > of the type (WOP-35) described - I note that it is still available from
> > certain on-line retailers and that the manufacturers site does not list
> > GNU/Linux as being supported on the product page - this patch however does
> > enable full use of this product:
> > http://www.a4tech.com/product.asp?cid=3D1&scid=3D8&id=3D22
> >
> > Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> > drivers/input/mouse/psmouse-base.c | 19 +++++++++++++++++--
> > 1 file changed, 17 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/input/mouse/psmouse-base.c
> > b/drivers/input/mouse/psmouse-base.c
> > index cbca668bb931f..d0e45124e7f43 100644
> > --- a/drivers/input/mouse/psmouse-base.c
> > +++ b/drivers/input/mouse/psmouse-base.c
> > @@ -70,6 +70,10 @@ static bool psmouse_smartscroll = true;
> > module_param_named(smartscroll, psmouse_smartscroll, bool, 0644);
> > MODULE_PARM_DESC(smartscroll, "Logitech Smartscroll autorepeat, 1 =
> > enabled (default), 0 = disabled.");
> >
> > +static bool psmouse_a4tech_2wheels;
> > +module_param_named(a4tech_workaround, psmouse_a4tech_2wheels, bool,
> > 0644);
> > +MODULE_PARM_DESC(a4tech_workaround, "A4Tech second scroll wheel
> > workaround, 1 = enabled, 0 = disabled (default).");
> > +
> > static unsigned int psmouse_resetafter = 5;
> > module_param_named(resetafter, psmouse_resetafter, uint, 0644);
> > MODULE_PARM_DESC(resetafter, "Reset device after so many bad packets (0 =
> > never).");
> > @@ -150,6 +154,7 @@ psmouse_ret_t psmouse_process_byte(struct psmouse
> > *psmouse)
> > {
> > struct input_dev *dev = psmouse->dev;
> > u8 *packet = psmouse->packet;
> > + int wheel;
> >
> > if (psmouse->pktcnt < psmouse->pktsize)
> > return PSMOUSE_GOOD_DATA;
> > @@ -175,8 +180,18 @@ psmouse_ret_t psmouse_process_byte(struct psmouse
> > *psmouse)
> > break;
> > case 0x00:
> > case 0xC0:
> > - input_report_rel(dev, REL_WHEEL,
> > - -sign_extend32(packet[3], 3));
> > + wheel = sign_extend32(packet[3], 3);
> > +
> > + /*
> > + * Some A4Tech mice have two scroll wheels, with
> > first
> > + * one reporting +/-1 in the lower nibble, and
> > second
> > + * one reporting +/-2.
> > + */
> > + if (psmouse_a4tech_2wheels && abs(wheel) > 1)
> > + input_report_rel(dev, REL_HWHEEL, wheel /
> > 2);
> > + else
> > + input_report_rel(dev, REL_WHEEL, -wheel);
> > +
> > input_report_key(dev, BTN_SIDE, BIT(4));
> > input_report_key(dev, BTN_EXTRA, BIT(5));
> > break;
> > --
> > 2.16.0.rc1.238.g530d649a79-goog
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-input" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Revert "Input: trackpoint - add new trackpoint firmware ID"
From: Dmitry Torokhov @ 2018-01-22 18:41 UTC (permalink / raw)
To: ulrik.debie-os; +Cc: Aaron Ma, Greg KH, Sebastian Schmidt, linux-input
In-Reply-To: <02711055-0359-46D3-B9DD-9182918A1632@gmail.com>
On Sun, Jan 21, 2018 at 01:08:17PM -0800, Dmitry Torokhov wrote:
> On January 21, 2018 12:37:01 PM PST, ulrik.debie-os@e2big.org wrote:
> >On Tue, Jan 16, 2018 at 03:49:19PM -0800, Dmitry Torokhov wrote:
> >>
> >> Hi Ulrik,
> >>
> >>
> >> On Sun, Jan 14, 2018 at 09:39:59PM +0100, ulrik.debie-os@e2big.org
> >wrote:
> >> > Hi Dmitry, Sebastian,
> >> > Nack for the patch..
> >>
> >> NAK because of the issue pointed by Sebastian, or bigger
> >disagreement?
> >I originally had 2 reasons:
> >1) issue pointed out by Sebastian, of course easy to fix
> >2) The remark I had below which was because I did not understand yet
> >the reason
> >why you made this patch in the first place. I now understand it is not
> >the
> >purpose to change anything for the Lenovo e470/e570 and it will also
> >not change
> >anything for those if you at least fix nr 1)
>
> I see. Yes, I did fix the problem mentioned by sensation of course, I did not bother posting updated patch because the change is trivial.
Gah, that should have read "Sebastian", not "sensation". Damn phones and
their swipe/autocorrect keyboards.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2] Input: Add support for PDP Xbox One controllers
From: Dmitry Torokhov @ 2018-01-22 19:25 UTC (permalink / raw)
To: Cameron Gutman
Cc: Mark Furneaux (by way of Mark Furneaux <mark@furneaux.ca>),
linux-input
In-Reply-To: <c3e05db2-801e-72d6-ebce-6a27ffaef538@gmail.com>
On Mon, Dec 11, 2017 at 09:42:11PM -0800, Cameron Gutman wrote:
> On 12/10/2017 03:52 PM, Mark Furneaux (by way of Mark Furneaux <mark@furneaux.ca>) wrote:
> > Input: Add support for PDP Xbox One controllers
> >
> > Adds support for the current lineup of Xbox One controllers from PDP
> > (Performance Designed Products). These controllers are very picky with
> > their initialization sequence and require an additional 2 packets before
> > they send any input reports.
> >
> > Signed-off-by: Mark Furneaux <mark@furneaux.ca>
> >
> > ---
> > Changes in v2:
> >
> > - Zeroed the sequence numbers and removed the redundant quirk entry.
> > - Corrected mangled formatting from my email client.
> >
>
> Thanks, LGTM.
>
> Reviewed-by: Cameron Gutman <aicommander@gmail.com>
Applied and earmarked for stable, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: ads7846 - use managed allocated resources
From: Dmitry Torokhov @ 2018-01-22 19:58 UTC (permalink / raw)
To: Andi Shyti; +Cc: linux-input, linux-kernel, Andi Shyti
In-Reply-To: <20171109140038.19771-1-andi@etezian.org>
Hi Andi,
On Thu, Nov 09, 2017 at 04:00:38PM +0200, Andi Shyti wrote:
> @@ -1462,31 +1435,11 @@ static int ads7846_remove(struct spi_device *spi)
> {
> struct ads7846 *ts = spi_get_drvdata(spi);
>
> - sysfs_remove_group(&spi->dev.kobj, &ads784x_attr_group);
> -
> ads7846_disable(ts);
> - free_irq(ts->spi->irq, ts);
> -
> - input_unregister_device(ts->input);
> -
> - ads784x_hwmon_unregister(spi, ts);
> -
> - regulator_put(ts->reg);
> -
> - if (!ts->get_pendown_state) {
> - /*
> - * If we are not using specialized pendown method we must
> - * have been relying on gpio we set up ourselves.
> - */
> - gpio_free(ts->gpio_pendown);
> - }
>
> if (ts->filter_cleanup)
> ts->filter_cleanup(ts->filter_data);
You need to either remove custom filter support from the driver, or
install an action to use devm* to do filter cleanup. Given that, as far
as I can see, we do not have users of custom filters in mainline, and
they are not compatible with DT-based systems, I'd rather we removed
them.
>
> - kfree(ts->packet);
> - kfree(ts);
> -
> dev_dbg(&spi->dev, "unregistered touchscreen\n");
>
> return 0;
> --
> 2.15.0
>
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v4 1/2] Input: Add driver for Cypress Generation 5 touchscreen
From: Marcus Folkesson @ 2018-01-22 20:11 UTC (permalink / raw)
To: Mylène Josserand
Cc: dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8
In-Reply-To: <20171201153957.13053-2-mylene.josserand-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 6537 bytes --]
Mylene,
On Fri, Dec 01, 2017 at 04:39:56PM +0100, Mylène Josserand wrote:
> +++ b/drivers/input/touchscreen/cyttsp5.c
> @@ -0,0 +1,1110 @@
> +/*
> + * Parade TrueTouch(TM) Standard Product V5 Module.
> + * For use with Parade touchscreen controllers.
> + *
> + * Copyright (C) 2015 Parade Technologies
> + * Copyright (C) 2012-2015 Cypress Semiconductor
> + * Copyright (C) 2017 Free Electrons
> + *
> + * Author: Mylène Josserand <mylene.josserand-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * version 2, and only version 2, as published by the
> + * Free Software Foundation.
Please use SPDX license Identifier to describe the license.
E.g.
SPDX-License-Identifier: GPL-2.0
Also, the MODULE_LICENSE means GPL 2.0 or later per module.h and this does
not match the license description.
Could you make sure they are in sync?
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <asm/unaligned.h>
> +#include <linux/crc-itu-t.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/gpio.h>
> +#include <linux/input/mt.h>
> +#include <linux/input/touchscreen.h>
> +#include <linux/interrupt.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/regmap.h>
#include <linux/bitops.h>
since you are using BIT()
[snip]
> +static int cyttsp5_setup_input_device(struct device *dev)
> +{
> + struct cyttsp5 *ts = dev_get_drvdata(dev);
> + struct cyttsp5_sysinfo *si = &ts->sysinfo;
> + int max_x, max_y, max_p;
> + int max_x_tmp, max_y_tmp;
> + int rc;
> +
> + __set_bit(EV_ABS, ts->input->evbit);
> + __set_bit(EV_REL, ts->input->evbit);
> + __set_bit(EV_KEY, ts->input->evbit);
> +
> + max_x_tmp = si->sensing_conf_data.res_x;
> + max_y_tmp = si->sensing_conf_data.res_y;
> + max_x = max_y_tmp - 1;
> + max_y = max_x_tmp - 1;
> + max_p = si->sensing_conf_data.max_z;
> +
> + input_mt_init_slots(ts->input, si->tch_abs[CY_TCH_T].max, 0);
> +
> + __set_bit(ABS_MT_POSITION_X, ts->input->absbit);
> + __set_bit(ABS_MT_POSITION_Y, ts->input->absbit);
> + __set_bit(ABS_MT_PRESSURE, ts->input->absbit);
Not needed, input_set_abs_params() will set the bits for you below.
> +
> + input_set_abs_params(ts->input, ABS_MT_POSITION_X, 0, max_x, 0, 0);
> + input_set_abs_params(ts->input, ABS_MT_POSITION_Y, 0, max_y, 0, 0);
> + input_set_abs_params(ts->input, ABS_MT_PRESSURE, 0, max_p, 0, 0);
> +
> + input_set_abs_params(ts->input, ABS_MT_TOUCH_MAJOR, 0, MAX_AREA, 0, 0);
> + input_set_abs_params(ts->input, ABS_MT_TOUCH_MINOR, 0, MAX_AREA, 0, 0);
> +
> + rc = input_register_device(ts->input);
> + if (rc < 0)
> + dev_err(dev, "Error, failed register input device r=%d\n", rc);
> +
> + return rc;
> +}
> +
> +
[snip]
> +static int cyttsp5_get_hid_descriptor(struct cyttsp5 *ts,
> + struct cyttsp5_hid_desc *desc)
> +{
> + struct device *dev = ts->dev;
> + __le16 hid_desc_register = HID_DESC_REG;
> + int rc;
> + u8 cmd[2];
> +
> + /* Read HID descriptor length and version */
> + mutex_lock(&ts->system_lock);
> + ts->hid_cmd_state = HID_CMD_BUSY;
> + mutex_unlock(&ts->system_lock);
> +
> + /* Set HID descriptor register */
> + memcpy(cmd, &hid_desc_register, sizeof(hid_desc_register));
> +
> + rc = cyttsp5_write(ts, HID_DESC_REG, NULL, 0);
> + if (rc < 0) {
> + dev_err(dev, "Failed to get HID descriptor, rc=%d\n", rc);
> + goto error;
> + }
> +
> + rc = wait_event_timeout(ts->wait_q, (ts->hid_cmd_state == HID_CMD_DONE),
> + msecs_to_jiffies(CY_HID_GET_HID_DESCRIPTOR_TIMEOUT));
> + if (!rc) {
> + dev_err(ts->dev, "HID get descriptor timed out\n");
> + rc = -ETIME;
> + goto error;
> + }
> +
> + memcpy(desc, ts->response_buf, sizeof(struct cyttsp5_hid_desc));
> +
> + /* Check HID descriptor length and version */
> + if (le16_to_cpu(desc->hid_desc_len) != sizeof(*desc) ||
> + le16_to_cpu(desc->bcd_version) != HID_VERSION) {
> + dev_err(dev, "Unsupported HID version\n");
> + return -ENODEV;
Maybe it is supposed to return here, but all other faults use "goto
error".
> + }
> +
> + goto exit;
> +
> +error:
> + mutex_lock(&ts->system_lock);
> + ts->hid_cmd_state = HID_CMD_DONE;
> + mutex_unlock(&ts->system_lock);
> +exit:
> + return rc;
> +}
> +
[snip]
> +static int cyttsp5_probe(struct device *dev, struct regmap *regmap, int irq,
> + const char *name)
> +{
> + struct cyttsp5 *ts;
> + struct cyttsp5_sysinfo *si;
> + int rc = 0, i;
> +
> + ts = devm_kzalloc(dev, sizeof(*ts), GFP_KERNEL);
> + if (!ts)
> + return -ENOMEM;
> +
> + /* Initialize device info */
> + ts->regmap = regmap;
> + ts->dev = dev;
> + si = &ts->sysinfo;
> + dev_set_drvdata(dev, ts);
> +
> + /* Initialize mutexes and spinlocks */
No spinlocks here :-)
> + mutex_init(&ts->system_lock);
> +
> + /* Initialize wait queue */
> + init_waitqueue_head(&ts->wait_q);
> +
> + /* Reset the gpio to be in a reset state */
> + ts->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
> + if (IS_ERR(ts->reset_gpio)) {
> + rc = PTR_ERR(ts->reset_gpio);
> + dev_err(dev, "Failed to request reset gpio, error %d\n", rc);
> + return rc;
> + }
> + gpiod_set_value(ts->reset_gpio, 1);
> +
[snip]
> +static struct i2c_driver cyttsp5_i2c_driver = {
> + .driver = {
> + .name = CYTTSP5_NAME,
> + .owner = THIS_MODULE,
> + .of_match_table = of_match_ptr(cyttsp5_of_match),
> + },
> + .probe = cyttsp5_i2c_probe,
> + .remove = cyttsp5_i2c_remove,
> + .id_table = cyttsp5_i2c_id,
> +};
> +
> +module_i2c_driver(cyttsp5_i2c_driver);
> +
> +MODULE_LICENSE("GPL");
From linux/module.h:
/*
* The following license idents are currently accepted as indicating free
* software modules
*
* "GPL" [GNU Public License v2 or later]
* "GPL v2" [GNU Public License v2]
> +MODULE_DESCRIPTION("Touchscreen driver for Cypress TrueTouch Gen 5 Product");
> +MODULE_AUTHOR("Mylène Josserand <mylene.josserand-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>");
> --
> 2.11.0
>
Best regards
Marcus Folkesson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH] Input: stmfts: set IRQ_NOAUTOEN to the irq flag
From: Dmitry Torokhov @ 2018-01-23 1:37 UTC (permalink / raw)
To: Andi Shyti; +Cc: linux-input, linux-kernel, Andi Shyti
In-Reply-To: <20170927125831.11759-1-andi.shyti@samsung.com>
On Wed, Sep 27, 2017 at 09:58:31PM +0900, Andi Shyti wrote:
> The interrupt is requested before the device is powered on and
> it's value in some cases cannot be reliable. It happens on some
> devices that an interrupt is generated as soon as requested
> before having the chance to disable the irq.
>
> Set the irq flag as IRQ_NOAUTOEN before requesting it.
>
> This patch mutes the error:
>
> stmfts 2-0049: failed to read events: -11
>
> received sometimes during boot time.
>
> Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
Applied, thank you.
> ---
> drivers/input/touchscreen/stmfts.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/input/touchscreen/stmfts.c b/drivers/input/touchscreen/stmfts.c
> index 2351199189a4..362de0001b33 100644
> --- a/drivers/input/touchscreen/stmfts.c
> +++ b/drivers/input/touchscreen/stmfts.c
> @@ -689,6 +689,14 @@ static int stmfts_probe(struct i2c_client *client,
>
> input_set_drvdata(sdata->input, sdata);
>
> + /*
> + * stmfts_power_on expects interrupt to be disabled, but
> + * at this point the device is still off and I do not trust
> + * the status of the irq line that can generate some spurious
> + * interrupts. To be on the safe side it's better to not enable
> + * the interrupts during their request.
> + */
> + irq_set_status_flags(client->irq, IRQ_NOAUTOEN);
> err = devm_request_threaded_irq(&client->dev, client->irq,
> NULL, stmfts_irq_handler,
> IRQF_ONESHOT,
> @@ -696,9 +704,6 @@ static int stmfts_probe(struct i2c_client *client,
> if (err)
> return err;
>
> - /* stmfts_power_on expects interrupt to be disabled */
> - disable_irq(client->irq);
> -
> dev_dbg(&client->dev, "initializing ST-Microelectronics FTS...\n");
>
> err = stmfts_power_on(sdata);
> --
> 2.14.2
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH v3 2/3] Input: ad7897 - use devm_add_action_or_reset to disable the device
From: Dmitry Torokhov @ 2018-01-23 1:40 UTC (permalink / raw)
To: Andi Shyti
Cc: Michael Hennerich, Lars-Peter Clausen, linux-input, linux-kernel,
Andi Shyti
In-Reply-To: <20171108155020.3851-3-andi@etezian.org>
Hi Andi,
On Wed, Nov 08, 2017 at 05:50:19PM +0200, Andi Shyti wrote:
> Use the ad7877_disable() as a custom action when the driver gets
> removed instead of calling it from the remove function.
>
> Because ad7877_remove() was just calling the disable function,
> get rid of it.
>
There is not reason why this change needs to be split from the first
one; I folded it back in and applied, thank you.
> CC: Michael Hennerich <michael.hennerich@analog.com>
> Signed-off-by: Andi Shyti <andi@etezian.org>
> ---
> drivers/input/touchscreen/ad7877.c | 20 +++++++-------------
> 1 file changed, 7 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/input/touchscreen/ad7877.c b/drivers/input/touchscreen/ad7877.c
> index dd59e4a5eb7d..04ab8fbc8184 100644
> --- a/drivers/input/touchscreen/ad7877.c
> +++ b/drivers/input/touchscreen/ad7877.c
> @@ -417,8 +417,10 @@ static irqreturn_t ad7877_irq(int irq, void *handle)
> return IRQ_HANDLED;
> }
>
> -static void ad7877_disable(struct ad7877 *ts)
> +static void ad7877_disable(void *data)
> {
> + struct ad7877 *ts = data;
> +
> mutex_lock(&ts->mutex);
>
> if (!ts->disabled) {
> @@ -712,6 +714,10 @@ static int ad7877_probe(struct spi_device *spi)
> if (!ts || !input_dev)
> return -ENOMEM;
>
> + err = devm_add_action_or_reset(&spi->dev, ad7877_disable, ts);
> + if (err)
> + return err;
> +
> spi_set_drvdata(spi, ts);
> ts->spi = spi;
> ts->input = input_dev;
> @@ -787,17 +793,6 @@ static int ad7877_probe(struct spi_device *spi)
> return input_register_device(input_dev);
> }
>
> -static int ad7877_remove(struct spi_device *spi)
> -{
> - struct ad7877 *ts = spi_get_drvdata(spi);
> -
> - ad7877_disable(ts);
> -
> - dev_dbg(&spi->dev, "unregistered touchscreen\n");
> -
> - return 0;
> -}
> -
> static int __maybe_unused ad7877_suspend(struct device *dev)
> {
> struct ad7877 *ts = dev_get_drvdata(dev);
> @@ -824,7 +819,6 @@ static struct spi_driver ad7877_driver = {
> .pm = &ad7877_pm,
> },
> .probe = ad7877_probe,
> - .remove = ad7877_remove,
> };
>
> module_spi_driver(ad7877_driver);
> --
> 2.15.0
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: stmfts,s6sy671 - add SPDX identifier
From: Dmitry Torokhov @ 2018-01-23 1:41 UTC (permalink / raw)
To: Andi Shyti; +Cc: linux-input, linux-kernel, Andi Shyti, Philippe Ombredanne
In-Reply-To: <20180109052331.GC11443@gangnam.samsung>
On Tue, Jan 09, 2018 at 02:23:31PM +0900, Andi Shyti wrote:
> Hi Dmitry,
>
> On Fri, Jan 05, 2018 at 08:49:58AM -0800, Dmitry Torokhov wrote:
> > Hi Andi,
> >
> > On Fri, Jan 05, 2018 at 06:57:15PM +0900, Andi Shyti wrote:
> > > Hi Dmitry,
> > >
> > > this is a kind ping, would you also mind giving me a feedback to
> >
> > Yes, sorry. Could you please split the patch for each driver
> > individually? Also, until we have an update to the CodingStyle doc
> > mandating the C++ style comments, I'd prefer keeping the original style
> > of comments. So // for the SPDX line and /* */ for the rest.
>
> I was actually following Linus guideline [1] [2]. I also had the
> same discussion previously in another context [3].
>
> Please let me know if you still want a mixed style of comments in
> the next patch.
Eh, I guess it does not matter in the grand scheme of things; applied as
is.
Thank you.
>
> Thanks,
> Andi
>
> [1] https://marc.info/?l=linux-kernel&m=151163713125320&w=2
> ...
> └-> https://marc.info/?l=linux-kernel&m=151163867325641&w=2
> ...
> └-> https://marc.info/?l=linux-kernel&m=151163867325641&w=2
>
> [2] https://marc.info/?l=linux-kernel&m=150964359922353
> [3] https://marc.info/?l=linux-kernel&m=151312974503109&w=2
--
Dmitry
^ permalink raw reply
* Re: [PATCH v4 1/2] Input: Add driver for Cypress Generation 5 touchscreen
From: Dmitry Torokhov @ 2018-01-23 4:15 UTC (permalink / raw)
To: Mylène Josserand
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8
In-Reply-To: <20171201153957.13053-2-mylene.josserand-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Hi Mylène,
On Fri, Dec 01, 2017 at 04:39:56PM +0100, Mylène Josserand wrote:
> This is the basic driver for the Cypress TrueTouch Gen5 touchscreen
> controllers. This driver supports only the I2C bus but it uses regmap
> so SPI support could be added later.
> The touchscreen can retrieve some defined zone that are handled as
> buttons (according to the hardware). That is why it handles
> button and multitouch events.
>
> Reviewed-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> Signed-off-by: Mylène Josserand <mylene.josserand-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
> drivers/input/touchscreen/Kconfig | 16 +
> drivers/input/touchscreen/Makefile | 1 +
> drivers/input/touchscreen/cyttsp5.c | 1110 +++++++++++++++++++++++++++++++++++
> 3 files changed, 1127 insertions(+)
> create mode 100644 drivers/input/touchscreen/cyttsp5.c
>
> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> index 0cfdb7cb610e..28eea6d5f1bb 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -238,6 +238,22 @@ config TOUCHSCREEN_CYTTSP4_SPI
> To compile this driver as a module, choose M here: the
> module will be called cyttsp4_spi.
>
> +config TOUCHSCREEN_CYTTSP5
> + tristate "Cypress TrueTouch Gen5 Touchscreen Driver"
> + depends on OF
Does it have to be? Please use generic device properties
(device_property_* API) and it can be used with ACPI and static board
files, of needed.
> + select REGMAP_I2C
> + select CRC_ITU_T
> + help
> + Driver for Parade TrueTouch Standard Product
> + Generation 5 touchscreen controllers.
> + I2C bus interface support only.
> +
> + Say Y here if you have a Cypress Gen5 touchscreen.
> + If unsure, say N.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called cyttsp5.
> +
> config TOUCHSCREEN_DA9034
> tristate "Touchscreen support for Dialog Semiconductor DA9034"
> depends on PMIC_DA903X
> diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
> index d2a2b3b7af27..e7d124901dd9 100644
> --- a/drivers/input/touchscreen/Makefile
> +++ b/drivers/input/touchscreen/Makefile
> @@ -26,6 +26,7 @@ obj-$(CONFIG_TOUCHSCREEN_CYTTSP_SPI) += cyttsp_spi.o
> obj-$(CONFIG_TOUCHSCREEN_CYTTSP4_CORE) += cyttsp4_core.o
> obj-$(CONFIG_TOUCHSCREEN_CYTTSP4_I2C) += cyttsp4_i2c.o cyttsp_i2c_common.o
> obj-$(CONFIG_TOUCHSCREEN_CYTTSP4_SPI) += cyttsp4_spi.o
> +obj-$(CONFIG_TOUCHSCREEN_CYTTSP5) += cyttsp5.o
> obj-$(CONFIG_TOUCHSCREEN_DA9034) += da9034-ts.o
> obj-$(CONFIG_TOUCHSCREEN_DA9052) += da9052_tsi.o
> obj-$(CONFIG_TOUCHSCREEN_DYNAPRO) += dynapro.o
> diff --git a/drivers/input/touchscreen/cyttsp5.c b/drivers/input/touchscreen/cyttsp5.c
> new file mode 100644
> index 000000000000..a41feea2cd5a
> --- /dev/null
> +++ b/drivers/input/touchscreen/cyttsp5.c
> @@ -0,0 +1,1110 @@
> +/*
> + * Parade TrueTouch(TM) Standard Product V5 Module.
> + * For use with Parade touchscreen controllers.
> + *
> + * Copyright (C) 2015 Parade Technologies
> + * Copyright (C) 2012-2015 Cypress Semiconductor
> + * Copyright (C) 2017 Free Electrons
> + *
> + * Author: Mylène Josserand <mylene.josserand-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * version 2, and only version 2, as published by the
> + * Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <asm/unaligned.h>
> +#include <linux/crc-itu-t.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/gpio.h>
> +#include <linux/input/mt.h>
> +#include <linux/input/touchscreen.h>
> +#include <linux/interrupt.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/regmap.h>
> +
> +#define CYTTSP5_NAME "cyttsp5"
> +#define CY_I2C_DATA_SIZE (2 * 256)
> +#define HID_VERSION 0x0100
> +#define CY_MAX_INPUT 512
> +#define CYTTSP5_PREALLOCATED_CMD_BUFFER 32
> +#define CY_BITS_PER_BTN 1
> +#define CY_NUM_BTN_EVENT_ID ((1 << CY_BITS_PER_BTN) - 1)
> +
> +#define MAX_AREA 255
> +#define HID_OUTPUT_BL_SOP 0x1
> +#define HID_OUTPUT_BL_EOP 0x17
> +#define HID_OUTPUT_BL_LAUNCH_APP 0x3B
> +#define HID_OUTPUT_BL_LAUNCH_APP_SIZE 11
> +#define HID_OUTPUT_GET_SYSINFO 0x2
> +#define HID_OUTPUT_GET_SYSINFO_SIZE 5
> +
> +#define HID_DESC_REG 0x1
> +#define HID_INPUT_REG 0x3
> +#define HID_OUTPUT_REG 0x4
> +
> +#define REPORT_ID_TOUCH 0x1
> +#define REPORT_ID_BTN 0x3
> +#define REPORT_SIZE_5 5
> +#define REPORT_SIZE_8 8
> +#define REPORT_SIZE_16 16
> +
> +/* Touch reports offsets */
> +/* Header offsets */
> +#define TOUCH_REPORT_DESC_HDR_CONTACTCOUNT 16
> +/* Record offsets */
> +#define TOUCH_REPORT_DESC_CONTACTID 8
> +#define TOUCH_REPORT_DESC_X 16
> +#define TOUCH_REPORT_DESC_Y 32
> +#define TOUCH_REPORT_DESC_P 48
> +#define TOUCH_REPORT_DESC_MAJ 56
> +#define TOUCH_REPORT_DESC_MIN 64
> +
> +/* HID */
> +#define HID_TOUCH_REPORT_ID 0x1
> +#define HID_BTN_REPORT_ID 0x3
> +#define HID_APP_RESPONSE_REPORT_ID 0x1F
> +#define HID_APP_OUTPUT_REPORT_ID 0x2F
> +#define HID_BL_RESPONSE_REPORT_ID 0x30
> +#define HID_BL_OUTPUT_REPORT_ID 0x40
> +
> +#define HID_OUTPUT_RESPONSE_REPORT_OFFSET 2
> +#define HID_OUTPUT_RESPONSE_CMD_OFFSET 4
> +#define HID_OUTPUT_RESPONSE_CMD_MASK 0x7F
> +
> +#define HID_SYSINFO_SENSING_OFFSET 33
> +#define HID_SYSINFO_BTN_OFFSET 48
> +#define HID_SYSINFO_BTN_MASK 0xFF
> +#define HID_SYSINFO_MAX_BTN 8
> +
> +/* Timeout in ms */
> +#define CY_HID_OUTPUT_TIMEOUT 200
> +#define CY_HID_OUTPUT_GET_SYSINFO_TIMEOUT 3000
> +#define CY_HID_GET_HID_DESCRIPTOR_TIMEOUT 4000
> +
> +/* maximum number of concurrent tracks */
> +#define TOUCH_REPORT_SIZE 10
> +#define TOUCH_INPUT_HEADER_SIZE 7
> +#define BTN_REPORT_SIZE 9
> +#define BTN_INPUT_HEADER_SIZE 5
> +
> +/* All usage pages for Touch Report */
> +#define TOUCH_REPORT_USAGE_PG_X 0x00010030
> +#define TOUCH_REPORT_USAGE_PG_Y 0x00010031
> +#define TOUCH_REPORT_USAGE_PG_P 0x000D0030
> +#define TOUCH_REPORT_USAGE_PG_CONTACTID 0x000D0051
> +#define TOUCH_REPORT_USAGE_PG_CONTACTCOUNT 0x000D0054
> +#define TOUCH_REPORT_USAGE_PG_MAJ 0xFF010062
> +#define TOUCH_REPORT_USAGE_PG_MIN 0xFF010063
> +#define TOUCH_COL_USAGE_PG 0x000D0022
> +
> +/* helpers */
> +#define HI_BYTE(x) (u8)(((x) >> 8) & 0xFF)
> +#define LOW_BYTE(x) (u8)((x) & 0xFF)
> +
> +/* System Information interface definitions */
> +struct cyttsp5_sensing_conf_data_dev {
> + u8 electrodes_x;
> + u8 electrodes_y;
> + __le16 len_x;
> + __le16 len_y;
> + __le16 res_x;
> + __le16 res_y;
> + __le16 max_z;
> + u8 origin_x;
> + u8 origin_y;
> + u8 btn;
> + u8 scan_mode;
> + u8 max_num_of_tch_per_refresh_cycle;
> +} __packed;
> +
> +struct cyttsp5_sensing_conf_data {
> + u16 res_x;
> + u16 res_y;
> + u16 max_z;
> + u16 len_x;
> + u16 len_y;
> + u8 origin_x;
> + u8 origin_y;
> + u8 max_tch;
> +};
> +
> +enum { HID_CMD_DONE, HID_CMD_BUSY } hid_cmd_state;
> +
> +enum cyttsp5_tch_abs { /* for ordering within the extracted touch data array */
> + CY_TCH_X, /* X */
> + CY_TCH_Y, /* Y */
> + CY_TCH_P, /* P (Z) */
> + CY_TCH_T, /* TOUCH ID */
> + CY_TCH_MAJ, /* TOUCH_MAJOR */
> + CY_TCH_MIN, /* TOUCH_MINOR */
> + CY_TCH_NUM_ABS,
> +};
> +
> +struct cyttsp5_tch_abs_params {
> + size_t ofs; /* abs byte offset */
> + size_t size; /* size in bits */
> + size_t min; /* min value */
> + size_t max; /* max value */
> + size_t bofs; /* bit offset */
> +};
> +
> +struct cyttsp5_touch {
> + int hdr;
> + int abs[CY_TCH_NUM_ABS];
> +};
> +
> +struct cyttsp5_sysinfo {
> + struct cyttsp5_sensing_conf_data sensing_conf_data;
> + int num_btns;
> + struct cyttsp5_tch_abs_params tch_hdr;
> + struct cyttsp5_tch_abs_params tch_abs[CY_TCH_NUM_ABS];
> + u32 key_code[HID_SYSINFO_MAX_BTN];
> + u8 *xy_mode;
> + u8 *xy_data;
> +};
> +
> +struct cyttsp5_hid_desc {
> + __le16 hid_desc_len;
> + u8 packet_id;
> + u8 reserved_byte;
> + __le16 bcd_version;
> + __le16 report_desc_len;
> + __le16 report_desc_register;
> + __le16 input_register;
> + __le16 max_input_len;
> + __le16 output_register;
> + __le16 max_output_len;
> + __le16 command_register;
> + __le16 data_register;
> + __le16 vendor_id;
> + __le16 product_id;
> + __le16 version_id;
> + u8 reserved[4];
> +} __packed;
> +
> +struct cyttsp5 {
> + struct device *dev;
> + struct mutex system_lock;
> + wait_queue_head_t wait_q;
> + struct cyttsp5_sysinfo sysinfo;
> + int hid_cmd_state;
> + struct cyttsp5_hid_desc hid_desc;
> + u8 cmd_buf[CYTTSP5_PREALLOCATED_CMD_BUFFER];
> + u8 input_buf[CY_MAX_INPUT];
> + u8 response_buf[CY_MAX_INPUT];
> + struct gpio_desc *reset_gpio;
> + struct input_dev *input;
> + char phys[NAME_MAX];
> + int num_prv_rec;
> + struct regmap *regmap;
> +};
> +
> +/*
> + * For what understood in the datasheet, the register does not
> + * matter. For consistency, used the Input Register address
> + * but it does mean anything to the device. The important data
> + * to send is the I2C address
> + */
> +static int cyttsp5_read(struct cyttsp5 *ts, u8 *buf, u32 max)
> +{
> + int rc;
> + u32 size;
> + u8 temp[2];
> +
> + if (!buf)
> + return -EINVAL;
Is it really possible? This is not a publicly facing API but private
driver data, do you ever happen to call it with NULL?
> +
> + /* Read the frame to retrieve the size */
> + rc = regmap_bulk_read(ts->regmap, HID_INPUT_REG, temp, 2);
> + if (rc < 0)
> + return rc;
Can we call this kind of variables "error" pelase?
> +
> + size = get_unaligned_le16(temp);
> + if (!size || size == 2)
> + return 0;
> +
> + if (size > max)
> + return -EINVAL;
> +
> + /* Get the real value */
> + return regmap_bulk_read(ts->regmap, HID_INPUT_REG, buf, size);
> +}
> +
> +static int cyttsp5_write(struct cyttsp5 *ts, unsigned int reg, u8 *data,
> + size_t size)
> +{
> + u8 cmd[size + 1];
> +
> + /* High bytes of register address needed as first byte of cmd */
> + cmd[0] = HI_BYTE(reg);
> +
> + /* Copy the rest of the data */
> + if (data)
> + memcpy(&cmd[1], data, size);
> +
> + /* The hardware wants to receive a frame with the address register
The multiline comment block should start with "/*" on a separate line
please.
> + * contains in the first two bytes. As the regmap_write function
> + * add the register adresse in the frame, we use the LOW_BYTE as
> + * first frame byte for the address register and the first
> + * data byte is the high register + left of the cmd to send
> + */
> + return regmap_bulk_write(ts->regmap, LOW_BYTE(reg), cmd, size + 1);
> +}
> +
> +static void cyttsp5_final_sync(struct input_dev *input, int max_slots,
> + unsigned long *ids)
> +{
> + int t;
> +
> + for (t = 0; t < max_slots; t++) {
for_each_set_bit() {
}
> + if (test_bit(t, ids))
> + continue;
> + input_mt_slot(input, t);
> + input_mt_report_slot_state(input, MT_TOOL_FINGER, false);
> + }
> +
> + input_sync(input);
> +}
> +
> +static void cyttsp5_report_slot_liftoff(struct cyttsp5 *ts, int max_slots)
> +{
> + int t;
> +
> + if (ts->num_prv_rec == 0)
> + return;
Do you need this check? The caller checks the same condition.
> +
> + for (t = 0; t < max_slots; t++) {
> + input_mt_slot(ts->input, t);
> + input_mt_report_slot_state(ts->input, MT_TOOL_FINGER, false);
> + }
> +}
> +
> +static void cyttsp5_mt_lift_all(struct cyttsp5 *ts)
> +{
> + struct cyttsp5_sysinfo *si = &ts->sysinfo;
> + int max = si->tch_abs[CY_TCH_T].max;
> +
> + if (ts->num_prv_rec != 0) {
> + cyttsp5_report_slot_liftoff(ts, max);
> + input_sync(ts->input);
> + ts->num_prv_rec = 0;
> + }
> +}
> +
> +static void cyttsp5_get_touch_axis(int *axis, int size, int max, u8 *xy_data,
> + int bofs)
> +{
> + int nbyte;
> + int next;
> +
> + for (nbyte = 0, *axis = 0, next = 0; nbyte < size; nbyte++)
> + *axis = *axis + ((xy_data[nbyte] >> bofs) << (nbyte * 8));
> +
> + *axis &= max - 1;
> +}
> +
> +static void cyttsp5_get_touch_record(struct cyttsp5 *ts,
> + struct cyttsp5_touch *touch, u8 *xy_data)
> +{
> + struct cyttsp5_sysinfo *si = &ts->sysinfo;
> + enum cyttsp5_tch_abs abs;
> +
> + for (abs = CY_TCH_X; abs < CY_TCH_NUM_ABS; abs++) {
> + cyttsp5_get_touch_axis(&touch->abs[abs],
> + si->tch_abs[abs].size,
> + si->tch_abs[abs].max,
> + xy_data + si->tch_abs[abs].ofs,
> + si->tch_abs[abs].bofs);
> + }
> +}
> +
> +static void cyttsp5_get_mt_touches(struct cyttsp5 *ts,
> + struct cyttsp5_touch *tch, int num_cur_tch)
> +{
> + struct cyttsp5_sysinfo *si = &ts->sysinfo;
> + int i, t = 0;
> + DECLARE_BITMAP(ids, si->tch_abs[CY_TCH_T].max);
> + u8 *tch_addr;
> + int tmp;
> +
> + bitmap_zero(ids, si->tch_abs[CY_TCH_T].max);
> + memset(tch->abs, 0, sizeof(tch->abs));
> +
> + for (i = 0; i < num_cur_tch; i++) {
> + tch_addr = si->xy_data + (i * TOUCH_REPORT_SIZE);
> + cyttsp5_get_touch_record(ts, tch, tch_addr);
> +
> + /* Convert MAJOR/MINOR from mm to resolution */
> + tmp = tch->abs[CY_TCH_MAJ] * 100 * si->sensing_conf_data.res_x;
> + tch->abs[CY_TCH_MAJ] = tmp / si->sensing_conf_data.len_x;
> + tmp = tch->abs[CY_TCH_MIN] * 100 * si->sensing_conf_data.res_x;
> + tch->abs[CY_TCH_MIN] = tmp / si->sensing_conf_data.len_x;
> +
> + t = tch->abs[CY_TCH_T];
> + input_mt_slot(ts->input, t);
> + input_mt_report_slot_state(ts->input, MT_TOOL_FINGER, true);
> + __set_bit(t, ids);
> +
> + /* position and pressure fields */
> + input_report_abs(ts->input, ABS_MT_POSITION_X,
> + tch->abs[CY_TCH_X]);
> + input_report_abs(ts->input, ABS_MT_POSITION_Y,
> + tch->abs[CY_TCH_Y]);
> + input_report_abs(ts->input, ABS_MT_PRESSURE,
> + tch->abs[CY_TCH_P]);
> +
> + /* Get the extended touch fields */
> + input_report_abs(ts->input, ABS_MT_TOUCH_MAJOR,
> + tch->abs[CY_TCH_MAJ]);
> + input_report_abs(ts->input, ABS_MT_TOUCH_MINOR,
> + tch->abs[CY_TCH_MIN]);
> + }
> +
> + cyttsp5_final_sync(ts->input, si->tch_abs[CY_TCH_T].max, ids);
> +
> + ts->num_prv_rec = num_cur_tch;
> +}
> +
> +/* read xy_data for all current touches */
> +static int cyttsp5_xy_worker(struct cyttsp5 *ts)
> +{
> + struct device *dev = ts->dev;
> + struct cyttsp5_sysinfo *si = &ts->sysinfo;
> + int max_tch = si->sensing_conf_data.max_tch;
> + struct cyttsp5_touch tch;
> + u8 num_cur_tch;
> +
> + cyttsp5_get_touch_axis(&tch.hdr, si->tch_hdr.size,
> + si->tch_hdr.max,
> + si->xy_mode + 3 + si->tch_hdr.ofs,
> + si->tch_hdr.bofs);
> +
> + num_cur_tch = tch.hdr;
> + if (num_cur_tch > max_tch) {
> + dev_err(dev, "Num touch err detected (n=%d)\n", num_cur_tch);
> + num_cur_tch = max_tch;
> + }
> +
> + if (num_cur_tch == 0 && ts->num_prv_rec == 0)
> + return 0;
> +
> + /* extract xy_data for all currently reported touches */
> + if (num_cur_tch)
> + cyttsp5_get_mt_touches(ts, &tch, num_cur_tch);
> + else
> + cyttsp5_mt_lift_all(ts);
> +
> + return 0;
> +}
> +
> +static int cyttsp5_mt_attention(struct device *dev)
> +{
> + struct cyttsp5 *ts = dev_get_drvdata(dev);
Why don't you pass ts into cyttsp5_mt_attention?
> + struct cyttsp5_sysinfo *si = &ts->sysinfo;
> + int rc;
> +
> + if (si->xy_mode[2] != HID_TOUCH_REPORT_ID)
> + return 0;
> +
> + /* core handles handshake */
> + rc = cyttsp5_xy_worker(ts);
> + if (rc < 0)
> + dev_err(dev, "xy_worker error r=%d\n", rc);
> +
> + return rc;
> +}
> +
> +static int cyttsp5_setup_input_device(struct device *dev)
> +{
> + struct cyttsp5 *ts = dev_get_drvdata(dev);
> + struct cyttsp5_sysinfo *si = &ts->sysinfo;
> + int max_x, max_y, max_p;
> + int max_x_tmp, max_y_tmp;
> + int rc;
> +
> + __set_bit(EV_ABS, ts->input->evbit);
> + __set_bit(EV_REL, ts->input->evbit);
> + __set_bit(EV_KEY, ts->input->evbit);
> +
> + max_x_tmp = si->sensing_conf_data.res_x;
> + max_y_tmp = si->sensing_conf_data.res_y;
> + max_x = max_y_tmp - 1;
> + max_y = max_x_tmp - 1;
> + max_p = si->sensing_conf_data.max_z;
> +
> + input_mt_init_slots(ts->input, si->tch_abs[CY_TCH_T].max, 0);
Error handling.
> +
> + __set_bit(ABS_MT_POSITION_X, ts->input->absbit);
> + __set_bit(ABS_MT_POSITION_Y, ts->input->absbit);
> + __set_bit(ABS_MT_PRESSURE, ts->input->absbit);
> +
> + input_set_abs_params(ts->input, ABS_MT_POSITION_X, 0, max_x, 0, 0);
> + input_set_abs_params(ts->input, ABS_MT_POSITION_Y, 0, max_y, 0, 0);
> + input_set_abs_params(ts->input, ABS_MT_PRESSURE, 0, max_p, 0, 0);
> +
> + input_set_abs_params(ts->input, ABS_MT_TOUCH_MAJOR, 0, MAX_AREA, 0, 0);
> + input_set_abs_params(ts->input, ABS_MT_TOUCH_MINOR, 0, MAX_AREA, 0, 0);
You might want to use the standard DT touchscreen properties and
reporting functions to support axis inversion and swapping of axes.
> +
> + rc = input_register_device(ts->input);
> + if (rc < 0)
> + dev_err(dev, "Error, failed register input device r=%d\n", rc);
> +
> + return rc;
> +}
> +
> +#ifdef CONFIG_OF
> +static int cyttsp5_parse_dt_key_code(struct device *dev)
> +{
> + struct cyttsp5 *ts = dev_get_drvdata(dev);
> + struct cyttsp5_sysinfo *si = &ts->sysinfo;
> + struct device_node *np;
> + int i;
> +
> + np = dev->of_node;
> + if (!np)
> + return -EINVAL;
> +
> + if (!si->num_btns)
> + return 0;
> +
> + /* Initialize the button to RESERVED */
> + for (i = 0; i < si->num_btns; i++)
> + si->key_code[i] = KEY_RESERVED;
> +
> + return of_property_read_u32_array(np, "linux,keycodes",
> + si->key_code, si->num_btns);
> +}
> +#else
> +static int cyttsp5_parse_dt_key_code(struct device *dev)
> +{
> + struct cyttsp5 *ts = dev_get_drvdata(dev);
> + struct cyttsp5_sysinfo *si = &ts->sysinfo;
> + int i;
> +
> + if (!si->num_btns)
> + return 0;
> +
> + /* Initialize the button to RESERVED */
> + for (i = 0; i < si->num_btns; i++)
> + si->key_code[i] = KEY_RESERVED;
> +
> + return 0;
> +}
> +#endif
> +
> +static int cyttsp5_btn_attention(struct device *dev)
> +{
> + struct cyttsp5 *ts = dev_get_drvdata(dev);
> + struct cyttsp5_sysinfo *si = &ts->sysinfo;
> + int cur_btn;
> + int cur_btn_state;
> +
> + if (si->xy_mode[2] != HID_BTN_REPORT_ID || !si->num_btns)
> + return 0;
> +
> + /* extract button press/release touch information */
> + for (cur_btn = 0; cur_btn < si->num_btns; cur_btn++) {
> + /* Get current button state */
> + cur_btn_state = (si->xy_data[0] >> (cur_btn * CY_BITS_PER_BTN))
> + & CY_NUM_BTN_EVENT_ID;
> +
> + input_report_key(ts->input, si->key_code[cur_btn],
> + cur_btn_state);
> + input_sync(ts->input);
You do not need to sen sync after each button, send it once after all
buttons sent.
> + }
> +
> + return 0;
> +}
> +
> +static u16 cyttsp5_compute_crc(u8 *buf, u32 size)
> +{
> + u16 remainder = 0xFFFF;
> + u16 xor_mask = 0x0000;
> + u32 index;
> + u32 byte_value;
> + u32 table_index;
> + u32 crc_bit_width = sizeof(u16) * 8;
> +
> + /* Divide the message by polynomial, via the table. */
> + for (index = 0; index < size; index++) {
> + byte_value = buf[index];
> + table_index = ((byte_value >> 4) & 0x0F)
> + ^ (remainder >> (crc_bit_width - 4));
> + remainder = crc_itu_t_table[table_index]
> + ^ (remainder << 4);
> + table_index = (byte_value & 0x0F)
> + ^ (remainder >> (crc_bit_width - 4));
> + remainder = crc_itu_t_table[table_index]
> + ^ (remainder << 4);
> + }
> +
> + /* Perform the final remainder CRC. */
> + return remainder ^ xor_mask;
> +}
> +
> +static int cyttsp5_validate_cmd_response(struct cyttsp5 *ts, u8 code)
> +{
> + u16 size, crc;
> + u8 status, offset;
> + int command_code;
> +
> + size = get_unaligned_le16(&ts->response_buf[0]);
> +
> + if (!size)
> + return 0;
> +
> + offset = ts->response_buf[HID_OUTPUT_RESPONSE_REPORT_OFFSET];
> +
> + if (offset == HID_BL_RESPONSE_REPORT_ID) {
> + if (ts->response_buf[4] != HID_OUTPUT_BL_SOP) {
> + dev_err(ts->dev, "HID output response, wrong SOP\n");
> + return -EPROTO;
> + }
> +
> + if (ts->response_buf[size - 1] != HID_OUTPUT_BL_EOP) {
> + dev_err(ts->dev, "HID output response, wrong EOP\n");
> + return -EPROTO;
> + }
> +
> + crc = cyttsp5_compute_crc(&ts->response_buf[4], size - 7);
> + if (ts->response_buf[size - 3] != LOW_BYTE(crc) ||
> + ts->response_buf[size - 2] != HI_BYTE(crc)) {
> + dev_err(ts->dev, "HID output response, wrong CRC 0x%X\n",
> + crc);
> + return -EPROTO;
> + }
> +
> + status = ts->response_buf[5];
> + if (status) {
> + dev_err(ts->dev, "HID output response, ERROR:%d\n",
> + status);
> + return -EPROTO;
> + }
> + }
> +
> + if (offset == HID_APP_RESPONSE_REPORT_ID) {
> + command_code = ts->response_buf[HID_OUTPUT_RESPONSE_CMD_OFFSET]
> + & HID_OUTPUT_RESPONSE_CMD_MASK;
> + if (command_code != code) {
> + dev_err(ts->dev,
> + "HID output response, wrong command_code:%X\n",
> + command_code);
> + return -EPROTO;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static void cyttsp5_si_get_btn_data(struct cyttsp5 *ts)
> +{
> + struct cyttsp5_sysinfo *si = &ts->sysinfo;
> + int i;
> + unsigned int btns = ts->response_buf[HID_SYSINFO_BTN_OFFSET]
> + & HID_SYSINFO_BTN_MASK;
> +
> + si->num_btns = 0;
> + for (i = 0; i < HID_SYSINFO_MAX_BTN; i++) {
> + if (btns & BIT(i))
> + si->num_btns++;
> + }
si->num_btns = hweight8(ts->response_buf[HID_SYSINFO_BTN_OFFSET]);
> +}
> +
> +static int cyttsp5_get_sysinfo_regs(struct cyttsp5 *ts)
> +{
> + struct cyttsp5_sensing_conf_data *scd = &ts->sysinfo.sensing_conf_data;
> + struct cyttsp5_sensing_conf_data_dev *scd_dev =
> + (struct cyttsp5_sensing_conf_data_dev *)
> + &ts->response_buf[HID_SYSINFO_SENSING_OFFSET];
> + struct cyttsp5_sysinfo *si = &ts->sysinfo;
> +
> + cyttsp5_si_get_btn_data(ts);
> +
> + scd->max_tch = scd_dev->max_num_of_tch_per_refresh_cycle;
> + scd->res_x = get_unaligned_le16(&scd_dev->res_x);
> + scd->res_y = get_unaligned_le16(&scd_dev->res_y);
> + scd->max_z = get_unaligned_le16(&scd_dev->max_z);
> + scd->len_x = get_unaligned_le16(&scd_dev->len_x);
> + scd->len_y = get_unaligned_le16(&scd_dev->len_y);
> +
> + si->xy_data = devm_kzalloc(ts->dev, scd->max_tch * TOUCH_REPORT_SIZE,
> + GFP_KERNEL);
> + if (!si->xy_data)
> + return -ENOMEM;
> +
> + si->xy_mode = devm_kzalloc(ts->dev, TOUCH_INPUT_HEADER_SIZE,
> + GFP_KERNEL);
> + if (!si->xy_mode)
> + return -ENOMEM;
Why do we need these 2 allocated separately form the driver data?
> +
> + return 0;
> +}
> +
> +static int cyttsp5_hid_output_get_sysinfo(struct cyttsp5 *ts)
> +{
> + int rc;
> + u8 cmd[HID_OUTPUT_GET_SYSINFO_SIZE];
> +
> + ts->hid_cmd_state = HID_CMD_BUSY;
> +
> + /* HI bytes of Output register address */
> + cmd[0] = LOW_BYTE(HID_OUTPUT_GET_SYSINFO_SIZE);
> + cmd[1] = HI_BYTE(HID_OUTPUT_GET_SYSINFO_SIZE);
> + cmd[2] = HID_APP_OUTPUT_REPORT_ID;
> + cmd[3] = 0x0; /* Reserved */
> + cmd[4] = HID_OUTPUT_GET_SYSINFO;
> +
> + rc = cyttsp5_write(ts, HID_OUTPUT_REG, cmd,
> + HID_OUTPUT_GET_SYSINFO_SIZE);
> + if (rc) {
> + dev_err(ts->dev, "Failed to write command %d", rc);
> + goto error;
> + }
> +
> + rc = wait_event_timeout(ts->wait_q, (ts->hid_cmd_state == HID_CMD_DONE),
> + msecs_to_jiffies(CY_HID_OUTPUT_GET_SYSINFO_TIMEOUT));
> + if (!rc) {
> + dev_err(ts->dev, "HID output cmd execution timed out\n");
> + rc = -ETIME;
> + goto error;
> + }
> +
> + rc = cyttsp5_validate_cmd_response(ts, HID_OUTPUT_GET_SYSINFO);
> + if (rc) {
> + dev_err(ts->dev, "Validation of the response failed\n");
> + goto error;
> + }
> +
> + return cyttsp5_get_sysinfo_regs(ts);
> +
> +error:
> + mutex_lock(&ts->system_lock);
> + ts->hid_cmd_state = HID_CMD_DONE;
> + mutex_unlock(&ts->system_lock);
> + return rc;
> +}
> +
> +static int cyttsp5_hid_output_bl_launch_app(struct cyttsp5 *ts)
> +{
> + int rc;
> + u8 cmd[HID_OUTPUT_BL_LAUNCH_APP];
> + u16 crc;
> +
> + mutex_lock(&ts->system_lock);
> + ts->hid_cmd_state = HID_CMD_BUSY;
> + mutex_unlock(&ts->system_lock);
> +
> + cmd[0] = LOW_BYTE(HID_OUTPUT_BL_LAUNCH_APP_SIZE);
> + cmd[1] = HI_BYTE(HID_OUTPUT_BL_LAUNCH_APP_SIZE);
> + cmd[2] = HID_BL_OUTPUT_REPORT_ID;
> + cmd[3] = 0x0; /* Reserved */
> + cmd[4] = HID_OUTPUT_BL_SOP;
> + cmd[5] = HID_OUTPUT_BL_LAUNCH_APP;
> + cmd[6] = 0x0; /* Low bytes of data */
> + cmd[7] = 0x0; /* Hi bytes of data */
> + crc = cyttsp5_compute_crc(&cmd[4], 4);
> + cmd[8] = LOW_BYTE(crc);
> + cmd[9] = HI_BYTE(crc);
> + cmd[10] = HID_OUTPUT_BL_EOP;
> +
> + rc = cyttsp5_write(ts, HID_OUTPUT_REG, cmd,
> + HID_OUTPUT_BL_LAUNCH_APP_SIZE);
> + if (rc) {
> + dev_err(ts->dev, "Failed to write command %d", rc);
> + goto error;
> + }
> +
> + rc = wait_event_timeout(ts->wait_q, (ts->hid_cmd_state == HID_CMD_DONE),
> + msecs_to_jiffies(CY_HID_OUTPUT_TIMEOUT));
> + if (!rc) {
> + dev_err(ts->dev, "HID output cmd execution timed out\n");
> + rc = -ETIME;
> + goto error;
> + }
> +
> + rc = cyttsp5_validate_cmd_response(ts, HID_OUTPUT_BL_LAUNCH_APP);
> + if (rc) {
> + dev_err(ts->dev, "Validation of the response failed\n");
> + goto error;
> + }
> +
> + return rc;
> +
> +error:
> + mutex_lock(&ts->system_lock);
> + ts->hid_cmd_state = HID_CMD_DONE;
> + mutex_unlock(&ts->system_lock);
> +
> + return rc;
> +}
> +
> +static int cyttsp5_get_hid_descriptor(struct cyttsp5 *ts,
> + struct cyttsp5_hid_desc *desc)
> +{
> + struct device *dev = ts->dev;
> + __le16 hid_desc_register = HID_DESC_REG;
> + int rc;
> + u8 cmd[2];
> +
> + /* Read HID descriptor length and version */
> + mutex_lock(&ts->system_lock);
> + ts->hid_cmd_state = HID_CMD_BUSY;
> + mutex_unlock(&ts->system_lock);
> +
> + /* Set HID descriptor register */
> + memcpy(cmd, &hid_desc_register, sizeof(hid_desc_register));
> +
> + rc = cyttsp5_write(ts, HID_DESC_REG, NULL, 0);
> + if (rc < 0) {
> + dev_err(dev, "Failed to get HID descriptor, rc=%d\n", rc);
> + goto error;
> + }
> +
> + rc = wait_event_timeout(ts->wait_q, (ts->hid_cmd_state == HID_CMD_DONE),
> + msecs_to_jiffies(CY_HID_GET_HID_DESCRIPTOR_TIMEOUT));
> + if (!rc) {
> + dev_err(ts->dev, "HID get descriptor timed out\n");
> + rc = -ETIME;
> + goto error;
> + }
> +
> + memcpy(desc, ts->response_buf, sizeof(struct cyttsp5_hid_desc));
> +
> + /* Check HID descriptor length and version */
> + if (le16_to_cpu(desc->hid_desc_len) != sizeof(*desc) ||
> + le16_to_cpu(desc->bcd_version) != HID_VERSION) {
> + dev_err(dev, "Unsupported HID version\n");
> + return -ENODEV;
> + }
> +
> + goto exit;
> +
> +error:
> + mutex_lock(&ts->system_lock);
> + ts->hid_cmd_state = HID_CMD_DONE;
> + mutex_unlock(&ts->system_lock);
> +exit:
> + return rc;
> +}
> +
> +static int fill_tch_abs(struct cyttsp5_tch_abs_params *tch_abs, int report_size,
> + int offset)
> +{
> + tch_abs->ofs = offset / 8;
> + tch_abs->size = report_size / 8;
> + if (report_size % 8)
> + tch_abs->size += 1;
> + tch_abs->min = 0;
> + tch_abs->max = 1 << report_size;
> + tch_abs->bofs = offset - (tch_abs->ofs << 3);
> +
> + return 0;
> +}
> +
> +static int move_button_data(struct cyttsp5 *ts, struct cyttsp5_sysinfo *si)
> +{
> + memcpy(si->xy_mode, ts->input_buf, BTN_INPUT_HEADER_SIZE);
> + memcpy(si->xy_data, &ts->input_buf[BTN_INPUT_HEADER_SIZE],
> + BTN_REPORT_SIZE);
> +
> + return 0;
> +}
> +
> +static int move_touch_data(struct cyttsp5 *ts, struct cyttsp5_sysinfo *si)
> +{
> + int max_tch = si->sensing_conf_data.max_tch;
> + int num_cur_tch;
> + int length;
> + struct cyttsp5_tch_abs_params *tch = &si->tch_hdr;
> +
> + memcpy(si->xy_mode, ts->input_buf, TOUCH_INPUT_HEADER_SIZE);
> +
> + cyttsp5_get_touch_axis(&num_cur_tch, tch->size,
> + tch->max, si->xy_mode + 3 + tch->ofs, tch->bofs);
> + if (unlikely(num_cur_tch > max_tch))
> + num_cur_tch = max_tch;
> +
> + length = num_cur_tch * TOUCH_REPORT_SIZE;
> +
> + memcpy(si->xy_data, &ts->input_buf[TOUCH_INPUT_HEADER_SIZE], length);
> +
> + return 0;
> +}
> +
> +static irqreturn_t cyttsp5_handle_irq(int irq, void *handle)
> +{
> + struct cyttsp5 *ts = handle;
> + int report_id;
> + int size;
> + int rc;
> +
> + rc = cyttsp5_read(ts, ts->input_buf, CY_MAX_INPUT);
> + if (rc)
> + return IRQ_HANDLED;
> +
> + size = get_unaligned_le16(&ts->input_buf[0]);
> +
> + /* check reset */
> + if (size == 0) {
> + memcpy(ts->response_buf, ts->input_buf, 2);
> +
> + mutex_lock(&ts->system_lock);
> + ts->hid_cmd_state = HID_CMD_DONE;
> + mutex_unlock(&ts->system_lock);
> + wake_up(&ts->wait_q);
I'd use spinlock in wait queue and wake_up_locked() instead of a
separate mutex.
> + return IRQ_HANDLED;
> + }
> +
> + report_id = ts->input_buf[2];
> +
switch(report_id) {
}
> + if (report_id == HID_TOUCH_REPORT_ID) {
> + move_touch_data(ts, &ts->sysinfo);
> + cyttsp5_mt_attention(ts->dev);
> + } else if (report_id == HID_BTN_REPORT_ID) {
> + move_button_data(ts, &ts->sysinfo);
> + cyttsp5_btn_attention(ts->dev);
> + } else {
> + /* It is not an input but a command response */
> + memcpy(ts->response_buf, ts->input_buf, size);
> +
> + mutex_lock(&ts->system_lock);
> + ts->hid_cmd_state = HID_CMD_DONE;
> + mutex_unlock(&ts->system_lock);
> + wake_up(&ts->wait_q);
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int cyttsp5_deassert_int(struct cyttsp5 *ts)
> +{
> + u16 size;
> + u8 buf[2];
> + int rc;
> +
> + rc = regmap_bulk_read(ts->regmap, HID_INPUT_REG, buf, 2);
> + if (rc < 0)
> + return rc;
> +
> + size = get_unaligned_le16(&buf[0]);
> + if (size == 2 || size == 0)
> + return 0;
> +
> + return -EINVAL;
> +}
> +
> +static int cyttsp5_fill_all_touch(struct cyttsp5 *ts)
> +{
> + struct cyttsp5_sysinfo *si = &ts->sysinfo;
> +
> + fill_tch_abs(&si->tch_abs[CY_TCH_X], REPORT_SIZE_16,
> + TOUCH_REPORT_DESC_X);
> + fill_tch_abs(&si->tch_abs[CY_TCH_Y], REPORT_SIZE_16,
> + TOUCH_REPORT_DESC_Y);
> + fill_tch_abs(&si->tch_abs[CY_TCH_P], REPORT_SIZE_8,
> + TOUCH_REPORT_DESC_P);
> + fill_tch_abs(&si->tch_abs[CY_TCH_T], REPORT_SIZE_5,
> + TOUCH_REPORT_DESC_CONTACTID);
> + fill_tch_abs(&si->tch_hdr, REPORT_SIZE_5,
> + TOUCH_REPORT_DESC_HDR_CONTACTCOUNT);
> + fill_tch_abs(&si->tch_abs[CY_TCH_MAJ], REPORT_SIZE_8,
> + TOUCH_REPORT_DESC_MAJ);
> + fill_tch_abs(&si->tch_abs[CY_TCH_MIN], REPORT_SIZE_8,
> + TOUCH_REPORT_DESC_MIN);
> +
> + return 0;
> +}
> +
> +static int cyttsp5_startup(struct cyttsp5 *ts)
> +{
> + int rc;
> +
> + rc = cyttsp5_deassert_int(ts);
> + if (rc) {
> + dev_err(ts->dev, "Error on deassert int r=%d\n", rc);
> + return -ENODEV;
> + }
> +
> + /*
> + * Launch the application as the device starts in bootloader mode
> + * because of a power-on-reset
> + */
> + rc = cyttsp5_hid_output_bl_launch_app(ts);
> + if (rc < 0) {
> + dev_err(ts->dev, "Error on launch app r=%d\n", rc);
> + return rc;
> + }
> +
> + rc = cyttsp5_get_hid_descriptor(ts, &ts->hid_desc);
> + if (rc < 0) {
> + dev_err(ts->dev, "Error on getting HID descriptor r=%d\n", rc);
> + return rc;
> + }
> +
> + rc = cyttsp5_fill_all_touch(ts);
> + if (rc < 0) {
> + dev_err(ts->dev, "Error on report descriptor r=%d\n", rc);
> + return rc;
> + }
> +
> + rc = cyttsp5_hid_output_get_sysinfo(ts);
> + if (rc) {
> + dev_err(ts->dev, "Error on getting sysinfo r=%d\n", rc);
> + return rc;
> + }
> +
> + return rc;
> +}
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id cyttsp5_of_match[] = {
> + { .compatible = "cypress,cyttsp5", },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, cyttsp5_of_match);
> +#endif
> +
> +static const struct i2c_device_id cyttsp5_i2c_id[] = {
> + { CYTTSP5_NAME, 0, },
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, cyttsp5_i2c_id);
> +
> +static int cyttsp5_probe(struct device *dev, struct regmap *regmap, int irq,
> + const char *name)
> +{
> + struct cyttsp5 *ts;
> + struct cyttsp5_sysinfo *si;
> + int rc = 0, i;
> +
> + ts = devm_kzalloc(dev, sizeof(*ts), GFP_KERNEL);
> + if (!ts)
> + return -ENOMEM;
> +
> + /* Initialize device info */
> + ts->regmap = regmap;
> + ts->dev = dev;
> + si = &ts->sysinfo;
> + dev_set_drvdata(dev, ts);
> +
> + /* Initialize mutexes and spinlocks */
> + mutex_init(&ts->system_lock);
> +
> + /* Initialize wait queue */
> + init_waitqueue_head(&ts->wait_q);
> +
> + /* Reset the gpio to be in a reset state */
> + ts->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
> + if (IS_ERR(ts->reset_gpio)) {
> + rc = PTR_ERR(ts->reset_gpio);
> + dev_err(dev, "Failed to request reset gpio, error %d\n", rc);
> + return rc;
> + }
> + gpiod_set_value(ts->reset_gpio, 1);
Why not request it as GPIOD_OUT_HIGH if you going to activate it
immediately? However it sounds as if you have a reset line that is
active low and you want to driver it low to reset, and then release. In
this case you need to do
ts->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
<delay> - chip usually needs to be some time in reset state
gpiod_set_value(ts->reset_gpio, 0);
and make sure that correct polarity is specified in device tree.
> +
> + /* Need a delay to have device up */
> + msleep(20);
> +
> + rc = devm_request_threaded_irq(dev, irq, NULL, cyttsp5_handle_irq,
> + IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
Just use IRQF_ONESHOT and make sure DTS specifies interrupt properly.
BTW,I'd recommend using level, not edge interrupts. It is so easy to
miss an edge.
> + name, ts);
> + if (rc) {
> + dev_err(dev, "unable to request IRQ\n");
> + return rc;
> + }
> +
> + rc = cyttsp5_startup(ts);
> + if (rc) {
> + dev_err(ts->dev, "Fail initial startup r=%d\n", rc);
> + return rc;
> + }
> +
> + rc = cyttsp5_parse_dt_key_code(dev);
> + if (rc < 0) {
> + dev_err(ts->dev, "Error while parsing dts %d\n", rc);
> + return rc;
> + }
> +
> + ts->input = devm_input_allocate_device(dev);
> + if (!ts->input) {
> + dev_err(dev, "Error, failed to allocate input device\n");
> + return -ENODEV;
> + }
> +
> + ts->input->name = "cyttsp5";
> + scnprintf(ts->phys, sizeof(ts->phys), "%s/input0", dev_name(dev));
> + ts->input->phys = ts->phys;
> + ts->input->dev.parent = ts->dev;
> + input_set_drvdata(ts->input, ts);
> +
> + touchscreen_parse_properties(ts->input, true, NULL);
> +
> + __set_bit(EV_KEY, ts->input->evbit);
> + for (i = 0; i < si->num_btns; i++)
> + __set_bit(si->key_code[i], ts->input->keybit);
> +
> + return cyttsp5_setup_input_device(dev);
> +}
> +
> +static int cyttsp5_i2c_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct regmap *regmap;
> + static const struct regmap_config config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + };
> +
> + regmap = devm_regmap_init_i2c(client, &config);
> + if (IS_ERR(regmap)) {
> + dev_err(&client->dev, "regmap allocation failed: %ld\n",
> + PTR_ERR(regmap));
> + return PTR_ERR(regmap);
> + }
> +
> + return cyttsp5_probe(&client->dev, regmap, client->irq, client->name);
> +}
> +
> +static int cyttsp5_remove(struct device *dev)
> +{
> + struct cyttsp5 *ts = dev_get_drvdata(dev);
> +
> + input_unregister_device(ts->input);
Not needed: you are using managed input device.
> +
> + return 0;
> +}
> +
> +static int cyttsp5_i2c_remove(struct i2c_client *client)
> +{
> + struct device *dev = &client->dev;
> +
> + return cyttsp5_remove(dev);
Not needed if you remove cyttsp5_remove().
> +}
> +
> +static struct i2c_driver cyttsp5_i2c_driver = {
> + .driver = {
> + .name = CYTTSP5_NAME,
> + .owner = THIS_MODULE,
> + .of_match_table = of_match_ptr(cyttsp5_of_match),
> + },
> + .probe = cyttsp5_i2c_probe,
> + .remove = cyttsp5_i2c_remove,
> + .id_table = cyttsp5_i2c_id,
> +};
> +
> +module_i2c_driver(cyttsp5_i2c_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Touchscreen driver for Cypress TrueTouch Gen 5 Product");
> +MODULE_AUTHOR("Mylène Josserand <mylene.josserand-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>");
> --
> 2.11.0
>
Thanks.
--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 1/2] Input: edt-ft5x06 - Add support for regulator
From: Lothar Waßmann @ 2018-01-23 8:04 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Mylène Josserand, Mark Rutland,
devicetree-u79uwXL29TY76Z2rM5mHXA, Russell King - ARM Linux, lkml,
Quentin Schulz, Chen-Yu Tsai, Rob Herring,
linux-input-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Maxime Ripard, Thomas Petazzoni,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <CAKdAkRQme1Y+t4qxLGOFojEji1L6z3L0XS4+W0FdQ36-2hxW=A@mail.gmail.com>
Hi,
On Mon, 22 Jan 2018 09:42:08 -0800 Dmitry Torokhov wrote:
> Hi Mylène,
>
> On Thu, Dec 28, 2017 at 8:33 AM, Mylène Josserand
> <mylene.josserand-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> > Add the support of regulator to use it as VCC source.
> >
> > Signed-off-by: Mylène Josserand <mylene.josserand-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> > ---
> > .../bindings/input/touchscreen/edt-ft5x06.txt | 1 +
> > drivers/input/touchscreen/edt-ft5x06.c | 33 ++++++++++++++++++++++
> > 2 files changed, 34 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
> > index 025cf8c9324a..48e975b9c1aa 100644
> > --- a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
> > +++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
> > @@ -30,6 +30,7 @@ Required properties:
> > Optional properties:
> > - reset-gpios: GPIO specification for the RESET input
> > - wake-gpios: GPIO specification for the WAKE input
> > + - vcc-supply: Regulator that supplies the touchscreen
> >
> > - pinctrl-names: should be "default"
> > - pinctrl-0: a phandle pointing to the pin settings for the
> > diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> > index c53a3d7239e7..5ee14a25a382 100644
> > --- a/drivers/input/touchscreen/edt-ft5x06.c
> > +++ b/drivers/input/touchscreen/edt-ft5x06.c
> > @@ -39,6 +39,7 @@
> > #include <linux/input/mt.h>
> > #include <linux/input/touchscreen.h>
> > #include <linux/of_device.h>
> > +#include <linux/regulator/consumer.h>
> >
> > #define WORK_REGISTER_THRESHOLD 0x00
> > #define WORK_REGISTER_REPORT_RATE 0x08
> > @@ -91,6 +92,7 @@ struct edt_ft5x06_ts_data {
> > struct touchscreen_properties prop;
> > u16 num_x;
> > u16 num_y;
> > + struct regulator *vcc;
> >
> > struct gpio_desc *reset_gpio;
> > struct gpio_desc *wake_gpio;
> > @@ -993,6 +995,23 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
> >
> > tsdata->max_support_points = chip_data->max_support_points;
> >
> > + tsdata->vcc = devm_regulator_get(&client->dev, "vcc");
> > + if (IS_ERR(tsdata->vcc)) {
> > + error = PTR_ERR(tsdata->vcc);
> > + dev_err(&client->dev, "failed to request regulator: %d\n",
> > + error);
>
I would check for -EPROBE_DEFER here and omit the error message in this
case.
Lothar Waßmann
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 1/2] Input: edt-ft5x06 - Add support for regulator
From: Mylene Josserand @ 2018-01-23 9:10 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Rob Herring, Mark Rutland, Russell King - ARM Linux,
Maxime Ripard, Chen-Yu Tsai, linux-arm-kernel,
linux-input@vger.kernel.org, devicetree, lkml, Thomas Petazzoni,
Quentin Schulz
In-Reply-To: <CAKdAkRQme1Y+t4qxLGOFojEji1L6z3L0XS4+W0FdQ36-2hxW=A@mail.gmail.com>
Hello Dimitry,
Thank you for the review!
Le Mon, 22 Jan 2018 09:42:08 -0800,
Dmitry Torokhov <dmitry.torokhov@gmail.com> a écrit :
> Hi Mylène,
>
> On Thu, Dec 28, 2017 at 8:33 AM, Mylène Josserand
> <mylene.josserand@free-electrons.com> wrote:
> > Add the support of regulator to use it as VCC source.
> >
> > Signed-off-by: Mylène Josserand <mylene.josserand@free-electrons.com>
> > ---
> > .../bindings/input/touchscreen/edt-ft5x06.txt | 1 +
> > drivers/input/touchscreen/edt-ft5x06.c | 33 ++++++++++++++++++++++
> > 2 files changed, 34 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
> > index 025cf8c9324a..48e975b9c1aa 100644
> > --- a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
> > +++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
> > @@ -30,6 +30,7 @@ Required properties:
> > Optional properties:
> > - reset-gpios: GPIO specification for the RESET input
> > - wake-gpios: GPIO specification for the WAKE input
> > + - vcc-supply: Regulator that supplies the touchscreen
> >
> > - pinctrl-names: should be "default"
> > - pinctrl-0: a phandle pointing to the pin settings for the
> > diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> > index c53a3d7239e7..5ee14a25a382 100644
> > --- a/drivers/input/touchscreen/edt-ft5x06.c
> > +++ b/drivers/input/touchscreen/edt-ft5x06.c
> > @@ -39,6 +39,7 @@
> > #include <linux/input/mt.h>
> > #include <linux/input/touchscreen.h>
> > #include <linux/of_device.h>
> > +#include <linux/regulator/consumer.h>
> >
> > #define WORK_REGISTER_THRESHOLD 0x00
> > #define WORK_REGISTER_REPORT_RATE 0x08
> > @@ -91,6 +92,7 @@ struct edt_ft5x06_ts_data {
> > struct touchscreen_properties prop;
> > u16 num_x;
> > u16 num_y;
> > + struct regulator *vcc;
> >
> > struct gpio_desc *reset_gpio;
> > struct gpio_desc *wake_gpio;
> > @@ -993,6 +995,23 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
> >
> > tsdata->max_support_points = chip_data->max_support_points;
> >
> > + tsdata->vcc = devm_regulator_get(&client->dev, "vcc");
> > + if (IS_ERR(tsdata->vcc)) {
> > + error = PTR_ERR(tsdata->vcc);
> > + dev_err(&client->dev, "failed to request regulator: %d\n",
> > + error);
> > + return error;
> > + };
>
> As 0-day pounted out, this semicolon is not needed.
Yes, thanks, I will fix that in next version.
>
> > +
> > + if (tsdata->vcc) {
>
> You do not need to check for non-NULL here, devm_regulator_get() wil
> lnever give you a NULL. If regulator is not defined in DT/board
> mappings, then dummy regulator will be provided. You can call
> regulator_enable() and regulator_disable() and other regulator APIs
> with dummy regulator.
Okay, thanks for the explanation, I will remove that.
>
> > + error = regulator_enable(tsdata->vcc);
> > + if (error < 0) {
> > + dev_err(&client->dev, "failed to enable vcc: %d\n",
> > + error);
> > + return error;
> > + }
> > + }
> > +
> > tsdata->reset_gpio = devm_gpiod_get_optional(&client->dev,
> > "reset", GPIOD_OUT_HIGH);
> > if (IS_ERR(tsdata->reset_gpio)) {
> > @@ -1122,20 +1141,34 @@ static int edt_ft5x06_ts_remove(struct i2c_client *client)
> > static int __maybe_unused edt_ft5x06_ts_suspend(struct device *dev)
> > {
> > struct i2c_client *client = to_i2c_client(dev);
> > + struct edt_ft5x06_ts_data *tsdata = i2c_get_clientdata(client);
> >
> > if (device_may_wakeup(dev))
> > enable_irq_wake(client->irq);
> >
> > + if (tsdata->vcc)
>
> Same here.
yep
>
> > + regulator_disable(tsdata->vcc);
> > +
> > return 0;
> > }
> >
> > static int __maybe_unused edt_ft5x06_ts_resume(struct device *dev)
> > {
> > struct i2c_client *client = to_i2c_client(dev);
> > + struct edt_ft5x06_ts_data *tsdata = i2c_get_clientdata(client);
> > + int ret;
> >
> > if (device_may_wakeup(dev))
> > disable_irq_wake(client->irq);
> >
> > + if (tsdata->vcc) {
>
> And here.
yep
>
> > + ret = regulator_enable(tsdata->vcc);
> > + if (ret < 0) {
> > + dev_err(dev, "failed to enable vcc: %d\n", ret);
> > + return ret;
> > + }
>
> Since power to the device may have been cut, I think you need to
> restore the register settings to whatever it was (factory vs work
> mode, threshold, gain and offset registers, etc, etc).
Okay. Could you tell me how can I do that?
>
> > + }
> > +
> > return 0;
> > }
> >
> > --
> > 2.11.0
> >
>
> Thanks.
>
About your V2's review, you suggested to add support for wake/reset in
suspend/resume (that I forgot in this version). I wanted to add it but
with my board, I can't test suspend/resume. What should I do about
that?
If I send a V3 in next few days, do you think you will have time to
merge it for v4.16?
Thank you in advance,
Best regards,
Mylène
--
Mylène Josserand, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox