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

Groups > linux.kernel > #1220234 > unrolled thread

[PATCH v5 0/9] Goodix touchscreen enhancements

Back to article view | Back to linux.kernel

Contents

  [PATCH v5 0/9] Goodix touchscreen enhancements Irina Tirdea <irina.tirdea@intel.com> - 2015-09-07 16:40 +0200
    [PATCH v5 8/9] Input: goodix - add sysfs interface to dump config Irina Tirdea <irina.tirdea@intel.com> - 2015-09-07 16:40 +0200
    [PATCH v5 1/9] Input: goodix - sort includes alphabetically Irina Tirdea <irina.tirdea@intel.com> - 2015-09-07 16:40 +0200
      Re: [PATCH v5 1/9] Input: goodix - sort includes alphabetically Bastien Nocera <hadess@hadess.net> - 2015-09-09 19:00 +0200
    [PATCH v5 9/9] Input: goodix - add runtime power management support Irina Tirdea <irina.tirdea@intel.com> - 2015-09-07 16:40 +0200
    [PATCH v5 4/9] Input: goodix - write configuration data to device Irina Tirdea <irina.tirdea@intel.com> - 2015-09-07 16:40 +0200
    [PATCH v5 2/9] Input: goodix - use actual config length for each device type Irina Tirdea <irina.tirdea@intel.com> - 2015-09-07 16:50 +0200
      Re: [PATCH v5 2/9] Input: goodix - use actual config length for  each device type Bastien Nocera <hadess@hadess.net> - 2015-09-09 19:00 +0200

#1220234 — [PATCH v5 0/9] Goodix touchscreen enhancements

Add several enhancements to the Goodix touchscreen driver.
This version adds runtime power management and includes some cleanup.

Thanks,
Irina

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 propery

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 (9):
  Input: goodix - sort includes alphabetically
  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          |  11 +
 drivers/input/touchscreen/goodix.c                 | 709 +++++++++++++++++++--
 2 files changed, 679 insertions(+), 41 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]

#1220237 — [PATCH v5 8/9] Input: goodix - add sysfs interface to dump config

Goodix devices have a configuration information register area that
specify various parameters for the device. The configuration information
has a specific format described in the Goodix datasheet. It includes X/Y
resolution, maximum supported touch points, interrupt flags, various
sesitivity factors and settings for advanced features (like gesture
recognition).

Export a sysfs interface that would allow reading the configuration
information. The default device configuration can be used as a starting
point for creating a valid configuration firmware used by the device at
init time to update its configuration.

This sysfs interface will be exported only if the gpio pins are properly
initialized from ACPI/DT.

Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
---
 drivers/input/touchscreen/goodix.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index 33a7b81..3179767 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -530,12 +530,35 @@ static ssize_t goodix_esd_timeout_store(struct device *dev,
 	return count;
 }
 
+static ssize_t goodix_dump_config_show(struct device *dev,
+				       struct device_attribute *attr, char *buf)
+{
+	struct goodix_ts_data *ts = dev_get_drvdata(dev);
+	u8 config[GOODIX_CONFIG_MAX_LENGTH];
+	int error, count = 0, i;
+
+	error = goodix_i2c_read(ts->client, GOODIX_REG_CONFIG_DATA,
+				config, ts->cfg_len);
+	if (error) {
+		dev_warn(&ts->client->dev,
+			 "Error reading config (%d)\n",  error);
+		return error;
+	}
+
+	for (i = 0; i < ts->cfg_len; i++)
+		count += scnprintf(buf + count, PAGE_SIZE - count, "%02x ",
+				   config[i]);
+	return count;
+}
+
 /* ESD timeout in ms. Default disabled (0). Recommended 2000 ms. */
 static DEVICE_ATTR(esd_timeout, S_IRUGO | S_IWUSR, goodix_esd_timeout_show,
 		   goodix_esd_timeout_store);
