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 7466EC433EF for ; Wed, 20 Apr 2022 23:34:31 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id A077B83EB5; Thu, 21 Apr 2022 01:34:28 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: by phobos.denx.de (Postfix, from userid 109) id 15D3F83AD0; Thu, 21 Apr 2022 01:34:27 +0200 (CEST) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by phobos.denx.de (Postfix) with ESMTP id B9BB983EB5 for ; Thu, 21 Apr 2022 01:34:22 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=andre.przywara@arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id EE23C1477; Wed, 20 Apr 2022 16:34:21 -0700 (PDT) Received: from slackpad.lan (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 048FA3F73B; Wed, 20 Apr 2022 16:34:19 -0700 (PDT) Date: Thu, 21 Apr 2022 00:33:10 +0100 From: Andre Przywara To: Chris Morgan Cc: Jagan Teki , Simon Glass , Tom Rini , Chen-Yu Tsai , Hauke Mehrtens , Jernej Skrabec , Samuel Holland , Icenowy Zheng , Joe Hershberger , Wolfgang Denk , Daniel Wagenknecht , u-boot@lists.denx.de Subject: Re: [PATCH 4/7] sunxi: use boot source for determining environment location Message-ID: <20220421003310.35dc0b05@slackpad.lan> In-Reply-To: <20220415172822.GA1010@wintermute.localdomain> References: <20220111124607.863952-1-andre.przywara@arm.com> <20220111124607.863952-5-andre.przywara@arm.com> <20220415172822.GA1010@wintermute.localdomain> Organization: Arm Ltd. X-Mailer: Claws Mail 4.0.0 (GTK+ 3.24.31; x86_64-slackware-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit 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.5 at phobos.denx.de X-Virus-Status: Clean On Fri, 15 Apr 2022 12:28:22 -0500 Chris Morgan wrote: Hi Chris, > On Tue, Jan 11, 2022 at 12:46:04PM +0000, Andre Przywara wrote: > > Currently we only support to load the environment from raw MMC or FAT > > locations on Allwinner boards. With the advent of SPI flash we probably > > also want to support using the environment there, so we need to become > > a bit more flexible. > > > > Change the environment priority function to take the boot source into > > account. When booted from eMMC or SD card, we use FAT or MMC, if > > configured, as before. > > If we are booted from SPI flash, we try to use the environment from > > there, if possible. The same is true for NAND flash booting, although > > this is somewhat theoretical right now (as untested). > > > > This way we can use the same image for SD and SPI flash booting, which > > allows us to simply copy a booted image from SD card to the SPI flash, > > for instance. > > Unfortunately after a git-bisect I can confirm this patch breaks > booting in FEL mode whenever the environment is set to > CONFIG_ENV_IS_NOWHERE. Tested and reproducible on an NTC CHIP or a > Source Parts Popcorn (both Allwinner R8 chips). > > The curious thing though is that I could never see env_get_location > called, but whenever I either set the environment to something like > FAT or comment out this change (which I believe causes the non-board > specific version of env_get_location to be used) it would boot again. > So while I can confirm this patch breaks booting FEL when the env is > set to nowhere, I was not able to figure out why that is. Alright, thanks for the report. I see the problem: The default fallback for the new env_get_location() is ENVL_FAT, as 157 out of the 160 Allwinner boards define CONFIG_ENV_IS_IN_FAT. However the C.H.I.P. boards are one of the three exceptions, and returning FAT when the FAT environment driver is not compiled in will not end well. I don't have any CHIP board, but simulated this by configuring a normal board with just defining CONFIG_ENV_IS_NOWHERE. I made a patch that tries to choose a matching fallback. It's not very elegant, but seems to fix the problem for me. Please test this once posted. Cheers, Andre > > Thank you. > > > > > Signed-off-by: Andre Przywara > > --- > > board/sunxi/board.c | 51 ++++++++++++++++++++++++++++++++++++++------- > > 1 file changed, 43 insertions(+), 8 deletions(-) > > > > diff --git a/board/sunxi/board.c b/board/sunxi/board.c > > index 2790a0f9e8..2472343d00 100644 > > --- a/board/sunxi/board.c > > +++ b/board/sunxi/board.c > > @@ -170,21 +170,56 @@ void i2c_init_board(void) > > #endif > > } > > > > -#if defined(CONFIG_ENV_IS_IN_MMC) && defined(CONFIG_ENV_IS_IN_FAT) > > +/* > > + * Try to use the environment from the boot source first. > > + * For MMC, this means a FAT partition on the boot device (SD or eMMC). > > + * If the raw MMC environment is also enabled, this is tried next. > > + * SPI flash falls back to FAT (on SD card). > > + */ > > enum env_location env_get_location(enum env_operation op, int prio) > > { > > - switch (prio) { > > - case 0: > > - return ENVL_FAT; > > + enum env_location boot_loc = ENVL_FAT; > > > > - case 1: > > - return ENVL_MMC; > > + gd->env_load_prio = prio; > > > > + switch (sunxi_get_boot_device()) { > > + case BOOT_DEVICE_MMC1: > > + case BOOT_DEVICE_MMC2: > > + boot_loc = ENVL_FAT; > > + break; > > + case BOOT_DEVICE_NAND: > > + if (IS_ENABLED(CONFIG_ENV_IS_IN_NAND)) > > + boot_loc = ENVL_NAND; > > + break; > > + case BOOT_DEVICE_SPI: > > + if (IS_ENABLED(CONFIG_ENV_IS_IN_SPI_FLASH)) > > + boot_loc = ENVL_SPI_FLASH; > > + break; > > + case BOOT_DEVICE_BOARD: > > + break; > > default: > > - return ENVL_UNKNOWN; > > + break; > > + } > > + > > + /* Always try to access the environment on the boot device first. */ > > + if (prio == 0) > > + return boot_loc; > > + > > + if (prio == 1) { > > + switch (boot_loc) { > > + case ENVL_SPI_FLASH: > > + return ENVL_FAT; > > + case ENVL_FAT: > > + if (IS_ENABLED(CONFIG_ENV_IS_IN_MMC)) > > + return ENVL_MMC; > > + break; > > + default: > > + break; > > + } > > } > > + > > + return ENVL_UNKNOWN; > > } > > -#endif > > > > #ifdef CONFIG_DM_MMC > > static void mmc_pinmux_setup(int sdc);