From 437d4f3797041942947ec838cf5d65f770562c5d Mon Sep 17 00:00:00 2001 From: Nick Dyer Date: Thu, 7 Aug 2014 09:56:01 -0700 Subject: Input: atmel_mxt_ts - mXT224 DMA quirk was fixed in firmware v2.0.AA Signed-off-by: Nick Dyer Signed-off-by: Dmitry Torokhov --- drivers/input/touchscreen/atmel_mxt_ts.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'drivers/input/touchscreen') diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c index 03b85711cb7..d50c6147bb7 100644 --- a/drivers/input/touchscreen/atmel_mxt_ts.c +++ b/drivers/input/touchscreen/atmel_mxt_ts.c @@ -1422,10 +1422,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 { -- cgit v1.2.3-70-g09d2 From 6cd1ab0fb67f21e6e35eee35106d8dd2f08b544c Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov Date: Fri, 8 Aug 2014 09:28:45 -0700 Subject: Input: atmel_mxt_ts - simplify mxt_initialize a bit 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 Signed-off-by: Nick Dyer --- drivers/input/touchscreen/atmel_mxt_ts.c | 81 +++++++++++++++++--------------- 1 file changed, 42 insertions(+), 39 deletions(-) (limited to 'drivers/input/touchscreen') diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c index d50c6147bb7..a5f943dedb2 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; -- cgit v1.2.3-70-g09d2 From efdbd7ae44f35cbb2b29d825640b917723319f7b Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov Date: Fri, 8 Aug 2014 09:29:06 -0700 Subject: Input: atmel_mxt_ts - split config update a bit 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 Signed-off-by: Nick Dyer --- drivers/input/touchscreen/atmel_mxt_ts.c | 270 +++++++++++++++++-------------- 1 file changed, 145 insertions(+), 125 deletions(-) (limited to 'drivers/input/touchscreen') diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c index a5f943dedb2..bbaf3ff680e 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) -- cgit v1.2.3-70-g09d2 From 041fa15951d0fb349d24c5619f35de1878bd030e Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov Date: Mon, 11 Aug 2014 10:58:29 -0700 Subject: Input: atmel_mxt_ts - fix a few issues reported by Coverity This should fix the following issues reported by Coverity: *** CID 1230625: Logically dead code (DEADCODE) /drivers/input/touchscreen/atmel_mxt_ts.c: 1692 in mxt_initialize() *** CID 1230627: Missing break in switch (MISSING_BREAK) /drivers/input/touchscreen/atmel_mxt_ts.c: 1436 in mxt_get_object_table() *** CID 1230629: Out-of-bounds write (OVERRUN) /drivers/input/touchscreen/atmel_mxt_ts.c: 1267 in mxt_update_cfg() *** CID 1230632: Unused pointer value (UNUSED_VALUE) /drivers/input/touchscreen/atmel_mxt_ts.c: 1211 in mxt_update_cfg() Acked-by: Nick Dyer Signed-off-by: Dmitry Torokhov --- drivers/input/touchscreen/atmel_mxt_ts.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) (limited to 'drivers/input/touchscreen') diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c index bbaf3ff680e..db178ed2b47 100644 --- a/drivers/input/touchscreen/atmel_mxt_ts.c +++ b/drivers/input/touchscreen/atmel_mxt_ts.c @@ -1098,8 +1098,12 @@ static int mxt_prepare_cfg_mem(struct mxt_data *data, /* Skip object */ for (i = 0; i < size; i++) { ret = sscanf(cfg->data + data_pos, "%hhx%n", - &val, - &offset); + &val, &offset); + if (ret != 1) { + dev_err(dev, "Bad format in T%d at %d\n", + type, i); + return -EINVAL; + } data_pos += offset; } continue; @@ -1139,7 +1143,8 @@ static int mxt_prepare_cfg_mem(struct mxt_data *data, &val, &offset); if (ret != 1) { - dev_err(dev, "Bad format in T%d\n", type); + dev_err(dev, "Bad format in T%d at %d\n", + type, i); return -EINVAL; } data_pos += offset; @@ -1149,8 +1154,7 @@ static int mxt_prepare_cfg_mem(struct mxt_data *data, byte_offset = reg + i - cfg_start_ofs; - if (byte_offset >= 0 && - byte_offset <= config_mem_size) { + 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", @@ -1454,6 +1458,7 @@ static int mxt_get_object_table(struct mxt_data *data) data->T5_msg_size = mxt_obj_size(object) - 1; } data->T5_address = object->start_address; + break; case MXT_GEN_COMMAND_T6: data->T6_reportid = min_id; data->T6_address = object->start_address; @@ -1707,7 +1712,7 @@ static int mxt_initialize(struct mxt_data *data) return error; } - mxt_acquire_irq(data); + error = mxt_acquire_irq(data); if (error) goto err_free_object_table; -- cgit v1.2.3-70-g09d2 From 3361a97601f243f1842bee6ca709e399f47b2ce3 Mon Sep 17 00:00:00 2001 From: Maks Naumov Date: Wed, 13 Aug 2014 15:27:52 -0700 Subject: Input: edt-ft5x06 - remove superfluous assignment Somehow we ended up with a duplicate line in edt_ft5x06_register_write() Signed-off-by: Maks Naumov Signed-off-by: Dmitry Torokhov --- drivers/input/touchscreen/edt-ft5x06.c | 1 - 1 file changed, 1 deletion(-) (limited to 'drivers/input/touchscreen') diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c index 5a6d50c004d..8857d5b9be7 100644 --- a/drivers/input/touchscreen/edt-ft5x06.c +++ b/drivers/input/touchscreen/edt-ft5x06.c @@ -262,7 +262,6 @@ static int edt_ft5x06_register_write(struct edt_ft5x06_ts_data *tsdata, case M06: wrbuf[0] = tsdata->factory_mode ? 0xf3 : 0xfc; wrbuf[1] = tsdata->factory_mode ? addr & 0x7f : addr & 0x3f; - wrbuf[1] = tsdata->factory_mode ? addr & 0x7f : addr & 0x3f; wrbuf[2] = value; wrbuf[3] = wrbuf[0] ^ wrbuf[1] ^ wrbuf[2]; return edt_ft5x06_ts_readwrite(tsdata->client, 4, -- cgit v1.2.3-70-g09d2