On 7/15/23 14:47, Mark Kettenis wrote:
Date: Fri, 14 Jul 2023 23:27:42 +0200 From: Marek Vasut marex@denx.de
On 7/14/23 22:43, Mark Kettenis wrote:
Find the appropriate EFI system partition on the internal NVMe storage and set the U-Boot environment variables such that the file system firmware loader can load firmware from it.
Signed-off-by: Mark Kettenis kettenis@openbsd.org
arch/arm/mach-apple/board.c | 60 +++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+)
diff --git a/arch/arm/mach-apple/board.c b/arch/arm/mach-apple/board.c index d501948118..7799a0f916 100644 --- a/arch/arm/mach-apple/board.c +++ b/arch/arm/mach-apple/board.c @@ -8,6 +8,8 @@ #include <dm/uclass-internal.h> #include <efi_loader.h> #include <lmb.h> +#include <nvme.h> +#include <part.h>
#include <asm/armv8/mmu.h> #include <asm/global_data.h> @@ -539,6 +541,60 @@ u64 get_page_table_size(void) return SZ_256K; }
+static char *asahi_esp_devpart(void) +{
- struct disk_partition info;
- struct blk_desc *nvme_blk;
- const char *uuid = NULL;
- int devnum, len, p, part, ret;
- static char devpart[64];
- struct udevice *dev;
- ofnode node;
- if (devpart[0])
return devpart;- node = ofnode_path("/chosen");
- if (ofnode_valid(node)) {
uuid = ofnode_get_property(node, "asahi,efi-system-partition",&len);- }
- nvme_scan_namespace();
- for (devnum = 0, part = 0;; devnum++) {
nvme_blk = blk_get_devnum_by_uclass_id(UCLASS_NVME, devnum);if (nvme_blk == NULL)break;dev = dev_get_parent(nvme_blk->bdev);if (!device_is_compatible(dev, "apple,nvme-ans2"))Can we somehow use ofnode_for_each_compatible_node() here ? That might simplify this code.
I don't really see how that would simplify things. I'm iterating over all NVMe devices here and then checking the compatible of the parent to make sure I pick the on-board one. I could do the inverse and lookup the node first and then use that to find the NVMe block device, but it will still involve a loop and several function calls.
What about:
" struct blk_desc *desc;
ofnode_for_each_compatible_node(node, "apple,nvme-ans2") { uclass_get_device_by_ofnode(UCLASS_NVME, node, &blk_dev); desc = dev_get_uclass_plat(blk_dev); for (count = 0, part = 1; part <= MAX_SEARCH_PARTITIONS; part++) { part_get_info(desc, part, &info); ... } } "
I'm _not_ sure anymore whether this is actually easier though. What do you think ?
continue;for (p = 1; p <= MAX_SEARCH_PARTITIONS; p++) {ret = part_get_info(nvme_blk, p, &info);if (ret < 0)break;if (info.bootable & PART_EFI_SYSTEM_PARTITION) {if (uuid && strcasecmp(uuid, info.uuid) == 0) {part = p;break;}if (part == 0)part = p;}}if (part > 0)break;- }
- if (part > 0)
snprintf(devpart, sizeof(devpart), "%d:%d", devnum, part);- return devpart;
+}
#define KERNEL_COMP_SIZE SZ_128M
int board_late_init(void)
@@ -546,6 +602,10 @@ int board_late_init(void) struct lmb lmb; u32 status = 0;
- status |= env_set("storage_interface",
blk_get_uclass_name(UCLASS_NVME));- status |= env_set("fw_dev_part", asahi_esp_devpart());
I think env_set() returns integer (and this could be negative too), so you might want to check the return value instead of casting it to unsigned integer.
I'm just using the existing idiom. But maybe I should just check the return value and throw a warning instead? Not having the firmware loader available isn't fatal. It just means some of the USB ports won't work.
That's better, yeah. I don't think you can safely do bitwise operations on signed types.