linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/3] Support wakeup methods of Atmel maXTouch controllers
@ 2020-12-05  5:47 Dmitry Osipenko
  2020-12-05  5:47 ` [PATCH v2 1/3] dt-bindings: input: atmel_mxt_ts: Document atmel,wakeup-method and wake-GPIO Dmitry Osipenko
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Dmitry Osipenko @ 2020-12-05  5:47 UTC (permalink / raw)
  To: Nick Dyer, Dmitry Torokhov, Rob Herring, Thierry Reding,
	Jonathan Hunter, Linus Walleij, Jiada Wang
  Cc: linux-input, devicetree, linux-tegra, linux-kernel

Some Atmel maXTouch controllers, like mXT1386 and mXT3432S1 for example,
have a WAKE line that needs to be asserted in order to wake controller
from a deep sleep, otherwise it will be unusable. This series implements
support for the wakeup methods in accordance to the mXT1386 datasheet [1],
see page 29 (chapter "5.8 WAKE Line").

The mXT1386 is a widely used controller found on many older Android tablet
devices. Touchscreen on Acer A500 tablet now works properly after this
series.

This patchset is a continuation of the work originally started by
Jiada Wang [2].

[1] https://ww1.microchip.com/downloads/en/DeviceDoc/mXT1386_1vx_Datasheet_LX.pdf
[2] https://patchwork.kernel.org/project/linux-input/list/?series=357875

Changelog:

v2: - Fixed copy-paste bug in the code.

Dmitry Osipenko (3):
  dt-bindings: input: atmel_mxt_ts: Document atmel,wakeup-method and
    wake-GPIO
  Input: atmel_mxt_ts - support wakeup methods
  ARM: tegra: acer-a500: Add atmel,wakeup-method property

 .../bindings/input/atmel,maxtouch.yaml        | 26 +++++++++
 .../boot/dts/tegra20-acer-a500-picasso.dts    |  3 +
 drivers/input/touchscreen/atmel_mxt_ts.c      | 55 +++++++++++++++++++
 include/dt-bindings/input/atmel-maxtouch.h    | 10 ++++
 4 files changed, 94 insertions(+)
 create mode 100644 include/dt-bindings/input/atmel-maxtouch.h

-- 
2.29.2


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

* [PATCH v2 1/3] dt-bindings: input: atmel_mxt_ts: Document atmel,wakeup-method and wake-GPIO
  2020-12-05  5:47 [PATCH v2 0/3] Support wakeup methods of Atmel maXTouch controllers Dmitry Osipenko
@ 2020-12-05  5:47 ` Dmitry Osipenko
  2020-12-05  5:47 ` [PATCH v2 2/3] Input: atmel_mxt_ts - support wakeup methods Dmitry Osipenko
  2020-12-05  5:47 ` [PATCH v2 3/3] ARM: tegra: acer-a500: Add atmel,wakeup-method property Dmitry Osipenko
  2 siblings, 0 replies; 6+ messages in thread
From: Dmitry Osipenko @ 2020-12-05  5:47 UTC (permalink / raw)
  To: Nick Dyer, Dmitry Torokhov, Rob Herring, Thierry Reding,
	Jonathan Hunter, Linus Walleij, Jiada Wang
  Cc: linux-input, devicetree, linux-tegra, linux-kernel

Some Atmel touchscreen controllers have a WAKE line that needs to be
asserted low in order to wake up controller from a deep sleep. Document
the wakeup methods and the wake-GPIO properties.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 .../bindings/input/atmel,maxtouch.yaml        | 26 +++++++++++++++++++
 include/dt-bindings/input/atmel-maxtouch.h    | 10 +++++++
 2 files changed, 36 insertions(+)
 create mode 100644 include/dt-bindings/input/atmel-maxtouch.h

diff --git a/Documentation/devicetree/bindings/input/atmel,maxtouch.yaml b/Documentation/devicetree/bindings/input/atmel,maxtouch.yaml
index 8c6418f76e94..7924a16dc248 100644
--- a/Documentation/devicetree/bindings/input/atmel,maxtouch.yaml
+++ b/Documentation/devicetree/bindings/input/atmel,maxtouch.yaml
@@ -39,6 +39,13 @@ properties:
       (active low). The line must be flagged with
       GPIO_ACTIVE_LOW.
 
+  wake-gpios:
+    maxItems: 1
+    description:
+      Optional GPIO specifier for the touchscreen's wake pin
+      (active low). The line must be flagged with
+      GPIO_ACTIVE_LOW.
+
   linux,gpio-keymap:
     $ref: /schemas/types.yaml#/definitions/uint32-array
     description: |
