On 2/17/23 03:55, Simon Glass wrote:
" properHi Heinrich,
On Thu, 16 Feb 2023 at 14:31, Heinrich Schuchardt
<[email protected]> wrote:
On 2/16/23 21:17, Simon Glass wrote:
Hi Heinrich,
On Thu, 16 Feb 2023 at 08:30, Heinrich Schuchardt
<[email protected]> wrote:
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 <[email protected]>
---
v2:
avoid if/endif in Kconfig
---
common/spl/Kconfig | 27 ++++++++++++++++++++++-----
common/spl/spl_mmc.c | 13 +++++++++++++
2 files changed, 35 insertions(+), 5 deletions(-)
diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index 3c2af453ab..9d12b48297 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: GPT by 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
What is this? Can we have a register of these hideous things and call
them by name?
Further, I don't see any documentation on this in U-Boot. Could you at
least add a list of these things?
+ 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..69bf1d6e98 100644
--- a/common/spl/spl_mmc.c
+++ b/common/spl/spl_mmc.c
@@ -191,6 +191,19 @@ static int mmc_load_image_raw_partition(struct
spl_image_info *spl_image,
struct disk_partition info;
int err;
+#ifdef CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_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(info.type_guid,
+
CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_GPT_PARTITION_TYPE,
+ UUID_STR_LEN)) {
+ partition = i;
+ break;
+ }
+ }
+#endif
#ifdef CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION_TYPE
int type_part;
/* Only support MBR so DOS_ENTRY_NUMBERS */
--
2.38.1
Is it possible to avoid using #ifdef here?
Unfortunately not. Field 'type_guid' is restricted by an #ifdef. So
unconditional compilation would fail.
Do you think it is worth adding an accessor as we have done with some
global_data things?
There are other places like disk/part_efi.c where we could trade #ifdef
for if with such an accessor. The accessor itself would require an #ifdef.
We should be careful about the value returned for
CONFIG_PARTITION_TYPE_GUID=n. Returning NULL would probably lead to
warnings from GCC and Coverity. We would better return a dummy string
like "00000000-0000-0000-0000-000000000000".
Longer term, I wonder if we can add a DT schema for all of
this...these CONFIG options for boot selection seem to be getting out
of hand!
Tom just moved a lot of constants hard coded in C code to Kconfig with a
big effort. Now you want to move Kconfig values to hard coded constants
in device-tree.
Running in circles does not sound like a winning strategy.
The values seem to be common across certain boards of the same type, SoC, etc.
As I described above using the same GUID value for different boards only
makes sense if they are supported by the very same U-Boot binary.
On boards with the same SoC (even from different vendors) I would very
much prefer a single U-Boot SPL selecting a board specific device-tree
to having multiple binaries. We should push developers into this direction.
I'm not sure, but at some point this is all going to get out of hand.
Already we have these options:
common/spl/Kconfig:config SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR
common/spl/Kconfig:config SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR
common/spl/Kconfig:config SYS_MMCSD_RAW_MODE_U_BOOT_DATA_PART_OFFSET
common/spl/Kconfig:config SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION
common/spl/Kconfig:config SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION
common/spl/Kconfig:config SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION_TYPE
common/spl/Kconfig:config SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION_TYPE
common/spl/Kconfig:config SYS_MMCSD_RAW_MODE_EMMC_BOOT_PARTITION
common/spl/Kconfig:config SYS_MMCSD_FS_BOOT_PARTITION
common/spl/Kconfig:config SYS_MMCSD_RAW_MODE_KERNEL_SECTOR
common/spl/Kconfig:config SYS_MMCSD_RAW_MODE_ARGS_SECTOR
common/spl/Kconfig:config SYS_MMCSD_RAW_MODE_ARGS_SECTORS
That is just for MMC raw mode.
For environment we have SYS_MMC_ENV_DEV and _PART. If you look around
you'll see loads of these options.
I see that rockchip uses u-boot,spl-boot-order as a way to determine
the boot order. This makes it configurable without rebuilding
U-Boot...although I don't think we need to make the MMC stuff
configurable, since I am assuming that the boot ROM determines at
least some of it...?
This patch is about SPL loading main U-Boot. So the boot ROM is not
involved.
It seems that the whole thing is crying out for a bit of organisation
and a proper schema.
The discussion was about hard-coding the values vs configuration.
OS distributions should have enough flexibility to deliver an
installation image with U-Boot for multiple boards on the same medium.
For the build process it is preferable to use different configurations
instead of patching source code per U-Boot which might be required if
hard-coded values for partition GUIDs in the device-trees are used.
I think Tom's approach is right. The U-Boot documentation should give
guidance on how new boards should find U-Boot SPL and main U-Boot.
Best regards
Heinrich
