linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/11] Face lift for bu21013_ts driver
@ 2019-08-10  0:20 Dmitry Torokhov
  2019-08-10  0:20 ` [PATCH 01/11] ARM: ux500: improve BU21013 touchpad bindings Dmitry Torokhov
                   ` (11 more replies)
  0 siblings, 12 replies; 15+ messages in thread
From: Dmitry Torokhov @ 2019-08-10  0:20 UTC (permalink / raw)
  To: Linus Walleij; +Cc: linux-input, linux-kernel

Hi Linus,

So your patch has prompted me to take a look at the driver and
try to clean it up. I am sure I screwed up somewhere, but you said
you have the device, so please take a look at the series and
see if you can salvage them

Thanks!

Dmitry Torokhov (10):
  ARM: ux500: improve BU21013 touchpad bindings
  Input: bu21013_ts - rename some variables
  Input: bu21013_ts - annotate supend/resume methods as __maybe_unused
  Input: bu21013_ts - remove useless comments
  Input: bu21013_ts - convert to using managed resources
  Input: bu21013_ts - remove support for platform data
  Input: bu21013_ts - use interrupt from I2C client
  Input: bu21013_ts - fix suspend when wake source
  Input: bu21013_ts - switch to using MT-B (slotted) protocol
  Input: bu21013_ts - switch to using standard touchscreen properties

Linus Walleij (1):
  Input: bu21013_ts - convert to use GPIO descriptors

 .../bindings/input/touchscreen/bu21013.txt    |  27 +-
 arch/arm/boot/dts/ste-hrefprev60-stuib.dts    |  14 +-
 arch/arm/boot/dts/ste-hrefv60plus-stuib.dts   |  14 +-
 drivers/input/touchscreen/bu21013_ts.c        | 740 ++++++++----------
 include/linux/input/bu21013.h                 |  34 -
 5 files changed, 362 insertions(+), 467 deletions(-)
 delete mode 100644 include/linux/input/bu21013.h

-- 
Dmitry

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 01/11] ARM: ux500: improve BU21013 touchpad bindings
  2019-08-10  0:20 [PATCH 00/11] Face lift for bu21013_ts driver Dmitry Torokhov
@ 2019-08-10  0:20 ` Dmitry Torokhov
  2019-08-10  0:20 ` [PATCH 02/11] Input: bu21013_ts - convert to use GPIO descriptors Dmitry Torokhov
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Dmitry Torokhov @ 2019-08-10  0:20 UTC (permalink / raw)
  To: Linus Walleij; +Cc: linux-input, linux-kernel

In preparation to update to bu21013_tp driver properly annotate GPIOs
property (the INT GPIOs are active low, not open drain), and also define
interrupt lines so we do not have to have special conversion in the driver.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 arch/arm/boot/dts/ste-hrefprev60-stuib.dts  | 14 ++++++++++----
 arch/arm/boot/dts/ste-hrefv60plus-stuib.dts | 14 ++++++++++----
 2 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/arch/arm/boot/dts/ste-hrefprev60-stuib.dts b/arch/arm/boot/dts/ste-hrefprev60-stuib.dts
index aed940bd65a8..b78be5f4c212 100644
--- a/arch/arm/boot/dts/ste-hrefprev60-stuib.dts
+++ b/arch/arm/boot/dts/ste-hrefprev60-stuib.dts
@@ -4,6 +4,8 @@
  */
 
 /dts-v1/;
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/interrupt-controller/irq.h>
 #include "ste-hrefprev60.dtsi"
 #include "ste-href-stuib.dtsi"
 
@@ -23,12 +25,16 @@
 		i2c@80110000 {
 			/* Only one of these will be used */
 			bu21013_tp@5c {
-				touch-gpio = <&gpio2 12 0x4>;
-				reset-gpio = <&tc3589x_gpio 13 0x4>;
+				interrupt-parent = <&gpio2>;
+				interrupts = <12 IRQ_TYPE_LEVEL_LOW>;
+				touch-gpios = <&gpio2 12 GPIO_ACTIVE_LOW>;
+				reset-gpios = <&tc3589x_gpio 13 GPIO_LINE_OPEN_DRAIN>;
 			};
 			bu21013_tp@5d {
-				touch-gpio = <&gpio2 12 0x4>;
-				reset-gpio = <&tc3589x_gpio 13 0x4>;
+				interrupt-parent = <&gpio2>;
+				interrupts = <12 IRQ_TYPE_LEVEL_LOW>;
+				touch-gpios = <&gpio2 12 GPIO_ACTIVE_LOW>;
+				reset-gpios = <&tc3589x_gpio 13 GPIO_LINE_OPEN_DRAIN>;
 			};
 		};
 	};
diff --git a/arch/arm/boot/dts/ste-hrefv60plus-stuib.dts b/arch/arm/boot/dts/ste-hrefv60plus-stuib.dts
index 0f3c3b86bb20..9be513aad549 100644
--- a/arch/arm/boot/dts/ste-hrefv60plus-stuib.dts
+++ b/arch/arm/boot/dts/ste-hrefv60plus-stuib.dts
@@ -6,6 +6,8 @@
  */
 
 /dts-v1/;
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/interrupt-controller/irq.h>
 #include "ste-hrefv60plus.dtsi"
 #include "ste-href-stuib.dtsi"
 
@@ -25,12 +27,16 @@
 		i2c@80110000 {
 			/* Only one of these will be used */
 			bu21013_tp@5c {
-				touch-gpio = <&gpio2 20 0x4>;
-				reset-gpio = <&gpio4 17 0x4>;
+				interrupt-parent = <&gpio2>;
+				interrupts = <20 IRQ_TYPE_LEVEL_LOW>;
+				touch-gpios = <&gpio2 20 GPIO_ACTIVE_LOW>;
+				reset-gpios = <&gpio4 17 GPIO_LINE_OPEN_DRAIN>;
 			};
 			bu21013_tp@5d {
-				touch-gpio = <&gpio2 20 0x4>;
-				reset-gpio = <&gpio4 17 0x4>;
+				interrupt-parent = <&gpio2>;
+				interrupts = <20 IRQ_TYPE_LEVEL_LOW>;
+				touch-gpios = <&gpio2 20 GPIO_ACTIVE_LOW>;
+				reset-gpios = <&gpio4 17 GPIO_LINE_OPEN_DRAIN>;
 			};
 		};
 	};
-- 
2.23.0.rc1.153.gdeed80330f-goog


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 02/11] Input: bu21013_ts - convert to use GPIO descriptors
  2019-08-10  0:20 [PATCH 00/11] Face lift for bu21013_ts driver Dmitry Torokhov
  2019-08-10  0:20 ` [PATCH 01/11] ARM: ux500: improve BU21013 touchpad bindings Dmitry Torokhov
@ 2019-08-10  0:20 ` Dmitry Torokhov
  2019-08-10  0:20 ` [PATCH 03/11] Input: bu21013_ts - rename some variables Dmitry Torokhov
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Dmitry Torokhov @ 2019-08-10  0:20 UTC (permalink / raw)
  To: Linus Walleij; +Cc: linux-input, linux-kernel

From: Linus Walleij <linus.walleij@linaro.org>

This driver can use GPIO descriptors rather than GPIO numbers
without any problems, convert it. Name the field variables after
the actual pins on the chip rather than the "reset" and "touch"
names from the devicetree bindings that are vaguely inaccurate.

No in-tree users pass GPIO numbers in platform data so drop
this. Descriptor tables can be used to get these GPIOs from a board
file if need be.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 .../bindings/input/touchscreen/bu21013.txt    |  5 +-
 drivers/input/touchscreen/bu21013_ts.c        | 86 ++++++++-----------
 include/linux/input/bu21013.h                 |  4 -
 3 files changed, 41 insertions(+), 54 deletions(-)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/bu21013.txt b/Documentation/devicetree/bindings/input/touchscreen/bu21013.txt
index 56d835242af2..43899fc36ecf 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/bu21013.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/bu21013.txt
@@ -2,10 +2,11 @@
 
 Required properties:
  - compatible              : "rohm,bu21013_tp"
- - reg                     :  I2C device address
+ - reg                     : I2C device address
+ - reset-gpios             : GPIO pin enabling (selecting) chip (CS)
 
 Optional properties:
- - touch-gpio              : GPIO pin registering a touch event
+ - touch-gpios             : GPIO pin registering a touch event
  - <supply_name>-supply    : Phandle to a regulator supply
  - rohm,touch-max-x        : Maximum outward permitted limit in the X axis
  - rohm,touch-max-y        : Maximum outward permitted limit in the Y axis
diff --git a/drivers/input/touchscreen/bu21013_ts.c b/drivers/input/touchscreen/bu21013_ts.c
index 1d703e230ac3..c20f86f98ffc 100644
--- a/drivers/input/touchscreen/bu21013_ts.c
+++ b/drivers/input/touchscreen/bu21013_ts.c
@@ -14,11 +14,9 @@
 #include <linux/slab.h>
 #include <linux/regulator/consumer.h>
 #include <linux/module.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/of.h>
-#include <linux/of_gpio.h>
 
-#define PEN_DOWN_INTR	0
 #define MAX_FINGERS	2
 #define RESET_DELAY	30
 #define PENUP_TIMEOUT	(10)
@@ -143,8 +141,9 @@
  * @touch_stopped: touch stop flag
  * @chip: pointer to the touch panel controller
  * @in_dev: pointer to the input device structure
- * @intr_pin: interrupt pin value
  * @regulator: pointer to the Regulator used for touch screen
+ * @cs_gpiod: chip select GPIO line
+ * @int_gpiod: touch interrupt GPIO line
  *
  * Touch panel device data structure
  */
@@ -154,8 +153,9 @@ struct bu21013_ts_data {
 	const struct bu21013_platform_device *chip;
 	struct input_dev *in_dev;
 	struct regulator *regulator;
+	struct gpio_desc *cs_gpiod;
+	struct gpio_desc *int_gpiod;
 	unsigned int irq;
-	unsigned int intr_pin;
 	bool touch_stopped;
 };
 
@@ -257,20 +257,21 @@ static irqreturn_t bu21013_gpio_irq(int irq, void *device_data)
 {
 	struct bu21013_ts_data *data = device_data;
 	struct i2c_client *i2c = data->client;
+	int keep_polling;
 	int retval;
 
 	do {
 		retval = bu21013_do_touch_report(data);
 		if (retval < 0) {
 			dev_err(&i2c->dev, "bu21013_do_touch_report failed\n");
-			return IRQ_NONE;
+			break;
 		}
 
-		data->intr_pin = gpio_get_value(data->chip->touch_pin);
-		if (data->intr_pin == PEN_DOWN_INTR)
+		keep_polling = gpiod_get_value(data->int_gpiod);
+		if (keep_polling)
 			wait_event_timeout(data->wait, data->touch_stopped,
 					   msecs_to_jiffies(2));
-	} while (!data->intr_pin && !data->touch_stopped);
+	} while (keep_polling && !data->touch_stopped);
 
 	return IRQ_HANDLED;
 }
@@ -425,28 +426,6 @@ static void bu21013_free_irq(struct bu21013_ts_data *bu21013_data)
 	free_irq(bu21013_data->irq, bu21013_data);
 }
 
-/**
- * bu21013_cs_disable() - deconfigures the touch panel controller
- * @bu21013_data: device structure pointer
- *
- * This function is used to deconfigure the chip selection
- * for touch panel controller.
- */
-static void bu21013_cs_disable(struct bu21013_ts_data *bu21013_data)
-{
-	int error;
-
-	error = gpio_direction_output(bu21013_data->chip->cs_pin, 0);
-	if (error < 0)
-		dev_warn(&bu21013_data->client->dev,
-			 "%s: gpio direction failed, error: %d\n",
-			 __func__, error);
-	else
-		gpio_set_value(bu21013_data->chip->cs_pin, 0);
-
-	gpio_free(bu21013_data->chip->cs_pin);
-}
-
 #ifdef CONFIG_OF
 static const struct bu21013_platform_device *
 bu21013_parse_dt(struct device *dev)
@@ -471,9 +450,6 @@ bu21013_parse_dt(struct device *dev)
 	of_property_read_u32(np, "rohm,touch-max-x", &pdata->touch_x_max);
 	of_property_read_u32(np, "rohm,touch-max-y", &pdata->touch_y_max);
 
-	pdata->touch_pin = of_get_named_gpio(np, "touch-gpio", 0);
-	pdata->cs_pin = of_get_named_gpio(np, "reset-gpio", 0);
-
 	pdata->ext_clk = false;
 
 	return pdata;
@@ -516,11 +492,6 @@ static int bu21013_probe(struct i2c_client *client,
 			return PTR_ERR(pdata);
 	}
 
-	if (!gpio_is_valid(pdata->touch_pin)) {
-		dev_err(&client->dev, "invalid touch_pin supplied\n");
-		return -EINVAL;
-	}
-
 	bu21013_data = kzalloc(sizeof(struct bu21013_ts_data), GFP_KERNEL);
 	in_dev = input_allocate_device();
 	if (!bu21013_data || !in_dev) {
@@ -529,16 +500,26 @@ static int bu21013_probe(struct i2c_client *client,
 		goto err_free_mem;
 	}
 
+	/* Named "INT" on the chip, DT binding is "touch" */
+	bu21013_data->int_gpiod = gpiod_get(&client->dev, "touch", GPIOD_IN);
+	error = PTR_ERR_OR_ZERO(bu21013_data->int_gpiod);
+	if (error) {
+		if (error != -EPROBE_DEFER)
+			dev_err(&client->dev, "failed to get INT GPIO\n");
+		goto err_free_mem;
+	}
+	gpiod_set_consumer_name(bu21013_data->int_gpiod, "BU21013 INT");
+
 	bu21013_data->in_dev = in_dev;
 	bu21013_data->chip = pdata;
 	bu21013_data->client = client;
-	bu21013_data->irq = gpio_to_irq(pdata->touch_pin);
+	bu21013_data->irq = gpiod_to_irq(bu21013_data->int_gpiod);
 
 	bu21013_data->regulator = regulator_get(&client->dev, "avdd");
 	if (IS_ERR(bu21013_data->regulator)) {
 		dev_err(&client->dev, "regulator_get failed\n");
 		error = PTR_ERR(bu21013_data->regulator);
-		goto err_free_mem;
+		goto err_put_int_gpio;
 	}
 
 	error = regulator_enable(bu21013_data->regulator);
@@ -550,13 +531,16 @@ static int bu21013_probe(struct i2c_client *client,
 	bu21013_data->touch_stopped = false;
 	init_waitqueue_head(&bu21013_data->wait);
 
-	/* configure the gpio pins */
-	error = gpio_request_one(pdata->cs_pin, GPIOF_OUT_INIT_HIGH,
-				 "touchp_reset");
-	if (error < 0) {
-		dev_err(&client->dev, "Unable to request gpio reset_pin\n");
+	/* Named "CS" on the chip, DT binding is "reset" */
+	bu21013_data->cs_gpiod = gpiod_get(&client->dev, "reset",
+					   GPIOD_OUT_HIGH);
+	error = PTR_ERR_OR_ZERO(bu21013_data->cs_gpiod);
+	if (error) {
+		if (error != -EPROBE_DEFER)
+			dev_err(&client->dev, "failed to get CS GPIO\n");
 		goto err_disable_regulator;
 	}
+	gpiod_set_consumer_name(bu21013_data->cs_gpiod, "BU21013 CS");
 
 	/* configure the touch panel controller */
 	error = bu21013_init_chip(bu21013_data);
@@ -604,11 +588,14 @@ static int bu21013_probe(struct i2c_client *client,
 err_free_irq:
 	bu21013_free_irq(bu21013_data);
 err_cs_disable:
-	bu21013_cs_disable(bu21013_data);
+	gpiod_set_value(bu21013_data->cs_gpiod, 0);
+	gpiod_put(bu21013_data->cs_gpiod);
 err_disable_regulator:
 	regulator_disable(bu21013_data->regulator);
 err_put_regulator:
 	regulator_put(bu21013_data->regulator);
+err_put_int_gpio:
+	gpiod_put(bu21013_data->int_gpiod);
 err_free_mem:
 	input_free_device(in_dev);
 	kfree(bu21013_data);
@@ -628,13 +615,16 @@ static int bu21013_remove(struct i2c_client *client)
 
 	bu21013_free_irq(bu21013_data);
 
-	bu21013_cs_disable(bu21013_data);
+	gpiod_set_value(bu21013_data->cs_gpiod, 0);
+	gpiod_put(bu21013_data->cs_gpiod);
 
 	input_unregister_device(bu21013_data->in_dev);
 
 	regulator_disable(bu21013_data->regulator);
 	regulator_put(bu21013_data->regulator);
 
+	gpiod_put(bu21013_data->int_gpiod);
+
 	kfree(bu21013_data);
 
 	return 0;
diff --git a/include/linux/input/bu21013.h b/include/linux/input/bu21013.h
index 7e5b7e978e8a..58b1a9d44443 100644
--- a/include/linux/input/bu21013.h
+++ b/include/linux/input/bu21013.h
@@ -11,8 +11,6 @@
  * struct bu21013_platform_device - Handle the platform data
  * @touch_x_max: touch x max
  * @touch_y_max: touch y max
- * @cs_pin: chip select pin
- * @touch_pin: touch gpio pin
  * @ext_clk: external clock flag
  * @x_flip: x flip flag
  * @y_flip: y flip flag
@@ -23,8 +21,6 @@
 struct bu21013_platform_device {
 	int touch_x_max;
 	int touch_y_max;
-	unsigned int cs_pin;
-	unsigned int touch_pin;
 	bool ext_clk;
 	bool x_flip;
 	bool y_flip;
-- 
2.23.0.rc1.153.gdeed80330f-goog


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 03/11] Input: bu21013_ts - rename some variables
  2019-08-10  0:20 [PATCH 00/11] Face lift for bu21013_ts driver Dmitry Torokhov
  2019-08-10  0:20 ` [PATCH 01/11] ARM: ux500: improve BU21013 touchpad bindings Dmitry Torokhov
  2019-08-10  0:20 ` [PATCH 02/11] Input: bu21013_ts - convert to use GPIO descriptors Dmitry Torokhov
@ 2019-08-10  0:20 ` Dmitry Torokhov
  2019-08-10  0:20 ` [PATCH 04/11] Input: bu21013_ts - annotate supend/resume methods as __maybe_unused Dmitry Torokhov
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Dmitry Torokhov @ 2019-08-10  0:20 UTC (permalink / raw)
  To: Linus Walleij; +Cc: linux-input, linux-kernel

"bu21013_data" and "struct bu21013_ts_data" are a tad long, let's call them
"ts" and "struct bu21013_ts".

Also rename retval to error in bu21013_init_chip() and adjust formatting;
i2c_smbus_write_byte_data() returns negative on error and 0 on success, so
we simply test if whether erro is 0 or not.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/bu21013_ts.c | 376 +++++++++++++------------
 1 file changed, 190 insertions(+), 186 deletions(-)

diff --git a/drivers/input/touchscreen/bu21013_ts.c b/drivers/input/touchscreen/bu21013_ts.c
index c20f86f98ffc..e9cb020ed725 100644
--- a/drivers/input/touchscreen/bu21013_ts.c
+++ b/drivers/input/touchscreen/bu21013_ts.c
@@ -135,7 +135,7 @@
 #define DRIVER_TP	"bu21013_tp"
 
 /**
- * struct bu21013_ts_data - touch panel data structure
+ * struct bu21013_ts - touch panel data structure
  * @client: pointer to the i2c client
  * @wait: variable to wait_queue_head_t structure
  * @touch_stopped: touch stop flag
@@ -147,7 +147,7 @@
  *
  * Touch panel device data structure
  */
-struct bu21013_ts_data {
+struct bu21013_ts {
 	struct i2c_client *client;
 	wait_queue_head_t wait;
 	const struct bu21013_platform_device *chip;
@@ -161,34 +161,35 @@ struct bu21013_ts_data {
 
 /**
  * bu21013_read_block_data(): read the touch co-ordinates
- * @data: bu21013_ts_data structure pointer
+ * @data: bu21013_ts structure pointer
  * @buf: byte pointer
  *
  * Read the touch co-ordinates using i2c read block into buffer
  * and returns integer.
  */
-static int bu21013_read_block_data(struct bu21013_ts_data *data, u8 *buf)
+static int bu21013_read_block_data(struct bu21013_ts *ts, u8 *buf)
 {
 	int ret, i;
 
 	for (i = 0; i < I2C_RETRY_COUNT; i++) {
-		ret = i2c_smbus_read_i2c_block_data
-			(data->client, BU21013_SENSORS_BTN_0_7_REG,
-				LENGTH_OF_BUFFER, buf);
+		ret = i2c_smbus_read_i2c_block_data(ts->client,
+						    BU21013_SENSORS_BTN_0_7_REG,
+						    LENGTH_OF_BUFFER, buf);
 		if (ret == LENGTH_OF_BUFFER)
 			return 0;
 	}
+
 	return -EINVAL;
 }
 
 /**
  * bu21013_do_touch_report(): Get the touch co-ordinates
- * @data: bu21013_ts_data structure pointer
+ * @data: bu21013_ts structure pointer
  *
  * Get the touch co-ordinates from touch sensor registers and writes
  * into device structure and returns integer.
  */
-static int bu21013_do_touch_report(struct bu21013_ts_data *data)
+static int bu21013_do_touch_report(struct bu21013_ts *ts)
 {
 	u8	buf[LENGTH_OF_BUFFER];
 	unsigned int pos_x[2], pos_y[2];
@@ -196,10 +197,7 @@ static int bu21013_do_touch_report(struct bu21013_ts_data *data)
 	int	finger_down_count = 0;
 	int	i;
 
-	if (data == NULL)
-		return -EINVAL;
-
-	if (bu21013_read_block_data(data, buf) < 0)
+	if (bu21013_read_block_data(ts, buf) < 0)
 		return -EINVAL;
 
 	has_x_sensors = hweight32(buf[0] & BU21013_SENSORS_EN_0_7);
@@ -227,21 +225,21 @@ static int bu21013_do_touch_report(struct bu21013_ts_data *data)
 		}
 
 		for (i = 0; i < finger_down_count; i++) {
-			if (data->chip->x_flip)
-				pos_x[i] = data->chip->touch_x_max - pos_x[i];
-			if (data->chip->y_flip)
-				pos_y[i] = data->chip->touch_y_max - pos_y[i];
+			if (ts->chip->x_flip)
+				pos_x[i] = ts->chip->touch_x_max - pos_x[i];
+			if (ts->chip->y_flip)
+				pos_y[i] = ts->chip->touch_y_max - pos_y[i];
 
-			input_report_abs(data->in_dev,
+			input_report_abs(ts->in_dev,
 					 ABS_MT_POSITION_X, pos_x[i]);
-			input_report_abs(data->in_dev,
+			input_report_abs(ts->in_dev,
 					 ABS_MT_POSITION_Y, pos_y[i]);
-			input_mt_sync(data->in_dev);
+			input_mt_sync(ts->in_dev);
 		}
 	} else
-		input_mt_sync(data->in_dev);
+		input_mt_sync(ts->in_dev);
 
-	input_sync(data->in_dev);
+	input_sync(ts->in_dev);
 
 	return 0;
 }
@@ -255,23 +253,22 @@ static int bu21013_do_touch_report(struct bu21013_ts_data *data)
  */
 static irqreturn_t bu21013_gpio_irq(int irq, void *device_data)
 {
-	struct bu21013_ts_data *data = device_data;
-	struct i2c_client *i2c = data->client;
+	struct bu21013_ts *ts = device_data;
 	int keep_polling;
-	int retval;
+	int error;
 
 	do {
-		retval = bu21013_do_touch_report(data);
-		if (retval < 0) {
-			dev_err(&i2c->dev, "bu21013_do_touch_report failed\n");
+		error = bu21013_do_touch_report(ts);
+		if (error) {
+			dev_err(&ts->client->dev, "%s failed\n", __func__);
 			break;
 		}
 
-		keep_polling = gpiod_get_value(data->int_gpiod);
+		keep_polling = gpiod_get_value(ts->int_gpiod);
 		if (keep_polling)
-			wait_event_timeout(data->wait, data->touch_stopped,
+			wait_event_timeout(ts->wait, ts->touch_stopped,
 					   msecs_to_jiffies(2));
-	} while (keep_polling && !data->touch_stopped);
+	} while (keep_polling && !ts->touch_stopped);
 
 	return IRQ_HANDLED;
 }
@@ -283,130 +280,138 @@ static irqreturn_t bu21013_gpio_irq(int irq, void *device_data)
  * This function is used to power on
  * the bu21013 controller and returns integer.
  */
-static int bu21013_init_chip(struct bu21013_ts_data *data)
+static int bu21013_init_chip(struct bu21013_ts *ts)
 {
-	int retval;
-	struct i2c_client *i2c = data->client;
+	struct i2c_client *client = ts->client;
+	int error;
 
-	retval = i2c_smbus_write_byte_data(i2c, BU21013_RESET_REG,
-					BU21013_RESET_ENABLE);
-	if (retval < 0) {
-		dev_err(&i2c->dev, "BU21013_RESET reg write failed\n");
-		return retval;
+	error = i2c_smbus_write_byte_data(client, BU21013_RESET_REG,
+					  BU21013_RESET_ENABLE);
+	if (error) {
+		dev_err(&client->dev, "BU21013_RESET reg write failed\n");
+		return error;
 	}
 	msleep(RESET_DELAY);
 
-	retval = i2c_smbus_write_byte_data(i2c, BU21013_SENSOR_0_7_REG,
-					BU21013_SENSORS_EN_0_7);
-	if (retval < 0) {
-		dev_err(&i2c->dev, "BU21013_SENSOR_0_7 reg write failed\n");
-		return retval;
+	error = i2c_smbus_write_byte_data(client, BU21013_SENSOR_0_7_REG,
+					  BU21013_SENSORS_EN_0_7);
+	if (error) {
+		dev_err(&client->dev, "BU21013_SENSOR_0_7 reg write failed\n");
+		return error;
 	}
 
-	retval = i2c_smbus_write_byte_data(i2c, BU21013_SENSOR_8_15_REG,
-						BU21013_SENSORS_EN_8_15);
-	if (retval < 0) {
-		dev_err(&i2c->dev, "BU21013_SENSOR_8_15 reg write failed\n");
-		return retval;
+	error = i2c_smbus_write_byte_data(client, BU21013_SENSOR_8_15_REG,
+					  BU21013_SENSORS_EN_8_15);
+	if (error) {
+		dev_err(&client->dev, "BU21013_SENSOR_8_15 reg write failed\n");
+		return error;
 	}
 
-	retval = i2c_smbus_write_byte_data(i2c, BU21013_SENSOR_16_23_REG,
-						BU21013_SENSORS_EN_16_23);
-	if (retval < 0) {
-		dev_err(&i2c->dev, "BU21013_SENSOR_16_23 reg write failed\n");
-		return retval;
+	error = i2c_smbus_write_byte_data(client, BU21013_SENSOR_16_23_REG,
+					  BU21013_SENSORS_EN_16_23);
+	if (error) {
+		dev_err(&client->dev, "BU21013_SENSOR_16_23 reg write failed\n");
+		return error;
 	}
 
-	retval = i2c_smbus_write_byte_data(i2c, BU21013_POS_MODE1_REG,
-				(BU21013_POS_MODE1_0 | BU21013_POS_MODE1_1));
-	if (retval < 0) {
-		dev_err(&i2c->dev, "BU21013_POS_MODE1 reg write failed\n");
-		return retval;
+	error = i2c_smbus_write_byte_data(client, BU21013_POS_MODE1_REG,
+					  BU21013_POS_MODE1_0 |
+						BU21013_POS_MODE1_1);
+	if (error) {
+		dev_err(&client->dev, "BU21013_POS_MODE1 reg write failed\n");
+		return error;
 	}
 
-	retval = i2c_smbus_write_byte_data(i2c, BU21013_POS_MODE2_REG,
-			(BU21013_POS_MODE2_ZERO | BU21013_POS_MODE2_AVG1 |
-			BU21013_POS_MODE2_AVG2 | BU21013_POS_MODE2_EN_RAW |
-			BU21013_POS_MODE2_MULTI));
-	if (retval < 0) {
-		dev_err(&i2c->dev, "BU21013_POS_MODE2 reg write failed\n");
-		return retval;
+	error = i2c_smbus_write_byte_data(client, BU21013_POS_MODE2_REG,
+					  BU21013_POS_MODE2_ZERO |
+						BU21013_POS_MODE2_AVG1 |
+						BU21013_POS_MODE2_AVG2 |
+						BU21013_POS_MODE2_EN_RAW |
+						BU21013_POS_MODE2_MULTI);
+	if (error) {
+		dev_err(&client->dev, "BU21013_POS_MODE2 reg write failed\n");
+		return error;
 	}
 
-	if (data->chip->ext_clk)
-		retval = i2c_smbus_write_byte_data(i2c, BU21013_CLK_MODE_REG,
-			(BU21013_CLK_MODE_EXT | BU21013_CLK_MODE_CALIB));
+	if (ts->chip->ext_clk)
+		error = i2c_smbus_write_byte_data(client, BU21013_CLK_MODE_REG,
+						  BU21013_CLK_MODE_EXT |
+							BU21013_CLK_MODE_CALIB);
 	else
-		retval = i2c_smbus_write_byte_data(i2c, BU21013_CLK_MODE_REG,
-			(BU21013_CLK_MODE_DIV | BU21013_CLK_MODE_CALIB));
-	if (retval < 0) {
-		dev_err(&i2c->dev, "BU21013_CLK_MODE reg write failed\n");
-		return retval;
+		error = i2c_smbus_write_byte_data(client, BU21013_CLK_MODE_REG,
+						  BU21013_CLK_MODE_DIV |
+							BU21013_CLK_MODE_CALIB);
+	if (error) {
+		dev_err(&client->dev, "BU21013_CLK_MODE reg write failed\n");
+		return error;
 	}
 
-	retval = i2c_smbus_write_byte_data(i2c, BU21013_IDLE_REG,
-				(BU21013_IDLET_0 | BU21013_IDLE_INTERMIT_EN));
-	if (retval < 0) {
-		dev_err(&i2c->dev, "BU21013_IDLE reg write failed\n");
-		return retval;
+	error = i2c_smbus_write_byte_data(client, BU21013_IDLE_REG,
+					  BU21013_IDLET_0 |
+						BU21013_IDLE_INTERMIT_EN);
+	if (error) {
+		dev_err(&client->dev, "BU21013_IDLE reg write failed\n");
+		return error;
 	}
 
-	retval = i2c_smbus_write_byte_data(i2c, BU21013_INT_MODE_REG,
-						BU21013_INT_MODE_LEVEL);
-	if (retval < 0) {
-		dev_err(&i2c->dev, "BU21013_INT_MODE reg write failed\n");
-		return retval;
+	error = i2c_smbus_write_byte_data(client, BU21013_INT_MODE_REG,
+					  BU21013_INT_MODE_LEVEL);
+	if (error) {
+		dev_err(&client->dev, "BU21013_INT_MODE reg write failed\n");
+		return error;
 	}
 
-	retval = i2c_smbus_write_byte_data(i2c, BU21013_FILTER_REG,
-						(BU21013_DELTA_0_6 |
-							BU21013_FILTER_EN));
-	if (retval < 0) {
-		dev_err(&i2c->dev, "BU21013_FILTER reg write failed\n");
-		return retval;
+	error = i2c_smbus_write_byte_data(client, BU21013_FILTER_REG,
+					  BU21013_DELTA_0_6 |
+						BU21013_FILTER_EN);
+	if (error) {
+		dev_err(&client->dev, "BU21013_FILTER reg write failed\n");
+		return error;
 	}
 
-	retval = i2c_smbus_write_byte_data(i2c, BU21013_TH_ON_REG,
-					BU21013_TH_ON_5);
-	if (retval < 0) {
-		dev_err(&i2c->dev, "BU21013_TH_ON reg write failed\n");
-		return retval;
+	error = i2c_smbus_write_byte_data(client, BU21013_TH_ON_REG,
+					  BU21013_TH_ON_5);
+	if (error) {
+		dev_err(&client->dev, "BU21013_TH_ON reg write failed\n");
+		return error;
 	}
 
-	retval = i2c_smbus_write_byte_data(i2c, BU21013_TH_OFF_REG,
-				BU21013_TH_OFF_4 | BU21013_TH_OFF_3);
-	if (retval < 0) {
-		dev_err(&i2c->dev, "BU21013_TH_OFF reg write failed\n");
-		return retval;
+	error = i2c_smbus_write_byte_data(client, BU21013_TH_OFF_REG,
+					  BU21013_TH_OFF_4 | BU21013_TH_OFF_3);
+	if (error) {
+		dev_err(&client->dev, "BU21013_TH_OFF reg write failed\n");
+		return error;
 	}
 
-	retval = i2c_smbus_write_byte_data(i2c, BU21013_GAIN_REG,
-					(BU21013_GAIN_0 | BU21013_GAIN_1));
-	if (retval < 0) {
-		dev_err(&i2c->dev, "BU21013_GAIN reg write failed\n");
-		return retval;
+	error = i2c_smbus_write_byte_data(client, BU21013_GAIN_REG,
+					  BU21013_GAIN_0 | BU21013_GAIN_1);
+	if (error) {
+		dev_err(&client->dev, "BU21013_GAIN reg write failed\n");
+		return error;
 	}
 
-	retval = i2c_smbus_write_byte_data(i2c, BU21013_OFFSET_MODE_REG,
-					BU21013_OFFSET_MODE_DEFAULT);
-	if (retval < 0) {
-		dev_err(&i2c->dev, "BU21013_OFFSET_MODE reg write failed\n");
-		return retval;
+	error = i2c_smbus_write_byte_data(client, BU21013_OFFSET_MODE_REG,
+					  BU21013_OFFSET_MODE_DEFAULT);
+	if (error) {
+		dev_err(&client->dev, "BU21013_OFFSET_MODE reg write failed\n");
+		return error;
 	}
 
-	retval = i2c_smbus_write_byte_data(i2c, BU21013_XY_EDGE_REG,
-				(BU21013_X_EDGE_0 | BU21013_X_EDGE_2 |
-				BU21013_Y_EDGE_1 | BU21013_Y_EDGE_3));
-	if (retval < 0) {
-		dev_err(&i2c->dev, "BU21013_XY_EDGE reg write failed\n");
-		return retval;
+	error = i2c_smbus_write_byte_data(client, BU21013_XY_EDGE_REG,
+					  BU21013_X_EDGE_0 |
+						BU21013_X_EDGE_2 |
+						BU21013_Y_EDGE_1 |
+						BU21013_Y_EDGE_3);
+	if (error) {
+		dev_err(&client->dev, "BU21013_XY_EDGE reg write failed\n");
+		return error;
 	}
 
-	retval = i2c_smbus_write_byte_data(i2c, BU21013_DONE_REG,
-							BU21013_DONE);
-	if (retval < 0) {
-		dev_err(&i2c->dev, "BU21013_REG_DONE reg write failed\n");
-		return retval;
+	error = i2c_smbus_write_byte_data(client, BU21013_DONE_REG,
+					  BU21013_DONE);
+	if (error) {
+		dev_err(&client->dev, "BU21013_REG_DONE reg write failed\n");
+		return error;
 	}
 
 	return 0;
@@ -414,16 +419,16 @@ static int bu21013_init_chip(struct bu21013_ts_data *data)
 
 /**
  * bu21013_free_irq() - frees IRQ registered for touchscreen
- * @bu21013_data: device structure pointer
+ * @ts: device structure pointer
  *
  * This function signals interrupt thread to stop processing and
  * frees interrupt.
  */
-static void bu21013_free_irq(struct bu21013_ts_data *bu21013_data)
+static void bu21013_free_irq(struct bu21013_ts *ts)
 {
-	bu21013_data->touch_stopped = true;
-	wake_up(&bu21013_data->wait);
-	free_irq(bu21013_data->irq, bu21013_data);
+	ts->touch_stopped = true;
+	wake_up(&ts->wait);
+	free_irq(ts->irq, ts);
 }
 
 #ifdef CONFIG_OF
@@ -476,12 +481,12 @@ static int bu21013_probe(struct i2c_client *client,
 {
 	const struct bu21013_platform_device *pdata =
 					dev_get_platdata(&client->dev);
-	struct bu21013_ts_data *bu21013_data;
+	struct bu21013_ts *ts;
 	struct input_dev *in_dev;
 	int error;
 
 	if (!i2c_check_functionality(client->adapter,
-					I2C_FUNC_SMBUS_BYTE_DATA)) {
+				     I2C_FUNC_SMBUS_BYTE_DATA)) {
 		dev_err(&client->dev, "i2c smbus byte data not supported\n");
 		return -EIO;
 	}
@@ -492,58 +497,57 @@ static int bu21013_probe(struct i2c_client *client,
 			return PTR_ERR(pdata);
 	}
 
-	bu21013_data = kzalloc(sizeof(struct bu21013_ts_data), GFP_KERNEL);
+	ts = kzalloc(sizeof(*ts), GFP_KERNEL);
 	in_dev = input_allocate_device();
-	if (!bu21013_data || !in_dev) {
+	if (!ts || !in_dev) {
 		dev_err(&client->dev, "device memory alloc failed\n");
 		error = -ENOMEM;
 		goto err_free_mem;
 	}
 
 	/* Named "INT" on the chip, DT binding is "touch" */
-	bu21013_data->int_gpiod = gpiod_get(&client->dev, "touch", GPIOD_IN);
-	error = PTR_ERR_OR_ZERO(bu21013_data->int_gpiod);
+	ts->int_gpiod = gpiod_get(&client->dev, "touch", GPIOD_IN);
+	error = PTR_ERR_OR_ZERO(ts->int_gpiod);
 	if (error) {
 		if (error != -EPROBE_DEFER)
 			dev_err(&client->dev, "failed to get INT GPIO\n");
 		goto err_free_mem;
 	}
-	gpiod_set_consumer_name(bu21013_data->int_gpiod, "BU21013 INT");
+	gpiod_set_consumer_name(ts->int_gpiod, "BU21013 INT");
 
-	bu21013_data->in_dev = in_dev;
-	bu21013_data->chip = pdata;
-	bu21013_data->client = client;
-	bu21013_data->irq = gpiod_to_irq(bu21013_data->int_gpiod);
+	ts->in_dev = in_dev;
+	ts->chip = pdata;
+	ts->client = client;
+	ts->irq = gpiod_to_irq(ts->int_gpiod);
 
-	bu21013_data->regulator = regulator_get(&client->dev, "avdd");
-	if (IS_ERR(bu21013_data->regulator)) {
+	ts->regulator = regulator_get(&client->dev, "avdd");
+	if (IS_ERR(ts->regulator)) {
 		dev_err(&client->dev, "regulator_get failed\n");
-		error = PTR_ERR(bu21013_data->regulator);
+		error = PTR_ERR(ts->regulator);
 		goto err_put_int_gpio;
 	}
 
-	error = regulator_enable(bu21013_data->regulator);
+	error = regulator_enable(ts->regulator);
 	if (error < 0) {
 		dev_err(&client->dev, "regulator enable failed\n");
 		goto err_put_regulator;
 	}
 
-	bu21013_data->touch_stopped = false;
-	init_waitqueue_head(&bu21013_data->wait);
+	ts->touch_stopped = false;
+	init_waitqueue_head(&ts->wait);
 
 	/* Named "CS" on the chip, DT binding is "reset" */
-	bu21013_data->cs_gpiod = gpiod_get(&client->dev, "reset",
-					   GPIOD_OUT_HIGH);
-	error = PTR_ERR_OR_ZERO(bu21013_data->cs_gpiod);
+	ts->cs_gpiod = gpiod_get(&client->dev, "reset", GPIOD_OUT_HIGH);
+	error = PTR_ERR_OR_ZERO(ts->cs_gpiod);
 	if (error) {
 		if (error != -EPROBE_DEFER)
 			dev_err(&client->dev, "failed to get CS GPIO\n");
 		goto err_disable_regulator;
 	}
-	gpiod_set_consumer_name(bu21013_data->cs_gpiod, "BU21013 CS");
+	gpiod_set_consumer_name(ts->cs_gpiod, "BU21013 CS");
 
 	/* configure the touch panel controller */
-	error = bu21013_init_chip(bu21013_data);
+	error = bu21013_init_chip(ts);
 	if (error) {
 		dev_err(&client->dev, "error in bu21013 config\n");
 		goto err_cs_disable;
@@ -558,19 +562,19 @@ static int bu21013_probe(struct i2c_client *client,
 	__set_bit(EV_KEY, in_dev->evbit);
 	__set_bit(EV_ABS, in_dev->evbit);
 
-	input_set_abs_params(in_dev, ABS_MT_POSITION_X, 0,
-						pdata->touch_x_max, 0, 0);
-	input_set_abs_params(in_dev, ABS_MT_POSITION_Y, 0,
-						pdata->touch_y_max, 0, 0);
-	input_set_drvdata(in_dev, bu21013_data);
+	input_set_abs_params(in_dev, ABS_MT_POSITION_X,
+			     0, pdata->touch_x_max, 0, 0);
+	input_set_abs_params(in_dev, ABS_MT_POSITION_Y,
+			     0, pdata->touch_y_max, 0, 0);
+	input_set_drvdata(in_dev, ts);
 
-	error = request_threaded_irq(bu21013_data->irq, NULL, bu21013_gpio_irq,
+	error = request_threaded_irq(ts->irq, NULL, bu21013_gpio_irq,
 				     IRQF_TRIGGER_FALLING | IRQF_SHARED |
 					IRQF_ONESHOT,
-				     DRIVER_TP, bu21013_data);
+				     DRIVER_TP, ts);
 	if (error) {
 		dev_err(&client->dev, "request irq %d failed\n",
-			bu21013_data->irq);
+			ts->irq);
 		goto err_cs_disable;
 	}
 
@@ -581,24 +585,24 @@ static int bu21013_probe(struct i2c_client *client,
 	}
 
 	device_init_wakeup(&client->dev, pdata->wakeup);
-	i2c_set_clientdata(client, bu21013_data);
+	i2c_set_clientdata(client, ts);
 
 	return 0;
 
 err_free_irq:
-	bu21013_free_irq(bu21013_data);
+	bu21013_free_irq(ts);
 err_cs_disable:
-	gpiod_set_value(bu21013_data->cs_gpiod, 0);
-	gpiod_put(bu21013_data->cs_gpiod);
+	gpiod_set_value(ts->cs_gpiod, 0);
+	gpiod_put(ts->cs_gpiod);
 err_disable_regulator:
-	regulator_disable(bu21013_data->regulator);
+	regulator_disable(ts->regulator);
 err_put_regulator:
-	regulator_put(bu21013_data->regulator);
+	regulator_put(ts->regulator);
 err_put_int_gpio:
-	gpiod_put(bu21013_data->int_gpiod);
+	gpiod_put(ts->int_gpiod);
 err_free_mem:
 	input_free_device(in_dev);
-	kfree(bu21013_data);
+	kfree(ts);
 
 	return error;
 }
@@ -611,21 +615,21 @@ static int bu21013_probe(struct i2c_client *client,
  */
 static int bu21013_remove(struct i2c_client *client)
 {
-	struct bu21013_ts_data *bu21013_data = i2c_get_clientdata(client);
+	struct bu21013_ts *ts = i2c_get_clientdata(client);
 
-	bu21013_free_irq(bu21013_data);
+	bu21013_free_irq(ts);
 
-	gpiod_set_value(bu21013_data->cs_gpiod, 0);
-	gpiod_put(bu21013_data->cs_gpiod);
+	gpiod_set_value(ts->cs_gpiod, 0);
+	gpiod_put(ts->cs_gpiod);
 
-	input_unregister_device(bu21013_data->in_dev);
+	input_unregister_device(ts->in_dev);
 
-	regulator_disable(bu21013_data->regulator);
-	regulator_put(bu21013_data->regulator);
+	regulator_disable(ts->regulator);
+	regulator_put(ts->regulator);
 
-	gpiod_put(bu21013_data->int_gpiod);
+	gpiod_put(ts->int_gpiod);
 
-	kfree(bu21013_data);
+	kfree(ts);
 
 	return 0;
 }
@@ -640,16 +644,16 @@ static int bu21013_remove(struct i2c_client *client)
  */
 static int bu21013_suspend(struct device *dev)
 {
-	struct bu21013_ts_data *bu21013_data = dev_get_drvdata(dev);
-	struct i2c_client *client = bu21013_data->client;
+	struct bu21013_ts *ts = dev_get_drvdata(dev);
+	struct i2c_client *client = ts->client;
 
-	bu21013_data->touch_stopped = true;
+	ts->touch_stopped = true;
 	if (device_may_wakeup(&client->dev))
-		enable_irq_wake(bu21013_data->irq);
+		enable_irq_wake(ts->irq);
 	else
-		disable_irq(bu21013_data->irq);
+		disable_irq(ts->irq);
 
-	regulator_disable(bu21013_data->regulator);
+	regulator_disable(ts->regulator);
 
 	return 0;
 }
@@ -663,28 +667,28 @@ static int bu21013_suspend(struct device *dev)
  */
 static int bu21013_resume(struct device *dev)
 {
-	struct bu21013_ts_data *bu21013_data = dev_get_drvdata(dev);
-	struct i2c_client *client = bu21013_data->client;
+	struct bu21013_ts *ts = dev_get_drvdata(dev);
+	struct i2c_client *client = ts->client;
 	int retval;
 
-	retval = regulator_enable(bu21013_data->regulator);
+	retval = regulator_enable(ts->regulator);
 	if (retval < 0) {
 		dev_err(&client->dev, "bu21013 regulator enable failed\n");
 		return retval;
 	}
 
-	retval = bu21013_init_chip(bu21013_data);
+	retval = bu21013_init_chip(ts);
 	if (retval < 0) {
 		dev_err(&client->dev, "bu21013 controller config failed\n");
 		return retval;
 	}
 
-	bu21013_data->touch_stopped = false;
+	ts->touch_stopped = false;
 
 	if (device_may_wakeup(&client->dev))
-		disable_irq_wake(bu21013_data->irq);
+		disable_irq_wake(ts->irq);
 	else
-		enable_irq(bu21013_data->irq);
+		enable_irq(ts->irq);
 
 	return 0;
 }
-- 
2.23.0.rc1.153.gdeed80330f-goog


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 04/11] Input: bu21013_ts - annotate supend/resume methods as __maybe_unused
  2019-08-10  0:20 [PATCH 00/11] Face lift for bu21013_ts driver Dmitry Torokhov
                   ` (2 preceding siblings ...)
  2019-08-10  0:20 ` [PATCH 03/11] Input: bu21013_ts - rename some variables Dmitry Torokhov
@ 2019-08-10  0:20 ` Dmitry Torokhov
  2019-08-10  0:20 ` [PATCH 05/11] Input: bu21013_ts - remove useless comments Dmitry Torokhov
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Dmitry Torokhov @ 2019-08-10  0:20 UTC (permalink / raw)
  To: Linus Walleij; +Cc: linux-input, linux-kernel

Instead if #ifdef-ing out suspend and resume methods, let's mark
them as __maybe_unused to get better compile time coverage.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/bu21013_ts.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/input/touchscreen/bu21013_ts.c b/drivers/input/touchscreen/bu21013_ts.c
index e9cb020ed725..0bdadd24296f 100644
--- a/drivers/input/touchscreen/bu21013_ts.c
+++ b/drivers/input/touchscreen/bu21013_ts.c
@@ -634,7 +634,6 @@ static int bu21013_remove(struct i2c_client *client)
 	return 0;
 }
 
-#ifdef CONFIG_PM
 /**
  * bu21013_suspend() - suspend the touch screen controller
  * @dev: pointer to device structure
@@ -642,7 +641,7 @@ static int bu21013_remove(struct i2c_client *client)
  * This function is used to suspend the
  * touch panel controller and returns integer
  */
-static int bu21013_suspend(struct device *dev)
+static int __maybe_unused bu21013_suspend(struct device *dev)
 {
 	struct bu21013_ts *ts = dev_get_drvdata(dev);
 	struct i2c_client *client = ts->client;
@@ -665,7 +664,7 @@ static int bu21013_suspend(struct device *dev)
  * This function is used to resume the touch panel
  * controller and returns integer.
  */
-static int bu21013_resume(struct device *dev)
+static int __maybe_unused bu21013_resume(struct device *dev)
 {
 	struct bu21013_ts *ts = dev_get_drvdata(dev);
 	struct i2c_client *client = ts->client;
@@ -693,11 +692,7 @@ static int bu21013_resume(struct device *dev)
 	return 0;
 }
 
-static const struct dev_pm_ops bu21013_dev_pm_ops = {
-	.suspend = bu21013_suspend,
-	.resume  = bu21013_resume,
-};
-#endif
+static SIMPLE_DEV_PM_OPS(bu21013_dev_pm_ops, bu21013_suspend, bu21013_resume);
 
 static const struct i2c_device_id bu21013_id[] = {
 	{ DRIVER_TP, 0 },
@@ -708,9 +703,7 @@ MODULE_DEVICE_TABLE(i2c, bu21013_id);
 static struct i2c_driver bu21013_driver = {
 	.driver	= {
 		.name	=	DRIVER_TP,
-#ifdef CONFIG_PM
 		.pm	=	&bu21013_dev_pm_ops,
-#endif
 	},
 	.probe		=	bu21013_probe,
 	.remove		=	bu21013_remove,
-- 
2.23.0.rc1.153.gdeed80330f-goog


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 05/11] Input: bu21013_ts - remove useless comments
  2019-08-10  0:20 [PATCH 00/11] Face lift for bu21013_ts driver Dmitry Torokhov
                   ` (3 preceding siblings ...)
  2019-08-10  0:20 ` [PATCH 04/11] Input: bu21013_ts - annotate supend/resume methods as __maybe_unused Dmitry Torokhov
@ 2019-08-10  0:20 ` Dmitry Torokhov
  2019-08-10  0:20 ` [PATCH 06/11] Input: bu21013_ts - convert to using managed resources Dmitry Torokhov
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Dmitry Torokhov @ 2019-08-10  0:20 UTC (permalink / raw)
  To: Linus Walleij; +Cc: linux-input, linux-kernel

The comments for individual functions in the driver do not provide any
additional information beyond what function names indicate.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/bu21013_ts.c | 61 +-------------------------
 1 file changed, 2 insertions(+), 59 deletions(-)

diff --git a/drivers/input/touchscreen/bu21013_ts.c b/drivers/input/touchscreen/bu21013_ts.c
index 0bdadd24296f..a5230f6ea5f0 100644
--- a/drivers/input/touchscreen/bu21013_ts.c
+++ b/drivers/input/touchscreen/bu21013_ts.c
@@ -159,14 +159,6 @@ struct bu21013_ts {
 	bool touch_stopped;
 };
 
-/**
- * bu21013_read_block_data(): read the touch co-ordinates
- * @data: bu21013_ts structure pointer
- * @buf: byte pointer
- *
- * Read the touch co-ordinates using i2c read block into buffer
- * and returns integer.
- */
 static int bu21013_read_block_data(struct bu21013_ts *ts, u8 *buf)
 {
 	int ret, i;
@@ -182,13 +174,6 @@ static int bu21013_read_block_data(struct bu21013_ts *ts, u8 *buf)
 	return -EINVAL;
 }
 
-/**
- * bu21013_do_touch_report(): Get the touch co-ordinates
- * @data: bu21013_ts structure pointer
- *
- * Get the touch co-ordinates from touch sensor registers and writes
- * into device structure and returns integer.
- */
 static int bu21013_do_touch_report(struct bu21013_ts *ts)
 {
 	u8	buf[LENGTH_OF_BUFFER];
@@ -243,14 +228,7 @@ static int bu21013_do_touch_report(struct bu21013_ts *ts)
 
 	return 0;
 }
-/**
- * bu21013_gpio_irq() - gpio thread function for touch interrupt
- * @irq: irq value
- * @device_data: void pointer
- *
- * This gpio thread function for touch interrupt
- * and returns irqreturn_t.
- */
+
 static irqreturn_t bu21013_gpio_irq(int irq, void *device_data)
 {
 	struct bu21013_ts *ts = device_data;
@@ -273,13 +251,6 @@ static irqreturn_t bu21013_gpio_irq(int irq, void *device_data)
 	return IRQ_HANDLED;
 }
 
-/**
- * bu21013_init_chip() - power on sequence for the bu21013 controller
- * @data: device structure pointer
- *
- * This function is used to power on
- * the bu21013 controller and returns integer.
- */
 static int bu21013_init_chip(struct bu21013_ts *ts)
 {
 	struct i2c_client *client = ts->client;
@@ -468,14 +439,6 @@ bu21013_parse_dt(struct device *dev)
 }
 #endif
 
-/**
- * bu21013_probe() - initializes the i2c-client touchscreen driver
- * @client: i2c client structure pointer
- * @id: i2c device id pointer
- *
- * This function used to initializes the i2c-client touchscreen
- * driver and returns integer.
- */
 static int bu21013_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
@@ -606,13 +569,7 @@ static int bu21013_probe(struct i2c_client *client,
 
 	return error;
 }
-/**
- * bu21013_remove() - removes the i2c-client touchscreen driver
- * @client: i2c client structure pointer
- *
- * This function uses to remove the i2c-client
- * touchscreen driver and returns integer.
- */
+
 static int bu21013_remove(struct i2c_client *client)
 {
 	struct bu21013_ts *ts = i2c_get_clientdata(client);
@@ -634,13 +591,6 @@ static int bu21013_remove(struct i2c_client *client)
 	return 0;
 }
 
-/**
- * bu21013_suspend() - suspend the touch screen controller
- * @dev: pointer to device structure
- *
- * This function is used to suspend the
- * touch panel controller and returns integer
- */
 static int __maybe_unused bu21013_suspend(struct device *dev)
 {
 	struct bu21013_ts *ts = dev_get_drvdata(dev);
@@ -657,13 +607,6 @@ static int __maybe_unused bu21013_suspend(struct device *dev)
 	return 0;
 }
 
-/**
- * bu21013_resume() - resume the touch screen controller
- * @dev: pointer to device structure
- *
- * This function is used to resume the touch panel
- * controller and returns integer.
- */
 static int __maybe_unused bu21013_resume(struct device *dev)
 {
 	struct bu21013_ts *ts = dev_get_drvdata(dev);
-- 
2.23.0.rc1.153.gdeed80330f-goog


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 06/11] Input: bu21013_ts - convert to using managed resources
  2019-08-10  0:20 [PATCH 00/11] Face lift for bu21013_ts driver Dmitry Torokhov
                   ` (4 preceding siblings ...)
  2019-08-10  0:20 ` [PATCH 05/11] Input: bu21013_ts - remove useless comments Dmitry Torokhov
@ 2019-08-10  0:20 ` Dmitry Torokhov
  2019-08-10  0:20 ` [PATCH 07/11] Input: bu21013_ts - remove support for platform data Dmitry Torokhov
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Dmitry Torokhov @ 2019-08-10  0:20 UTC (permalink / raw)
  To: Linus Walleij; +Cc: linux-input, linux-kernel

This allows trimming error unwinding and device removal handling.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/bu21013_ts.c | 182 ++++++++++++-------------
 1 file changed, 84 insertions(+), 98 deletions(-)

diff --git a/drivers/input/touchscreen/bu21013_ts.c b/drivers/input/touchscreen/bu21013_ts.c
index a5230f6ea5f0..4b6f9544e94a 100644
--- a/drivers/input/touchscreen/bu21013_ts.c
+++ b/drivers/input/touchscreen/bu21013_ts.c
@@ -137,7 +137,6 @@
 /**
  * struct bu21013_ts - touch panel data structure
  * @client: pointer to the i2c client
- * @wait: variable to wait_queue_head_t structure
  * @touch_stopped: touch stop flag
  * @chip: pointer to the touch panel controller
  * @in_dev: pointer to the input device structure
@@ -149,7 +148,6 @@
  */
 struct bu21013_ts {
 	struct i2c_client *client;
-	wait_queue_head_t wait;
 	const struct bu21013_platform_device *chip;
 	struct input_dev *in_dev;
 	struct regulator *regulator;
@@ -242,11 +240,13 @@ static irqreturn_t bu21013_gpio_irq(int irq, void *device_data)
 			break;
 		}
 
+		if (unlikely(ts->touch_stopped))
+			break;
+
 		keep_polling = gpiod_get_value(ts->int_gpiod);
 		if (keep_polling)
-			wait_event_timeout(ts->wait, ts->touch_stopped,
-					   msecs_to_jiffies(2));
-	} while (keep_polling && !ts->touch_stopped);
+			usleep_range(2000, 2500);
+	} while (keep_polling);
 
 	return IRQ_HANDLED;
 }
@@ -388,20 +388,6 @@ static int bu21013_init_chip(struct bu21013_ts *ts)
 	return 0;
 }
 
-/**
- * bu21013_free_irq() - frees IRQ registered for touchscreen
- * @ts: device structure pointer
- *
- * This function signals interrupt thread to stop processing and
- * frees interrupt.
- */
-static void bu21013_free_irq(struct bu21013_ts *ts)
-{
-	ts->touch_stopped = true;
-	wake_up(&ts->wait);
-	free_irq(ts->irq, ts);
-}
-
 #ifdef CONFIG_OF
 static const struct bu21013_platform_device *
 bu21013_parse_dt(struct device *dev)
@@ -439,6 +425,20 @@ bu21013_parse_dt(struct device *dev)
 }
 #endif
 
+static void bu21013_power_off(void *_ts)
+{
+	struct bu21013_ts *ts = ts;
+
+	regulator_disable(ts->regulator);
+}
+
+static void bu21013_disable_chip(void *_ts)
+{
+	struct bu21013_ts *ts = ts;
+
+	gpiod_set_value(ts->cs_gpiod, 0);
+}
+
 static int bu21013_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
@@ -460,133 +460,119 @@ static int bu21013_probe(struct i2c_client *client,
 			return PTR_ERR(pdata);
 	}
 
-	ts = kzalloc(sizeof(*ts), GFP_KERNEL);
-	in_dev = input_allocate_device();
-	if (!ts || !in_dev) {
+	ts = devm_kzalloc(&client->dev, sizeof(*ts), GFP_KERNEL);
+	if (!ts)
+		return -ENOMEM;
+
+	ts->chip = pdata;
+	ts->client = client;
+
+	in_dev = devm_input_allocate_device(&client->dev);
+	if (!in_dev) {
 		dev_err(&client->dev, "device memory alloc failed\n");
-		error = -ENOMEM;
-		goto err_free_mem;
+		return -ENOMEM;
 	}
+	ts->in_dev = in_dev;
 
-	/* Named "INT" on the chip, DT binding is "touch" */
-	ts->int_gpiod = gpiod_get(&client->dev, "touch", GPIOD_IN);
-	error = PTR_ERR_OR_ZERO(ts->int_gpiod);
-	if (error) {
-		if (error != -EPROBE_DEFER)
-			dev_err(&client->dev, "failed to get INT GPIO\n");
-		goto err_free_mem;
-	}
-	gpiod_set_consumer_name(ts->int_gpiod, "BU21013 INT");
+	/* register the device to input subsystem */
+	in_dev->name = DRIVER_TP;
+	in_dev->id.bustype = BUS_I2C;
 
-	ts->in_dev = in_dev;
-	ts->chip = pdata;
-	ts->client = client;
-	ts->irq = gpiod_to_irq(ts->int_gpiod);
+	__set_bit(EV_SYN, in_dev->evbit);
+	__set_bit(EV_KEY, in_dev->evbit);
+	__set_bit(EV_ABS, in_dev->evbit);
+
+	input_set_abs_params(in_dev, ABS_MT_POSITION_X,
+			     0, pdata->touch_x_max, 0, 0);
+	input_set_abs_params(in_dev, ABS_MT_POSITION_Y,
+			     0, pdata->touch_y_max, 0, 0);
+	input_set_drvdata(in_dev, ts);
 
-	ts->regulator = regulator_get(&client->dev, "avdd");
+	ts->regulator = devm_regulator_get(&client->dev, "avdd");
 	if (IS_ERR(ts->regulator)) {
 		dev_err(&client->dev, "regulator_get failed\n");
-		error = PTR_ERR(ts->regulator);
-		goto err_put_int_gpio;
+		return PTR_ERR(ts->regulator);
 	}
 
 	error = regulator_enable(ts->regulator);
-	if (error < 0) {
+	if (error) {
 		dev_err(&client->dev, "regulator enable failed\n");
-		goto err_put_regulator;
+		return error;
 	}
 
-	ts->touch_stopped = false;
-	init_waitqueue_head(&ts->wait);
+	error = devm_add_action_or_reset(&client->dev, bu21013_power_off, ts);
+	if (error) {
+		dev_err(&client->dev, "failed to install power off handler\n");
+		return error;
+	}
 
 	/* Named "CS" on the chip, DT binding is "reset" */
-	ts->cs_gpiod = gpiod_get(&client->dev, "reset", GPIOD_OUT_HIGH);
+	ts->cs_gpiod = devm_gpiod_get(&client->dev, "reset", GPIOD_OUT_HIGH);
 	error = PTR_ERR_OR_ZERO(ts->cs_gpiod);
 	if (error) {
 		if (error != -EPROBE_DEFER)
 			dev_err(&client->dev, "failed to get CS GPIO\n");
-		goto err_disable_regulator;
+		return error;
 	}
 	gpiod_set_consumer_name(ts->cs_gpiod, "BU21013 CS");
 
+	error = devm_add_action_or_reset(&client->dev,
+					 bu21013_disable_chip, ts);
+	if (error) {
+		dev_err(&client->dev,
+			"failed to install chip disable handler\n");
+		return error;
+	}
+
+	/* Named "INT" on the chip, DT binding is "touch" */
+	ts->int_gpiod = devm_gpiod_get(&client->dev, "touch", GPIOD_IN);
+	error = PTR_ERR_OR_ZERO(ts->int_gpiod);
+	if (error) {
+		if (error != -EPROBE_DEFER)
+			dev_err(&client->dev, "failed to get INT GPIO\n");
+		return error;
+	}
+	gpiod_set_consumer_name(ts->int_gpiod, "BU21013 INT");
+
 	/* configure the touch panel controller */
 	error = bu21013_init_chip(ts);
 	if (error) {
 		dev_err(&client->dev, "error in bu21013 config\n");
-		goto err_cs_disable;
+		return error;
 	}
 
-	/* register the device to input subsystem */
-	in_dev->name = DRIVER_TP;
-	in_dev->id.bustype = BUS_I2C;
-	in_dev->dev.parent = &client->dev;
-
-	__set_bit(EV_SYN, in_dev->evbit);
-	__set_bit(EV_KEY, in_dev->evbit);
-	__set_bit(EV_ABS, in_dev->evbit);
-
-	input_set_abs_params(in_dev, ABS_MT_POSITION_X,
-			     0, pdata->touch_x_max, 0, 0);
-	input_set_abs_params(in_dev, ABS_MT_POSITION_Y,
-			     0, pdata->touch_y_max, 0, 0);
-	input_set_drvdata(in_dev, ts);
-
-	error = request_threaded_irq(ts->irq, NULL, bu21013_gpio_irq,
-				     IRQF_TRIGGER_FALLING | IRQF_SHARED |
-					IRQF_ONESHOT,
-				     DRIVER_TP, ts);
+	ts->irq = gpiod_to_irq(ts->int_gpiod);
+	error = devm_request_threaded_irq(&client->dev, ts->irq,
+					  NULL, bu21013_gpio_irq,
+					  IRQF_TRIGGER_FALLING |
+						IRQF_SHARED |
+						IRQF_ONESHOT,
+					  DRIVER_TP, ts);
 	if (error) {
 		dev_err(&client->dev, "request irq %d failed\n",
 			ts->irq);
-		goto err_cs_disable;
+		return error;
 	}
 
 	error = input_register_device(in_dev);
 	if (error) {
 		dev_err(&client->dev, "failed to register input device\n");
-		goto err_free_irq;
+		return error;
 	}
 
 	device_init_wakeup(&client->dev, pdata->wakeup);
 	i2c_set_clientdata(client, ts);
 
 	return 0;
-
-err_free_irq:
-	bu21013_free_irq(ts);
-err_cs_disable:
-	gpiod_set_value(ts->cs_gpiod, 0);
-	gpiod_put(ts->cs_gpiod);
-err_disable_regulator:
-	regulator_disable(ts->regulator);
-err_put_regulator:
-	regulator_put(ts->regulator);
-err_put_int_gpio:
-	gpiod_put(ts->int_gpiod);
-err_free_mem:
-	input_free_device(in_dev);
-	kfree(ts);
-
-	return error;
 }
 
 static int bu21013_remove(struct i2c_client *client)
 {
 	struct bu21013_ts *ts = i2c_get_clientdata(client);
 
-	bu21013_free_irq(ts);
-
-	gpiod_set_value(ts->cs_gpiod, 0);
-	gpiod_put(ts->cs_gpiod);
-
-	input_unregister_device(ts->in_dev);
-
-	regulator_disable(ts->regulator);
-	regulator_put(ts->regulator);
-
-	gpiod_put(ts->int_gpiod);
-
-	kfree(ts);
+	/* Make sure IRQ will exit quickly even if there is contact */
+	ts->touch_stopped = true;
+	/* The resources will be freed by devm */
 
 	return 0;
 }
-- 
2.23.0.rc1.153.gdeed80330f-goog


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 07/11] Input: bu21013_ts - remove support for platform data
  2019-08-10  0:20 [PATCH 00/11] Face lift for bu21013_ts driver Dmitry Torokhov
                   ` (5 preceding siblings ...)
  2019-08-10  0:20 ` [PATCH 06/11] Input: bu21013_ts - convert to using managed resources Dmitry Torokhov
@ 2019-08-10  0:20 ` Dmitry Torokhov
  2019-08-10  0:20 ` [PATCH 08/11] Input: bu21013_ts - use interrupt from I2C client Dmitry Torokhov
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Dmitry Torokhov @ 2019-08-10  0:20 UTC (permalink / raw)
  To: Linus Walleij; +Cc: linux-input, linux-kernel

There are no current users of the platform data in the tree, and
any new users should either use device tree, or static device
properties to describe the device.

This change drop the platform data definition and handling and moves the
driver over to generic device properties API. We also drop support for the
external clock. If it is needed we will have to extend the bindings to
supply the clock reference and handle it properly in the driver.

Also, wakeup setting should be coming from I2C client.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/bu21013_ts.c | 109 +++++++++----------------
 include/linux/input/bu21013.h          |  30 -------
 2 files changed, 37 insertions(+), 102 deletions(-)
 delete mode 100644 include/linux/input/bu21013.h

diff --git a/drivers/input/touchscreen/bu21013_ts.c b/drivers/input/touchscreen/bu21013_ts.c
index 4b6f9544e94a..79de7327a460 100644
--- a/drivers/input/touchscreen/bu21013_ts.c
+++ b/drivers/input/touchscreen/bu21013_ts.c
@@ -4,18 +4,18 @@
  * Author: Naveen Kumar G <naveen.gaddipati@stericsson.com> for ST-Ericsson
  */
 
-#include <linux/kernel.h>
+#include <linux/bitops.h>
 #include <linux/delay.h>
-#include <linux/interrupt.h>
+#include <linux/gpio/consumer.h>
 #include <linux/i2c.h>
-#include <linux/workqueue.h>
 #include <linux/input.h>
-#include <linux/input/bu21013.h>
-#include <linux/slab.h>
-#include <linux/regulator/consumer.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
 #include <linux/module.h>
-#include <linux/gpio/consumer.h>
-#include <linux/of.h>
+#include <linux/property.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+#include <linux/types.h>
 
 #define MAX_FINGERS	2
 #define RESET_DELAY	30
@@ -137,23 +137,32 @@
 /**
  * struct bu21013_ts - touch panel data structure
  * @client: pointer to the i2c client
- * @touch_stopped: touch stop flag
- * @chip: pointer to the touch panel controller
  * @in_dev: pointer to the input device structure
  * @regulator: pointer to the Regulator used for touch screen
  * @cs_gpiod: chip select GPIO line
  * @int_gpiod: touch interrupt GPIO line
+ * @irq: interrupt number the device is using
+ * @touch_x_max: maximum X coordinate reported by the device
+ * @touch_y_max: maximum Y coordinate reported by the device
+ * @x_flip: indicates that the driver should invert X coordinate before
+ *	reporting
+ * @y_flip: indicates that the driver should invert Y coordinate before
+ *	reporting
+ * @touch_stopped: touch stop flag
  *
  * Touch panel device data structure
  */
 struct bu21013_ts {
 	struct i2c_client *client;
-	const struct bu21013_platform_device *chip;
 	struct input_dev *in_dev;
 	struct regulator *regulator;
 	struct gpio_desc *cs_gpiod;
 	struct gpio_desc *int_gpiod;
 	unsigned int irq;
+	u32 touch_x_max;
+	u32 touch_y_max;
+	bool x_flip;
+	bool y_flip;
 	bool touch_stopped;
 };
 
@@ -208,10 +217,10 @@ static int bu21013_do_touch_report(struct bu21013_ts *ts)
 		}
 
 		for (i = 0; i < finger_down_count; i++) {
-			if (ts->chip->x_flip)
-				pos_x[i] = ts->chip->touch_x_max - pos_x[i];
-			if (ts->chip->y_flip)
-				pos_y[i] = ts->chip->touch_y_max - pos_y[i];
+			if (ts->x_flip)
+				pos_x[i] = ts->touch_x_max - pos_x[i];
+			if (ts->y_flip)
+				pos_y[i] = ts->touch_y_max - pos_y[i];
 
 			input_report_abs(ts->in_dev,
 					 ABS_MT_POSITION_X, pos_x[i]);
@@ -304,14 +313,9 @@ static int bu21013_init_chip(struct bu21013_ts *ts)
 		return error;
 	}
 
-	if (ts->chip->ext_clk)
-		error = i2c_smbus_write_byte_data(client, BU21013_CLK_MODE_REG,
-						  BU21013_CLK_MODE_EXT |
-							BU21013_CLK_MODE_CALIB);
-	else
-		error = i2c_smbus_write_byte_data(client, BU21013_CLK_MODE_REG,
-						  BU21013_CLK_MODE_DIV |
-							BU21013_CLK_MODE_CALIB);
+	error = i2c_smbus_write_byte_data(client, BU21013_CLK_MODE_REG,
+					  BU21013_CLK_MODE_DIV |
+						BU21013_CLK_MODE_CALIB);
 	if (error) {
 		dev_err(&client->dev, "BU21013_CLK_MODE reg write failed\n");
 		return error;
@@ -388,43 +392,6 @@ static int bu21013_init_chip(struct bu21013_ts *ts)
 	return 0;
 }
 
-#ifdef CONFIG_OF
-static const struct bu21013_platform_device *
-bu21013_parse_dt(struct device *dev)
-{
-	struct device_node *np = dev->of_node;
-	struct bu21013_platform_device *pdata;
-
-	if (!np) {
-		dev_err(dev, "no device tree or platform data\n");
-		return ERR_PTR(-EINVAL);
-	}
-
-	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
-	if (!pdata)
-		return ERR_PTR(-ENOMEM);
-
-	pdata->y_flip = pdata->x_flip = false;
-
-	pdata->x_flip = of_property_read_bool(np, "rohm,flip-x");
-	pdata->y_flip = of_property_read_bool(np, "rohm,flip-y");
-
-	of_property_read_u32(np, "rohm,touch-max-x", &pdata->touch_x_max);
-	of_property_read_u32(np, "rohm,touch-max-y", &pdata->touch_y_max);
-
-	pdata->ext_clk = false;
-
-	return pdata;
-}
-#else
-static inline const struct bu21013_platform_device *
-bu21013_parse_dt(struct device *dev)
-{
-	dev_err(dev, "no platform data available\n");
-	return ERR_PTR(-EINVAL);
-}
-#endif
-
 static void bu21013_power_off(void *_ts)
 {
 	struct bu21013_ts *ts = ts;
@@ -442,8 +409,6 @@ static void bu21013_disable_chip(void *_ts)
 static int bu21013_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
-	const struct bu21013_platform_device *pdata =
-					dev_get_platdata(&client->dev);
 	struct bu21013_ts *ts;
 	struct input_dev *in_dev;
 	int error;
@@ -454,19 +419,20 @@ static int bu21013_probe(struct i2c_client *client,
 		return -EIO;
 	}
 
-	if (!pdata) {
-		pdata = bu21013_parse_dt(&client->dev);
-		if (IS_ERR(pdata))
-			return PTR_ERR(pdata);
-	}
-
 	ts = devm_kzalloc(&client->dev, sizeof(*ts), GFP_KERNEL);
 	if (!ts)
 		return -ENOMEM;
 
-	ts->chip = pdata;
 	ts->client = client;
 
+	ts->x_flip = device_property_read_bool(&client->dev, "rohm,flip-x");
+	ts->y_flip = device_property_read_bool(&client->dev, "rohm,flip-y");
+
+	device_property_read_u32(&client->dev, "rohm,touch-max-x",
+				 &ts->touch_x_max);
+	device_property_read_u32(&client->dev, "rohm,touch-max-y",
+				 &ts->touch_y_max);
+
 	in_dev = devm_input_allocate_device(&client->dev);
 	if (!in_dev) {
 		dev_err(&client->dev, "device memory alloc failed\n");
@@ -483,9 +449,9 @@ static int bu21013_probe(struct i2c_client *client,
 	__set_bit(EV_ABS, in_dev->evbit);
 
 	input_set_abs_params(in_dev, ABS_MT_POSITION_X,
-			     0, pdata->touch_x_max, 0, 0);
+			     0, ts->touch_x_max, 0, 0);
 	input_set_abs_params(in_dev, ABS_MT_POSITION_Y,
-			     0, pdata->touch_y_max, 0, 0);
+			     0, ts->touch_y_max, 0, 0);
 	input_set_drvdata(in_dev, ts);
 
 	ts->regulator = devm_regulator_get(&client->dev, "avdd");
@@ -560,7 +526,6 @@ static int bu21013_probe(struct i2c_client *client,
 		return error;
 	}
 
-	device_init_wakeup(&client->dev, pdata->wakeup);
 	i2c_set_clientdata(client, ts);
 
 	return 0;
diff --git a/include/linux/input/bu21013.h b/include/linux/input/bu21013.h
deleted file mode 100644
index 58b1a9d44443..000000000000
--- a/include/linux/input/bu21013.h
+++ /dev/null
@@ -1,30 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-/*
- * Copyright (C) ST-Ericsson SA 2010
- * Author: Naveen Kumar G <naveen.gaddipati@stericsson.com> for ST-Ericsson
- */
-
-#ifndef _BU21013_H
-#define _BU21013_H
-
-/**
- * struct bu21013_platform_device - Handle the platform data
- * @touch_x_max: touch x max
- * @touch_y_max: touch y max
- * @ext_clk: external clock flag
- * @x_flip: x flip flag
- * @y_flip: y flip flag
- * @wakeup: wakeup flag
- *
- * This is used to handle the platform data
- */
-struct bu21013_platform_device {
-	int touch_x_max;
-	int touch_y_max;
-	bool ext_clk;
-	bool x_flip;
-	bool y_flip;
-	bool wakeup;
-};
-
-#endif
-- 
2.23.0.rc1.153.gdeed80330f-goog


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 08/11] Input: bu21013_ts - use interrupt from I2C client
  2019-08-10  0:20 [PATCH 00/11] Face lift for bu21013_ts driver Dmitry Torokhov
                   ` (6 preceding siblings ...)
  2019-08-10  0:20 ` [PATCH 07/11] Input: bu21013_ts - remove support for platform data Dmitry Torokhov
@ 2019-08-10  0:20 ` Dmitry Torokhov
  2019-08-10  0:20 ` [PATCH 09/11] Input: bu21013_ts - fix suspend when wake source Dmitry Torokhov
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Dmitry Torokhov @ 2019-08-10  0:20 UTC (permalink / raw)
  To: Linus Walleij; +Cc: linux-input, linux-kernel

Instead of trying to map INT GPIO to interrupt, let's use one supplied by
I2C client. If there is none - bail. This will also allow us to treat INT
GPIO as optional, as per the binding.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 .../bindings/input/touchscreen/bu21013.txt    |  6 +++-
 drivers/input/touchscreen/bu21013_ts.c        | 35 ++++++++++---------
 2 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/bu21013.txt b/Documentation/devicetree/bindings/input/touchscreen/bu21013.txt
index 43899fc36ecf..7ddb5de8343d 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/bu21013.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/bu21013.txt
@@ -4,6 +4,8 @@ Required properties:
  - compatible              : "rohm,bu21013_tp"
  - reg                     : I2C device address
  - reset-gpios             : GPIO pin enabling (selecting) chip (CS)
+ - interrupt-parent        : the phandle for the gpio controller
+ - interrupts              : (gpio) interrupt to which the chip is connected
 
 Optional properties:
  - touch-gpios             : GPIO pin registering a touch event
@@ -19,7 +21,9 @@ Example:
 		bu21013_tp@5c {
 			compatible = "rohm,bu21013_tp";
 			reg = <0x5c>;
-			touch-gpio = <&gpio2 20 0x4>;
+			interrupt-parent = <&gpio2>;
+			interrupts <&20 IRQ_TYPE_LEVEL_LOW>;
+			touch-gpio = <&gpio2 20 GPIO_ACTIVE_LOW>;
 			avdd-supply = <&ab8500_ldo_aux1_reg>;
 
 			rohm,touch-max-x = <384>;
diff --git a/drivers/input/touchscreen/bu21013_ts.c b/drivers/input/touchscreen/bu21013_ts.c
index 79de7327a460..d745643861cb 100644
--- a/drivers/input/touchscreen/bu21013_ts.c
+++ b/drivers/input/touchscreen/bu21013_ts.c
@@ -141,7 +141,6 @@
  * @regulator: pointer to the Regulator used for touch screen
  * @cs_gpiod: chip select GPIO line
  * @int_gpiod: touch interrupt GPIO line
- * @irq: interrupt number the device is using
  * @touch_x_max: maximum X coordinate reported by the device
  * @touch_y_max: maximum Y coordinate reported by the device
  * @x_flip: indicates that the driver should invert X coordinate before
@@ -158,7 +157,6 @@ struct bu21013_ts {
 	struct regulator *regulator;
 	struct gpio_desc *cs_gpiod;
 	struct gpio_desc *int_gpiod;
-	unsigned int irq;
 	u32 touch_x_max;
 	u32 touch_y_max;
 	bool x_flip;
@@ -252,7 +250,8 @@ static irqreturn_t bu21013_gpio_irq(int irq, void *device_data)
 		if (unlikely(ts->touch_stopped))
 			break;
 
-		keep_polling = gpiod_get_value(ts->int_gpiod);
+		keep_polling = ts->int_gpiod ?
+			gpiod_get_value(ts->int_gpiod) : false;
 		if (keep_polling)
 			usleep_range(2000, 2500);
 	} while (keep_polling);
@@ -419,6 +418,11 @@ static int bu21013_probe(struct i2c_client *client,
 		return -EIO;
 	}
 
+	if (!client->irq) {
+		dev_err(&client->dev, "No IRQ set up\n");
+		return -EINVAL;
+	}
+
 	ts = devm_kzalloc(&client->dev, sizeof(*ts), GFP_KERNEL);
 	if (!ts)
 		return -ENOMEM;
@@ -491,14 +495,17 @@ static int bu21013_probe(struct i2c_client *client,
 	}
 
 	/* Named "INT" on the chip, DT binding is "touch" */
-	ts->int_gpiod = devm_gpiod_get(&client->dev, "touch", GPIOD_IN);
+	ts->int_gpiod = devm_gpiod_get_optional(&client->dev,
+						"touch", GPIOD_IN);
 	error = PTR_ERR_OR_ZERO(ts->int_gpiod);
 	if (error) {
 		if (error != -EPROBE_DEFER)
 			dev_err(&client->dev, "failed to get INT GPIO\n");
 		return error;
 	}
-	gpiod_set_consumer_name(ts->int_gpiod, "BU21013 INT");
+
+	if (ts->int_gpiod)
+		gpiod_set_consumer_name(ts->int_gpiod, "BU21013 INT");
 
 	/* configure the touch panel controller */
 	error = bu21013_init_chip(ts);
@@ -507,16 +514,12 @@ static int bu21013_probe(struct i2c_client *client,
 		return error;
 	}
 
-	ts->irq = gpiod_to_irq(ts->int_gpiod);
-	error = devm_request_threaded_irq(&client->dev, ts->irq,
+	error = devm_request_threaded_irq(&client->dev, client->irq,
 					  NULL, bu21013_gpio_irq,
-					  IRQF_TRIGGER_FALLING |
-						IRQF_SHARED |
-						IRQF_ONESHOT,
-					  DRIVER_TP, ts);
+					  IRQF_ONESHOT, DRIVER_TP, ts);
 	if (error) {
 		dev_err(&client->dev, "request irq %d failed\n",
-			ts->irq);
+			client->irq);
 		return error;
 	}
 
@@ -549,9 +552,9 @@ static int __maybe_unused bu21013_suspend(struct device *dev)
 
 	ts->touch_stopped = true;
 	if (device_may_wakeup(&client->dev))
-		enable_irq_wake(ts->irq);
+		enable_irq_wake(client->irq);
 	else
-		disable_irq(ts->irq);
+		disable_irq(client->irq);
 
 	regulator_disable(ts->regulator);
 
@@ -579,9 +582,9 @@ static int __maybe_unused bu21013_resume(struct device *dev)
 	ts->touch_stopped = false;
 
 	if (device_may_wakeup(&client->dev))
-		disable_irq_wake(ts->irq);
+		disable_irq_wake(client->irq);
 	else
-		enable_irq(ts->irq);
+		enable_irq(client->irq);
 
 	return 0;
 }
-- 
2.23.0.rc1.153.gdeed80330f-goog


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 09/11] Input: bu21013_ts - fix suspend when wake source
  2019-08-10  0:20 [PATCH 00/11] Face lift for bu21013_ts driver Dmitry Torokhov
                   ` (7 preceding siblings ...)
  2019-08-10  0:20 ` [PATCH 08/11] Input: bu21013_ts - use interrupt from I2C client Dmitry Torokhov
@ 2019-08-10  0:20 ` Dmitry Torokhov
  2019-08-10  0:20 ` [PATCH 10/11] Input: bu21013_ts - switch to using MT-B (slotted) protocol Dmitry Torokhov
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Dmitry Torokhov @ 2019-08-10  0:20 UTC (permalink / raw)
  To: Linus Walleij; +Cc: linux-input, linux-kernel

If the touchscreen is configured as wakeup source we should not be cutting
off power to it.

Also, now that the driver relies on I2C client to supply IRQ, we do not
need to explicitly enable and disable IRQ for wakeup: if device is created
as wakeup source, I2C core will mark interrupt as wakeup one.

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

diff --git a/drivers/input/touchscreen/bu21013_ts.c b/drivers/input/touchscreen/bu21013_ts.c
index d745643861cb..d7e16e915743 100644
--- a/drivers/input/touchscreen/bu21013_ts.c
+++ b/drivers/input/touchscreen/bu21013_ts.c
@@ -547,44 +547,44 @@ static int bu21013_remove(struct i2c_client *client)
 
 static int __maybe_unused bu21013_suspend(struct device *dev)
 {
-	struct bu21013_ts *ts = dev_get_drvdata(dev);
-	struct i2c_client *client = ts->client;
+	struct i2c_client *client = to_i2c_client(dev);
+	struct bu21013_ts *ts = i2c_get_clientdata(client);
 
 	ts->touch_stopped = true;
-	if (device_may_wakeup(&client->dev))
-		enable_irq_wake(client->irq);
-	else
-		disable_irq(client->irq);
+	mb();
+	disable_irq(client->irq);
 
-	regulator_disable(ts->regulator);
+	if (!device_may_wakeup(&client->dev))
+		regulator_disable(ts->regulator);
 
 	return 0;
 }
 
 static int __maybe_unused bu21013_resume(struct device *dev)
 {
-	struct bu21013_ts *ts = dev_get_drvdata(dev);
-	struct i2c_client *client = ts->client;
-	int retval;
+	struct i2c_client *client = to_i2c_client(dev);
+	struct bu21013_ts *ts = i2c_get_clientdata(client);
+	int error;
 
-	retval = regulator_enable(ts->regulator);
-	if (retval < 0) {
-		dev_err(&client->dev, "bu21013 regulator enable failed\n");
-		return retval;
-	}
+	if (!device_may_wakeup(&client->dev)) {
+		error = regulator_enable(ts->regulator);
+		if (error) {
+			dev_err(&client->dev,
+				"failed to re-enable regulator when resuming\n");
+			return error;
+		}
 
-	retval = bu21013_init_chip(ts);
-	if (retval < 0) {
-		dev_err(&client->dev, "bu21013 controller config failed\n");
-		return retval;
+		error = bu21013_init_chip(ts);
+		if (error) {
+			dev_err(&client->dev,
+				"failed to reinitialize chip when resuming\n");
+			return error;
+		}
 	}
 
 	ts->touch_stopped = false;
-
-	if (device_may_wakeup(&client->dev))
-		disable_irq_wake(client->irq);
-	else
-		enable_irq(client->irq);
+	mb();
+	enable_irq(client->irq);
 
 	return 0;
 }
-- 
2.23.0.rc1.153.gdeed80330f-goog


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 10/11] Input: bu21013_ts - switch to using MT-B (slotted) protocol
  2019-08-10  0:20 [PATCH 00/11] Face lift for bu21013_ts driver Dmitry Torokhov
                   ` (8 preceding siblings ...)
  2019-08-10  0:20 ` [PATCH 09/11] Input: bu21013_ts - fix suspend when wake source Dmitry Torokhov
@ 2019-08-10  0:20 ` Dmitry Torokhov
  2019-08-10  0:20 ` [PATCH 11/11] Input: bu21013_ts - switch to using standard touchscreen properties Dmitry Torokhov
  2019-08-21 12:39 ` [PATCH 00/11] Face lift for bu21013_ts driver Linus Walleij
  11 siblings, 0 replies; 15+ messages in thread
