Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH] Input: matrix-keymap: Delete an error message for a failed memory allocation in matrix_keypad_build_keymap()
From: SF Markus Elfring @ 2018-01-25 20:12 UTC (permalink / raw)
  To: linux-input, Dmitry Torokhov; +Cc: LKML, kernel-janitors

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 25 Jan 2018 21:07:00 +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/input/matrix-keymap.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/input/matrix-keymap.c b/drivers/input/matrix-keymap.c
index 8ccefc15c7a4..298bd56b02bd 100644
--- a/drivers/input/matrix-keymap.c
+++ b/drivers/input/matrix-keymap.c
@@ -173,11 +173,8 @@ int matrix_keypad_build_keymap(const struct matrix_keymap_data *keymap_data,
 		keymap = devm_kzalloc(input_dev->dev.parent,
 				      max_keys * sizeof(*keymap),
 				      GFP_KERNEL);
-		if (!keymap) {
-			dev_err(input_dev->dev.parent,
-				"Unable to allocate memory for keymap");
+		if (!keymap)
 			return -ENOMEM;
-		}
 	}
 
 	input_dev->keycode = keymap;
-- 
2.16.1


^ permalink raw reply related

* [PATCH] Input: tegra-kbc: Delete an error message for a failed memory allocation in tegra_kbc_probe()
From: SF Markus Elfring @ 2018-01-25 20:41 UTC (permalink / raw)
  To: linux-input, linux-tegra, Dmitry Torokhov, Jonathan Hunter,
	Laxman Dewangan, Rakesh Iyer, Thierry Reding
  Cc: LKML, kernel-janitors

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 25 Jan 2018 21:36:46 +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/input/keyboard/tegra-kbc.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/input/keyboard/tegra-kbc.c b/drivers/input/keyboard/tegra-kbc.c
index 875205f445b5..a4d6eea7aebe 100644
--- a/drivers/input/keyboard/tegra-kbc.c
+++ b/drivers/input/keyboard/tegra-kbc.c
@@ -622,10 +622,8 @@ static int tegra_kbc_probe(struct platform_device *pdev)
 	match = of_match_device(tegra_kbc_of_match, &pdev->dev);
 
 	kbc = devm_kzalloc(&pdev->dev, sizeof(*kbc), GFP_KERNEL);
-	if (!kbc) {
-		dev_err(&pdev->dev, "failed to alloc memory for kbc\n");
+	if (!kbc)
 		return -ENOMEM;
-	}
 
 	kbc->dev = &pdev->dev;
 	kbc->hw_support = match->data;
-- 
2.16.1

^ permalink raw reply related

* [PATCH] Input: spear-keyboard: Delete an error message for a failed memory allocation in spear_kbd_probe()
From: SF Markus Elfring @ 2018-01-25 20:52 UTC (permalink / raw)
  To: linux-input, Dmitry Torokhov; +Cc: LKML, kernel-janitors

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 25 Jan 2018 21:49:13 +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/input/keyboard/spear-keyboard.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/input/keyboard/spear-keyboard.c b/drivers/input/keyboard/spear-keyboard.c
index 7d25fa338ab4..1386440a870d 100644
--- a/drivers/input/keyboard/spear-keyboard.c
+++ b/drivers/input/keyboard/spear-keyboard.c
@@ -197,10 +197,8 @@ static int spear_kbd_probe(struct platform_device *pdev)
 	}
 
 	kbd = devm_kzalloc(&pdev->dev, sizeof(*kbd), GFP_KERNEL);
