[PATCH v3 0/3] spl: allow loading via partition type GUID
Some boards provide main U-Boot as a dedicated partition to SPL. Currently we can define either a fixed partition number or an MBR partition type to define which partition is to be used.
Partition numbers tend to conflict with established partitioning schemes of Linux distros. MBR partitioning is more and more replaced by GPT partitioning.
Allow defining a partition type GUID identifying the partition to load main U-Boot from.
To avoid using #ifdef in the implementation:
* introduce accessors for fields of struct disk_partition * provide a macro IF_ENABLED
v3: avoid usage of #ifdef v2: avoid if/endif in Kconfig
The necessity of the Kconfig setting was discussed in https://lore.kernel.org/u-boot/20230216152956.130038-1-heinrich.schuchardt@c...
Heinrich Schuchardt (3): disk: accessors for conditional partition fields kconfig: new macro IF_ENABLED() spl: allow loading via partition type GUID
common/spl/Kconfig | 27 ++++++++++++++++++++++----- common/spl/spl_mmc.c | 18 ++++++++++++++++++ include/linux/kconfig.h | 7 +++++++ include/part.h | 36 ++++++++++++++++++++++++++++++++++++ 4 files changed, 83 insertions(+), 5 deletions(-)
Structure disk_partition contains some fields whose existence depends on configuration variables. Provide macros that return a value irrespective of the value of configuration. This allows to replace #ifdefs by simple ifs in codes that relies on these fields which is our preferred coding style.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- v3: new patch --- include/part.h | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+)
diff --git a/include/part.h b/include/part.h index be75c73549..c0355a3602 100644 --- a/include/part.h +++ b/include/part.h @@ -80,6 +80,42 @@ struct disk_partition { #endif };
+/** + * get_part_uuid() - get partition UUID + * + * @a: disk partition as struct disk_partition + * Return: partition UUID as string + */ +#if CONFIG_IS_ENABLED(PARTITION_UUIDS) +#define get_part_uuid(a) (a.uuid) +#else +#define get_part_uuid(a) ("") +#endif + +/** + * get_part_type_guid() - get partition type GUID + * + * @a: disk partition as struct disk_partition + * Return: partition type GUID as string + */ +#ifdef CONFIG_PARTITION_TYPE_GUID +#define get_part_type_guid(a) (a.type_guid) +#else +#define get_part_type_guid(a) ("") +#endif + +/** + * get_part_type() - get partition type + * + * @a: disk partition as struct disk_partition + * Return: partition type a unsigned char + */ +#ifdef CONFIG_DOS_PARTITION +#define get_part_type(a) (a.sys_ind) +#else +#define get_part_type(a) ('\0') +#endif + struct disk_part { int partnum; struct disk_partition gpt_part_info;
On Sun, 19 Feb 2023 at 04:36, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
Structure disk_partition contains some fields whose existence depends on configuration variables. Provide macros that return a value irrespective of the value of configuration. This allows to replace #ifdefs by simple ifs in codes that relies on these fields which is our preferred coding style.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
v3: new patch
include/part.h | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
But I do think static inlines are better
We want to move from using #ifdef to using if in our code. A lot of our code using #ifdef is structured like:
#ifdef CONFIG_FOO fun(CONFIG_FOO_OPT); #endif
In Kconfig you will find
config FOO bool "enable foo"
config FOO_OPT string "value for foo" depends on FOO
We cannot use CONFIG_FOO_OPT in our code directly if CONFIG_FOO is not defined.
if (IS_ENABLED(CONFIG_FOO) fun(CONFIG_FOO_OPT);
This would result in an error due to the undefined symbol CONFIG_FOO_OPT.
The new macro IF_ENABLED(CONFIG_FOO, opt_cfg, def_val) which evaluates to opt_cfg if CONFIG_FOO is set to 'y' and to def_val otherwise comes to the rescue:
if (IS_ENABLED(CONFIG_FOO) fun(IF_ENABLED(CONFIG_FOO, CONFIG_FOO_OPT, "");
If CONFIG_FOO is not defined, the compiler will see
if (0) fun("");
and be happy.
If CONFIG_FOO is defined, the compiler will see
if (1) fun(CONFIG_FOO_OPT)
and be equally happy.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- v3: new patch --- include/linux/kconfig.h | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h index 2bc704e110..7fea72da1a 100644 --- a/include/linux/kconfig.h +++ b/include/linux/kconfig.h @@ -28,6 +28,13 @@ */ #define IS_ENABLED(option) config_enabled(option, 0)
+/* + * IF_ENABLED(CONFIG_FOO, opt_cfg, def_val) evalutes to opt_cfg if + * CONFIG_FOO is set to 'y' and to def_val otherwise. + */ +#define IF_ENABLED(option, opt_cfg, def_val) \ + config_opt_enabled(IS_ENABLED(option), opt_cfg, def_val) + /* * U-Boot add-on: Helper macros to reference to different macros (prefixed by * CONFIG_, CONFIG_SPL_, CONFIG_TPL_ or CONFIG_TOOLS_), depending on the build
On Sun, 19 Feb 2023 12:36:28 +0100 Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
Hi,
We want to move from using #ifdef to using if in our code. A lot of our code using #ifdef is structured like:
#ifdef CONFIG_FOO fun(CONFIG_FOO_OPT); #endifIn Kconfig you will find
config FOO bool "enable foo" config FOO_OPT string "value for foo" depends on FOOWe cannot use CONFIG_FOO_OPT in our code directly if CONFIG_FOO is not defined.
if (IS_ENABLED(CONFIG_FOO) fun(CONFIG_FOO_OPT);This would result in an error due to the undefined symbol CONFIG_FOO_OPT.
The new macro IF_ENABLED(CONFIG_FOO, opt_cfg, def_val) which evaluates to opt_cfg if CONFIG_FOO is set to 'y' and to def_val otherwise comes to the rescue:
if (IS_ENABLED(CONFIG_FOO) fun(IF_ENABLED(CONFIG_FOO, CONFIG_FOO_OPT, "");
Ah, yeah, I like this one, this is indeed the most common pattern that prevents many "#if to if" conversions. I briefly thought about unifying those two lines, but it's probably limiting, as it appears more flexible this way:
If CONFIG_FOO is not defined, the compiler will see
if (0) fun("");and be happy.
If CONFIG_FOO is defined, the compiler will see
if (1) fun(CONFIG_FOO_OPT)and be equally happy.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
Reviewed-by: Andre Przywara andre.przywara@arm.com
Cheers, Andre
v3: new patch
include/linux/kconfig.h | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h index 2bc704e110..7fea72da1a 100644 --- a/include/linux/kconfig.h +++ b/include/linux/kconfig.h @@ -28,6 +28,13 @@ */ #define IS_ENABLED(option) config_enabled(option, 0)
+/*
- IF_ENABLED(CONFIG_FOO, opt_cfg, def_val) evalutes to opt_cfg if
- CONFIG_FOO is set to 'y' and to def_val otherwise.
- */
+#define IF_ENABLED(option, opt_cfg, def_val) \
- config_opt_enabled(IS_ENABLED(option), opt_cfg, def_val)
/*
- U-Boot add-on: Helper macros to reference to different macros (prefixed by
- CONFIG_, CONFIG_SPL_, CONFIG_TPL_ or CONFIG_TOOLS_), depending on the build
Hi Heinrich,
On Mon, 20 Feb 2023 at 10:15, Andre Przywara andre.przywara@arm.com wrote:
On Sun, 19 Feb 2023 12:36:28 +0100 Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
Hi,
We want to move from using #ifdef to using if in our code. A lot of our code using #ifdef is structured like:
#ifdef CONFIG_FOO fun(CONFIG_FOO_OPT); #endifIn Kconfig you will find
config FOO bool "enable foo" config FOO_OPT string "value for foo" depends on FOOWe cannot use CONFIG_FOO_OPT in our code directly if CONFIG_FOO is not defined.
if (IS_ENABLED(CONFIG_FOO) fun(CONFIG_FOO_OPT);This would result in an error due to the undefined symbol CONFIG_FOO_OPT.
The new macro IF_ENABLED(CONFIG_FOO, opt_cfg, def_val) which evaluates to opt_cfg if CONFIG_FOO is set to 'y' and to def_val otherwise comes to the rescue:
if (IS_ENABLED(CONFIG_FOO) fun(IF_ENABLED(CONFIG_FOO, CONFIG_FOO_OPT, "");Ah, yeah, I like this one, this is indeed the most common pattern that prevents many "#if to if" conversions. I briefly thought about unifying those two lines, but it's probably limiting, as it appears more flexible this way:
If CONFIG_FOO is not defined, the compiler will see
if (0) fun("");and be happy.
If CONFIG_FOO is defined, the compiler will see
if (1) fun(CONFIG_FOO_OPT)and be equally happy.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
Reviewed-by: Andre Przywara andre.przywara@arm.com
Cheers, Andre
v3: new patch
include/linux/kconfig.h | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h index 2bc704e110..7fea72da1a 100644 --- a/include/linux/kconfig.h +++ b/include/linux/kconfig.h @@ -28,6 +28,13 @@ */ #define IS_ENABLED(option) config_enabled(option, 0)
+/*
- IF_ENABLED(CONFIG_FOO, opt_cfg, def_val) evalutes to opt_cfg if
- CONFIG_FOO is set to 'y' and to def_val otherwise.
- */
+#define IF_ENABLED(option, opt_cfg, def_val) \
config_opt_enabled(IS_ENABLED(option), opt_cfg, def_val)/*
- U-Boot add-on: Helper macros to reference to different macros (prefixed by
- CONFIG_, CONFIG_SPL_, CONFIG_TPL_ or CONFIG_TOOLS_), depending on the build
This looks good to me, but what should we do with the existing IF_ENABLED_INT()? Perhaps we should drop that in favour of this one, or do we need both?
Regards, Simon
Some boards provide main U-Boot as a dedicated partition to SPL. Currently we can define either a fixed partition number or an MBR partition type to define which partition is to be used.
Partition numbers tend to conflict with established partitioning schemes of Linux distros. MBR partitioning is more and more replaced by GPT partitioning.
Allow defining a partition type GUID identifying the partition to load main U-Boot from.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- v3: avoid usage of #ifdef v2: avoid if/endif in Kconfig --- common/spl/Kconfig | 27 ++++++++++++++++++++++----- common/spl/spl_mmc.c | 18 ++++++++++++++++++ 2 files changed, 40 insertions(+), 5 deletions(-)
diff --git a/common/spl/Kconfig b/common/spl/Kconfig index 3c2af453ab..9af8283cb5 100644 --- a/common/spl/Kconfig +++ b/common/spl/Kconfig @@ -514,19 +514,36 @@ config SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION used in raw mode
config SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION_TYPE - bool "MMC raw mode: by partition type" + bool "MMC raw mode: by MBR partition type" depends on DOS_PARTITION && SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION help - Use partition type for specifying U-Boot partition on MMC/SD in + Use MBR partition type for specifying U-Boot partition on MMC/SD in raw mode. U-Boot will be loaded from the first partition of this type to be found.
config SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION_TYPE - hex "Partition Type on the MMC to load U-Boot from" + hex "MBR Partition Type on the MMC to load U-Boot from" depends on SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION_TYPE help - Partition Type on the MMC to load U-Boot from, when the MMC is being - used in raw mode. + MBR Partition Type on the MMC to load U-Boot from, when the MMC is + being used in raw mode. + +config SYS_MMCSD_RAW_MODE_U_BOOT_USE_GPT_PARTITION_TYPE + bool "MMC raw mode: by GPT partition type" + depends on PARTITION_TYPE_GUID && SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION + help + Use GPT partition type for specifying U-Boot partition on MMC/SD in + raw mode. U-Boot will be loaded from the first partition of this + type to be found. + +config SYS_MMCSD_RAW_MODE_U_BOOT_GPT_PARTITION_TYPE + string "GPT Partition Type on the MMC to load U-Boot from" + depends on SYS_MMCSD_RAW_MODE_U_BOOT_USE_GPT_PARTITION_TYPE + default d2f002f8-e4e7-4269-b8ac-3bb6fabeaff6 + help + GPT Partition Type on the MMC to load U-Boot from, when the MMC is + being used in raw mode. The GUID must be lower case, low endian, + and formatted like d2f002f8-e4e7-4269-b8ac-3bb6fabeaff6.
config SUPPORT_EMMC_BOOT_OVERRIDE_PART_CONFIG bool "Override eMMC EXT_CSC_PART_CONFIG by user defined partition" diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c index e4135b2048..2417fa43e6 100644 --- a/common/spl/spl_mmc.c +++ b/common/spl/spl_mmc.c @@ -191,6 +191,24 @@ static int mmc_load_image_raw_partition(struct spl_image_info *spl_image, struct disk_partition info; int err;
+ if (IS_ENABLED(CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_GPT_PARTITION_TYPE)) { + const char *guid = + IF_ENABLED(CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_GPT_PARTITION_TYPE, + CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_GPT_PARTITION_TYPE, + ""); + + for (int i = 1; i <= MAX_SEARCH_PARTITIONS; ++i) { + err = part_get_info(mmc_get_blk_desc(mmc), i, &info); + if (err) + continue; + if (!strncmp(get_part_type_guid(info), guid, + UUID_STR_LEN)) { + partition = i; + break; + } + } + } + #ifdef CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION_TYPE int type_part; /* Only support MBR so DOS_ENTRY_NUMBERS */
participants (3)
-
Andre Przywara -
Heinrich Schuchardt -
Simon Glass