From: Dmitry Torokhov @ 2019-08-10  0:20 UTC (permalink / raw)
  To: Linus Walleij; +Cc: linux-input, linux-kernel

MT-B protocol is more efficient and everyone expects it. We use in-kernel
tracking to identify contacts.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/bu21013_ts.c | 80 ++++++++++++++------------
 1 file changed, 43 insertions(+), 37 deletions(-)

diff --git a/drivers/input/touchscreen/bu21013_ts.c b/drivers/input/touchscreen/bu21013_ts.c
index d7e16e915743..2c534aa61687 100644
--- a/drivers/input/touchscreen/bu21013_ts.c
+++ b/drivers/input/touchscreen/bu21013_ts.c
@@ -9,6 +9,7 @@
 #include <linux/gpio/consumer.h>
 #include <linux/i2c.h>
 #include <linux/input.h>
+#include <linux/input/mt.h>
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
@@ -181,11 +182,13 @@ static int bu21013_read_block_data(struct bu21013_ts *ts, u8 *buf)
 
 static int bu21013_do_touch_report(struct bu21013_ts *ts)
 {
-	u8	buf[LENGTH_OF_BUFFER];
-	unsigned int pos_x[2], pos_y[2];
-	bool	has_x_sensors, has_y_sensors;
-	int	finger_down_count = 0;
-	int	i;
+	struct input_dev *input = ts->in_dev;
+	struct input_mt_pos pos[MAX_FINGERS];
+	int slots[MAX_FINGERS];
+	u8 buf[LENGTH_OF_BUFFER];
+	bool has_x_sensors, has_y_sensors;
+	int finger_down_count = 0;
+	int i;
 
 	if (bu21013_read_block_data(ts, buf) < 0)
 		return -EINVAL;
@@ -197,39 +200,38 @@ static int bu21013_do_touch_report(struct bu21013_ts *ts)
 		return 0;
 
 	for (i = 0; i < MAX_FINGERS; i++) {
-		const u8 *p = &buf[4 * i + 3];
-		unsigned int x = p[0] << SHIFT_2 | (p[1] & MASK_BITS);
-		unsigned int y = p[2] << SHIFT_2 | (p[3] & MASK_BITS);
-		if (x == 0 || y == 0)
+		const u8 *data = &buf[4 * i + 3];
+		struct input_mt_pos *p = &pos[finger_down_count];
+
+		p->x = data[0] << SHIFT_2 | (data[1] & MASK_BITS);
+		p->y = data[2] << SHIFT_2 | (data[3] & MASK_BITS);
+		if (p->x == 0 || p->y == 0)
 			continue;
-		pos_x[finger_down_count] = x;
-		pos_y[finger_down_count] = y;
+
 		finger_down_count++;
+
+		if (ts->x_flip)
+			p->x = ts->touch_x_max - p->x;
+		if (ts->y_flip)
+			p->y = ts->touch_y_max - p->y;
 	}
 
-	if (finger_down_count) {
-		if (finger_down_count == 2 &&
-		    (abs(pos_x[0] - pos_x[1]) < DELTA_MIN ||
-		     abs(pos_y[0] - pos_y[1]) < DELTA_MIN)) {
-			return 0;
-		}
+	if (finger_down_count == 2 &&
+	    (abs(pos[0].x - pos[1].x) < DELTA_MIN ||
+	     abs(pos[0].y - pos[1].y) < DELTA_MIN)) {
+		return 0;
+	}
 
-		for (i = 0; i < finger_down_count; i++) {
-			if (ts->x_flip)
-				pos_x[i] = ts->touch_x_max - pos_x[i];
-			if (ts->y_flip)
-				pos_y[i] = ts->touch_y_max - pos_y[i];
-
-			input_report_abs(ts->in_dev,
-					 ABS_MT_POSITION_X, pos_x[i]);
-			input_report_abs(ts->in_dev,
-					 ABS_MT_POSITION_Y, pos_y[i]);
-			input_mt_sync(ts->in_dev);
-		}
-	} else
-		input_mt_sync(ts->in_dev);
+	input_mt_assign_slots(input, slots, pos, finger_down_count, DELTA_MIN);
+	for (i = 0; i < finger_down_count; i++) {
+		input_mt_slot(input, slots[i]);
+		input_mt_report_slot_state(input, MT_TOOL_FINGER, true);
+		input_report_abs(input, ABS_MT_POSITION_X, pos[i].x);
+		input_report_abs(input, ABS_MT_POSITION_Y, pos[i].y);
+	}
 
-	input_sync(ts->in_dev);
+	input_mt_sync_frame(input);
+	input_sync(input);
 
 	return 0;
 }
@@ -443,20 +445,24 @@ static int bu21013_probe(struct i2c_client *client,
 		return -ENOMEM;
 	}
 	ts->in_dev = in_dev;
+	input_set_drvdata(in_dev, ts);
 
 	/* register the device to input subsystem */
 	in_dev->name = DRIVER_TP;
 	in_dev->id.bustype = BUS_I2C;
 
-	__set_bit(EV_SYN, in_dev->evbit);
-	__set_bit(EV_KEY, in_dev->evbit);
-	__set_bit(EV_ABS, in_dev->evbit);
-
 	input_set_abs_params(in_dev, ABS_MT_POSITION_X,
 			     0, ts->touch_x_max, 0, 0);
 	input_set_abs_params(in_dev, ABS_MT_POSITION_Y,
 			     0, ts->touch_y_max, 0, 0);
-	input_set_drvdata(in_dev, ts);
+
+	error = input_mt_init_slots(in_dev, MAX_FINGERS,
+				    INPUT_MT_DIRECT | INPUT_MT_TRACK |
+					INPUT_MT_DROP_UNUSED);
+	if (error) {
+		dev_err(&client->dev, "failed to initialize MT slots");
+		return error;
+	}
 
 	ts->regulator = devm_regulator_get(&client->dev, "avdd");
 	if (IS_ERR(ts->regulator)) {
-- 
2.23.0.rc1.153.gdeed80330f-goog


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 11/11] Input: bu21013_ts - switch to using standard touchscreen properties
  2019-08-10  0:20 [PATCH 00/11] Face lift for bu21013_ts driver Dmitry Torokhov
                   ` (9 preceding siblings ...)
  2019-08-10  0:20 ` [PATCH 10/11] Input: bu21013_ts - switch to using MT-B (slotted) protocol Dmitry Torokhov
@ 2019-08-10  0:20 ` Dmitry Torokhov
  2019-08-21 12:39 ` [PATCH 00/11] Face lift for bu21013_ts driver Linus Walleij
  11 siblings, 0 replies; 15+ messages in thread
From: Dmitry Torokhov @ 2019-08-10  0:20 UTC (permalink / raw)
  To: Linus Walleij; +Cc: linux-input, linux-kernel

This switches the driver over to the standard touchscreen properties for
coordinate transformation, while keeping old bindings working as well.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 .../bindings/input/touchscreen/bu21013.txt    | 16 ++++--
 drivers/input/touchscreen/bu21013_ts.c        | 54 +++++++++++--------
 2 files changed, 46 insertions(+), 24 deletions(-)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/bu21013.txt b/Documentation/devicetree/bindings/input/touchscreen/bu21013.txt
index 7ddb5de8343d..da4c9d8b99b1 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/bu21013.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/bu21013.txt
@@ -10,6 +10,16 @@ Required properties:
 Optional properties:
  - touch-gpios             : GPIO pin registering a touch event
  - <supply_name>-supply    : Phandle to a regulator supply
+ - touchscreen-size-x      : General touchscreen binding, see [1].
+ - touchscreen-size-y      : General touchscreen binding, see [1].
+ - touchscreen-inverted-x  : General touchscreen binding, see [1].
+ - touchscreen-inverted-y  : General touchscreen binding, see [1].
+ - touchscreen-swapped-x-y : General touchscreen binding, see [1].
+
+[1] All general touchscreen properties are described in
+    Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt.
+
+Deprecated properties:
  - rohm,touch-max-x        : Maximum outward permitted limit in the X axis
  - rohm,touch-max-y        : Maximum outward permitted limit in the Y axis
  - rohm,flip-x             : Flip touch coordinates on the X axis
