[PATCH v1 40/63] input: atmel_mxt_ts: Add Missing Delay for reset handling of Atmel touch panel controller in detachable displays.

[PATCH v1 40/63] input: atmel_mxt_ts: Add Missing Delay for reset handling of Atmel touch panel controller in detachable displays.

[Jiada Wang]

From: keerthikumarp <Keerthikumar.Padmanabha@in.bosch.com>

In case of attached display, the touchpanel reset is controlled
via imx gpio's from  atmel driver and the delay between
touchpanel reset and the time at which the chip becomes capable to
communicate with the host processor, has be taken care.

However in case of detachable displays, the touchpanel reset is
controlled via a deserializer gpio which is triggered just before
the atmel driver is probed.The delay between touchpanel reset and
the time at which the chip becomes capable to communicate (as
specified in datasheet) was not being accounted for. This patch
introduces that delay.

Signed-off-by: keerthikumarp <Keerthikumar.Padmanabha@in.bosch.com>
Signed-off-by: George G. Davis <george_davis@mentor.com>
Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 3b9544c0a209..bc94adec6631 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -4407,6 +4407,10 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		msleep(MXT_RESET_GPIO_TIME);
 		gpiod_set_value(data->reset_gpio, 1);
 		msleep(MXT_RESET_INVALID_CHG);
+	} else {
+		dev_dbg(&client->dev,
+			"atmel reset pin not found in device tree");
+		msleep(MXT_RESET_TIME);
 	}
 
 	error = sysfs_create_group(&client->dev.kobj, &mxt_fw_attr_group);
-- 
2.19.2

[PATCH v1 41/63] Input: touchscreen: Atmel: Enable IRQ_DISABLE_UNLAZY flag for interrupt

[Jiada Wang]

From: Bhuvanesh Surachari <bhuvanesh_surachari@mentor.com>

The de-/serializer driver has defined only irq_mask "ds90ub927_irq_mask" and
irq_unmask "ds90ub927_irq_unmask" callback functions. And de-/serializer
driver doesn't implement the irq_disable and irq_enable callback functions.
Hence inorder to invoke irq_mask callback function when disable_irq_nosync is
called the IRQ_DISABLE_UNLAZY interrupt flag should be set. If not the
disable_irq_nosync will just increment the depth field in the irq
descriptor only once as shown below.

disable_irq_nosync
 __disable_irq_nosync
  __disable_irq (desc->depth++)
   irq_disable
    if irq_disable present -----------> if IRQ_DISABLE_UNLAZYflag set
             |                  no                  |
         yes |                                  yes |
             |                                      |
     desc->irq_data.chip->irq_disable   desc->irq_data.chip->irq_unmask
                                         (ds90ub927_irq_mask)
                                          disable_irq
                                           __disable_irq_nosync
                                            __disable_irq
(desc->depth++)
But the enable_irq will try to decrement the depth field twice which generates
the backtrace stating "Unbalanced enable for irq 293". This is because there is
no IRQ_DISABLE_UNLAZY flag check while calling irq_unmask callback function
of the "ds90ub927_irq_unmask" de-/serializer via enable_irq.

enable_irq
 __enable_irq (desc->depth--)
  irq_enable
   if irq_enable present -------------> desc->irq_data.chip->irq_unmask
              |                no        (ds90ub927_irq_unmask)
          yes |                            enable_irq
              |                             __enable_irq (desc->depth--)
    (desc->irq_data.chip->irq_enable)

Signed-off-by: Bhuvanesh Surachari <bhuvanesh_surachari@mentor.com>
Signed-off-by: George G. Davis <george_davis@mentor.com>
Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index bc94adec6631..c6ba061098c0 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -4349,6 +4349,8 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	snprintf(data->phys, sizeof(data->phys), "i2c-%u-%04x/input0",
 		 client->adapter->nr, client->addr);
 
+	irq_set_status_flags(client->irq, IRQ_DISABLE_UNLAZY);
+
 	data->client = client;
 	i2c_set_clientdata(client, data);
 
@@ -4434,6 +4436,8 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		sysfs_remove_link(&client->dev.kobj, "reset");
 		gpiod_unexport(data->reset_gpio);
 	}
+	if (data->irq)
+		irq_clear_status_flags(data->irq, IRQ_DISABLE_UNLAZY);
 	return error;
 }
 
@@ -4449,6 +4453,8 @@ static int mxt_remove(struct i2c_client *client)
 	}
 	mxt_debug_msg_remove(data);
 	mxt_sysfs_remove(data);
+	if (data->irq)
+		irq_clear_status_flags(data->irq, IRQ_DISABLE_UNLAZY);
 	mxt_free_input_device(data);
 	mxt_free_object_table(data);
 
-- 
2.19.2

Re: [PATCH v1 41/63] Input: touchscreen: Atmel: Enable IRQ_DISABLE_UNLAZY flag for interrupt

[Dmitry Torokhov]

