[PATCH 1/1] efi_loader: simplify dp_fill()
Move the recursive dp_fill(dev->parent) call to a single location. Determine uclass_id only once.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- lib/efi_loader/efi_device_path.c | 45 +++++++++++++------------------- 1 file changed, 18 insertions(+), 27 deletions(-)
diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index 5b050fa17c..ed7214f3a3 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -548,14 +548,19 @@ __maybe_unused static unsigned int dp_size(struct udevice *dev) */ __maybe_unused static void *dp_fill(void *buf, struct udevice *dev) { + enum uclass_id uclass_id; + if (!dev || !dev->driver) return buf;
- switch (device_get_uclass_id(dev)) { + uclass_id = device_get_uclass_id(dev); + if (uclass_id != UCLASS_ROOT) + buf = dp_fill(buf, dev->parent); + + switch (uclass_id) { #ifdef CONFIG_NETDEVICES case UCLASS_ETH: { - struct efi_device_path_mac_addr *dp = - dp_fill(buf, dev->parent); + struct efi_device_path_mac_addr *dp = buf; struct eth_pdata *pdata = dev_get_plat(dev);
dp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE; @@ -573,8 +578,7 @@ __maybe_unused static void *dp_fill(void *buf, struct udevice *dev) switch (device_get_uclass_id(dev->parent)) { #ifdef CONFIG_IDE case UCLASS_IDE: { - struct efi_device_path_atapi *dp = - dp_fill(buf, dev->parent); + struct efi_device_path_atapi *dp = buf; struct blk_desc *desc = dev_get_uclass_plat(dev);
dp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE; @@ -590,8 +594,7 @@ __maybe_unused static void *dp_fill(void *buf, struct udevice *dev) #endif #if defined(CONFIG_SCSI) case UCLASS_SCSI: { - struct efi_device_path_scsi *dp = - dp_fill(buf, dev->parent); + struct efi_device_path_scsi *dp = buf; struct blk_desc *desc = dev_get_uclass_plat(dev);
dp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE; @@ -604,8 +607,7 @@ __maybe_unused static void *dp_fill(void *buf, struct udevice *dev) #endif #if defined(CONFIG_MMC) case UCLASS_MMC: { - struct efi_device_path_sd_mmc_path *sddp = - dp_fill(buf, dev->parent); + struct efi_device_path_sd_mmc_path *sddp = buf; struct blk_desc *desc = dev_get_uclass_plat(dev);
sddp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE; @@ -619,8 +621,7 @@ __maybe_unused static void *dp_fill(void *buf, struct udevice *dev) #endif #if defined(CONFIG_AHCI) || defined(CONFIG_SATA) case UCLASS_AHCI: { - struct efi_device_path_sata *dp = - dp_fill(buf, dev->parent); + struct efi_device_path_sata *dp = buf; struct blk_desc *desc = dev_get_uclass_plat(dev);
dp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE; @@ -635,8 +636,7 @@ __maybe_unused static void *dp_fill(void *buf, struct udevice *dev) #endif #if defined(CONFIG_NVME) case UCLASS_NVME: { - struct efi_device_path_nvme *dp = - dp_fill(buf, dev->parent); + struct efi_device_path_nvme *dp = buf; u32 ns_id;
dp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE; @@ -650,8 +650,7 @@ __maybe_unused static void *dp_fill(void *buf, struct udevice *dev) #if defined(CONFIG_USB) case UCLASS_MASS_STORAGE: { struct blk_desc *desc = dev_get_uclass_plat(dev); - struct efi_device_path_controller *dp = - dp_fill(buf, dev->parent); + struct efi_device_path_controller *dp = buf;
dp->dp.type = DEVICE_PATH_TYPE_HARDWARE_DEVICE; dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_CONTROLLER; @@ -662,10 +661,9 @@ __maybe_unused static void *dp_fill(void *buf, struct udevice *dev) #endif default: { /* UCLASS_BLKMAP, UCLASS_HOST, UCLASS_VIRTIO */ - struct efi_device_path_udevice *dp; + struct efi_device_path_udevice *dp = buf; struct blk_desc *desc = dev_get_uclass_plat(dev);
- dp = dp_fill(buf, dev->parent); dp->dp.type = DEVICE_PATH_TYPE_HARDWARE_DEVICE; dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_VENDOR; dp->dp.length = sizeof(*dp); @@ -680,8 +678,7 @@ __maybe_unused static void *dp_fill(void *buf, struct udevice *dev) } #if defined(CONFIG_MMC) case UCLASS_MMC: { - struct efi_device_path_sd_mmc_path *sddp = - dp_fill(buf, dev->parent); + struct efi_device_path_sd_mmc_path *sddp = buf; struct mmc *mmc = mmc_get_mmc_dev(dev); struct blk_desc *desc = mmc_get_blk_desc(mmc);
@@ -697,7 +694,7 @@ __maybe_unused static void *dp_fill(void *buf, struct udevice *dev) #endif case UCLASS_MASS_STORAGE: case UCLASS_USB_HUB: { - struct efi_device_path_usb *udp = dp_fill(buf, dev->parent); + struct efi_device_path_usb *udp = buf;
switch (device_get_uclass_id(dev->parent)) { case UCLASS_USB_HUB: { @@ -717,13 +714,7 @@ __maybe_unused static void *dp_fill(void *buf, struct udevice *dev) return &udp[1]; } 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); + struct efi_device_path_udevice *vdp = buf;
vdp->dp.type = DEVICE_PATH_TYPE_HARDWARE_DEVICE; vdp->dp.sub_type = DEVICE_PATH_SUB_TYPE_VENDOR;
Hi Heinrich,
On Fri, 21 Jul 2023 at 00:34, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
Move the recursive dp_fill(dev->parent) call to a single location. Determine uclass_id only once.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
lib/efi_loader/efi_device_path.c | 45 +++++++++++++------------------- 1 file changed, 18 insertions(+), 27 deletions(-)
diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index 5b050fa17c..ed7214f3a3 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -548,14 +548,19 @@ __maybe_unused static unsigned int dp_size(struct udevice *dev) */ __maybe_unused static void *dp_fill(void *buf, struct udevice *dev) {
enum uclass_id uclass_id;if (!dev || !dev->driver) return buf;
switch (device_get_uclass_id(dev)) {
uclass_id = device_get_uclass_id(dev);if (uclass_id != UCLASS_ROOT)
Can we fix this one now? We should use EFI_MEDIA here I think?
buf = dp_fill(buf, dev->parent);switch (uclass_id) {#ifdef CONFIG_NETDEVICES case UCLASS_ETH: {
struct efi_device_path_mac_addr *dp =dp_fill(buf, dev->parent);
So how does the parent part get added? I am missing something here...or it was it never needed??
struct efi_device_path_mac_addr *dp = buf; struct eth_pdata *pdata = dev_get_plat(dev);
[..]
Regards, Simon
On 23.07.23 05:48, Simon Glass wrote:
Hi Heinrich,
On Fri, 21 Jul 2023 at 00:34, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
Move the recursive dp_fill(dev->parent) call to a single location. Determine uclass_id only once.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
lib/efi_loader/efi_device_path.c | 45 +++++++++++++------------------- 1 file changed, 18 insertions(+), 27 deletions(-)
diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index 5b050fa17c..ed7214f3a3 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -548,14 +548,19 @@ __maybe_unused static unsigned int dp_size(struct udevice *dev) */ __maybe_unused static void *dp_fill(void *buf, struct udevice *dev) {
enum uclass_id uclass_id;if (!dev || !dev->driver) return buf;
switch (device_get_uclass_id(dev)) {
uclass_id = device_get_uclass_id(dev);if (uclass_id != UCLASS_ROOT)Can we fix this one now? We should use EFI_MEDIA here I think?
The function dp_fill() is used to create an EFI device path for any DM device.
Given a device we recursively create a device path with nodes for every device in the dm tree up to the root node. We must stop the recursion at the root node.
buf = dp_fill(buf, dev->parent);switch (uclass_id) {
struct efi_device_path_mac_addr *dp =dp_fill(buf, dev->parent);So how does the parent part get added? I am missing something here...or it was it never needed??
dp_fill() is a recursive function as explained above.
Best regards
Heinrich
struct efi_device_path_mac_addr *dp = buf; struct eth_pdata *pdata = dev_get_plat(dev);[..]
Regards, Simon
On Wed, 26 Jul 2023 at 07:02, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 23.07.23 05:48, Simon Glass wrote:
Hi Heinrich,
On Fri, 21 Jul 2023 at 00:34, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
Move the recursive dp_fill(dev->parent) call to a single location. Determine uclass_id only once.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
lib/efi_loader/efi_device_path.c | 45 +++++++++++++------------------- 1 file changed, 18 insertions(+), 27 deletions(-)
diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index 5b050fa17c..ed7214f3a3 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -548,14 +548,19 @@ __maybe_unused static unsigned int dp_size(struct udevice *dev) */ __maybe_unused static void *dp_fill(void *buf, struct udevice *dev) {
enum uclass_id uclass_id;if (!dev || !dev->driver) return buf;
switch (device_get_uclass_id(dev)) {
uclass_id = device_get_uclass_id(dev);if (uclass_id != UCLASS_ROOT)Can we fix this one now? We should use EFI_MEDIA here I think?
The function dp_fill() is used to create an EFI device path for any DM device.
Given a device we recursively create a device path with nodes for every device in the dm tree up to the root node. We must stop the recursion at the root node.
buf = dp_fill(buf, dev->parent);switch (uclass_id) {
struct efi_device_path_mac_addr *dp =dp_fill(buf, dev->parent);So how does the parent part get added? I am missing something here...or it was it never needed??
dp_fill() is a recursive function as explained above.
OK I see
Reviewed-by: Simon Glass sjg@chromium.org
Regards, SImon
participants (2)
-
Heinrich Schuchardt -
Simon Glass