From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 0524CC433F5 for ; Tue, 11 Oct 2022 12:07:29 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id B163284E34; Tue, 11 Oct 2022 14:07:27 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=baylibre.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=baylibre-com.20210112.gappssmtp.com header.i=@baylibre-com.20210112.gappssmtp.com header.b="lVaWIuXL"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id E96D084DB0; Tue, 11 Oct 2022 14:07:25 +0200 (CEST) Received: from mail-wr1-x432.google.com (mail-wr1-x432.google.com [IPv6:2a00:1450:4864:20::432]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 1EE6A84EE8 for ; Tue, 11 Oct 2022 14:07:22 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=baylibre.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=mkorpershoek@baylibre.com Received: by mail-wr1-x432.google.com with SMTP id b4so21290517wrs.1 for ; Tue, 11 Oct 2022 05:07:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=baylibre-com.20210112.gappssmtp.com; s=20210112; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:from:to:cc:subject:date:message-id:reply-to; bh=va+2l8+86uwepzBcTHbPmV0ueDRpOJf70LwsF/XJqWM=; b=lVaWIuXLlrbGgK7OD1tR71DEF8KNpVFA3vYonkognIT373i4Cjd+DUniYOYxdEIQR3 j5NgKggP8b4aO8sQpOMoC5/GwJKfh4ggh6tFu5UZkFcW3cJwY7ZyOjYolV9p0E3NdnDi 1rd/8i63c21g7jOr2cX0RqzTostQj/H7k+R3s8n/ZDh/gp1TXmlHKQMQfP3jSDc73hZ1 Ht+sgfZ6aVWOm/MmDjog6Y8aqCRwPZ6rYKveWHKSB4E1yRsXk5k2oeE7jwH6fG8Bfry4 d8oyAFBNsG/5IMapopAyVel3TahvhG+/z3Hsm7K68LYIDe/QsaXvxa+as6y95alNk7Fk tvyQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=va+2l8+86uwepzBcTHbPmV0ueDRpOJf70LwsF/XJqWM=; b=UhuLVlSpgcTBQD2j81fubLNTEBCWMNKCsmDIVFpymkSYQabNncj0tTtxuXxvUMV8a5 LvjPynrkQUAhODcy+uGkSMe1L2tcjNDG+IWK6PRvjl5QtKSKOsog0iL3Rn4K7jKHeFlE a7/tcunhM1Vxg0cSaDkyBZZqmDj8nFewsaytBRleKOc1ETetmZuxTzIevoVblXrZ6Mxq EWJFqiUABTq8JkLdDJO2DW0K+lZHMUHCVjTC8MIYUwLJQhmMxG8YQOtl2IIoiqWoaJwo qczb18HuYeW/0QBoZiMTD/EYeHEHBurfVvbag8R3YSxCxmSeioJjWhAvzHXUyDoUF91E 24wQ== X-Gm-Message-State: ACrzQf0WI99VhbT+un/lTlPsBuj++OLyJ99BawnMrgsLDyYrCtFnfNmp 1uUB7G/yZtTofHfSAy03RduGAg== X-Google-Smtp-Source: AMsMyM6NA74eRrqQk7TMNXrJdDB7MDJL+dI+NGv3J7RtDIOYATyquG4gt6f4SegZciauTKERZg1N3A== X-Received: by 2002:a05:6000:1786:b0:22e:41c0:cb0e with SMTP id e6-20020a056000178600b0022e41c0cb0emr14354327wrg.93.1665490041427; Tue, 11 Oct 2022 05:07:21 -0700 (PDT) Received: from localhost ([82.66.159.240]) by smtp.gmail.com with ESMTPSA id o13-20020a5d62cd000000b00226dba960b4sm11130592wrv.3.2022.10.11.05.07.20 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 11 Oct 2022 05:07:20 -0700 (PDT) From: Mattijs Korpershoek To: Sean Anderson , Patrick Delaunay Cc: u-boot@lists.denx.de, Simon Glass , Jaehoon Chung , Matthias Schiffer Subject: Re: [PATCH] fastboot: mmc: switch to user hwpart after erase/flash In-Reply-To: References: <20221010-switch-hwpart-v1-0-52cbd34c72b7@baylibre.com> Date: Tue, 11 Oct 2022 14:07:20 +0200 Message-ID: <87k05636o7.fsf@baylibre.com> MIME-Version: 1.0 Content-Type: text/plain X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.6 at phobos.denx.de X-Virus-Status: Clean Hi Sean, Thank you for your review. On Mon, Oct 10, 2022 at 11:38, Sean Anderson 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 " 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 >> --- >> 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 >