On Fri, Aug 16, 2019 at 05:35:36PM +0900, Jiada Wang wrote:
> From: Bhuvanesh Surachari <bhuvanesh_surachari@mentor.com>
> 
> The de-/serializer driver has defined only irq_mask "ds90ub927_irq_mask" and
> irq_unmask "ds90ub927_irq_unmask" callback functions. And de-/serializer
> driver doesn't implement the irq_disable and irq_enable callback functions.
> Hence inorder to invoke irq_mask callback function when disable_irq_nosync is
> called the IRQ_DISABLE_UNLAZY interrupt flag should be set. If not the
> disable_irq_nosync will just increment the depth field in the irq
> descriptor only once as shown below.
> 
> disable_irq_nosync
>  __disable_irq_nosync
>   __disable_irq (desc->depth++)
>    irq_disable
>     if irq_disable present -----------> if IRQ_DISABLE_UNLAZYflag set
>              |                  no                  |
>          yes |                                  yes |
>              |                                      |
>      desc->irq_data.chip->irq_disable   desc->irq_data.chip->irq_unmask
>                                          (ds90ub927_irq_mask)
>                                           disable_irq
>                                            __disable_irq_nosync
>                                             __disable_irq
> (desc->depth++)
> But the enable_irq will try to decrement the depth field twice which generates
> the backtrace stating "Unbalanced enable for irq 293". This is because there is
> no IRQ_DISABLE_UNLAZY flag check while calling irq_unmask callback function
> of the "ds90ub927_irq_unmask" de-/serializer via enable_irq.
> 
> enable_irq
>  __enable_irq (desc->depth--)
>   irq_enable
>    if irq_enable present -------------> desc->irq_data.chip->irq_unmask
>               |                no        (ds90ub927_irq_unmask)
>           yes |                            enable_irq
>               |                             __enable_irq (desc->depth--)
>     (desc->irq_data.chip->irq_enable)

I'd prefer if we instead did not use the disable_irq_nosync() in the
driver.

Thanks.

-- 
Dmitry

Re: [PATCH v1 41/63] Input: touchscreen: Atmel: Enable IRQ_DISABLE_UNLAZY flag for interrupt

[Jiada Wang]

Hi Dmitry

On 2019/08/17 2:26, Dmitry Torokhov wrote:
> On Fri, Aug 16, 2019 at 05:35:36PM +0900, Jiada Wang wrote:
>> From: Bhuvanesh Surachari <bhuvanesh_surachari@mentor.com>
>>
>> The de-/serializer driver has defined only irq_mask "ds90ub927_irq_mask" and
>> irq_unmask "ds90ub927_irq_unmask" callback functions. And de-/serializer
>> driver doesn't implement the irq_disable and irq_enable callback functions.
>> Hence inorder to invoke irq_mask callback function when disable_irq_nosync is
>> called the IRQ_DISABLE_UNLAZY interrupt flag should be set. If not the
>> disable_irq_nosync will just increment the depth field in the irq
>> descriptor only once as shown below.
>>
>> disable_irq_nosync
>>   __disable_irq_nosync
>>    __disable_irq (desc->depth++)
>>     irq_disable
>>      if irq_disable present -----------> if IRQ_DISABLE_UNLAZYflag set
>>               |                  no                  |
>>           yes |                                  yes |
>>               |                                      |
>>       desc->irq_data.chip->irq_disable   desc->irq_data.chip->irq_unmask
>>                                           (ds90ub927_irq_mask)
>>                                            disable_irq
>>                                             __disable_irq_nosync
>>                                              __disable_irq
>> (desc->depth++)
>> But the enable_irq will try to decrement the depth field twice which generates
>> the backtrace stating "Unbalanced enable for irq 293". This is because there is
>> no IRQ_DISABLE_UNLAZY flag check while calling irq_unmask callback function
>> of the "ds90ub927_irq_unmask" de-/serializer via enable_irq.
>>
>> enable_irq
>>   __enable_irq (desc->depth--)
>>    irq_enable
>>     if irq_enable present -------------> desc->irq_data.chip->irq_unmask
>>                |                no        (ds90ub927_irq_unmask)
>>            yes |                            enable_irq
>>                |                             __enable_irq (desc->depth--)
>>      (desc->irq_data.chip->irq_enable)
> 
> I'd prefer if we instead did not use the disable_irq_nosync() in the
> driver.
>
sorry for the mistake, during forward port,
I have already eliminated disable_irq_nosync(),
so this patch is no longer needed,
will drop it in v2 patch-set

Thanks，
Jiada

> Thanks.
> 

[PATCH v2 42/63] Input: atmel_mxt_ts: Limit the max bytes transferred in an i2c transaction

[Jiada Wang]

From: Balasubramani Vivekanandan <balasubramani_vivekanandan@mentor.com>

In mxt_process_messages_until_invalid() function, driver tries to read
all possible reportid in a single i2c transaction. Number of bytes read
is limited by the max_reportid parameter.
If the max_reportid is a very large value, then a large chunk of bytes
will be requested from the controller in a single i2c transaction.
This transaction can fail due to timeout. This is visible when the
Atmel controller is connected to the SOC via a i2c mux hardware.
New property 'atmel,mtu' is created. This property limits the maximum
number of bytes that can read/transferred in an i2c transcation