@@ -53,6 +60,23 @@ properties:
       or experiment to determine which bit corresponds to which input. Use
       KEY_RESERVED for unused padding values.
 
+  atmel,wakeup-method:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      The WAKE line is an active-low input that is used to wake up the touch
+      controller from deep-sleep mode before communication with the controller
+      could be started. This feature used to minimize current consumption
+      when the controller is in deep sleep mode.
+
+      The WAKE pin can be connected in one of the following ways:
+       1) left permanently low
+       2) connected to the I2C-compatible SCL pin
+       3) connected to a GPIO pin on the host
+    enum:
+      - 0 # ATMEL_MXT_WAKEUP_NONE
+      - 1 # ATMEL_MXT_WAKEUP_I2C_SCL
+      - 2 # ATMEL_MXT_WAKEUP_GPIO
+
 required:
   - compatible
   - reg
@@ -63,6 +87,7 @@ additionalProperties: false
 examples:
   - |
     #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/input/atmel-maxtouch.h>
     #include <dt-bindings/gpio/gpio.h>
     i2c {
       #address-cells = <1>;
@@ -75,6 +100,7 @@ examples:
         reset-gpios = <&gpio 27 GPIO_ACTIVE_LOW>;
         vdda-supply = <&ab8500_ldo_aux2_reg>;
         vdd-supply = <&ab8500_ldo_aux5_reg>;
+        atmel,wakeup-method = <ATMEL_MXT_WAKEUP_I2C_SCL>;
       };
     };
 
diff --git a/include/dt-bindings/input/atmel-maxtouch.h b/include/dt-bindings/input/atmel-maxtouch.h
new file mode 100644
index 000000000000..7345ab32224d
--- /dev/null
+++ b/include/dt-bindings/input/atmel-maxtouch.h
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+
+#ifndef _DT_BINDINGS_ATMEL_MAXTOUCH_H
+#define _DT_BINDINGS_ATMEL_MAXTOUCH_H
+
+#define ATMEL_MXT_WAKEUP_NONE		0
+#define ATMEL_MXT_WAKEUP_I2C_SCL	1
+#define ATMEL_MXT_WAKEUP_GPIO		2
+
+#endif /* _DT_BINDINGS_ATMEL_MAXTOUCH_H */
-- 
2.29.2


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

* [PATCH v2 2/3] Input: atmel_mxt_ts - support wakeup methods
  2020-12-05  5:47 [PATCH v2 0/3] Support wakeup methods of Atmel maXTouch controllers Dmitry Osipenko
  2020-12-05  5:47 ` [PATCH v2 1/3] dt-bindings: input: atmel_mxt_ts: Document atmel,wakeup-method and wake-GPIO Dmitry Osipenko
@ 2020-12-05  5:47 ` Dmitry Osipenko
  2020-12-06 15:20   ` Linus Walleij
  2020-12-05  5:47 ` [PATCH v2 3/3] ARM: tegra: acer-a500: Add atmel,wakeup-method property Dmitry Osipenko
  2 siblings, 1 reply; 6+ messages in thread
From: Dmitry Osipenko @ 2020-12-05  5:47 UTC (permalink / raw)
  To: Nick Dyer, Dmitry Torokhov, Rob Herring, Thierry Reding,
	Jonathan Hunter, Linus Walleij, Jiada Wang
  Cc: linux-input, devicetree, linux-tegra, linux-kernel

According to datasheets, chips like mXT1386 have a WAKE line, it is used
to wake the chip up from deep sleep mode before communicating with it via
the I2C-compatible interface.

If the WAKE line is connected to a GPIO line, the line must be asserted
25 ms before the host attempts to communicate with the controller. If the
WAKE line is connected to the SCL pin, the controller will send a NACK on
the first attempt to address it, the host must then retry 25 ms later.

Implement the wake-up methods in the driver. Touchscreen now works
properly on devices like Acer A500 tablet, fixing problems like this:

 atmel_mxt_ts 0-004c: __mxt_read_reg: i2c transfer failed (-121)
 atmel_mxt_ts 0-004c: mxt_bootloader_read: i2c recv failed (-121)
 atmel_mxt_ts 0-004c: Trying alternate bootloader address
 atmel_mxt_ts 0-004c: mxt_bootloader_read: i2c recv failed (-121)
 atmel_mxt_ts: probe of 0-004c failed with error -121

Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 55 ++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 2b3fff0822fe..cd52420a1f2b 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -31,6 +31,7 @@
 #include <media/v4l2-ioctl.h>
 #include <media/videobuf2-v4l2.h>
 #include <media/videobuf2-vmalloc.h>
