On 7/15/23 14:47, Mark Kettenis wrote:
Date: Fri, 14 Jul 2023 23:27:42 +0200
From: Marek Vasut <ma...@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 <kette...@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.
