Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1273097 > unrolled thread

[PATCH v11 0/8] Goodix touchscreen enhancements

Started byIrina Tirdea <irina.tirdea@intel.com>
First post2015-11-19 13:30 +0100
Last post2015-11-23 17:40 +0100
Articles 7 — 4 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH v11 0/8] Goodix touchscreen enhancements Irina Tirdea <irina.tirdea@intel.com> - 2015-11-19 13:30 +0100
    [PATCH v11 1/8] Input: goodix - use actual config length for each device type Irina Tirdea <irina.tirdea@intel.com> - 2015-11-19 13:40 +0100
    [PATCH v11 4/8] Input: goodix - add power management support Irina Tirdea <irina.tirdea@intel.com> - 2015-11-19 13:40 +0100
      Re: [PATCH v11 4/8] Input: goodix - add power management support Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2015-11-19 19:30 +0100
        Re: [PATCH v11 4/8] Input: goodix - add power management support "Rafael J. Wysocki" <rafael@kernel.org> - 2015-11-19 23:20 +0100
          Re: [PATCH v11 4/8] Input: goodix - add power management support Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2015-11-19 23:40 +0100
      Re: [PATCH v11 4/8] Input: goodix - add power management support Bastien Nocera <hadess@hadess.net> - 2015-11-23 17:40 +0100

#1273097 — [PATCH v11 0/8] Goodix touchscreen enhancements

FromIrina Tirdea <irina.tirdea@intel.com>
Date2015-11-19 13:30 +0100
Subject[PATCH v11 0/8] Goodix touchscreen enhancements
Message-ID<qwwvE-Oq-9@gated-at.bofh.it>
This patchset depends on Dmitry's patch that fixes the
GPIO ACPI API[1], so devm_gpiod_get_optional can be properly
used in the code. 

[1] https://lkml.org/lkml/2015/11/11/465

Changes in v11:
 - renamed irq-gpio and reset-gpio to irq-gpios and reset-gpios
 - added Rob's Acked-by for the binding in patch 6.

Changes in v10:
 - use devm_gpiod_get_optional and no longer ignore errors returned
by it
 - use completion to signal other code paths in the driver that
the config firmware is loaded on the device
 - dropped the patch that ordered the includes

Changes in v9:
 - add GPIOLIB to driver dependencies
 - add Tested-by tag from Bastien and Aleksei

Changes in v8:
 - only allow new functionality for devices that declare named
gpios (using _DSD properties in ACPI or named DT properties)

Changes in v7:
 - add dmi quirk to skip gpio pins setup and functionality that
depends on them for Onda v975w, WinBook TW100 and WinBook TW700.
 - add support for named gpio pins
 - rework the runtime pm patch to fix a couple of issues
 - sort includes using inverse Xmas tree ordering

Changes in v6:
 - skip runtime power manangent calls in open/close if the device
ACPI/DT configuration does not declare interrupt and reset gpio pins.
 - reset the device before starting i2c communication
 - add Bastien's ack to the first 2 patches

Changes in v5:
 - add some more style cleanup (reorder includes, use error instead
of ret for return values)
 - add runtime power management patch

Changes in v4:
 - use dmi quirk to determine the order of irq and reset pins
 - use actual config length depending on device
 - add sysfs interface to dump config
 - initialize esd timeout from ACPI/DT properly

Changes in v3:
 - dropped the first 3 patches that got merged
 - handle -EPROBE_DEFER and -ENOENT for gpio pins
 - skip functionality depending on the gpio pins if the pins are not
properly initialized from ACPI/DT (reset, write config, power management,
ESD)
 - dropped #ifdef CONFIG_PM_SLEEP and annotated with __maybe_unused instead
 - use sysfs property to set ESD timeout instead of ACPI/DT property
 - use request_firmware_nowait to read configuration firmware and use
defaults if firmware is not found
 - use ACPI IDs to determine the order of the GPIO pins in the ACPI tables
(interrupt pin first or reset pin first)

