[PATCH] fastboot: mmc: switch to user hwpart after erase/flash

Mattijs Korpershoek mkorpershoek at baylibre.com
Mon Oct 10 10:05:05 CEST 2022


Before erasing (or flashing) an mmc hardware partition, such as
mmc0boot0 or mmc0boot1, we select it using blk_dselect_hwpart().

However, we don't select back the normal (userdata) hwpartition
afterwards.

For example, the following sequence is broken:

$ fastboot erase mmc2boot1    # switch to hwpart 1
$ fastboot reboot bootloader  # attempts to read GPT from hwpart 1

> writing 128 blocks starting at 8064...
> ........ wrote 65536 bytes to 'mmc0boot1'
> GUID Partition Table Header signature is wrong: 0xFB4EC30FC5B7E5B2 != 0x5452415020494645
> find_valid_gpt: *** ERROR: Invalid GPT ***
> GUID Partition Table Header signature is wrong: 0x0 != 0x5452415020494645
> find_valid_gpt: *** ERROR: Invalid Backup GPT ***
> Error: mmc 2:misc read failed (-2)

The GPT is stored in hwpart0 (userdata), not hwpart1 (mmc0boot1)

As the blk documentation states:
> Once selected only the region of the device
> covered by that partition is accessible.

Add a cleanup function, fastboot_mmc_select_default_hwpart() which
selects back the default hwpartition (userdata).

Note: this is more visible since
commit a362ce214f03 ("fastboot: Implement generic fastboot_set_reboot_flag")
because "fastboot reboot bootloader" will access the "misc" partition.

Signed-off-by: Mattijs Korpershoek <mkorpershoek at baylibre.com>
---
This fix has been used for over a year in the AOSP tree at [1]

[1] https://android.googlesource.com/device/amlogic/yukawa/+/c008ddaec0943adda394b229e83e147f6165773c
---
 drivers/fastboot/fb_mmc.c | 20 +++++++++++++++++---
 include/mmc.h             |  1 +
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/fastboot/fb_mmc.c b/drivers/fastboot/fb_mmc.c
index 033c510bc096..dedef7c4deb1 100644
--- a/drivers/fastboot/fb_mmc.c
+++ b/drivers/fastboot/fb_mmc.c
@@ -199,6 +199,12 @@ static void write_raw_image(struct blk_desc *dev_desc,
 	fastboot_okay(NULL, response);
 }
 
+static void fastboot_mmc_select_default_hwpart(struct blk_desc *dev_desc)
+{
+	if (blk_dselect_hwpart(dev_desc, MMC_PART_USERDATA))
+		pr_err("Failed to restore hwpart to userdata\n");
+}
+
 #if defined(CONFIG_FASTBOOT_MMC_BOOT_SUPPORT) || \
 	defined(CONFIG_FASTBOOT_MMC_USER_SUPPORT)
 static int fb_mmc_erase_mmc_hwpart(struct blk_desc *dev_desc)
@@ -246,7 +252,7 @@ static void fb_mmc_boot_ops(struct blk_desc *dev_desc, void *buffer,
 		if (blkcnt > dev_desc->lba) {
 			pr_err("Image size too large\n");
 			fastboot_fail("Image size too large", response);
-			return;
+			goto fail_restore_hwpart;
 		}
 
 		debug("Start Flashing Image to EMMC_BOOT%d...\n", hwpart);
@@ -257,7 +263,7 @@ static void fb_mmc_boot_ops(struct blk_desc *dev_desc, void *buffer,
 			pr_err("Failed to write EMMC_BOOT%d\n", hwpart);
 			fastboot_fail("Failed to write EMMC_BOOT part",
 				      response);
-			return;
+			goto fail_restore_hwpart;
 		}
 
 		printf("........ wrote %lu bytes to EMMC_BOOT%d\n",
@@ -267,11 +273,15 @@ static void fb_mmc_boot_ops(struct blk_desc *dev_desc, void *buffer,
 			pr_err("Failed to erase EMMC_BOOT%d\n", hwpart);
 			fastboot_fail("Failed to erase EMMC_BOOT part",
 				      response);
-			return;
+			goto fail_restore_hwpart;
 		}
 	}
 
 	fastboot_okay(NULL, response);
+	return;
+
+fail_restore_hwpart:
+	fastboot_mmc_select_default_hwpart(dev_desc);
 }
 #endif
 
@@ -630,6 +640,8 @@ void fastboot_mmc_flash_write(const char *cmd, void *download_buffer,
 		write_raw_image(dev_desc, &info, cmd, download_buffer,
 				download_bytes, response);
 	}
+
+	fastboot_mmc_select_default_hwpart(dev_desc);
 }
 
 /**
@@ -694,6 +706,8 @@ void fastboot_mmc_erase(const char *cmd, char *response)
 
 	blks = fb_mmc_blk_write(dev_desc, blks_start, blks_size, NULL);
 
+	fastboot_mmc_select_default_hwpart(dev_desc);
+
 	if (blks != blks_size) {
 		pr_err("failed erasing from device %d\n", dev_desc->devnum);
 		fastboot_fail("failed erasing from device", response);
diff --git a/include/mmc.h b/include/mmc.h
index 027e8bcc73a6..cdd457a60a5b 100644
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -362,6 +362,7 @@ enum mmc_voltage {
 /* The number of MMC physical partitions.  These consist of:
  * boot partitions (2), general purpose partitions (4) in MMC v4.4.
  */
+#define MMC_PART_USERDATA	0
 #define MMC_NUM_BOOT_PARTITION	2
 #define MMC_PART_RPMB           3       /* RPMB partition number */
 

---
base-commit: f5717231abad983b4d8f87db612ae60dec86cb1b
change-id: 20221010-switch-hwpart-7fed7746db59

Best regards,
-- 
Mattijs Korpershoek <mkorpershoek at baylibre.com>


More information about the U-Boot mailing list