On Wed, Jul 19, 2023 at 06:43:08AM +0200, Heinrich Schuchardt wrote: > On devices with multiple USB mass storage devices errors like > > Path /../USB(0x0,0x0)/USB(0x1,0x0)/Ctrl(0x0) > already installed. > > are seen. This is due to creating non-unique device paths. To uniquely > identify devices we must provide path nodes for all devices on the path > from the root device. > > Add support for generating device path nodes for all uclasses.
I think that this is the last resort for devices that cannot be classified/ identified by any other means. If possible, should we provide a more concrete device path? I have submitted a patch specifically for USB devices in the past: https://lists.denx.de/pipermail/u-boot/2021-November/468216.html # I'm no longer working on this patch, though. -Takahiro Akashi > Reported-by: Suniel Mahesh <su...@amarulasolutions.com> > Signed-off-by: Heinrich Schuchardt <heinrich.schucha...@canonical.com> > --- > include/efi_api.h | 7 ++++ > lib/efi_loader/efi_device_path.c | 56 ++++++++++++++------------------ > 2 files changed, 32 insertions(+), 31 deletions(-) > > diff --git a/include/efi_api.h b/include/efi_api.h > index 55a4c989fc..8f5ef5f680 100644 > --- a/include/efi_api.h > +++ b/include/efi_api.h > @@ -579,6 +579,13 @@ struct efi_device_path_vendor { > u8 vendor_data[]; > } __packed; > > +struct efi_device_path_udevice { > + struct efi_device_path dp; > + efi_guid_t guid; > + int uclass_id; > + int dev_number; > +} __packed; > + > struct efi_device_path_controller { > struct efi_device_path dp; > u32 controller_number; > diff --git a/lib/efi_loader/efi_device_path.c > b/lib/efi_loader/efi_device_path.c > index 04ebb449ca..d0be869d94 100644 > --- a/lib/efi_loader/efi_device_path.c > +++ b/lib/efi_loader/efi_device_path.c > @@ -10,6 +10,7 @@ > #include <common.h> > #include <blk.h> > #include <dm.h> > +#include <dm/root.h> > #include <log.h> > #include <net.h> > #include <usb.h> > @@ -38,16 +39,6 @@ const struct efi_device_path END = { > .length = sizeof(END), > }; > > -/* template ROOT node: */ > -static const struct efi_device_path_vendor ROOT = { > - .dp = { > - .type = DEVICE_PATH_TYPE_HARDWARE_DEVICE, > - .sub_type = DEVICE_PATH_SUB_TYPE_VENDOR, > - .length = sizeof(ROOT), > - }, > - .guid = U_BOOT_GUID, > -}; > - > #if defined(CONFIG_MMC) > /* > * Determine if an MMC device is an SD card. > @@ -497,13 +488,12 @@ bool efi_dp_is_multi_instance(const struct > efi_device_path *dp) > __maybe_unused static unsigned int dp_size(struct udevice *dev) > { > if (!dev || !dev->driver) > - return sizeof(ROOT); > + return sizeof(struct efi_device_path_udevice); > > switch (device_get_uclass_id(dev)) { > case UCLASS_ROOT: > - case UCLASS_SIMPLE_BUS: > /* stop traversing parents at this point: */ > - return sizeof(ROOT); > + return sizeof(struct efi_device_path_udevice); > case UCLASS_ETH: > return dp_size(dev->parent) + > sizeof(struct efi_device_path_mac_addr); > @@ -582,8 +572,8 @@ __maybe_unused static unsigned int dp_size(struct udevice > *dev) > return dp_size(dev->parent) + > sizeof(struct efi_device_path_usb); > default: > - /* just skip over unknown classes: */ > - return dp_size(dev->parent); > + return dp_size(dev->parent) + > + sizeof(struct efi_device_path_udevice); > } > } > > @@ -600,13 +590,6 @@ __maybe_unused static void *dp_fill(void *buf, struct > udevice *dev) > return buf; > > switch (device_get_uclass_id(dev)) { > - case UCLASS_ROOT: > - case UCLASS_SIMPLE_BUS: { > - /* stop traversing parents at this point: */ > - struct efi_device_path_vendor *vdp = buf; > - *vdp = ROOT; > - return &vdp[1]; > - } > #ifdef CONFIG_NETDEVICES > case UCLASS_ETH: { > struct efi_device_path_mac_addr *dp = > @@ -811,11 +794,24 @@ __maybe_unused static void *dp_fill(void *buf, struct > udevice *dev) > > return &udp[1]; > } > - default: > - /* If the uclass driver is missing, this will show NULL */ > - log_debug("unhandled device class: %s (%s)\n", dev->name, > - dev_get_uclass_name(dev)); > - return dp_fill(buf, dev->parent); > + default: { > + struct efi_device_path_udevice *vdp; > + enum uclass_id uclass_id = device_get_uclass_id(dev); > + > + if (uclass_id == UCLASS_ROOT) > + vdp = buf; > + else > + vdp = dp_fill(buf, dev->parent); > + > + vdp->dp.type = DEVICE_PATH_TYPE_HARDWARE_DEVICE; > + vdp->dp.sub_type = DEVICE_PATH_SUB_TYPE_VENDOR; > + vdp->dp.length = sizeof(*vdp); > + memcpy(&vdp->guid, &efi_u_boot_guid, sizeof(efi_guid_t)); > + vdp->uclass_id = uclass_id; > + vdp->dev_number = dev->seq_; > + > + return &vdp[1]; > + } > } > } > > @@ -1052,14 +1048,12 @@ struct efi_device_path *efi_dp_from_uart(void) > { > void *buf, *pos; > struct efi_device_path_uart *uart; > - size_t dpsize = sizeof(ROOT) + sizeof(*uart) + sizeof(END); > + size_t dpsize = dp_size(dm_root()) + sizeof(*uart) + sizeof(END); > > buf = efi_alloc(dpsize); > if (!buf) > return NULL; > - pos = buf; > - memcpy(pos, &ROOT, sizeof(ROOT)); > - pos += sizeof(ROOT); > + pos = dp_fill(buf, dm_root()); > uart = pos; > uart->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE; > uart->dp.sub_type = DEVICE_PATH_SUB_TYPE_MSG_UART; > -- > 2.40.1 >