@@ -26,8 +36,8 @@ Example:
 			touch-gpio = <&gpio2 20 GPIO_ACTIVE_LOW>;
 			avdd-supply = <&ab8500_ldo_aux1_reg>;
 
-			rohm,touch-max-x = <384>;
-			rohm,touch-max-y = <704>;
-			rohm,flip-y;
+			touchscreen-size-x = <384>;
+			touchscreen-size-y = <704>;
+			touchscreen-inverted-y;
 		};
 	};
diff --git a/drivers/input/touchscreen/bu21013_ts.c b/drivers/input/touchscreen/bu21013_ts.c
index 2c534aa61687..c89a00a6e67c 100644
--- a/drivers/input/touchscreen/bu21013_ts.c
+++ b/drivers/input/touchscreen/bu21013_ts.c
@@ -10,6 +10,7 @@
 #include <linux/i2c.h>
 #include <linux/input.h>
 #include <linux/input/mt.h>
+#include <linux/input/touchscreen.h>
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
@@ -139,6 +140,7 @@
  * struct bu21013_ts - touch panel data structure
  * @client: pointer to the i2c client
  * @in_dev: pointer to the input device structure
+ * @props: the device coordinate transformation properties
  * @regulator: pointer to the Regulator used for touch screen
  * @cs_gpiod: chip select GPIO line
  * @int_gpiod: touch interrupt GPIO line