Signed-off-by: Balasubramani Vivekanandan <balasubramani_vivekanandan@mentor.com>
Signed-off-by: George G. Davis <george_davis@mentor.com>
Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
---
 .../bindings/input/atmel,maxtouch.txt         |  3 +++
 drivers/input/touchscreen/atmel_mxt_ts.c      | 26 +++++++++++++++++--
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/input/atmel,maxtouch.txt b/Documentation/devicetree/bindings/input/atmel,maxtouch.txt
index 7afe12a93202..a7f9a8e551f7 100644
--- a/Documentation/devicetree/bindings/input/atmel,maxtouch.txt
+++ b/Documentation/devicetree/bindings/input/atmel,maxtouch.txt
@@ -45,6 +45,8 @@ Optional properties for main touchpad device:
 - atmel,gpios: Specify the GPIO input pins whose status will be read via the
     /sys/class/input/input<n>/backlight_error<x> sysfs entries.
 
+- atmel,mtu: Maximum number of bytes that can read/transferred in an i2c transaction
+
 Example:
 
 	touch@4b {
@@ -52,6 +54,7 @@ Example:
 		reg = <0x4b>;
 		interrupt-parent = <&gpio>;
 		interrupts = <TEGRA_GPIO(W, 3) IRQ_TYPE_LEVEL_LOW>;
+		atmel,mtu = <200>
 
 		atmel,gpios {
 			backlight_error1 {
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index c6ba061098c0..e315ad3a8d2a 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -421,6 +421,7 @@ struct mxt_data {
 	unsigned long gpio_input_pin_status;
 	struct attribute_group gpio_attrs;
 	unsigned long gpio_input_pin_status_default;
+	unsigned int mtu;
 
 	bool t25_status;
 };
@@ -1522,13 +1523,30 @@ static irqreturn_t mxt_process_messages_t44(struct mxt_data *data)
 	return IRQ_HANDLED;
 }
 
+static u8 mxt_max_msg_read_count(struct mxt_data *data)
+{
+	u8 count_limit = data->mtu / data->T5_msg_size;
+
+	if (!data->mtu)
+		return data->max_reportid;
+
+	if (data->mtu < data->T5_msg_size) {
+		WARN(1, "mtu set is lesser than the T5 message size\n");
+		/* Return count of 1, as fallback */
+		return 1;
+	}
+
+	return min(count_limit, data->max_reportid);
+}
+
 static int mxt_process_messages_until_invalid(struct mxt_data *data)
 {
 	struct device *dev = &data->client->dev;
 	int count, read;
-	u8 tries = 2;
+	int tries;
 
-	count = data->max_reportid;
+	count = mxt_max_msg_read_count(data);
+	tries = (data->max_reportid / count) + 1;
 
 	/* Read messages until we force an invalid */
 	do {
@@ -4290,6 +4308,10 @@ static int mxt_parse_device_properties(struct mxt_data *data)
 			of_node_put(np_gpio);
 	}
 
+	device_property_read_u32(dev, "atmel,mtu", &data->mtu);
+	if (data->mtu)
+		dev_dbg(dev, "mtu is set as %d\n", data->mtu);
+
 	return 0;
 
 err_gpios_property_put:
-- 
2.19.2

[PATCH v1 43/63] Input: atmel_mxt_ts: update stale use_retrigen_workaround flag

[Jiada Wang]

From: Balasubramani Vivekanandan <balasubramani_vivekanandan@mentor.com>

If after configuration update, retrigen status is enabled, the
mxt_check_retrigen() function, called after configuration update,
does not clear the use_retrigen_workaround flag, if it was previously
set.

Signed-off-by: Balasubramani Vivekanandan <balasubramani_vivekanandan@mentor.com>
Signed-off-by: George G. Davis <george_davis@mentor.com>
Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index e315ad3a8d2a..b3d40390abb6 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -1857,6 +1857,8 @@ static int mxt_check_retrigen(struct mxt_data *data)
 	int error;
 	int val;
 
+	data->use_retrigen_workaround = false;
+
 	if (irq_get_trigger_type(data->irq) & IRQF_TRIGGER_LOW)
 		return 0;
 
-- 
2.19.2

[PATCH v1 44/63] Input: atmel_mxt_ts: return error from mxt_process_messages_until_invalid()

[Jiada Wang]

From: Dean Jenkins <Dean_Jenkins@mentor.com>

mxt_process_messages_until_invalid() failed to propagate the error
code from mxt_read_and_process_messages() so return the error code.

Signed-off-by: Dean Jenkins <Dean_Jenkins@mentor.com>
Signed-off-by: Deepak Das <deepak_das@mentor.com>
Signed-off-by: George G. Davis <george_davis@mentor.com>
Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index b3d40390abb6..ed5b98c067e8 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -1551,6 +1551,8 @@ static int mxt_process_messages_until_invalid(struct mxt_data *data)
 	/* Read messages until we force an invalid */
 	do {
 		read = mxt_read_and_process_messages(data, count);
+		if (read < 0)
+			return read;
 		if (read < count)
 			return 0;
 	} while (--tries);
-- 
2.19.2