* [PATCH 8/8] Input: mms114 - fix typo in definition
From: Andi Shyti @ 2018-01-29 11:33 UTC (permalink / raw)
To: Dmitry Torokhov, Simon Shields
Cc: linux-input, linux-kernel, Andi Shyti, Andi Shyti
In-Reply-To: <20180129113323.18961-1-andi.shyti@samsung.com>
It's 'MMS114_INFORMATION', not 'MMS114_INFOMATION'
Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
---
drivers/input/touchscreen/mms114.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
index 3230c92de1ed..5b609531b3aa 100644
--- a/drivers/input/touchscreen/mms114.c
+++ b/drivers/input/touchscreen/mms114.c
@@ -28,7 +28,7 @@
/* Read only registers */
#define MMS114_PACKET_SIZE 0x0f
-#define MMS114_INFOMATION 0x10
+#define MMS114_INFORMATION 0x10
#define MMS114_TSP_REV 0xf0
#define MMS152_FW_REV 0xe1
@@ -138,7 +138,7 @@ static irqreturn_t mms114_interrupt(int irq, void *dev_id)
touch_size = packet_size / MMS114_PACKET_NUM;
- error = i2c_smbus_read_i2c_block_data(data->client, MMS114_INFOMATION,
+ error = i2c_smbus_read_i2c_block_data(data->client, MMS114_INFORMATION,
packet_size, (u8 *)touch);
if (error < 0)
goto out;
--
2.15.1
^ permalink raw reply related
* [PATCH 0/8] Melfas MMS114 touchscreen cleanups
From: Andi Shyti @ 2018-01-29 11:33 UTC (permalink / raw)
To: Dmitry Torokhov, Simon Shields
Cc: linux-input, linux-kernel, Andi Shyti, Andi Shyti
In-Reply-To: <CGME20180129113330epcas2p353fa87845f4a1e21661032f16b11be82@epcas2p3.samsung.com>
Hi Dmitry,
this patchset contains some cleanups for the mms114 driver. The
first two patches are the most important because they get rid of
the custom i2c read/write functions and use the smbus instead.
The others come from a "cleanup rush" I felt into.
I tested the patchest on the mms114 touchscreen in trats2, I
would appreaciate if Simon can test it on mms152.
Thanks,
Andi
Andi Shyti (8):
Input: mms114 - use smbus functions whenever possible
Input: mms114 - get read of custm i2c read/write functions
Input: mms114 - replace mdelay with msleep
Input: mms114 - remove unused variable
Input: mms114 - use lower case for hexadecimal values
Input: mms114 - Use BIT() macro instead of explicit shifting
Input: mms114 - add SPDX identifier
Input: mms114 - fix typo in definition
drivers/input/touchscreen/mms114.c | 150 +++++++++++--------------------------
1 file changed, 44 insertions(+), 106 deletions(-)
--
2.15.1
^ permalink raw reply
* [PATCH 7/8] Input: mms114 - add SPDX identifier
From: Andi Shyti @ 2018-01-29 11:33 UTC (permalink / raw)
To: Dmitry Torokhov, Simon Shields
Cc: linux-input, linux-kernel, Andi Shyti, Andi Shyti
In-Reply-To: <20180129113323.18961-1-andi.shyti@samsung.com>
Replace the original license statement with the SPDX identifier.
Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
---
drivers/input/touchscreen/mms114.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
index d70c03adf148..3230c92de1ed 100644
--- a/drivers/input/touchscreen/mms114.c
+++ b/drivers/input/touchscreen/mms114.c
@@ -1,11 +1,8 @@
-/*
- * Copyright (C) 2012 Samsung Electronics Co.Ltd
- * Author: Joonyoung Shim <jy0922.shim@samsung.com>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
+// SPDX-License-Identifier: GPL-2.0
+// Samsung S6SY761 Touchscreen device driver
+//
+// Copyright (c) 2012 Samsung Electronics Co., Ltd.
+// Author: Joonyoung Shim <jy0922.shim@samsung.com>
#include <linux/module.h>
#include <linux/delay.h>
--
2.15.1
^ permalink raw reply related
* Re: [PATCH v1] i2c-hid: Use ACPI_COMPANION() directly
From: Andy Shevchenko @ 2018-01-29 15:48 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input
In-Reply-To: <20180119143504.16684-1-andriy.shevchenko@linux.intel.com>
On Fri, 2018-01-19 at 16:35 +0200, Andy Shevchenko wrote:
> Instead of doing additional checks and functional calls,
> just get ACPI companion device directly.
>
Any comments on this? The rationale is to prevent people copy'n'paste
current approach instead of better one which is presented by this
change.
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> drivers/hid/i2c-hid/i2c-hid.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-
> hid.c
> index 7230243b94d3..0d3ca542e987 100644
> --- a/drivers/hid/i2c-hid/i2c-hid.c
> +++ b/drivers/hid/i2c-hid/i2c-hid.c
> @@ -891,10 +891,10 @@ static int i2c_hid_acpi_pdata(struct i2c_client
> *client,
>
> static void i2c_hid_acpi_fix_up_power(struct device *dev)
> {
> - acpi_handle handle = ACPI_HANDLE(dev);
> struct acpi_device *adev;
>
> - if (handle && acpi_bus_get_device(handle, &adev) == 0)
> + adev = ACPI_COMPANION(dev);
> + if (adev)
> acpi_device_fix_up_power(adev);
> }
>
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply
* Re: [PATCH v3 3/3] Input: goodix - use generic touchscreen_properties
From: Marcin Niestroj @ 2018-01-29 16:49 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Bastien Nocera, Antonio Ospite, linux-input
In-Reply-To: <20180126191945.q3sx2pcs7rvpil33@dtor-ws>
Hi Dmitry,
On 26.01.2018 20:19, Dmitry Torokhov wrote:
> Hi Marcin,
>
> On Thu, Jan 25, 2018 at 08:08:29PM +0100, Marcin Niestroj wrote:
>> <...snip...>
>
> You want to do reading of the config and parsing properties and applying
> them to the input device before input device is registered. Actually,
> you want to do that before you call input_mt_init_slots() to make sure
> multitouch and single-touch data is consistend (ABS_MT_POSITION_X/Y vs
> ABS_X/Y).
>
> Can you please try the version below (note that I sqashed all your 3
> patches in one as, as I mentioed, I do not see why they need to be split
> in the first place).
I've tested code below and everything works great on my hardware.
Regards,
Marcin
>
> Thanks.
>
> --
> Dmitry
>
>
> Input: goodix - use generic touchscreen_properties
>
> From: Marcin Niestroj <m.niestroj@grinn-global.com>
>
> Use touchscreen_properties structure instead of implementing all
> properties by our own. It allows us to reuse generic code for parsing
> device-tree properties (which was implemented manually in the driver for
> now). Additionally, it allows us to report events using generic
> touchscreen_report_pos(), which automatically handles inverted and
> swapped axes.
>
> This fixes the issue with the custom code incorrectly handling case where
> ts->inverted_x and ts->swapped_x_y were true, but ts->inverted_y was
> false. Assuming we have 720x1280 touch panel, ts->abs_x_max == 1279 and
> ts->abs_y_max == 719 (because we inverted that in goodix_read_config()).
> Now let's assume that we received event from (0:0) position (in touch
> panel original coordinates). In function goodix_ts_report_touch() we
> calculate input_x as 1279, but after swapping input_y takes that value
> (which is more that maximum 719 value reported during initialization).
>
> Note that since touchscreen coordinates are 0-indexed, we now report
> touchscreen range as (0:size-1).
>
> Developed and tested on custom DT-based device with gt1151 touch
> panel.
>
> Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
> Patchwork-Id: 10184731
> [dtor: fix endianness annotation reported by sparse, handle errors when
> initializing MT slots]
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> drivers/input/touchscreen/goodix.c | 141 +++++++++++++++---------------------
> 1 file changed, 58 insertions(+), 83 deletions(-)
>
> diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
> index ecec8eb17f28b..9736c83dd418f 100644
> --- a/drivers/input/touchscreen/goodix.c
> +++ b/drivers/input/touchscreen/goodix.c
> @@ -22,6 +22,7 @@
> #include <linux/i2c.h>
> #include <linux/input.h>
> #include <linux/input/mt.h>
> +#include <linux/input/touchscreen.h>
> #include <linux/module.h>
> #include <linux/delay.h>
> #include <linux/irq.h>
> @@ -43,11 +44,7 @@ struct goodix_ts_data {
> struct i2c_client *client;
> struct input_dev *input_dev;
> const struct goodix_chip_data *chip;
> - int abs_x_max;
> - int abs_y_max;
> - bool swapped_x_y;
> - bool inverted_x;
> - bool inverted_y;
> + struct touchscreen_properties prop;
> unsigned int max_touch_num;
> unsigned int int_trigger_type;
> struct gpio_desc *gpiod_int;
> @@ -160,7 +157,7 @@ static int goodix_i2c_read(struct i2c_client *client,
> u16 reg, u8 *buf, int len)
> {
> struct i2c_msg msgs[2];
> - u16 wbuf = cpu_to_be16(reg);
> + __be16 wbuf = cpu_to_be16(reg);
> int ret;
>
> msgs[0].flags = 0;
> @@ -295,18 +292,10 @@ static void goodix_ts_report_touch(struct goodix_ts_data *ts, u8 *coor_data)
> int input_y = get_unaligned_le16(&coor_data[3]);
> int input_w = get_unaligned_le16(&coor_data[5]);
>
> - /* Inversions have to happen before axis swapping */
> - if (ts->inverted_x)
> - input_x = ts->abs_x_max - input_x;
> - if (ts->inverted_y)
> - input_y = ts->abs_y_max - input_y;
> - if (ts->swapped_x_y)
> - swap(input_x, input_y);
> -
> input_mt_slot(ts->input_dev, id);
> input_mt_report_slot_state(ts->input_dev, MT_TOOL_FINGER, true);
> - input_report_abs(ts->input_dev, ABS_MT_POSITION_X, input_x);
> - input_report_abs(ts->input_dev, ABS_MT_POSITION_Y, input_y);
> + touchscreen_report_pos(ts->input_dev, &ts->prop,
> + input_x, input_y, true);
> input_report_abs(ts->input_dev, ABS_MT_TOUCH_MAJOR, input_w);
> input_report_abs(ts->input_dev, ABS_MT_WIDTH_MAJOR, input_w);
> }
> @@ -579,44 +568,27 @@ static int goodix_get_gpio_config(struct goodix_ts_data *ts)
> static void goodix_read_config(struct goodix_ts_data *ts)
> {
> u8 config[GOODIX_CONFIG_MAX_LENGTH];
> + int x_max, y_max;
> int error;
>
> error = goodix_i2c_read(ts->client, ts->chip->config_addr,
> config, ts->chip->config_len);
> if (error) {
> - dev_warn(&ts->client->dev,
> - "Error reading config (%d), using defaults\n",
> + dev_warn(&ts->client->dev, "Error reading config: %d\n",
> error);
> - ts->abs_x_max = GOODIX_MAX_WIDTH;
> - ts->abs_y_max = GOODIX_MAX_HEIGHT;
> - if (ts->swapped_x_y)
> - swap(ts->abs_x_max, ts->abs_y_max);
> ts->int_trigger_type = GOODIX_INT_TRIGGER;
> ts->max_touch_num = GOODIX_MAX_CONTACTS;
> return;
> }
>
> - ts->abs_x_max = get_unaligned_le16(&config[RESOLUTION_LOC]);
> - ts->abs_y_max = get_unaligned_le16(&config[RESOLUTION_LOC + 2]);
> - if (ts->swapped_x_y)
> - swap(ts->abs_x_max, ts->abs_y_max);
> ts->int_trigger_type = config[TRIGGER_LOC] & 0x03;
> ts->max_touch_num = config[MAX_CONTACTS_LOC] & 0x0f;
> - if (!ts->abs_x_max || !ts->abs_y_max || !ts->max_touch_num) {
> - dev_err(&ts->client->dev,
> - "Invalid config, using defaults\n");
> - ts->abs_x_max = GOODIX_MAX_WIDTH;
> - ts->abs_y_max = GOODIX_MAX_HEIGHT;
> - if (ts->swapped_x_y)
> - swap(ts->abs_x_max, ts->abs_y_max);
> - ts->max_touch_num = GOODIX_MAX_CONTACTS;
> - }
>
> - if (dmi_check_system(rotated_screen)) {
> - ts->inverted_x = true;
> - ts->inverted_y = true;
> - dev_dbg(&ts->client->dev,
> - "Applying '180 degrees rotated screen' quirk\n");
> + x_max = get_unaligned_le16(&config[RESOLUTION_LOC]);
> + y_max = get_unaligned_le16(&config[RESOLUTION_LOC + 2]);
> + if (x_max && y_max) {
> + input_abs_set_max(ts->input_dev, ABS_MT_POSITION_X, x_max - 1);
> + input_abs_set_max(ts->input_dev, ABS_MT_POSITION_Y, y_max - 1);
> }
> }
>
> @@ -676,32 +648,28 @@ static int goodix_i2c_test(struct i2c_client *client)
> }
>
> /**
> - * goodix_request_input_dev - Allocate, populate and register the input device
> + * goodix_configure_dev - Finish device initialization
> *
> * @ts: our goodix_ts_data pointer
> *
> - * Must be called during probe
> + * Must be called from probe to finish initialization of the device.
> + * Contains the common initialization code for both devices that
> + * declare gpio pins and devices that do not. It is either called
> + * directly from probe or from request_firmware_wait callback.
> */
> -static int goodix_request_input_dev(struct goodix_ts_data *ts)
> +static int goodix_configure_dev(struct goodix_ts_data *ts)
> {
> int error;
>
> + ts->int_trigger_type = GOODIX_INT_TRIGGER;
> + ts->max_touch_num = GOODIX_MAX_CONTACTS;
> +
> ts->input_dev = devm_input_allocate_device(&ts->client->dev);
> if (!ts->input_dev) {
> dev_err(&ts->client->dev, "Failed to allocate input device.");
> return -ENOMEM;
> }
>
> - input_set_abs_params(ts->input_dev, ABS_MT_POSITION_X,
> - 0, ts->abs_x_max, 0, 0);
> - input_set_abs_params(ts->input_dev, ABS_MT_POSITION_Y,
> - 0, ts->abs_y_max, 0, 0);
> - input_set_abs_params(ts->input_dev, ABS_MT_WIDTH_MAJOR, 0, 255, 0, 0);
> - input_set_abs_params(ts->input_dev, ABS_MT_TOUCH_MAJOR, 0, 255, 0, 0);
> -
> - input_mt_init_slots(ts->input_dev, ts->max_touch_num,
> - INPUT_MT_DIRECT | INPUT_MT_DROP_UNUSED);
> -
> ts->input_dev->name = "Goodix Capacitive TouchScreen";
> ts->input_dev->phys = "input/ts";
> ts->input_dev->id.bustype = BUS_I2C;
> @@ -712,42 +680,49 @@ static int goodix_request_input_dev(struct goodix_ts_data *ts)
> /* Capacitive Windows/Home button on some devices */
> input_set_capability(ts->input_dev, EV_KEY, KEY_LEFTMETA);
>
> - error = input_register_device(ts->input_dev);
> - if (error) {
> - dev_err(&ts->client->dev,
> - "Failed to register input device: %d", error);
> - return error;
> - }
> + input_set_capability(ts->input_dev, EV_ABS, ABS_MT_POSITION_X);
> + input_set_capability(ts->input_dev, EV_ABS, ABS_MT_POSITION_Y);
> + input_set_abs_params(ts->input_dev, ABS_MT_WIDTH_MAJOR, 0, 255, 0, 0);
> + input_set_abs_params(ts->input_dev, ABS_MT_TOUCH_MAJOR, 0, 255, 0, 0);
>
> - return 0;
> -}
> + /* Read configuration and apply touchscreen parameters */
> + goodix_read_config(ts);
>
> -/**
> - * goodix_configure_dev - Finish device initialization
> - *
> - * @ts: our goodix_ts_data pointer
> - *
> - * Must be called from probe to finish initialization of the device.
> - * Contains the common initialization code for both devices that
> - * declare gpio pins and devices that do not. It is either called
> - * directly from probe or from request_firmware_wait callback.
> - */
> -static int goodix_configure_dev(struct goodix_ts_data *ts)
> -{
> - int error;
> + /* Try overriding touchscreen parameters via device properties */
> + touchscreen_parse_properties(ts->input_dev, true, &ts->prop);
>
> - ts->swapped_x_y = device_property_read_bool(&ts->client->dev,
> - "touchscreen-swapped-x-y");
> - ts->inverted_x = device_property_read_bool(&ts->client->dev,
> - "touchscreen-inverted-x");
> - ts->inverted_y = device_property_read_bool(&ts->client->dev,
> - "touchscreen-inverted-y");
> + if (!ts->prop.max_x || !ts->prop.max_y || !ts->max_touch_num) {
> + dev_err(&ts->client->dev, "Invalid config, using defaults\n");
> + ts->prop.max_x = GOODIX_MAX_WIDTH - 1;
> + ts->prop.max_y = GOODIX_MAX_HEIGHT - 1;
> + ts->max_touch_num = GOODIX_MAX_CONTACTS;
> + input_abs_set_max(ts->input_dev,
> + ABS_MT_POSITION_X, ts->prop.max_x);
> + input_abs_set_max(ts->input_dev,
> + ABS_MT_POSITION_Y, ts->prop.max_y);
> + }
>
> - goodix_read_config(ts);
> + if (dmi_check_system(rotated_screen)) {
> + ts->prop.invert_x = true;
> + ts->prop.invert_y = true;
> + dev_dbg(&ts->client->dev,
> + "Applying '180 degrees rotated screen' quirk\n");
> + }
>
> - error = goodix_request_input_dev(ts);
> - if (error)
> + error = input_mt_init_slots(ts->input_dev, ts->max_touch_num,
> + INPUT_MT_DIRECT | INPUT_MT_DROP_UNUSED);
> + if (error) {
> + dev_err(&ts->client->dev,
> + "Failed to initialize MT slots: %d", error);
> + return error;
> + }
> +
> + error = input_register_device(ts->input_dev);
> + if (error) {
> + dev_err(&ts->client->dev,
> + "Failed to register input device: %d", error);
> return error;
> + }
>
> ts->irq_flags = goodix_irq_flags[ts->int_trigger_type] | IRQF_ONESHOT;
> error = goodix_request_irq(ts);
--
Marcin Niestroj
^ permalink raw reply
* Re: [PATCH 4/8] Input: mms114 - remove unused variable
From: Dmitry Torokhov @ 2018-01-29 18:43 UTC (permalink / raw)
To: Andi Shyti; +Cc: Simon Shields, linux-input, linux-kernel, Andi Shyti
In-Reply-To: <20180129113323.18961-5-andi.shyti@samsung.com>
On Mon, Jan 29, 2018 at 08:33:19PM +0900, Andi Shyti wrote:
> '__packed' is not used anywhere, remove it.
Umm, this is not a variable, this is type annotation meaning that the
structure is packed. Still not needed, as we are not using anything but
u8 data elements, but justification is completely wrong.
>
> Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
> ---
> drivers/input/touchscreen/mms114.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
> index fb4435ae506b..11dba8bb48e3 100644
> --- a/drivers/input/touchscreen/mms114.c
> +++ b/drivers/input/touchscreen/mms114.c
> @@ -78,7 +78,7 @@ struct mms114_touch {
> u8 width;
> u8 strength;
> u8 reserved[2];
> -} __packed;
> +};
>
> static void mms114_process_mt(struct mms114_data *data, struct mms114_touch *touch)
> {
> --
> 2.15.1
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH 5/8] Input: mms114 - use lower case for hexadecimal values
From: Dmitry Torokhov @ 2018-01-29 18:56 UTC (permalink / raw)
To: Andi Shyti; +Cc: Simon Shields, linux-input, linux-kernel, Andi Shyti
In-Reply-To: <20180129113323.18961-6-andi.shyti@samsung.com>
On Mon, Jan 29, 2018 at 08:33:20PM +0900, Andi Shyti wrote:
> Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
But why?
> ---
> drivers/input/touchscreen/mms114.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
> index 11dba8bb48e3..fdf23bc416af 100644
> --- a/drivers/input/touchscreen/mms114.c
> +++ b/drivers/input/touchscreen/mms114.c
> @@ -20,7 +20,7 @@
>
> /* Write only registers */
> #define MMS114_MODE_CONTROL 0x01
> -#define MMS114_OPERATION_MODE_MASK 0xE
> +#define MMS114_OPERATION_MODE_MASK 0xe
> #define MMS114_ACTIVE (1 << 1)
>
> #define MMS114_XY_RESOLUTION_H 0x02
> @@ -30,12 +30,12 @@
> #define MMS114_MOVING_THRESHOLD 0x06
>
> /* Read only registers */
> -#define MMS114_PACKET_SIZE 0x0F
> +#define MMS114_PACKET_SIZE 0x0f
> #define MMS114_INFOMATION 0x10
> -#define MMS114_TSP_REV 0xF0
> +#define MMS114_TSP_REV 0xf0
>
> -#define MMS152_FW_REV 0xE1
> -#define MMS152_COMPAT_GROUP 0xF2
> +#define MMS152_FW_REV 0xe1
> +#define MMS152_COMPAT_GROUP 0xf2
>
> /* 200ms needs after power on */
> #define MMS114_POWERON_DELAY 200
> --
> 2.15.1
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH 2/8] Input: mms114 - get read of custm i2c read/write functions
From: Dmitry Torokhov @ 2018-01-29 19:01 UTC (permalink / raw)
To: Andi Shyti; +Cc: Simon Shields, linux-input, linux-kernel, Andi Shyti
In-Reply-To: <20180129113323.18961-3-andi.shyti@samsung.com>
On Mon, Jan 29, 2018 at 08:33:17PM +0900, Andi Shyti wrote:
> The 'mms114_read_reg' and 'mms114_write_reg' are used when
> reading or writing to the 'MMS114_MODE_CONTROL' register for
> updating the 'cache_mode_control' variable.
>
> Update the 'cache_mode_control' variable in the calling
> mms114_set_active() function and get rid of all the custom i2c
> read/write functions.
>
> With this remove also the redundant sleep of MMS114_I2C_DELAY
> (50us) between i2c operations. The waiting should to be handled
> by the i2c driver.
>
> Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
> ---
> drivers/input/touchscreen/mms114.c | 87 +++++---------------------------------
> 1 file changed, 10 insertions(+), 77 deletions(-)
>
> diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
> index 0b8b1f0e8ba6..94a97049d711 100644
> --- a/drivers/input/touchscreen/mms114.c
> +++ b/drivers/input/touchscreen/mms114.c
> @@ -37,9 +37,6 @@
> #define MMS152_FW_REV 0xE1
> #define MMS152_COMPAT_GROUP 0xF2
>
> -/* Minimum delay time is 50us between stop and start signal of i2c */
> -#define MMS114_I2C_DELAY 50
> -
> /* 200ms needs after power on */
> #define MMS114_POWERON_DELAY 200
>
> @@ -83,76 +80,6 @@ struct mms114_touch {
> u8 reserved[2];
> } __packed;
>
> -static int __mms114_read_reg(struct mms114_data *data, unsigned int reg,
> - unsigned int len, u8 *val)
> -{
> - struct i2c_client *client = data->client;
> - struct i2c_msg xfer[2];
> - u8 buf = reg & 0xff;
> - int error;
> -
> - if (reg <= MMS114_MODE_CONTROL && reg + len > MMS114_MODE_CONTROL)
> - BUG();
> -
> - /* Write register: use repeated start */
> - xfer[0].addr = client->addr;
> - xfer[0].flags = I2C_M_TEN | I2C_M_NOSTART;
So the chip does not use 10-bit addressing? What about I2C_M_NOSTART? It
is not needed also?
> - xfer[0].len = 1;
> - xfer[0].buf = &buf;
> -
> - /* Read data */
> - xfer[1].addr = client->addr;
> - xfer[1].flags = I2C_M_RD;
> - xfer[1].len = len;
> - xfer[1].buf = val;
> -
> - error = i2c_transfer(client->adapter, xfer, 2);
> - if (error != 2) {
> - dev_err(&client->dev,
> - "%s: i2c transfer failed (%d)\n", __func__, error);
> - return error < 0 ? error : -EIO;
> - }
> - udelay(MMS114_I2C_DELAY);
> -
> - return 0;
> -}
> -
> -static int mms114_read_reg(struct mms114_data *data, unsigned int reg)
> -{
> - u8 val;
> - int error;
> -
> - if (reg == MMS114_MODE_CONTROL)
> - return data->cache_mode_control;
> -
> - error = __mms114_read_reg(data, reg, 1, &val);
> - return error < 0 ? error : val;
> -}
> -
> -static int mms114_write_reg(struct mms114_data *data, unsigned int reg,
> - unsigned int val)
> -{
> - struct i2c_client *client = data->client;
> - u8 buf[2];
> - int error;
> -
> - buf[0] = reg & 0xff;
> - buf[1] = val & 0xff;
> -
> - error = i2c_master_send(client, buf, 2);
> - if (error != 2) {
> - dev_err(&client->dev,
> - "%s: i2c send failed (%d)\n", __func__, error);
> - return error < 0 ? error : -EIO;
> - }
> - udelay(MMS114_I2C_DELAY);
> -
> - if (reg == MMS114_MODE_CONTROL)
> - data->cache_mode_control = val;
> -
> - return 0;
> -}
> -
> static void mms114_process_mt(struct mms114_data *data, struct mms114_touch *touch)
> {
> struct i2c_client *client = data->client;
> @@ -231,19 +158,25 @@ static irqreturn_t mms114_interrupt(int irq, void *dev_id)
>
> static int mms114_set_active(struct mms114_data *data, bool active)
> {
> - int val;
> + int val, err;
>
> - val = mms114_read_reg(data, MMS114_MODE_CONTROL);
> + val = i2c_smbus_read_byte_data(data->client, MMS114_MODE_CONTROL);
If I understand the original commit for the driver, the control
register is write only and is not to be read from, that is why we have
this cached value. With your change you read form it.
By the way, have you looked into converting it all to regmap?
> if (val < 0)
> return val;
>
> - val &= ~MMS114_OPERATION_MODE_MASK;
> + val = data->cache_mode_control &= ~MMS114_OPERATION_MODE_MASK;
>
> /* If active is false, sleep mode */
> if (active)
> val |= MMS114_ACTIVE;
>
> - return mms114_write_reg(data, MMS114_MODE_CONTROL, val);
> + err = i2c_smbus_write_byte_data(data->client, MMS114_MODE_CONTROL, val);
> + if (err < 0)
> + return err;
> +
> + data->cache_mode_control = val;
> +
> + return 0;
> }
>
> static int mms114_get_version(struct mms114_data *data)
> --
> 2.15.1
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH 3/8] Input: mms114 - replace mdelay with msleep
From: Dmitry Torokhov @ 2018-01-29 19:46 UTC (permalink / raw)
To: Andi Shyti; +Cc: Simon Shields, linux-input, linux-kernel, Andi Shyti
In-Reply-To: <20180129113323.18961-4-andi.shyti@samsung.com>
On Mon, Jan 29, 2018 at 08:33:18PM +0900, Andi Shyti wrote:
> 200ms seconds is a very long time to keep the CPU busy looping.
> Use msleep instead.
>
> Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
Applied, thank you.
> ---
> drivers/input/touchscreen/mms114.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
> index 94a97049d711..fb4435ae506b 100644
> --- a/drivers/input/touchscreen/mms114.c
> +++ b/drivers/input/touchscreen/mms114.c
> @@ -290,7 +290,7 @@ static int mms114_start(struct mms114_data *data)
> return error;
> }
>
> - mdelay(MMS114_POWERON_DELAY);
> + msleep(MMS114_POWERON_DELAY);
>
> error = mms114_setup_regs(data);
> if (error < 0) {
> --
> 2.15.1
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH 6/8] Input: mms114 - Use BIT() macro instead of explicit shifting
From: Dmitry Torokhov @ 2018-01-29 19:46 UTC (permalink / raw)
To: Andi Shyti; +Cc: Simon Shields, linux-input, linux-kernel, Andi Shyti
In-Reply-To: <20180129113323.18961-7-andi.shyti@samsung.com>
On Mon, Jan 29, 2018 at 08:33:21PM +0900, Andi Shyti wrote:
> Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
Applied, thank you.
> ---
> drivers/input/touchscreen/mms114.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
> index fdf23bc416af..d70c03adf148 100644
> --- a/drivers/input/touchscreen/mms114.c
> +++ b/drivers/input/touchscreen/mms114.c
> @@ -21,7 +21,7 @@
> /* Write only registers */
> #define MMS114_MODE_CONTROL 0x01
> #define MMS114_OPERATION_MODE_MASK 0xe
> -#define MMS114_ACTIVE (1 << 1)
> +#define MMS114_ACTIVE BIT(1)
>
> #define MMS114_XY_RESOLUTION_H 0x02
> #define MMS114_X_RESOLUTION 0x03
> --
> 2.15.1
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH 7/8] Input: mms114 - add SPDX identifier
From: Dmitry Torokhov @ 2018-01-29 19:46 UTC (permalink / raw)
To: Andi Shyti; +Cc: Simon Shields, linux-input, linux-kernel, Andi Shyti
In-Reply-To: <20180129113323.18961-8-andi.shyti@samsung.com>
On Mon, Jan 29, 2018 at 08:33:22PM +0900, Andi Shyti wrote:
> Replace the original license statement with the SPDX identifier.
>
> Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
Applied, thank you.
> ---
> drivers/input/touchscreen/mms114.c | 13 +++++--------
> 1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
> index d70c03adf148..3230c92de1ed 100644
> --- a/drivers/input/touchscreen/mms114.c
> +++ b/drivers/input/touchscreen/mms114.c
> @@ -1,11 +1,8 @@
> -/*
> - * Copyright (C) 2012 Samsung Electronics Co.Ltd
> - * Author: Joonyoung Shim <jy0922.shim@samsung.com>
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License version 2 as
> - * published by the Free Software Foundation.
> - */
> +// SPDX-License-Identifier: GPL-2.0
> +// Samsung S6SY761 Touchscreen device driver
> +//
> +// Copyright (c) 2012 Samsung Electronics Co., Ltd.
> +// Author: Joonyoung Shim <jy0922.shim@samsung.com>
>
> #include <linux/module.h>
> #include <linux/delay.h>
> --
> 2.15.1
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH 8/8] Input: mms114 - fix typo in definition
From: Dmitry Torokhov @ 2018-01-29 19:47 UTC (permalink / raw)
To: Andi Shyti; +Cc: Simon Shields, linux-input, linux-kernel, Andi Shyti
In-Reply-To: <20180129113323.18961-9-andi.shyti@samsung.com>
On Mon, Jan 29, 2018 at 08:33:23PM +0900, Andi Shyti wrote:
> It's 'MMS114_INFORMATION', not 'MMS114_INFOMATION'
>
> Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
Applied, thank you.
> ---
> drivers/input/touchscreen/mms114.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
> index 3230c92de1ed..5b609531b3aa 100644
> --- a/drivers/input/touchscreen/mms114.c
> +++ b/drivers/input/touchscreen/mms114.c
> @@ -28,7 +28,7 @@
>
> /* Read only registers */
> #define MMS114_PACKET_SIZE 0x0f
> -#define MMS114_INFOMATION 0x10
> +#define MMS114_INFORMATION 0x10
> #define MMS114_TSP_REV 0xf0
>
> #define MMS152_FW_REV 0xe1
> @@ -138,7 +138,7 @@ static irqreturn_t mms114_interrupt(int irq, void *dev_id)
>
> touch_size = packet_size / MMS114_PACKET_NUM;
>
> - error = i2c_smbus_read_i2c_block_data(data->client, MMS114_INFOMATION,
> + error = i2c_smbus_read_i2c_block_data(data->client, MMS114_INFORMATION,
> packet_size, (u8 *)touch);
> if (error < 0)
> goto out;
> --
> 2.15.1
>
--
Dmitry
^ permalink raw reply
* [PATCH] iio/pressure: Delete an error message for a failed memory allocation in hid_press_probe()
From: SF Markus Elfring @ 2018-01-29 21:04 UTC (permalink / raw)
To: linux-iio, linux-input, Archana Patni, Hartmut Knaack,
Jiri Kosina, Jonathan Cameron, Lars-Peter Clausen,
Peter Meerwald-Stadler, Srinivas Pandruvada
Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 29 Jan 2018 21:55:08 +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>
---
drivers/iio/pressure/hid-sensor-press.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/iio/pressure/hid-sensor-press.c b/drivers/iio/pressure/hid-sensor-press.c
index 4c437918f1d2..9de7590d50b0 100644
--- a/drivers/iio/pressure/hid-sensor-press.c
+++ b/drivers/iio/pressure/hid-sensor-press.c
@@ -280,10 +280,8 @@ static int hid_press_probe(struct platform_device *pdev)
indio_dev->channels = kmemdup(press_channels, sizeof(press_channels),
GFP_KERNEL);
- if (!indio_dev->channels) {
- dev_err(&pdev->dev, "failed to duplicate channels\n");
+ if (!indio_dev->channels)
return -ENOMEM;
- }
ret = press_parse_report(pdev, hsdev,
(struct iio_chan_spec *)indio_dev->channels,
--
2.16.1
^ permalink raw reply related
* [PATCH v2] Input: synaptics-rmi4 - convert irq distribution to irq_domain
From: Nick Dyer @ 2018-01-29 21:12 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Benjamin Tissoires, Andrew Duggan, Christopher Heiny, linux-input,
Chris Healy, Lucas Stach, Nick Dyer
In-Reply-To: <20180124222315.GA28780@lava.h.shmanahar.org>
Convert the RMI driver to use the standard mechanism for
distributing IRQs to the various functions.
Tested on:
* S7300 (F11, F34, F54)
* S7817 (F12, F34, F54)
Signed-off-by: Nick Dyer <nick@shmanahar.org>
---
drivers/input/rmi4/Kconfig | 1 +
drivers/input/rmi4/rmi_bus.c | 42 +++++++++++++++++++++++++++++++++-
drivers/input/rmi4/rmi_bus.h | 2 +-
drivers/input/rmi4/rmi_driver.c | 50 +++++++++++++++--------------------------
drivers/input/rmi4/rmi_f01.c | 10 ++++-----
drivers/input/rmi4/rmi_f03.c | 9 ++++----
drivers/input/rmi4/rmi_f11.c | 25 +++++++++------------
drivers/input/rmi4/rmi_f12.c | 8 +++----
drivers/input/rmi4/rmi_f30.c | 9 ++++----
drivers/input/rmi4/rmi_f34.c | 5 +++--
drivers/input/rmi4/rmi_f54.c | 6 -----
include/linux/rmi.h | 2 ++
12 files changed, 95 insertions(+), 74 deletions(-)
diff --git a/drivers/input/rmi4/Kconfig b/drivers/input/rmi4/Kconfig
index 7172b88..fad2eae 100644
--- a/drivers/input/rmi4/Kconfig
+++ b/drivers/input/rmi4/Kconfig
@@ -3,6 +3,7 @@
#
config RMI4_CORE
tristate "Synaptics RMI4 bus support"
+ select IRQ_DOMAIN
help
Say Y here if you want to support the Synaptics RMI4 bus. This is
required for all RMI4 device support.
diff --git a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c
index ae1bffe..6360188 100644
--- a/drivers/input/rmi4/rmi_bus.c
+++ b/drivers/input/rmi4/rmi_bus.c
@@ -9,6 +9,8 @@
#include <linux/kernel.h>
#include <linux/device.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
#include <linux/list.h>
#include <linux/pm.h>
#include <linux/rmi.h>
@@ -167,6 +169,37 @@ static inline void rmi_function_of_probe(struct rmi_function *fn)
{}
#endif
+static struct irq_chip rmi_irq_chip = {
+ .name = "rmi4",
+};
+
+static int rmi_create_function_irq(struct rmi_function *fn,
+ struct rmi_function_handler *handler) {
+ struct rmi_driver_data *drvdata = dev_get_drvdata(&fn->rmi_dev->dev);
+ int i, irq, error;
+
+ for (i = 0; i < fn->num_of_irqs; i++) {
+ set_bit(fn->irq_pos + i, fn->irq_mask);
+
+ irq = irq_create_mapping(drvdata->irqdomain,
+ fn->irq_pos + i);
+
+ irq_set_chip_data(irq, fn);
+ irq_set_chip_and_handler(irq, &rmi_irq_chip, handle_simple_irq);
+ irq_set_nested_thread(irq, 1);
+
+ error = devm_request_threaded_irq(&fn->dev, irq, NULL,
+ handler->attention, IRQF_ONESHOT,
+ dev_name(&fn->dev), fn);
+ if (error) {
+ dev_err(&fn->dev, "Failed registering IRQ %d\n", error);
+ return error;
+ }
+ }
+
+ return 0;
+}
+
static int rmi_function_probe(struct device *dev)
{
struct rmi_function *fn = to_rmi_function(dev);
@@ -178,7 +211,14 @@ static int rmi_function_probe(struct device *dev)
if (handler->probe) {
error = handler->probe(fn);
- return error;
+ if (error)
+ return error;
+ }
+
+ if (fn->num_of_irqs && handler->attention) {
+ error = rmi_create_function_irq(fn, handler);
+ if (error)
+ return error;
}
return 0;
diff --git a/drivers/input/rmi4/rmi_bus.h b/drivers/input/rmi4/rmi_bus.h
index b7625a9..cbf8f3a 100644
--- a/drivers/input/rmi4/rmi_bus.h
+++ b/drivers/input/rmi4/rmi_bus.h
@@ -76,7 +76,7 @@ struct rmi_function_handler {
void (*remove)(struct rmi_function *fn);
int (*config)(struct rmi_function *fn);
int (*reset)(struct rmi_function *fn);
- int (*attention)(struct rmi_function *fn, unsigned long *irq_bits);
+ irqreturn_t (*attention)(int irq, void *ctx);
int (*suspend)(struct rmi_function *fn);
int (*resume)(struct rmi_function *fn);
};
diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index 141ea22..bc3a383 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -21,6 +21,7 @@
#include <linux/pm.h>
#include <linux/slab.h>
#include <linux/of.h>
+#include <linux/irqdomain.h>
#include <uapi/linux/input.h>
#include <linux/rmi.h>
#include "rmi_bus.h"
@@ -127,28 +128,11 @@ static int rmi_driver_process_config_requests(struct rmi_device *rmi_dev)
return 0;
}
-static void process_one_interrupt(struct rmi_driver_data *data,
- struct rmi_function *fn)
-{
- struct rmi_function_handler *fh;
-
- if (!fn || !fn->dev.driver)
- return;
-
- fh = to_rmi_function_handler(fn->dev.driver);
- if (fh->attention) {
- bitmap_and(data->fn_irq_bits, data->irq_status, fn->irq_mask,
- data->irq_count);
- if (!bitmap_empty(data->fn_irq_bits, data->irq_count))
- fh->attention(fn, data->fn_irq_bits);
- }
-}
-
static int rmi_process_interrupt_requests(struct rmi_device *rmi_dev)
{
struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
struct device *dev = &rmi_dev->dev;
- struct rmi_function *entry;
+ int i;
int error;
if (!data)
@@ -173,16 +157,8 @@ static int rmi_process_interrupt_requests(struct rmi_device *rmi_dev)
*/
mutex_unlock(&data->irq_mutex);
- /*
- * It would be nice to be able to use irq_chip to handle these
- * nested IRQs. Unfortunately, most of the current customers for
- * this driver are using older kernels (3.0.x) that don't support
- * the features required for that. Once they've shifted to more
- * recent kernels (say, 3.3 and higher), this should be switched to
- * use irq_chip.
- */
- list_for_each_entry(entry, &data->function_list, node)
- process_one_interrupt(data, entry);
+ for_each_set_bit(i, data->irq_status, data->irq_count)
+ handle_nested_irq(irq_find_mapping(data->irqdomain, i));
if (data->input)
input_sync(data->input);
@@ -1000,9 +976,12 @@ EXPORT_SYMBOL_GPL(rmi_driver_resume);
static int rmi_driver_remove(struct device *dev)
{
struct rmi_device *rmi_dev = to_rmi_device(dev);
+ struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
rmi_disable_irq(rmi_dev, false);
+ irq_domain_remove(data->irqdomain);
+
rmi_f34_remove_sysfs(rmi_dev);
rmi_free_function_list(rmi_dev);
@@ -1034,7 +1013,8 @@ int rmi_probe_interrupts(struct rmi_driver_data *data)
{
struct rmi_device *rmi_dev = data->rmi_dev;
struct device *dev = &rmi_dev->dev;
- int irq_count;
+ struct device_node *rmi_of_node = rmi_dev->xport->dev->of_node;
+ int irq_count = 0;
size_t size;
int retval;
@@ -1045,7 +1025,6 @@ int rmi_probe_interrupts(struct rmi_driver_data *data)
* being accessed.
*/
rmi_dbg(RMI_DEBUG_CORE, dev, "%s: Counting IRQs.\n", __func__);
- irq_count = 0;
data->bootloader_mode = false;
retval = rmi_scan_pdt(rmi_dev, &irq_count, rmi_count_irqs);
@@ -1057,6 +1036,14 @@ int rmi_probe_interrupts(struct rmi_driver_data *data)
if (data->bootloader_mode)
dev_warn(dev, "Device in bootloader mode.\n");
+ /* Allocate and register a linear revmap irq_domain */
+ data->irqdomain = irq_domain_add_linear(rmi_of_node, irq_count,
+ &irq_domain_simple_ops, data);
+ if (!data->irqdomain) {
+ dev_err(&rmi_dev->dev, "Failed to create IRQ domain\n");
+ return PTR_ERR(data->irqdomain);
+ }
+
data->irq_count = irq_count;
data->num_of_irq_regs = (data->irq_count + 7) / 8;
@@ -1079,10 +1066,9 @@ int rmi_init_functions(struct rmi_driver_data *data)
{
struct rmi_device *rmi_dev = data->rmi_dev;
struct device *dev = &rmi_dev->dev;
- int irq_count;
+ int irq_count = 0;
int retval;
- irq_count = 0;
rmi_dbg(RMI_DEBUG_CORE, dev, "%s: Creating functions.\n", __func__);
retval = rmi_scan_pdt(rmi_dev, &irq_count, rmi_create_function);
if (retval < 0) {
diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
index 8a07ae1..4edaa14 100644
--- a/drivers/input/rmi4/rmi_f01.c
+++ b/drivers/input/rmi4/rmi_f01.c
@@ -681,9 +681,9 @@ static int rmi_f01_resume(struct rmi_function *fn)
return 0;
}
-static int rmi_f01_attention(struct rmi_function *fn,
- unsigned long *irq_bits)
+static irqreturn_t rmi_f01_attention(int irq, void *ctx)
{
+ struct rmi_function *fn = ctx;
struct rmi_device *rmi_dev = fn->rmi_dev;
int error;
u8 device_status;
@@ -692,7 +692,7 @@ static int rmi_f01_attention(struct rmi_function *fn,
if (error) {
dev_err(&fn->dev,
"Failed to read device status: %d.\n", error);
- return error;
+ return IRQ_RETVAL(error);
}
if (RMI_F01_STATUS_BOOTLOADER(device_status))
@@ -704,11 +704,11 @@ static int rmi_f01_attention(struct rmi_function *fn,
error = rmi_dev->driver->reset_handler(rmi_dev);
if (error) {
dev_err(&fn->dev, "Device reset failed: %d\n", error);
- return error;
+ return IRQ_RETVAL(error);
}
}
- return 0;
+ return IRQ_HANDLED;
}
struct rmi_function_handler rmi_f01_handler = {
diff --git a/drivers/input/rmi4/rmi_f03.c b/drivers/input/rmi4/rmi_f03.c
index ad71a5e..b9f07c9 100644
--- a/drivers/input/rmi4/rmi_f03.c
+++ b/drivers/input/rmi4/rmi_f03.c
@@ -199,8 +199,9 @@ static int rmi_f03_config(struct rmi_function *fn)
return 0;
}
-static int rmi_f03_attention(struct rmi_function *fn, unsigned long *irq_bits)
+static irqreturn_t rmi_f03_attention(int irq, void *ctx)
{
+ struct rmi_function *fn = ctx;
struct rmi_device *rmi_dev = fn->rmi_dev;
struct rmi_driver_data *drvdata = dev_get_drvdata(&rmi_dev->dev);
struct f03_data *f03 = dev_get_drvdata(&fn->dev);
@@ -217,7 +218,7 @@ static int rmi_f03_attention(struct rmi_function *fn, unsigned long *irq_bits)
/* First grab the data passed by the transport device */
if (drvdata->attn_data.size < ob_len) {
dev_warn(&fn->dev, "F03 interrupted, but data is missing!\n");
- return 0;
+ return IRQ_HANDLED;
}
memcpy(obs, drvdata->attn_data.data, ob_len);
@@ -233,7 +234,7 @@ static int rmi_f03_attention(struct rmi_function *fn, unsigned long *irq_bits)
"%s: Failed to read F03 output buffers: %d\n",
__func__, error);
serio_interrupt(f03->serio, 0, SERIO_TIMEOUT);
- return error;
+ return IRQ_RETVAL(error);
}
}
@@ -259,7 +260,7 @@ static int rmi_f03_attention(struct rmi_function *fn, unsigned long *irq_bits)
serio_interrupt(f03->serio, ob_data, serio_flags);
}
- return 0;
+ return IRQ_HANDLED;
}
static void rmi_f03_remove(struct rmi_function *fn)
diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
index bc5e37f..e587c87 100644
--- a/drivers/input/rmi4/rmi_f11.c
+++ b/drivers/input/rmi4/rmi_f11.c
@@ -571,8 +571,7 @@ static inline u8 rmi_f11_parse_finger_state(const u8 *f_state, u8 n_finger)
static void rmi_f11_finger_handler(struct f11_data *f11,
struct rmi_2d_sensor *sensor,
- unsigned long *irq_bits, int num_irq_regs,
- int size)
+ int num_irq_regs, int size)
{
const u8 *f_state = f11->data.f_state;
u8 finger_state;
@@ -581,12 +580,7 @@ static void rmi_f11_finger_handler(struct f11_data *f11,
int rel_fingers;
int abs_size = sensor->nbr_fingers * RMI_F11_ABS_BYTES;
- int abs_bits = bitmap_and(f11->result_bits, irq_bits, f11->abs_mask,
- num_irq_regs * 8);
- int rel_bits = bitmap_and(f11->result_bits, irq_bits, f11->rel_mask,
- num_irq_regs * 8);
-
- if (abs_bits) {
+ if (sensor->report_abs) {
if (abs_size > size)
abs_fingers = size / RMI_F11_ABS_BYTES;
else
@@ -606,7 +600,7 @@ static void rmi_f11_finger_handler(struct f11_data *f11,
}
}
- if (rel_bits) {
+ if (sensor->report_rel) {
if ((abs_size + sensor->nbr_fingers * RMI_F11_REL_BYTES) > size)
rel_fingers = (size - abs_size) / RMI_F11_REL_BYTES;
else
@@ -616,7 +610,7 @@ static void rmi_f11_finger_handler(struct f11_data *f11,
rmi_f11_rel_pos_report(f11, i);
}
- if (abs_bits) {
+ if (sensor->report_abs) {
/*
* the absolute part is made in 2 parts to allow the kernel
* tracking to take place.
@@ -1275,8 +1269,9 @@ static int rmi_f11_config(struct rmi_function *fn)
return 0;
}
-static int rmi_f11_attention(struct rmi_function *fn, unsigned long *irq_bits)
+static irqreturn_t rmi_f11_attention(int irq, void *ctx)
{
+ struct rmi_function *fn = ctx;
struct rmi_device *rmi_dev = fn->rmi_dev;
struct rmi_driver_data *drvdata = dev_get_drvdata(&rmi_dev->dev);
struct f11_data *f11 = dev_get_drvdata(&fn->dev);
@@ -1302,13 +1297,13 @@ static int rmi_f11_attention(struct rmi_function *fn, unsigned long *irq_bits)
data_base_addr, f11->sensor.data_pkt,
f11->sensor.pkt_size);
if (error < 0)
- return error;
+ return IRQ_RETVAL(error);
}
- rmi_f11_finger_handler(f11, &f11->sensor, irq_bits,
- drvdata->num_of_irq_regs, valid_bytes);
+ rmi_f11_finger_handler(f11, &f11->sensor,
+ drvdata->num_of_irq_regs, valid_bytes);
- return 0;
+ return IRQ_HANDLED;
}
static int rmi_f11_resume(struct rmi_function *fn)
diff --git a/drivers/input/rmi4/rmi_f12.c b/drivers/input/rmi4/rmi_f12.c
index 8b0db08..e226def 100644
--- a/drivers/input/rmi4/rmi_f12.c
+++ b/drivers/input/rmi4/rmi_f12.c
@@ -197,10 +197,10 @@ static void rmi_f12_process_objects(struct f12_data *f12, u8 *data1, int size)
rmi_2d_sensor_abs_report(sensor, &sensor->objs[i], i);
}
-static int rmi_f12_attention(struct rmi_function *fn,
- unsigned long *irq_nr_regs)
+static irqreturn_t rmi_f12_attention(int irq, void *ctx)
{
int retval;
+ struct rmi_function *fn = ctx;
struct rmi_device *rmi_dev = fn->rmi_dev;
struct rmi_driver_data *drvdata = dev_get_drvdata(&rmi_dev->dev);
struct f12_data *f12 = dev_get_drvdata(&fn->dev);
@@ -222,7 +222,7 @@ static int rmi_f12_attention(struct rmi_function *fn,
if (retval < 0) {
dev_err(&fn->dev, "Failed to read object data. Code: %d.\n",
retval);
- return retval;
+ return IRQ_RETVAL(retval);
}
}
@@ -232,7 +232,7 @@ static int rmi_f12_attention(struct rmi_function *fn,
input_mt_sync_frame(sensor->input);
- return 0;
+ return IRQ_HANDLED;
}
static int rmi_f12_write_control_regs(struct rmi_function *fn)
diff --git a/drivers/input/rmi4/rmi_f30.c b/drivers/input/rmi4/rmi_f30.c
index 82e0f0d..5e3ed5a 100644
--- a/drivers/input/rmi4/rmi_f30.c
+++ b/drivers/input/rmi4/rmi_f30.c
@@ -122,8 +122,9 @@ static void rmi_f30_report_button(struct rmi_function *fn,
}
}
-static int rmi_f30_attention(struct rmi_function *fn, unsigned long *irq_bits)
+static irqreturn_t rmi_f30_attention(int irq, void *ctx)
{
+ struct rmi_function *fn = ctx;
struct f30_data *f30 = dev_get_drvdata(&fn->dev);
struct rmi_driver_data *drvdata = dev_get_drvdata(&fn->rmi_dev->dev);
int error;
@@ -134,7 +135,7 @@ static int rmi_f30_attention(struct rmi_function *fn, unsigned long *irq_bits)
if (drvdata->attn_data.size < f30->register_count) {
dev_warn(&fn->dev,
"F30 interrupted, but data is missing\n");
- return 0;
+ return IRQ_HANDLED;
}
memcpy(f30->data_regs, drvdata->attn_data.data,
f30->register_count);
@@ -147,7 +148,7 @@ static int rmi_f30_attention(struct rmi_function *fn, unsigned long *irq_bits)
dev_err(&fn->dev,
"%s: Failed to read F30 data registers: %d\n",
__func__, error);
- return error;
+ return IRQ_RETVAL(error);
}
}
@@ -159,7 +160,7 @@ static int rmi_f30_attention(struct rmi_function *fn, unsigned long *irq_bits)
rmi_f03_commit_buttons(f30->f03);
}
- return 0;
+ return IRQ_HANDLED;
}
static int rmi_f30_config(struct rmi_function *fn)
diff --git a/drivers/input/rmi4/rmi_f34.c b/drivers/input/rmi4/rmi_f34.c
index 4cfe970..7268cf9 100644
--- a/drivers/input/rmi4/rmi_f34.c
+++ b/drivers/input/rmi4/rmi_f34.c
@@ -101,8 +101,9 @@ static int rmi_f34_command(struct f34_data *f34, u8 command,
return 0;
}
-static int rmi_f34_attention(struct rmi_function *fn, unsigned long *irq_bits)
+static irqreturn_t rmi_f34_attention(int irq, void *ctx)
{
+ struct rmi_function *fn = ctx;
struct f34_data *f34 = dev_get_drvdata(&fn->dev);
int ret;
u8 status;
@@ -127,7 +128,7 @@ static int rmi_f34_attention(struct rmi_function *fn, unsigned long *irq_bits)
complete(&f34->v7.cmd_done);
}
- return 0;
+ return IRQ_HANDLED;
}
static int rmi_f34_write_blocks(struct f34_data *f34, const void *data,
diff --git a/drivers/input/rmi4/rmi_f54.c b/drivers/input/rmi4/rmi_f54.c
index 5343f2c..98a935e 100644
--- a/drivers/input/rmi4/rmi_f54.c
+++ b/drivers/input/rmi4/rmi_f54.c
@@ -610,11 +610,6 @@ static void rmi_f54_work(struct work_struct *work)
mutex_unlock(&f54->data_mutex);
}
-static int rmi_f54_attention(struct rmi_function *fn, unsigned long *irqbits)
-{
- return 0;
-}
-
static int rmi_f54_config(struct rmi_function *fn)
{
struct rmi_driver *drv = fn->rmi_dev->driver;
@@ -756,6 +751,5 @@ struct rmi_function_handler rmi_f54_handler = {
.func = 0x54,
.probe = rmi_f54_probe,
.config = rmi_f54_config,
- .attention = rmi_f54_attention,
.remove = rmi_f54_remove,
};
diff --git a/include/linux/rmi.h b/include/linux/rmi.h
index 64125443..5ef5c7c 100644
--- a/include/linux/rmi.h
+++ b/include/linux/rmi.h
@@ -354,6 +354,8 @@ struct rmi_driver_data {
struct mutex irq_mutex;
struct input_dev *input;
+ struct irq_domain *irqdomain;
+
u8 pdt_props;
u8 num_rx_electrodes;
--
2.7.4
^ permalink raw reply related
* [PATCH] iio/orientation: Delete an error message for a failed memory allocation in two functions
From: SF Markus Elfring @ 2018-01-29 21:26 UTC (permalink / raw)
To: linux-iio, linux-input, Hartmut Knaack, Jiri Kosina,
Jonathan Cameron, Lars-Peter Clausen, Peter Meerwald-Stadler,
Srinivas Pandruvada
Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 29 Jan 2018 22:20:07 +0100
Omit an extra message for a memory allocation failure in these functions.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/iio/orientation/hid-sensor-incl-3d.c | 4 +---
drivers/iio/orientation/hid-sensor-rotation.c | 4 +---
2 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/iio/orientation/hid-sensor-incl-3d.c b/drivers/iio/orientation/hid-sensor-incl-3d.c
index 1e5451d1ff88..742c0e0eb4cf 100644
--- a/drivers/iio/orientation/hid-sensor-incl-3d.c
+++ b/drivers/iio/orientation/hid-sensor-incl-3d.c
@@ -336,10 +336,8 @@ static int hid_incl_3d_probe(struct platform_device *pdev)
indio_dev->channels = kmemdup(incl_3d_channels,
sizeof(incl_3d_channels), GFP_KERNEL);
- if (!indio_dev->channels) {
- dev_err(&pdev->dev, "failed to duplicate channels\n");
+ if (!indio_dev->channels)
return -ENOMEM;
- }
ret = incl_3d_parse_report(pdev, hsdev,
(struct iio_chan_spec *)indio_dev->channels,
diff --git a/drivers/iio/orientation/hid-sensor-rotation.c b/drivers/iio/orientation/hid-sensor-rotation.c
index a69db2002414..cbf5e406b1ee 100644
--- a/drivers/iio/orientation/hid-sensor-rotation.c
+++ b/drivers/iio/orientation/hid-sensor-rotation.c
@@ -277,10 +277,8 @@ static int hid_dev_rot_probe(struct platform_device *pdev)
indio_dev->channels = devm_kmemdup(&pdev->dev, dev_rot_channels,
sizeof(dev_rot_channels),
GFP_KERNEL);
- if (!indio_dev->channels) {
- dev_err(&pdev->dev, "failed to duplicate channels\n");
+ if (!indio_dev->channels)
return -ENOMEM;
- }
ret = dev_rot_parse_report(pdev, hsdev,
(struct iio_chan_spec *)indio_dev->channels,
--
2.16.1
^ permalink raw reply related
* Re: [PATCH] Input: ALPS - fix DualPoint flag for 74 03 28 devices
From: dmitry.torokhov @ 2018-01-29 22:51 UTC (permalink / raw)
To: Masaki Ota
Cc: Pali Rohár, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org, Aaron Ma
In-Reply-To: <TYXPR01MB07195A0116F835764DE99882C72E0@TYXPR01MB0719.jpnprd01.prod.outlook.com>
Hi,
On Thu, Nov 16, 2017 at 07:27:02AM +0000, Masaki Ota wrote:
> Hi, Pali, Aaron,
>
> Current code is correct device setting, previous code is wrong.
> If the trackstick does not work(DUALPOINT flag disable), Device Firmware setting is wrong.
>
> But recently I received the same report from Thinkpad L570 user, and I checked this device and found this device Firmware setting is wrong. Sorry for our mistake.
> Is your laptop L570 ?
>
> I will add code that supports the trackstick for this device.
Sorry for resurrecting this old thread, I am just trying to understand
what went wrong here. Is the sequence of "f0 f0 e9" and "ea ea e9" is
important in getting the correct OTP data and we originally got this
order wrong? It is not clear from the original patch and discussion that
this change was intentional.
Thanks.
>
> Best Regards,
> Masaki Ota
> -----Original Message-----
> From: Pali Rohár [mailto:pali.rohar@gmail.com]
> Sent: Wednesday, November 15, 2017 5:35 PM
> To: 太田 真喜 Masaki Ota <masaki.ota@jp.alps.com>
> Cc: linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; dmitry.torokhov@gmail.com; Aaron Ma <aaron.ma@canonical.com>
> Subject: Re: [PATCH] Input: ALPS - fix DualPoint flag for 74 03 28 devices
>
> On Wednesday 15 November 2017 14:34:04 Aaron Ma wrote:
> > There is a regression of commit 4a646580f793 ("Input: ALPS - fix
> > two-finger scroll breakage"), ALPS device fails with log:
> >
> > psmouse serio1: alps: Rejected trackstick packet from non DualPoint
> > device
> >
> > ALPS device with id "74 03 28" report OTP[0] data 0xCE after commit
> > 4a646580f793, after restore the OTP reading order, it becomes to 0x10
> > as before and reports the right flag.
> >
> > Fixes: 4a646580f793 ("Input: ALPS - fix two-finger scroll breakage")
> > Cc: <stable@vger.kernel.org>
> > Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
> > ---
> > drivers/input/mouse/alps.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> > index 579b899add26..c59b8f7ca2fc 100644
> > --- a/drivers/input/mouse/alps.c
> > +++ b/drivers/input/mouse/alps.c
> > @@ -2562,8 +2562,8 @@ static int alps_set_defaults_ss4_v2(struct
> > psmouse *psmouse,
> >
> > memset(otp, 0, sizeof(otp));
> >
> > - if (alps_get_otp_values_ss4_v2(psmouse, 1, &otp[1][0]) ||
> > - alps_get_otp_values_ss4_v2(psmouse, 0, &otp[0][0]))
> > + if (alps_get_otp_values_ss4_v2(psmouse, 0, &otp[0][0]) ||
> > + alps_get_otp_values_ss4_v2(psmouse, 1, &otp[1][0]))
> > return -1;
> >
> > alps_update_device_area_ss4_v2(otp, priv);
>
> Masaki Ota, please look at this patch as it partially revert your commit
> 4a646580f793 ("Input: ALPS - fix two-finger scroll breakage"). Something smells here.
>
> --
> Pali Rohár
> pali.rohar@gmail.com
--
Dmitry
^ permalink raw reply
* Re: [PATCH 2/8] Input: mms114 - get read of custm i2c read/write functions
From: Andi Shyti @ 2018-01-29 23:25 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Simon Shields, linux-input, linux-kernel, Andi Shyti
In-Reply-To: <20180129190141.w2q55gfjjnvepl6k@dtor-ws>
Hi Dmitry,
On Mon, Jan 29, 2018 at 11:01:41AM -0800, Dmitry Torokhov wrote:
> On Mon, Jan 29, 2018 at 08:33:17PM +0900, Andi Shyti wrote:
> > The 'mms114_read_reg' and 'mms114_write_reg' are used when
> > reading or writing to the 'MMS114_MODE_CONTROL' register for
> > updating the 'cache_mode_control' variable.
> >
> > Update the 'cache_mode_control' variable in the calling
> > mms114_set_active() function and get rid of all the custom i2c
> > read/write functions.
> >
> > With this remove also the redundant sleep of MMS114_I2C_DELAY
> > (50us) between i2c operations. The waiting should to be handled
> > by the i2c driver.
> >
> > Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
> > ---
> > drivers/input/touchscreen/mms114.c | 87 +++++---------------------------------
> > 1 file changed, 10 insertions(+), 77 deletions(-)
> >
> > diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
> > index 0b8b1f0e8ba6..94a97049d711 100644
> > --- a/drivers/input/touchscreen/mms114.c
> > +++ b/drivers/input/touchscreen/mms114.c
> > @@ -37,9 +37,6 @@
> > #define MMS152_FW_REV 0xE1
> > #define MMS152_COMPAT_GROUP 0xF2
> >
> > -/* Minimum delay time is 50us between stop and start signal of i2c */
> > -#define MMS114_I2C_DELAY 50
> > -
> > /* 200ms needs after power on */
> > #define MMS114_POWERON_DELAY 200
> >
> > @@ -83,76 +80,6 @@ struct mms114_touch {
> > u8 reserved[2];
> > } __packed;
> >
> > -static int __mms114_read_reg(struct mms114_data *data, unsigned int reg,
> > - unsigned int len, u8 *val)
> > -{
> > - struct i2c_client *client = data->client;
> > - struct i2c_msg xfer[2];
> > - u8 buf = reg & 0xff;
> > - int error;
> > -
> > - if (reg <= MMS114_MODE_CONTROL && reg + len > MMS114_MODE_CONTROL)
> > - BUG();
> > -
> > - /* Write register: use repeated start */
> > - xfer[0].addr = client->addr;
> > - xfer[0].flags = I2C_M_TEN | I2C_M_NOSTART;
>
> So the chip does not use 10-bit addressing? What about I2C_M_NOSTART? It
> is not needed also?
>From the datasheet I have no, indeed I don't understand why this
is coming. That's why I asked Simon to test it on his device as
well.
On my device this patch works just fine.
> > - xfer[0].len = 1;
> > - xfer[0].buf = &buf;
> > -
> > - /* Read data */
> > - xfer[1].addr = client->addr;
> > - xfer[1].flags = I2C_M_RD;
> > - xfer[1].len = len;
> > - xfer[1].buf = val;
> > -
> > - error = i2c_transfer(client->adapter, xfer, 2);
> > - if (error != 2) {
> > - dev_err(&client->dev,
> > - "%s: i2c transfer failed (%d)\n", __func__, error);
> > - return error < 0 ? error : -EIO;
> > - }
> > - udelay(MMS114_I2C_DELAY);
> > -
> > - return 0;
> > -}
> > -
> > -static int mms114_read_reg(struct mms114_data *data, unsigned int reg)
> > -{
> > - u8 val;
> > - int error;
> > -
> > - if (reg == MMS114_MODE_CONTROL)
> > - return data->cache_mode_control;
> > -
> > - error = __mms114_read_reg(data, reg, 1, &val);
> > - return error < 0 ? error : val;
> > -}
> > -
> > -static int mms114_write_reg(struct mms114_data *data, unsigned int reg,
> > - unsigned int val)
> > -{
> > - struct i2c_client *client = data->client;
> > - u8 buf[2];
> > - int error;
> > -
> > - buf[0] = reg & 0xff;
> > - buf[1] = val & 0xff;
> > -
> > - error = i2c_master_send(client, buf, 2);
> > - if (error != 2) {
> > - dev_err(&client->dev,
> > - "%s: i2c send failed (%d)\n", __func__, error);
> > - return error < 0 ? error : -EIO;
> > - }
> > - udelay(MMS114_I2C_DELAY);
> > -
> > - if (reg == MMS114_MODE_CONTROL)
> > - data->cache_mode_control = val;
> > -
> > - return 0;
> > -}
> > -
> > static void mms114_process_mt(struct mms114_data *data, struct mms114_touch *touch)
> > {
> > struct i2c_client *client = data->client;
> > @@ -231,19 +158,25 @@ static irqreturn_t mms114_interrupt(int irq, void *dev_id)
> >
> > static int mms114_set_active(struct mms114_data *data, bool active)
> > {
> > - int val;
> > + int val, err;
> >
> > - val = mms114_read_reg(data, MMS114_MODE_CONTROL);
> > + val = i2c_smbus_read_byte_data(data->client, MMS114_MODE_CONTROL);
>
> If I understand the original commit for the driver, the control
> register is write only and is not to be read from, that is why we have
> this cached value. With your change you read form it.
Still in my datasheet thee MMS114_MODE_CONTROL is a R/W register.
Still I don't understand the original choice, but it doesn't
change much anyway, I can keep the original idea of never reading
from it. Or I can rework this function in a better way.
> By the way, have you looked into converting it all to regmap?
I could.
Andi
^ permalink raw reply
* Re: [PATCH 4/8] Input: mms114 - remove unused variable
From: Andi Shyti @ 2018-01-29 23:27 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Simon Shields, linux-input, linux-kernel, Andi Shyti
In-Reply-To: <20180129184309.ljnw63owyuhofqko@dtor-ws>
Hi Dmitry,
On Mon, Jan 29, 2018 at 10:43:09AM -0800, Dmitry Torokhov wrote:
> On Mon, Jan 29, 2018 at 08:33:19PM +0900, Andi Shyti wrote:
> > '__packed' is not used anywhere, remove it.
>
> Umm, this is not a variable, this is type annotation meaning that the
> structure is packed. Still not needed, as we are not using anything but
> u8 data elements, but justification is completely wrong.
Oh dammit! :)
of course! The original idea was the alignment of the structure,
but I must have got confused while re-editing all the commits.
Sorry, I'll fix it.
Andi
^ permalink raw reply
* Re: [PATCH 5/8] Input: mms114 - use lower case for hexadecimal values
From: Andi Shyti @ 2018-01-29 23:29 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Simon Shields, linux-input, linux-kernel, Andi Shyti
In-Reply-To: <20180129185631.wvgl3il6tu6nk7ew@dtor-ws>
Hi Dmitry,
On Mon, Jan 29, 2018 at 10:56:31AM -0800, Dmitry Torokhov wrote:
> On Mon, Jan 29, 2018 at 08:33:20PM +0900, Andi Shyti wrote:
> > Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
>
> But why?
I think this patch is trivial and the subject itself is quite
self-explanatory and the commit message would be a copy-paste of
it.
OK, if you want I will add "some comment".
Andi
> > ---
> > drivers/input/touchscreen/mms114.c | 10 +++++-----
> > 1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
> > index 11dba8bb48e3..fdf23bc416af 100644
> > --- a/drivers/input/touchscreen/mms114.c
> > +++ b/drivers/input/touchscreen/mms114.c
> > @@ -20,7 +20,7 @@
> >
> > /* Write only registers */
> > #define MMS114_MODE_CONTROL 0x01
> > -#define MMS114_OPERATION_MODE_MASK 0xE
> > +#define MMS114_OPERATION_MODE_MASK 0xe
> > #define MMS114_ACTIVE (1 << 1)
> >
> > #define MMS114_XY_RESOLUTION_H 0x02
> > @@ -30,12 +30,12 @@
> > #define MMS114_MOVING_THRESHOLD 0x06
> >
> > /* Read only registers */
> > -#define MMS114_PACKET_SIZE 0x0F
> > +#define MMS114_PACKET_SIZE 0x0f
> > #define MMS114_INFOMATION 0x10
> > -#define MMS114_TSP_REV 0xF0
> > +#define MMS114_TSP_REV 0xf0
> >
> > -#define MMS152_FW_REV 0xE1
> > -#define MMS152_COMPAT_GROUP 0xF2
> > +#define MMS152_FW_REV 0xe1
> > +#define MMS152_COMPAT_GROUP 0xf2
> >
> > /* 200ms needs after power on */
> > #define MMS114_POWERON_DELAY 200
> > --
> > 2.15.1
> >
>
> --
> Dmitry
>
^ permalink raw reply
* Re: [PATCH 5/8] Input: mms114 - use lower case for hexadecimal values
From: Dmitry Torokhov @ 2018-01-29 23:33 UTC (permalink / raw)
To: Andi Shyti; +Cc: Simon Shields, linux-input, linux-kernel, Andi Shyti
In-Reply-To: <20180129232923.GW7575@gangnam.samsung>
On Tue, Jan 30, 2018 at 08:29:23AM +0900, Andi Shyti wrote:
> Hi Dmitry,
>
> On Mon, Jan 29, 2018 at 10:56:31AM -0800, Dmitry Torokhov wrote:
> > On Mon, Jan 29, 2018 at 08:33:20PM +0900, Andi Shyti wrote:
> > > Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
> >
> > But why?
>
> I think this patch is trivial and the subject itself is quite
> self-explanatory and the commit message would be a copy-paste of
> it.
Here is where we disagree. Why lowercase is preferred over uppercase? Do
we have this in coding style (and I completely forgot about it)? Or is
it your preference (mine is too to be fair, but I also do not care
about churning the code for it)?
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 5/8] Input: mms114 - use lower case for hexadecimal values
From: Andi Shyti @ 2018-01-29 23:41 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Simon Shields, linux-input, linux-kernel, Andi Shyti
In-Reply-To: <20180129233335.ikqioeetkkvocblm@dtor-ws>
Hi Dmitry,
On Mon, Jan 29, 2018 at 03:33:35PM -0800, Dmitry Torokhov wrote:
> On Tue, Jan 30, 2018 at 08:29:23AM +0900, Andi Shyti wrote:
> > Hi Dmitry,
> >
> > On Mon, Jan 29, 2018 at 10:56:31AM -0800, Dmitry Torokhov wrote:
> > > On Mon, Jan 29, 2018 at 08:33:20PM +0900, Andi Shyti wrote:
> > > > Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
> > >
> > > But why?
> >
> > I think this patch is trivial and the subject itself is quite
> > self-explanatory and the commit message would be a copy-paste of
> > it.
>
> Here is where we disagree. Why lowercase is preferred over uppercase? Do
> we have this in coding style (and I completely forgot about it)? Or is
> it your preference (mine is too to be fair, but I also do not care
> about churning the code for it)?
I think this is the case where the cultural habits become law, I've
been writing lower case hex values from years and my eyes are so
used to it that it looks wrong to have them written upper case.
Many maintainers reject upper case hex values.
As well as many reject the " ? : " conditional form.
In any case, you're basically right here, at this point I
don't mind if you drop it :)
Thanks,
Andi
^ permalink raw reply
* Trackpoint not found when loading psmouse before rmi_smbus
From: Damjan Georgievski @ 2018-01-30 12:46 UTC (permalink / raw)
To: linux-input
This is tested on X1 Carbon (gen 5)
pnp id: LEN0072
kernel 4.15.0
Since by default psmouse complains about
"synaptics: Your touchpad (PNP: LEN0072 PNP0f13) says it can support a
different bus....",
I've patched the kernel to add the pnp id to smbus_pnp_ids in
drivers/input/mouse/synaptics.c [1],
and the trackpad seems to work at least a bit smoother (it could be
placebo though).
But, with the default setup, and rmi_smbus and psmouse are both
compiled as modules, the trackpoint device never appears in dmesg.
I can make it appear if I put rmi_smbus in /etc/modules-load.d/ so it
loads before psmouse, or if I unload psmouse and load it again.
I can even reproduce it by unloading both rmi_smbus and psmouse, and
then only loading psmouse
(psmouse seems to load rmi_smbus and switch the trackpad to it but the
trackpoint is lost)
Here's the output of dmesg. first part when psmouse is loaded before
rmi_smbus, second when rmi_smbus is first loaded, then psmouse:
[ 3720.674955] psmouse serio1: synaptics: queried max coordinates: x
[..5676], y [..4760]
[ 3720.707854] psmouse serio1: synaptics: queried min coordinates: x
[1266..], y [1094..]
[ 3720.707866] psmouse serio1: synaptics: Trying to set up SMBus access
[ 3720.719392] rmi4_smbus 6-002c: registering SMbus-connected sensor
[ 3720.780999] rmi4_f01 rmi4-02.fn01: found RMI device, manufacturer:
Synaptics, product: TM3289-002, fw id: 2492434
[ 3720.848800] input: Synaptics TM3289-002 as /devices/rmi4-02/input/input26
<... no other messages here I just put a separator to be a bit more clear ...>
[ 3755.533836] psmouse serio1: synaptics: queried max coordinates: x
[..5676], y [..4760]
[ 3755.565863] psmouse serio1: synaptics: queried min coordinates: x
[1266..], y [1094..]
[ 3755.565875] psmouse serio1: synaptics: Trying to set up SMBus access
[ 3755.569661] rmi4_smbus 6-002c: registering SMbus-connected sensor
[ 3755.636795] rmi4_f01 rmi4-03.fn01: found RMI device, manufacturer:
Synaptics, product: TM3289-002, fw id: 2492434
[ 3755.701859] input: Synaptics TM3289-002 as /devices/rmi4-03/input/input30
[ 3755.836966] psmouse serio5: trackpoint: ALPS TrackPoint firmware:
0x01, buttons: 3/3
[ 3755.880664] input: TPPS/2 ALPS TrackPoint as
/devices/rmi4-03/rmi4-03.fn03/serio5/input/input31
[ 3758.969259] psmouse serio5: TrackPoint at
synaptics-rmi4-pt/serio1/input0 lost synchronization, throwing 1 bytes
away.
[1]
https://github.com/gdamjan/linux/commit/fa6b3c893bea6d017b575438a4b367f2a7fc7717
--
damjan
^ permalink raw reply
* Re: i8042 AUX port [serio1] suspend takes a second on Dell XPS 13 9360 and TUXEDO Book 1406
From: Dmitry Torokhov @ 2018-01-30 17:52 UTC (permalink / raw)
To: Paul Menzel; +Cc: linux-input, Linux Kernel Mailing List, it+linux-input
In-Reply-To: <5a74e1a7-2083-c6b2-f51a-f44908ac8bc4@molgen.mpg.de>
Hi Paul,
On Tue, Jan 30, 2018 at 06:36:34PM +0100, Paul Menzel wrote:
> Dear Linux folks,
>
>
> I do not know, when it started, but with Linux 4.14-rc8 and 4.15,
> benchmarking suspend and resume time with `sleepgraph.py` [1][2], there is a
> regression, that i8042 AUX port [serio1] suspend takes a second on Dell XPS
> 13 9360 and TUXEDO Book 1406.
It would be really helpful to know when the regression started.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: i8042 AUX port [serio1] suspend takes a second on Dell XPS 13 9360 and TUXEDO Book 1406
From: Dmitry Torokhov @ 2018-01-30 18:07 UTC (permalink / raw)
To: Paul Menzel; +Cc: linux-input, Linux Kernel Mailing List, it+linux-input
In-Reply-To: <20180130175017.72inurhk5l7tcvae@dtor-ws>
On Tue, Jan 30, 2018 at 09:52:45AM -0800, Dmitry Torokhov wrote:
> Hi Paul,
>
> On Tue, Jan 30, 2018 at 06:36:34PM +0100, Paul Menzel wrote:
> > Dear Linux folks,
> >
> >
> > I do not know, when it started, but with Linux 4.14-rc8 and 4.15,
> > benchmarking suspend and resume time with `sleepgraph.py` [1][2], there is a
> > regression, that i8042 AUX port [serio1] suspend takes a second on Dell XPS
> > 13 9360 and TUXEDO Book 1406.
>
> It would be really helpful to know when the regression started.
So the reason it takes longer is because the touchpad does not want to
talk to us for some reason and we wait until commands time out:
[ 94.591636] calling serio1+ @ 2299, parent: i8042
[ 94.794292] psmouse serio1: Failed to disable mouse on isa0060/serio1
[ 95.593303] call serio1+ returned 0 after 974280 usecs
but it is not clear why it happens, I do not think we changed anything
in that path for a while, so it might be some other change affecting
things indirectly. I'm afraid you'll have to narrow the scope, and
ideally bisect.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: i8042 AUX port [serio1] suspend takes a second on Dell XPS 13 9360 and TUXEDO Book 1406
From: Paul Menzel @ 2018-01-30 18:39 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, it+linux-input
In-Reply-To: <20180130180736.ehucqywbz7zdl4v7@dtor-ws>
[-- Attachment #1: Type: text/plain, Size: 1512 bytes --]
Dear Dmitry,
On 01/30/18 19:07, Dmitry Torokhov wrote:
> On Tue, Jan 30, 2018 at 09:52:45AM -0800, Dmitry Torokhov wrote:
>> On Tue, Jan 30, 2018 at 06:36:34PM +0100, Paul Menzel wrote:
>>> I do not know, when it started, but with Linux 4.14-rc8 and 4.15,
>>> benchmarking suspend and resume time with `sleepgraph.py` [1][2], there is a
>>> regression, that i8042 AUX port [serio1] suspend takes a second on Dell XPS
>>> 13 9360 and TUXEDO Book 1406.
>>
>> It would be really helpful to know when the regression started.
>
> So the reason it takes longer is because the touchpad does not want to
> talk to us for some reason and we wait until commands time out:
>
> [ 94.591636] calling serio1+ @ 2299, parent: i8042
> [ 94.794292] psmouse serio1: Failed to disable mouse on isa0060/serio1
> [ 95.593303] call serio1+ returned 0 after 974280 usecs
>
> but it is not clear why it happens, I do not think we changed anything
> in that path for a while, so it might be some other change affecting
> things indirectly. I'm afraid you'll have to narrow the scope, and
> ideally bisect.
Thank you for looking into this. Just to note, that it worked with Linux
4.14.1 from Ubuntu 17.10, so it’s probably something in 4.15. Bisecting
will take a bit, as the FTRACE stuff broke in Linux 4.15-rc1 and was
only fixed in 4.15-rc9. Hopefully, it’s just one commit I need to
backport. I’ll get back to you hopefully at the end of the week.
Kind regards,
Paul
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5174 bytes --]
^ 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