-	if (!kbd) {
-		dev_err(&pdev->dev, "not enough memory for driver data\n");
+	if (!kbd)
 		return -ENOMEM;
-	}
 
 	input_dev = devm_input_allocate_device(&pdev->dev);
 	if (!input_dev) {
-- 
2.16.1


^ permalink raw reply related

* [PATCH] Input: sh_keysc: Delete an error message for a failed memory allocation in sh_keysc_probe()
From: SF Markus Elfring @ 2018-01-25 21:06 UTC (permalink / raw)
  To: linux-input, Dmitry Torokhov; +Cc: LKML, kernel-janitors

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 25 Jan 2018 22:02:46 +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/input/keyboard/sh_keysc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/input/keyboard/sh_keysc.c b/drivers/input/keyboard/sh_keysc.c
index 7abf03b4cc9c..e214d47724f8 100644
--- a/drivers/input/keyboard/sh_keysc.c
+++ b/drivers/input/keyboard/sh_keysc.c
@@ -191,7 +191,6 @@ static int sh_keysc_probe(struct platform_device *pdev)
 
 	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
 	if (priv == NULL) {
-		dev_err(&pdev->dev, "failed to allocate driver data\n");
 		error = -ENOMEM;
 		goto err0;
 	}
-- 
2.16.1

^ permalink raw reply related

* [PATCH] Input: pxa27x_keypad: Delete an error message for a failed memory allocation
From: SF Markus Elfring @ 2018-01-25 21:20 UTC (permalink / raw)
  To: linux-input, Arvind Yadav, Dmitry Torokhov; +Cc: LKML, kernel-janitors

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 25 Jan 2018 22:15:27 +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/input/keyboard/pxa27x_keypad.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/input/keyboard/pxa27x_keypad.c b/drivers/input/keyboard/pxa27x_keypad.c
index d0bdaeadf86d..bafd075344e2 100644
--- a/drivers/input/keyboard/pxa27x_keypad.c
+++ b/drivers/input/keyboard/pxa27x_keypad.c
@@ -290,10 +290,8 @@ static int pxa27x_keypad_build_keycode_from_dt(struct pxa27x_keypad *keypad)
 	int error;
 
 	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
-	if (!pdata) {
-		dev_err(dev, "failed to allocate memory for pdata\n");
+	if (!pdata)
 		return -ENOMEM;
-	}
 
 	error = pxa27x_keypad_matrix_key_parse_dt(keypad, pdata);
 	if (error) {
-- 
2.16.1


^ permalink raw reply related

* [git pull] Input updates for v4.15-rc9
From: Dmitry Torokhov @ 2018-01-26  1:17 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-kernel, linux-input

Hi Linus,

Please pull from:

	git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git for-linus

to receive updates for the input subsystem. The main item is that we try
to handle better newer trackpoints on Lenovo devices that now being
produced by Elan/ALPS/NXP and only implement a small subset of the
original IBM trackpoint controls.

Changelog:
---------

Aaron Ma (1):
      Input: trackpoint - force 3 buttons if 0 button is reported

Andi Shyti (1):
      Input: stmfts,s6sy671 - add SPDX identifier

Dmitry Torokhov (1):
      Input: trackpoint - only expose supported controls for Elan, ALPS and NXP

Mark Furneaux (1):
      Input: xpad - add support for PDP Xbox One controllers

Nick Dyer (1):
      Revert "Input: synaptics_rmi4 - use devm_device_add_group() for attributes in F01"

Diffstat:
--------

 drivers/input/joystick/xpad.c       |  19 +++
 drivers/input/mouse/trackpoint.c    | 245 ++++++++++++++++++++++--------------
 drivers/input/mouse/trackpoint.h    |  34 +++--
 drivers/input/rmi4/rmi_f01.c        |  12 +-
 drivers/input/touchscreen/s6sy761.c |  15 +--
 drivers/input/touchscreen/stmfts.c  |  15 +--
 6 files changed, 210 insertions(+), 130 deletions(-)

Thanks.


-- 
Dmitry

^ permalink raw reply

* Re: [PATCH 1/4] Input: mms114 - do not clobber interrupt trigger
From: Andi Shyti @ 2018-01-26  4:54 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Simon Shields, Rob Herring, linux-input, linux-kernel
In-Reply-To: <20180124193804.155654-2-dmitry.torokhov@gmail.com>

Hi Dmitry,

On Wed, Jan 24, 2018 at 11:38:01AM -0800, Dmitry Torokhov wrote:
> Rely on the platform (device tree, ACPI, etc) to properly configure
> interrupt trigger/polarity instead of hardcoding the falling edge.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Reviewed-by: Andi Shyti <andi.shyti@samsung.com>
Tested-by: Andi Shyti <andi.shyti@samsung.com>

Thanks,
Andi

^ permalink raw reply

* Re: [PATCH 2/4] Input: mms114 - mark as direct input device
From: Andi Shyti @ 2018-01-26  4:55 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Simon Shields, Rob Herring, linux-input, linux-kernel
In-Reply-To: <20180124193804.155654-3-dmitry.torokhov@gmail.com>

Hi Dmitry,

On Wed, Jan 24, 2018 at 11:38:02AM -0800, Dmitry Torokhov wrote:
> mms14 is a touchscreen and thus a direct input device; let's mark it
> as such. This also allows us to drop some initialization code as
> input_init_mt_slots() will do that for us.
> 
> Also add error handling for input_mt_init_slots().
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Reviewed-by: Andi Shyti <andi.shyti@samsung.com>
Tested-by: Andi Shyti <andi.shyti@samsung.com>

Thanks,
Andi

^ permalink raw reply

* Re: [PATCH 3/4] Input: mms114 - drop platform data and use generic APIs
From: Andi Shyti @ 2018-01-26  5:06 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Simon Shields, Rob Herring, linux-input, linux-kernel
In-Reply-To: <20180124193804.155654-4-dmitry.torokhov@gmail.com>

Hi Simon and Dmitry,

On Wed, Jan 24, 2018 at 11:38:03AM -0800, Dmitry Torokhov wrote:
> From: Simon Shields <simon@lineageos.org>
> 
> The MMS114 platform data has no in-tree users, so drop it.
> 
> Switch to using the standard touchscreen properties via
> touchscreen_parse_properties(), and move the old DT parsing code
> to use device_property_*() APIs.
> 
> Finally, use touchscreen_report_pos to report x/y coordinates
> and drop the custom x/y inversion code.
> 
> Signed-off-by: Simon Shields <simon@lineageos.org>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Patchwork-Id: 10162101
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

yes, looks better. I'm happy you dropped the
(mms114_parse_dt(data) < 0).

Reviewed-by: Andi Shyti <andi.shyti@samsung.com>
Tested-by: Andi Shyti <andi.shyti@samsung.com>

Thanks,
Andi

> ---
>  .../bindings/input/touchscreen/mms114.txt          |  29 ++--
>  drivers/input/touchscreen/mms114.c                 | 147 +++++++++------------
>  include/linux/platform_data/mms114.h               |  24 ----
>  3 files changed, 82 insertions(+), 118 deletions(-)
>  delete mode 100644 include/linux/platform_data/mms114.h
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/mms114.txt b/Documentation/devicetree/bindings/input/touchscreen/mms114.txt
> index 89d4c56c56711..8f9f9f38eff4a 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/mms114.txt
> +++ b/Documentation/devicetree/bindings/input/touchscreen/mms114.txt
> @@ -4,14 +4,18 @@ Required properties:
>  - compatible: must be "melfas,mms114"
>  - reg: I2C address of the chip
>  - interrupts: interrupt to which the chip is connected
> -- x-size: horizontal resolution of touchscreen
> -- y-size: vertical resolution of touchscreen
> +- touchscreen-size-x: See [1]
> +- touchscreen-size-y: See [1]
>  
>  Optional properties:
> -- contact-threshold:
> -- moving-threshold:
> -- x-invert: invert X axis
> -- y-invert: invert Y axis
> +- touchscreen-fuzz-x: See [1]
> +- touchscreen-fuzz-y: See [1]
> +- touchscreen-fuzz-pressure: See [1]
> +- touchscreen-inverted-x: See [1]
> +- touchscreen-inverted-y: See [1]
> +- touchscreen-swapped-x-y: See [1]
> +
> +[1]: Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt
>  
>  Example:
>  
> @@ -22,12 +26,13 @@ Example:
>  			compatible = "melfas,mms114";
>  			reg = <0x48>;
>  			interrupts = <39 0>;
> -			x-size = <720>;
> -			y-size = <1280>;
> -			contact-threshold = <10>;
> -			moving-threshold = <10>;
> -			x-invert;
> -			y-invert;
> +			touchscreen-size-x = <720>;
> +			touchscreen-size-y = <1280>;
> +			touchscreen-fuzz-x = <10>;
> +			touchscreen-fuzz-y = <10>;
> +			touchscreen-fuzz-pressure = <10>;
> +			touchscreen-inverted-x;
> +			touchscreen-inverted-y;
>  		};
>  
>  		/* ... */
> diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
> index c3480db5d21ed..69e4288bf8aa3 100644
> --- a/drivers/input/touchscreen/mms114.c
> +++ b/drivers/input/touchscreen/mms114.c
> @@ -12,8 +12,8 @@
>  #include <linux/of.h>
>  #include <linux/i2c.h>
>  #include <linux/input/mt.h>
> +#include <linux/input/touchscreen.h>
>  #include <linux/interrupt.h>
> -#include <linux/platform_data/mms114.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/slab.h>
>  
> @@ -55,7 +55,9 @@ struct mms114_data {
>  	struct input_dev	*input_dev;
>  	struct regulator	*core_reg;
>  	struct regulator	*io_reg;
> -	const struct mms114_platform_data	*pdata;
> +	struct touchscreen_properties props;
> +	unsigned int		contact_threshold;
> +	unsigned int		moving_threshold;
>  
>  	/* Use cache data for mode control register(write only) */
>  	u8			cache_mode_control;
> @@ -143,7 +145,6 @@ static int mms114_write_reg(struct mms114_data *data, unsigned int reg,
>  
>  static void mms114_process_mt(struct mms114_data *data, struct mms114_touch *touch)
>  {
> -	const struct mms114_platform_data *pdata = data->pdata;
>  	struct i2c_client *client = data->client;
>  	struct input_dev *input_dev = data->input_dev;
>  	unsigned int id;
> @@ -163,16 +164,6 @@ static void mms114_process_mt(struct mms114_data *data, struct mms114_touch *tou
>  	id = touch->id - 1;
>  	x = touch->x_lo | touch->x_hi << 8;
>  	y = touch->y_lo | touch->y_hi << 8;
> -	if (x > pdata->x_size || y > pdata->y_size) {
> -		dev_dbg(&client->dev,
> -			"Wrong touch coordinates (%d, %d)\n", x, y);
> -		return;
> -	}
> -
> -	if (pdata->x_invert)
> -		x = pdata->x_size - x;
> -	if (pdata->y_invert)
> -		y = pdata->y_size - y;
>  
>  	dev_dbg(&client->dev,
>  		"id: %d, type: %d, pressed: %d, x: %d, y: %d, width: %d, strength: %d\n",
> @@ -183,9 +174,8 @@ static void mms114_process_mt(struct mms114_data *data, struct mms114_touch *tou
>  	input_mt_report_slot_state(input_dev, MT_TOOL_FINGER, touch->pressed);
>  
>  	if (touch->pressed) {
> +		touchscreen_report_pos(input_dev, &data->props, x, y, true);
>  		input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR, touch->width);
> -		input_report_abs(input_dev, ABS_MT_POSITION_X, x);
> -		input_report_abs(input_dev, ABS_MT_POSITION_Y, y);
>  		input_report_abs(input_dev, ABS_MT_PRESSURE, touch->strength);
>  	}
>  }
> @@ -263,7 +253,7 @@ static int mms114_get_version(struct mms114_data *data)
>  
>  static int mms114_setup_regs(struct mms114_data *data)
>  {
> -	const struct mms114_platform_data *pdata = data->pdata;
> +	const struct touchscreen_properties *props = &data->props;
>  	int val;
>  	int error;
>  
> @@ -275,32 +265,32 @@ static int mms114_setup_regs(struct mms114_data *data)
>  	if (error < 0)
>  		return error;
>  
> -	val = (pdata->x_size >> 8) & 0xf;
> -	val |= ((pdata->y_size >> 8) & 0xf) << 4;
> +	val = (props->max_x >> 8) & 0xf;
> +	val |= ((props->max_y >> 8) & 0xf) << 4;
>  	error = mms114_write_reg(data, MMS114_XY_RESOLUTION_H, val);
>  	if (error < 0)
>  		return error;
>  
> -	val = pdata->x_size & 0xff;
> +	val = props->max_x & 0xff;
>  	error = mms114_write_reg(data, MMS114_X_RESOLUTION, val);
>  	if (error < 0)
>  		return error;
>  
> -	val = pdata->y_size & 0xff;
> +	val = props->max_x & 0xff;
>  	error = mms114_write_reg(data, MMS114_Y_RESOLUTION, val);
>  	if (error < 0)
>  		return error;
>  
> -	if (pdata->contact_threshold) {
> +	if (data->contact_threshold) {
>  		error = mms114_write_reg(data, MMS114_CONTACT_THRESHOLD,
> -				pdata->contact_threshold);
> +				data->contact_threshold);
>  		if (error < 0)
>  			return error;
>  	}
>  
> -	if (pdata->moving_threshold) {
> +	if (data->moving_threshold) {
>  		error = mms114_write_reg(data, MMS114_MOVING_THRESHOLD,
> -				pdata->moving_threshold);
> +				data->moving_threshold);
>  		if (error < 0)
>  			return error;
>  	}
> @@ -335,9 +325,6 @@ static int mms114_start(struct mms114_data *data)
>  		return error;
>  	}
>  
> -	if (data->pdata->cfg_pin)
> -		data->pdata->cfg_pin(true);
> -
>  	enable_irq(client->irq);
>  
>  	return 0;
> @@ -350,9 +337,6 @@ static void mms114_stop(struct mms114_data *data)
>  
>  	disable_irq(client->irq);
>  
> -	if (data->pdata->cfg_pin)
> -		data->pdata->cfg_pin(false);
> -
>  	error = regulator_disable(data->io_reg);
>  	if (error)
>  		dev_warn(&client->dev, "Failed to disable vdd: %d\n", error);
> @@ -376,67 +360,43 @@ static void mms114_input_close(struct input_dev *dev)
>  	mms114_stop(data);
>  }
>  
> -#ifdef CONFIG_OF
> -static struct mms114_platform_data *mms114_parse_dt(struct device *dev)
> +static int mms114_parse_legacy_bindings(struct mms114_data *data)
>  {
> -	struct mms114_platform_data *pdata;
> -	struct device_node *np = dev->of_node;
> -
> -	if (!np)
> -		return NULL;
> +	struct device *dev = &data->client->dev;
> +	struct touchscreen_properties *props = &data->props;
>  
> -	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> -	if (!pdata) {
> -		dev_err(dev, "failed to allocate platform data\n");
> -		return NULL;
> +	if (device_property_read_u32(dev, "x-size", &props->max_x)) {
> +		dev_dbg(dev, "failed to get legacy x-size property\n");
> +		return -EINVAL;
>  	}
>  
> -	if (of_property_read_u32(np, "x-size", &pdata->x_size)) {
> -		dev_err(dev, "failed to get x-size property\n");
> -		return NULL;
> +	if (device_property_read_u32(dev, "y-size", &props->max_y)) {
> +		dev_dbg(dev, "failed to get legacy y-size property\n");
> +		return -EINVAL;
>  	}
>  
> -	if (of_property_read_u32(np, "y-size", &pdata->y_size)) {
> -		dev_err(dev, "failed to get y-size property\n");
> -		return NULL;
> -	}
> +	device_property_read_u32(dev, "contact-threshold",
> +				&data->contact_threshold);
> +	device_property_read_u32(dev, "moving-threshold",
> +				&data->moving_threshold);
>  
> -	of_property_read_u32(np, "contact-threshold",
> -				&pdata->contact_threshold);
> -	of_property_read_u32(np, "moving-threshold",
> -				&pdata->moving_threshold);
> +	if (device_property_read_bool(dev, "x-invert"))
> +		props->invert_x = true;
> +	if (device_property_read_bool(dev, "y-invert"))
> +		props->invert_y = true;
>  
> -	if (of_find_property(np, "x-invert", NULL))
> -		pdata->x_invert = true;
> -	if (of_find_property(np, "y-invert", NULL))
> -		pdata->y_invert = true;
> +	props->swap_x_y = false;
>  
> -	return pdata;
> -}
> -#else
> -static inline struct mms114_platform_data *mms114_parse_dt(struct device *dev)
> -{
> -	return NULL;
> +	return 0;
>  }
> -#endif
>  
>  static int mms114_probe(struct i2c_client *client,
>  				  const struct i2c_device_id *id)
>  {
> -	const struct mms114_platform_data *pdata;
>  	struct mms114_data *data;
>  	struct input_dev *input_dev;
>  	int error;
>  
> -	pdata = dev_get_platdata(&client->dev);
> -	if (!pdata)
> -		pdata = mms114_parse_dt(&client->dev);
> -
> -	if (!pdata) {
> -		dev_err(&client->dev, "Need platform data\n");
> -		return -EINVAL;
> -	}
> -
>  	if (!i2c_check_functionality(client->adapter,
>  				I2C_FUNC_PROTOCOL_MANGLING)) {
>  		dev_err(&client->dev,
> @@ -454,7 +414,38 @@ static int mms114_probe(struct i2c_client *client,
>  
>  	data->client = client;
>  	data->input_dev = input_dev;
> -	data->pdata = pdata;
> +
> +	input_set_capability(input_dev, EV_ABS, ABS_MT_POSITION_X);
> +	input_set_capability(input_dev, EV_ABS, ABS_MT_POSITION_Y);
> +	input_set_abs_params(input_dev, ABS_MT_PRESSURE, 0, 255, 0, 0);
> +	input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR,
> +			     0, MMS114_MAX_AREA, 0, 0);
> +
> +	touchscreen_parse_properties(input_dev, true, &data->props);
> +	if (!data->props.max_x || !data->props.max_y) {
> +		dev_dbg(&client->dev,
> +			"missing X/Y size properties, trying legacy bindings\n");
> +		error = mms114_parse_legacy_bindings(data);
> +		if (error)
> +			return error;
> +
> +		input_set_abs_params(input_dev, ABS_MT_POSITION_X,
> +				     0, data->props.max_x, 0, 0);
> +		input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
> +				     0, data->props.max_y, 0, 0);
> +	}
> +
> +	/*
> +	 * The firmware handles movement and pressure fuzz, so
> +	 * don't duplicate that in software.
> +	 */
> +	data->moving_threshold = input_abs_get_fuzz(input_dev,
> +						    ABS_MT_POSITION_X);
> +	data->contact_threshold = input_abs_get_fuzz(input_dev,
> +						     ABS_MT_PRESSURE);
> +	input_abs_set_fuzz(input_dev, ABS_MT_POSITION_X, 0);
> +	input_abs_set_fuzz(input_dev, ABS_MT_POSITION_Y, 0);
> +	input_abs_set_fuzz(input_dev, ABS_MT_PRESSURE, 0);
>  
>  	input_dev->name = "MELFAS MMS114 Touchscreen";
>  	input_dev->id.bustype = BUS_I2C;
> @@ -462,14 +453,6 @@ static int mms114_probe(struct i2c_client *client,
>  	input_dev->open = mms114_input_open;
>  	input_dev->close = mms114_input_close;
>  
> -	input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR,
> -			     0, MMS114_MAX_AREA, 0, 0);
> -	input_set_abs_params(input_dev, ABS_MT_POSITION_X,
> -			     0, data->pdata->x_size, 0, 0);
> -	input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
> -			     0, data->pdata->y_size, 0, 0);
> -	input_set_abs_params(input_dev, ABS_MT_PRESSURE, 0, 255, 0, 0);
> -
>  	error = input_mt_init_slots(input_dev, MMS114_MAX_TOUCH,
>  				    INPUT_MT_DIRECT);
>  	if (error)
> diff --git a/include/linux/platform_data/mms114.h b/include/linux/platform_data/mms114.h
> deleted file mode 100644
> index 5722ebfb27382..0000000000000
> --- a/include/linux/platform_data/mms114.h
> +++ /dev/null
> @@ -1,24 +0,0 @@
> -/*
> - * 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 Foundationr
> - */
> -
> -#ifndef __LINUX_MMS114_H
> -#define __LINUX_MMS114_H
> -
> -struct mms114_platform_data {
> -	unsigned int x_size;
> -	unsigned int y_size;
> -	unsigned int contact_threshold;
> -	unsigned int moving_threshold;
> -	bool x_invert;
> -	bool y_invert;
> -
> -	void (*cfg_pin)(bool);
> -};
> -
> -#endif	/* __LINUX_MMS114_H */
> -- 
> 2.16.0.rc1.238.g530d649a79-goog
> 
> 

^ permalink raw reply

* Re: [PATCH 4/4 v2] Input: mms114 - add support for mms152
From: Andi Shyti @ 2018-01-26  5:14 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Marcus Folkesson, Simon Shields, Rob Herring, linux-input,
	linux-kernel
In-Reply-To: <20180124213201.vemutoltuz43hiwr@dtor-ws>

Hi Simon and Dmitry,

On Wed, Jan 24, 2018 at 01:32:01PM -0800, Dmitry Torokhov wrote:
> From: Simon Shields <simon@lineageos.org>
> 
> MMS152 has no configuration registers, but the packet format used in
> interrupts is identical to mms114.
> 
> Signed-off-by: Simon Shields <simon@lineageos.org>
> Patchwork-Id: 10125841
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

also here

Reviewed-by: Andi Shyti <andi.shyti@samsung.com>
Tested-by: Andi Shyti <andi.shyti@samsung.com>

one small nitpick:

> @@ -239,14 +249,33 @@ static int mms114_get_version(struct mms114_data *data)
>  {
>  	struct device *dev = &data->client->dev;
>  	u8 buf[6];
> +	int group;
>  	int error;

do we really need to define a new 'group' variable?

Andi

> -	error = __mms114_read_reg(data, MMS114_TSP_REV, 6, buf);
> -	if (error < 0)
> -		return error;
> +	switch (data->type) {
> +	case TYPE_MMS152:
> +		error = __mms114_read_reg(data, MMS152_FW_REV, 3, buf);
> +		if (error)
> +			return error;
> +
> +		group = i2c_smbus_read_byte_data(data->client,
> +						  MMS152_COMPAT_GROUP);
> +		if (group < 0)
> +			return group;
> +
> +		dev_info(dev, "TSP FW Rev: bootloader 0x%x / core 0x%x / config 0x%x, Compat group: %c\n",
> +			 buf[0], buf[1], buf[2], group);
> +		break;
> +
> +	case TYPE_MMS114:
> +		error = __mms114_read_reg(data, MMS114_TSP_REV, 6, buf);
> +		if (error)
> +			return error;
>  
> -	dev_info(dev, "TSP Rev: 0x%x, HW Rev: 0x%x, Firmware Ver: 0x%x\n",
> -		 buf[0], buf[1], buf[3]);
> +		dev_info(dev, "TSP Rev: 0x%x, HW Rev: 0x%x, Firmware Ver: 0x%x\n",
> +			 buf[0], buf[1], buf[3]);
> +		break;
> +	}
>  
>  	return 0;
>  }

^ permalink raw reply

* Darlehen Geld für Einzelpersonen und Fachleute in weniger als 72 Stunden
From: Peter Schuster @ 2018-01-26  7:22 UTC (permalink / raw)


Hallo,

Sind Sie in einer schwierigen Situation, für die Sie sich für ein
Darlehen suchen? Benötigen Sie eine Finanzierung, um eine Schuld zu
begleichen oder eine Aktivität zu finanzieren? Haben Sie einen
Verbraucherkredit, eine Hypothek, einen persönlichen Kredit, eine
Hypothek, Investition Darlehen, Schuldenkonsolidierung Darlehen oder
andere braucht?

Ich bin ein einzelner Investor. I zur Verfügung stellen die Kredit
kurz-, mittel- und langfristige. Ihr Finanzierungsbedingungen sind
sehr einfach und meine Zinssatz beträgt 3% pro Jahr.

Für alle Anfragen, bleibe ich zur Verfügung, um Ihre Fragen zu beantworten.

Danke, dass Sie mir per E-Mail an Sie von  :   klaus.peterschuster@outlook.de

Mit freundlichen Grüßen.

Peter Schuster

Financial Bank
https://firstfinancialsa.com/de

^ permalink raw reply

* Re: [PATCH v3 3/3] Input: goodix - use generic touchscreen_properties
From: Bastien Nocera @ 2018-01-26  9:44 UTC (permalink / raw)
  To: Marcin Niestroj, Dmitry Torokhov; +Cc: Antonio Ospite, linux-input
In-Reply-To: <20180125190829.8968-4-m.niestroj@grinn-global.com>

On Thu, 2018-01-25 at 20:08 +0100, Marcin Niestroj wrote:
> 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.
> 
> Developed and tested on custom DT-based device with gt1151 touch
> panel.
> 
> Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>

Reviewed-by: Bastien Nocera <hadess@hadess.net>

> ---
> Changes v2 -> v3 (suggested by Bastien):
>  - fix commit description
> 
> Changes v1 -> v2:
>  - rebased on patches 1 and 2 in series
>  - added description of test board in changelog (suggested by
> Bastien)
> 
>  drivers/input/touchscreen/goodix.c | 93 ++++++++++++++------------
> ------------
>  1 file changed, 33 insertions(+), 60 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/goodix.c
> b/drivers/input/touchscreen/goodix.c
> index dc832890f6d3..f82101cd9c04 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;
> @@ -295,24 +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->swapped_x_y) {
> -		if (ts->inverted_x)
> -			input_x = ts->abs_x_max - input_x;
> -		if (ts->inverted_y)
> -			input_y = ts->abs_y_max - input_y;
> -	} else {
> -		if (ts->inverted_x)
> -			input_x = ts->abs_y_max - input_x;
> -		if (ts->inverted_y)
> -			input_y = ts->abs_x_max - input_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);
>  }
> @@ -585,6 +568,7 @@ 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,
> @@ -593,37 +577,34 @@ static void goodix_read_config(struct
> goodix_ts_data *ts)
>  		dev_warn(&ts->client->dev,
>  			 "Error reading config (%d), using
> defaults\n",
>  			 error);
> -		ts->abs_x_max = GOODIX_MAX_WIDTH - 1;
> -		ts->abs_y_max = GOODIX_MAX_HEIGHT - 1;
> -		if (ts->swapped_x_y)
> -			swap(ts->abs_x_max, ts->abs_y_max);
> +		x_max = GOODIX_MAX_WIDTH;
> +		y_max = GOODIX_MAX_HEIGHT;
>  		ts->int_trigger_type = GOODIX_INT_TRIGGER;
>  		ts->max_touch_num = GOODIX_MAX_CONTACTS;
> -		return;
> +		goto input_set_params;
>  	}
>  
> -	ts->abs_x_max = get_unaligned_le16(&config[RESOLUTION_LOC])
> - 1;
> -	ts->abs_y_max = get_unaligned_le16(&config[RESOLUTION_LOC +
> 2]) - 1;
> -	if (ts->swapped_x_y)
> -		swap(ts->abs_x_max, ts->abs_y_max);
> +	x_max = get_unaligned_le16(&config[RESOLUTION_LOC]);
> +	y_max = get_unaligned_le16(&config[RESOLUTION_LOC + 2]);
>  	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)
> {
> +	if (!x_max || !y_max || !ts->max_touch_num) {
>  		dev_err(&ts->client->dev,
>  			"Invalid config, using defaults\n");
> -		ts->abs_x_max = GOODIX_MAX_WIDTH - 1;
> -		ts->abs_y_max = GOODIX_MAX_HEIGHT - 1;
> -		if (ts->swapped_x_y)
> -			swap(ts->abs_x_max, ts->abs_y_max);
> +		x_max = GOODIX_MAX_WIDTH;
> +		y_max = GOODIX_MAX_HEIGHT;
>  		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");
> -	}
> +input_set_params:
> +	input_set_abs_params(ts->input_dev, ABS_MT_POSITION_X,
> +			0, x_max - 1, 0, 0);
> +	input_set_abs_params(ts->input_dev, ABS_MT_POSITION_Y,
> +			0, y_max - 1, 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);
>  }
>  
>  /**
> @@ -698,16 +679,6 @@ static int goodix_request_input_dev(struct
> goodix_ts_data *ts)
>  		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;
> @@ -742,19 +713,21 @@ static int goodix_configure_dev(struct
> goodix_ts_data *ts)
>  {
>  	int error;
>  
> -	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");
> -
> -	goodix_read_config(ts);
> -
>  	error = goodix_request_input_dev(ts);
>  	if (error)
>  		return error;
>  
> +	goodix_read_config(ts);
> +
> +	touchscreen_parse_properties(ts->input_dev, true, &ts-
> >prop);
> +
> +	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");
> +	}
> +
>  	ts->irq_flags = goodix_irq_flags[ts->int_trigger_type] |
> IRQF_ONESHOT;
>  	error = goodix_request_irq(ts);
>  	if (error) {

^ permalink raw reply

* Re: [PATCH v3 2/3] Input: goodix - fix simultaneous axes inversion and swap
From: Bastien Nocera @ 2018-01-26  9:44 UTC (permalink / raw)
  To: Marcin Niestroj, Dmitry Torokhov; +Cc: Antonio Ospite, linux-input
In-Reply-To: <20180125190829.8968-3-m.niestroj@grinn-global.com>

On Thu, 2018-01-25 at 20:08 +0100, Marcin Niestroj wrote:
> goodix_ts_data structure contains abs_x_max and abs_y_max members,
> which contain already swapped maximum ranges. That causes reporting
> touch events with invalid position (out of range values).
> 
> Problem occurs for example when ts->inverted_x and ts->swapped_x_y
> are
> true, but ts->inverted_y is 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).
> 
> Take into account that abs_x_max and abs_y_max are already swapped
> in goodix_ts_report_touch(), so position for inverted axes will be
> calculated correctly.
> 
> Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>

Reviewed-by: Bastien Nocera <hadess@hadess.net>

> Fixes: ad48cf5e9597 ("Input: goodix - add axis swapping and axis
> inversion support")
> ---
> Changes v2 -> v3 (suggested by Bastien):
>  - add explanation about when the problem occurs
> 
> Changes v1 -> v2 (suggested by Bastien):
>  - patch splitted off from patch 3
> 
>  drivers/input/touchscreen/goodix.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/goodix.c
> b/drivers/input/touchscreen/goodix.c
> index 7896097ca69b..dc832890f6d3 100644
> --- a/drivers/input/touchscreen/goodix.c
> +++ b/drivers/input/touchscreen/goodix.c
> @@ -296,12 +296,18 @@ static void goodix_ts_report_touch(struct
> goodix_ts_data *ts, u8 *coor_data)
>  	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)
> +	if (!ts->swapped_x_y) {
> +		if (ts->inverted_x)
> +			input_x = ts->abs_x_max - input_x;
> +		if (ts->inverted_y)
> +			input_y = ts->abs_y_max - input_y;
> +	} else {
> +		if (ts->inverted_x)
> +			input_x = ts->abs_y_max - input_x;
> +		if (ts->inverted_y)
> +			input_y = ts->abs_x_max - input_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);

^ permalink raw reply

* Re: [PATCH v3 1/3] Input: goodix - fix reported range
From: Bastien Nocera @ 2018-01-26  9:44 UTC (permalink / raw)
  To: Marcin Niestroj, Dmitry Torokhov; +Cc: Antonio Ospite, linux-input
In-Reply-To: <20180125190829.8968-2-m.niestroj@grinn-global.com>

On Thu, 2018-01-25 at 20:08 +0100, Marcin Niestroj wrote:
> Touchscreen coordinates are 0-indexed, so report touchscreen range
> as (0:size-1).
> 
> Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
> Fixes: ca96ea86eed4 ("Input: add driver for the Goodix touchpanel")

Reviewed-by: Bastien Nocera <hadess@hadess.net>

> ---
> Changes v2 -> v3: fix commit description (suggested by Bastien)
> 
> Changes v1 -> v2: patch splitted off from patch 3 (suggested by
> Bastien)
> 
>  drivers/input/touchscreen/goodix.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/goodix.c
> b/drivers/input/touchscreen/goodix.c
> index 69d0b8cbc71f..7896097ca69b 100644
> --- a/drivers/input/touchscreen/goodix.c
> +++ b/drivers/input/touchscreen/goodix.c
> @@ -587,8 +587,8 @@ static void goodix_read_config(struct
> goodix_ts_data *ts)
>  		dev_warn(&ts->client->dev,
>  			 "Error reading config (%d), using
> defaults\n",
>  			 error);
> -		ts->abs_x_max = GOODIX_MAX_WIDTH;
> -		ts->abs_y_max = GOODIX_MAX_HEIGHT;
> +		ts->abs_x_max = GOODIX_MAX_WIDTH - 1;
> +		ts->abs_y_max = GOODIX_MAX_HEIGHT - 1;
>  		if (ts->swapped_x_y)
>  			swap(ts->abs_x_max, ts->abs_y_max);
>  		ts->int_trigger_type = GOODIX_INT_TRIGGER;
> @@ -596,8 +596,8 @@ static void goodix_read_config(struct
> goodix_ts_data *ts)
>  		return;
>  	}
>  
> -	ts->abs_x_max = get_unaligned_le16(&config[RESOLUTION_LOC]);
> -	ts->abs_y_max = get_unaligned_le16(&config[RESOLUTION_LOC +
> 2]);
> +	ts->abs_x_max = get_unaligned_le16(&config[RESOLUTION_LOC])
> - 1;
> +	ts->abs_y_max = get_unaligned_le16(&config[RESOLUTION_LOC +
> 2]) - 1;
>  	if (ts->swapped_x_y)
>  		swap(ts->abs_x_max, ts->abs_y_max);
>  	ts->int_trigger_type = config[TRIGGER_LOC] & 0x03;
> @@ -605,8 +605,8 @@ static void goodix_read_config(struct
> goodix_ts_data *ts)
>  	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;
> +		ts->abs_x_max = GOODIX_MAX_WIDTH - 1;
> +		ts->abs_y_max = GOODIX_MAX_HEIGHT - 1;
>  		if (ts->swapped_x_y)
>  			swap(ts->abs_x_max, ts->abs_y_max);
>  		ts->max_touch_num = GOODIX_MAX_CONTACTS;

^ permalink raw reply

* Re: [PATCH] HID: asus: only support backlight when it's not driven by WMI
From: Andy Shevchenko @ 2018-01-26  9:47 UTC (permalink / raw)
  To: Daniel Drake
  Cc: Mustafa Kuscu, Jiri Kosina, Benjamin Tissoires, linux-input,
	Linux Upstreaming Team, Carlo Caione, Corentin Chary,
	acpi4asus-user, Platform Driver
In-Reply-To: <CAD8Lp476c1stJOScWBuX2ZsNrORKVYnUJL1UnwCp6hQOzXASvw@mail.gmail.com>

On Mon, Jan 15, 2018 at 10:33 PM, Daniel Drake <drake@endlessm.com> wrote:
> On Fri, Jan 12, 2018 at 1:33 PM, Mustafa Kuscu <mustafakuscu@gmail.com> wrote:

> Andy, still waiting for any more feedback on the issue detailed in
> this thread once you have a chance.

As I said before I don't like code duplication and even more I don't
like functionality duplication.
So, check what can we re-use from asus-wmi.c and adopt it accordingly.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH v2 1/2] Input: edt-ft5x06 - Add support for regulator
From: Mylene Josserand @ 2018-01-26 10:11 UTC (permalink / raw)
  To: Dmitry Torokhov
  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, simon.budig-t93Ne7XHvje5bSeCtf/tX7NAH6kLmebB
In-Reply-To: <20180123175829.2axe3pdntrueqlnw@dtor-ws>

Hello Dmitry,

Le Tue, 23 Jan 2018 09:58:29 -0800,
Dmitry Torokhov <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> a écrit :

> On Tue, Jan 23, 2018 at 10:10:55AM +0100, Mylene Josserand wrote:
> > Hello Dimitry,
> > 
> > Thank you for the review!
> > 
> > Le Mon, 22 Jan 2018 09:42:08 -0800,
> > Dmitry Torokhov <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> a écrit :
> >   
> > > 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.  
> > 
> > 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?  
> 
> If you take a look at edt_ft5x06_work_mode() at the end ther eis code to
> restore parameters. You want to factor it out and apply to the device
> when resuming. You probably also want to see what mode (factory/normal)
> the device was before suspend and restore the previous mode on resume.

Okay, thank you for your help.

About the factory/normal mode to be sure that I understand correctly,
in the resume, I should use something like "edt_ft5x06_debugfs_mode_set"
to check the "tsdata->factory_mode" and call "edt_ft5x06_factory_mode"
or "edt_ft5x06_work_mode", is not it?

As the current suspend/resume code is only enabling/disabling an irq, I
would like to understand exactly the modifications you would like.

> 
> >   
> > >   
> > > > +       }
> > > > +
> > > >         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?  
> 
> See if Simon Budig <simon.budig-t93Ne7XHvje5bSeCtf/tX7NAH6kLmebB@public.gmane.org> can help us here.

Thank you, I contacted Simon (added him in CC) and unfortunately, he
doesn't have a setup with working suspend/resume so he can't help me
on that.
My patches are ready (except for factory/normal mode, see above) but as
I can't test suspend/resume, I prefer not to send them without testing
it..

> 
> > 
> > If I send a V3 in next few days, do you think you will have time to
> > merge it for v4.16?  
> 
> It all depends on the patch shape...

Sure I understand.

Best regards,

-- 
Mylène Josserand, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
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

* [PATCH 0/2] Input: omap4-keypad: Adjustments for omap4_keypad_probe()
From: SF Markus Elfring @ 2018-01-26 13:48 UTC (permalink / raw)
  To: linux-input, Christophe Jaillet, Dmitry Torokhov; +Cc: LKML, kernel-janitors

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 26 Jan 2018 14:45:54 +0100

Two update suggestions were taken into account
from static source code analysis.

Markus Elfring (2):
  Delete two error messages for a failed memory allocation
  Improve a size determination

 drivers/input/keyboard/omap4-keypad.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

-- 
2.16.1


^ permalink raw reply

* [PATCH 1/2] Input: omap4-keypad: Delete two error messages for a failed memory allocation in omap4_keypad_probe()
From: SF Markus Elfring @ 2018-01-26 13:49 UTC (permalink / raw)
  To: linux-input, Christophe Jaillet, Dmitry Torokhov; +Cc: LKML, kernel-janitors
In-Reply-To: <3df8e062-2a39-05c9-bf49-c85fd636abff@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 26 Jan 2018 14:24:35 +0100

Omit extra messages 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/input/keyboard/omap4-keypad.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/keyboard/omap4-keypad.c
index 940d38b08e6b..6fe554e4026e 100644
--- a/drivers/input/keyboard/omap4-keypad.c
+++ b/drivers/input/keyboard/omap4-keypad.c
@@ -257,10 +257,8 @@ static int omap4_keypad_probe(struct platform_device *pdev)
 	}
 
 	keypad_data = kzalloc(sizeof(struct omap4_keypad), GFP_KERNEL);
-	if (!keypad_data) {
-		dev_err(&pdev->dev, "keypad_data memory allocation failed\n");
+	if (!keypad_data)
 		return -ENOMEM;
-	}
 
 	keypad_data->irq = irq;
 
@@ -340,7 +338,6 @@ static int omap4_keypad_probe(struct platform_device *pdev)
 	keypad_data->keymap = kzalloc(max_keys * sizeof(keypad_data->keymap[0]),
 				      GFP_KERNEL);
 	if (!keypad_data->keymap) {
-		dev_err(&pdev->dev, "Not enough memory for keymap\n");
 		error = -ENOMEM;
 		goto err_free_input;
 	}
-- 
2.16.1

^ permalink raw reply related

* [PATCH 2/2] Input: omap4-keypad: Improve a size determination in omap4_keypad_probe()
From: SF Markus Elfring @ 2018-01-26 13:50 UTC (permalink / raw)
  To: linux-input, Christophe Jaillet, Dmitry Torokhov; +Cc: LKML, kernel-janitors
In-Reply-To: <3df8e062-2a39-05c9-bf49-c85fd636abff@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 26 Jan 2018 14:28:07 +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>
---
 drivers/input/keyboard/omap4-keypad.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/keyboard/omap4-keypad.c
index 6fe554e4026e..4b5bd0c611d6 100644
--- a/drivers/input/keyboard/omap4-keypad.c
+++ b/drivers/input/keyboard/omap4-keypad.c
@@ -256,7 +256,7 @@ static int omap4_keypad_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	keypad_data = kzalloc(sizeof(struct omap4_keypad), GFP_KERNEL);
+	keypad_data = kzalloc(sizeof(*keypad_data), GFP_KERNEL);
 	if (!keypad_data)
 		return -ENOMEM;
 
-- 
2.16.1

^ permalink raw reply related

* Re: [PATCH v3 3/3] Input: goodix - use generic touchscreen_properties
From: Dmitry Torokhov @ 2018-01-26 19:19 UTC (permalink / raw)
  To: Marcin Niestroj; +Cc: Bastien Nocera, Antonio Ospite, linux-input
In-Reply-To: <20180125190829.8968-4-m.niestroj@grinn-global.com>

Hi Marcin,

On Thu, Jan 25, 2018 at 08:08:29PM +0100, Marcin Niestroj wrote:
> 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.
> 
> Developed and tested on custom DT-based device with gt1151 touch
> panel.
> 
> Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
> ---
> Changes v2 -> v3 (suggested by Bastien):
>  - fix commit description
> 
> Changes v1 -> v2:
>  - rebased on patches 1 and 2 in series
>  - added description of test board in changelog (suggested by Bastien)
> 
>  drivers/input/touchscreen/goodix.c | 93 ++++++++++++++------------------------
>  1 file changed, 33 insertions(+), 60 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
> index dc832890f6d3..f82101cd9c04 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;
> @@ -295,24 +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->swapped_x_y) {
> -		if (ts->inverted_x)
> -			input_x = ts->abs_x_max - input_x;
> -		if (ts->inverted_y)
> -			input_y = ts->abs_y_max - input_y;
> -	} else {
> -		if (ts->inverted_x)
> -			input_x = ts->abs_y_max - input_x;
> -		if (ts->inverted_y)
> -			input_y = ts->abs_x_max - input_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);
>  }
> @@ -585,6 +568,7 @@ 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,
> @@ -593,37 +577,34 @@ static void goodix_read_config(struct goodix_ts_data *ts)
>  		dev_warn(&ts->client->dev,
>  			 "Error reading config (%d), using defaults\n",
>  			 error);
> -		ts->abs_x_max = GOODIX_MAX_WIDTH - 1;
> -		ts->abs_y_max = GOODIX_MAX_HEIGHT - 1;
> -		if (ts->swapped_x_y)
> -			swap(ts->abs_x_max, ts->abs_y_max);
> +		x_max = GOODIX_MAX_WIDTH;
> +		y_max = GOODIX_MAX_HEIGHT;
>  		ts->int_trigger_type = GOODIX_INT_TRIGGER;
>  		ts->max_touch_num = GOODIX_MAX_CONTACTS;
> -		return;
> +		goto input_set_params;
>  	}
>  
> -	ts->abs_x_max = get_unaligned_le16(&config[RESOLUTION_LOC]) - 1;
> -	ts->abs_y_max = get_unaligned_le16(&config[RESOLUTION_LOC + 2]) - 1;
> -	if (ts->swapped_x_y)
> -		swap(ts->abs_x_max, ts->abs_y_max);
> +	x_max = get_unaligned_le16(&config[RESOLUTION_LOC]);
> +	y_max = get_unaligned_le16(&config[RESOLUTION_LOC + 2]);
>  	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) {
> +	if (!x_max || !y_max || !ts->max_touch_num) {
>  		dev_err(&ts->client->dev,
>  			"Invalid config, using defaults\n");
> -		ts->abs_x_max = GOODIX_MAX_WIDTH - 1;
> -		ts->abs_y_max = GOODIX_MAX_HEIGHT - 1;
> -		if (ts->swapped_x_y)
> -			swap(ts->abs_x_max, ts->abs_y_max);
> +		x_max = GOODIX_MAX_WIDTH;
> +		y_max = GOODIX_MAX_HEIGHT;
>  		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");
> -	}
> +input_set_params:
> +	input_set_abs_params(ts->input_dev, ABS_MT_POSITION_X,
> +			0, x_max - 1, 0, 0);
> +	input_set_abs_params(ts->input_dev, ABS_MT_POSITION_Y,
> +			0, y_max - 1, 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);
>  }
>  
>  /**
> @@ -698,16 +679,6 @@ static int goodix_request_input_dev(struct goodix_ts_data *ts)
>  		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;
> @@ -742,19 +713,21 @@ static int goodix_configure_dev(struct goodix_ts_data *ts)
>  {
>  	int error;
>  
> -	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");
> -
> -	goodix_read_config(ts);
> -
>  	error = goodix_request_input_dev(ts);
>  	if (error)
>  		return error;
>  
> +	goodix_read_config(ts);
> +
> +	touchscreen_parse_properties(ts->input_dev, true, &ts->prop);

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).

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);

^ permalink raw reply related

* Re: [PATCH 4/4 v2] Input: mms114 - add support for mms152
From: Dmitry Torokhov @ 2018-01-26 19:25 UTC (permalink / raw)
  To: Andi Shyti
  Cc: Marcus Folkesson, Simon Shields, Rob Herring, linux-input,
	linux-kernel
In-Reply-To: <20180126051438.GS7575@gangnam.samsung>

On Fri, Jan 26, 2018 at 02:14:38PM +0900, Andi Shyti wrote:
> Hi Simon and Dmitry,
> 
> On Wed, Jan 24, 2018 at 01:32:01PM -0800, Dmitry Torokhov wrote:
> > From: Simon Shields <simon@lineageos.org>
> > 
> > MMS152 has no configuration registers, but the packet format used in
> > interrupts is identical to mms114.
> > 
> > Signed-off-by: Simon Shields <simon@lineageos.org>
> > Patchwork-Id: 10125841
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> 
> also here
> 
> Reviewed-by: Andi Shyti <andi.shyti@samsung.com>
> Tested-by: Andi Shyti <andi.shyti@samsung.com>
> 
> one small nitpick:
> 
> > @@ -239,14 +249,33 @@ static int mms114_get_version(struct mms114_data *data)
> >  {
> >  	struct device *dev = &data->client->dev;
> >  	u8 buf[6];
> > +	int group;
> >  	int error;
> 
> do we really need to define a new 'group' variable?

I like it, that's why I added it ;)

As Marcus mentioned, we cant use 'buf' to store the value as buf is
unsigned and i2c_smbus_read_byte_data() may return negative error code.
And I do not want to use 'error' variable because logically we are
trying to get group value, not error. Hence a dedicated variable. It's
gonna be optimized away anyway.

Thanks.

> 
> Andi
> 
> > -	error = __mms114_read_reg(data, MMS114_TSP_REV, 6, buf);
> > -	if (error < 0)
> > -		return error;
> > +	switch (data->type) {
> > +	case TYPE_MMS152:
> > +		error = __mms114_read_reg(data, MMS152_FW_REV, 3, buf);
> > +		if (error)
> > +			return error;
> > +
> > +		group = i2c_smbus_read_byte_data(data->client,
> > +						  MMS152_COMPAT_GROUP);
> > +		if (group < 0)
> > +			return group;
> > +
> > +		dev_info(dev, "TSP FW Rev: bootloader 0x%x / core 0x%x / config 0x%x, Compat group: %c\n",
> > +			 buf[0], buf[1], buf[2], group);
> > +		break;
> > +
> > +	case TYPE_MMS114:
> > +		error = __mms114_read_reg(data, MMS114_TSP_REV, 6, buf);
> > +		if (error)
> > +			return error;
> >  
> > -	dev_info(dev, "TSP Rev: 0x%x, HW Rev: 0x%x, Firmware Ver: 0x%x\n",
> > -		 buf[0], buf[1], buf[3]);
> > +		dev_info(dev, "TSP Rev: 0x%x, HW Rev: 0x%x, Firmware Ver: 0x%x\n",
> > +			 buf[0], buf[1], buf[3]);
> > +		break;
> > +	}
> >  
> >  	return 0;
> >  }

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v3 3/3] Input: goodix - use generic touchscreen_properties
From: Bastien Nocera @ 2018-01-26 23:48 UTC (permalink / raw)
  To: Dmitry Torokhov, Marcin Niestroj; +Cc: Antonio Ospite, linux-input
In-Reply-To: <20180126191945.q3sx2pcs7rvpil33@dtor-ws>

On Fri, 2018-01-26 at 11:19 -0800, Dmitry Torokhov wrote:
> 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).

It was easier to read, and backport. Patch 1 and 2 at least are easily
backportable, and "easy" fixes on top of the existing code. Breakages
would be easier to spot with the 3 separate fixes, and that's how I'd
prefer it if it's not too much bother.

^ permalink raw reply

* Re: [PATCH v3 3/3] Input: goodix - use generic touchscreen_properties
From: Dmitry Torokhov @ 2018-01-26 23:57 UTC (permalink / raw)
  To: Bastien Nocera
  Cc: Marcin Niestroj, Antonio Ospite, linux-input@vger.kernel.org
In-Reply-To: <1517010494.2592.27.camel@hadess.net>

On Fri, Jan 26, 2018 at 3:48 PM, Bastien Nocera <hadess@hadess.net> wrote:
> On Fri, 2018-01-26 at 11:19 -0800, Dmitry Torokhov wrote:
>> 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).
>
> It was easier to read, and backport. Patch 1 and 2 at least are easily
> backportable, and "easy" fixes on top of the existing code. Breakages
> would be easier to spot with the 3 separate fixes, and that's how I'd
> prefer it if it's not too much bother.

There is nothing to read if you drop the patch 2, as it is something
that was done and immediately deleted.

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v3 3/3] Input: goodix - use generic touchscreen_properties
From: Bastien Nocera @ 2018-01-27  1:19 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Marcin Niestroj, Antonio Ospite, linux-input@vger.kernel.org
In-Reply-To: <CAKdAkRTcsTOqj_jT7uu09k5i82wDEB0Fm-sb4TrNviwKbeb9iw@mail.gmail.com>



> On 27 Jan 2018, at 00:57, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> 
>> On Fri, Jan 26, 2018 at 3:48 PM, Bastien Nocera <hadess@hadess.net> wrote:
>>> On Fri, 2018-01-26 at 11:19 -0800, Dmitry Torokhov wrote:
>>> 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).
>> 
>> It was easier to read, and backport. Patch 1 and 2 at least are easily
>> backportable, and "easy" fixes on top of the existing code. Breakages
>> would be easier to spot with the 3 separate fixes, and that's how I'd
>> prefer it if it's not too much bother.
> 
> There is nothing to read if you drop the patch 2, as it is something
> that was done and immediately deleted.

It’s easier to backport and understand. Fine if you don’t want it. I’d rather it was still there.

> 
> Thanks.
> 
> -- 
> Dmitry


^ permalink raw reply

* [PATCH 0/2] Input: nspire-keypad: Adjustments for nspire_keypad_probe()
From: SF Markus Elfring @ 2018-01-27  9:52 UTC (permalink / raw)
  To: linux-input, Daniel Tang, Dmitry Torokhov; +Cc: LKML, kernel-janitors

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 27 Jan 2018 10:48:28 +0100

Two update suggestions were taken into account
from static source code analysis.

Markus Elfring (2):
  Delete an error message for a failed memory allocation
  Improve a size determination

 drivers/input/keyboard/nspire-keypad.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

-- 
2.16.1


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox