On 08/24/2015 03:16 PM, Franklin S Cooper Jr. wrote:

On 08/24/2015 03:01 PM, Dmitry Torokhov wrote:

quoted

On Mon, Aug 24, 2015 at 02:48:36PM -0500, Franklin S Cooper Jr. wrote:

quoted

On 08/24/2015 02:41 PM, Dmitry Torokhov wrote:

quoted

On Fri, Aug 21, 2015 at 02:08:32PM -0500, Franklin S Cooper Jr wrote:

quoted

The current/old gpio framework used doesn't properly listen to
ACTIVE_LOW and ACTIVE_HIGH flags. The newer gpio framework takes into
account these flags when setting gpio values.

Also use gpiod_set_value_cansleep since wake and reset pins can be
provided by bus based io expanders.

Signed-off-by: Franklin S Cooper Jr <redacted>
---
 .../bindings/input/touchscreen/edt-ft5x06.txt      |   4 +-
 drivers/input/touchscreen/edt-ft5x06.c             | 115 +++++++--------------
 include/linux/input/edt-ft5x06.h                   |   4 +-
 3 files changed, 43 insertions(+), 80 deletions(-)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
index 76db967..9330d4d 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt

@@ -50,6 +50,6 @@ Example:
 		pinctrl-0 = <&edt_ft5x06_pins>;
 		interrupt-parent = <&gpio2>;
 		interrupts = <5 0>;
-		reset-gpios = <&gpio2 6 1>;
-		wake-gpios = <&gpio4 9 0>;
+		reset-gpios = <&gpio2 6 GPIO_ACTIVE_LOW>;
+		wake-gpios = <&gpio4 9 GPIO_ACTIVE_HIGH>;
 	};

diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index 394b1de..6b128b3 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c

@@ -91,9 +91,9 @@ struct edt_ft5x06_ts_data {
 	u16 num_x;
 	u16 num_y;
 
-	int reset_pin;
-	int irq_pin;
-	int wake_pin;
+	struct gpio_desc *reset_pin;
+	struct gpio_desc *wake_pin;
+	struct gpio_desc *irq_pin;
 
 #if defined(CONFIG_DEBUG_FS)
 	struct dentry *debug_dir;

@@ -755,36 +755,14 @@ edt_ft5x06_ts_teardown_debugfs(struct edt_ft5x06_ts_data *tsdata)
 static int edt_ft5x06_ts_reset(struct i2c_client *client,
 			struct edt_ft5x06_ts_data *tsdata)
 {
-	int error;
-
-	if (gpio_is_valid(tsdata->wake_pin)) {
-		error = devm_gpio_request_one(&client->dev,
-					tsdata->wake_pin, GPIOF_OUT_INIT_LOW,
-					"edt-ft5x06 wake");
-		if (error) {
-			dev_err(&client->dev,
-				"Failed to request GPIO %d as wake pin, error %d\n",
-				tsdata->wake_pin, error);
-			return error;
-		}
-
+	if (tsdata->wake_pin) {
 		msleep(5);
-		gpio_set_value(tsdata->wake_pin, 1);
+		gpiod_set_value_cansleep(tsdata->wake_pin, 1);
 	}
-	if (gpio_is_valid(tsdata->reset_pin)) {
-		/* this pulls reset down, enabling the low active reset */
-		error = devm_gpio_request_one(&client->dev,
-					tsdata->reset_pin, GPIOF_OUT_INIT_LOW,
-					"edt-ft5x06 reset");
-		if (error) {
-			dev_err(&client->dev,
-				"Failed to request GPIO %d as reset pin, error %d\n",
-				tsdata->reset_pin, error);
-			return error;
-		}
 
+	if (tsdata->reset_pin) {
 		msleep(5);
-		gpio_set_value(tsdata->reset_pin, 1);
+		gpiod_set_value_cansleep(tsdata->reset_pin, 1);

So this leaves the reset pin active. How exactly was this tested?

Normally if the output gpio connected to the reset pin is ACTIVE_HIGH then this will take the tsc out of reset since
the reset pin is active low. However, I have a board that has an inverter between the gpio and reset pin. So if I leave
the gpio as ACTIVE_HIGH then the inverter would cause the reset pin to go low which will keep it in reset. So instead
I set the gpio to ACTIVE_LOW which gives me the expected result.

I do not really care about particular board. Assuming that polarity of
the GPIO in DTS is specified correctly the effect of:

	gpiod_set_value_cansleep(tsdata->reset_pin, 1);

is reset pin being _active_, i.e. the chip is staying in reset state.

Setting the reset pin to 1/high will take the tsc out of reset. Setting the pin to 0/low will put the tsc into reset mode.

quoted

By the way, both reset and wake pins are active low according to the
data sheet I found.

Your right. Reset and wake are both active low. However, that part of the code is trying to get the tsc out of reset which is
why its setting the value to 1. Setting the pin to 1 was being done even before my patch.

What has changed was removing the below line which puts the tsc in reset.

error = devm_gpio_request_one(&client->dev, tsdata->reset_pin, GPIOF_OUT_INIT_LOW,"edt-ft5x06 reset");

However, the above line isn't needed since when I request the gpio I already configured it as an output
with a default value of 0.

Sorry accidentally hit reply in this thread instead of reply all. Adding back everyone removed from CC list.

quoted

Thanks.