* Minor fixes for atmel_mxt_ts
@ 2014-08-07 14:07 nick.dyer
2014-08-07 14:07 ` [PATCH 1/2] Input: atmel_mxt_ts - Fix double free of input device nick.dyer
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: nick.dyer @ 2014-08-07 14:07 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Yufeng Shen, Daniel Kurtz, Henrik Rydberg, Joonyoung Shim,
Alan Bowens, linux-input, linux-kernel, Peter Meerwald,
Benson Leung, Olof Johansson, Sekhar Nori, Stephen Warren
Hi Dmitry-
It would be good to get these minor fixes applied to atmel_mxt_ts.
regards
Nick Dyer
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] Input: atmel_mxt_ts - Fix double free of input device
2014-08-07 14:07 Minor fixes for atmel_mxt_ts nick.dyer
@ 2014-08-07 14:07 ` nick.dyer
2014-08-07 16:55 ` Dmitry Torokhov
2014-08-07 14:07 ` [PATCH 2/2] Input: atmel_mxt_ts - mXT224 DMA quirk was fixed in firmware v2.0.AA nick.dyer
2014-08-07 16:59 ` Minor fixes for atmel_mxt_ts Dmitry Torokhov
2 siblings, 1 reply; 11+ messages in thread
From: nick.dyer @ 2014-08-07 14:07 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Yufeng Shen, Daniel Kurtz, Henrik Rydberg, Joonyoung Shim,
Alan Bowens, linux-input, linux-kernel, Peter Meerwald,
Benson Leung, Olof Johansson, Sekhar Nori, Stephen Warren,
Nick Dyer
From: Stephen Warren <swarren@wwwdotorg.org>
Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
---
drivers/input/touchscreen/atmel_mxt_ts.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 03b8571..da497db 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -1353,8 +1353,10 @@ static int mxt_get_info(struct mxt_data *data)
static void mxt_free_object_table(struct mxt_data *data)
{
- input_unregister_device(data->input_dev);
- data->input_dev = NULL;
+ if (data->input_dev) {
+ input_unregister_device(data->input_dev);
+ data->input_dev = NULL;
+ }
kfree(data->object_table);
data->object_table = NULL;
@@ -2194,7 +2196,6 @@ static int mxt_remove(struct i2c_client *client)
sysfs_remove_group(&client->dev.kobj, &mxt_attr_group);
free_irq(data->irq, data);
- input_unregister_device(data->input_dev);
mxt_free_object_table(data);
kfree(data);
--
1.9.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] Input: atmel_mxt_ts - mXT224 DMA quirk was fixed in firmware v2.0.AA
2014-08-07 14:07 Minor fixes for atmel_mxt_ts nick.dyer
2014-08-07 14:07 ` [PATCH 1/2] Input: atmel_mxt_ts - Fix double free of input device nick.dyer
@ 2014-08-07 14:07 ` nick.dyer
2014-08-26 20:10 ` Benson Leung
2014-08-07 16:59 ` Minor fixes for atmel_mxt_ts Dmitry Torokhov
2 siblings, 1 reply; 11+ messages in thread
From: nick.dyer @ 2014-08-07 14:07 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Yufeng Shen, Daniel Kurtz, Henrik Rydberg, Joonyoung Shim,
Alan Bowens, linux-input, linux-kernel, Peter Meerwald,
Benson Leung, Olof Johansson, Sekhar Nori, Stephen Warren,
Nick Dyer
From: Nick Dyer <nick.dyer@itdev.co.uk>
Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
---
drivers/input/touchscreen/atmel_mxt_ts.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index da497db..67ea728 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -1424,10 +1424,12 @@ static int mxt_get_object_table(struct mxt_data *data)
switch (object->type) {
case MXT_GEN_MESSAGE_T5:
- if (data->info.family_id == 0x80) {
+ if ((data->info.family_id == 0x80)
+ && (data->info.version < 0x20)) {
/*
- * On mXT224 read and discard unused CRC byte
- * otherwise DMA reads are misaligned
+ * On mXT224 firmware versions prior to V2.0
+ * read and discard unused CRC byte otherwise
+ * DMA reads are misaligned
*/
data->T5_msg_size = mxt_obj_size(object);
} else {
--
1.9.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] Input: atmel_mxt_ts - Fix double free of input device
2014-08-07 14:07 ` [PATCH 1/2] Input: atmel_mxt_ts - Fix double free of input device nick.dyer
@ 2014-08-07 16:55 ` Dmitry Torokhov
0 siblings, 0 replies; 11+ messages in thread
From: Dmitry Torokhov @ 2014-08-07 16:55 UTC (permalink / raw)
To: nick.dyer
Cc: Yufeng Shen, Daniel Kurtz, Henrik Rydberg, Joonyoung Shim,
Alan Bowens, linux-input, linux-kernel, Peter Meerwald,
Benson Leung, Olof Johansson, Sekhar Nori, Stephen Warren
On Thu, Aug 07, 2014 at 03:07:17PM +0100, nick.dyer@itdev.co.uk wrote:
> From: Stephen Warren <swarren@wwwdotorg.org>
>
> Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
> ---
> drivers/input/touchscreen/atmel_mxt_ts.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> index 03b8571..da497db 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -1353,8 +1353,10 @@ static int mxt_get_info(struct mxt_data *data)
>
> static void mxt_free_object_table(struct mxt_data *data)
> {
> - input_unregister_device(data->input_dev);
> - data->input_dev = NULL;
> + if (data->input_dev) {
> + input_unregister_device(data->input_dev);
> + data->input_dev = NULL;
> + }
>
> kfree(data->object_table);
> data->object_table = NULL;
> @@ -2194,7 +2196,6 @@ static int mxt_remove(struct i2c_client *client)
>
> sysfs_remove_group(&client->dev.kobj, &mxt_attr_group);
> free_irq(data->irq, data);
> - input_unregister_device(data->input_dev);
> mxt_free_object_table(data);
> kfree(data);
No. Let's have mxt_free_object_table() only free object table, and let's
call input_unregister_device() explicitly where needed, like we do in
mxt_remove().
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Minor fixes for atmel_mxt_ts
2014-08-07 14:07 Minor fixes for atmel_mxt_ts nick.dyer
2014-08-07 14:07 ` [PATCH 1/2] Input: atmel_mxt_ts - Fix double free of input device nick.dyer
2014-08-07 14:07 ` [PATCH 2/2] Input: atmel_mxt_ts - mXT224 DMA quirk was fixed in firmware v2.0.AA nick.dyer
@ 2014-08-07 16:59 ` Dmitry Torokhov
2014-08-08 15:58 ` nick.dyer
2 siblings, 1 reply; 11+ messages in thread
From: Dmitry Torokhov @ 2014-08-07 16:59 UTC (permalink / raw)
To: nick.dyer
Cc: Yufeng Shen, Daniel Kurtz, Henrik Rydberg, Joonyoung Shim,
Alan Bowens, linux-input, linux-kernel, Peter Meerwald,
Benson Leung, Olof Johansson, Sekhar Nori, Stephen Warren
Hi Nick,
On Thu, Aug 07, 2014 at 03:07:16PM +0100, nick.dyer@itdev.co.uk wrote:
> Hi Dmitry-
>
> It would be good to get these minor fixes applied to atmel_mxt_ts.
>
I applied your 2nd batch but it will have to go in 2nd batch as I am
ready to post pull request.
BTW, did you have any issues with the 2 patches I posted a couple weeks
ago or I can apply them as well?
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Minor fixes for atmel_mxt_ts
2014-08-07 16:59 ` Minor fixes for atmel_mxt_ts Dmitry Torokhov
@ 2014-08-08 15:58 ` nick.dyer
2014-08-08 15:58 ` [PATCH 1/2] Input: atmel_mxt_ts - simplify mxt_initialize a bit nick.dyer
2014-08-08 15:58 ` [PATCH 2/2] Input: atmel_mxt_ts - split config update " nick.dyer
0 siblings, 2 replies; 11+ messages in thread
From: nick.dyer @ 2014-08-08 15:58 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Yufeng Shen, Daniel Kurtz, Henrik Rydberg, Joonyoung Shim,
Alan Bowens, linux-input, linux-kernel, Peter Meerwald,
Benson Leung, Olof Johansson, Sekhar Nori, Stephen Warren
Hi Dmitry-
I tested both your patches, here they are rebased on your current tree and
with my signoff.
I need to have a little more of a play with the change to the
input_unregister_device code before putting it it. I want to be sure that all
the possible probe failures do the sensible thing. Hopefully will get that
done early next week.
regards
Nick
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] Input: atmel_mxt_ts - simplify mxt_initialize a bit
2014-08-08 15:58 ` nick.dyer
@ 2014-08-08 15:58 ` nick.dyer
2014-08-08 16:19 ` Dmitry Torokhov
2014-08-08 15:58 ` [PATCH 2/2] Input: atmel_mxt_ts - split config update " nick.dyer
1 sibling, 1 reply; 11+ messages in thread
From: nick.dyer @ 2014-08-08 15:58 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Yufeng Shen, Daniel Kurtz, Henrik Rydberg, Joonyoung Shim,
Alan Bowens, linux-input, linux-kernel, Peter Meerwald,
Benson Leung, Olof Johansson, Sekhar Nori, Stephen Warren,
Nick Dyer
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
I think having control flow with 2 goto/labels/flags is quite hard to read,
this version is a bit more readable IMO.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
---
drivers/input/touchscreen/atmel_mxt_ts.c | 81 +++++++++++++++++---------------
1 file changed, 42 insertions(+), 39 deletions(-)
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index d50c614..a5f943d 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -359,7 +359,6 @@ static int mxt_bootloader_read(struct mxt_data *data,
msg.buf = val;
ret = i2c_transfer(data->client->adapter, &msg, 1);
-
if (ret == 1) {
ret = 0;
} else {
@@ -414,6 +413,7 @@ static int mxt_lookup_bootloader_address(struct mxt_data *data, bool retry)
case 0x5b:
bootloader = appmode - 0x26;
break;
+
default:
dev_err(&data->client->dev,
"Appmode i2c address 0x%02x not found\n",
@@ -425,20 +425,20 @@ static int mxt_lookup_bootloader_address(struct mxt_data *data, bool retry)
return 0;
}
-static int mxt_probe_bootloader(struct mxt_data *data, bool retry)
+static int mxt_probe_bootloader(struct mxt_data *data, bool alt_address)
{
struct device *dev = &data->client->dev;
- int ret;
+ int error;
u8 val;
bool crc_failure;
- ret = mxt_lookup_bootloader_address(data, retry);
- if (ret)
- return ret;
+ error = mxt_lookup_bootloader_address(data, alt_address);
+ if (error)
+ return error;
- ret = mxt_bootloader_read(data, &val, 1);
- if (ret)
- return ret;
+ error = mxt_bootloader_read(data, &val, 1);
+ if (error)
+ return error;
/* Check app crc fail mode */
crc_failure = (val & ~MXT_BOOT_STATUS_MASK) == MXT_APP_CRC_FAIL;
@@ -1645,41 +1645,39 @@ static void mxt_config_cb(const struct firmware *cfg, void *ctx)
static int mxt_initialize(struct mxt_data *data)
{
struct i2c_client *client = data->client;
+ int recovery_attempts = 0;
int error;
- bool alt_bootloader_addr = false;
- bool retry = false;
-retry_info:
- error = mxt_get_info(data);
- if (error) {
-retry_bootloader:
- error = mxt_probe_bootloader(data, alt_bootloader_addr);
+ while (1) {
+ error = mxt_get_info(data);
+ if (!error)
+ break;
+
+ /* Check bootloader state */
+ error = mxt_probe_bootloader(data, false);
if (error) {
- if (alt_bootloader_addr) {
+ dev_info(&client->dev, "Trying alternate bootloader address\n");
+ error = mxt_probe_bootloader(data, true);
+ if (error) {
/* Chip is not in appmode or bootloader mode */
return error;
}
+ }
- dev_info(&client->dev, "Trying alternate bootloader address\n");
- alt_bootloader_addr = true;
- goto retry_bootloader;
- } else {
- if (retry) {
- dev_err(&client->dev, "Could not recover from bootloader mode\n");
- /*
- * We can reflash from this state, so do not
- * abort init
- */
- data->in_bootloader = true;
- return 0;
- }
-
- /* Attempt to exit bootloader into app mode */
- mxt_send_bootloader_cmd(data, false);
- msleep(MXT_FW_RESET_TIME);
- retry = true;
- goto retry_info;
+ /* OK, we are in bootloader, see if we can recover */
+ if (++recovery_attempts > 1) {
+ dev_err(&client->dev, "Could not recover from bootloader mode\n");
+ /*
+ * We can reflash from this state, so do not
+ * abort initialization.
+ */
+ data->in_bootloader = true;
+ return 0;
}
+
+ /* Attempt to exit bootloader into app mode */
+ mxt_send_bootloader_cmd(data, false);
+ msleep(MXT_FW_RESET_TIME);
}
/* Get object table information */
@@ -1693,9 +1691,14 @@ retry_bootloader:
if (error)
goto err_free_object_table;
- request_firmware_nowait(THIS_MODULE, true, MXT_CFG_NAME,
- &data->client->dev, GFP_KERNEL, data,
- mxt_config_cb);
+ error = request_firmware_nowait(THIS_MODULE, true, MXT_CFG_NAME,
+ &client->dev, GFP_KERNEL, data,
+ mxt_config_cb);
+ if (error) {
+ dev_err(&client->dev, "Failed to invoke firmware loader: %d\n",
+ error);
+ goto err_free_object_table;
+ }
return 0;
--
1.9.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] Input: atmel_mxt_ts - split config update a bit
2014-08-08 15:58 ` nick.dyer
2014-08-08 15:58 ` [PATCH 1/2] Input: atmel_mxt_ts - simplify mxt_initialize a bit nick.dyer
@ 2014-08-08 15:58 ` nick.dyer
1 sibling, 0 replies; 11+ messages in thread
From: nick.dyer @ 2014-08-08 15:58 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Yufeng Shen, Daniel Kurtz, Henrik Rydberg, Joonyoung Shim,
Alan Bowens, linux-input, linux-kernel, Peter Meerwald,
Benson Leung, Olof Johansson, Sekhar Nori, Stephen Warren,
Nick Dyer
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Let's split config update code a bit so it is hopefully a bit easier to
read. Also, the firmware update callback should be the entity releasing
firmware blob, not lower layers.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
---
drivers/input/touchscreen/atmel_mxt_ts.c | 270 +++++++++++++++++--------------
1 file changed, 145 insertions(+), 125 deletions(-)
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index a5f943d..bbaf3ff 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -1064,6 +1064,133 @@ static u32 mxt_calculate_crc(u8 *base, off_t start_off, off_t end_off)
return crc;
}
+static int mxt_prepare_cfg_mem(struct mxt_data *data,
+ const struct firmware *cfg,
+ unsigned int data_pos,
+ unsigned int cfg_start_ofs,
+ u8 *config_mem,
+ size_t config_mem_size)
+{
+ struct device *dev = &data->client->dev;
+ struct mxt_object *object;
+ unsigned int type, instance, size, byte_offset;
+ int offset;
+ int ret;
+ int i;
+ u16 reg;
+ u8 val;
+
+ while (data_pos < cfg->size) {
+ /* Read type, instance, length */
+ ret = sscanf(cfg->data + data_pos, "%x %x %x%n",
+ &type, &instance, &size, &offset);
+ if (ret == 0) {
+ /* EOF */
+ break;
+ } else if (ret != 3) {
+ dev_err(dev, "Bad format: failed to parse object\n");
+ return -EINVAL;
+ }
+ data_pos += offset;
+
+ object = mxt_get_object(data, type);
+ if (!object) {
+ /* Skip object */
+ for (i = 0; i < size; i++) {
+ ret = sscanf(cfg->data + data_pos, "%hhx%n",
+ &val,
+ &offset);
+ data_pos += offset;
+ }
+ continue;
+ }
+
+ if (size > mxt_obj_size(object)) {
+ /*
+ * Either we are in fallback mode due to wrong
+ * config or config from a later fw version,
+ * or the file is corrupt or hand-edited.
+ */
+ dev_warn(dev, "Discarding %zu byte(s) in T%u\n",
+ size - mxt_obj_size(object), type);
+ } else if (mxt_obj_size(object) > size) {
+ /*
+ * If firmware is upgraded, new bytes may be added to
+ * end of objects. It is generally forward compatible
+ * to zero these bytes - previous behaviour will be
+ * retained. However this does invalidate the CRC and
+ * will force fallback mode until the configuration is
+ * updated. We warn here but do nothing else - the
+ * malloc has zeroed the entire configuration.
+ */
+ dev_warn(dev, "Zeroing %zu byte(s) in T%d\n",
+ mxt_obj_size(object) - size, type);
+ }
+
+ if (instance >= mxt_obj_instances(object)) {
+ dev_err(dev, "Object instances exceeded!\n");
+ return -EINVAL;
+ }
+
+ reg = object->start_address + mxt_obj_size(object) * instance;
+
+ for (i = 0; i < size; i++) {
+ ret = sscanf(cfg->data + data_pos, "%hhx%n",
+ &val,
+ &offset);
+ if (ret != 1) {
+ dev_err(dev, "Bad format in T%d\n", type);
+ return -EINVAL;
+ }
+ data_pos += offset;
+
+ if (i > mxt_obj_size(object))
+ continue;
+
+ byte_offset = reg + i - cfg_start_ofs;
+
+ if (byte_offset >= 0 &&
+ byte_offset <= config_mem_size) {
+ *(config_mem + byte_offset) = val;
+ } else {
+ dev_err(dev, "Bad object: reg:%d, T%d, ofs=%d\n",
+ reg, object->type, byte_offset);
+ return -EINVAL;
+ }
+ }
+ }
+
+ return 0;
+}
+
+static int mxt_upload_cfg_mem(struct mxt_data *data, unsigned int cfg_start,
+ u8 *config_mem, size_t config_mem_size)
+{
+ unsigned int byte_offset = 0;
+ int error;
+
+ /* Write configuration as blocks */
+ while (byte_offset < config_mem_size) {
+ unsigned int size = config_mem_size - byte_offset;
+
+ if (size > MXT_MAX_BLOCK_WRITE)
+ size = MXT_MAX_BLOCK_WRITE;
+
+ error = __mxt_write_reg(data->client,
+ cfg_start + byte_offset,
+ size, config_mem + byte_offset);
+ if (error) {
+ dev_err(&data->client->dev,
+ "Config write error, ret=%d\n", error);
+ return error;
+ }
+
+ byte_offset += size;
+ }
+
+ return 0;
+}
+
/*
* mxt_update_cfg - download configuration to chip
*
@@ -1087,26 +1214,20 @@ static int mxt_update_cfg(struct mxt_data *data, const struct firmware *cfg)
{
struct device *dev = &data->client->dev;
struct mxt_info cfg_info;
- struct mxt_object *object;
int ret;
int offset;
int data_pos;
- int byte_offset;
int i;
int cfg_start_ofs;
u32 info_crc, config_crc, calculated_crc;
u8 *config_mem;
size_t config_mem_size;
- unsigned int type, instance, size;
- u8 val;
- u16 reg;
mxt_update_crc(data, MXT_COMMAND_REPORTALL, 1);
if (strncmp(cfg->data, MXT_CFG_MAGIC, strlen(MXT_CFG_MAGIC))) {
dev_err(dev, "Unrecognised config file\n");
- ret = -EINVAL;
- goto release;
+ return -EINVAL;
}
data_pos = strlen(MXT_CFG_MAGIC);
@@ -1118,8 +1239,7 @@ static int mxt_update_cfg(struct mxt_data *data, const struct firmware *cfg)
&offset);
if (ret != 1) {
dev_err(dev, "Bad format\n");
- ret = -EINVAL;
- goto release;
+ return -EINVAL;
}
data_pos += offset;
@@ -1127,30 +1247,26 @@ static int mxt_update_cfg(struct mxt_data *data, const struct firmware *cfg)
if (cfg_info.family_id != data->info.family_id) {
dev_err(dev, "Family ID mismatch!\n");
- ret = -EINVAL;
- goto release;
+ return -EINVAL;
}
if (cfg_info.variant_id != data->info.variant_id) {
dev_err(dev, "Variant ID mismatch!\n");
- ret = -EINVAL;
- goto release;
+ return -EINVAL;
}
/* Read CRCs */
ret = sscanf(cfg->data + data_pos, "%x%n", &info_crc, &offset);
if (ret != 1) {
dev_err(dev, "Bad format: failed to parse Info CRC\n");
- ret = -EINVAL;
- goto release;
+ return -EINVAL;
}
data_pos += offset;
ret = sscanf(cfg->data + data_pos, "%x%n", &config_crc, &offset);
if (ret != 1) {
dev_err(dev, "Bad format: failed to parse Config CRC\n");
- ret = -EINVAL;
- goto release;
+ return -EINVAL;
}
data_pos += offset;
@@ -1166,8 +1282,7 @@ static int mxt_update_cfg(struct mxt_data *data, const struct firmware *cfg)
} else if (config_crc == data->config_crc) {
dev_dbg(dev, "Config CRC 0x%06X: OK\n",
data->config_crc);
- ret = 0;
- goto release;
+ return 0;
} else {
dev_info(dev, "Config CRC 0x%06X: does not match file 0x%06X\n",
data->config_crc, config_crc);
@@ -1186,93 +1301,13 @@ static int mxt_update_cfg(struct mxt_data *data, const struct firmware *cfg)
config_mem = kzalloc(config_mem_size, GFP_KERNEL);
if (!config_mem) {
dev_err(dev, "Failed to allocate memory\n");
- ret = -ENOMEM;
- goto release;
+ return -ENOMEM;
}
- while (data_pos < cfg->size) {
- /* Read type, instance, length */
- ret = sscanf(cfg->data + data_pos, "%x %x %x%n",
- &type, &instance, &size, &offset);
- if (ret == 0) {
- /* EOF */
- break;
- } else if (ret != 3) {
- dev_err(dev, "Bad format: failed to parse object\n");
- ret = -EINVAL;
- goto release_mem;
- }
- data_pos += offset;
-
- object = mxt_get_object(data, type);
- if (!object) {
- /* Skip object */
- for (i = 0; i < size; i++) {
- ret = sscanf(cfg->data + data_pos, "%hhx%n",
- &val,
- &offset);
- data_pos += offset;
- }
- continue;
- }
-
- if (size > mxt_obj_size(object)) {
- /*
- * Either we are in fallback mode due to wrong
- * config or config from a later fw version,
- * or the file is corrupt or hand-edited.
- */
- dev_warn(dev, "Discarding %zu byte(s) in T%u\n",
- size - mxt_obj_size(object), type);
- } else if (mxt_obj_size(object) > size) {
- /*
- * If firmware is upgraded, new bytes may be added to
- * end of objects. It is generally forward compatible
- * to zero these bytes - previous behaviour will be
- * retained. However this does invalidate the CRC and
- * will force fallback mode until the configuration is
- * updated. We warn here but do nothing else - the
- * malloc has zeroed the entire configuration.
- */
- dev_warn(dev, "Zeroing %zu byte(s) in T%d\n",
- mxt_obj_size(object) - size, type);
- }
-
- if (instance >= mxt_obj_instances(object)) {
- dev_err(dev, "Object instances exceeded!\n");
- ret = -EINVAL;
- goto release_mem;
- }
-
- reg = object->start_address + mxt_obj_size(object) * instance;
-
- for (i = 0; i < size; i++) {
- ret = sscanf(cfg->data + data_pos, "%hhx%n",
- &val,
- &offset);
- if (ret != 1) {
- dev_err(dev, "Bad format in T%d\n", type);
- ret = -EINVAL;
- goto release_mem;
- }
- data_pos += offset;
-
- if (i > mxt_obj_size(object))
- continue;
-
- byte_offset = reg + i - cfg_start_ofs;
-
- if ((byte_offset >= 0)
- && (byte_offset <= config_mem_size)) {
- *(config_mem + byte_offset) = val;
- } else {
- dev_err(dev, "Bad object: reg:%d, T%d, ofs=%d\n",
- reg, object->type, byte_offset);
- ret = -EINVAL;
- goto release_mem;
- }
- }
- }
+ ret = mxt_prepare_cfg_mem(data, cfg, data_pos, cfg_start_ofs,
+ config_mem, config_mem_size);
+ if (ret)
+ goto release_mem;
/* Calculate crc of the received configs (not the raw config file) */
if (data->T7_address < cfg_start_ofs) {
@@ -1286,28 +1321,14 @@ static int mxt_update_cfg(struct mxt_data *data, const struct firmware *cfg)
data->T7_address - cfg_start_ofs,
config_mem_size);
- if (config_crc > 0 && (config_crc != calculated_crc))
+ if (config_crc > 0 && config_crc != calculated_crc)
dev_warn(dev, "Config CRC error, calculated=%06X, file=%06X\n",
calculated_crc, config_crc);
- /* Write configuration as blocks */
- byte_offset = 0;
- while (byte_offset < config_mem_size) {
- size = config_mem_size - byte_offset;
-
- if (size > MXT_MAX_BLOCK_WRITE)
- size = MXT_MAX_BLOCK_WRITE;
-
- ret = __mxt_write_reg(data->client,
- cfg_start_ofs + byte_offset,
- size, config_mem + byte_offset);
- if (ret != 0) {
- dev_err(dev, "Config write error, ret=%d\n", ret);
- goto release_mem;
- }
-
- byte_offset += size;
- }
+ ret = mxt_upload_cfg_mem(data, cfg_start_ofs,
+ config_mem, config_mem_size);
+ if (ret)
+ goto release_mem;
mxt_update_crc(data, MXT_COMMAND_BACKUPNV, MXT_BACKUP_VALUE);
@@ -1319,8 +1340,6 @@ static int mxt_update_cfg(struct mxt_data *data, const struct firmware *cfg)
release_mem:
kfree(config_mem);
-release:
- release_firmware(cfg);
return ret;
}
@@ -1640,6 +1659,7 @@ static int mxt_configure_objects(struct mxt_data *data,
static void mxt_config_cb(const struct firmware *cfg, void *ctx)
{
mxt_configure_objects(ctx, cfg);
+ release_firmware(cfg);
}
static int mxt_initialize(struct mxt_data *data)
--
1.9.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] Input: atmel_mxt_ts - simplify mxt_initialize a bit
2014-08-08 15:58 ` [PATCH 1/2] Input: atmel_mxt_ts - simplify mxt_initialize a bit nick.dyer
@ 2014-08-08 16:19 ` Dmitry Torokhov
2014-08-08 16:19 ` Dmitry Torokhov
0 siblings, 1 reply; 11+ messages in thread
From: Dmitry Torokhov @ 2014-08-08 16:19 UTC (permalink / raw)
To: nick.dyer
Cc: Yufeng Shen, Daniel Kurtz, Henrik Rydberg, Joonyoung Shim,
Alan Bowens, linux-input, linux-kernel, Peter Meerwald,
Benson Leung, Olof Johansson, Sekhar Nori, Stephen Warren
On Fri, Aug 08, 2014 at 04:58:02PM +0100, nick.dyer@itdev.co.uk wrote:
> From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>
> I think having control flow with 2 goto/labels/flags is quite hard to read,
> this version is a bit more readable IMO.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
Nick, are there any changes from the version I posted? (If not then I do
not need to drop my patch from the queue and re-import this one).
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] Input: atmel_mxt_ts - simplify mxt_initialize a bit
2014-08-08 16:19 ` Dmitry Torokhov
@ 2014-08-08 16:19 ` Dmitry Torokhov
0 siblings, 0 replies; 11+ messages in thread
From: Dmitry Torokhov @ 2014-08-08 16:19 UTC (permalink / raw)
To: nick.dyer
Cc: Yufeng Shen, Daniel Kurtz, Henrik Rydberg, Joonyoung Shim,
Alan Bowens, linux-input, linux-kernel, Peter Meerwald,
Benson Leung, Olof Johansson, Sekhar Nori, Stephen Warren
On Fri, Aug 08, 2014 at 09:19:01AM -0700, Dmitry Torokhov wrote:
> On Fri, Aug 08, 2014 at 04:58:02PM +0100, nick.dyer@itdev.co.uk wrote:
> > From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> >
> > I think having control flow with 2 goto/labels/flags is quite hard to read,
> > this version is a bit more readable IMO.
> >
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
>
> Nick, are there any changes from the version I posted? (If not then I do
> not need to drop my patch from the queue and re-import this one).
Ah, sorry, should have read all my email before replying...
--
Dmitry
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] Input: atmel_mxt_ts - mXT224 DMA quirk was fixed in firmware v2.0.AA
2014-08-07 14:07 ` [PATCH 2/2] Input: atmel_mxt_ts - mXT224 DMA quirk was fixed in firmware v2.0.AA nick.dyer
@ 2014-08-26 20:10 ` Benson Leung
0 siblings, 0 replies; 11+ messages in thread
From: Benson Leung @ 2014-08-26 20:10 UTC (permalink / raw)
To: Nick Dyer
Cc: Dmitry Torokhov, Yufeng Shen, Daniel Kurtz, Henrik Rydberg,
Joonyoung Shim, Alan Bowens, linux-input, linux-kernel,
Peter Meerwald, Olof Johansson, Sekhar Nori, Stephen Warren
On Thu, Aug 7, 2014 at 7:07 AM, <nick.dyer@itdev.co.uk> wrote:
>
> From: Nick Dyer <nick.dyer@itdev.co.uk>
>
> Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
Reviewed-by: Benson Leung <bleung@chromium.org>
--
Benson Leung
Software Engineer, Chrom* OS
bleung@chromium.org
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2014-08-26 20:10 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-07 14:07 Minor fixes for atmel_mxt_ts nick.dyer
2014-08-07 14:07 ` [PATCH 1/2] Input: atmel_mxt_ts - Fix double free of input device nick.dyer
2014-08-07 16:55 ` Dmitry Torokhov
2014-08-07 14:07 ` [PATCH 2/2] Input: atmel_mxt_ts - mXT224 DMA quirk was fixed in firmware v2.0.AA nick.dyer
2014-08-26 20:10 ` Benson Leung
2014-08-07 16:59 ` Minor fixes for atmel_mxt_ts Dmitry Torokhov
2014-08-08 15:58 ` nick.dyer
2014-08-08 15:58 ` [PATCH 1/2] Input: atmel_mxt_ts - simplify mxt_initialize a bit nick.dyer
2014-08-08 16:19 ` Dmitry Torokhov
2014-08-08 16:19 ` Dmitry Torokhov
2014-08-08 15:58 ` [PATCH 2/2] Input: atmel_mxt_ts - split config update " nick.dyer
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.