This fixes the firmware update function with bootloader v8, allows to
recover from interrupted updates with v7/v8, and does some code cleanup.
I believe that the code that allows to recover from a broken partition
table is also necessary to make flashing a different partition table
work at all, but I wasn't able to verify that, as I don't have any firmware
images with different partition tables to test with. In any case, I'm
pretty sure that it is working correctly now, as recovery from a mostly
empty flash without partition table has been tested successfully.
I have only tested the new code with bootloader v8, and I don't have the
documentation / interfacing guide for v7, so it would be great if anyone
could check that I didn't break updates for v7.
Matthias Schiffer (9):
Input: synaptics-rmi4 - fix firmware update operations with bootloader
v8
Input: synaptics-rmi4 - introduce rmi_f34v7_check_command_status()
helper
Input: synaptics-rmi4 - fix command completion check for bootloader
v7/v8
Input: synaptics-rmi4 - rewrite partition table unconditionally
Input: synaptics-rmi4 - reset after writing partition table
Input: synaptics-rmi4 - make rmi_f34v7_erase_all() use the "erase all"
command
Input: synaptics-rmi4 - remove unneeded struct register_offset
Input: synaptics-rmi4 - simplify rmi_f34v7_start_reflash()
Input: synaptics-rmi4 - drop useless gotos in rmi_f34v7_do_reflash()
drivers/input/rmi4/rmi_f34.c | 16 +-
drivers/input/rmi4/rmi_f34.h | 17 --
drivers/input/rmi4/rmi_f34v7.c | 349 +++++++--------------------------
3 files changed, 81 insertions(+), 301 deletions(-)
--
2.25.1
Commit a6977d758fed ("Input: synaptics-rmi4 - support bootloader v8 in
f34v7") allowed the F34v7 driver to probe with bootloader v8, but it did
not update various other bootloader version checks in the F34 code.
Fixes: a6977d758fed ("Input: synaptics-rmi4 - support bootloader v8 in f34v7")
Signed-off-by: Matthias Schiffer <[email protected]>
---
drivers/input/rmi4/rmi_f34.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/input/rmi4/rmi_f34.c b/drivers/input/rmi4/rmi_f34.c
index e5dca9868f87..3afc94f679ed 100644
--- a/drivers/input/rmi4/rmi_f34.c
+++ b/drivers/input/rmi4/rmi_f34.c
@@ -370,7 +370,7 @@ static int rmi_firmware_update(struct rmi_driver_data *data,
f34 = dev_get_drvdata(&data->f34_container->dev);
- if (f34->bl_version == 7) {
+ if (f34->bl_version >= 7) {
if (data->pdt_props & HAS_BSR) {
dev_err(dev, "%s: LTS not supported\n", __func__);
return -ENODEV;
@@ -382,7 +382,7 @@ static int rmi_firmware_update(struct rmi_driver_data *data,
}
/* Enter flash mode */
- if (f34->bl_version == 7)
+ if (f34->bl_version >= 7)
ret = rmi_f34v7_start_reflash(f34, fw);
else
ret = rmi_f34_enable_flash(f34);
@@ -413,7 +413,7 @@ static int rmi_firmware_update(struct rmi_driver_data *data,
f34 = dev_get_drvdata(&data->f34_container->dev);
/* Perform firmware update */
- if (f34->bl_version == 7)
+ if (f34->bl_version >= 7)
ret = rmi_f34v7_do_reflash(f34, fw);
else
ret = rmi_f34_update_firmware(f34, fw);
--
2.25.1
Add a function that waits for the last command to complete and checks
the status, and use it where appropriate. This prepares for the subsequent
fix of the completion condition in rmi_f34_attention(), which would
previously lead to a timeout instead of a more detailed error message
whenever a command was unsuccessful with v7/v8 bootloaders.
Signed-off-by: Matthias Schiffer <[email protected]>
---
drivers/input/rmi4/rmi_f34v7.c | 36 +++++++++++++++++++++++++++-------
1 file changed, 29 insertions(+), 7 deletions(-)
diff --git a/drivers/input/rmi4/rmi_f34v7.c b/drivers/input/rmi4/rmi_f34v7.c
index 8d7ec9d89b18..9049acb3a994 100644
--- a/drivers/input/rmi4/rmi_f34v7.c
+++ b/drivers/input/rmi4/rmi_f34v7.c
@@ -72,6 +72,24 @@ static int rmi_f34v7_wait_for_idle(struct f34_data *f34, int timeout_ms)
return 0;
}
+static int rmi_f34v7_check_command_status(struct f34_data *f34, int timeout_ms)
+{
+ int ret;
+
+ ret = rmi_f34v7_wait_for_idle(f34, timeout_ms);
+ if (ret < 0)
+ return ret;
+
+ ret = rmi_f34v7_read_flash_status(f34);
+ if (ret < 0)
+ return ret;
+
+ if (f34->v7.flash_status != 0x00)
+ return -EIO;
+
+ return 0;
+}
+
static int rmi_f34v7_write_command_single_transaction(struct f34_data *f34,
u8 cmd)
{
@@ -318,6 +336,10 @@ static int rmi_f34v7_read_partition_table(struct f34_data *f34)
return ret;
}
+ /*
+ * rmi_f34v7_check_command_status() can't be used here, as this
+ * function is called before IRQs are available
+ */
timeout = msecs_to_jiffies(F34_WRITE_WAIT_MS);
while (time_before(jiffies, timeout)) {
usleep_range(5000, 6000);
@@ -674,7 +696,7 @@ static int rmi_f34v7_erase_config(struct f34_data *f34)
break;
}
- ret = rmi_f34v7_wait_for_idle(f34, F34_ERASE_WAIT_MS);
+ ret = rmi_f34v7_check_command_status(f34, F34_ERASE_WAIT_MS);
if (ret < 0)
return ret;
@@ -693,7 +715,7 @@ static int rmi_f34v7_erase_guest_code(struct f34_data *f34)
if (ret < 0)
return ret;
- ret = rmi_f34v7_wait_for_idle(f34, F34_ERASE_WAIT_MS);
+ ret = rmi_f34v7_check_command_status(f34, F34_ERASE_WAIT_MS);
if (ret < 0)
return ret;
@@ -712,7 +734,7 @@ static int rmi_f34v7_erase_all(struct f34_data *f34)
if (ret < 0)
return ret;
- ret = rmi_f34v7_wait_for_idle(f34, F34_ERASE_WAIT_MS);
+ ret = rmi_f34v7_check_command_status(f34, F34_ERASE_WAIT_MS);
if (ret < 0)
return ret;
@@ -787,7 +809,7 @@ static int rmi_f34v7_read_blocks(struct f34_data *f34,
if (ret < 0)
return ret;
- ret = rmi_f34v7_wait_for_idle(f34, F34_ENABLE_WAIT_MS);
+ ret = rmi_f34v7_check_command_status(f34, F34_ENABLE_WAIT_MS);
if (ret < 0)
return ret;
@@ -871,7 +893,7 @@ static int rmi_f34v7_write_f34v7_blocks(struct f34_data *f34,
return ret;
}
- ret = rmi_f34v7_wait_for_idle(f34, F34_ENABLE_WAIT_MS);
+ ret = rmi_f34v7_check_command_status(f34, F34_ENABLE_WAIT_MS);
if (ret < 0)
return ret;
@@ -944,7 +966,7 @@ static int rmi_f34v7_write_flash_config(struct f34_data *f34)
rmi_dbg(RMI_DEBUG_FN, &f34->fn->dev,
"%s: Erase flash config command written\n", __func__);
- ret = rmi_f34v7_wait_for_idle(f34, F34_WRITE_WAIT_MS);
+ ret = rmi_f34v7_check_command_status(f34, F34_WRITE_WAIT_MS);
if (ret < 0)
return ret;
@@ -1297,7 +1319,7 @@ static int rmi_f34v7_enter_flash_prog(struct f34_data *f34)
if (ret < 0)
return ret;
- ret = rmi_f34v7_wait_for_idle(f34, F34_ENABLE_WAIT_MS);
+ ret = rmi_f34v7_check_command_status(f34, F34_ENABLE_WAIT_MS);
if (ret < 0)
return ret;
--
2.25.1
The command register is reset to 0 when a command has completed. Check
for this condition instead of the error status, which will not accurately
reflect completion. In particular, the incorrect condition caused every
command error to be reported as a timeout.
Signed-off-by: Matthias Schiffer <[email protected]>
---
drivers/input/rmi4/rmi_f34.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/input/rmi4/rmi_f34.c b/drivers/input/rmi4/rmi_f34.c
index 3afc94f679ed..b811706fb77b 100644
--- a/drivers/input/rmi4/rmi_f34.c
+++ b/drivers/input/rmi4/rmi_f34.c
@@ -114,13 +114,13 @@ static irqreturn_t rmi_f34_attention(int irq, void *ctx)
complete(&f34->v5.cmd_done);
} else {
ret = rmi_read_block(f34->fn->rmi_dev,
- f34->fn->fd.data_base_addr +
- f34->v7.off.flash_status,
- &status, sizeof(status));
- rmi_dbg(RMI_DEBUG_FN, &fn->dev, "%s: status: %#02x, ret: %d\n",
+ f34->fn->fd.data_base_addr +
+ f34->v7.off.flash_cmd,
+ &status, sizeof(status));
+ rmi_dbg(RMI_DEBUG_FN, &f34->fn->dev, "%s: cmd: %#02x, ret: %d\n",
__func__, status, ret);
- if (!ret && !(status & 0x1f))
+ if (!ret && status == CMD_V7_IDLE)
complete(&f34->v7.cmd_done);
}
--
2.25.1
Preparation for use of the "erase application" command, which is required
to recover from a bad partition table error condition. Rather than adding
complex fallback error paths for such errors, it seems more robust to do
the full erase unconditionally.
Signed-off-by: Matthias Schiffer <[email protected]>
---
drivers/input/rmi4/rmi_f34.h | 2 -
drivers/input/rmi4/rmi_f34v7.c | 153 +++------------------------------
2 files changed, 13 insertions(+), 142 deletions(-)
diff --git a/drivers/input/rmi4/rmi_f34.h b/drivers/input/rmi4/rmi_f34.h
index 99faa8c2269d..9495c8542824 100644
--- a/drivers/input/rmi4/rmi_f34.h
+++ b/drivers/input/rmi4/rmi_f34.h
@@ -262,7 +262,6 @@ struct f34v5_data {
struct f34v7_data {
bool has_display_cfg;
bool has_guest_code;
- bool force_update;
bool in_bl_mode;
u8 *read_config_buf;
size_t read_config_buf_size;
@@ -276,7 +275,6 @@ struct f34v7_data {
u16 payload_length;
u8 partitions;
u16 partition_table_bytes;
- bool new_partition_table;
struct register_offset off;
struct block_count blkcount;
diff --git a/drivers/input/rmi4/rmi_f34v7.c b/drivers/input/rmi4/rmi_f34v7.c
index 9049acb3a994..19b94b1c1a33 100644
--- a/drivers/input/rmi4/rmi_f34v7.c
+++ b/drivers/input/rmi4/rmi_f34v7.c
@@ -593,68 +593,6 @@ static int rmi_f34v7_read_queries(struct f34_data *f34)
return 0;
}
-static int rmi_f34v7_check_ui_firmware_size(struct f34_data *f34)
-{
- u16 block_count;
-
- block_count = f34->v7.img.ui_firmware.size / f34->v7.block_size;
- f34->update_size += block_count;
-
- if (block_count != f34->v7.blkcount.ui_firmware) {
- dev_err(&f34->fn->dev,
- "UI firmware size mismatch: %d != %d\n",
- block_count, f34->v7.blkcount.ui_firmware);
- return -EINVAL;
- }
-
- return 0;
-}
-
-static int rmi_f34v7_check_ui_config_size(struct f34_data *f34)
-{
- u16 block_count;
-
- block_count = f34->v7.img.ui_config.size / f34->v7.block_size;
- f34->update_size += block_count;
-
- if (block_count != f34->v7.blkcount.ui_config) {
- dev_err(&f34->fn->dev, "UI config size mismatch\n");
- return -EINVAL;
- }
-
- return 0;
-}
-
-static int rmi_f34v7_check_dp_config_size(struct f34_data *f34)
-{
- u16 block_count;
-
- block_count = f34->v7.img.dp_config.size / f34->v7.block_size;
- f34->update_size += block_count;
-
- if (block_count != f34->v7.blkcount.dp_config) {
- dev_err(&f34->fn->dev, "Display config size mismatch\n");
- return -EINVAL;
- }
-
- return 0;
-}
-
-static int rmi_f34v7_check_guest_code_size(struct f34_data *f34)
-{
- u16 block_count;
-
- block_count = f34->v7.img.guest_code.size / f34->v7.block_size;
- f34->update_size += block_count;
-
- if (block_count != f34->v7.blkcount.guest_code) {
- dev_err(&f34->fn->dev, "Guest code size mismatch\n");
- return -EINVAL;
- }
-
- return 0;
-}
-
static int rmi_f34v7_check_bl_config_size(struct f34_data *f34)
{
u16 block_count;
@@ -750,7 +688,7 @@ static int rmi_f34v7_erase_all(struct f34_data *f34)
return ret;
}
- if (f34->v7.new_partition_table && f34->v7.has_guest_code) {
+ if (f34->v7.has_guest_code) {
ret = rmi_f34v7_erase_guest_code(f34);
if (ret < 0)
return ret;
@@ -1029,33 +967,6 @@ static int rmi_f34v7_write_firmware(struct f34_data *f34)
blk_count, v7_CMD_WRITE_FW);
}
-static void rmi_f34v7_compare_partition_tables(struct f34_data *f34)
-{
- if (f34->v7.phyaddr.ui_firmware != f34->v7.img.phyaddr.ui_firmware) {
- f34->v7.new_partition_table = true;
- return;
- }
-
- if (f34->v7.phyaddr.ui_config != f34->v7.img.phyaddr.ui_config) {
- f34->v7.new_partition_table = true;
- return;
- }
-
- if (f34->v7.has_display_cfg &&
- f34->v7.phyaddr.dp_config != f34->v7.img.phyaddr.dp_config) {
- f34->v7.new_partition_table = true;
- return;
- }
-
- if (f34->v7.has_guest_code &&
- f34->v7.phyaddr.guest_code != f34->v7.img.phyaddr.guest_code) {
- f34->v7.new_partition_table = true;
- return;
- }
-
- f34->v7.new_partition_table = false;
-}
-
static void rmi_f34v7_parse_img_header_10_bl_container(struct f34_data *f34,
const void *image)
{
@@ -1202,8 +1113,6 @@ static int rmi_f34v7_parse_image_info(struct f34_data *f34)
rmi_f34v7_parse_partition_table(f34, f34->v7.img.fl_config.data,
&f34->v7.img.blkcount, &f34->v7.img.phyaddr);
- rmi_f34v7_compare_partition_tables(f34);
-
return 0;
}
@@ -1224,44 +1133,18 @@ int rmi_f34v7_do_reflash(struct f34_data *f34, const struct firmware *fw)
if (ret < 0)
goto fail;
- if (!f34->v7.new_partition_table) {
- ret = rmi_f34v7_check_ui_firmware_size(f34);
- if (ret < 0)
- goto fail;
-
- ret = rmi_f34v7_check_ui_config_size(f34);
- if (ret < 0)
- goto fail;
-
- if (f34->v7.has_display_cfg &&
- f34->v7.img.contains_display_cfg) {
- ret = rmi_f34v7_check_dp_config_size(f34);
- if (ret < 0)
- goto fail;
- }
-
- if (f34->v7.has_guest_code && f34->v7.img.contains_guest_code) {
- ret = rmi_f34v7_check_guest_code_size(f34);
- if (ret < 0)
- goto fail;
- }
- } else {
- ret = rmi_f34v7_check_bl_config_size(f34);
- if (ret < 0)
- goto fail;
- }
+ ret = rmi_f34v7_check_bl_config_size(f34);
+ if (ret < 0)
+ goto fail;
ret = rmi_f34v7_erase_all(f34);
if (ret < 0)
goto fail;
- if (f34->v7.new_partition_table) {
- ret = rmi_f34v7_write_partition_table(f34);
- if (ret < 0)
- goto fail;
- dev_info(&f34->fn->dev, "%s: Partition table programmed\n",
- __func__);
- }
+ ret = rmi_f34v7_write_partition_table(f34);
+ if (ret < 0)
+ goto fail;
+ dev_info(&f34->fn->dev, "%s: Partition table programmed\n", __func__);
dev_info(&f34->fn->dev, "Writing firmware (%d bytes)...\n",
f34->v7.img.ui_firmware.size);
@@ -1286,14 +1169,12 @@ int rmi_f34v7_do_reflash(struct f34_data *f34, const struct firmware *fw)
goto fail;
}
- if (f34->v7.new_partition_table) {
- if (f34->v7.has_guest_code && f34->v7.img.contains_guest_code) {
- dev_info(&f34->fn->dev, "Writing guest code...\n");
+ if (f34->v7.has_guest_code && f34->v7.img.contains_guest_code) {
+ dev_info(&f34->fn->dev, "Writing guest code...\n");
- ret = rmi_f34v7_write_guest_code(f34);
- if (ret < 0)
- goto fail;
- }
+ ret = rmi_f34v7_write_guest_code(f34);
+ if (ret < 0)
+ goto fail;
}
fail:
@@ -1339,13 +1220,6 @@ int rmi_f34v7_start_reflash(struct f34_data *f34, const struct firmware *fw)
if (ret < 0)
goto exit;
- if (!f34->v7.force_update && f34->v7.new_partition_table) {
- dev_err(&f34->fn->dev, "%s: Partition table mismatch\n",
- __func__);
- ret = -EINVAL;
- goto exit;
- }
-
dev_info(&f34->fn->dev, "Firmware image OK\n");
ret = rmi_f34v7_read_flash_status(f34);
@@ -1406,6 +1280,5 @@ int rmi_f34v7_probe(struct f34_data *f34)
if (ret < 0)
return ret;
- f34->v7.force_update = true;
return 0;
}
--
2.25.1
When recovering from a bad partition table (for example after an
interrupted update), a reset is necessary for the new partition table to
become effective. Without this reset, writing the core code partition
will fail with status 0x03 (Invalid Command).
Signed-off-by: Matthias Schiffer <[email protected]>
---
drivers/input/rmi4/rmi_f34v7.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/input/rmi4/rmi_f34v7.c b/drivers/input/rmi4/rmi_f34v7.c
index 19b94b1c1a33..9b78f98bb21c 100644
--- a/drivers/input/rmi4/rmi_f34v7.c
+++ b/drivers/input/rmi4/rmi_f34v7.c
@@ -1146,6 +1146,14 @@ int rmi_f34v7_do_reflash(struct f34_data *f34, const struct firmware *fw)
goto fail;
dev_info(&f34->fn->dev, "%s: Partition table programmed\n", __func__);
+ /*
+ * Reset to reload partition table - as the previous firmware has been
+ * erased, we remain in bootloader mode.
+ */
+ ret = rmi_scan_pdt(f34->fn->rmi_dev, NULL, rmi_initial_reset);
+ if (ret < 0)
+ dev_warn(&f34->fn->dev, "RMI reset failed!\n");
+
dev_info(&f34->fn->dev, "Writing firmware (%d bytes)...\n",
f34->v7.img.ui_firmware.size);
--
2.25.1
A full erase is required to recover from error conditions like "Bad
Partition Table". Various individual partition erase commands can be
(and need to be) omitted, as they will fail until a new partition table
has been written.
Signed-off-by: Matthias Schiffer <[email protected]>
---
drivers/input/rmi4/rmi_f34v7.c | 87 +---------------------------------
1 file changed, 1 insertion(+), 86 deletions(-)
diff --git a/drivers/input/rmi4/rmi_f34v7.c b/drivers/input/rmi4/rmi_f34v7.c
index 9b78f98bb21c..9c1a73611761 100644
--- a/drivers/input/rmi4/rmi_f34v7.c
+++ b/drivers/input/rmi4/rmi_f34v7.c
@@ -608,58 +608,6 @@ static int rmi_f34v7_check_bl_config_size(struct f34_data *f34)
return 0;
}
-static int rmi_f34v7_erase_config(struct f34_data *f34)
-{
- int ret;
-
- dev_info(&f34->fn->dev, "Erasing config...\n");
-
- init_completion(&f34->v7.cmd_done);
-
- switch (f34->v7.config_area) {
- case v7_UI_CONFIG_AREA:
- ret = rmi_f34v7_write_command(f34, v7_CMD_ERASE_UI_CONFIG);
- if (ret < 0)
- return ret;
- break;
- case v7_DP_CONFIG_AREA:
- ret = rmi_f34v7_write_command(f34, v7_CMD_ERASE_DISP_CONFIG);
- if (ret < 0)
- return ret;
- break;
- case v7_BL_CONFIG_AREA:
- ret = rmi_f34v7_write_command(f34, v7_CMD_ERASE_BL_CONFIG);
- if (ret < 0)
- return ret;
- break;
- }
-
- ret = rmi_f34v7_check_command_status(f34, F34_ERASE_WAIT_MS);
- if (ret < 0)
- return ret;
-
- return 0;
-}
-
-static int rmi_f34v7_erase_guest_code(struct f34_data *f34)
-{
- int ret;
-
- dev_info(&f34->fn->dev, "Erasing guest code...\n");
-
- init_completion(&f34->v7.cmd_done);
-
- ret = rmi_f34v7_write_command(f34, v7_CMD_ERASE_GUEST_CODE);
- if (ret < 0)
- return ret;
-
- ret = rmi_f34v7_check_command_status(f34, F34_ERASE_WAIT_MS);
- if (ret < 0)
- return ret;
-
- return 0;
-}
-
static int rmi_f34v7_erase_all(struct f34_data *f34)
{
int ret;
@@ -668,7 +616,7 @@ static int rmi_f34v7_erase_all(struct f34_data *f34)
init_completion(&f34->v7.cmd_done);
- ret = rmi_f34v7_write_command(f34, v7_CMD_ERASE_UI_FIRMWARE);
+ ret = rmi_f34v7_write_command(f34, v7_CMD_ERASE_ALL);
if (ret < 0)
return ret;
@@ -676,24 +624,6 @@ static int rmi_f34v7_erase_all(struct f34_data *f34)
if (ret < 0)
return ret;
- f34->v7.config_area = v7_UI_CONFIG_AREA;
- ret = rmi_f34v7_erase_config(f34);
- if (ret < 0)
- return ret;
-
- if (f34->v7.has_display_cfg) {
- f34->v7.config_area = v7_DP_CONFIG_AREA;
- ret = rmi_f34v7_erase_config(f34);
- if (ret < 0)
- return ret;
- }
-
- if (f34->v7.has_guest_code) {
- ret = rmi_f34v7_erase_guest_code(f34);
- if (ret < 0)
- return ret;
- }
-
return 0;
}
@@ -897,17 +827,6 @@ static int rmi_f34v7_write_flash_config(struct f34_data *f34)
init_completion(&f34->v7.cmd_done);
- ret = rmi_f34v7_write_command(f34, v7_CMD_ERASE_FLASH_CONFIG);
- if (ret < 0)
- return ret;
-
- rmi_dbg(RMI_DEBUG_FN, &f34->fn->dev,
- "%s: Erase flash config command written\n", __func__);
-
- ret = rmi_f34v7_check_command_status(f34, F34_WRITE_WAIT_MS);
- if (ret < 0)
- return ret;
-
ret = rmi_f34v7_write_config(f34);
if (ret < 0)
return ret;
@@ -937,10 +856,6 @@ static int rmi_f34v7_write_partition_table(struct f34_data *f34)
if (ret < 0)
return ret;
- ret = rmi_f34v7_erase_config(f34);
- if (ret < 0)
- return ret;
-
ret = rmi_f34v7_write_flash_config(f34);
if (ret < 0)
return ret;
--
2.25.1
All register offsets are fixed, and a number of places even read or
write multiple registers as a block, so there is no way to support
reordering them without move involved changes. Remove the unneeded level
of indirection in the register access.
Signed-off-by: Matthias Schiffer <[email protected]>
---
drivers/input/rmi4/rmi_f34.c | 2 +-
drivers/input/rmi4/rmi_f34.h | 15 ---------------
drivers/input/rmi4/rmi_f34v7.c | 35 ++++++++++++++--------------------
3 files changed, 15 insertions(+), 37 deletions(-)
diff --git a/drivers/input/rmi4/rmi_f34.c b/drivers/input/rmi4/rmi_f34.c
index b811706fb77b..30169b584573 100644
--- a/drivers/input/rmi4/rmi_f34.c
+++ b/drivers/input/rmi4/rmi_f34.c
@@ -115,7 +115,7 @@ static irqreturn_t rmi_f34_attention(int irq, void *ctx)
} else {
ret = rmi_read_block(f34->fn->rmi_dev,
f34->fn->fd.data_base_addr +
- f34->v7.off.flash_cmd,
+ V7_COMMAND_OFFSET,
&status, sizeof(status));
rmi_dbg(RMI_DEBUG_FN, &f34->fn->dev, "%s: cmd: %#02x, ret: %d\n",
__func__, status, ret);
diff --git a/drivers/input/rmi4/rmi_f34.h b/drivers/input/rmi4/rmi_f34.h
index 9495c8542824..cfa3039804fd 100644
--- a/drivers/input/rmi4/rmi_f34.h
+++ b/drivers/input/rmi4/rmi_f34.h
@@ -222,20 +222,6 @@ struct image_metadata {
struct physical_address phyaddr;
};
-struct register_offset {
- u8 properties;
- u8 properties_2;
- u8 block_size;
- u8 block_count;
- u8 gc_block_count;
- u8 flash_status;
- u8 partition_id;
- u8 block_number;
- u8 transfer_length;
- u8 flash_cmd;
- u8 payload;
-};
-
struct rmi_f34_firmware {
__le32 checksum;
u8 pad1[3];
@@ -276,7 +262,6 @@ struct f34v7_data {
u8 partitions;
u16 partition_table_bytes;
- struct register_offset off;
struct block_count blkcount;
struct physical_address phyaddr;
struct image_metadata img;
diff --git a/drivers/input/rmi4/rmi_f34v7.c b/drivers/input/rmi4/rmi_f34v7.c
index 9c1a73611761..5c22ad4bcc74 100644
--- a/drivers/input/rmi4/rmi_f34v7.c
+++ b/drivers/input/rmi4/rmi_f34v7.c
@@ -25,7 +25,7 @@ static int rmi_f34v7_read_flash_status(struct f34_data *f34)
int ret;
ret = rmi_read_block(f34->fn->rmi_dev,
- f34->fn->fd.data_base_addr + f34->v7.off.flash_status,
+ f34->fn->fd.data_base_addr + V7_FLASH_STATUS_OFFSET,
&status,
sizeof(status));
if (ret < 0) {
@@ -43,7 +43,7 @@ static int rmi_f34v7_read_flash_status(struct f34_data *f34)
}
ret = rmi_read_block(f34->fn->rmi_dev,
- f34->fn->fd.data_base_addr + f34->v7.off.flash_cmd,
+ f34->fn->fd.data_base_addr + V7_COMMAND_OFFSET,
&command,
sizeof(command));
if (ret < 0) {
@@ -140,7 +140,7 @@ static int rmi_f34v7_write_command_single_transaction(struct f34_data *f34,
data_1_5.payload[1] = f34->bootloader_id[1];
ret = rmi_write_block(f34->fn->rmi_dev,
- base + f34->v7.off.partition_id,
+ base + V7_PARTITION_ID_OFFSET,
&data_1_5, sizeof(data_1_5));
if (ret < 0) {
dev_err(&f34->fn->dev,
@@ -213,7 +213,7 @@ static int rmi_f34v7_write_command(struct f34_data *f34, u8 cmd)
__func__, command);
ret = rmi_write_block(f34->fn->rmi_dev,
- base + f34->v7.off.flash_cmd,
+ base + V7_COMMAND_OFFSET,
&command, sizeof(command));
if (ret < 0) {
dev_err(&f34->fn->dev, "%s: Failed to write flash command\n",
@@ -280,7 +280,7 @@ static int rmi_f34v7_write_partition_id(struct f34_data *f34, u8 cmd)
}
ret = rmi_write_block(f34->fn->rmi_dev,
- base + f34->v7.off.partition_id,
+ base + V7_PARTITION_ID_OFFSET,
&partition, sizeof(partition));
if (ret < 0) {
dev_err(&f34->fn->dev, "%s: Failed to write partition ID\n",
@@ -308,7 +308,7 @@ static int rmi_f34v7_read_partition_table(struct f34_data *f34)
return ret;
ret = rmi_write_block(f34->fn->rmi_dev,
- base + f34->v7.off.block_number,
+ base + V7_BLOCK_NUMBER_OFFSET,
&block_number, sizeof(block_number));
if (ret < 0) {
dev_err(&f34->fn->dev, "%s: Failed to write block number\n",
@@ -319,7 +319,7 @@ static int rmi_f34v7_read_partition_table(struct f34_data *f34)
put_unaligned_le16(f34->v7.flash_config_length, &length);
ret = rmi_write_block(f34->fn->rmi_dev,
- base + f34->v7.off.transfer_length,
+ base + V7_TRANSFER_LENGTH_OFFSET,
&length, sizeof(length));
if (ret < 0) {
dev_err(&f34->fn->dev, "%s: Failed to write transfer length\n",
@@ -352,7 +352,7 @@ static int rmi_f34v7_read_partition_table(struct f34_data *f34)
}
ret = rmi_read_block(f34->fn->rmi_dev,
- base + f34->v7.off.payload,
+ base + V7_PAYLOAD_OFFSET,
f34->v7.read_config_buf,
f34->v7.partition_table_bytes);
if (ret < 0) {
@@ -526,13 +526,6 @@ static int rmi_f34v7_read_queries(struct f34_data *f34)
rmi_dbg(RMI_DEBUG_FN, &f34->fn->dev, "%s: f34->v7.block_size = %d\n",
__func__, f34->v7.block_size);
- f34->v7.off.flash_status = V7_FLASH_STATUS_OFFSET;
- f34->v7.off.partition_id = V7_PARTITION_ID_OFFSET;
- f34->v7.off.block_number = V7_BLOCK_NUMBER_OFFSET;
- f34->v7.off.transfer_length = V7_TRANSFER_LENGTH_OFFSET;
- f34->v7.off.flash_cmd = V7_COMMAND_OFFSET;
- f34->v7.off.payload = V7_PAYLOAD_OFFSET;
-
f34->v7.has_display_cfg = query_1_7.partition_support[1] & HAS_DISP_CFG;
f34->v7.has_guest_code =
query_1_7.partition_support[1] & HAS_GUEST_CODE;
@@ -646,7 +639,7 @@ static int rmi_f34v7_read_blocks(struct f34_data *f34,
return ret;
ret = rmi_write_block(f34->fn->rmi_dev,
- base + f34->v7.off.block_number,
+ base + V7_BLOCK_NUMBER_OFFSET,
&block_number, sizeof(block_number));
if (ret < 0) {
dev_err(&f34->fn->dev, "%s: Failed to write block number\n",
@@ -662,7 +655,7 @@ static int rmi_f34v7_read_blocks(struct f34_data *f34,
put_unaligned_le16(transfer, &length);
ret = rmi_write_block(f34->fn->rmi_dev,
- base + f34->v7.off.transfer_length,
+ base + V7_TRANSFER_LENGTH_OFFSET,
&length, sizeof(length));
if (ret < 0) {
dev_err(&f34->fn->dev,
@@ -682,7 +675,7 @@ static int rmi_f34v7_read_blocks(struct f34_data *f34,
return ret;
ret = rmi_read_block(f34->fn->rmi_dev,
- base + f34->v7.off.payload,
+ base + V7_PAYLOAD_OFFSET,
&f34->v7.read_config_buf[index],
transfer * f34->v7.block_size);
if (ret < 0) {
@@ -718,7 +711,7 @@ static int rmi_f34v7_write_f34v7_blocks(struct f34_data *f34,
return ret;
ret = rmi_write_block(f34->fn->rmi_dev,
- base + f34->v7.off.block_number,
+ base + V7_BLOCK_NUMBER_OFFSET,
&block_number, sizeof(block_number));
if (ret < 0) {
dev_err(&f34->fn->dev, "%s: Failed to write block number\n",
@@ -738,7 +731,7 @@ static int rmi_f34v7_write_f34v7_blocks(struct f34_data *f34,
init_completion(&f34->v7.cmd_done);
ret = rmi_write_block(f34->fn->rmi_dev,
- base + f34->v7.off.transfer_length,
+ base + V7_TRANSFER_LENGTH_OFFSET,
&length, sizeof(length));
if (ret < 0) {
dev_err(&f34->fn->dev,
@@ -752,7 +745,7 @@ static int rmi_f34v7_write_f34v7_blocks(struct f34_data *f34,
return ret;
ret = rmi_write_block(f34->fn->rmi_dev,
- base + f34->v7.off.payload,
+ base + V7_PAYLOAD_OFFSET,
block_ptr, transfer * f34->v7.block_size);
if (ret < 0) {
dev_err(&f34->fn->dev,
--
2.25.1
rmi_f34v7_enter_flash_prog() already enables IRQs and checks the flash
status - there's no need for rmi_f34v7_start_reflash() to do the same just
before calling rmi_f34v7_enter_flash_prog().
Signed-off-by: Matthias Schiffer <[email protected]>
---
drivers/input/rmi4/rmi_f34v7.c | 25 ++++++-------------------
1 file changed, 6 insertions(+), 19 deletions(-)
diff --git a/drivers/input/rmi4/rmi_f34v7.c b/drivers/input/rmi4/rmi_f34v7.c
index 5c22ad4bcc74..f16c67eb6cc6 100644
--- a/drivers/input/rmi4/rmi_f34v7.c
+++ b/drivers/input/rmi4/rmi_f34v7.c
@@ -1107,8 +1107,11 @@ static int rmi_f34v7_enter_flash_prog(struct f34_data *f34)
if (ret < 0)
return ret;
- if (f34->v7.in_bl_mode)
+ if (f34->v7.in_bl_mode) {
+ dev_info(&f34->fn->dev, "%s: Device in bootloader mode\n",
+ __func__);
return 0;
+ }
init_completion(&f34->v7.cmd_done);
@@ -1127,32 +1130,16 @@ int rmi_f34v7_start_reflash(struct f34_data *f34, const struct firmware *fw)
{
int ret = 0;
- f34->fn->rmi_dev->driver->set_irq_bits(f34->fn->rmi_dev, f34->fn->irq_mask);
-
f34->v7.config_area = v7_UI_CONFIG_AREA;
f34->v7.image = fw->data;
ret = rmi_f34v7_parse_image_info(f34);
if (ret < 0)
- goto exit;
+ return ret;
dev_info(&f34->fn->dev, "Firmware image OK\n");
- ret = rmi_f34v7_read_flash_status(f34);
- if (ret < 0)
- goto exit;
-
- if (f34->v7.in_bl_mode) {
- dev_info(&f34->fn->dev, "%s: Device in bootloader mode\n",
- __func__);
- }
-
- rmi_f34v7_enter_flash_prog(f34);
-
- return 0;
-
-exit:
- return ret;
+ return rmi_f34v7_enter_flash_prog(f34);
}
int rmi_f34v7_probe(struct f34_data *f34)
--
2.25.1
Returning directly makes the code clearer.
Signed-off-by: Matthias Schiffer <[email protected]>
---
drivers/input/rmi4/rmi_f34v7.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/input/rmi4/rmi_f34v7.c b/drivers/input/rmi4/rmi_f34v7.c
index f16c67eb6cc6..886557b01eba 100644
--- a/drivers/input/rmi4/rmi_f34v7.c
+++ b/drivers/input/rmi4/rmi_f34v7.c
@@ -1039,19 +1039,19 @@ int rmi_f34v7_do_reflash(struct f34_data *f34, const struct firmware *fw)
ret = rmi_f34v7_parse_image_info(f34);
if (ret < 0)
- goto fail;
+ return ret;
ret = rmi_f34v7_check_bl_config_size(f34);
if (ret < 0)
- goto fail;
+ return ret;
ret = rmi_f34v7_erase_all(f34);
if (ret < 0)
- goto fail;
+ return ret;
ret = rmi_f34v7_write_partition_table(f34);
if (ret < 0)
- goto fail;
+ return ret;
dev_info(&f34->fn->dev, "%s: Partition table programmed\n", __func__);
/*
@@ -1067,7 +1067,7 @@ int rmi_f34v7_do_reflash(struct f34_data *f34, const struct firmware *fw)
ret = rmi_f34v7_write_firmware(f34);
if (ret < 0)
- goto fail;
+ return ret;
dev_info(&f34->fn->dev, "Writing config (%d bytes)...\n",
f34->v7.img.ui_config.size);
@@ -1075,14 +1075,14 @@ int rmi_f34v7_do_reflash(struct f34_data *f34, const struct firmware *fw)
f34->v7.config_area = v7_UI_CONFIG_AREA;
ret = rmi_f34v7_write_ui_config(f34);
if (ret < 0)
- goto fail;
+ return ret;
if (f34->v7.has_display_cfg && f34->v7.img.contains_display_cfg) {
dev_info(&f34->fn->dev, "Writing display config...\n");
ret = rmi_f34v7_write_dp_config(f34);
if (ret < 0)
- goto fail;
+ return ret;
}
if (f34->v7.has_guest_code && f34->v7.img.contains_guest_code) {
@@ -1090,11 +1090,10 @@ int rmi_f34v7_do_reflash(struct f34_data *f34, const struct firmware *fw)
ret = rmi_f34v7_write_guest_code(f34);
if (ret < 0)
- goto fail;
+ return ret;
}
-fail:
- return ret;
+ return 0;
}
static int rmi_f34v7_enter_flash_prog(struct f34_data *f34)
--
2.25.1
On Wed, 2022-06-08 at 14:47 +0200, Matthias Schiffer wrote:
> This fixes the firmware update function with bootloader v8, allows to
> recover from interrupted updates with v7/v8, and does some code cleanup.
>
> I believe that the code that allows to recover from a broken partition
> table is also necessary to make flashing a different partition table
> work at all, but I wasn't able to verify that, as I don't have any firmware
> images with different partition tables to test with. In any case, I'm
> pretty sure that it is working correctly now, as recovery from a mostly
> empty flash without partition table has been tested successfully.
>
> I have only tested the new code with bootloader v8, and I don't have the
> documentation / interfacing guide for v7, so it would be great if anyone
> could check that I didn't break updates for v7.
Hi everyone,
any news regarding this patch series?
Kind regards,
Matthias
>
>
> Matthias Schiffer (9):
> Input: synaptics-rmi4 - fix firmware update operations with bootloader
> v8
> Input: synaptics-rmi4 - introduce rmi_f34v7_check_command_status()
> helper
> Input: synaptics-rmi4 - fix command completion check for bootloader
> v7/v8
> Input: synaptics-rmi4 - rewrite partition table unconditionally
> Input: synaptics-rmi4 - reset after writing partition table
> Input: synaptics-rmi4 - make rmi_f34v7_erase_all() use the "erase all"
> command
> Input: synaptics-rmi4 - remove unneeded struct register_offset
> Input: synaptics-rmi4 - simplify rmi_f34v7_start_reflash()
> Input: synaptics-rmi4 - drop useless gotos in rmi_f34v7_do_reflash()
>
> drivers/input/rmi4/rmi_f34.c | 16 +-
> drivers/input/rmi4/rmi_f34.h | 17 --
> drivers/input/rmi4/rmi_f34v7.c | 349 +++++++--------------------------
> 3 files changed, 81 insertions(+), 301 deletions(-)
>
On Mon, 2022-06-27 at 10:31 +0200, Matthias Schiffer wrote:
> On Wed, 2022-06-08 at 14:47 +0200, Matthias Schiffer wrote:
> > This fixes the firmware update function with bootloader v8, allows to
> > recover from interrupted updates with v7/v8, and does some code cleanup.
> >
> > I believe that the code that allows to recover from a broken partition
> > table is also necessary to make flashing a different partition table
> > work at all, but I wasn't able to verify that, as I don't have any firmware
> > images with different partition tables to test with. In any case, I'm
> > pretty sure that it is working correctly now, as recovery from a mostly
> > empty flash without partition table has been tested successfully.
> >
> > I have only tested the new code with bootloader v8, and I don't have the
> > documentation / interfacing guide for v7, so it would be great if anyone
> > could check that I didn't break updates for v7.
>
> Hi everyone,
>
> any news regarding this patch series?
>
>
> Kind regards,
> Matthias
Ping - can we get this applied, or at least any kind of feedback?
Kind regards,
Matthias
>
>
>
> >
> > Matthias Schiffer (9):
> > Input: synaptics-rmi4 - fix firmware update operations with bootloader
> > v8
> > Input: synaptics-rmi4 - introduce rmi_f34v7_check_command_status()
> > helper
> > Input: synaptics-rmi4 - fix command completion check for bootloader
> > v7/v8
> > Input: synaptics-rmi4 - rewrite partition table unconditionally
> > Input: synaptics-rmi4 - reset after writing partition table
> > Input: synaptics-rmi4 - make rmi_f34v7_erase_all() use the "erase all"
> > command
> > Input: synaptics-rmi4 - remove unneeded struct register_offset
> > Input: synaptics-rmi4 - simplify rmi_f34v7_start_reflash()
> > Input: synaptics-rmi4 - drop useless gotos in rmi_f34v7_do_reflash()
> >
> > drivers/input/rmi4/rmi_f34.c | 16 +-
> > drivers/input/rmi4/rmi_f34.h | 17 --
> > drivers/input/rmi4/rmi_f34v7.c | 349 +++++++--------------------------
> > 3 files changed, 81 insertions(+), 301 deletions(-)
> >
On Mon, 2022-08-15 at 09:43 +0200, Matthias Schiffer wrote:
> On Mon, 2022-06-27 at 10:31 +0200, Matthias Schiffer wrote:
> > On Wed, 2022-06-08 at 14:47 +0200, Matthias Schiffer wrote:
> > > This fixes the firmware update function with bootloader v8, allows to
> > > recover from interrupted updates with v7/v8, and does some code cleanup.
> > >
> > > I believe that the code that allows to recover from a broken partition
> > > table is also necessary to make flashing a different partition table
> > > work at all, but I wasn't able to verify that, as I don't have any firmware
> > > images with different partition tables to test with. In any case, I'm
> > > pretty sure that it is working correctly now, as recovery from a mostly
> > > empty flash without partition table has been tested successfully.
> > >
> > > I have only tested the new code with bootloader v8, and I don't have the
> > > documentation / interfacing guide for v7, so it would be great if anyone
> > > could check that I didn't break updates for v7.
> >
> > Hi everyone,
> >
> > any news regarding this patch series?
> >
> >
> > Kind regards,
> > Matthias
>
> Ping - can we get this applied, or at least any kind of feedback?
>
>
> Kind regards,
> Matthias
Ping - another month has passed.
Should I resend the series? Not much has happened in the RMI4 driver,
so the patches still apply cleanly to latest linux-next.
Kind regards,
Matthias
>
>
>
> >
> >
> > > Matthias Schiffer (9):
> > > Input: synaptics-rmi4 - fix firmware update operations with bootloader
> > > v8
> > > Input: synaptics-rmi4 - introduce rmi_f34v7_check_command_status()
> > > helper
> > > Input: synaptics-rmi4 - fix command completion check for bootloader
> > > v7/v8
> > > Input: synaptics-rmi4 - rewrite partition table unconditionally
> > > Input: synaptics-rmi4 - reset after writing partition table
> > > Input: synaptics-rmi4 - make rmi_f34v7_erase_all() use the "erase all"
> > > command
> > > Input: synaptics-rmi4 - remove unneeded struct register_offset
> > > Input: synaptics-rmi4 - simplify rmi_f34v7_start_reflash()
> > > Input: synaptics-rmi4 - drop useless gotos in rmi_f34v7_do_reflash()
> > >
> > > drivers/input/rmi4/rmi_f34.c | 16 +-
> > > drivers/input/rmi4/rmi_f34.h | 17 --
> > > drivers/input/rmi4/rmi_f34v7.c | 349 +++++++--------------------------
> > > 3 files changed, 81 insertions(+), 301 deletions(-)
> > >
On Fri, Sep 16, 2022 at 06:39:18PM -0400, Lyude Paul wrote:
> Would my review count at all here? I hadn't reviewed until now because I
> wasn't sure it would, but I'm happy to take a look if you think that'd help.
Absolutely. All reviews count, and they also show that there is interest
in the patches/new functionality.
Thanks.
--
Dmitry
sgtm then :), will try to look at this early next week
On Fri, 2022-09-16 at 15:50 -0700, Dmitry Torokhov wrote:
> On Fri, Sep 16, 2022 at 06:39:18PM -0400, Lyude Paul wrote:
> > Would my review count at all here? I hadn't reviewed until now because I
> > wasn't sure it would, but I'm happy to take a look if you think that'd help.
>
> Absolutely. All reviews count, and they also show that there is interest
> in the patches/new functionality.
>
> Thanks.
>
--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat
Would my review count at all here? I hadn't reviewed until now because I
wasn't sure it would, but I'm happy to take a look if you think that'd help.
On Thu, 2022-09-15 at 10:02 +0200, Matthias Schiffer wrote:
> On Mon, 2022-08-15 at 09:43 +0200, Matthias Schiffer wrote:
> > On Mon, 2022-06-27 at 10:31 +0200, Matthias Schiffer wrote:
> > > On Wed, 2022-06-08 at 14:47 +0200, Matthias Schiffer wrote:
> > > > This fixes the firmware update function with bootloader v8, allows to
> > > > recover from interrupted updates with v7/v8, and does some code cleanup.
> > > >
> > > > I believe that the code that allows to recover from a broken partition
> > > > table is also necessary to make flashing a different partition table
> > > > work at all, but I wasn't able to verify that, as I don't have any firmware
> > > > images with different partition tables to test with. In any case, I'm
> > > > pretty sure that it is working correctly now, as recovery from a mostly
> > > > empty flash without partition table has been tested successfully.
> > > >
> > > > I have only tested the new code with bootloader v8, and I don't have the
> > > > documentation / interfacing guide for v7, so it would be great if anyone
> > > > could check that I didn't break updates for v7.
> > >
> > > Hi everyone,
> > >
> > > any news regarding this patch series?
> > >
> > >
> > > Kind regards,
> > > Matthias
> >
> > Ping - can we get this applied, or at least any kind of feedback?
> >
> >
> > Kind regards,
> > Matthias
>
>
> Ping - another month has passed.
>
> Should I resend the series? Not much has happened in the RMI4 driver,
> so the patches still apply cleanly to latest linux-next.
>
> Kind regards,
> Matthias
>
>
>
> >
> >
> >
> > >
> > >
> > > > Matthias Schiffer (9):
> > > > Input: synaptics-rmi4 - fix firmware update operations with bootloader
> > > > v8
> > > > Input: synaptics-rmi4 - introduce rmi_f34v7_check_command_status()
> > > > helper
> > > > Input: synaptics-rmi4 - fix command completion check for bootloader
> > > > v7/v8
> > > > Input: synaptics-rmi4 - rewrite partition table unconditionally
> > > > Input: synaptics-rmi4 - reset after writing partition table
> > > > Input: synaptics-rmi4 - make rmi_f34v7_erase_all() use the "erase all"
> > > > command
> > > > Input: synaptics-rmi4 - remove unneeded struct register_offset
> > > > Input: synaptics-rmi4 - simplify rmi_f34v7_start_reflash()
> > > > Input: synaptics-rmi4 - drop useless gotos in rmi_f34v7_do_reflash()
> > > >
> > > > drivers/input/rmi4/rmi_f34.c | 16 +-
> > > > drivers/input/rmi4/rmi_f34.h | 17 --
> > > > drivers/input/rmi4/rmi_f34v7.c | 349 +++++++--------------------------
> > > > 3 files changed, 81 insertions(+), 301 deletions(-)
> > > >
>
--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat
Would add a Cc: [email protected] for this
With that fixed: Reviewed-by: Lyude Paul <[email protected]>
On Wed, 2022-06-08 at 14:48 +0200, Matthias Schiffer wrote:
> Commit a6977d758fed ("Input: synaptics-rmi4 - support bootloader v8 in
> f34v7") allowed the F34v7 driver to probe with bootloader v8, but it did
> not update various other bootloader version checks in the F34 code.
>
> Fixes: a6977d758fed ("Input: synaptics-rmi4 - support bootloader v8 in f34v7")
> Signed-off-by: Matthias Schiffer <[email protected]>
> ---
> drivers/input/rmi4/rmi_f34.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_f34.c b/drivers/input/rmi4/rmi_f34.c
> index e5dca9868f87..3afc94f679ed 100644
> --- a/drivers/input/rmi4/rmi_f34.c
> +++ b/drivers/input/rmi4/rmi_f34.c
> @@ -370,7 +370,7 @@ static int rmi_firmware_update(struct rmi_driver_data *data,
>
> f34 = dev_get_drvdata(&data->f34_container->dev);
>
> - if (f34->bl_version == 7) {
> + if (f34->bl_version >= 7) {
> if (data->pdt_props & HAS_BSR) {
> dev_err(dev, "%s: LTS not supported\n", __func__);
> return -ENODEV;
> @@ -382,7 +382,7 @@ static int rmi_firmware_update(struct rmi_driver_data *data,
> }
>
> /* Enter flash mode */
> - if (f34->bl_version == 7)
> + if (f34->bl_version >= 7)
> ret = rmi_f34v7_start_reflash(f34, fw);
> else
> ret = rmi_f34_enable_flash(f34);
> @@ -413,7 +413,7 @@ static int rmi_firmware_update(struct rmi_driver_data *data,
> f34 = dev_get_drvdata(&data->f34_container->dev);
>
> /* Perform firmware update */
> - if (f34->bl_version == 7)
> + if (f34->bl_version >= 7)
> ret = rmi_f34v7_do_reflash(f34, fw);
> else
> ret = rmi_f34_update_firmware(f34, fw);
--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat
Sorry this took me a little while to get to, but more sorry it took so long
for anyone to review this for you!
Anyway-luckily it seems I retained most of my RMI4 knowledge, so I was able to
follow along with the patch series pretty easily. I basically only had one
small comment about a missing Cc: [email protected] tag on one patch, but
the rest looks good to me:
Reviewed-by: Lyude Paul <[email protected]>
Feel free to poke me if you need my help getting anything else reviewed
On Wed, 2022-06-08 at 14:47 +0200, Matthias Schiffer wrote:
> This fixes the firmware update function with bootloader v8, allows to
> recover from interrupted updates with v7/v8, and does some code cleanup.
>
> I believe that the code that allows to recover from a broken partition
> table is also necessary to make flashing a different partition table
> work at all, but I wasn't able to verify that, as I don't have any firmware
> images with different partition tables to test with. In any case, I'm
> pretty sure that it is working correctly now, as recovery from a mostly
> empty flash without partition table has been tested successfully.
>
> I have only tested the new code with bootloader v8, and I don't have the
> documentation / interfacing guide for v7, so it would be great if anyone
> could check that I didn't break updates for v7.
>
>
> Matthias Schiffer (9):
> Input: synaptics-rmi4 - fix firmware update operations with bootloader
> v8
> Input: synaptics-rmi4 - introduce rmi_f34v7_check_command_status()
> helper
> Input: synaptics-rmi4 - fix command completion check for bootloader
> v7/v8
> Input: synaptics-rmi4 - rewrite partition table unconditionally
> Input: synaptics-rmi4 - reset after writing partition table
> Input: synaptics-rmi4 - make rmi_f34v7_erase_all() use the "erase all"
> command
> Input: synaptics-rmi4 - remove unneeded struct register_offset
> Input: synaptics-rmi4 - simplify rmi_f34v7_start_reflash()
> Input: synaptics-rmi4 - drop useless gotos in rmi_f34v7_do_reflash()
>
> drivers/input/rmi4/rmi_f34.c | 16 +-
> drivers/input/rmi4/rmi_f34.h | 17 --
> drivers/input/rmi4/rmi_f34v7.c | 349 +++++++--------------------------
> 3 files changed, 81 insertions(+), 301 deletions(-)
>
--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat
On Thu, 2022-09-22 at 16:06 -0400, Lyude Paul wrote:
> Would add a Cc: [email protected] for this
>
> With that fixed: Reviewed-by: Lyude Paul <[email protected]>
Thanks for the review!
Should I reroll with the added Cc? In my experience, patches will end
up queued for stable through AUTOSEL anyways as soon as the word "fix"
appears somewhere in the commit message.
>
> On Wed, 2022-06-08 at 14:48 +0200, Matthias Schiffer wrote:
> > Commit a6977d758fed ("Input: synaptics-rmi4 - support bootloader v8 in
> > f34v7") allowed the F34v7 driver to probe with bootloader v8, but it did
> > not update various other bootloader version checks in the F34 code.
> >
> > Fixes: a6977d758fed ("Input: synaptics-rmi4 - support bootloader v8 in f34v7")
> > Signed-off-by: Matthias Schiffer <[email protected]>
> > ---
> > drivers/input/rmi4/rmi_f34.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/input/rmi4/rmi_f34.c b/drivers/input/rmi4/rmi_f34.c
> > index e5dca9868f87..3afc94f679ed 100644
> > --- a/drivers/input/rmi4/rmi_f34.c
> > +++ b/drivers/input/rmi4/rmi_f34.c
> > @@ -370,7 +370,7 @@ static int rmi_firmware_update(struct rmi_driver_data *data,
> >
> > f34 = dev_get_drvdata(&data->f34_container->dev);
> >
> > - if (f34->bl_version == 7) {
> > + if (f34->bl_version >= 7) {
> > if (data->pdt_props & HAS_BSR) {
> > dev_err(dev, "%s: LTS not supported\n", __func__);
> > return -ENODEV;
> > @@ -382,7 +382,7 @@ static int rmi_firmware_update(struct rmi_driver_data *data,
> > }
> >
> > /* Enter flash mode */
> > - if (f34->bl_version == 7)
> > + if (f34->bl_version >= 7)
> > ret = rmi_f34v7_start_reflash(f34, fw);
> > else
> > ret = rmi_f34_enable_flash(f34);
> > @@ -413,7 +413,7 @@ static int rmi_firmware_update(struct rmi_driver_data *data,
> > f34 = dev_get_drvdata(&data->f34_container->dev);
> >
> > /* Perform firmware update */
> > - if (f34->bl_version == 7)
> > + if (f34->bl_version >= 7)
> > ret = rmi_f34v7_do_reflash(f34, fw);
> > else
> > ret = rmi_f34_update_firmware(f34, fw);
On Fri, 2022-09-23 at 11:12 +0200, Matthias Schiffer wrote:
> On Thu, 2022-09-22 at 16:06 -0400, Lyude Paul wrote:
> > Would add a Cc: [email protected] for this
> >
> > With that fixed: Reviewed-by: Lyude Paul <[email protected]>
>
> Thanks for the review!
>
> Should I reroll with the added Cc? In my experience, patches will end
> up queued for stable through AUTOSEL anyways as soon as the word "fix"
> appears somewhere in the commit message.
Ahhh, totally forgot about this. Nevermind then, it should be fine as-is :)
>
>
> >
> > On Wed, 2022-06-08 at 14:48 +0200, Matthias Schiffer wrote:
> > > Commit a6977d758fed ("Input: synaptics-rmi4 - support bootloader v8 in
> > > f34v7") allowed the F34v7 driver to probe with bootloader v8, but it did
> > > not update various other bootloader version checks in the F34 code.
> > >
> > > Fixes: a6977d758fed ("Input: synaptics-rmi4 - support bootloader v8 in f34v7")
> > > Signed-off-by: Matthias Schiffer <[email protected]>
> > > ---
> > > drivers/input/rmi4/rmi_f34.c | 6 +++---
> > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/input/rmi4/rmi_f34.c b/drivers/input/rmi4/rmi_f34.c
> > > index e5dca9868f87..3afc94f679ed 100644
> > > --- a/drivers/input/rmi4/rmi_f34.c
> > > +++ b/drivers/input/rmi4/rmi_f34.c
> > > @@ -370,7 +370,7 @@ static int rmi_firmware_update(struct rmi_driver_data *data,
> > >
> > > f34 = dev_get_drvdata(&data->f34_container->dev);
> > >
> > > - if (f34->bl_version == 7) {
> > > + if (f34->bl_version >= 7) {
> > > if (data->pdt_props & HAS_BSR) {
> > > dev_err(dev, "%s: LTS not supported\n", __func__);
> > > return -ENODEV;
> > > @@ -382,7 +382,7 @@ static int rmi_firmware_update(struct rmi_driver_data *data,
> > > }
> > >
> > > /* Enter flash mode */
> > > - if (f34->bl_version == 7)
> > > + if (f34->bl_version >= 7)
> > > ret = rmi_f34v7_start_reflash(f34, fw);
> > > else
> > > ret = rmi_f34_enable_flash(f34);
> > > @@ -413,7 +413,7 @@ static int rmi_firmware_update(struct rmi_driver_data *data,
> > > f34 = dev_get_drvdata(&data->f34_container->dev);
> > >
> > > /* Perform firmware update */
> > > - if (f34->bl_version == 7)
> > > + if (f34->bl_version >= 7)
> > > ret = rmi_f34v7_do_reflash(f34, fw);
> > > else
> > > ret = rmi_f34_update_firmware(f34, fw);
>
--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat