On Fri, Apr 15, 2022 at 10:21:39AM +0200, Heinrich Schuchardt wrote:
On 4/15/22 09:11, AKASHI Takahiro wrote:
We see some increase of spl code size due to partition support (disk/*) while none of particular partition types (CONFIG_SPL_XXX_PARTITION) are enabled. With this patch applied, part.c is no longer included unless really necessary.
In addition, fix errors in CI build revealed after this change is made.
Fixes: commit 88ca8e26958b ("disk: Add an option for partitions in SPL") Fixes: commit 17f8cda505e3 ("efi_loader: set partition GUID in device path for SIG_TYPE_GUID") Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
cmd/Kconfig | 1 + configs/cortina_presidio-asic-emmc_defconfig | 1 - disk/Kconfig | 37 ++++++++++---------- fs/fat/fat.c | 3 +- include/part.h | 14 ++++++-- include/sandboxblockdev.h | 2 ++ lib/efi_loader/Kconfig | 1 + 7 files changed, 37 insertions(+), 22 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig index d3abe3a06bff..b69c26912568 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1239,6 +1239,7 @@ config CMD_OSD
config CMD_PART bool "part"
- depends on PARTITIONS select HAVE_BLOCK_DEVICE select PARTITION_UUIDS help
diff --git a/configs/cortina_presidio-asic-emmc_defconfig b/configs/cortina_presidio-asic-emmc_defconfig index c22dcef7ec05..c217a00a1cf0 100644 --- a/configs/cortina_presidio-asic-emmc_defconfig +++ b/configs/cortina_presidio-asic-emmc_defconfig @@ -18,7 +18,6 @@ CONFIG_LAST_STAGE_INIT=y CONFIG_SYS_PROMPT="G3#" CONFIG_CMD_I2C=y CONFIG_CMD_MMC=y -CONFIG_CMD_PART=y CONFIG_CMD_WDT=y CONFIG_BOOTP_BOOTFILESIZE=y CONFIG_CMD_CACHE=y
This change seems to be unrelated to the commit message.
I removed this option because the build failed in Azure CI due to lack of CONFIG_PARTITIONS. What is strange in this defconfig is that, while none of any partition table types are enabled, but CMD_PART is enabled, which simply makes no sense.
But I will revert this change since I was able to build "part" command even without CONFIG_PARTITIONS thanks to the change I made at include/part.h.
Why would you remove the command on a board that supports EXT2 and EXT4 as well as partitions?
diff --git a/disk/Kconfig b/disk/Kconfig index 13700322e976..359af3b27e6d 100644 --- a/disk/Kconfig +++ b/disk/Kconfig @@ -2,8 +2,7 @@ menu "Partition Types"
config PARTITIONS
- bool "Enable Partition Labels (disklabels) support"
- default y
- bool help Partition Labels (disklabels) Supported: Zero or more of the following:
@@ -20,8 +19,7 @@ config PARTITIONS as well.
config SPL_PARTITIONS
- bool "Enable Partition Labels (disklabels) support in SPL"
- default y if PARTITIONS
- bool select SPL_SPRINTF select SPL_STRTO help
@@ -30,8 +28,7 @@ config SPL_PARTITIONS small amount of size to SPL, typically 500 bytes.
config TPL_PARTITIONS
- bool "Enable Partition Labels (disklabels) support in TPL"
- default y if PARTITIONS
- bool select TPL_SPRINTF select TPL_STRTO help
@@ -41,57 +38,61 @@ config TPL_PARTITIONS
config MAC_PARTITION bool "Enable Apple's MacOS partition table"
- depends on PARTITIONS
select PARTITIONS help Say Y here if you would like to use device under U-Boot which were partitioned on a Macintosh.
config SPL_MAC_PARTITION bool "Enable Apple's MacOS partition table for SPL"
- depends on SPL && PARTITIONS
depends on SPL default y if MAC_PARTITION
select SPL_PARTITIONS
config DOS_PARTITION bool "Enable MS Dos partition table"
- depends on PARTITIONS default y if DISTRO_DEFAULTS default y if x86 || CMD_FAT || USB_STORAGE
select PARTITIONS help traditional on the Intel architecture, USB sticks, etc.
config SPL_DOS_PARTITION bool "Enable MS Dos partition table for SPL"
- depends on SPL && PARTITIONS
depends on SPL default n if ARCH_SUNXI default y if DOS_PARTITION
select SPL_PARTITIONS
config ISO_PARTITION bool "Enable ISO partition table"
- depends on PARTITIONS default y if DISTRO_DEFAULTS default y if MIPS || ARCH_TEGRA
select PARTITIONS
config SPL_ISO_PARTITION bool "Enable ISO partition table for SPL"
- depends on SPL && PARTITIONS
depends on SPL
select SPL_PARTITIONS
config AMIGA_PARTITION bool "Enable AMIGA partition table"
- depends on PARTITIONS
select PARTITIONS help Say Y here if you would like to use device under U-Boot which were partitioned under AmigaOS.
config SPL_AMIGA_PARTITION bool "Enable AMIGA partition table for SPL"
- depends on SPL && PARTITIONS
depends on SPL default y if AMIGA_PARTITION
select SPL_PARTITIONS
config EFI_PARTITION bool "Enable EFI GPT partition table"
- depends on PARTITIONS default y if DISTRO_DEFAULTS default y if ARCH_TEGRA
- select PARTITIONS select LIB_UUID help Say Y here if you would like to use device under U-Boot which
@@ -128,9 +129,10 @@ config EFI_PARTITION_ENTRIES_OFF
config SPL_EFI_PARTITION bool "Enable EFI GPT partition table for SPL"
- depends on SPL && PARTITIONS
depends on SPL default n if ARCH_SUNXI default y if EFI_PARTITION
select SPL_PARTITIONS
config PARTITION_UUIDS bool "Enable support of UUID for partition"
@@ -143,12 +145,11 @@ config PARTITION_UUIDS
config SPL_PARTITION_UUIDS bool "Enable support of UUID for partition in SPL"
- depends on SPL && PARTITIONS
depends on SPL_PARTITIONS default y if SPL_EFI_PARTITION
config PARTITION_TYPE_GUID bool "Enable support of GUID for partition type"
- depends on PARTITIONS depends on EFI_PARTITION help Activate the configuration of GUID type
diff --git a/fs/fat/fat.c b/fs/fat/fat.c index df9ea2c028fc..a7ec1c4b759c 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -95,7 +95,8 @@ int fat_register_device(struct blk_desc *dev_desc, int part_no) cur_dev = NULL;
/* Read the partition table, if present */
- if (part_get_info(dev_desc, part_no, &info)) {
- if (CONFIG_IS_ENABLED(DOS_PARTITION) &&
part_get_info(dev_desc, part_no, &info)) {diff --git a/include/part.h b/include/part.h index 36b76f00563f..612d4c32b5c7 100644 --- a/include/part.h +++ b/include/part.h @@ -10,6 +10,7 @@ #include <ide.h> #include <uuid.h> #include <linker_lists.h> +#include <linux/errno.h> #include <linux/list.h>
struct block_drvr { @@ -86,7 +87,7 @@ struct disk_part { };
/* Misc _get_dev functions */ -#ifdef CONFIG_PARTITIONS +#if CONFIG_IS_ENABLED(PARTITIONS) /**
- blk_get_dev() - get a pointer to a block device given its type and number
@@ -275,6 +276,15 @@ static inline int blk_get_device_part_str(const char *ifname, struct disk_partition *info, int allow_whole_dev) { *dev_desc = NULL; return -1; } +static inline int part_get_info_by_name_type(struct blk_desc *dev_desc,
const char *name,struct disk_partition *info,int part_type)+{ return -ENOENT; } +static inline int part_get_info_by_name(struct blk_desc *dev_desc,
const char *name,struct disk_partition *info)+{ return -ENOENT; } static inline int part_get_info_by_dev_and_name_or_num(const char *dev_iface, const char *dev_part_str, @@ -513,7 +523,7 @@ int layout_mbr_partitions(struct disk_partition *p, int count,
#endif
-#ifdef CONFIG_PARTITIONS +#if CONFIG_IS_ENABLED(PARTITIONS) /**
- part_driver_get_count() - get partition driver count
diff --git a/include/sandboxblockdev.h b/include/sandboxblockdev.h index 4ca9554e38a0..dc983f0417b2 100644 --- a/include/sandboxblockdev.h +++ b/include/sandboxblockdev.h @@ -26,4 +26,6 @@ struct host_block_dev { */ int host_dev_bind(int dev, char *filename, bool removable);
+int host_get_dev_err(int dev, struct blk_desc **blk_devp);
- #endif
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index 7dc24fbf0aa9..b615abf598b9 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -17,6 +17,7 @@ config EFI_LOADER select DM_EVENT select EVENT_DYNAMIC select LIB_UUID
- select DOS_PARTITION
EFI_LOADER should imply EFI_PARTITION. MBR support is not required by the UEFI specification. You may still want to imply DOS_PARTITION.
As disk/Kconfig already has 'default y if' for both I would prefer to add EFI_LOADER there.
Basically I agree with you, but as Tom mentioned in his reply, "imply (or select) [EFI|DOS]_PARTITION" may increase the size of U-Boot proper on some platforms.
The reason why I added "select DOS_PARTITION" is the next line, "select PARTITION_UUIDS". While PARTITION_UUIDS depends on PARTITION, EFI_LOADER *selects* PARTITION_UUIDS unconditionally, which beaks dependency as [EFI|DOS]_PARTITION, as you said above, is not a requirement for EFI_LOADER from a perspective of UEFI specification.
Instead of adding any "select *_PARTITION", I will fix this issue simply by changing the dependency to "imply PARTITION_UUIDS" with a small hack on efi_device_path.c.
diff --git a/disk/Kconfig b/disk/Kconfig index 13700322e9..87b2e52af4 100644 --- a/disk/Kconfig +++ b/disk/Kconfig @@ -54,7 +54,7 @@ config SPL_MAC_PARTITION config DOS_PARTITION bool "Enable MS Dos partition table" depends on PARTITIONS
default y if DISTRO_DEFAULTS
default y if DISTRO_DEFAULTS || EFI_LOADER default y if x86 || CMD_FAT || USB_STORAGE help traditional on the Intel architecture, USB sticks, etc.@@ -90,7 +90,7 @@ config SPL_AMIGA_PARTITION config EFI_PARTITION bool "Enable EFI GPT partition table" depends on PARTITIONS
default y if DISTRO_DEFAULTS
default y if DISTRO_DEFAULTS || EFI_LOADER default y if ARCH_TEGRA select LIB_UUID help
If we want to merge this hack, we will have to negotiate with Tom.
-Takahiro Akashi
Best regards
Heinrich
select PARTITION_UUIDS select HAVE_BLOCK_DEVICE select REGEX