Hi Sean,
Thank you for your review.
On Mon, Oct 10, 2022 at 11:38, Sean Anderson <sean.ander...@seco.com> wrote:
> On 10/10/22 4:05 AM, Mattijs Korpershoek wrote:
>> 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)
>
> This seems like a problem with your boot script. You should run `mmc dev
> ${mmcdev}` (and `mmc rescan`) before trying to access any MMC
> partitions. This is especially important after fastbooting, since the
> partition layout (or active hardware partition) may have changed.
I don't think I can fix this in my boot script.
My boot script runs => fastboot usb 0 which will start a loop to receive
fastboot commands from the host.
The full u-boot env i'm using is in include/configs/meson64_android.h
This loop will go on until "fastboot reboot <reboot_reason>" is called.
I do can "work around" this by using the following fastboot sequence:
$ fastboot erase mmc2boot1
$ fastboot oem format # switch backs to hwpart 0 (USER)
$ fastboot reboot bootloader
But that's not a good option to me. From a user's perspective, it should
not matter in which order we issue fastboot commands to the board.
Maybe I should fix bcb.c instead to make sure that it selects hwpart0
before reading/writing into the bcb partition ?
>
>> 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 <mkorpersh...@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))
>
> If you really want to do this, we shoud save the current partition and
> restore if after fastbooting. Always switching to hardware partition 0
> is just as broken as the current behavior.
ACK. This patch just replaces one broken behaviour by another one. I
will see if I can save the hardware partition state instead or patch the
bcb command.
>
> --Sean
>