+static DEVICE_ATTR(dump_config, S_IRUGO, goodix_dump_config_show, NULL);
 
 static struct attribute *goodix_attrs[] = {
 	&dev_attr_esd_timeout.attr,
+	&dev_attr_dump_config.attr,
 	NULL
 };
 
-- 
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]

#1220238 — [PATCH v5 1/9] Input: goodix - sort includes alphabetically

Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
---
 drivers/input/touchscreen/goodix.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index e36162b..6ae28c5 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -14,18 +14,18 @@
  * Software Foundation; version 2 of the License.
  */
 
-#include <linux/kernel.h>
+#include <linux/acpi.h>
+#include <linux/delay.h>
 #include <linux/dmi.h>
 #include <linux/i2c.h>
 #include <linux/input.h>
 #include <linux/input/mt.h>
-#include <linux/module.h>
-#include <linux/delay.h>
-#include <linux/irq.h>
 #include <linux/interrupt.h>
-#include <linux/slab.h>
-#include <linux/acpi.h>
+#include <linux/irq.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
 #include <linux/of.h>
+#include <linux/slab.h>
 #include <asm/unaligned.h>
 
 struct goodix_ts_data {
-- 
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]

#1221566 — Re: [PATCH v5 1/9] Input: goodix - sort includes alphabetically

On Mon, 2015-09-07 at 17:36 +0300, Irina Tirdea wrote:
> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>

Acked-by: Bastien Nocera <hadess@hadess.net>

> ---
>  drivers/input/touchscreen/goodix.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/goodix.c
> b/drivers/input/touchscreen/goodix.c
> index e36162b..6ae28c5 100644
> --- a/drivers/input/touchscreen/goodix.c
> +++ b/drivers/input/touchscreen/goodix.c
> @@ -14,18 +14,18 @@
>   * Software Foundation; version 2 of the License.
>   */
>  
> -#include <linux/kernel.h>
> +#include <linux/acpi.h>
> +#include <linux/delay.h>
>  #include <linux/dmi.h>
>  #include <linux/i2c.h>
>  #include <linux/input.h>
>  #include <linux/input/mt.h>
> -#include <linux/module.h>
> -#include <linux/delay.h>
> -#include <linux/irq.h>
>  #include <linux/interrupt.h>
> -#include <linux/slab.h>
> -#include <linux/acpi.h>
> +#include <linux/irq.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
>  #include <linux/of.h>
> +#include <linux/slab.h>
>  #include <asm/unaligned.h>
>  
>  struct goodix_ts_data {
--
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]

#1220242 — [PATCH v5 9/9] Input: goodix - add runtime power management support

Add support for runtime power management so that the device is
turned off when not used (when the userspace holds no open
handles of the input device). The device uses autosuspend with a
default delay of 2 seconds, so the device will suspend if no
handles to it are open for 2 seconds.

The runtime management support is only available if the gpio pins
are properly initialized from ACPI/DT.

Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
---
 drivers/input/touchscreen/goodix.c | 57 +++++++++++++++++++++++++++++++++++---
 1 file changed, 53 insertions(+), 4 deletions(-)

diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index 3179767..34c0183 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -27,6 +27,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/pm_runtime.h>
 #include <linux/slab.h>
 #include <asm/unaligned.h>
 