@@ -155,6 +157,7 @@
 struct bu21013_ts {
 	struct i2c_client *client;
 	struct input_dev *in_dev;
+	struct touchscreen_properties props;
 	struct regulator *regulator;
 	struct gpio_desc *cs_gpiod;
 	struct gpio_desc *int_gpiod;
@@ -201,19 +204,13 @@ static int bu21013_do_touch_report(struct bu21013_ts *ts)
 
 	for (i = 0; i < MAX_FINGERS; i++) {
 		const u8 *data = &buf[4 * i + 3];
-		struct input_mt_pos *p = &pos[finger_down_count];
+		unsigned int x, y;
 
-		p->x = data[0] << SHIFT_2 | (data[1] & MASK_BITS);
-		p->y = data[2] << SHIFT_2 | (data[3] & MASK_BITS);
-		if (p->x == 0 || p->y == 0)
-			continue;
-
-		finger_down_count++;
-
-		if (ts->x_flip)
-			p->x = ts->touch_x_max - p->x;
-		if (ts->y_flip)
-			p->y = ts->touch_y_max - p->y;
+		x = data[0] << SHIFT_2 | (data[1] & MASK_BITS);
+		y = data[2] << SHIFT_2 | (data[3] & MASK_BITS);
+		if (x != 0 && y != 0)
+			touchscreen_set_mt_pos(&pos[finger_down_count++],
+					       &ts->props, x, y);
 	}
 
 	if (finger_down_count == 2 &&
@@ -412,6 +409,8 @@ static int bu21013_probe(struct i2c_client *client,
 {
 	struct bu21013_ts *ts;
 	struct input_dev *in_dev;
+	struct input_absinfo *info;
+	u32 max_x = 0, max_y = 0;
 	int error;
 
 	if (!i2c_check_functionality(client->adapter,
@@ -434,11 +433,6 @@ static int bu21013_probe(struct i2c_client *client,
 	ts->x_flip = device_property_read_bool(&client->dev, "rohm,flip-x");
 	ts->y_flip = device_property_read_bool(&client->dev, "rohm,flip-y");
 
-	device_property_read_u32(&client->dev, "rohm,touch-max-x",
-				 &ts->touch_x_max);
-	device_property_read_u32(&client->dev, "rohm,touch-max-y",
-				 &ts->touch_y_max);
-
 	in_dev = devm_input_allocate_device(&client->dev);
 	if (!in_dev) {
 		dev_err(&client->dev, "device memory alloc failed\n");
@@ -451,10 +445,28 @@ static int bu21013_probe(struct i2c_client *client,
 	in_dev->name = DRIVER_TP;
 	in_dev->id.bustype = BUS_I2C;
 
-	input_set_abs_params(in_dev, ABS_MT_POSITION_X,
-			     0, ts->touch_x_max, 0, 0);
-	input_set_abs_params(in_dev, ABS_MT_POSITION_Y,
-			     0, ts->touch_y_max, 0, 0);
+	device_property_read_u32(&client->dev, "rohm,touch-max-x", &max_x);
+	device_property_read_u32(&client->dev, "rohm,touch-max-y", &max_y);
+
+	input_set_abs_params(in_dev, ABS_MT_POSITION_X, 0, max_x, 0, 0);
+	input_set_abs_params(in_dev, ABS_MT_POSITION_Y, 0, max_y, 0, 0);
+
+	touchscreen_parse_properties(in_dev, true, &ts->props);
+
+	/* Adjust for the legacy "flip" properties, if present */
+	if (!ts->props.invert_x &&
+	    device_property_read_bool(&client->dev, "rohm,flip-x")) {
+		info = &in_dev->absinfo[ABS_MT_POSITION_X];
+		info->maximum -= info->minimum;
+		info->minimum = 0;
+	}
+
+	if (!ts->props.invert_y &&
+	    device_property_read_bool(&client->dev, "rohm,flip-y")) {
+		info = &in_dev->absinfo[ABS_MT_POSITION_Y];
+		info->maximum -= info->minimum;
+		info->minimum = 0;
+	}
 
 	error = input_mt_init_slots(in_dev, MAX_FINGERS,
 				    INPUT_MT_DIRECT | INPUT_MT_TRACK |
-- 
2.23.0.rc1.153.gdeed80330f-goog


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 00/11] Face lift for bu21013_ts driver
  2019-08-10  0:20 [PATCH 00/11] Face lift for bu21013_ts driver Dmitry Torokhov
                   ` (10 preceding siblings ...)
  2019-08-10  0:20 ` [PATCH 11/11] Input: bu21013_ts - switch to using standard touchscreen properties Dmitry Torokhov
@ 2019-08-21 12:39 ` Linus Walleij
  2019-08-21 17:42   ` Dmitry Torokhov
  11 siblings, 1 reply; 15+ messages in thread
From: Linus Walleij @ 2019-08-21 12:39 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Linux Input, linux-kernel

On Sat, Aug 10, 2019 at 2:20 AM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:

> So your patch has prompted me to take a look at the driver and
> try to clean it up. I am sure I screwed up somewhere, but you said
> you have the device, so please take a look at the series and
> see if you can salvage them

I will funnel patch 1/11 in the ARM SoC tree.

The rest work fine except on the resource release in the error path. I had
to do this:

diff --git a/drivers/input/touchscreen/bu21013_ts.c
b/drivers/input/touchscreen/bu21013_ts.c
index c89a00a6e67c..bdae4cd4243a 100644
--- a/drivers/input/touchscreen/bu21013_ts.c
+++ b/drivers/input/touchscreen/bu21013_ts.c
@@ -390,18 +390,18 @@ static int bu21013_init_chip(struct bu21013_ts *ts)
  return 0;
 }

-static void bu21013_power_off(void *_ts)
+static void bu21013_power_off(void *data)
 {
- struct bu21013_ts *ts = ts;
+ struct regulator *regulator = data;

- regulator_disable(ts->regulator);
+ regulator_disable(regulator);
 }

-static void bu21013_disable_chip(void *_ts)
+static void bu21013_disable_chip(void *data)
 {
- struct bu21013_ts *ts = ts;
+ struct gpio_desc *gpiod = data;

- gpiod_set_value(ts->cs_gpiod, 0);
+ gpiod_set_value(gpiod, 0);
 }

 static int bu21013_probe(struct i2c_client *client,
@@ -488,7 +488,8 @@ static int bu21013_probe(struct i2c_client *client,
  return error;
  }

- error = devm_add_action_or_reset(&client->dev, bu21013_power_off, ts);
+ error = devm_add_action_or_reset(&client->dev, bu21013_power_off,
+ ts->regulator);
  if (error) {
  dev_err(&client->dev, "failed to install power off handler\n");
  return error;
@@ -505,7 +506,7 @@ static int bu21013_probe(struct i2c_client *client,
  gpiod_set_consumer_name(ts->cs_gpiod, "BU21013 CS");

  error = devm_add_action_or_reset(&client->dev,
- bu21013_disable_chip, ts);
+ bu21013_disable_chip, ts->cs_gpiod);
  if (error) {
  dev_err(&client->dev,
  "failed to install chip disable handler\n");


I think this is because when probe() fails it first free:s the devm_kzalloc()
allocations, so the ts->foo will result in NULL dereference.

If I just reference the gpio desc or regulator directly like this in the
callback, things work fine.

With these changes:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Tested-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 00/11] Face lift for bu21013_ts driver
  2019-08-21 12:39 ` [PATCH 00/11] Face lift for bu21013_ts driver Linus Walleij
@ 2019-08-21 17:42   ` Dmitry Torokhov
  2019-08-23 11:30     ` Linus Walleij
  0 siblings, 1 reply; 15+ messages in thread
From: Dmitry Torokhov @ 2019-08-21 17:42 UTC (permalink / raw)
  To: Linus Walleij; +Cc: Linux Input, linux-kernel

On Wed, Aug 21, 2019 at 02:39:41PM +0200, Linus Walleij wrote:
> On Sat, Aug 10, 2019 at 2:20 AM Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> 
> > So your patch has prompted me to take a look at the driver and
> > try to clean it up. I am sure I screwed up somewhere, but you said
> > you have the device, so please take a look at the series and
> > see if you can salvage them
> 
> I will funnel patch 1/11 in the ARM SoC tree.
> 
> The rest work fine except on the resource release in the error path. I had
> to do this:
> 
> diff --git a/drivers/input/touchscreen/bu21013_ts.c
> b/drivers/input/touchscreen/bu21013_ts.c
> index c89a00a6e67c..bdae4cd4243a 100644
> --- a/drivers/input/touchscreen/bu21013_ts.c
> +++ b/drivers/input/touchscreen/bu21013_ts.c
> @@ -390,18 +390,18 @@ static int bu21013_init_chip(struct bu21013_ts *ts)
>   return 0;
>  }
> 
> -static void bu21013_power_off(void *_ts)
> +static void bu21013_power_off(void *data)
>  {
> - struct bu21013_ts *ts = ts;
> + struct regulator *regulator = data;
> 
> - regulator_disable(ts->regulator);
> + regulator_disable(regulator);
>  }
> 
> -static void bu21013_disable_chip(void *_ts)
> +static void bu21013_disable_chip(void *data)
>  {
> - struct bu21013_ts *ts = ts;
> + struct gpio_desc *gpiod = data;
> 
> - gpiod_set_value(ts->cs_gpiod, 0);
> + gpiod_set_value(gpiod, 0);
>  }
> 
>  static int bu21013_probe(struct i2c_client *client,
> @@ -488,7 +488,8 @@ static int bu21013_probe(struct i2c_client *client,
>   return error;
>   }
> 
> - error = devm_add_action_or_reset(&client->dev, bu21013_power_off, ts);
> + error = devm_add_action_or_reset(&client->dev, bu21013_power_off,
> + ts->regulator);
>   if (error) {
>   dev_err(&client->dev, "failed to install power off handler\n");
>   return error;
> @@ -505,7 +506,7 @@ static int bu21013_probe(struct i2c_client *client,
>   gpiod_set_consumer_name(ts->cs_gpiod, "BU21013 CS");
> 
>   error = devm_add_action_or_reset(&client->dev,
> - bu21013_disable_chip, ts);
> + bu21013_disable_chip, ts->cs_gpiod);
>   if (error) {
>   dev_err(&client->dev,
>   "failed to install chip disable handler\n");
> 
> 
> I think this is because when probe() fails it first free:s the devm_kzalloc()
> allocations, so the ts->foo will result in NULL dereference.

No, the release is done in opposite order of acquiring resources,
anything else would be madness and would not work.

The issue is this:

static void bu21013_disable_chip(void *_ts)
{
	struct bu21013_ts *ts = ts;

which shuts up gcc about the fact that 'ts' is uninitialized, it should
have said "ts = _ts". I guess it is a lesson for me to not call the voi
d pointer argument almost the same name as the structure, as it is easy
to miss in the review. The compiler would not care in either case, but a
human might have noticed.

Can you please try making this change (and the same in power off
handler)?

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 00/11] Face lift for bu21013_ts driver
  2019-08-21 17:42   ` Dmitry Torokhov
@ 2019-08-23 11:30     ` Linus Walleij
  0 siblings, 0 replies; 15+ messages in thread
From: Linus Walleij @ 2019-08-23 11:30 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Linux Input, linux-kernel

On Wed, Aug 21, 2019 at 7:43 PM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:

> The issue is this:
>
> static void bu21013_disable_chip(void *_ts)
> {
>         struct bu21013_ts *ts = ts;
>
> which shuts up gcc about the fact that 'ts' is uninitialized, it should
> have said "ts = _ts". I guess it is a lesson for me to not call the voi
> d pointer argument almost the same name as the structure, as it is easy
> to miss in the review. The compiler would not care in either case, but a
> human might have noticed.
>
> Can you please try making this change (and the same in power off
> handler)?

Yes this works! :)

Tested-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2019-08-23 11:30 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-10  0:20 [PATCH 00/11] Face lift for bu21013_ts driver Dmitry Torokhov
2019-08-10  0:20 ` [PATCH 01/11] ARM: ux500: improve BU21013 touchpad bindings Dmitry Torokhov
2019-08-10  0:20 ` [PATCH 02/11] Input: bu21013_ts - convert to use GPIO descriptors Dmitry Torokhov
2019-08-10  0:20 ` [PATCH 03/11] Input: bu21013_ts - rename some variables Dmitry Torokhov
2019-08-10  0:20 ` [PATCH 04/11] Input: bu21013_ts - annotate supend/resume methods as __maybe_unused Dmitry Torokhov
2019-08-10  0:20 ` [PATCH 05/11] Input: bu21013_ts - remove useless comments Dmitry Torokhov
2019-08-10  0:20 ` [PATCH 06/11] Input: bu21013_ts - convert to using managed resources Dmitry Torokhov
2019-08-10  0:20 ` [PATCH 07/11] Input: bu21013_ts - remove support for platform data Dmitry Torokhov
2019-08-10  0:20 ` [PATCH 08/11] Input: bu21013_ts - use interrupt from I2C client Dmitry Torokhov
2019-08-10  0:20 ` [PATCH 09/11] Input: bu21013_ts - fix suspend when wake source Dmitry Torokhov
2019-08-10  0:20 ` [PATCH 10/11] Input: bu21013_ts - switch to using MT-B (slotted) protocol Dmitry Torokhov
2019-08-10  0:20 ` [PATCH 11/11] Input: bu21013_ts - switch to using standard touchscreen properties Dmitry Torokhov
2019-08-21 12:39 ` [PATCH 00/11] Face lift for bu21013_ts driver Linus Walleij
2019-08-21 17:42   ` Dmitry Torokhov
2019-08-23 11:30     ` Linus Walleij

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).