Changes in v2:
 - use request_firmware instead of ACPI/DT property for config
 - dropped "input: goodix: add ACPI IDs for GT911 and GT9271" patch
 - add ACPI DSDT excerpt in commit message where necessary
 - add comments for suspend/resume sleep values
 - dropped the checkpatch fixes that did not make sense
 - added Bastien's ack to the first patch

Irina Tirdea (8):
  Input: goodix - use actual config length for each device type
  Input: goodix - reset device at init
  Input: goodix - write configuration data to device
  Input: goodix - add power management support
  Input: goodix - use goodix_i2c_write_u8 instead of i2c_master_send
  Input: goodix - add support for ESD
  Input: goodix - add sysfs interface to dump config
  Input: goodix - add runtime power management support

 .../bindings/input/touchscreen/goodix.txt          |  13 +
 drivers/input/touchscreen/Kconfig                  |   1 +
 drivers/input/touchscreen/goodix.c                 | 766 ++++++++++++++++++++-
 3 files changed, 742 insertions(+), 38 deletions(-)

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1273100 — [PATCH v11 1/8] Input: goodix - use actual config length for each device type

FromIrina Tirdea <irina.tirdea@intel.com>
Date2015-11-19 13:40 +0100
Subject[PATCH v11 1/8] Input: goodix - use actual config length for each device type
Message-ID<qwwFj-RT-1@gated-at.bofh.it>
In reply to#1273097
Each of the Goodix devices supported by this driver has a fixed size for
the configuration information registers. The size varies depending on the
device and is specified in the datasheet.

Use the proper configuration length as specified in the datasheet for
each device model, so we do not read more than the actual size of the
configuration registers.

Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
Acked-by: Bastien Nocera <hadess@hadess.net>
Tested-by: Bastien Nocera <hadess@hadess.net>
Tested-by: Aleksei Mamlin <mamlinav@gmail.com>
---
 drivers/input/touchscreen/goodix.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index 4d113c9..56d0330 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -36,6 +36,7 @@ struct goodix_ts_data {
 	unsigned int max_touch_num;
 	unsigned int int_trigger_type;
 	bool rotated_screen;
+	int cfg_len;
 };
 
 #define GOODIX_MAX_HEIGHT		4096
