[PATCH 0/9] Input: synaptics-rmi4 - Bootloader v7/v8 firmware update improvements

[PATCH 0/9] Input: synaptics-rmi4 - Bootloader v7/v8 firmware update improvements

[Matthias Schiffer]

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

[PATCH 1/9] Input: synaptics-rmi4 - fix firmware update operations with bootloader v8

[Matthias Schiffer]

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 <matthias.schiffer@ew.tq-group.com>
---
 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

[PATCH 2/9] Input: synaptics-rmi4 - introduce rmi_f34v7_check_command_status() helper

[Matthias Schiffer]

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 <matthias.schiffer@ew.tq-group.com>
---
 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

[PATCH 3/9] Input: synaptics-rmi4 - fix command completion check for bootloader v7/v8

[Matthias Schiffer]

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 <matthias.schiffer@ew.tq-group.com>
---
 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

[PATCH 4/9] Input: synaptics-rmi4 - rewrite partition table unconditionally

[Matthias Schiffer]

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 <matthias.schiffer@ew.tq-group.com>
---
 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

[PATCH 5/9] Input: synaptics-rmi4 - reset after writing partition table

[Matthias Schiffer]

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 <matthias.schiffer@ew.tq-group.com>
---
 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

[PATCH 6/9] Input: synaptics-rmi4 - make rmi_f34v7_erase_all() use the "erase all" command

[Matthias Schiffer]

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 <matthias.schiffer@ew.tq-group.com>
---
 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

[PATCH 7/9] Input: synaptics-rmi4 - remove unneeded struct register_offset

[Matthias Schiffer]

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 <matthias.schiffer@ew.tq-group.com>
---
 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

[PATCH 8/9] Input: synaptics-rmi4 - simplify rmi_f34v7_start_reflash()

[Matthias Schiffer]

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 <matthias.schiffer@ew.tq-group.com>
---
 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

[PATCH 9/9] Input: synaptics-rmi4 - drop useless gotos in rmi_f34v7_do_reflash()

[Matthias Schiffer]

Returning directly makes the code clearer.

Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
---
 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

Re: [PATCH 0/9] Input: synaptics-rmi4 - Bootloader v7/v8 firmware update improvements

[Matthias Schiffer]

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(-)
> 

Re: [PATCH 0/9] Input: synaptics-rmi4 - Bootloader v7/v8 firmware update improvements

[Matthias Schiffer]

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(-)
> > 

Re: [PATCH 0/9] Input: synaptics-rmi4 - Bootloader v7/v8 firmware update improvements

[Matthias Schiffer]

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(-)
> > > 

Re: [PATCH 0/9] Input: synaptics-rmi4 - Bootloader v7/v8 firmware update improvements

[Lyude Paul]

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

Re: [PATCH 0/9] Input: synaptics-rmi4 - Bootloader v7/v8 firmware update improvements

[Dmitry Torokhov]

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

Re: [PATCH 0/9] Input: synaptics-rmi4 - Bootloader v7/v8 firmware update improvements

[Lyude Paul]

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

Re: [PATCH 1/9] Input: synaptics-rmi4 - fix firmware update operations with bootloader v8

[Lyude Paul]

Would add a Cc: stable@vger.kernel.org for this

With that fixed: Reviewed-by: Lyude Paul <lyude@redhat.com>

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 <matthias.schiffer@ew.tq-group.com>
> ---
>  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

Re: [PATCH 0/9] Input: synaptics-rmi4 - Bootloader v7/v8 firmware update improvements

[Lyude Paul]

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: stable@vger.kernel.org tag on one patch, but
the rest looks good to me:

Reviewed-by: Lyude Paul <lyude@redhat.com>

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

Re: [PATCH 1/9] Input: synaptics-rmi4 - fix firmware update operations with bootloader v8

[Matthias Schiffer]

On Thu, 2022-09-22 at 16:06 -0400, Lyude Paul wrote:
> Would add a Cc: stable@vger.kernel.org for this
> 
> With that fixed: Reviewed-by: Lyude Paul <lyude@redhat.com>

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 <matthias.schiffer@ew.tq-group.com>
> > ---
> >  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);

Re: [PATCH 1/9] Input: synaptics-rmi4 - fix firmware update operations with bootloader v8

[Lyude Paul]

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: stable@vger.kernel.org for this
> > 
> > With that fixed: Reviewed-by: Lyude Paul <lyude@redhat.com>
> 
> 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 <matthias.schiffer@ew.tq-group.com>
> > > ---
> > >  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