[PATCH 0/4 v2] Input: zforce - update interrupt handling

[PATCH 0/4 v2] Input: zforce - update interrupt handling

[Dirk Behme]

This is version 2 of the previous sent zforce interrupt patches.

Patch #1 and #2 are new: Patch #1 fixes an issue found while implementing
patch #3. Patch #2 is just a preparation for #3.

Patch #3 implements the review comments of the first version of this
patch about not dropping the interrupt GPIO completely, but making it
optional.

Patch #4 is unchanged compared to the first version and is still under
discussion in

http://article.gmane.org/gmane.linux.kernel.input/44580

It's sent here for completeness and can be handled later depending on the
outcome of the discussion

Best regards

Dirk

[PATCH 1/4] Input: zforce - don't invert the interrupt GPIO

[Dirk Behme]

Commit 2d53809594af ("Input: zforce_ts - convert to use the gpiod
interface") converted this driver to use the gpiod functions. These
functions take the active low property into account, so we don't have
to invert the result of gpiod_get_value_cansleep(). This has been
missed in that commit. Fix it.

Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
---
 drivers/input/touchscreen/zforce_ts.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/zforce_ts.c b/drivers/input/touchscreen/zforce_ts.c
index 32749db..edf01c3 100644
--- a/drivers/input/touchscreen/zforce_ts.c
+++ b/drivers/input/touchscreen/zforce_ts.c
@@ -510,7 +510,7 @@ static irqreturn_t zforce_irq_thread(int irq, void *dev_id)
 	if (!ts->suspending && device_may_wakeup(&client->dev))
 		pm_stay_awake(&client->dev);
 
-	while (!gpiod_get_value_cansleep(ts->gpio_int)) {
+	while (gpiod_get_value_cansleep(ts->gpio_int)) {
 		ret = zforce_read_packet(ts, payload_buffer);
 		if (ret < 0) {
 			dev_err(&client->dev,
-- 
2.3.4

Re: [PATCH 1/4] Input: zforce - don't invert the interrupt GPIO

[Dmitry Torokhov]

On Mon, Jul 20, 2015 at 02:56:34PM +0200, Dirk Behme wrote:
> Commit 2d53809594af ("Input: zforce_ts - convert to use the gpiod
> interface") converted this driver to use the gpiod functions. These
> functions take the active low property into account, so we don't have
> to invert the result of gpiod_get_value_cansleep(). This has been
> missed in that commit. Fix it.
> 
> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>

Applied, thank you.

> ---
>  drivers/input/touchscreen/zforce_ts.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/input/touchscreen/zforce_ts.c b/drivers/input/touchscreen/zforce_ts.c
> index 32749db..edf01c3 100644
> --- a/drivers/input/touchscreen/zforce_ts.c
> +++ b/drivers/input/touchscreen/zforce_ts.c
> @@ -510,7 +510,7 @@ static irqreturn_t zforce_irq_thread(int irq, void *dev_id)
>  	if (!ts->suspending && device_may_wakeup(&client->dev))
>  		pm_stay_awake(&client->dev);
>  
> -	while (!gpiod_get_value_cansleep(ts->gpio_int)) {
> +	while (gpiod_get_value_cansleep(ts->gpio_int)) {
>  		ret = zforce_read_packet(ts, payload_buffer);
>  		if (ret < 0) {
>  			dev_err(&client->dev,
> -- 
> 2.3.4
> 

-- 
Dmitry

[PATCH 2/4] Input: zforce - swap reset and interrupt GPIO

[Dirk Behme]

From: Dirk Behme <dirk.behme@gmail.com>

Swap the order of the reset and interrupt GPIO. This is the preparation
to make the interrupt GPIO optional.

Note that this needs the same change in the device tree. But as mainline
doesn't contain any zforce device tree entries, nothing has to be done
for us, here.

All non-mainline device trees have to be adapted, though.

Signed-off-by: Dirk Behme <dirk.behme@gmail.com>
---
 .../bindings/input/touchscreen/zforce_ts.txt         |  4 ++--
 drivers/input/touchscreen/zforce_ts.c                | 20 ++++++++++----------
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt b/Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt
index 80c37df..a19b508 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt
@@ -23,8 +23,8 @@ Example:
 			interrupts = <2 0>;
 			vdd-supply = <&reg_zforce_vdd>;
 
-			gpios = <&gpio5 6 0>, /* INT */
-				<&gpio5 9 0>; /* RST */
+			gpios = <&gpio5 9 0>, /* RST */
+				<&gpio5 6 0>; /* INT */
 
 			x-size = <800>;
 			y-size = <600>;
diff --git a/drivers/input/touchscreen/zforce_ts.c b/drivers/input/touchscreen/zforce_ts.c
index edf01c3..ac7b661 100644
--- a/drivers/input/touchscreen/zforce_ts.c
+++ b/drivers/input/touchscreen/zforce_ts.c
@@ -754,17 +754,8 @@ static int zforce_probe(struct i2c_client *client,
 	if (!ts)
 		return -ENOMEM;
 
-	/* INT GPIO */
-	ts->gpio_int = devm_gpiod_get_index(&client->dev, NULL, 0, GPIOD_IN);
-	if (IS_ERR(ts->gpio_int)) {
-		ret = PTR_ERR(ts->gpio_int);
-		dev_err(&client->dev,
-			"failed to request interrupt GPIO: %d\n", ret);
-		return ret;
-	}
-
 	/* RST GPIO */
-	ts->gpio_rst = devm_gpiod_get_index(&client->dev, NULL, 1,
+	ts->gpio_rst = devm_gpiod_get_index(&client->dev, NULL, 0,
 					    GPIOD_OUT_HIGH);
 	if (IS_ERR(ts->gpio_rst)) {
 		ret = PTR_ERR(ts->gpio_rst);
@@ -773,6 +764,15 @@ static int zforce_probe(struct i2c_client *client,
 		return ret;
 	}
 
+	/* INT GPIO */
+	ts->gpio_int = devm_gpiod_get_index(&client->dev, NULL, 1, GPIOD_IN);
+	if (IS_ERR(ts->gpio_int)) {
+		ret = PTR_ERR(ts->gpio_int);
+		dev_err(&client->dev,
+			"failed to request interrupt GPIO: %d\n", ret);
+		return ret;
+	}
+
 	ts->reg_vdd = devm_regulator_get_optional(&client->dev, "vdd");
 	if (IS_ERR(ts->reg_vdd)) {
 		ret = PTR_ERR(ts->reg_vdd);
-- 
2.3.4

Re: [PATCH 2/4] Input: zforce - swap reset and interrupt GPIO

[Dmitry Torokhov]

On Mon, Jul 20, 2015 at 02:56:35PM +0200, Dirk Behme wrote:
> From: Dirk Behme <dirk.behme@gmail.com>
> 
> Swap the order of the reset and interrupt GPIO. This is the preparation
> to make the interrupt GPIO optional.
> 
> Note that this needs the same change in the device tree. But as mainline
> doesn't contain any zforce device tree entries, nothing has to be done
> for us, here.
> 
> All non-mainline device trees have to be adapted, though.

No, we can not do that. We will have to switch to using named gpios
(probably call then attn-gpio and reset-gpio respectively and
handle old-style DTSes with unnamed 2-entry gpios list).

Thanks.

> 
> Signed-off-by: Dirk Behme <dirk.behme@gmail.com>
> ---
>  .../bindings/input/touchscreen/zforce_ts.txt         |  4 ++--
>  drivers/input/touchscreen/zforce_ts.c                | 20 ++++++++++----------
>  2 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt b/Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt
> index 80c37df..a19b508 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt
> +++ b/Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt
> @@ -23,8 +23,8 @@ Example:
>  			interrupts = <2 0>;
>  			vdd-supply = <&reg_zforce_vdd>;
>  
> -			gpios = <&gpio5 6 0>, /* INT */
> -				<&gpio5 9 0>; /* RST */
> +			gpios = <&gpio5 9 0>, /* RST */
> +				<&gpio5 6 0>; /* INT */
>  
>  			x-size = <800>;
>  			y-size = <600>;
> diff --git a/drivers/input/touchscreen/zforce_ts.c b/drivers/input/touchscreen/zforce_ts.c
> index edf01c3..ac7b661 100644
> --- a/drivers/input/touchscreen/zforce_ts.c
> +++ b/drivers/input/touchscreen/zforce_ts.c
> @@ -754,17 +754,8 @@ static int zforce_probe(struct i2c_client *client,
>  	if (!ts)
>  		return -ENOMEM;
>  
> -	/* INT GPIO */
> -	ts->gpio_int = devm_gpiod_get_index(&client->dev, NULL, 0, GPIOD_IN);
> -	if (IS_ERR(ts->gpio_int)) {
> -		ret = PTR_ERR(ts->gpio_int);
> -		dev_err(&client->dev,
> -			"failed to request interrupt GPIO: %d\n", ret);
> -		return ret;
> -	}
> -
>  	/* RST GPIO */
> -	ts->gpio_rst = devm_gpiod_get_index(&client->dev, NULL, 1,
> +	ts->gpio_rst = devm_gpiod_get_index(&client->dev, NULL, 0,
>  					    GPIOD_OUT_HIGH);
>  	if (IS_ERR(ts->gpio_rst)) {
>  		ret = PTR_ERR(ts->gpio_rst);
> @@ -773,6 +764,15 @@ static int zforce_probe(struct i2c_client *client,
>  		return ret;
>  	}
>  
> +	/* INT GPIO */
> +	ts->gpio_int = devm_gpiod_get_index(&client->dev, NULL, 1, GPIOD_IN);
> +	if (IS_ERR(ts->gpio_int)) {
> +		ret = PTR_ERR(ts->gpio_int);
> +		dev_err(&client->dev,
> +			"failed to request interrupt GPIO: %d\n", ret);
> +		return ret;
> +	}
> +
>  	ts->reg_vdd = devm_regulator_get_optional(&client->dev, "vdd");
>  	if (IS_ERR(ts->reg_vdd)) {
>  		ret = PTR_ERR(ts->reg_vdd);
> -- 
> 2.3.4
> 

-- 
Dmitry

Re: [PATCH 2/4] Input: zforce - swap reset and interrupt GPIO

[Dirk Behme]

On 24.07.2015 23:32, Dmitry Torokhov wrote:
> On Mon, Jul 20, 2015 at 02:56:35PM +0200, Dirk Behme wrote:
>> From: Dirk Behme <dirk.behme@gmail.com>
>>
>> Swap the order of the reset and interrupt GPIO. This is the preparation
>> to make the interrupt GPIO optional.
>>
>> Note that this needs the same change in the device tree. But as mainline
>> doesn't contain any zforce device tree entries, nothing has to be done
>> for us, here.
>>
>> All non-mainline device trees have to be adapted, though.
>
> No, we can not do that.


The proposal to make the interrupt GPIO optional and not completely 
drop it came from you. Any other proposal how you like to get this 
implemented? With completely dropping the interrupt GPIO we would 
avoid this.


> We will have to switch to using named gpios
> (probably call then attn-gpio and reset-gpio respectively and
> handle old-style DTSes with unnamed 2-entry gpios list).


Just to understand correctly: You want the driver to support two GPIOs 
DT styles? One with unnamed 2-entry gpios list, where the interrupt 
GPIO can't be optional, and one with two named GPIOs, where the 
attn-gpio can be optional?

Btw, what does 'attn' stand for? I'd name it int-gpio.


Best regards

Dirk


>>
>> Signed-off-by: Dirk Behme <dirk.behme@gmail.com>
>> ---
>>   .../bindings/input/touchscreen/zforce_ts.txt         |  4 ++--
>>   drivers/input/touchscreen/zforce_ts.c                | 20 ++++++++++----------
>>   2 files changed, 12 insertions(+), 12 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt b/Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt
>> index 80c37df..a19b508 100644
>> --- a/Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt
>> +++ b/Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt
>> @@ -23,8 +23,8 @@ Example:
>>   			interrupts = <2 0>;
>>   			vdd-supply = <&reg_zforce_vdd>;
>>
>> -			gpios = <&gpio5 6 0>, /* INT */
>> -				<&gpio5 9 0>; /* RST */
>> +			gpios = <&gpio5 9 0>, /* RST */
>> +				<&gpio5 6 0>; /* INT */
>>
>>   			x-size = <800>;
>>   			y-size = <600>;
>> diff --git a/drivers/input/touchscreen/zforce_ts.c b/drivers/input/touchscreen/zforce_ts.c
>> index edf01c3..ac7b661 100644
>> --- a/drivers/input/touchscreen/zforce_ts.c
>> +++ b/drivers/input/touchscreen/zforce_ts.c
>> @@ -754,17 +754,8 @@ static int zforce_probe(struct i2c_client *client,
>>   	if (!ts)
>>   		return -ENOMEM;
>>
>> -	/* INT GPIO */
>> -	ts->gpio_int = devm_gpiod_get_index(&client->dev, NULL, 0, GPIOD_IN);
>> -	if (IS_ERR(ts->gpio_int)) {
>> -		ret = PTR_ERR(ts->gpio_int);
>> -		dev_err(&client->dev,
>> -			"failed to request interrupt GPIO: %d\n", ret);
>> -		return ret;
>> -	}
>> -
>>   	/* RST GPIO */
>> -	ts->gpio_rst = devm_gpiod_get_index(&client->dev, NULL, 1,
>> +	ts->gpio_rst = devm_gpiod_get_index(&client->dev, NULL, 0,
>>   					    GPIOD_OUT_HIGH);
>>   	if (IS_ERR(ts->gpio_rst)) {
>>   		ret = PTR_ERR(ts->gpio_rst);
>> @@ -773,6 +764,15 @@ static int zforce_probe(struct i2c_client *client,
>>   		return ret;
>>   	}
>>
>> +	/* INT GPIO */
>> +	ts->gpio_int = devm_gpiod_get_index(&client->dev, NULL, 1, GPIOD_IN);
>> +	if (IS_ERR(ts->gpio_int)) {
>> +		ret = PTR_ERR(ts->gpio_int);
>> +		dev_err(&client->dev,
>> +			"failed to request interrupt GPIO: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>>   	ts->reg_vdd = devm_regulator_get_optional(&client->dev, "vdd");
>>   	if (IS_ERR(ts->reg_vdd)) {
>>   		ret = PTR_ERR(ts->reg_vdd);
>> --
>> 2.3.4
>>
>

Re: [PATCH 2/4] Input: zforce - swap reset and interrupt GPIO

[Dirk Behme]

On 25.07.2015 07:19, Dirk Behme wrote:
> On 24.07.2015 23:32, Dmitry Torokhov wrote:
>> On Mon, Jul 20, 2015 at 02:56:35PM +0200, Dirk Behme wrote:
>>> From: Dirk Behme <dirk.behme@gmail.com>
>>>
>>> Swap the order of the reset and interrupt GPIO. This is the preparation
>>> to make the interrupt GPIO optional.
>>>
>>> Note that this needs the same change in the device tree. But as mainline
>>> doesn't contain any zforce device tree entries, nothing has to be done
>>> for us, here.
>>>
>>> All non-mainline device trees have to be adapted, though.
>>
>> No, we can not do that.
>
>
> The proposal to make the interrupt GPIO optional and not completely drop
> it came from you. Any other proposal how you like to get this
> implemented? With completely dropping the interrupt GPIO we would avoid
> this.
>
>
>> We will have to switch to using named gpios
>> (probably call then attn-gpio and reset-gpio respectively and
>> handle old-style DTSes with unnamed 2-entry gpios list).
>
>
> Just to understand correctly: You want the driver to support two GPIOs
> DT styles? One with unnamed 2-entry gpios list, where the interrupt GPIO
> can't be optional, and one with two named GPIOs, where the attn-gpio can
> be optional?


Are you talking about anything like below [1] ?


Best regards

Dirk

[1]

Subject: [PATCH] Input: zforce - make the interrupt GPIO optional

Add support for hardware which uses an I2C Serializer / Deserializer
(SerDes) to communicate with the zFroce touch driver. In this case the
SerDes will be configured as an interrupt controller and the zForce driver
will have no access to poll the GPIO line.

To support this, we add two dedicated new GPIOs in the device tree:
rst-gpio and int-gpio. With the int-gpio being optional, then.

To not break the existing device trees, the index based 'gpios' entries
are still supported, but marked as depreciated.

With this, if the interrupt GPIO is available, either via the old or new
device tree style, the while loop will read and handle the packets as long
as the GPIO indicates that the interrupt is asserted (existing, unchanged
driver behavior).

If the interrupt GPIO isn't available, i.e. not configured via the new
device tree stlye, we are falling back to one read per ISR invocation
(new behavior to support the SerDes).

Note that the gpiod functions help to handle the optional GPIO:
devm_gpiod_get_index_optional() will return NULL in case the interrupt
GPIO isn't available. And gpiod_get_value_cansleep() does cover this, too,
by returning 0 in this case.

Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
---
  .../bindings/input/touchscreen/zforce_ts.txt       | 15 ++++++--
  drivers/input/touchscreen/zforce_ts.c              | 42 
+++++++++++++++++++++-
  2 files changed, 53 insertions(+), 4 deletions(-)

diff --git 
a/Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt 
b/Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt
index 80c37df..1957877 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt
@@ -4,12 +4,16 @@ Required properties:
  - compatible: must be "neonode,zforce"
  - reg: I2C address of the chip
  - interrupts: interrupt to which the chip is connected
-- gpios: gpios the chip is connected to
-  first one is the interrupt gpio and second one the reset gpio
+- gpios:
+  - rst-gpio: reset gpio the chip is connected to
+  or
+  - gpios (old style, deprecated): gpios the chip is connected to
+    first one is the interrupt gpio and second one the reset gpio
  - x-size: horizontal resolution of touchscreen
  - y-size: vertical resolution of touchscreen

  Optional properties:
+- int-gpio : interrupt gpio the chip is connected to
  - vdd-supply: Regulator controlling the controller supply

  Example:
@@ -23,9 +27,14 @@ Example:
  			interrupts = <2 0>;
  			vdd-supply = <&reg_zforce_vdd>;

+			rst-gpio = <&gpio5 9 0>; /* RST */
+			int-gpio = <&gpio5 6 0>; /* INT, optional */
+
+		/* or deprecated, use new style above for
+		   new device trees:
  			gpios = <&gpio5 6 0>, /* INT */
  				<&gpio5 9 0>; /* RST */
-
+		*/
  			x-size = <800>;
  			y-size = <600>;
  		};
diff --git a/drivers/input/touchscreen/zforce_ts.c 
b/drivers/input/touchscreen/zforce_ts.c
index edf01c3..1e3b1c2 100644
--- a/drivers/input/touchscreen/zforce_ts.c
+++ b/drivers/input/touchscreen/zforce_ts.c
@@ -494,6 +494,7 @@ static irqreturn_t zforce_irq_thread(int irq, void 
*dev_id)
  	int ret;
  	u8 payload_buffer[FRAME_MAXSIZE];
  	u8 *payload;
+	int run = 1;

  	/*
  	 * When still suspended, return.
@@ -510,7 +511,18 @@ static irqreturn_t zforce_irq_thread(int irq, void 
*dev_id)
  	if (!ts->suspending && device_may_wakeup(&client->dev))
  		pm_stay_awake(&client->dev);

-	while (gpiod_get_value_cansleep(ts->gpio_int)) {
+	while (run) {
+		/*
+		 * Exit the loop if either
+		 * - the optional interrupt GPIO isn't specified
+		 *   (there is only one packet read per ISR invocation, then)
+		 * or
+		 * - the GPIO isn't active any more
+		 *   (packet read until the level GPIO indicates that there is
+		 *    no IRQ any more)
+		 */
+		run = gpiod_get_value_cansleep(ts->gpio_int);
+
  		ret = zforce_read_packet(ts, payload_buffer);
  		if (ret < 0) {
  			dev_err(&client->dev,
@@ -754,6 +766,33 @@ static int zforce_probe(struct i2c_client *client,
  	if (!ts)
  		return -ENOMEM;

+	/*
+	 * The reset GPIO isn't optional, but we might get it
+	 * via the old style DT entries below, too. So it's
+	 * not an error if we don't get it here. Therefore use
+	 * devm_gpiod_get_optional() here.
+	 */
+	ts->gpio_rst = devm_gpiod_get_optional(&client->dev, "rst",
+					       GPIOD_OUT_HIGH);
+	if (IS_ERR(ts->gpio_rst)) {
+		ret = PTR_ERR(ts->gpio_rst);
+		dev_err(&client->dev,
+			"failed to request reset GPIO: %d\n", ret);
+		return ret;
+	}
+
+	ts->gpio_int = devm_gpiod_get_optional(&client->dev, "int", GPIOD_IN);
+	if (IS_ERR(ts->gpio_int)) {
+		ret = PTR_ERR(ts->gpio_int);
+		dev_err(&client->dev,
+			"failed to request interrupt GPIO: %d\n", ret);
+		return ret;
+	}
+
+	/* Skip the old style GPIO if we have the new one */
+	if (ts->gpio_rst)
+		goto skip;
+
  	/* INT GPIO */
  	ts->gpio_int = devm_gpiod_get_index(&client->dev, NULL, 0, GPIOD_IN);
  	if (IS_ERR(ts->gpio_int)) {
@@ -773,6 +812,7 @@ static int zforce_probe(struct i2c_client *client,
  		return ret;
  	}

+skip:
  	ts->reg_vdd = devm_regulator_get_optional(&client->dev, "vdd");
  	if (IS_ERR(ts->reg_vdd)) {
  		ret = PTR_ERR(ts->reg_vdd);

Re: [PATCH 2/4] Input: zforce - swap reset and interrupt GPIO

[Dmitry Torokhov]

Hi Dirk,

On Mon, Jul 27, 2015 at 10:12:43AM +0200, Dirk Behme wrote:
> On 25.07.2015 07:19, Dirk Behme wrote:
> >On 24.07.2015 23:32, Dmitry Torokhov wrote:
> >>On Mon, Jul 20, 2015 at 02:56:35PM +0200, Dirk Behme wrote:
> >>>From: Dirk Behme <dirk.behme@gmail.com>
> >>>
> >>>Swap the order of the reset and interrupt GPIO. This is the preparation
> >>>to make the interrupt GPIO optional.
> >>>
> >>>Note that this needs the same change in the device tree. But as mainline
> >>>doesn't contain any zforce device tree entries, nothing has to be done
> >>>for us, here.
> >>>
> >>>All non-mainline device trees have to be adapted, though.
> >>
> >>No, we can not do that.
> >
> >
> >The proposal to make the interrupt GPIO optional and not completely drop
> >it came from you. Any other proposal how you like to get this
> >implemented? With completely dropping the interrupt GPIO we would avoid
> >this.
> >
> >
> >>We will have to switch to using named gpios
> >>(probably call then attn-gpio and reset-gpio respectively and
> >>handle old-style DTSes with unnamed 2-entry gpios list).
> >
> >
> >Just to understand correctly: You want the driver to support two GPIOs
> >DT styles? One with unnamed 2-entry gpios list, where the interrupt GPIO
> >can't be optional, and one with two named GPIOs, where the attn-gpio can
> >be optional?
> 
> 
> Are you talking about anything like below [1] ?

Yes.

> 
> 
> Best regards
> 
> Dirk
> 
> [1]
> 
> Subject: [PATCH] Input: zforce - make the interrupt GPIO optional
> 
> Add support for hardware which uses an I2C Serializer / Deserializer
> (SerDes) to communicate with the zFroce touch driver. In this case the
> SerDes will be configured as an interrupt controller and the zForce driver
> will have no access to poll the GPIO line.
> 
> To support this, we add two dedicated new GPIOs in the device tree:
> rst-gpio and int-gpio. With the int-gpio being optional, then.
> 
> To not break the existing device trees, the index based 'gpios' entries
> are still supported, but marked as depreciated.
> 
> With this, if the interrupt GPIO is available, either via the old or new
> device tree style, the while loop will read and handle the packets as long
> as the GPIO indicates that the interrupt is asserted (existing, unchanged
> driver behavior).
> 
> If the interrupt GPIO isn't available, i.e. not configured via the new
> device tree stlye, we are falling back to one read per ISR invocation
> (new behavior to support the SerDes).
> 
> Note that the gpiod functions help to handle the optional GPIO:
> devm_gpiod_get_index_optional() will return NULL in case the interrupt
> GPIO isn't available. And gpiod_get_value_cansleep() does cover this, too,
> by returning 0 in this case.
> 
> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
> ---
>  .../bindings/input/touchscreen/zforce_ts.txt       | 15 ++++++--
>  drivers/input/touchscreen/zforce_ts.c              | 42
> +++++++++++++++++++++-
>  2 files changed, 53 insertions(+), 4 deletions(-)
> 
> diff --git
> a/Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt
> b/Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt
> index 80c37df..1957877 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt
> +++ b/Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt
> @@ -4,12 +4,16 @@ Required properties:
>  - compatible: must be "neonode,zforce"
>  - reg: I2C address of the chip
>  - interrupts: interrupt to which the chip is connected
> -- gpios: gpios the chip is connected to
> -  first one is the interrupt gpio and second one the reset gpio
> +- gpios:
> +  - rst-gpio: reset gpio the chip is connected to
> +  or
> +  - gpios (old style, deprecated): gpios the chip is connected to
> +    first one is the interrupt gpio and second one the reset gpio

We do not need to keep documentation for deprecated style, so just
remove reference to old "gpios" property,

>  - x-size: horizontal resolution of touchscreen
>  - y-size: vertical resolution of touchscreen
> 
>  Optional properties:
> +- int-gpio : interrupt gpio the chip is connected to
>  - vdd-supply: Regulator controlling the controller supply
> 
>  Example:
> @@ -23,9 +27,14 @@ Example:
>  			interrupts = <2 0>;
>  			vdd-supply = <&reg_zforce_vdd>;
> 
> +			rst-gpio = <&gpio5 9 0>; /* RST */
> +			int-gpio = <&gpio5 6 0>; /* INT, optional */
> +
> +		/* or deprecated, use new style above for
> +		   new device trees:
>  			gpios = <&gpio5 6 0>, /* INT */
>  				<&gpio5 9 0>; /* RST */

Here as well.

Thanks.

-- 
Dmitry

Re: [PATCH 2/4] Input: zforce - swap reset and interrupt GPIO

[Dirk Behme]

On 27.07.2015 23:17, Dmitry Torokhov wrote:
> Hi Dirk,
>
> On Mon, Jul 27, 2015 at 10:12:43AM +0200, Dirk Behme wrote:
>> On 25.07.2015 07:19, Dirk Behme wrote:
>>> On 24.07.2015 23:32, Dmitry Torokhov wrote:
>>>> On Mon, Jul 20, 2015 at 02:56:35PM +0200, Dirk Behme wrote:
>>>>> From: Dirk Behme <dirk.behme@gmail.com>
>>>>>
>>>>> Swap the order of the reset and interrupt GPIO. This is the preparation
>>>>> to make the interrupt GPIO optional.
>>>>>
>>>>> Note that this needs the same change in the device tree. But as mainline
>>>>> doesn't contain any zforce device tree entries, nothing has to be done
>>>>> for us, here.
>>>>>
>>>>> All non-mainline device trees have to be adapted, though.
>>>>
>>>> No, we can not do that.
>>>
>>>
>>> The proposal to make the interrupt GPIO optional and not completely drop
>>> it came from you. Any other proposal how you like to get this
>>> implemented? With completely dropping the interrupt GPIO we would avoid
>>> this.
>>>
>>>
>>>> We will have to switch to using named gpios
>>>> (probably call then attn-gpio and reset-gpio respectively and
>>>> handle old-style DTSes with unnamed 2-entry gpios list).
>>>
>>>
>>> Just to understand correctly: You want the driver to support two GPIOs
>>> DT styles? One with unnamed 2-entry gpios list, where the interrupt GPIO
>>> can't be optional, and one with two named GPIOs, where the attn-gpio can
>>> be optional?
>>
>>
>> Are you talking about anything like below [1] ?
>
> Yes.
>
>>
>>
>> Best regards
>>
>> Dirk
>>
>> [1]
>>
>> Subject: [PATCH] Input: zforce - make the interrupt GPIO optional
>>
>> Add support for hardware which uses an I2C Serializer / Deserializer
>> (SerDes) to communicate with the zFroce touch driver. In this case the
>> SerDes will be configured as an interrupt controller and the zForce driver
>> will have no access to poll the GPIO line.
>>
>> To support this, we add two dedicated new GPIOs in the device tree:
>> rst-gpio and int-gpio. With the int-gpio being optional, then.
>>
>> To not break the existing device trees, the index based 'gpios' entries
>> are still supported, but marked as depreciated.
>>
>> With this, if the interrupt GPIO is available, either via the old or new
>> device tree style, the while loop will read and handle the packets as long
>> as the GPIO indicates that the interrupt is asserted (existing, unchanged
>> driver behavior).
>>
>> If the interrupt GPIO isn't available, i.e. not configured via the new
>> device tree stlye, we are falling back to one read per ISR invocation
>> (new behavior to support the SerDes).
>>
>> Note that the gpiod functions help to handle the optional GPIO:
>> devm_gpiod_get_index_optional() will return NULL in case the interrupt
>> GPIO isn't available. And gpiod_get_value_cansleep() does cover this, too,
>> by returning 0 in this case.
>>
>> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
>> ---
>>   .../bindings/input/touchscreen/zforce_ts.txt       | 15 ++++++--
>>   drivers/input/touchscreen/zforce_ts.c              | 42
>> +++++++++++++++++++++-
>>   2 files changed, 53 insertions(+), 4 deletions(-)
>>
>> diff --git
>> a/Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt
>> b/Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt
>> index 80c37df..1957877 100644
>> --- a/Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt
>> +++ b/Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt
>> @@ -4,12 +4,16 @@ Required properties:
>>   - compatible: must be "neonode,zforce"
>>   - reg: I2C address of the chip
>>   - interrupts: interrupt to which the chip is connected
>> -- gpios: gpios the chip is connected to
>> -  first one is the interrupt gpio and second one the reset gpio
>> +- gpios:
>> +  - rst-gpio: reset gpio the chip is connected to
>> +  or
>> +  - gpios (old style, deprecated): gpios the chip is connected to
>> +    first one is the interrupt gpio and second one the reset gpio
>
> We do not need to keep documentation for deprecated style, so just
> remove reference to old "gpios" property,
>
>>   - x-size: horizontal resolution of touchscreen
>>   - y-size: vertical resolution of touchscreen
>>
>>   Optional properties:
>> +- int-gpio : interrupt gpio the chip is connected to
>>   - vdd-supply: Regulator controlling the controller supply
>>
>>   Example:
>> @@ -23,9 +27,14 @@ Example:
>>   			interrupts = <2 0>;
>>   			vdd-supply = <&reg_zforce_vdd>;
>>
>> +			rst-gpio = <&gpio5 9 0>; /* RST */
>> +			int-gpio = <&gpio5 6 0>; /* INT, optional */
>> +
>> +		/* or deprecated, use new style above for
>> +		   new device trees:
>>   			gpios = <&gpio5 6 0>, /* INT */
>>   				<&gpio5 9 0>; /* RST */
>
> Here as well.

Done:

http://article.gmane.org/gmane.linux.kernel.input/44836

Thanks!

Best regards

Dirk

Re: [PATCH 2/4] Input: zforce - swap reset and interrupt GPIO

[Dmitry Torokhov]

On Sat, Jul 25, 2015 at 07:19:04AM +0200, Dirk Behme wrote:
> 
> Btw, what does 'attn' stand for?

"attention"

> I'd name it int-gpio.

Works as well.

Thanks.

-- 
Dmitry

[PATCH 3/4 v2] Input: zforce - make the interrupt GPIO optional

[Dirk Behme]

Add support for hardware which uses an I2C Serializer / Deserializer
(SerDes) to communicate with the zFroce touch driver. In this case the
SerDes will be configured as an interrupt controller and the zForce driver
will have no access to poll the GPIO line.

To support this, we make the interrupt GPIO optional and use it only if
it is specified.

With this, if the GPIO is available, the while loop will read and handle
the packets as long as the GPIO indicates that the interrupt is asserted
(existing, unchanged driver behavior).

If the GPIO isn't available, we are falling back to one read per ISR
invocation (new behavior to support the SerDes).

Note that the gpiod functions help to handle the optional GPIO:
devm_gpiod_get_index_optional() will return NULL in case the interrupt
GPIO isn't available. And gpiod_get_value_cansleep() does cover this, too,
by returning 0 in this case.

Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
---
Changes in v2: Instead of dropping the interrupt GPIO completely, make
               it optional.

 .../bindings/input/touchscreen/zforce_ts.txt          |  2 +-
 drivers/input/touchscreen/zforce_ts.c                 | 19 ++++++++++++++++---
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt b/Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt
index a19b508..5843957 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt
@@ -24,7 +24,7 @@ Example:
 			vdd-supply = <&reg_zforce_vdd>;
 
 			gpios = <&gpio5 9 0>, /* RST */
-				<&gpio5 6 0>; /* INT */
+				<&gpio5 6 0>; /* INT, optional */
 
 			x-size = <800>;
 			y-size = <600>;
diff --git a/drivers/input/touchscreen/zforce_ts.c b/drivers/input/touchscreen/zforce_ts.c
index ac7b661..fdd00b1 100644
--- a/drivers/input/touchscreen/zforce_ts.c
+++ b/drivers/input/touchscreen/zforce_ts.c
@@ -494,6 +494,7 @@ static irqreturn_t zforce_irq_thread(int irq, void *dev_id)
 	int ret;
 	u8 payload_buffer[FRAME_MAXSIZE];
 	u8 *payload;
+	int run = 1;
 
 	/*
 	 * When still suspended, return.
@@ -510,7 +511,18 @@ static irqreturn_t zforce_irq_thread(int irq, void *dev_id)
 	if (!ts->suspending && device_may_wakeup(&client->dev))
 		pm_stay_awake(&client->dev);
 
-	while (gpiod_get_value_cansleep(ts->gpio_int)) {
+	while (run) {
+		/*
+		 * Exit the loop if either
+		 * - the optional interrupt GPIO isn't specified
+		 *   (there is only one packet read per ISR invocation, then)
+		 * or
+		 * - the GPIO isn't active any more
+		 *   (packet read until the level GPIO indicates that there is
+		 *    no IRQ any more)
+		 */
+		run = gpiod_get_value_cansleep(ts->gpio_int);
+
 		ret = zforce_read_packet(ts, payload_buffer);
 		if (ret < 0) {
 			dev_err(&client->dev,
@@ -764,8 +776,9 @@ static int zforce_probe(struct i2c_client *client,
 		return ret;
 	}
 
-	/* INT GPIO */
-	ts->gpio_int = devm_gpiod_get_index(&client->dev, NULL, 1, GPIOD_IN);
+	/* INT GPIO, optional, can return NULL if not used */
+	ts->gpio_int = devm_gpiod_get_index_optional(&client->dev, NULL, 1,
+						     GPIOD_IN);
 	if (IS_ERR(ts->gpio_int)) {
 		ret = PTR_ERR(ts->gpio_int);
 		dev_err(&client->dev,
-- 
2.3.4

[PATCH 4/4 v2] Input: zforce - enable irq only if we are ready to process it

[Dirk Behme]

From: Oleksij Rempel <external.Oleksij.Rempel@de.bosch.com>

If zforce is not ready to process the interrupt, the touchscreen will
be lost forever. Make sure we enable the interrupt only if processing
is ready.

Signed-off-by: Oleksij Rempel <external.Oleksij.Rempel@de.bosch.com>
Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
---
Changes in v2: No changes, just re-send

 drivers/input/touchscreen/zforce_ts.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/zforce_ts.c b/drivers/input/touchscreen/zforce_ts.c
index fdd00b1..fab1bb0 100644
--- a/drivers/input/touchscreen/zforce_ts.c
+++ b/drivers/input/touchscreen/zforce_ts.c
@@ -22,6 +22,7 @@
 #include <linux/slab.h>
 #include <linux/input.h>
 #include <linux/interrupt.h>
+#include <linux/irq.h>
 #include <linux/i2c.h>
 #include <linux/delay.h>
 #include <linux/gpio/consumer.h>
@@ -755,6 +756,7 @@ static int zforce_probe(struct i2c_client *client,
 	struct zforce_ts *ts;
 	struct input_dev *input_dev;
 	int ret;
+	unsigned int irq;
 
 	if (!pdata) {
 		pdata = zforce_parse_dt(&client->dev);
@@ -859,6 +861,7 @@ static int zforce_probe(struct i2c_client *client,
 
 	init_completion(&ts->command_done);
 
+	irq = client->irq;
 	/*
 	 * The zforce pulls the interrupt low when it has data ready.
 	 * After it is triggered the isr thread runs until all the available
@@ -866,7 +869,8 @@ static int zforce_probe(struct i2c_client *client,
 	 * Therefore we can trigger the interrupt anytime it is low and do
 	 * not need to limit it to the interrupt edge.
 	 */
-	ret = devm_request_threaded_irq(&client->dev, client->irq,
+	irq_set_status_flags(irq, IRQ_NOAUTOEN);
+	ret = devm_request_threaded_irq(&client->dev, irq,
 					zforce_irq, zforce_irq_thread,
 					IRQF_TRIGGER_LOW | IRQF_ONESHOT,
 					input_dev->name, ts);
@@ -879,6 +883,7 @@ static int zforce_probe(struct i2c_client *client,
 
 	/* let the controller boot */
 	zforce_reset_deassert(ts);
+	enable_irq(irq);
 
 	ts->command_waiting = NOTIFICATION_BOOTCOMPLETE;
 	if (wait_for_completion_timeout(&ts->command_done, WAIT_TIMEOUT) == 0)
-- 
2.3.4

Re: [PATCH 4/4 v2] Input: zforce - enable irq only if we are ready to process it

[Dirk Behme]

On 20.07.2015 14:56, Dirk Behme wrote:
> From: Oleksij Rempel <external.Oleksij.Rempel@de.bosch.com>
>
> If zforce is not ready to process the interrupt, the touchscreen will
> be lost forever. Make sure we enable the interrupt only if processing
> is ready.


Talking with Oleksij about the discussion in v1 of this patch I think 
the conclusion is that we can drop this patch from this series.

We'll revert this change in our local version, too, and see if it has 
any consequences. If so, we might come back, maybe with a better 
explanation, then ;)

Just for the logs, what's left open from this series are patch 2/4 and 
3/4, then.

Best regards

Dirk


> Signed-off-by: Oleksij Rempel <external.Oleksij.Rempel@de.bosch.com>
> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
> ---
> Changes in v2: No changes, just re-send
>
>   drivers/input/touchscreen/zforce_ts.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/input/touchscreen/zforce_ts.c b/drivers/input/touchscreen/zforce_ts.c
> index fdd00b1..fab1bb0 100644
> --- a/drivers/input/touchscreen/zforce_ts.c
> +++ b/drivers/input/touchscreen/zforce_ts.c
> @@ -22,6 +22,7 @@
>   #include <linux/slab.h>
>   #include <linux/input.h>
>   #include <linux/interrupt.h>
> +#include <linux/irq.h>
>   #include <linux/i2c.h>
>   #include <linux/delay.h>
>   #include <linux/gpio/consumer.h>
> @@ -755,6 +756,7 @@ static int zforce_probe(struct i2c_client *client,
>   	struct zforce_ts *ts;
>   	struct input_dev *input_dev;
>   	int ret;
> +	unsigned int irq;
>
>   	if (!pdata) {
>   		pdata = zforce_parse_dt(&client->dev);
> @@ -859,6 +861,7 @@ static int zforce_probe(struct i2c_client *client,
>
>   	init_completion(&ts->command_done);
>
> +	irq = client->irq;
>   	/*
>   	 * The zforce pulls the interrupt low when it has data ready.
>   	 * After it is triggered the isr thread runs until all the available
> @@ -866,7 +869,8 @@ static int zforce_probe(struct i2c_client *client,
>   	 * Therefore we can trigger the interrupt anytime it is low and do
>   	 * not need to limit it to the interrupt edge.
>   	 */
> -	ret = devm_request_threaded_irq(&client->dev, client->irq,
> +	irq_set_status_flags(irq, IRQ_NOAUTOEN);
> +	ret = devm_request_threaded_irq(&client->dev, irq,
>   					zforce_irq, zforce_irq_thread,
>   					IRQF_TRIGGER_LOW | IRQF_ONESHOT,
>   					input_dev->name, ts);
> @@ -879,6 +883,7 @@ static int zforce_probe(struct i2c_client *client,
>
>   	/* let the controller boot */
>   	zforce_reset_deassert(ts);
> +	enable_irq(irq);
>
>   	ts->command_waiting = NOTIFICATION_BOOTCOMPLETE;
>   	if (wait_for_completion_timeout(&ts->command_done, WAIT_TIMEOUT) == 0)
>