@@ -45,6 +46,8 @@ struct goodix_ts_data {
 #define GOODIX_MAX_CONTACTS		10
 
 #define GOODIX_CONFIG_MAX_LENGTH	240
+#define GOODIX_CONFIG_911_LENGTH	186
+#define GOODIX_CONFIG_967_LENGTH	228
 
 /* Register defines */
 #define GOODIX_READ_COOR_ADDR		0x814E
@@ -115,6 +118,23 @@ static int goodix_i2c_read(struct i2c_client *client,
 	return ret < 0 ? ret : (ret != ARRAY_SIZE(msgs) ? -EIO : 0);
 }
 
+static int goodix_get_cfg_len(u16 id)
+{
+	switch (id) {
+	case 911:
+	case 9271:
+	case 9110:
+	case 927:
+	case 928:
+		return GOODIX_CONFIG_911_LENGTH;
+	case 912:
+	case 967:
+		return GOODIX_CONFIG_967_LENGTH;
+	default:
+		return GOODIX_CONFIG_MAX_LENGTH;
+	}
+}
+
 static int goodix_ts_read_input_report(struct goodix_ts_data *ts, u8 *data)
 {
 	int touch_num;
@@ -230,8 +250,7 @@ static void goodix_read_config(struct goodix_ts_data *ts)
 	int error;
 
 	error = goodix_i2c_read(ts->client, GOODIX_REG_CONFIG_DATA,
-				config,
-				GOODIX_CONFIG_MAX_LENGTH);
+				config, ts->cfg_len);
 	if (error) {
 		dev_warn(&ts->client->dev,
 			 "Error reading config (%d), using defaults\n",
@@ -398,6 +417,8 @@ static int goodix_ts_probe(struct i2c_client *client,
 		return error;
 	}
 
+	ts->cfg_len = goodix_get_cfg_len(id_info);
+
 	goodix_read_config(ts);
 
 	error = goodix_request_input_dev(ts, version_info, id_info);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1273103 — [PATCH v11 4/8] Input: goodix - add power management support

FromIrina Tirdea <irina.tirdea@intel.com>
Date2015-11-19 13:40 +0100
Subject[PATCH v11 4/8] Input: goodix - add power management support
Message-ID<qwwFk-RT-15@gated-at.bofh.it>
In reply to#1273097
Implement suspend/resume for goodix driver.

The suspend and resume process uses the gpio pins.
If the device ACPI/DT information does not declare gpio pins,
suspend/resume will not be available for these devices.

This is based on Goodix datasheets for GT911 and GT9271
and on Goodix driver gt9xx.c for Android (publicly available
in Android kernel trees for various devices).

Signed-off-by: Octavian Purdila <octavian.purdila@intel.com>
Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
---
 drivers/input/touchscreen/goodix.c | 96 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 91 insertions(+), 5 deletions(-)

diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index 0911b0c9..0fd472d 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -45,6 +45,7 @@ struct goodix_ts_data {
 	u16 version;
 	char *cfg_name;
 	struct completion firmware_loading_complete;
+	unsigned long irq_flags;
 };
 
 #define GOODIX_GPIO_INT_NAME		"irq"
@@ -61,6 +62,9 @@ struct goodix_ts_data {
 #define GOODIX_CONFIG_967_LENGTH	228
 
 /* Register defines */
+#define GOODIX_REG_COMMAND		0x8040
+#define GOODIX_CMD_SCREEN_OFF		0x05
+
 #define GOODIX_READ_COOR_ADDR		0x814E
 #define GOODIX_REG_CONFIG_DATA		0x8047
 #define GOODIX_REG_ID			0x8140
@@ -162,6 +166,11 @@ static int goodix_i2c_write(struct i2c_client *client, u16 reg, const u8 *buf,
 	return ret < 0 ? ret : (ret != 1 ? -EIO : 0);
 }
 
+static int goodix_i2c_write_u8(struct i2c_client *client, u16 reg, u8 value)
+{
+	return goodix_i2c_write(client, reg, &value, sizeof(value));
+}
+
 static int goodix_get_cfg_len(u16 id)
 {
 	switch (id) {
@@ -281,6 +290,18 @@ static irqreturn_t goodix_ts_irq_handler(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static void goodix_free_irq(struct goodix_ts_data *ts)
+{
+	devm_free_irq(&ts->client->dev, ts->client->irq, ts);
+}
+
+static int goodix_request_irq(struct goodix_ts_data *ts)
+{
+	return devm_request_threaded_irq(&ts->client->dev, ts->client->irq,
+					 NULL, goodix_ts_irq_handler,
+					 ts->irq_flags, ts->client->name, ts);
+}
+
 /**
  * goodix_check_cfg - Checks if config fw is valid
  *
@@ -585,7 +606,6 @@ static int goodix_request_input_dev(struct goodix_ts_data *ts)
 static int goodix_configure_dev(struct goodix_ts_data *ts)
 {
 	int error;
-	unsigned long irq_flags;
 
 	goodix_read_config(ts);
 
@@ -593,10 +613,8 @@ static int goodix_configure_dev(struct goodix_ts_data *ts)
 	if (error)
 		return error;
 
-	irq_flags = goodix_irq_flags[ts->int_trigger_type] | IRQF_ONESHOT;
-	error = devm_request_threaded_irq(&ts->client->dev, ts->client->irq,
-					  NULL, goodix_ts_irq_handler,
-					  irq_flags, ts->client->name, ts);
+	ts->irq_flags = goodix_irq_flags[ts->int_trigger_type] | IRQF_ONESHOT;
+	error = goodix_request_irq(ts);
 	if (error) {
 		dev_err(&ts->client->dev, "request IRQ failed: %d\n", error);
 		return error;
@@ -720,6 +738,73 @@ static int goodix_ts_remove(struct i2c_client *client)
 	return 0;
 }
 
+static int __maybe_unused goodix_suspend(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct goodix_ts_data *ts = i2c_get_clientdata(client);
+	int error;
+
+	/* We need gpio pins to suspend/resume */
+	if (!ts->gpiod_int || !ts->gpiod_rst)
+		return 0;
+
+	wait_for_completion(&ts->firmware_loading_complete);
+
+	/* Free IRQ as IRQ pin is used as output in the suspend sequence */
+	goodix_free_irq(ts);
+	/* Output LOW on the INT pin for 5 ms */
+	error = gpiod_direction_output(ts->gpiod_int, 0);
+	if (error) {
+		goodix_request_irq(ts);
+		return error;
+	}
+	usleep_range(5000, 6000);
+
+	error = goodix_i2c_write_u8(ts->client, GOODIX_REG_COMMAND,
+				    GOODIX_CMD_SCREEN_OFF);
+	if (error) {
+		dev_err(&ts->client->dev, "Screen off command failed\n");
+		gpiod_direction_input(ts->gpiod_int);
+		goodix_request_irq(ts);
+		return -EAGAIN;
+	}
+
+	/*
+	 * The datasheet specifies that the interval between sending screen-off
+	 * command and wake-up should be longer than 58 ms. To avoid waking up
+	 * sooner, delay 58ms here.
+	 */
+	msleep(58);
+	return 0;
+}
+
+static int __maybe_unused goodix_resume(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct goodix_ts_data *ts = i2c_get_clientdata(client);
+	int error;
+
+	if (!ts->gpiod_int || !ts->gpiod_rst)
+		return 0;
+
+	/*
+	 * Exit sleep mode by outputting HIGH level to INT pin
+	 * for 2ms~5ms.
+	 */
+	error = gpiod_direction_output(ts->gpiod_int, 1);
+	if (error)
+		return error;
+	usleep_range(2000, 5000);
+
+	error = goodix_int_sync(ts);
+	if (error)
+		return error;
+
+	return goodix_request_irq(ts);
+}
+
+static SIMPLE_DEV_PM_OPS(goodix_pm_ops, goodix_suspend, goodix_resume);
+
 static const struct i2c_device_id goodix_ts_id[] = {
 	{ "GDIX1001:00", 0 },
 	{ }
@@ -756,6 +841,7 @@ static struct i2c_driver goodix_ts_driver = {
 		.name = "Goodix-TS",
 		.acpi_match_table = ACPI_PTR(goodix_acpi_match),
 		.of_match_table = of_match_ptr(goodix_of_match),
+		.pm = &goodix_pm_ops,
 	},
 };
 module_i2c_driver(goodix_ts_driver);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1273355 — Re: [PATCH v11 4/8] Input: goodix - add power management support

FromDmitry Torokhov <dmitry.torokhov@gmail.com>
Date2015-11-19 19:30 +0100
SubjectRe: [PATCH v11 4/8] Input: goodix - add power management support
Message-ID<qwC82-4vu-15@gated-at.bofh.it>
In reply to#1273103
On Thu, Nov 19, 2015 at 02:26:37PM +0200, Irina Tirdea wrote:
> Implement suspend/resume for goodix driver.
> 
> The suspend and resume process uses the gpio pins.
> If the device ACPI/DT information does not declare gpio pins,
> suspend/resume will not be available for these devices.
> 
> This is based on Goodix datasheets for GT911 and GT9271
> and on Goodix driver gt9xx.c for Android (publicly available
> in Android kernel trees for various devices).
> 
> Signed-off-by: Octavian Purdila <octavian.purdila@intel.com>
> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
> ---
>  drivers/input/touchscreen/goodix.c | 96 ++++++++++++++++++++++++++++++++++++--
>  1 file changed, 91 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
> index 0911b0c9..0fd472d 100644
> --- a/drivers/input/touchscreen/goodix.c
> +++ b/drivers/input/touchscreen/goodix.c
> @@ -45,6 +45,7 @@ struct goodix_ts_data {
>  	u16 version;
>  	char *cfg_name;
>  	struct completion firmware_loading_complete;
> +	unsigned long irq_flags;
>  };
>  
>  #define GOODIX_GPIO_INT_NAME		"irq"
> @@ -61,6 +62,9 @@ struct goodix_ts_data {
>  #define GOODIX_CONFIG_967_LENGTH	228
>  
>  /* Register defines */
> +#define GOODIX_REG_COMMAND		0x8040
> +#define GOODIX_CMD_SCREEN_OFF		0x05
> +
>  #define GOODIX_READ_COOR_ADDR		0x814E
>  #define GOODIX_REG_CONFIG_DATA		0x8047
>  #define GOODIX_REG_ID			0x8140
> @@ -162,6 +166,11 @@ static int goodix_i2c_write(struct i2c_client *client, u16 reg, const u8 *buf,
>  	return ret < 0 ? ret : (ret != 1 ? -EIO : 0);
>  }
>  
> +static int goodix_i2c_write_u8(struct i2c_client *client, u16 reg, u8 value)
> +{
> +	return goodix_i2c_write(client, reg, &value, sizeof(value));
> +}
> +
>  static int goodix_get_cfg_len(u16 id)
>  {
>  	switch (id) {
> @@ -281,6 +290,18 @@ static irqreturn_t goodix_ts_irq_handler(int irq, void *dev_id)
>  	return IRQ_HANDLED;
>  }
>  
> +static void goodix_free_irq(struct goodix_ts_data *ts)
> +{
> +	devm_free_irq(&ts->client->dev, ts->client->irq, ts);
> +}
> +
> +static int goodix_request_irq(struct goodix_ts_data *ts)
> +{
> +	return devm_request_threaded_irq(&ts->client->dev, ts->client->irq,
> +					 NULL, goodix_ts_irq_handler,
> +					 ts->irq_flags, ts->client->name, ts);
> +}
> +
>  /**
>   * goodix_check_cfg - Checks if config fw is valid
>   *
> @@ -585,7 +606,6 @@ static int goodix_request_input_dev(struct goodix_ts_data *ts)
>  static int goodix_configure_dev(struct goodix_ts_data *ts)
>  {
>  	int error;
> -	unsigned long irq_flags;
>  
>  	goodix_read_config(ts);
>  
> @@ -593,10 +613,8 @@ static int goodix_configure_dev(struct goodix_ts_data *ts)
>  	if (error)
>  		return error;
>  
> -	irq_flags = goodix_irq_flags[ts->int_trigger_type] | IRQF_ONESHOT;
> -	error = devm_request_threaded_irq(&ts->client->dev, ts->client->irq,
> -					  NULL, goodix_ts_irq_handler,
> -					  irq_flags, ts->client->name, ts);
> +	ts->irq_flags = goodix_irq_flags[ts->int_trigger_type] | IRQF_ONESHOT;
> +	error = goodix_request_irq(ts);
>  	if (error) {
>  		dev_err(&ts->client->dev, "request IRQ failed: %d\n", error);
>  		return error;
> @@ -720,6 +738,73 @@ static int goodix_ts_remove(struct i2c_client *client)
>  	return 0;
>  }
>  
> +static int __maybe_unused goodix_suspend(struct device *dev)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct goodix_ts_data *ts = i2c_get_clientdata(client);
> +	int error;
> +
> +	/* We need gpio pins to suspend/resume */
> +	if (!ts->gpiod_int || !ts->gpiod_rst)
> +		return 0;
> +
> +	wait_for_completion(&ts->firmware_loading_complete);

This is not that nice as it may lead to angry splats from the PM core if
firmware loading takes too long and we start suspending before it
completes.

Rafael, if we issue pm_stay_awake() before requesting firmware and
pm_relax() once it is done, will this prevent the current suspend
timeouts?

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1273549 — Re: [PATCH v11 4/8] Input: goodix - add power management support

From"Rafael J. Wysocki" <rafael@kernel.org>
Date2015-11-19 23:20 +0100
SubjectRe: [PATCH v11 4/8] Input: goodix - add power management support
Message-ID<qwFIC-6YF-31@gated-at.bofh.it>
In reply to#1273355
Hi,

On Thu, Nov 19, 2015 at 7:24 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Thu, Nov 19, 2015 at 02:26:37PM +0200, Irina Tirdea wrote:
>> Implement suspend/resume for goodix driver.
>>

[cut]

>>
>> +static int __maybe_unused goodix_suspend(struct device *dev)
>> +{
>> +     struct i2c_client *client = to_i2c_client(dev);
>> +     struct goodix_ts_data *ts = i2c_get_clientdata(client);
>> +     int error;
>> +
>> +     /* We need gpio pins to suspend/resume */
>> +     if (!ts->gpiod_int || !ts->gpiod_rst)
>> +             return 0;
>> +
>> +     wait_for_completion(&ts->firmware_loading_complete);
>
> This is not that nice as it may lead to angry splats from the PM core if
> firmware loading takes too long and we start suspending before it
> completes.

What exactly do you mean?  The suspend watchdog or something else?

> Rafael, if we issue pm_stay_awake() before requesting firmware and
> pm_relax() once it is done, will this prevent the current suspend
> timeouts?

pm_stay_awake() only works if the checking of wakeup sources is
enabled which generally depends on what user space does.

Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1273556 — Re: [PATCH v11 4/8] Input: goodix - add power management support

FromDmitry Torokhov <dmitry.torokhov@gmail.com>
Date2015-11-19 23:40 +0100
SubjectRe: [PATCH v11 4/8] Input: goodix - add power management support
Message-ID<qwG1X-74P-5@gated-at.bofh.it>
In reply to#1273549
On Thu, Nov 19, 2015 at 2:18 PM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> Hi,
>
> On Thu, Nov 19, 2015 at 7:24 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
>> On Thu, Nov 19, 2015 at 02:26:37PM +0200, Irina Tirdea wrote:
>>> Implement suspend/resume for goodix driver.
>>>
>
> [cut]
>
>>>
>>> +static int __maybe_unused goodix_suspend(struct device *dev)
>>> +{
>>> +     struct i2c_client *client = to_i2c_client(dev);
>>> +     struct goodix_ts_data *ts = i2c_get_clientdata(client);
>>> +     int error;
>>> +
>>> +     /* We need gpio pins to suspend/resume */
>>> +     if (!ts->gpiod_int || !ts->gpiod_rst)
>>> +             return 0;
>>> +
>>> +     wait_for_completion(&ts->firmware_loading_complete);
>>
>> This is not that nice as it may lead to angry splats from the PM core if
>> firmware loading takes too long and we start suspending before it
>> completes.
>
> What exactly do you mean?  The suspend watchdog or something else?

I was thinking about dpm watchdog that will panic the system if
suspend takes too long. Hmm, this is debug facility and the default
timeout is 60 seconds, so I guess we can ignore my concern.

>
>> Rafael, if we issue pm_stay_awake() before requesting firmware and
>> pm_relax() once it is done, will this prevent the current suspend
>> timeouts?
>
> pm_stay_awake() only works if the checking of wakeup sources is
> enabled which generally depends on what user space does.

I see.

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1275612 — Re: [PATCH v11 4/8] Input: goodix - add power management support

FromBastien Nocera <hadess@hadess.net>
Date2015-11-23 17:40 +0100
SubjectRe: [PATCH v11 4/8] Input: goodix - add power management support
Message-ID<qy2jM-4lG-29@gated-at.bofh.it>
In reply to#1273103
On Thu, 2015-11-19 at 14:26 +0200, Irina Tirdea wrote:
> Implement suspend/resume for goodix driver.
> 
> The suspend and resume process uses the gpio pins.
> If the device ACPI/DT information does not declare gpio pins,
> suspend/resume will not be available for these devices.
> 
> This is based on Goodix datasheets for GT911 and GT9271
> and on Goodix driver gt9xx.c for Android (publicly available
> in Android kernel trees for various devices).
> 
> Signed-off-by: Octavian Purdila <octavian.purdila@intel.com>
> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>

Not functional on the WinBook TW100, but doesn't break anything

Tested-by: Bastien Nocera <hadess@hadess.net>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web