[PATCH 1/5] Input: pixcir_i2c_ts - Remove unneeded gpio.h header file

[PATCH 1/5] Input: pixcir_i2c_ts - Remove unneeded gpio.h header file

[Fabio Estevam]

The touchscreen device is a GPIO consumer, not a GPIO controller,
so there is no need to include <linux/gpio.h>.

Remove the unneeded header file.

Signed-off-by: Fabio Estevam <festevam@gmail.com>
---
 drivers/input/touchscreen/pixcir_i2c_ts.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/input/touchscreen/pixcir_i2c_ts.c b/drivers/input/touchscreen/pixcir_i2c_ts.c
index e146dfa257b1..4561d65e7a1e 100644
--- a/drivers/input/touchscreen/pixcir_i2c_ts.c
+++ b/drivers/input/touchscreen/pixcir_i2c_ts.c
@@ -13,7 +13,6 @@
 #include <linux/input.h>
 #include <linux/input/mt.h>
 #include <linux/input/touchscreen.h>
-#include <linux/gpio.h>
 #include <linux/gpio/consumer.h>
 #include <linux/of_device.h>
 #include <linux/platform_data/pixcir_i2c_ts.h>
-- 
2.17.1

[PATCH 2/5] Input: pixcir_i2c_ts - Move definitions into a single file

[Fabio Estevam]

All the defined symbols from linux/platform_data/pixcir_i2c_ts.h
are only used by the pixcir_i2c_ts driver, so move all the definitions
locally and get rid of the pixcir_i2c_ts.h file.

Signed-off-by: Fabio Estevam <festevam@gmail.com>
---
 drivers/input/touchscreen/pixcir_i2c_ts.c   | 60 ++++++++++++++++++-
 include/linux/platform_data/pixcir_i2c_ts.h | 64 ---------------------
 2 files changed, 59 insertions(+), 65 deletions(-)
 delete mode 100644 include/linux/platform_data/pixcir_i2c_ts.h

diff --git a/drivers/input/touchscreen/pixcir_i2c_ts.c b/drivers/input/touchscreen/pixcir_i2c_ts.c
index 4561d65e7a1e..efe5f1e86ef1 100644
--- a/drivers/input/touchscreen/pixcir_i2c_ts.c
+++ b/drivers/input/touchscreen/pixcir_i2c_ts.c
@@ -15,11 +15,69 @@
 #include <linux/input/touchscreen.h>
 #include <linux/gpio/consumer.h>
 #include <linux/of_device.h>
-#include <linux/platform_data/pixcir_i2c_ts.h>
 #include <asm/unaligned.h>
 
 #define PIXCIR_MAX_SLOTS       5 /* Max fingers supported by driver */
 