+#include <dt-bindings/input/atmel-maxtouch.h>
 
 /* Firmware files */
 #define MXT_FW_NAME		"maxtouch.fw"
@@ -199,6 +200,7 @@ enum t100_type {
 #define MXT_CRC_TIMEOUT		1000	/* msec */
 #define MXT_FW_RESET_TIME	3000	/* msec */
 #define MXT_FW_CHG_TIMEOUT	300	/* msec */
+#define MXT_WAKEUP_TIME		25	/* msec */
 
 /* Command to unlock bootloader */
 #define MXT_UNLOCK_CMD_MSB	0xaa
@@ -312,6 +314,7 @@ struct mxt_data {
 	struct mxt_dbg dbg;
 	struct regulator_bulk_data regulators[2];
 	struct gpio_desc *reset_gpio;
+	struct gpio_desc *wake_gpio;
 	bool use_retrigen_workaround;
 
 	/* Cached parameters from object table */
@@ -342,6 +345,8 @@ struct mxt_data {
 	unsigned int t19_num_keys;
 
 	enum mxt_suspend_mode suspend_mode;
+
+	u32 wakeup_method;
 };
 
 struct mxt_vb2_buffer {
@@ -626,10 +631,25 @@ static int mxt_send_bootloader_cmd(struct mxt_data *data, bool unlock)
 	return 0;
 }
 
+static bool mxt_wake_up(struct i2c_client *client)
+{
+	struct mxt_data *data = i2c_get_clientdata(client);
+
+	if (data->wakeup_method != ATMEL_MXT_WAKEUP_I2C_SCL)
+		return false;
+
+	dev_dbg(&client->dev, "waking up controller\n");
+
+	msleep(MXT_WAKEUP_TIME);
+
+	return true;
+}
+
 static int __mxt_read_reg(struct i2c_client *client,
 			       u16 reg, u16 len, void *val)
 {
 	struct i2c_msg xfer[2];
+	bool retried = false;
 	u8 buf[2];
 	int ret;
 
@@ -648,9 +668,13 @@ static int __mxt_read_reg(struct i2c_client *client,
 	xfer[1].len = len;
 	xfer[1].buf = val;
 
+retry:
 	ret = i2c_transfer(client->adapter, xfer, 2);
 	if (ret == 2) {
 		ret = 0;
+	} else if (!retried && mxt_wake_up(client)) {
+		retried = true;
+		goto retry;
 	} else {
 		if (ret >= 0)
 			ret = -EIO;
@@ -664,6 +688,7 @@ static int __mxt_read_reg(struct i2c_client *client,
 static int __mxt_write_reg(struct i2c_client *client, u16 reg, u16 len,
 			   const void *val)
 {
+	bool retried = false;
 	u8 *buf;
 	size_t count;
 	int ret;
@@ -677,9 +702,13 @@ static int __mxt_write_reg(struct i2c_client *client, u16 reg, u16 len,
 	buf[1] = (reg >> 8) & 0xff;
 	memcpy(&buf[2], val, len);
 
+retry:
 	ret = i2c_master_send(client, buf, count);
 	if (ret == count) {
 		ret = 0;
+	} else if (!retried && mxt_wake_up(client)) {
+		retried = true;
+		goto retry;
 	} else {
 		if (ret >= 0)
 			ret = -EIO;
@@ -3160,6 +3189,15 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		return error;
 	}
 
+	/* Request the WAKE line as asserted so controller won't sleep */
+	data->wake_gpio = devm_gpiod_get_optional(&client->dev,
+						  "wake", GPIOD_OUT_HIGH);
+	if (IS_ERR(data->wake_gpio)) {
+		error = PTR_ERR(data->wake_gpio);
+		dev_err(&client->dev, "Failed to get wake gpio: %d\n", error);
+		return error;
+	}
+
 	error = devm_request_threaded_irq(&client->dev, client->irq,
 					  NULL, mxt_interrupt, IRQF_ONESHOT,
 					  client->name, data);
@@ -3190,6 +3228,23 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		msleep(MXT_RESET_INVALID_CHG);
 	}
 
+	/*
+	 * Controllers like mXT1386 have a dedicated WAKE line that could be
+	 * connected to a GPIO or to I2C SCL pin, or permanently asserted low.
+	 *
+	 * This WAKE line is used for waking controller from a deep-sleep and
+	 * it needs to be asserted low for 25 milliseconds before I2C transfers
+	 * could be accepted by controller if it was in a deep-sleep mode.
+	 *
+	 * If WAKE line is connected to I2C SCL pin, then the first I2C transfer
+	 * will get an instant NAK and transfer needs to be retried after 25ms.
+	 *
+	 * If WAKE line is connected to a GPIO line, the line must be asserted
+	 * 25ms before the host attempts to communicate with the controller.
+	 */
+	device_property_read_u32(&client->dev, "atmel,wakeup-method",
+				 &data->wakeup_method);
+
 	error = mxt_initialize(data);
 	if (error)
 		goto err_disable_regulators;
-- 
2.29.2


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

* [PATCH v2 3/3] ARM: tegra: acer-a500: Add atmel,wakeup-method property
  2020-12-05  5:47 [PATCH v2 0/3] Support wakeup methods of Atmel maXTouch controllers Dmitry Osipenko
  2020-12-05  5:47 ` [PATCH v2 1/3] dt-bindings: input: atmel_mxt_ts: Document atmel,wakeup-method and wake-GPIO Dmitry Osipenko
  2020-12-05  5:47 ` [PATCH v2 2/3] Input: atmel_mxt_ts - support wakeup methods Dmitry Osipenko
@ 2020-12-05  5:47 ` Dmitry Osipenko
  2 siblings, 0 replies; 6+ messages in thread
From: Dmitry Osipenko @ 2020-12-05  5:47 UTC (permalink / raw)
  To: Nick Dyer, Dmitry Torokhov, Rob Herring, Thierry Reding,
	Jonathan Hunter, Linus Walleij, Jiada Wang
  Cc: linux-input, devicetree, linux-tegra, linux-kernel

Add atmel,wakeup-method property to the touchscreen node.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 arch/arm/boot/dts/tegra20-acer-a500-picasso.dts | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/boot/dts/tegra20-acer-a500-picasso.dts b/arch/arm/boot/dts/tegra20-acer-a500-picasso.dts
index d3b99535d755..40c1bab22155 100644
--- a/arch/arm/boot/dts/tegra20-acer-a500-picasso.dts
+++ b/arch/arm/boot/dts/tegra20-acer-a500-picasso.dts
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 /dts-v1/;
 
+#include <dt-bindings/input/atmel-maxtouch.h>
 #include <dt-bindings/input/gpio-keys.h>
 #include <dt-bindings/input/input.h>
 #include <dt-bindings/thermal/thermal.h>
@@ -450,6 +451,8 @@ touchscreen@4c {
 
 			avdd-supply = <&vdd_3v3_sys>;
 			vdd-supply  = <&vdd_3v3_sys>;
+
+			atmel,wakeup-method = <ATMEL_MXT_WAKEUP_I2C_SCL>;
 		};
 
 		gyroscope@68 {
-- 
2.29.2


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

* Re: [PATCH v2 2/3] Input: atmel_mxt_ts - support wakeup methods
  2020-12-05  5:47 ` [PATCH v2 2/3] Input: atmel_mxt_ts - support wakeup methods Dmitry Osipenko
@ 2020-12-06 15:20   ` Linus Walleij
  2020-12-06 19:42     ` Dmitry Osipenko
  0 siblings, 1 reply; 6+ messages in thread
From: Linus Walleij @ 2020-12-06 15:20 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: Nick Dyer, Dmitry Torokhov, Rob Herring, Thierry Reding,
	Jonathan Hunter, Jiada Wang, Linux Input,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-tegra, linux-kernel

On Sat, Dec 5, 2020 at 6:48 AM Dmitry Osipenko <digetx@gmail.com> wrote:

> According to datasheets, chips like mXT1386 have a WAKE line, it is used
> to wake the chip up from deep sleep mode before communicating with it via
> the I2C-compatible interface.
>
> If the WAKE line is connected to a GPIO line, the line must be asserted
> 25 ms before the host attempts to communicate with the controller. If the
> WAKE line is connected to the SCL pin, the controller will send a NACK on
> the first attempt to address it, the host must then retry 25 ms later.
>
> Implement the wake-up methods in the driver. Touchscreen now works
> properly on devices like Acer A500 tablet, fixing problems like this:
>
>  atmel_mxt_ts 0-004c: __mxt_read_reg: i2c transfer failed (-121)
>  atmel_mxt_ts 0-004c: mxt_bootloader_read: i2c recv failed (-121)
>  atmel_mxt_ts 0-004c: Trying alternate bootloader address
>  atmel_mxt_ts 0-004c: mxt_bootloader_read: i2c recv failed (-121)
>  atmel_mxt_ts: probe of 0-004c failed with error -121
>
> Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>

OK looks interesting!

> +       /* Request the WAKE line as asserted so controller won't sleep */
> +       data->wake_gpio = devm_gpiod_get_optional(&client->dev,
> +                                                 "wake", GPIOD_OUT_HIGH);
> +       if (IS_ERR(data->wake_gpio)) {
> +               error = PTR_ERR(data->wake_gpio);
> +               dev_err(&client->dev, "Failed to get wake gpio: %d\n", error);
> +               return error;
> +       }

That is a bit brutal, don't you think? Now you force the controller
to be on at all times. Even across suspend/resume.

Shouldn't the same patch drive this low in mxt_suspend()
and driver it high + wait 25 ms in mxt_resume()?
Waiting 25ms in mxt_resume() is chill, it is anyway on the
slowpath.

Yours,
Linus Walleij

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

* Re: [PATCH v2 2/3] Input: atmel_mxt_ts - support wakeup methods
  2020-12-06 15:20   ` Linus Walleij
@ 2020-12-06 19:42     ` Dmitry Osipenko
  0 siblings, 0 replies; 6+ messages in thread
From: Dmitry Osipenko @ 2020-12-06 19:42 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Nick Dyer, Dmitry Torokhov, Rob Herring, Thierry Reding,
	Jonathan Hunter, Jiada Wang, Linux Input,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-tegra, linux-kernel

06.12.2020 18:20, Linus Walleij пишет:
> On Sat, Dec 5, 2020 at 6:48 AM Dmitry Osipenko <digetx@gmail.com> wrote:
> 
>> According to datasheets, chips like mXT1386 have a WAKE line, it is used
>> to wake the chip up from deep sleep mode before communicating with it via
>> the I2C-compatible interface.
>>
>> If the WAKE line is connected to a GPIO line, the line must be asserted
>> 25 ms before the host attempts to communicate with the controller. If the
>> WAKE line is connected to the SCL pin, the controller will send a NACK on
>> the first attempt to address it, the host must then retry 25 ms later.
>>
>> Implement the wake-up methods in the driver. Touchscreen now works
>> properly on devices like Acer A500 tablet, fixing problems like this:
>>
>>  atmel_mxt_ts 0-004c: __mxt_read_reg: i2c transfer failed (-121)
>>  atmel_mxt_ts 0-004c: mxt_bootloader_read: i2c recv failed (-121)
>>  atmel_mxt_ts 0-004c: Trying alternate bootloader address
>>  atmel_mxt_ts 0-004c: mxt_bootloader_read: i2c recv failed (-121)
>>  atmel_mxt_ts: probe of 0-004c failed with error -121
>>
>> Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> 
> OK looks interesting!
> 
>> +       /* Request the WAKE line as asserted so controller won't sleep */
>> +       data->wake_gpio = devm_gpiod_get_optional(&client->dev,
>> +                                                 "wake", GPIOD_OUT_HIGH);
>> +       if (IS_ERR(data->wake_gpio)) {
>> +               error = PTR_ERR(data->wake_gpio);
>> +               dev_err(&client->dev, "Failed to get wake gpio: %d\n", error);
>> +               return error;
>> +       }
> 
> That is a bit brutal, don't you think? Now you force the controller
> to be on at all times. Even across suspend/resume.

Still it's better than a disfunctional hardware :)

> Shouldn't the same patch drive this low in mxt_suspend()
> and driver it high + wait 25 ms in mxt_resume()?
> Waiting 25ms in mxt_resume() is chill, it is anyway on the
> slowpath.

I don't have hardware which uses the wake-gpio in order to test that it
works properly, hence wanted to keep it minimal. But indeed sounds like
toggling the GPIO in suspend/resume should be okay to do. Alright, I'll
improve it in the v3. Thank you!

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

end of thread, other threads:[~2020-12-06 19:43 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-05  5:47 [PATCH v2 0/3] Support wakeup methods of Atmel maXTouch controllers Dmitry Osipenko
2020-12-05  5:47 ` [PATCH v2 1/3] dt-bindings: input: atmel_mxt_ts: Document atmel,wakeup-method and wake-GPIO Dmitry Osipenko
2020-12-05  5:47 ` [PATCH v2 2/3] Input: atmel_mxt_ts - support wakeup methods Dmitry Osipenko
2020-12-06 15:20   ` Linus Walleij
2020-12-06 19:42     ` Dmitry Osipenko
2020-12-05  5:47 ` [PATCH v2 3/3] ARM: tegra: acer-a500: Add atmel,wakeup-method property Dmitry Osipenko

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).