@@ -75,6 +76,8 @@ struct goodix_ts_data {
 #define MAX_CONTACTS_LOC	5
 #define TRIGGER_LOC		6
 
+#define GOODIX_AUTOSUSPEND_DELAY_MS	2000
+
 static const unsigned long goodix_irq_flags[] = {
 	IRQ_TYPE_EDGE_RISING,
 	IRQ_TYPE_EDGE_FALLING,
@@ -566,6 +569,27 @@ static const struct attribute_group goodix_attr_group = {
 	.attrs = goodix_attrs,
 };
 
+static int goodix_open(struct input_dev *input_dev)
+{
+	struct goodix_ts_data *ts = input_get_drvdata(input_dev);
+	int error;
+
+	error = pm_runtime_get_sync(&ts->client->dev);
+	if (error < 0) {
+		pm_runtime_put_noidle(&ts->client->dev);
+		return error;
+	}
+	return 0;
+}
+
+static void goodix_close(struct input_dev *input_dev)
+{
+	struct goodix_ts_data *ts = input_get_drvdata(input_dev);
+
+	pm_runtime_mark_last_busy(&ts->client->dev);
+	pm_runtime_put_autosuspend(&ts->client->dev);
+}
+
 /**
  * goodix_get_gpio_config - Get GPIO config from ACPI/DT
  *
@@ -751,6 +775,9 @@ static int goodix_request_input_dev(struct goodix_ts_data *ts)
 	ts->input_dev->id.vendor = 0x0416;
 	ts->input_dev->id.product = ts->id;
 	ts->input_dev->id.version = ts->version;
+	ts->input_dev->open = goodix_open;
+	ts->input_dev->close = goodix_close;
+	input_set_drvdata(ts->input_dev, ts);
 
 	error = input_register_device(ts->input_dev);
 	if (error) {
@@ -798,7 +825,8 @@ static int goodix_configure_dev(struct goodix_ts_data *ts)
  * @ts: our goodix_ts_data pointer
  *
  * request_firmware_wait callback that finishes
- * initialization of the device.
+ * initialization of the device. This will only be called
+ * when ts->gpiod_int and ts->gpiod_rst are properly initialized.
  */
 static void goodix_config_cb(const struct firmware *cfg, void *ctx)
 {
@@ -811,7 +839,21 @@ static void goodix_config_cb(const struct firmware *cfg, void *ctx)
 		if (error)
 			goto err_release_cfg;
 	}
-	goodix_configure_dev(ts);
+	error = goodix_configure_dev(ts);
+	if (error)
+		goto err_release_cfg;
+
+	error = pm_runtime_set_active(&ts->client->dev);
+	if (error) {
+		dev_err(&ts->client->dev, "failed to set active: %d\n", error);
+		goto err_release_cfg;
+	}
+	/* input_dev is a child of client->dev, ignore it for runtime pm */
+	pm_suspend_ignore_children(&ts->client->dev, true);
+	pm_runtime_enable(&ts->client->dev);
+	pm_runtime_set_autosuspend_delay(&ts->client->dev,
+					 GOODIX_AUTOSUSPEND_DELAY_MS);
+	pm_runtime_use_autosuspend(&ts->client->dev);
 
 err_release_cfg:
 	release_firmware(cfg);
@@ -915,8 +957,12 @@ static int goodix_ts_remove(struct i2c_client *client)
 {
 	struct goodix_ts_data *ts = i2c_get_clientdata(client);
 
-	if (ts->gpiod_int && ts->gpiod_rst)
+	if (ts->gpiod_int && ts->gpiod_rst) {
+		pm_runtime_disable(&client->dev);
+		pm_runtime_set_suspended(&client->dev);
+		pm_runtime_put_noidle(&client->dev);
 		sysfs_remove_group(&client->dev.kobj, &goodix_attr_group);
+	}
 	goodix_disable_esd(ts);
 	kfree(ts->cfg_name);
 	return 0;
@@ -990,7 +1036,10 @@ static int __maybe_unused goodix_resume(struct device *dev)
 	return goodix_enable_esd(ts);
 }
 
-static SIMPLE_DEV_PM_OPS(goodix_pm_ops, goodix_suspend, goodix_resume);
+static const struct dev_pm_ops goodix_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(goodix_suspend, goodix_resume)
+	SET_RUNTIME_PM_OPS(goodix_suspend, goodix_resume, NULL)
+};
 
 static const struct i2c_device_id goodix_ts_id[] = {
 	{ "GDIX1001:00", 0 },
-- 
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]

#1220243 — [PATCH v5 4/9] Input: goodix - write configuration data to device

Goodix devices can be configured by writing custom data to the device at
init. The configuration data is read with request_firmware from
"goodix_<id>_cfg.bin", where <id> is the product id read from the device
(e.g.: goodix_911_cfg.bin for Goodix GT911, goodix_9271_cfg.bin for
GT9271).

The configuration information has a specific format described in the Goodix
datasheet. It includes X/Y resolution, maximum supported touch points,
interrupt flags, various sesitivity factors and settings for advanced
features (like gesture recognition).

Before writing the firmware, it is necessary to reset the device. If
the device ACPI/DT information does not declare gpio pins (needed for
reset), writing the firmware 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 | 225 +++++++++++++++++++++++++++++++------
 1 file changed, 192 insertions(+), 33 deletions(-)

diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index 8edfc06..9cf16ff7 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -17,6 +17,7 @@
 #include <linux/acpi.h>
 #include <linux/delay.h>
 #include <linux/dmi.h>
+#include <linux/firmware.h>
 #include <linux/gpio.h>
 #include <linux/i2c.h>
 #include <linux/input.h>
@@ -40,6 +41,9 @@ struct goodix_ts_data {
 	int cfg_len;
 	struct gpio_desc *gpiod_int;
 	struct gpio_desc *gpiod_rst;
+	u16 id;
+	u16 version;
+	char *cfg_name;
 };
 
 #define GOODIX_MAX_HEIGHT		4096
@@ -145,6 +149,39 @@ static int goodix_i2c_read(struct i2c_client *client,
 	return ret < 0 ? ret : (ret != ARRAY_SIZE(msgs) ? -EIO : 0);
 }
 
+/**
+ * goodix_i2c_write - write data to a register of the i2c slave device.
+ *
+ * @client: i2c device.
+ * @reg: the register to write to.
+ * @buf: raw data buffer to write.
+ * @len: length of the buffer to write
+ */
+static int goodix_i2c_write(struct i2c_client *client, u16 reg, const u8 *buf,
+			    unsigned len)
+{
+	u8 *addr_buf;
+	struct i2c_msg msg;
+	int ret;
+
+	addr_buf = kmalloc(len + 2, GFP_KERNEL);
+	if (!addr_buf)
+		return -ENOMEM;
+
+	addr_buf[0] = reg >> 8;
+	addr_buf[1] = reg & 0xFF;
+	memcpy(&addr_buf[2], buf, len);
+
+	msg.flags = 0;
+	msg.addr = client->addr;
+	msg.buf = addr_buf;
+	msg.len = len + 2;
+
+	ret = i2c_transfer(client->adapter, &msg, 1);
+	kfree(addr_buf);
+	return ret < 0 ? ret : (ret != 1 ? -EIO : 0);
+}
+
 static int goodix_get_cfg_len(u16 id)
 {
 	switch (id) {
@@ -264,6 +301,73 @@ static irqreturn_t goodix_ts_irq_handler(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+/**
+ * goodix_check_cfg - Checks if config fw is valid
+ *
+ * @ts: goodix_ts_data pointer
+ * @cfg: firmware config data
+ */
+static int goodix_check_cfg(struct goodix_ts_data *ts,
+			    const struct firmware *cfg)
+{
+	int i, raw_cfg_len;
+	u8 check_sum = 0;
+
+	if (cfg->size > GOODIX_CONFIG_MAX_LENGTH) {
+		dev_err(&ts->client->dev,
+			"The length of the config fw is not correct");
+		return -EINVAL;
+	}
+
+	raw_cfg_len = cfg->size - 2;
+	for (i = 0; i < raw_cfg_len; i++)
+		check_sum += cfg->data[i];
+	check_sum = (~check_sum) + 1;
+	if (check_sum != cfg->data[raw_cfg_len]) {
+		dev_err(&ts->client->dev,
+			"The checksum of the config fw is not correct");
+		return -EINVAL;
+	}
+
+	if (cfg->data[raw_cfg_len + 1] != 1) {
+		dev_err(&ts->client->dev,
+			"Config fw must have Config_Fresh register set");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+/**
+ * goodix_send_cfg - Write fw config to device
+ *
+ * @ts: goodix_ts_data pointer
+ * @cfg: config firmware to write to device
+ */
+static int goodix_send_cfg(struct goodix_ts_data *ts,
+			   const struct firmware *cfg)
+{
+	int error;
+
+	error = goodix_check_cfg(ts, cfg);
+	if (error)
+		return error;
+
+	error = goodix_i2c_write(ts->client, GOODIX_REG_CONFIG_DATA, cfg->data,
+				 cfg->size);
+	if (error) {
+		dev_err(&ts->client->dev, "Failed to write config data: %d",
+			error);
+		return error;
+	}
+	dev_dbg(&ts->client->dev, "Config sent successfully.");
+
+	/* Let the firmware reconfigure itself, so sleep for 10ms */
+	usleep_range(10000, 11000);
+
+	return 0;
+}
+
 static int goodix_int_sync(struct goodix_ts_data *ts)
 {
 	int error;
@@ -406,30 +510,29 @@ static void goodix_read_config(struct goodix_ts_data *ts)
 /**
  * goodix_read_version - Read goodix touchscreen version
  *
- * @client: the i2c client
- * @version: output buffer containing the version on success
- * @id: output buffer containing the id on success
+ * @ts: our goodix_ts_data pointer
  */
-static int goodix_read_version(struct i2c_client *client, u16 *version, u16 *id)
+static int goodix_read_version(struct goodix_ts_data *ts)
 {
 	int error;
 	u8 buf[6];
 	char id_str[5];
 
-	error = goodix_i2c_read(client, GOODIX_REG_ID, buf, sizeof(buf));
+	error = goodix_i2c_read(ts->client, GOODIX_REG_ID, buf, sizeof(buf));
 	if (error) {
-		dev_err(&client->dev, "read version failed: %d\n", error);
+		dev_err(&ts->client->dev, "read version failed: %d\n", error);
 		return error;
 	}
 
 	memcpy(id_str, buf, 4);
 	id_str[4] = 0;
-	if (kstrtou16(id_str, 10, id))
-		*id = 0x1001;
+	if (kstrtou16(id_str, 10, &ts->id))
+		ts->id = 0x1001;
 
-	*version = get_unaligned_le16(&buf[4]);
+	ts->version = get_unaligned_le16(&buf[4]);
 
-	dev_info(&client->dev, "ID %d, version: %04x\n", *id, *version);
+	dev_info(&ts->client->dev, "ID %d, version: %04x\n", ts->id,
+		 ts->version);
 
 	return 0;
 }
@@ -463,13 +566,10 @@ static int goodix_i2c_test(struct i2c_client *client)
  * goodix_request_input_dev - Allocate, populate and register the input device
  *
  * @ts: our goodix_ts_data pointer
- * @version: device firmware version
- * @id: device ID
  *
  * Must be called during probe
  */
-static int goodix_request_input_dev(struct goodix_ts_data *ts, u16 version,
-				    u16 id)
+static int goodix_request_input_dev(struct goodix_ts_data *ts)
 {
 	int error;
 
@@ -493,8 +593,8 @@ static int goodix_request_input_dev(struct goodix_ts_data *ts, u16 version,
 	ts->input_dev->phys = "input/ts";
 	ts->input_dev->id.bustype = BUS_I2C;
 	ts->input_dev->id.vendor = 0x0416;
-	ts->input_dev->id.product = id;
-	ts->input_dev->id.version = version;
+	ts->input_dev->id.product = ts->id;
+	ts->input_dev->id.version = ts->version;
 
 	error = input_register_device(ts->input_dev);
 	if (error) {
@@ -506,13 +606,68 @@ static int goodix_request_input_dev(struct goodix_ts_data *ts, u16 version,
 	return 0;
 }
 
+/**
+ * goodix_configure_dev - Finish device initialization
+ *
+ * @ts: our goodix_ts_data pointer
+ *
+ * Must be called from request_firmware_wait callback to
+ * finish initialization of the device.
+ */
+static int goodix_configure_dev(struct goodix_ts_data *ts)
+{
+	int error;
+	unsigned long irq_flags;
+
+	goodix_read_config(ts);
+
+	error = goodix_request_input_dev(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);
+	if (error) {
+		dev_err(&ts->client->dev, "request IRQ failed: %d\n", error);
+		return error;
+	}
+
+	return 0;
+}
+
+/**
+ * goodix_config_cb - Callback to finish device init
+ *
+ * @ts: our goodix_ts_data pointer
+ *
+ * request_firmware_wait callback that finishes
+ * initialization of the device.
+ */
+static void goodix_config_cb(const struct firmware *cfg, void *ctx)
+{
+	struct goodix_ts_data *ts = (struct goodix_ts_data *)ctx;
+	int error;
+
+	if (cfg) {
+		/* send device configuration to the firmware */
+		error = goodix_send_cfg(ts, cfg);
+		if (error)
+			goto err_release_cfg;
+	}
+	goodix_configure_dev(ts);
+
+err_release_cfg:
+	kfree(ts->cfg_name);
+	release_firmware(cfg);
+}
+
 static int goodix_ts_probe(struct i2c_client *client,
 			   const struct i2c_device_id *id)
 {
 	struct goodix_ts_data *ts;
-	unsigned long irq_flags;
 	int error;
-	u16 version_info, id_info;
 
 	dev_dbg(&client->dev, "I2C Address: 0x%02x\n", client->addr);
 
@@ -534,13 +689,13 @@ static int goodix_ts_probe(struct i2c_client *client,
 		return error;
 	}
 
-	error = goodix_read_version(client, &version_info, &id_info);
+	error = goodix_read_version(ts);
 	if (error) {
 		dev_err(&client->dev, "Read version failed.\n");
 		return error;
 	}
 
-	ts->cfg_len = goodix_get_cfg_len(id_info);
+	ts->cfg_len = goodix_get_cfg_len(ts->id);
 
 	error = goodix_get_gpio_config(ts, id);
 	if (error)
@@ -553,24 +708,28 @@ static int goodix_ts_probe(struct i2c_client *client,
 			dev_err(&client->dev, "Controller reset failed.\n");
 			return error;
 		}
-	}
 
-	goodix_read_config(ts);
+		/* update device config */
+		ts->cfg_name = kasprintf(GFP_KERNEL, "goodix_%d_cfg.bin",
+					 ts->id);
+		if (!ts->cfg_name)
+			return -ENOMEM;
 
-	error = goodix_request_input_dev(ts, version_info, id_info);
-	if (error)
-		return error;
+		error = request_firmware_nowait(THIS_MODULE, true, ts->cfg_name,
+						&client->dev, GFP_KERNEL, ts,
+						goodix_config_cb);
+		if (error) {
+			dev_err(&client->dev,
+				"Failed to invoke firmware loader: %d\n",
+				error);
+			kfree(ts->cfg_name);
+			return error;
+		}
 
-	irq_flags = goodix_irq_flags[ts->int_trigger_type] | IRQF_ONESHOT;
-	error = devm_request_threaded_irq(&ts->client->dev, client->irq,
-					  NULL, goodix_ts_irq_handler,
-					  irq_flags, client->name, ts);
-	if (error) {
-		dev_err(&client->dev, "request IRQ failed: %d\n", error);
-		return error;
+		return 0;
 	}
 
-	return 0;
+	return goodix_configure_dev(ts);
 }
 
 static const struct i2c_device_id goodix_ts_id[] = {
-- 
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]

#1220249 — [PATCH v5 2/9] Input: goodix - use actual config length for each device type

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>
---
 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 6ae28c5..7be6eab 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]

#1221570 — Re: [PATCH v5 2/9] Input: goodix - use actual config length for each device type

On Mon, 2015-09-07 at 17:36 +0300, Irina Tirdea wrote:
> 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>

> ---
>  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 6ae28c5..7be6eab 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);
--
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