+/*
+ * Register map
+ */
+#define PIXCIR_REG_POWER_MODE	51
+#define PIXCIR_REG_INT_MODE	52
+
+/*
+ * Power modes:
+ * active: max scan speed
+ * idle: lower scan speed with automatic transition to active on touch
+ * halt: datasheet says sleep but this is more like halt as the chip
+ *       clocks are cut and it can only be brought out of this mode
+ *	 using the RESET pin.
+ */
+enum pixcir_power_mode {
+	PIXCIR_POWER_ACTIVE,
+	PIXCIR_POWER_IDLE,
+	PIXCIR_POWER_HALT,
+};
+
+#define PIXCIR_POWER_MODE_MASK	0x03
+#define PIXCIR_POWER_ALLOW_IDLE (1UL << 2)
+
+/*
+ * Interrupt modes:
+ * periodical: interrupt is asserted periodicaly
+ * diff coordinates: interrupt is asserted when coordinates change
+ * level on touch: interrupt level asserted during touch
+ * pulse on touch: interrupt pulse asserted during touch
+ *
+ */
+enum pixcir_int_mode {
+	PIXCIR_INT_PERIODICAL,
+	PIXCIR_INT_DIFF_COORD,
+	PIXCIR_INT_LEVEL_TOUCH,
+	PIXCIR_INT_PULSE_TOUCH,
+};
+
+#define PIXCIR_INT_MODE_MASK	0x03
+#define PIXCIR_INT_ENABLE	(1UL << 3)
+#define PIXCIR_INT_POL_HIGH	(1UL << 2)
+
+/**
+ * struct pixcir_irc_chip_data - chip related data
+ * @max_fingers:	Max number of fingers reported simultaneously by h/w
+ * @has_hw_ids:		Hardware supports finger tracking IDs
+ *
+ */
+struct pixcir_i2c_chip_data {
+	u8 max_fingers;
+	bool has_hw_ids;
+};
+
+struct pixcir_ts_platform_data {
+	int x_max;
+	int y_max;
+	struct pixcir_i2c_chip_data chip;
+};
+
 struct pixcir_i2c_ts_data {
 	struct i2c_client *client;
 	struct input_dev *input;
diff --git a/include/linux/platform_data/pixcir_i2c_ts.h b/include/linux/platform_data/pixcir_i2c_ts.h
deleted file mode 100644
index 4ab3cd6f1cc2..000000000000
--- a/include/linux/platform_data/pixcir_i2c_ts.h
+++ /dev/null
@@ -1,64 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef	_PIXCIR_I2C_TS_H
-#define	_PIXCIR_I2C_TS_H
-
-/*
- * Register map
- */
-#define PIXCIR_REG_POWER_MODE	51
-#define PIXCIR_REG_INT_MODE	52
-
-/*
- * Power modes:
- * active: max scan speed
- * idle: lower scan speed with automatic transition to active on touch
- * halt: datasheet says sleep but this is more like halt as the chip
- *       clocks are cut and it can only be brought out of this mode
- *	 using the RESET pin.
- */
-enum pixcir_power_mode {
-	PIXCIR_POWER_ACTIVE,
-	PIXCIR_POWER_IDLE,
-	PIXCIR_POWER_HALT,
-};
-
-#define PIXCIR_POWER_MODE_MASK	0x03
-#define PIXCIR_POWER_ALLOW_IDLE (1UL << 2)
-
-/*
- * Interrupt modes:
- * periodical: interrupt is asserted periodicaly
- * diff coordinates: interrupt is asserted when coordinates change
- * level on touch: interrupt level asserted during touch
- * pulse on touch: interrupt pulse asserted druing touch
- *
- */
-enum pixcir_int_mode {
-	PIXCIR_INT_PERIODICAL,
-	PIXCIR_INT_DIFF_COORD,
-	PIXCIR_INT_LEVEL_TOUCH,
-	PIXCIR_INT_PULSE_TOUCH,
-};
-
-#define PIXCIR_INT_MODE_MASK	0x03
-#define PIXCIR_INT_ENABLE	(1UL << 3)
-#define PIXCIR_INT_POL_HIGH	(1UL << 2)
-
-/**
- * struct pixcir_irc_chip_data - chip related data
- * @max_fingers:	Max number of fingers reported simultaneously by h/w
- * @has_hw_ids:		Hardware supports finger tracking IDs
- *
- */
-struct pixcir_i2c_chip_data {
-	u8 max_fingers;
-	bool has_hw_ids;
-};
-
-struct pixcir_ts_platform_data {
-	int x_max;
-	int y_max;
-	struct pixcir_i2c_chip_data chip;
-};
-
-#endif
-- 
2.17.1

[PATCH 3/5] Input: pixcir_i2c_ts - Keep header files sorted

[Fabio Estevam]

Keep the header files in alphabetical order to keep it
more organized.

Signed-off-by: Fabio Estevam <festevam@gmail.com>
---
 drivers/input/touchscreen/pixcir_i2c_ts.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/input/touchscreen/pixcir_i2c_ts.c b/drivers/input/touchscreen/pixcir_i2c_ts.c
index efe5f1e86ef1..af2336da6954 100644
--- a/drivers/input/touchscreen/pixcir_i2c_ts.c
+++ b/drivers/input/touchscreen/pixcir_i2c_ts.c
@@ -5,17 +5,17 @@
  * Copyright (C) 2010-2011 Pixcir, Inc.
  */
 
+#include <asm/unaligned.h>
 #include <linux/delay.h>
-#include <linux/module.h>
-#include <linux/interrupt.h>
-#include <linux/slab.h>
+#include <linux/gpio/consumer.h>
 #include <linux/i2c.h>
 #include <linux/input.h>
 #include <linux/input/mt.h>
 #include <linux/input/touchscreen.h>
-#include <linux/gpio/consumer.h>
+#include <linux/interrupt.h>
 #include <linux/of_device.h>
-#include <asm/unaligned.h>
+#include <linux/module.h>
+#include <linux/slab.h>
 
 #define PIXCIR_MAX_SLOTS       5 /* Max fingers supported by driver */
 
-- 
2.17.1

[PATCH 4/5] Input: pixcir_i2c_ts: Print register address in decimal

[Fabio Estevam]

The pixcir datasheet lists the registers addresses in decimal and
so are PIXCIR_REG_POWER_MODE and PIXCIR_REG_INT_MODE defined in decimal.

Change the error messages to print the register addresses in decimal
instead of hexadecimal for better readability.

Signed-off-by: Fabio Estevam <festevam@gmail.com>
---
 drivers/input/touchscreen/pixcir_i2c_ts.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/input/touchscreen/pixcir_i2c_ts.c b/drivers/input/touchscreen/pixcir_i2c_ts.c
index af2336da6954..c8a35e97b595 100644
--- a/drivers/input/touchscreen/pixcir_i2c_ts.c
+++ b/drivers/input/touchscreen/pixcir_i2c_ts.c
@@ -249,7 +249,7 @@ static int pixcir_set_power_mode(struct pixcir_i2c_ts_data *ts,
 
 	ret = i2c_smbus_read_byte_data(ts->client, PIXCIR_REG_POWER_MODE);
 	if (ret < 0) {
-		dev_err(dev, "%s: can't read reg 0x%x : %d\n",
+		dev_err(dev, "%s: can't read reg %d : %d\n",
 			__func__, PIXCIR_REG_POWER_MODE, ret);
 		return ret;
 	}
@@ -262,7 +262,7 @@ static int pixcir_set_power_mode(struct pixcir_i2c_ts_data *ts,
 
 	ret = i2c_smbus_write_byte_data(ts->client, PIXCIR_REG_POWER_MODE, ret);
 	if (ret < 0) {
-		dev_err(dev, "%s: can't write reg 0x%x : %d\n",
+		dev_err(dev, "%s: can't write reg %d : %d\n",
 			__func__, PIXCIR_REG_POWER_MODE, ret);
 		return ret;
 	}
@@ -288,7 +288,7 @@ static int pixcir_set_int_mode(struct pixcir_i2c_ts_data *ts,
 
 	ret = i2c_smbus_read_byte_data(ts->client, PIXCIR_REG_INT_MODE);
 	if (ret < 0) {
-		dev_err(dev, "%s: can't read reg 0x%x : %d\n",
+		dev_err(dev, "%s: can't read reg %d : %d\n",
 			__func__, PIXCIR_REG_INT_MODE, ret);
 		return ret;
 	}
@@ -303,7 +303,7 @@ static int pixcir_set_int_mode(struct pixcir_i2c_ts_data *ts,
 
 	ret = i2c_smbus_write_byte_data(ts->client, PIXCIR_REG_INT_MODE, ret);
 	if (ret < 0) {
-		dev_err(dev, "%s: can't write reg 0x%x : %d\n",
+		dev_err(dev, "%s: can't write reg %d : %d\n",
 			__func__, PIXCIR_REG_INT_MODE, ret);
 		return ret;
 	}
@@ -321,7 +321,7 @@ static int pixcir_int_enable(struct pixcir_i2c_ts_data *ts, bool enable)
 
 	ret = i2c_smbus_read_byte_data(ts->client, PIXCIR_REG_INT_MODE);
 	if (ret < 0) {
-		dev_err(dev, "%s: can't read reg 0x%x : %d\n",
+		dev_err(dev, "%s: can't read reg %d : %d\n",
 			__func__, PIXCIR_REG_INT_MODE, ret);
 		return ret;
 	}
@@ -333,7 +333,7 @@ static int pixcir_int_enable(struct pixcir_i2c_ts_data *ts, bool enable)
 
 	ret = i2c_smbus_write_byte_data(ts->client, PIXCIR_REG_INT_MODE, ret);
 	if (ret < 0) {
-		dev_err(dev, "%s: can't write reg 0x%x : %d\n",
+		dev_err(dev, "%s: can't write reg %d : %d\n",
 			__func__, PIXCIR_REG_INT_MODE, ret);
 		return ret;
 	}
-- 
2.17.1

[PATCH 5/5] Input: pixcir_i2c_ts: Do not print error on defer probe

[Fabio Estevam]

In the case of defer probe we should not print an error message.

This also aligns with how defer probe is handled in the other GPIOs
used by this driver.

Signed-off-by: Fabio Estevam <festevam@gmail.com>
---
 drivers/input/touchscreen/pixcir_i2c_ts.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/pixcir_i2c_ts.c b/drivers/input/touchscreen/pixcir_i2c_ts.c
index c8a35e97b595..b5d53aaef718 100644
--- a/drivers/input/touchscreen/pixcir_i2c_ts.c
+++ b/drivers/input/touchscreen/pixcir_i2c_ts.c
@@ -567,7 +567,9 @@ static int pixcir_i2c_ts_probe(struct i2c_client *client,
 	tsdata->gpio_attb = devm_gpiod_get(dev, "attb", GPIOD_IN);
 	if (IS_ERR(tsdata->gpio_attb)) {
 		error = PTR_ERR(tsdata->gpio_attb);
-		dev_err(dev, "Failed to request ATTB gpio: %d\n", error);
+		if (error != -EPROBE_DEFER)
+			dev_err(dev, "Failed to request ATTB gpio: %d\n",
+				error);
 		return error;
 	}
 
@@ -575,7 +577,9 @@ static int pixcir_i2c_ts_probe(struct i2c_client *client,
 						     GPIOD_OUT_LOW);
 	if (IS_ERR(tsdata->gpio_reset)) {
 		error = PTR_ERR(tsdata->gpio_reset);
-		dev_err(dev, "Failed to request RESET gpio: %d\n", error);
+		if (error != -EPROBE_DEFER)
+			dev_err(dev, "Failed to request RESET gpio: %d\n",
+				error);
 		return error;
 	}
 
-- 
2.17.1

Re: [PATCH 1/5] Input: pixcir_i2c_ts - Remove unneeded gpio.h header file

[Roger Quadros]

Hi,

On 07/10/2019 15:16, Fabio Estevam wrote:
> The touchscreen device is a GPIO consumer, not a GPIO controller,
> so there is no need to include <linux/gpio.h>.
> 
> Remove the unneeded header file.
> 
> Signed-off-by: Fabio Estevam <festevam@gmail.com>

For all 5 patches,

Reviewed-by: Roger Quadros <rogerq@ti.com>

> ---
>   drivers/input/touchscreen/pixcir_i2c_ts.c | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/input/touchscreen/pixcir_i2c_ts.c b/drivers/input/touchscreen/pixcir_i2c_ts.c
> index e146dfa257b1..4561d65e7a1e 100644
> --- a/drivers/input/touchscreen/pixcir_i2c_ts.c
> +++ b/drivers/input/touchscreen/pixcir_i2c_ts.c
> @@ -13,7 +13,6 @@
>   #include <linux/input.h>
>   #include <linux/input/mt.h>
>   #include <linux/input/touchscreen.h>
> -#include <linux/gpio.h>
>   #include <linux/gpio/consumer.h>
>   #include <linux/of_device.h>
>   #include <linux/platform_data/pixcir_i2c_ts.h>
> 

-- 
cheers,
-roger

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [PATCH 1/5] Input: pixcir_i2c_ts - Remove unneeded gpio.h header file

[Dmitry Torokhov]

On Tue, Oct 08, 2019 at 01:11:15PM +0300, Roger Quadros wrote:
> Hi,
> 
> On 07/10/2019 15:16, Fabio Estevam wrote:
> > The touchscreen device is a GPIO consumer, not a GPIO controller,
> > so there is no need to include <linux/gpio.h>.
> > 
> > Remove the unneeded header file.
> > 
> > Signed-off-by: Fabio Estevam <festevam@gmail.com>
> 
> For all 5 patches,
> 
> Reviewed-by: Roger Quadros <rogerq@ti.com>

I guess we can also do this:

Input: pixcir_i2c_ts - remove platform data

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Previous change moved platform data definition into the driver, making it
unusable for users. Given that we want to move away from custom platform
data structures, and always use device properties (DT, ACPI or static) to
configure devices, let's complete the removal.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/pixcir_i2c_ts.c |  100 +++++++----------------------
 1 file changed, 25 insertions(+), 75 deletions(-)

diff --git a/drivers/input/touchscreen/pixcir_i2c_ts.c b/drivers/input/touchscreen/pixcir_i2c_ts.c
index ec768ab6148e..9aa098577350 100644
--- a/drivers/input/touchscreen/pixcir_i2c_ts.c
+++ b/drivers/input/touchscreen/pixcir_i2c_ts.c
@@ -62,7 +62,7 @@ enum pixcir_int_mode {
 #define PIXCIR_INT_POL_HIGH	(1UL << 2)
 
 /**
- * struct pixcir_irc_chip_data - chip related data
+ * struct pixcir_i2c_chip_data - chip related data
  * @max_fingers:	Max number of fingers reported simultaneously by h/w
  * @has_hw_ids:		Hardware supports finger tracking IDs
  *
@@ -72,12 +72,6 @@ struct pixcir_i2c_chip_data {
 	bool has_hw_ids;
 };
 
-struct pixcir_ts_platform_data {
-	int x_max;
-	int y_max;
-	struct pixcir_i2c_chip_data chip;
-};
-
 struct pixcir_i2c_ts_data {
 	struct i2c_client *client;
 	struct input_dev *input;
@@ -87,7 +81,6 @@ struct pixcir_i2c_ts_data {
 	struct gpio_desc *gpio_wake;
 	const struct pixcir_i2c_chip_data *chip;
 	struct touchscreen_properties prop;
-	int max_fingers;	/* Max fingers supported in this instance */
 	bool running;
 };
 
@@ -111,7 +104,7 @@ static void pixcir_ts_parse(struct pixcir_i2c_ts_data *tsdata,
 	memset(report, 0, sizeof(struct pixcir_report_data));
 
 	i = chip->has_hw_ids ? 1 : 0;
-	readsize = 2 + tsdata->max_fingers * (4 + i);
+	readsize = 2 + tsdata->chip->max_fingers * (4 + i);
 	if (readsize > sizeof(rdbuf))
 		readsize = sizeof(rdbuf);
 
@@ -132,8 +125,8 @@ static void pixcir_ts_parse(struct pixcir_i2c_ts_data *tsdata,
 	}
 
 	touch = rdbuf[0] & 0x7;
-	if (touch > tsdata->max_fingers)
-		touch = tsdata->max_fingers;
+	if (touch > tsdata->chip->max_fingers)
+		touch = tsdata->chip->max_fingers;
 
 	report->num_touches = touch;
 	bufptr = &rdbuf[2];
@@ -469,31 +462,9 @@ static int __maybe_unused pixcir_i2c_ts_resume(struct device *dev)
 static SIMPLE_DEV_PM_OPS(pixcir_dev_pm_ops,
 			 pixcir_i2c_ts_suspend, pixcir_i2c_ts_resume);
 
-#ifdef CONFIG_OF
-static const struct of_device_id pixcir_of_match[];
-
-static int pixcir_parse_dt(struct device *dev,
-			   struct pixcir_i2c_ts_data *tsdata)
-{
-	tsdata->chip = of_device_get_match_data(dev);
-	if (!tsdata->chip)
-		return -EINVAL;
-
-	return 0;
-}
-#else
-static int pixcir_parse_dt(struct device *dev,
-			   struct pixcir_i2c_ts_data *tsdata)
-{
-	return -EINVAL;
-}
-#endif
-
 static int pixcir_i2c_ts_probe(struct i2c_client *client,
 			       const struct i2c_device_id *id)
 {
-	const struct pixcir_ts_platform_data *pdata =
-			dev_get_platdata(&client->dev);
 	struct device *dev = &client->dev;
 	struct pixcir_i2c_ts_data *tsdata;
 	struct input_dev *input;
@@ -503,19 +474,11 @@ static int pixcir_i2c_ts_probe(struct i2c_client *client,
 	if (!tsdata)
 		return -ENOMEM;
 
-	if (pdata) {
-		tsdata->chip = &pdata->chip;
-	} else if (dev->of_node) {
-		error = pixcir_parse_dt(dev, tsdata);
-		if (error)
-			return error;
-	} else {
-		dev_err(dev, "platform data not defined\n");
-		return -EINVAL;
-	}
-
-	if (!tsdata->chip->max_fingers) {
-		dev_err(dev, "Invalid max_fingers in chip data\n");
+	tsdata->chip = device_get_match_data(dev);
+	if (!tsdata->chip && id)
+		tsdata->chip = (const void *)id->driver_data;
+	if (!tsdata->chip) {
+		dev_err(dev, "can't locate chip data\n");
 		return -EINVAL;
 	}
 
@@ -532,30 +495,17 @@ static int pixcir_i2c_ts_probe(struct i2c_client *client,
 	input->id.bustype = BUS_I2C;
 	input->open = pixcir_input_open;
 	input->close = pixcir_input_close;
-	input->dev.parent = dev;
-
-	if (pdata) {
-		input_set_abs_params(input, ABS_MT_POSITION_X, 0, pdata->x_max, 0, 0);
-		input_set_abs_params(input, ABS_MT_POSITION_Y, 0, pdata->y_max, 0, 0);
-	} else {
-		input_set_capability(input, EV_ABS, ABS_MT_POSITION_X);
-		input_set_capability(input, EV_ABS, ABS_MT_POSITION_Y);
-		touchscreen_parse_properties(input, true, &tsdata->prop);
-		if (!input_abs_get_max(input, ABS_MT_POSITION_X) ||
-		    !input_abs_get_max(input, ABS_MT_POSITION_Y)) {
-			dev_err(dev, "Touchscreen size is not specified\n");
-			return -EINVAL;
-		}
-	}
 
-	tsdata->max_fingers = tsdata->chip->max_fingers;
-	if (tsdata->max_fingers > PIXCIR_MAX_SLOTS) {
-		tsdata->max_fingers = PIXCIR_MAX_SLOTS;
-		dev_info(dev, "Limiting maximum fingers to %d\n",
-			 tsdata->max_fingers);
+	input_set_capability(input, EV_ABS, ABS_MT_POSITION_X);
+	input_set_capability(input, EV_ABS, ABS_MT_POSITION_Y);
+	touchscreen_parse_properties(input, true, &tsdata->prop);
+	if (!input_abs_get_max(input, ABS_MT_POSITION_X) ||
+	    !input_abs_get_max(input, ABS_MT_POSITION_Y)) {
+		dev_err(dev, "Touchscreen size is not specified\n");
+		return -EINVAL;
 	}
 
-	error = input_mt_init_slots(input, tsdata->max_fingers,
+	error = input_mt_init_slots(input, tsdata->chip->max_fingers,
 				    INPUT_MT_DIRECT | INPUT_MT_DROP_UNUSED);
 	if (error) {
 		dev_err(dev, "Error initializing Multi-Touch slots\n");
@@ -635,14 +585,6 @@ static int pixcir_i2c_ts_probe(struct i2c_client *client,
 	return 0;
 }
 
-static const struct i2c_device_id pixcir_i2c_ts_id[] = {
-	{ "pixcir_ts", 0 },
-	{ "pixcir_tangoc", 0 },
-	{ }
-};
-MODULE_DEVICE_TABLE(i2c, pixcir_i2c_ts_id);
-
-#ifdef CONFIG_OF
 static const struct pixcir_i2c_chip_data pixcir_ts_data = {
 	.max_fingers = 2,
 	/* no hw id support */
@@ -653,6 +595,14 @@ static const struct pixcir_i2c_chip_data pixcir_tangoc_data = {
 	.has_hw_ids = true,
 };
 
+static const struct i2c_device_id pixcir_i2c_ts_id[] = {
+	{ "pixcir_ts", (unsigned long) &pixcir_ts_data },
+	{ "pixcir_tangoc", (unsigned long) &pixcir_tangoc_data },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, pixcir_i2c_ts_id);
+
+#ifdef CONFIG_OF
 static const struct of_device_id pixcir_of_match[] = {
 	{ .compatible = "pixcir,pixcir_ts", .data = &pixcir_ts_data },
 	{ .compatible = "pixcir,pixcir_tangoc", .data = &pixcir_tangoc_data },



Thanks.

-- 
Dmitry

Re: [PATCH 1/5] Input: pixcir_i2c_ts - Remove unneeded gpio.h header file

[Fabio Estevam]

Hi Dmitry,

On Tue, Oct 8, 2019 at 3:44 PM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:

> I guess we can also do this:
>
> Input: pixcir_i2c_ts - remove platform data
>
> From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>
> Previous change moved platform data definition into the driver, making it
> unusable for users. Given that we want to move away from custom platform
> data structures, and always use device properties (DT, ACPI or static) to
> configure devices, let's complete the removal.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>  drivers/input/touchscreen/pixcir_i2c_ts.c |  100 +++++++----------------------
>  1 file changed, 25 insertions(+), 75 deletions(-)

Yes, this simplifies the code a lot:

Tested-by: Fabio Estevam <festevam@gmail.com>

Re: [PATCH 1/5] Input: pixcir_i2c_ts - Remove unneeded gpio.h header file

[Michal Vokáč]

On 08. 10. 19 20:44, Dmitry Torokhov wrote:
> On Tue, Oct 08, 2019 at 01:11:15PM +0300, Roger Quadros wrote:
>> Hi,
>>
>> On 07/10/2019 15:16, Fabio Estevam wrote:
>>> The touchscreen device is a GPIO consumer, not a GPIO controller,
>>> so there is no need to include <linux/gpio.h>.
>>>
>>> Remove the unneeded header file.
>>>
>>> Signed-off-by: Fabio Estevam <festevam@gmail.com>
>>
>> For all 5 patches,
>>
>> Reviewed-by: Roger Quadros <rogerq@ti.com>
> 
> I guess we can also do this:
> 
> Input: pixcir_i2c_ts - remove platform data
> 
> From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> 
> Previous change moved platform data definition into the driver, making it
> unusable for users. Given that we want to move away from custom platform
> data structures, and always use device properties (DT, ACPI or static) to
> configure devices, let's complete the removal.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>   drivers/input/touchscreen/pixcir_i2c_ts.c |  100 +++++++----------------------
>   1 file changed, 25 insertions(+), 75 deletions(-)

Compiled and boot tested the whole series including this patch from Dmitry
on our imx6dl-yapp4-draco with Pixcir Tango C48.

Everything still works for me, thanks!

Tested-by: michal.vokac@ysoft.com

Re: [PATCH 1/5] Input: pixcir_i2c_ts - Remove unneeded gpio.h header file

[Roger Quadros]

On 08/10/2019 21:44, Dmitry Torokhov wrote:
> On Tue, Oct 08, 2019 at 01:11:15PM +0300, Roger Quadros wrote:
>> Hi,
>>
>> On 07/10/2019 15:16, Fabio Estevam wrote:
>>> The touchscreen device is a GPIO consumer, not a GPIO controller,
>>> so there is no need to include <linux/gpio.h>.
>>>
>>> Remove the unneeded header file.
>>>
>>> Signed-off-by: Fabio Estevam <festevam@gmail.com>
>>
>> For all 5 patches,
>>
>> Reviewed-by: Roger Quadros <rogerq@ti.com>
> 
> I guess we can also do this:
> 
> Input: pixcir_i2c_ts - remove platform data
> 
> From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> 
> Previous change moved platform data definition into the driver, making it
> unusable for users. Given that we want to move away from custom platform
> data structures, and always use device properties (DT, ACPI or static) to
> configure devices, let's complete the removal.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Reviewed-by: Roger Quadros <rogerq@ti.com>

cheers,
-roger


> ---
>   drivers/input/touchscreen/pixcir_i2c_ts.c |  100 +++++++----------------------
>   1 file changed, 25 insertions(+), 75 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/pixcir_i2c_ts.c b/drivers/input/touchscreen/pixcir_i2c_ts.c
> index ec768ab6148e..9aa098577350 100644
> --- a/drivers/input/touchscreen/pixcir_i2c_ts.c
> +++ b/drivers/input/touchscreen/pixcir_i2c_ts.c
> @@ -62,7 +62,7 @@ enum pixcir_int_mode {
>   #define PIXCIR_INT_POL_HIGH	(1UL << 2)
>   
>   /**
> - * struct pixcir_irc_chip_data - chip related data
> + * struct pixcir_i2c_chip_data - chip related data
>    * @max_fingers:	Max number of fingers reported simultaneously by h/w
>    * @has_hw_ids:		Hardware supports finger tracking IDs
>    *
> @@ -72,12 +72,6 @@ struct pixcir_i2c_chip_data {
>   	bool has_hw_ids;
>   };
>   
> -struct pixcir_ts_platform_data {
> -	int x_max;
> -	int y_max;
> -	struct pixcir_i2c_chip_data chip;
> -};
> -
>   struct pixcir_i2c_ts_data {
>   	struct i2c_client *client;
>   	struct input_dev *input;
> @@ -87,7 +81,6 @@ struct pixcir_i2c_ts_data {
>   	struct gpio_desc *gpio_wake;
>   	const struct pixcir_i2c_chip_data *chip;
>   	struct touchscreen_properties prop;
> -	int max_fingers;	/* Max fingers supported in this instance */
>   	bool running;
>   };
>   
> @@ -111,7 +104,7 @@ static void pixcir_ts_parse(struct pixcir_i2c_ts_data *tsdata,
>   	memset(report, 0, sizeof(struct pixcir_report_data));
>   
>   	i = chip->has_hw_ids ? 1 : 0;
> -	readsize = 2 + tsdata->max_fingers * (4 + i);
> +	readsize = 2 + tsdata->chip->max_fingers * (4 + i);
>   	if (readsize > sizeof(rdbuf))
>   		readsize = sizeof(rdbuf);
>   
> @@ -132,8 +125,8 @@ static void pixcir_ts_parse(struct pixcir_i2c_ts_data *tsdata,
>   	}
>   
>   	touch = rdbuf[0] & 0x7;
> -	if (touch > tsdata->max_fingers)
> -		touch = tsdata->max_fingers;
> +	if (touch > tsdata->chip->max_fingers)
> +		touch = tsdata->chip->max_fingers;
>   
>   	report->num_touches = touch;
>   	bufptr = &rdbuf[2];
> @@ -469,31 +462,9 @@ static int __maybe_unused pixcir_i2c_ts_resume(struct device *dev)
>   static SIMPLE_DEV_PM_OPS(pixcir_dev_pm_ops,
>   			 pixcir_i2c_ts_suspend, pixcir_i2c_ts_resume);
>   
> -#ifdef CONFIG_OF
> -static const struct of_device_id pixcir_of_match[];
> -
> -static int pixcir_parse_dt(struct device *dev,
> -			   struct pixcir_i2c_ts_data *tsdata)
> -{
> -	tsdata->chip = of_device_get_match_data(dev);
> -	if (!tsdata->chip)
> -		return -EINVAL;
> -
> -	return 0;
> -}
> -#else
> -static int pixcir_parse_dt(struct device *dev,
> -			   struct pixcir_i2c_ts_data *tsdata)
> -{
> -	return -EINVAL;
> -}
> -#endif
> -
>   static int pixcir_i2c_ts_probe(struct i2c_client *client,
>   			       const struct i2c_device_id *id)
>   {
> -	const struct pixcir_ts_platform_data *pdata =
> -			dev_get_platdata(&client->dev);
>   	struct device *dev = &client->dev;
>   	struct pixcir_i2c_ts_data *tsdata;
>   	struct input_dev *input;
> @@ -503,19 +474,11 @@ static int pixcir_i2c_ts_probe(struct i2c_client *client,
>   	if (!tsdata)
>   		return -ENOMEM;
>   
> -	if (pdata) {
> -		tsdata->chip = &pdata->chip;
> -	} else if (dev->of_node) {
> -		error = pixcir_parse_dt(dev, tsdata);
> -		if (error)
> -			return error;
> -	} else {
> -		dev_err(dev, "platform data not defined\n");
> -		return -EINVAL;
> -	}
> -
> -	if (!tsdata->chip->max_fingers) {
> -		dev_err(dev, "Invalid max_fingers in chip data\n");
> +	tsdata->chip = device_get_match_data(dev);
> +	if (!tsdata->chip && id)
> +		tsdata->chip = (const void *)id->driver_data;
> +	if (!tsdata->chip) {
> +		dev_err(dev, "can't locate chip data\n");
>   		return -EINVAL;
>   	}
>   
> @@ -532,30 +495,17 @@ static int pixcir_i2c_ts_probe(struct i2c_client *client,
>   	input->id.bustype = BUS_I2C;
>   	input->open = pixcir_input_open;
>   	input->close = pixcir_input_close;
> -	input->dev.parent = dev;
> -
> -	if (pdata) {
> -		input_set_abs_params(input, ABS_MT_POSITION_X, 0, pdata->x_max, 0, 0);
> -		input_set_abs_params(input, ABS_MT_POSITION_Y, 0, pdata->y_max, 0, 0);
> -	} else {
> -		input_set_capability(input, EV_ABS, ABS_MT_POSITION_X);
> -		input_set_capability(input, EV_ABS, ABS_MT_POSITION_Y);
> -		touchscreen_parse_properties(input, true, &tsdata->prop);
> -		if (!input_abs_get_max(input, ABS_MT_POSITION_X) ||
> -		    !input_abs_get_max(input, ABS_MT_POSITION_Y)) {
> -			dev_err(dev, "Touchscreen size is not specified\n");
> -			return -EINVAL;
> -		}
> -	}
>   
> -	tsdata->max_fingers = tsdata->chip->max_fingers;
> -	if (tsdata->max_fingers > PIXCIR_MAX_SLOTS) {
> -		tsdata->max_fingers = PIXCIR_MAX_SLOTS;
> -		dev_info(dev, "Limiting maximum fingers to %d\n",
> -			 tsdata->max_fingers);
> +	input_set_capability(input, EV_ABS, ABS_MT_POSITION_X);
> +	input_set_capability(input, EV_ABS, ABS_MT_POSITION_Y);
> +	touchscreen_parse_properties(input, true, &tsdata->prop);
> +	if (!input_abs_get_max(input, ABS_MT_POSITION_X) ||
> +	    !input_abs_get_max(input, ABS_MT_POSITION_Y)) {
> +		dev_err(dev, "Touchscreen size is not specified\n");
> +		return -EINVAL;
>   	}
>   
> -	error = input_mt_init_slots(input, tsdata->max_fingers,
> +	error = input_mt_init_slots(input, tsdata->chip->max_fingers,
>   				    INPUT_MT_DIRECT | INPUT_MT_DROP_UNUSED);
>   	if (error) {
>   		dev_err(dev, "Error initializing Multi-Touch slots\n");
> @@ -635,14 +585,6 @@ static int pixcir_i2c_ts_probe(struct i2c_client *client,
>   	return 0;
>   }
>   
> -static const struct i2c_device_id pixcir_i2c_ts_id[] = {
> -	{ "pixcir_ts", 0 },
> -	{ "pixcir_tangoc", 0 },
> -	{ }
> -};
> -MODULE_DEVICE_TABLE(i2c, pixcir_i2c_ts_id);
> -
> -#ifdef CONFIG_OF
>   static const struct pixcir_i2c_chip_data pixcir_ts_data = {
>   	.max_fingers = 2,
>   	/* no hw id support */
> @@ -653,6 +595,14 @@ static const struct pixcir_i2c_chip_data pixcir_tangoc_data = {
>   	.has_hw_ids = true,
>   };
>   
> +static const struct i2c_device_id pixcir_i2c_ts_id[] = {
> +	{ "pixcir_ts", (unsigned long) &pixcir_ts_data },
> +	{ "pixcir_tangoc", (unsigned long) &pixcir_tangoc_data },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, pixcir_i2c_ts_id);
> +
> +#ifdef CONFIG_OF
>   static const struct of_device_id pixcir_of_match[] = {
>   	{ .compatible = "pixcir,pixcir_ts", .data = &pixcir_ts_data },
>   	{ .compatible = "pixcir,pixcir_tangoc", .data = &pixcir_tangoc_data },
> 
> 
> 
> Thanks.
> 

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki