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 3B481C43334 for ; Wed, 29 Jun 2022 13:37:10 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 90E9D842BE; Wed, 29 Jun 2022 15:37:07 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=thorsis.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 6CEB8842C0; Wed, 29 Jun 2022 15:37:05 +0200 (CEST) Received: from mail.thorsis.com (mail.thorsis.com [92.198.35.195]) by phobos.denx.de (Postfix) with ESMTP id B453283E68 for ; Wed, 29 Jun 2022 15:37:01 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=thorsis.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=ada@thorsis.com From: Alexander Dahl To: u-boot@lists.denx.de Cc: Pali =?ISO-8859-1?Q?Roh=E1r?= , Simon Glass , Sjoerd Simons , Govindaraji Sivanantham , Hiremath Gireesh , Marcel Ziswiler , Frieder Schrempf , Parthiban Nallathambi , Navin Sankar Velliangiri , "Derald D. Woods" , Martyn Welch , Patrick Delaunay , Patrice Chotard Subject: Re: [PATCH] distroboot: Fix ubifs Date: Wed, 29 Jun 2022 15:36:57 +0200 Message-ID: <8595563.AsjhMEDtd8@ada> In-Reply-To: <20220531083236.18968-1-pali@kernel.org> References: <20220531083236.18968-1-pali@kernel.org> Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="iso-8859-1" X-Clacks-Overhead: GNU Terry Pratchett 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 Hello Pali, had a look at this patch, and have some questions. See below. Am Dienstag, 31. Mai 2022, 10:32:36 CEST schrieb Pali Roh=E1r: > Fix multiple issues in ubifs distroboot code: >=20 > U-Boot supports attaching only one MTD device as UBI at the time. So > always call 'ubifsmount ubi0:${bootubivol}' for mounting UBI volume > ${bootubivol}. Usage of 'ubi${devnum}' is incorrect as 'ubi part' > command attach MTD device always as UBI device ubi0. Agreed. > Set distroboot ${bootfstype} variable to ubifs in ubifs_boot command. > Distroboot scripts require ${bootfstype} variable to be properly set and = it > is already set for all other boot types. =46rom grepping through u-boot source, I can not quite follow. Where is th= at=20 variable set and where is it used? > Set distroboot ${distro_bootpart} variable to ${bootubivol} value. UBI > device does not have partitions, but has volumes. Distroboot scripts > require something to be set in ${distro_bootpart} variable, so set it to > the UBI volume which is currently mounted by ubifs. >=20 > Set distroboot ${devnum} variable to fixed string "ubi0". ubifs code > differs from the other partition code that it requires "ubi" prefix before > number. Okay. > Explicitly unmount ubifs volume after loading all data from it. This allo= ws > to detach UBI device from MTD device. Okay. > Move definition of MTD device with UBI and UBI volume with ubifs filesyst= em > from global env variables ${bootubipart} and ${bootubivol} into the > distroboot "func" macro, defined in board include config files. UBIFS > distroboot macros then set ${bootubipart} and ${bootubivol} local variabl= es > for compatibility with existing distroboot scripts. This might be problematic. For a downstream board we use the 'bootubivol' = env=20 variable to switch between different volumes for update purposes. Means on= =20 firmware update we modify U-Boot environment from Linux (fw_setenv) after=20 successfully updating the inactive volume. > This last change allows to define more UBIFS target devices and make it > clear what is boot MTD/UBI device. How would one switch the default UBI volume to be booted from then? Greets Alex > All board include config files are adjusted to use this new scheme of > specifying boot MTD/UBI device. >=20 > Signed-off-by: Pali Roh=E1r > --- > CI test passed on https://github.com/u-boot/u-boot/pull/179 > --- > include/config_distro_bootcmd.h | 27 ++++++++++++++++----------- > include/configs/am335x_guardian.h | 3 +-- > include/configs/colibri-imx6ull.h | 1 - > include/configs/colibri_imx7.h | 1 - > include/configs/kontron-sl-mx6ul.h | 2 +- > include/configs/mys_6ulx.h | 2 +- > include/configs/npi_imx6ull.h | 2 +- > include/configs/omap3_beagle.h | 4 +--- > include/configs/omap3_evm.h | 4 +--- > include/configs/pcl063.h | 2 +- > include/configs/stm32mp15_common.h | 2 +- > include/configs/uniphier.h | 2 +- > 12 files changed, 25 insertions(+), 27 deletions(-) >=20 > diff --git a/include/config_distro_bootcmd.h > b/include/config_distro_bootcmd.h index c55023889cab..c6e9c497413d 100644 > --- a/include/config_distro_bootcmd.h > +++ b/include/config_distro_bootcmd.h > @@ -70,18 +70,23 @@ > #ifdef CONFIG_CMD_UBIFS > #define BOOTENV_SHARED_UBIFS \ > "ubifs_boot=3D" \ > - "env exists bootubipart || " \ > - "env set bootubipart UBI; " \ > - "env exists bootubivol || " \ > - "env set bootubivol boot; " \ > "if ubi part ${bootubipart} && " \ > - "ubifsmount ubi${devnum}:${bootubivol}; " \ > + "ubifsmount ubi0:${bootubivol}; " \ > "then " \ > "devtype=3Dubi; " \ > + "devnum=3Dubi0; " \ > + "bootfstype=3Dubifs; " \ > + "distro_bootpart=3D${bootubivol}; " \ > "run scan_dev_for_boot; " \ > + "ubifsumount; " \ > "fi\0" > -#define BOOTENV_DEV_UBIFS BOOTENV_DEV_BLKDEV > -#define BOOTENV_DEV_NAME_UBIFS BOOTENV_DEV_NAME_BLKDEV > +#define BOOTENV_DEV_UBIFS(devtypeu, devtypel, instance, bootubipart, > bootubivol) \ + "bootcmd_ubifs" #instance "=3D" \ > + "bootubipart=3D" #bootubipart "; " \ > + "bootubivol=3D" #bootubivol "; " \ > + "run ubifs_boot\0" > +#define BOOTENV_DEV_NAME_UBIFS(devtypeu, devtypel, instance, bootubipart, > bootubivol) \ + #devtypel #instance " " > #else > #define BOOTENV_SHARED_UBIFS > #define BOOTENV_DEV_UBIFS \ > @@ -411,13 +416,13 @@ > BOOT_TARGET_DEVICES_references_PXE_without_CONFIG_CMD_DHCP_or_PXE > #endif >=20 > -#define BOOTENV_DEV_NAME(devtypeu, devtypel, instance) \ > - BOOTENV_DEV_NAME_##devtypeu(devtypeu, devtypel, instance) > +#define BOOTENV_DEV_NAME(devtypeu, devtypel, instance, ...) \ > + BOOTENV_DEV_NAME_##devtypeu(devtypeu, devtypel, instance, ## __VA_ARGS_= _) > #define BOOTENV_BOOT_TARGETS \ > "boot_targets=3D" BOOT_TARGET_DEVICES(BOOTENV_DEV_NAME) "\0" >=20 > -#define BOOTENV_DEV(devtypeu, devtypel, instance) \ > - BOOTENV_DEV_##devtypeu(devtypeu, devtypel, instance) > +#define BOOTENV_DEV(devtypeu, devtypel, instance, ...) \ > + BOOTENV_DEV_##devtypeu(devtypeu, devtypel, instance, ## __VA_ARGS__) > #define BOOTENV \ > BOOTENV_SHARED_HOST \ > BOOTENV_SHARED_MMC \ > diff --git a/include/configs/am335x_guardian.h > b/include/configs/am335x_guardian.h index b92703205cde..340715dad5c6 1006= 44 > --- a/include/configs/am335x_guardian.h > +++ b/include/configs/am335x_guardian.h > @@ -29,7 +29,7 @@ > "ramdisk_addr_r=3D0x88080000\0" \ >=20 > #define BOOT_TARGET_DEVICES(func) \ > - func(UBIFS, ubifs, 0) > + func(UBIFS, ubifs, 0, UBI, rootfs) >=20 > #define AM335XX_BOARD_FDTFILE "fdtfile=3D" CONFIG_DEFAULT_DEVICE_TREE > ".dtb\0" >=20 > @@ -54,7 +54,6 @@ > GUARDIAN_DEFAULT_PROD_ENV \ > "autoload=3Dno\0" \ > "backlight_brightness=3D50\0" \ > - "bootubivol=3Drootfs\0" \ > "distro_bootcmd=3D" \ > "setenv rootflags \"bulk_read,chk_data_crc\"; " \ > "setenv ethact usb_ether; " \ > diff --git a/include/configs/colibri-imx6ull.h > b/include/configs/colibri-imx6ull.h index 9e5212acb2ee..a0a0e1767fe0 1006= 44 > --- a/include/configs/colibri-imx6ull.h > +++ b/include/configs/colibri-imx6ull.h > @@ -91,7 +91,6 @@ > UBI_BOOTCMD \ > UBOOT_UPDATE \ > "boot_script_dhcp=3Dboot.scr\0" \ > - "bootubipart=3Dubi\0" \ > "console=3Dttymxc0\0" \ > "defargs=3Duser_debug=3D30\0" \ > "fdt_board=3Deval-v3\0" \ > diff --git a/include/configs/colibri_imx7.h b/include/configs/colibri_imx= 7.h > index 3dba7bcef258..3cad17777975 100644 > --- a/include/configs/colibri_imx7.h > +++ b/include/configs/colibri_imx7.h > @@ -131,7 +131,6 @@ > UBOOT_UPDATE \ > "boot_file=3DzImage\0" \ > "boot_script_dhcp=3Dboot.scr\0" \ > - "bootubipart=3Dubi\0" \ > "console=3Dttymxc0\0" \ > "defargs=3D\0" \ > "fdt_board=3Deval-v3\0" \ > diff --git a/include/configs/kontron-sl-mx6ul.h > b/include/configs/kontron-sl-mx6ul.h index 7bc402d578e8..b4808d2bbf75 > 100644 > --- a/include/configs/kontron-sl-mx6ul.h > +++ b/include/configs/kontron-sl-mx6ul.h > @@ -45,7 +45,7 @@ > #define BOOT_TARGET_DEVICES(func) \ > func(MMC, mmc, 1) \ > func(MMC, mmc, 0) \ > - func(UBIFS, ubifs, 0) \ > + func(UBIFS, ubifs, 0, UBI, boot) \ > func(USB, usb, 0) \ > func(PXE, pxe, na) \ > func(DHCP, dhcp, na) > diff --git a/include/configs/mys_6ulx.h b/include/configs/mys_6ulx.h > index 6801fc109eae..663820177a3e 100644 > --- a/include/configs/mys_6ulx.h > +++ b/include/configs/mys_6ulx.h > @@ -59,7 +59,7 @@ >=20 > #define BOOT_TARGET_DEVICES(func) \ > func(MMC, mmc, 0) \ > - func(UBIFS, ubifs, 0) \ > + func(UBIFS, ubifs, 0, UBI, boot) \ > func(PXE, pxe, na) \ > func(DHCP, dhcp, na) >=20 > diff --git a/include/configs/npi_imx6ull.h b/include/configs/npi_imx6ull.h > index c250fa650603..ebb887544e08 100644 > --- a/include/configs/npi_imx6ull.h > +++ b/include/configs/npi_imx6ull.h > @@ -67,7 +67,7 @@ >=20 > #define BOOT_TARGET_DEVICES(func) \ > func(MMC, mmc, 0) \ > - func(UBIFS, ubifs, 0) \ > + func(UBIFS, ubifs, 0, UBI, boot) \ > func(PXE, pxe, na) \ > func(DHCP, dhcp, na) >=20 > diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagl= e.h > index 158773acedb9..f5da08cf3359 100644 > --- a/include/configs/omap3_beagle.h > +++ b/include/configs/omap3_beagle.h > @@ -65,7 +65,7 @@ > #define BOOT_TARGET_DEVICES(func) \ > func(MMC, mmc, 0) \ > func(LEGACY_MMC, legacy_mmc, 0) \ > - func(UBIFS, ubifs, 0) \ > + func(UBIFS, ubifs, 0, rootfs, rootfs) \ > func(NAND, nand, 0) >=20 > #else /* !CONFIG_MTD_RAW_NAND */ > @@ -87,8 +87,6 @@ > "bootenv=3DuEnv.txt\0" \ > "bootfile=3DzImage\0" \ > "bootpart=3D0:2\0" \ > - "bootubivol=3Drootfs\0" \ > - "bootubipart=3Drootfs\0" \ > "usbtty=3Dcdc_acm\0" \ > "mpurate=3Dauto\0" \ > "buddy=3Dnone\0" \ > diff --git a/include/configs/omap3_evm.h b/include/configs/omap3_evm.h > index eeb9ef8c741a..cc98e03096ab 100644 > --- a/include/configs/omap3_evm.h > +++ b/include/configs/omap3_evm.h > @@ -60,7 +60,7 @@ > #define BOOT_TARGET_DEVICES(func) \ > func(MMC, mmc, 0) \ > func(LEGACY_MMC, legacy_mmc, 0) \ > - func(UBIFS, ubifs, 0) \ > + func(UBIFS, ubifs, 0, rootfs, rootfs) \ > func(NAND, nand, 0) >=20 > #else /* !CONFIG_MTD_RAW_NAND */ > @@ -88,8 +88,6 @@ > "bootenv=3DuEnv.txt\0" \ > "bootfile=3DzImage\0" \ > "bootpart=3D0:2\0" \ > - "bootubivol=3Drootfs\0" \ > - "bootubipart=3Drootfs\0" \ > "optargs=3D\0" \ > "nandroot=3Dubi0:rootfs ubi.mtd=3Drootfs rw noinitrd\0" \ > "nandrootfstype=3Dubifs rootwait\0" \ > diff --git a/include/configs/pcl063.h b/include/configs/pcl063.h > index 31b7d07a24cd..c3f7e7eb2c4b 100644 > --- a/include/configs/pcl063.h > +++ b/include/configs/pcl063.h > @@ -71,7 +71,7 @@ >=20 > #define BOOT_TARGET_DEVICES(func) \ > func(MMC, mmc, 0) \ > - func(UBIFS, ubifs, 0) \ > + func(UBIFS, ubifs, 0, UBI, boot) \ > func(PXE, pxe, na) \ > func(DHCP, dhcp, na) >=20 > diff --git a/include/configs/stm32mp15_common.h > b/include/configs/stm32mp15_common.h index 6b40cdb01779..68ea56e69c98 > 100644 > --- a/include/configs/stm32mp15_common.h > +++ b/include/configs/stm32mp15_common.h > @@ -77,7 +77,7 @@ > #endif >=20 > #ifdef CONFIG_CMD_UBIFS > -#define BOOT_TARGET_UBIFS(func) func(UBIFS, ubifs, 0) > +#define BOOT_TARGET_UBIFS(func) func(UBIFS, ubifs, 0, UBI, boot) > #else > #define BOOT_TARGET_UBIFS(func) > #endif > diff --git a/include/configs/uniphier.h b/include/configs/uniphier.h > index f813f88cdd7a..640a29067d85 100644 > --- a/include/configs/uniphier.h > +++ b/include/configs/uniphier.h > @@ -20,7 +20,7 @@ > #endif >=20 > #ifdef CONFIG_CMD_UBIFS > -#define BOOT_TARGET_DEVICE_UBIFS(func) func(UBIFS, ubifs, 0) > +#define BOOT_TARGET_DEVICE_UBIFS(func) func(UBIFS, ubifs, 0, UBI, boot) > #else > #define BOOT_TARGET_DEVICE_UBIFS(func) > #endif