From: Alexander Dahl <ada@thorsis.com>
To: u-boot@lists.denx.de
Cc: "Pali Rohár" <pali@kernel.org>, "Simon Glass" <sjg@chromium.org>,
"Sjoerd Simons" <sjoerd.simons@collabora.co.uk>,
"Govindaraji Sivanantham" <Govindaraji.Sivanantham@in.bosch.com>,
"Hiremath Gireesh" <Gireesh.Hiremath@in.bosch.com>,
"Marcel Ziswiler" <marcel.ziswiler@toradex.com>,
"Frieder Schrempf" <frieder.schrempf@kontron.de>,
"Parthiban Nallathambi" <parthiban@linumiz.com>,
"Navin Sankar Velliangiri" <navin@linumiz.com>,
"Derald D. Woods" <woods.technical@gmail.com>,
"Martyn Welch" <martyn.welch@collabora.com>,
"Patrick Delaunay" <patrick.delaunay@foss.st.com>,
"Patrice Chotard" <patrice.chotard@foss.st.com>
Subject: Re: [PATCH] distroboot: Fix ubifs
Date: Wed, 29 Jun 2022 15:36:57 +0200 [thread overview]
Message-ID: <8595563.AsjhMEDtd8@ada> (raw)
In-Reply-To: <20220531083236.18968-1-pali@kernel.org>
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ár:
> Fix multiple issues in ubifs distroboot code:
>
> 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.
From grepping through u-boot source, I can not quite follow. Where is that
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.
>
> 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 allows
> to detach UBI device from MTD device.
Okay.
> Move definition of MTD device with UBI and UBI volume with ubifs filesystem
> 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 variables
> for compatibility with existing distroboot scripts.
This might be problematic. For a downstream board we use the 'bootubivol' env
variable to switch between different volumes for update purposes. Means on
firmware update we modify U-Boot environment from Linux (fw_setenv) after
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.
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
> 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(-)
>
> 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=" \
> - "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=ubi; " \
> + "devnum=ubi0; " \
> + "bootfstype=ubifs; " \
> + "distro_bootpart=${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 "=" \
> + "bootubipart=" #bootubipart "; " \
> + "bootubivol=" #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
>
> -#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=" BOOT_TARGET_DEVICES(BOOTENV_DEV_NAME) "\0"
>
> -#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 100644
> --- a/include/configs/am335x_guardian.h
> +++ b/include/configs/am335x_guardian.h
> @@ -29,7 +29,7 @@
> "ramdisk_addr_r=0x88080000\0" \
>
> #define BOOT_TARGET_DEVICES(func) \
> - func(UBIFS, ubifs, 0)
> + func(UBIFS, ubifs, 0, UBI, rootfs)
>
> #define AM335XX_BOARD_FDTFILE "fdtfile=" CONFIG_DEFAULT_DEVICE_TREE
> ".dtb\0"
>
> @@ -54,7 +54,6 @@
> GUARDIAN_DEFAULT_PROD_ENV \
> "autoload=no\0" \
> "backlight_brightness=50\0" \
> - "bootubivol=rootfs\0" \
> "distro_bootcmd=" \
> "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 100644
> --- a/include/configs/colibri-imx6ull.h
> +++ b/include/configs/colibri-imx6ull.h
> @@ -91,7 +91,6 @@
> UBI_BOOTCMD \
> UBOOT_UPDATE \
> "boot_script_dhcp=boot.scr\0" \
> - "bootubipart=ubi\0" \
> "console=ttymxc0\0" \
> "defargs=user_debug=30\0" \
> "fdt_board=eval-v3\0" \
> diff --git a/include/configs/colibri_imx7.h b/include/configs/colibri_imx7.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=zImage\0" \
> "boot_script_dhcp=boot.scr\0" \
> - "bootubipart=ubi\0" \
> "console=ttymxc0\0" \
> "defargs=\0" \
> "fdt_board=eval-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 @@
>
> #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)
>
> 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 @@
>
> #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)
>
> diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.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)
>
> #else /* !CONFIG_MTD_RAW_NAND */
> @@ -87,8 +87,6 @@
> "bootenv=uEnv.txt\0" \
> "bootfile=zImage\0" \
> "bootpart=0:2\0" \
> - "bootubivol=rootfs\0" \
> - "bootubipart=rootfs\0" \
> "usbtty=cdc_acm\0" \
> "mpurate=auto\0" \
> "buddy=none\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)
>
> #else /* !CONFIG_MTD_RAW_NAND */
> @@ -88,8 +88,6 @@
> "bootenv=uEnv.txt\0" \
> "bootfile=zImage\0" \
> "bootpart=0:2\0" \
> - "bootubivol=rootfs\0" \
> - "bootubipart=rootfs\0" \
> "optargs=\0" \
> "nandroot=ubi0:rootfs ubi.mtd=rootfs rw noinitrd\0" \
> "nandrootfstype=ubifs 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 @@
>
> #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)
>
> 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
>
> #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
>
> #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
next prev parent reply other threads:[~2022-06-29 13:37 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-31 8:32 [PATCH] distroboot: Fix ubifs Pali Rohár
2022-06-23 16:09 ` Pali Rohár
2022-06-29 10:29 ` Frieder Schrempf
2022-07-05 10:15 ` Pali Rohár
2022-06-29 13:36 ` Alexander Dahl [this message]
2022-06-29 13:55 ` Pali Rohár
2022-06-30 6:46 ` Alexander Dahl
2022-07-04 7:55 ` Pali Rohár
2022-07-05 7:32 ` Alexander Dahl
2022-07-05 10:13 ` Pali Rohár
2022-07-08 16:38 ` Tom Rini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8595563.AsjhMEDtd8@ada \
--to=ada@thorsis.com \
--cc=Gireesh.Hiremath@in.bosch.com \
--cc=Govindaraji.Sivanantham@in.bosch.com \
--cc=frieder.schrempf@kontron.de \
--cc=marcel.ziswiler@toradex.com \
--cc=martyn.welch@collabora.com \
--cc=navin@linumiz.com \
--cc=pali@kernel.org \
--cc=parthiban@linumiz.com \
--cc=patrice.chotard@foss.st.com \
--cc=patrick.delaunay@foss.st.com \
--cc=sjg@chromium.org \
--cc=sjoerd.simons@collabora.co.uk \
--cc=u-boot@lists.denx.de \
--cc=woods.technical@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox