[RFC 0/7] efi_loader: move device-path node generation to DM
UEFI device-paths are used in the EFI world to depict the parent-child relationship between devices. No two device can have the same device-path. Currently we fail to generate unique devices-paths.
The nodes in UEFI device paths should match the devices on the path to the DM root node.
I have started drafting a solution. Several block device classes still need to be handled (IDE, NVMe, EFI, VirtIO). Also handling of PCI addresses is missing. But this series contains enough to see the direction of development.
Heinrich Schuchardt (7): dm: add get_dp_node() to struct uclass_driver dm: implement uclass_get_dp_node() dm: implement get_dp_node for block devices dm: implement get_dp_node for USB hub devices dm: implement get_dp_node for USB mass storage devices dm: implement get_dp_node for MMC devices efi_loader: use uclass_get_dp_node
common/usb_hub.c | 33 ++++ common/usb_storage.c | 33 ++++ drivers/block/blk-uclass.c | 56 ++++++ drivers/core/uclass.c | 26 +++ drivers/mmc/mmc-uclass.c | 25 +++ include/dm/uclass.h | 13 ++ include/efi_loader.h | 7 + lib/efi_loader/efi_device_path.c | 325 +++---------------------------- 8 files changed, 221 insertions(+), 297 deletions(-)
Currently the device paths don't match the dm tree. We should create a device path node per dm tree node.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- include/dm/uclass.h | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/include/dm/uclass.h b/include/dm/uclass.h index ee15c92063..e11637ce4d 100644 --- a/include/dm/uclass.h +++ b/include/dm/uclass.h @@ -11,6 +11,7 @@
#include <dm/ofnode.h> #include <dm/uclass-id.h> +#include <efi.h> #include <linker_lists.h> #include <linux/list.h>
@@ -68,6 +69,7 @@ struct udevice; * @child_post_probe: Called after a child in this uclass is probed * @init: Called to set up the uclass * @destroy: Called to destroy the uclass + * @get_dp_node: Get EFI device path node * @priv_auto: If non-zero this is the size of the private data * to be allocated in the uclass's ->priv pointer. If zero, then the uclass * driver is responsible for allocating any data required. @@ -99,6 +101,9 @@ struct uclass_driver { int (*child_post_probe)(struct udevice *dev); int (*init)(struct uclass *class); int (*destroy)(struct uclass *class); +#if CONFIG_IS_ENABLED(EFI_LOADER) + struct efi_device_path *(*get_dp_node)(struct udevice *dev); +#endif int priv_auto; int per_device_auto; int per_device_plat_auto;
Hi Heinrich,
On Mon, 27 Mar 2023 at 06:27, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
Currently the device paths don't match the dm tree. We should create a device path node per dm tree node.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
include/dm/uclass.h | 5 +++++ 1 file changed, 5 insertions(+)
This affects very few uclasses but adds a field to all of them. I think an event might be best for this. We can add an event spy in each of the affected uclasses.
diff --git a/include/dm/uclass.h b/include/dm/uclass.h index ee15c92063..e11637ce4d 100644 --- a/include/dm/uclass.h +++ b/include/dm/uclass.h @@ -11,6 +11,7 @@
#include <dm/ofnode.h> #include <dm/uclass-id.h> +#include <efi.h> #include <linker_lists.h> #include <linux/list.h>
@@ -68,6 +69,7 @@ struct udevice;
- @child_post_probe: Called after a child in this uclass is probed
- @init: Called to set up the uclass
- @destroy: Called to destroy the uclass
- @get_dp_node: Get EFI device path node
- @priv_auto: If non-zero this is the size of the private data
- to be allocated in the uclass's ->priv pointer. If zero, then the uclass
- driver is responsible for allocating any data required.
@@ -99,6 +101,9 @@ struct uclass_driver { int (*child_post_probe)(struct udevice *dev); int (*init)(struct uclass *class); int (*destroy)(struct uclass *class); +#if CONFIG_IS_ENABLED(EFI_LOADER)
struct efi_device_path *(*get_dp_node)(struct udevice *dev);+#endif int priv_auto; int per_device_auto; int per_device_plat_auto; -- 2.39.2
Hi Simon,
On Mon, 27 Mar 2023 at 07:01, Simon Glass sjg@chromium.org wrote:
Hi Heinrich,
On Mon, 27 Mar 2023 at 06:27, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
Currently the device paths don't match the dm tree. We should create a device path node per dm tree node.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
include/dm/uclass.h | 5 +++++ 1 file changed, 5 insertions(+)
This affects very few uclasses but adds a field to all of them. I think an event might be best for this. We can add an event spy in each of the affected uclasses.
This is supposed to be a call from the efi subsystem do get a DP for a given device. I am not sure how we could replace this with an event.
Regards /Ilias
diff --git a/include/dm/uclass.h b/include/dm/uclass.h index ee15c92063..e11637ce4d 100644 --- a/include/dm/uclass.h +++ b/include/dm/uclass.h @@ -11,6 +11,7 @@
#include <dm/ofnode.h> #include <dm/uclass-id.h> +#include <efi.h> #include <linker_lists.h> #include <linux/list.h>
@@ -68,6 +69,7 @@ struct udevice;
- @child_post_probe: Called after a child in this uclass is probed
- @init: Called to set up the uclass
- @destroy: Called to destroy the uclass
- @get_dp_node: Get EFI device path node
- @priv_auto: If non-zero this is the size of the private data
- to be allocated in the uclass's ->priv pointer. If zero, then the uclass
- driver is responsible for allocating any data required.
@@ -99,6 +101,9 @@ struct uclass_driver { int (*child_post_probe)(struct udevice *dev); int (*init)(struct uclass *class); int (*destroy)(struct uclass *class); +#if CONFIG_IS_ENABLED(EFI_LOADER)
struct efi_device_path *(*get_dp_node)(struct udevice *dev);+#endif int priv_auto; int per_device_auto; int per_device_plat_auto; -- 2.39.2
On 3/27/23 06:00, Simon Glass wrote:
Hi Heinrich,
On Mon, 27 Mar 2023 at 06:27, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
Currently the device paths don't match the dm tree. We should create a device path node per dm tree node.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
include/dm/uclass.h | 5 +++++ 1 file changed, 5 insertions(+)
This affects very few uclasses but adds a field to all of them. I think an event might be best for this. We can add an event spy in each of the affected uclasses.
EVENT_SPY does not allow to return information to the emitter of the event.
event_notfify() does not allow to select a single recipient.
In struct uclass_driver there are many other fields that are rarely used:
int (*post_bind)(struct udevice *dev); int (*pre_unbind)(struct udevice *dev); int (*pre_probe)(struct udevice *dev); int (*post_probe)(struct udevice *dev); int (*pre_remove)(struct udevice *dev); int (*child_post_bind)(struct udevice *dev); int (*child_pre_probe)(struct udevice *dev); int (*child_post_probe)(struct udevice *dev);
If we wanted to save memory in linker generated lists we would convert structures with pointers to variable size arrays e.g.
struct ops * = { { POST_BIND, post_bind }, { CHILD_POST_BIND, child_post_bind}, { END, NULL }, }
Best regards
Heinrich
diff --git a/include/dm/uclass.h b/include/dm/uclass.h index ee15c92063..e11637ce4d 100644 --- a/include/dm/uclass.h +++ b/include/dm/uclass.h @@ -11,6 +11,7 @@
#include <dm/ofnode.h> #include <dm/uclass-id.h> +#include <efi.h> #include <linker_lists.h> #include <linux/list.h>
@@ -68,6 +69,7 @@ struct udevice;
- @child_post_probe: Called after a child in this uclass is probed
- @init: Called to set up the uclass
- @destroy: Called to destroy the uclass
- @get_dp_node: Get EFI device path node
- @priv_auto: If non-zero this is the size of the private data
- to be allocated in the uclass's ->priv pointer. If zero, then the uclass
- driver is responsible for allocating any data required.
@@ -99,6 +101,9 @@ struct uclass_driver { int (*child_post_probe)(struct udevice *dev); int (*init)(struct uclass *class); int (*destroy)(struct uclass *class); +#if CONFIG_IS_ENABLED(EFI_LOADER)
struct efi_device_path *(*get_dp_node)(struct udevice *dev);+#endif int priv_auto; int per_device_auto; int per_device_plat_auto; -- 2.39.2
Hi Heinrich,
On Mon, 27 Mar 2023 at 19:48, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 3/27/23 06:00, Simon Glass wrote:
Hi Heinrich,
On Mon, 27 Mar 2023 at 06:27, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
Currently the device paths don't match the dm tree. We should create a device path node per dm tree node.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
include/dm/uclass.h | 5 +++++ 1 file changed, 5 insertions(+)
This affects very few uclasses but adds a field to all of them. I think an event might be best for this. We can add an event spy in each of the affected uclasses.
EVENT_SPY does not allow to return information to the emitter of the event.
You just add it into unio event_data
event_notfify() does not allow to select a single recipient.
Do you need that? You can check whether the uclass matches, for example.
In struct uclass_driver there are many other fields that are rarely used:
int (*post_bind)(struct udevice *dev); int (*pre_unbind)(struct udevice *dev); int (*pre_probe)(struct udevice *dev); int (*post_probe)(struct udevice *dev); int (*pre_remove)(struct udevice *dev); int (*child_post_bind)(struct udevice *dev); int (*child_pre_probe)(struct udevice *dev); int (*child_post_probe)(struct udevice *dev);If we wanted to save memory in linker generated lists we would convert structures with pointers to variable size arrays e.g.
struct ops * = { { POST_BIND, post_bind }, { CHILD_POST_BIND, child_post_bind}, { END, NULL }, }
Yes. We have the same problem with devices. We have a way to resolve this today, using tags. We use DM_TAG_EFI and there are tags for the devices but support for using them to reduce the struct sizes is not yet added.
We might have a problem with efficiency, since direct pointers are faster, but of course we can address that using a suitable data structure.
Regards, Simon
On 3/27/23 10:24, Simon Glass wrote:
Hi Heinrich,
On Mon, 27 Mar 2023 at 19:48, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 3/27/23 06:00, Simon Glass wrote:
Hi Heinrich,
On Mon, 27 Mar 2023 at 06:27, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
Currently the device paths don't match the dm tree. We should create a device path node per dm tree node.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
include/dm/uclass.h | 5 +++++ 1 file changed, 5 insertions(+)
This affects very few uclasses but adds a field to all of them. I think an event might be best for this. We can add an event spy in each of the affected uclasses.
EVENT_SPY does not allow to return information to the emitter of the event.
You just add it into unio event_data
If you have multiple listeners for a single event, you can't be sure who added what.
event_notfify() does not allow to select a single recipient.
Do you need that? You can check whether the uclass matches, for example.
Checking the uclass multiple times is not efficient.
We are talking about 1 KiB for the pointers vs repetitive coding. We should go for the simplest code and most efficient code.
If you want to save memory, (temporarily) remove unused methods in struct uclass_driver: pre_unbind, post_bind, post_remove.
Best regards
Heinrich
In struct uclass_driver there are many other fields that are rarely used:
int (*post_bind)(struct udevice *dev); int (*pre_unbind)(struct udevice *dev); int (*pre_probe)(struct udevice *dev); int (*post_probe)(struct udevice *dev); int (*pre_remove)(struct udevice *dev); int (*child_post_bind)(struct udevice *dev); int (*child_pre_probe)(struct udevice *dev); int (*child_post_probe)(struct udevice *dev);If we wanted to save memory in linker generated lists we would convert structures with pointers to variable size arrays e.g.
struct ops * = { { POST_BIND, post_bind }, { CHILD_POST_BIND, child_post_bind}, { END, NULL }, }
Yes. We have the same problem with devices. We have a way to resolve this today, using tags. We use DM_TAG_EFI and there are tags for the devices but support for using them to reduce the struct sizes is not yet added.
We might have a problem with efficiency, since direct pointers are faster, but of course we can address that using a suitable data structure.
Regards, Simon
Hi Heinrich,
On Mon, 27 Mar 2023 at 22:41, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 3/27/23 10:24, Simon Glass wrote:
Hi Heinrich,
On Mon, 27 Mar 2023 at 19:48, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 3/27/23 06:00, Simon Glass wrote:
Hi Heinrich,
On Mon, 27 Mar 2023 at 06:27, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
Currently the device paths don't match the dm tree. We should create a device path node per dm tree node.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
include/dm/uclass.h | 5 +++++ 1 file changed, 5 insertions(+)
This affects very few uclasses but adds a field to all of them. I think an event might be best for this. We can add an event spy in each of the affected uclasses.
EVENT_SPY does not allow to return information to the emitter of the event.
You just add it into unio event_data
If you have multiple listeners for a single event, you can't be sure who added what.
It is possible for a spy to claim an event and put its info in it.
event_notfify() does not allow to select a single recipient.
Do you need that? You can check whether the uclass matches, for example.
Checking the uclass multiple times is not efficient.
We are talking about 1 KiB for the pointers vs repetitive coding. We should go for the simplest code and most efficient code.
Firstly, this should be done in an API so that the actual implementation can be adjusted in future. Let's continue the discussion on the other patch.
If you want to save memory, (temporarily) remove unused methods in struct uclass_driver: pre_unbind, post_bind, post_remove.
That is a separate issue from adding more. I would like to do that, but it would be easy enough for someone else to take this on, given that the tag support is in there now.
[..]
Regards, Simon
Provide a function to get the EFI device path node representing a device.
If implemented, it invokes the uclass driver's get_dp_node() function. Otherwise a vendor hardware node is created.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- drivers/core/uclass.c | 26 ++++++++++++++++++++++++++ include/dm/uclass.h | 8 ++++++++ include/efi_loader.h | 7 +++++++ 3 files changed, 41 insertions(+)
diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c index 1762a0796d..153c954ee3 100644 --- a/drivers/core/uclass.c +++ b/drivers/core/uclass.c @@ -10,6 +10,7 @@
#include <common.h> #include <dm.h> +#include <efi_loader.h> #include <errno.h> #include <log.h> #include <malloc.h> @@ -802,6 +803,31 @@ int uclass_pre_remove_device(struct udevice *dev) } #endif
+#if CONFIG_IS_ENABLED(EFI_LOADER) +struct efi_device_path *uclass_get_dp_node(struct udevice *dev) +{ + struct uclass *uc; + struct efi_device_path_uboot *dp; + + uc = dev->uclass; + if (uc->uc_drv->get_dp_node) + return uc->uc_drv->get_dp_node(dev); + + dp = efi_alloc(sizeof(struct efi_device_path_uboot)); + if (!dp) + return NULL; + + dp->dp.type = DEVICE_PATH_TYPE_HARDWARE_DEVICE; + dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_VENDOR; + dp->dp.length = sizeof(struct efi_device_path_uboot); + dp->guid = efi_u_boot_guid; + dp->uclass_id = uc->uc_drv->id; + dp->seq_ = dev->seq_; + + return &dp->dp; +} +#endif + int uclass_probe_all(enum uclass_id id) { struct udevice *dev; diff --git a/include/dm/uclass.h b/include/dm/uclass.h index e11637ce4d..f39dbac21d 100644 --- a/include/dm/uclass.h +++ b/include/dm/uclass.h @@ -503,3 +503,11 @@ int uclass_id_count(enum uclass_id id); uclass_next_device(&dev))
#endif + +/** + * uclass_get_dp_node() - get EFI device path node for device + * + * @dev: device + * Return: device path node or NULL if out of memory + */ +struct efi_device_path *uclass_get_dp_node(struct udevice *dev); diff --git a/include/efi_loader.h b/include/efi_loader.h index cee04cbb9d..f111bc616d 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -22,6 +22,13 @@ struct blk_desc; struct jmp_buf_data;
+struct efi_device_path_uboot { + struct efi_device_path dp; + efi_guid_t guid; + enum uclass_id uclass_id; + int seq_; +} __packed; + static inline int guidcmp(const void *g1, const void *g2) { return memcmp(g1, g2, sizeof(efi_guid_t));
Hi Heinrich,
On Mon, 27 Mar 2023 at 06:27, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
Provide a function to get the EFI device path node representing a device.
If implemented, it invokes the uclass driver's get_dp_node() function. Otherwise a vendor hardware node is created.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
drivers/core/uclass.c | 26 ++++++++++++++++++++++++++ include/dm/uclass.h | 8 ++++++++ include/efi_loader.h | 7 +++++++ 3 files changed, 41 insertions(+)
Can you add a new file, like drivers/core/efi.c for this stuff?
You should also pull out the header stuff you need and put it in include/dm/efi.h since we don't want to include efi_loader.h from driver/core.
diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c index 1762a0796d..153c954ee3 100644 --- a/drivers/core/uclass.c +++ b/drivers/core/uclass.c @@ -10,6 +10,7 @@
#include <common.h> #include <dm.h> +#include <efi_loader.h> #include <errno.h> #include <log.h> #include <malloc.h> @@ -802,6 +803,31 @@ int uclass_pre_remove_device(struct udevice *dev) } #endif
+#if CONFIG_IS_ENABLED(EFI_LOADER) +struct efi_device_path *uclass_get_dp_node(struct udevice *dev) +{
struct uclass *uc;struct efi_device_path_uboot *dp;uc = dev->uclass;if (uc->uc_drv->get_dp_node)return uc->uc_drv->get_dp_node(dev);dp = efi_alloc(sizeof(struct efi_device_path_uboot));
This should call malloc(), right?
How does this memory get freed? How do we avoid allocating it twice if it is called twice?
if (!dp)return NULL;
-ENOMEM ? I suggest changing the function return value to an int.
dp->dp.type = DEVICE_PATH_TYPE_HARDWARE_DEVICE;dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_VENDOR;dp->dp.length = sizeof(struct efi_device_path_uboot);dp->guid = efi_u_boot_guid;dp->uclass_id = uc->uc_drv->id;dp->seq_ = dev->seq_;return &dp->dp;+} +#endif
int uclass_probe_all(enum uclass_id id) { struct udevice *dev; diff --git a/include/dm/uclass.h b/include/dm/uclass.h index e11637ce4d..f39dbac21d 100644 --- a/include/dm/uclass.h +++ b/include/dm/uclass.h @@ -503,3 +503,11 @@ int uclass_id_count(enum uclass_id id); uclass_next_device(&dev))
#endif
+/**
- uclass_get_dp_node() - get EFI device path node for device
- @dev: device
- Return: device path node or NULL if out of memory
- */
+struct efi_device_path *uclass_get_dp_node(struct udevice *dev); diff --git a/include/efi_loader.h b/include/efi_loader.h index cee04cbb9d..f111bc616d 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -22,6 +22,13 @@ struct blk_desc; struct jmp_buf_data;
+struct efi_device_path_uboot {
struct efi_device_path dp;efi_guid_t guid;enum uclass_id uclass_id;int seq_;
What is this member for? Can you please comment this struct?
+} __packed;
static inline int guidcmp(const void *g1, const void *g2) { return memcmp(g1, g2, sizeof(efi_guid_t)); -- 2.39.2
Regards, Simon
Generate a Ctrl() node for block devices.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- drivers/block/blk-uclass.c | 56 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+)
diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c index c69fc4d518..08202aaaba 100644 --- a/drivers/block/blk-uclass.c +++ b/drivers/block/blk-uclass.c @@ -9,6 +9,7 @@ #include <common.h> #include <blk.h> #include <dm.h> +#include <efi_loader.h> #include <log.h> #include <malloc.h> #include <part.h> @@ -779,9 +780,64 @@ static int blk_post_probe(struct udevice *dev) return 0; }
+#if CONFIG_IS_ENABLED(EFI_LOADER) +__maybe_unused +static struct efi_device_path *blk_scsi_get_dp_node(struct udevice *dev) +{ + struct efi_device_path_scsi *dp; + struct blk_desc *desc = dev_get_uclass_plat(dev); + + dp = efi_alloc(sizeof(struct efi_device_path_scsi)); + if (!dp) + return NULL; + + dp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE; + dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MSG_SCSI; + dp->dp.length = sizeof(*dp); + dp->logical_unit_number = desc->lun; + dp->target_id = desc->target; + + return &dp->dp; +} + +static struct efi_device_path *blk_get_dp_node(struct udevice *dev) +{ + struct efi_device_path_controller *dp; + struct blk_desc *desc = dev_get_uclass_plat(dev); + u32 controller_number; + + switch (device_get_uclass_id(dev->parent)) { +#if CONFIG_IS_ENABLED(SCSI) + case UCLASS_SCSI: + return blk_scsi_get_dp_node(dev); +#endif + case UCLASS_MMC: + dp->controller_number = desc->hwpart; + break; + default: + dp->controller_number = desc->lun; + break; + } + + dp = efi_alloc(sizeof(struct efi_device_path_controller)); + if (!dp) + return NULL; + + dp->dp.type = DEVICE_PATH_TYPE_HARDWARE_DEVICE; + dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_CONTROLLER; + dp->dp.length = sizeof(*dp); + dp->controller_number = controller_number; + + return &dp->dp; +} +#endif + UCLASS_DRIVER(blk) = { .id = UCLASS_BLK, .name = "blk", .post_probe = blk_post_probe, .per_device_plat_auto = sizeof(struct blk_desc), +#if CONFIG_IS_ENABLED(EFI_LOADER) + .get_dp_node = blk_get_dp_node, +#endif };
Hi Heinrich,
On Mon, 27 Mar 2023 at 06:27, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
Generate a Ctrl() node for block devices.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
drivers/block/blk-uclass.c | 56 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+)
Can this go in drivers/block/blk_efi.c ?
Can you create a function which returns the path given a device? Then we are discuss the get_dp_node() member or moving to an event. I favour the event for now.
diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c index c69fc4d518..08202aaaba 100644 --- a/drivers/block/blk-uclass.c +++ b/drivers/block/blk-uclass.c @@ -9,6 +9,7 @@ #include <common.h> #include <blk.h> #include <dm.h> +#include <efi_loader.h> #include <log.h> #include <malloc.h> #include <part.h> @@ -779,9 +780,64 @@ static int blk_post_probe(struct udevice *dev) return 0; }
+#if CONFIG_IS_ENABLED(EFI_LOADER) +__maybe_unused +static struct efi_device_path *blk_scsi_get_dp_node(struct udevice *dev) +{
struct efi_device_path_scsi *dp;struct blk_desc *desc = dev_get_uclass_plat(dev);dp = efi_alloc(sizeof(struct efi_device_path_scsi));if (!dp)return NULL;dp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE;dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MSG_SCSI;dp->dp.length = sizeof(*dp);dp->logical_unit_number = desc->lun;dp->target_id = desc->target;return &dp->dp;+}
+static struct efi_device_path *blk_get_dp_node(struct udevice *dev) +{
struct efi_device_path_controller *dp;struct blk_desc *desc = dev_get_uclass_plat(dev);u32 controller_number;switch (device_get_uclass_id(dev->parent)) {+#if CONFIG_IS_ENABLED(SCSI)
case UCLASS_SCSI:return blk_scsi_get_dp_node(dev);+#endif
case UCLASS_MMC:dp->controller_number = desc->hwpart;break;default:
Since this is checking the parent class, it seems to me that this info should be attacked there (the 'media' device) instead of the block device.
dp->controller_number = desc->lun;break;}dp = efi_alloc(sizeof(struct efi_device_path_controller));if (!dp)return NULL;dp->dp.type = DEVICE_PATH_TYPE_HARDWARE_DEVICE;dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_CONTROLLER;dp->dp.length = sizeof(*dp);dp->controller_number = controller_number;return &dp->dp;+} +#endif
UCLASS_DRIVER(blk) = { .id = UCLASS_BLK, .name = "blk", .post_probe = blk_post_probe, .per_device_plat_auto = sizeof(struct blk_desc), +#if CONFIG_IS_ENABLED(EFI_LOADER)
.get_dp_node = blk_get_dp_node,+#endif }; -- 2.39.2
Regards, Simon
On 3/27/23 06:00, Simon Glass wrote:
Hi Heinrich,
On Mon, 27 Mar 2023 at 06:27, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
Generate a Ctrl() node for block devices.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
drivers/block/blk-uclass.c | 56 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+)
Can this go in drivers/block/blk_efi.c ?
Can you create a function which returns the path given a device? Then
That function already exists in lib/efi_loader/device_path.
we are discuss the get_dp_node() member or moving to an event. I favour the event for now.
If for a device we already have an EFI handle with a device-path installed, the device-path of its children has to be constructed by adding a uclass specific device path node.
Otherwise we have to recurse to the root node to collect all device path nodes.
diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c index c69fc4d518..08202aaaba 100644 --- a/drivers/block/blk-uclass.c +++ b/drivers/block/blk-uclass.c @@ -9,6 +9,7 @@ #include <common.h> #include <blk.h> #include <dm.h> +#include <efi_loader.h> #include <log.h> #include <malloc.h> #include <part.h> @@ -779,9 +780,64 @@ static int blk_post_probe(struct udevice *dev) return 0; }
+#if CONFIG_IS_ENABLED(EFI_LOADER) +__maybe_unused +static struct efi_device_path *blk_scsi_get_dp_node(struct udevice *dev) +{
struct efi_device_path_scsi *dp;struct blk_desc *desc = dev_get_uclass_plat(dev);dp = efi_alloc(sizeof(struct efi_device_path_scsi));if (!dp)return NULL;dp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE;dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MSG_SCSI;dp->dp.length = sizeof(*dp);dp->logical_unit_number = desc->lun;dp->target_id = desc->target;return &dp->dp;+}
+static struct efi_device_path *blk_get_dp_node(struct udevice *dev) +{
struct efi_device_path_controller *dp;struct blk_desc *desc = dev_get_uclass_plat(dev);u32 controller_number;switch (device_get_uclass_id(dev->parent)) {+#if CONFIG_IS_ENABLED(SCSI)
case UCLASS_SCSI:return blk_scsi_get_dp_node(dev);+#endif
case UCLASS_MMC:dp->controller_number = desc->hwpart;break;default:Since this is checking the parent class, it seems to me that this info should be attacked there (the 'media' device) instead of the block device.
For MMC we have these layers:
* MMC controller - UCLASS_MMC * MMC slot (or channel) - missing uclass * hardware partition - UCLASS_BLK
See https://www.sage-micro.com/us/portfolio-items/s881/
Currently in the DM tree we don't model the MMC controller and the MMC slot separately. I think this is a gap that should be closed.
A single UCLASS_MMC (= MMC slot) device can have multiple UCLASS_BLK children. One for each hardware partition.
An MMC controller in EFI is modelled as VenHW() node, a slot as SD() or eMMC() node.
An eMMC() or SD() node has a field for the slot number but none for the hardware partition. Hence, we cannot attach the hardware partition information to the UCLASS_MMC device. For the UCLASS_BLK device I use a Ctrl() node. We could also consider using a Unit() node but Unit() nodes are meant to be used for LUNs.
We have a similar situation for USB and SCSI LUNs.
Best regards
Heinrich
dp->controller_number = desc->lun;break;}dp = efi_alloc(sizeof(struct efi_device_path_controller));if (!dp)return NULL;dp->dp.type = DEVICE_PATH_TYPE_HARDWARE_DEVICE;dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_CONTROLLER;dp->dp.length = sizeof(*dp);dp->controller_number = controller_number;return &dp->dp;+} +#endif
- UCLASS_DRIVER(blk) = { .id = UCLASS_BLK, .name = "blk", .post_probe = blk_post_probe, .per_device_plat_auto = sizeof(struct blk_desc),
+#if CONFIG_IS_ENABLED(EFI_LOADER)
.get_dp_node = blk_get_dp_node,+#endif }; -- 2.39.2
Regards, Simon
Hi Heinrich,
On Mon, 27 Mar 2023 at 19:13, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 3/27/23 06:00, Simon Glass wrote:
Hi Heinrich,
On Mon, 27 Mar 2023 at 06:27, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
Generate a Ctrl() node for block devices.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
drivers/block/blk-uclass.c | 56 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+)
Can this go in drivers/block/blk_efi.c ?
Can you create a function which returns the path given a device? Then
That function already exists in lib/efi_loader/device_path.
Yes, but I mean one for each media type.
we are discuss the get_dp_node() member or moving to an event. I favour the event for now.
If for a device we already have an EFI handle with a device-path installed, the device-path of its children has to be constructed by adding a uclass specific device path node.
Otherwise we have to recurse to the root node to collect all device path nodes.
OK
diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c index c69fc4d518..08202aaaba 100644 --- a/drivers/block/blk-uclass.c +++ b/drivers/block/blk-uclass.c @@ -9,6 +9,7 @@ #include <common.h> #include <blk.h> #include <dm.h> +#include <efi_loader.h> #include <log.h> #include <malloc.h> #include <part.h> @@ -779,9 +780,64 @@ static int blk_post_probe(struct udevice *dev) return 0; }
+#if CONFIG_IS_ENABLED(EFI_LOADER) +__maybe_unused +static struct efi_device_path *blk_scsi_get_dp_node(struct udevice *dev) +{
struct efi_device_path_scsi *dp;struct blk_desc *desc = dev_get_uclass_plat(dev);dp = efi_alloc(sizeof(struct efi_device_path_scsi));if (!dp)return NULL;dp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE;dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MSG_SCSI;dp->dp.length = sizeof(*dp);dp->logical_unit_number = desc->lun;dp->target_id = desc->target;return &dp->dp;+}
+static struct efi_device_path *blk_get_dp_node(struct udevice *dev) +{
struct efi_device_path_controller *dp;struct blk_desc *desc = dev_get_uclass_plat(dev);u32 controller_number;switch (device_get_uclass_id(dev->parent)) {+#if CONFIG_IS_ENABLED(SCSI)
case UCLASS_SCSI:return blk_scsi_get_dp_node(dev);+#endif
case UCLASS_MMC:dp->controller_number = desc->hwpart;break;default:Since this is checking the parent class, it seems to me that this info should be attacked there (the 'media' device) instead of the block device.
For MMC we have these layers:
- MMC controller - UCLASS_MMC
- MMC slot (or channel) - missing uclass
- hardware partition - UCLASS_BLK
See https://www.sage-micro.com/us/portfolio-items/s881/
Currently in the DM tree we don't model the MMC controller and the MMC slot separately. I think this is a gap that should be closed.
A single UCLASS_MMC (= MMC slot) device can have multiple UCLASS_BLK children. One for each hardware partition.
An MMC controller in EFI is modelled as VenHW() node, a slot as SD() or eMMC() node.
An eMMC() or SD() node has a field for the slot number but none for the hardware partition. Hence, we cannot attach the hardware partition information to the UCLASS_MMC device. For the UCLASS_BLK device I use a Ctrl() node. We could also consider using a Unit() node but Unit() nodes are meant to be used for LUNs.
We have a similar situation for USB and SCSI LUNs.
The current model is to add multiple BLK children for each media device, if needed. That seems to work OK for now. What do you think?
Regards, Simon
On 3/27/23 10:24, Simon Glass wrote:
Hi Heinrich,
On Mon, 27 Mar 2023 at 19:13, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 3/27/23 06:00, Simon Glass wrote:
Hi Heinrich,
On Mon, 27 Mar 2023 at 06:27, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
Generate a Ctrl() node for block devices.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
drivers/block/blk-uclass.c | 56 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+)
Can this go in drivers/block/blk_efi.c ?
Can you create a function which returns the path given a device? Then
That function already exists in lib/efi_loader/device_path.
Yes, but I mean one for each media type.
we are discuss the get_dp_node() member or moving to an event. I favour the event for now.
If for a device we already have an EFI handle with a device-path installed, the device-path of its children has to be constructed by adding a uclass specific device path node.
Otherwise we have to recurse to the root node to collect all device path nodes.
OK
diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c index c69fc4d518..08202aaaba 100644 --- a/drivers/block/blk-uclass.c +++ b/drivers/block/blk-uclass.c @@ -9,6 +9,7 @@ #include <common.h> #include <blk.h> #include <dm.h> +#include <efi_loader.h> #include <log.h> #include <malloc.h> #include <part.h> @@ -779,9 +780,64 @@ static int blk_post_probe(struct udevice *dev) return 0; }
+#if CONFIG_IS_ENABLED(EFI_LOADER) +__maybe_unused +static struct efi_device_path *blk_scsi_get_dp_node(struct udevice *dev) +{
struct efi_device_path_scsi *dp;struct blk_desc *desc = dev_get_uclass_plat(dev);dp = efi_alloc(sizeof(struct efi_device_path_scsi));if (!dp)return NULL;dp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE;dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MSG_SCSI;dp->dp.length = sizeof(*dp);dp->logical_unit_number = desc->lun;dp->target_id = desc->target;return &dp->dp;+}
+static struct efi_device_path *blk_get_dp_node(struct udevice *dev) +{
struct efi_device_path_controller *dp;struct blk_desc *desc = dev_get_uclass_plat(dev);u32 controller_number;switch (device_get_uclass_id(dev->parent)) {+#if CONFIG_IS_ENABLED(SCSI)
case UCLASS_SCSI:return blk_scsi_get_dp_node(dev);+#endif
case UCLASS_MMC:dp->controller_number = desc->hwpart;break;default:Since this is checking the parent class, it seems to me that this info should be attacked there (the 'media' device) instead of the block device.
For MMC we have these layers:
- MMC controller - UCLASS_MMC
- MMC slot (or channel) - missing uclass
- hardware partition - UCLASS_BLK
See https://www.sage-micro.com/us/portfolio-items/s881/
Currently in the DM tree we don't model the MMC controller and the MMC slot separately. I think this is a gap that should be closed.
A single UCLASS_MMC (= MMC slot) device can have multiple UCLASS_BLK children. One for each hardware partition.
An MMC controller in EFI is modelled as VenHW() node, a slot as SD() or eMMC() node.
An eMMC() or SD() node has a field for the slot number but none for the hardware partition. Hence, we cannot attach the hardware partition information to the UCLASS_MMC device. For the UCLASS_BLK device I use a Ctrl() node. We could also consider using a Unit() node but Unit() nodes are meant to be used for LUNs.
We have a similar situation for USB and SCSI LUNs.
The current model is to add multiple BLK children for each media device, if needed. That seems to work OK for now. What do you think?
There is nothing in DM tree we need to change now. As I understood your first reply it was not clear to you which EFI device-path node type with which information matches which DM tree node:
MMC controller/MMC slot (UCLASS_MMC) -> SD(slot) or MMC(slot) Hardware partition (UCLASS_BLK) -> Ctrl()
SCSI drive (UCLASS_SCSI) -> VenHw() SCSI LUN (UClASS_BLK) -> Scsi(PUN, LUN)
Best regards
Heinrich
Hi Heinrich,
On Mon, 27 Mar 2023 at 22:31, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 3/27/23 10:24, Simon Glass wrote:
Hi Heinrich,
On Mon, 27 Mar 2023 at 19:13, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 3/27/23 06:00, Simon Glass wrote:
Hi Heinrich,
On Mon, 27 Mar 2023 at 06:27, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
Generate a Ctrl() node for block devices.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
drivers/block/blk-uclass.c | 56 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+)
Can this go in drivers/block/blk_efi.c ?
Can you create a function which returns the path given a device? Then
That function already exists in lib/efi_loader/device_path.
Yes, but I mean one for each media type.
we are discuss the get_dp_node() member or moving to an event. I favour the event for now.
If for a device we already have an EFI handle with a device-path installed, the device-path of its children has to be constructed by adding a uclass specific device path node.
Otherwise we have to recurse to the root node to collect all device path nodes.
OK
diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c index c69fc4d518..08202aaaba 100644 --- a/drivers/block/blk-uclass.c +++ b/drivers/block/blk-uclass.c @@ -9,6 +9,7 @@ #include <common.h> #include <blk.h> #include <dm.h> +#include <efi_loader.h> #include <log.h> #include <malloc.h> #include <part.h> @@ -779,9 +780,64 @@ static int blk_post_probe(struct udevice *dev) return 0; }
+#if CONFIG_IS_ENABLED(EFI_LOADER) +__maybe_unused +static struct efi_device_path *blk_scsi_get_dp_node(struct udevice *dev) +{
struct efi_device_path_scsi *dp;struct blk_desc *desc = dev_get_uclass_plat(dev);dp = efi_alloc(sizeof(struct efi_device_path_scsi));if (!dp)return NULL;dp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE;dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MSG_SCSI;dp->dp.length = sizeof(*dp);dp->logical_unit_number = desc->lun;dp->target_id = desc->target;return &dp->dp;+}
+static struct efi_device_path *blk_get_dp_node(struct udevice *dev) +{
struct efi_device_path_controller *dp;struct blk_desc *desc = dev_get_uclass_plat(dev);u32 controller_number;switch (device_get_uclass_id(dev->parent)) {+#if CONFIG_IS_ENABLED(SCSI)
case UCLASS_SCSI:return blk_scsi_get_dp_node(dev);+#endif
case UCLASS_MMC:dp->controller_number = desc->hwpart;break;default:Since this is checking the parent class, it seems to me that this info should be attacked there (the 'media' device) instead of the block device.
For MMC we have these layers:
- MMC controller - UCLASS_MMC
- MMC slot (or channel) - missing uclass
- hardware partition - UCLASS_BLK
See https://www.sage-micro.com/us/portfolio-items/s881/
Currently in the DM tree we don't model the MMC controller and the MMC slot separately. I think this is a gap that should be closed.
A single UCLASS_MMC (= MMC slot) device can have multiple UCLASS_BLK children. One for each hardware partition.
An MMC controller in EFI is modelled as VenHW() node, a slot as SD() or eMMC() node.
An eMMC() or SD() node has a field for the slot number but none for the hardware partition. Hence, we cannot attach the hardware partition information to the UCLASS_MMC device. For the UCLASS_BLK device I use a Ctrl() node. We could also consider using a Unit() node but Unit() nodes are meant to be used for LUNs.
We have a similar situation for USB and SCSI LUNs.
The current model is to add multiple BLK children for each media device, if needed. That seems to work OK for now. What do you think?
There is nothing in DM tree we need to change now. As I understood your first reply it was not clear to you which EFI device-path node type with which information matches which DM tree node:
MMC controller/MMC slot (UCLASS_MMC) -> SD(slot) or MMC(slot) Hardware partition (UCLASS_BLK) -> Ctrl()
SCSI drive (UCLASS_SCSI) -> VenHw() SCSI LUN (UClASS_BLK) -> Scsi(PUN, LUN)
OK, so this is explaining is why you must attach the functions to the BLK device rather than the storage device? If so, I understand.
What are VenHw and Ctrl ?
Regrads, Simon
Generate a Usb() node for USB hub devices.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- common/usb_hub.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)
diff --git a/common/usb_hub.c b/common/usb_hub.c index 85c0822d8b..ccf9e16023 100644 --- a/common/usb_hub.c +++ b/common/usb_hub.c @@ -24,6 +24,7 @@ #include <common.h> #include <command.h> #include <dm.h> +#include <efi_loader.h> #include <env.h> #include <errno.h> #include <log.h> @@ -939,6 +940,35 @@ static int usb_hub_post_probe(struct udevice *dev) return usb_hub_scan(dev); }
+#if CONFIG_IS_ENABLED(EFI_LOADER) +struct efi_device_path *usb_hub_get_dp_node(struct udevice *dev) +{ + struct efi_device_path_usb *dp; + + dp = efi_alloc(sizeof(struct efi_device_path_usb)); + if (!dp) + return NULL; + + switch (device_get_uclass_id(dev->parent)) { + case UCLASS_USB_HUB: { + struct usb_device *udev = dev_get_parent_priv(dev); + + dp->parent_port_number = udev->portnr; + break; + } + default: + dp->parent_port_number = 0; + } + + dp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE; + dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MSG_USB; + dp->dp.length = sizeof(*dp); + dp->usb_interface = 0; + + return &dp->dp; +} +#endif + static const struct udevice_id usb_hub_ids[] = { { .compatible = "usb-hub" }, { } @@ -960,6 +990,9 @@ UCLASS_DRIVER(usb_hub) = { .per_child_auto = sizeof(struct usb_device), .per_child_plat_auto = sizeof(struct usb_dev_plat), .per_device_auto = sizeof(struct usb_hub_device), +#if CONFIG_IS_ENABLED(EFI_LOADER) + .get_dp_node = usb_hub_get_dp_node, +#endif };
static const struct usb_device_id hub_id_table[] = {
Generate a Usb() node for USB mass storage devices.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- common/usb_storage.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)
diff --git a/common/usb_storage.c b/common/usb_storage.c index ac64275773..03bc136156 100644 --- a/common/usb_storage.c +++ b/common/usb_storage.c @@ -37,6 +37,7 @@ #include <bootdev.h> #include <command.h> #include <dm.h> +#include <efi_loader.h> #include <errno.h> #include <log.h> #include <mapmem.h> @@ -1534,6 +1535,35 @@ static int usb_mass_storage_probe(struct udevice *dev) return ret; }
+#if CONFIG_IS_ENABLED(EFI_LOADER) +struct efi_device_path *usb_storage_get_dp_node(struct udevice *dev) +{ + struct efi_device_path_usb *dp; + + dp = efi_alloc(sizeof(struct efi_device_path_usb)); + if (!dp) + return NULL; + + switch (device_get_uclass_id(dev->parent)) { + case UCLASS_USB_HUB: { + struct usb_device *udev = dev_get_parent_priv(dev); + + dp->parent_port_number = udev->portnr; + break; + } + default: + dp->parent_port_number = 0; + } + + dp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE; + dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MSG_USB; + dp->dp.length = sizeof(*dp); + dp->usb_interface = 0; + + return &dp->dp; +} +#endif + static const struct udevice_id usb_mass_storage_ids[] = { { .compatible = "usb-mass-storage" }, { } @@ -1552,6 +1582,9 @@ U_BOOT_DRIVER(usb_mass_storage) = { UCLASS_DRIVER(usb_mass_storage) = { .id = UCLASS_MASS_STORAGE, .name = "usb_mass_storage", +#if CONFIG_IS_ENABLED(EFI_LOADER) + .get_dp_node = usb_storage_get_dp_node, +#endif };
static const struct usb_device_id mass_storage_id_table[] = {
Generate an SD() or eMMC() node for MMC devices.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- drivers/mmc/mmc-uclass.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)
diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c index 01d9b0201f..4f85d80273 100644 --- a/drivers/mmc/mmc-uclass.c +++ b/drivers/mmc/mmc-uclass.c @@ -9,6 +9,7 @@
#include <common.h> #include <bootdev.h> +#include <efi_loader.h> #include <log.h> #include <mmc.h> #include <dm.h> @@ -505,6 +506,27 @@ static int mmc_blk_probe(struct udevice *dev) return 0; }
+#if CONFIG_IS_ENABLED(EFI_LOADER) +struct efi_device_path *mmc_get_dp_node(struct udevice *dev) +{ + struct efi_device_path_sd_mmc_path *dp; + struct mmc *mmc = mmc_get_mmc_dev(dev); + + dp = efi_alloc(sizeof(struct efi_device_path_sd_mmc_path)); + if (!dp) + return NULL; + + dp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE; + dp->dp.sub_type = IS_SD(mmc) ? + DEVICE_PATH_SUB_TYPE_MSG_SD : + DEVICE_PATH_SUB_TYPE_MSG_MMC; + dp->dp.length = sizeof(*dp); + dp->slot_number = dev_seq(dev); + + return &dp->dp; +} +#endif + #if CONFIG_IS_ENABLED(MMC_UHS_SUPPORT) || \ CONFIG_IS_ENABLED(MMC_HS200_SUPPORT) || \ CONFIG_IS_ENABLED(MMC_HS400_SUPPORT) @@ -547,4 +569,7 @@ UCLASS_DRIVER(mmc) = { .name = "mmc", .flags = DM_UC_FLAG_SEQ_ALIAS, .per_device_auto = sizeof(struct mmc_uclass_priv), +#if CONFIG_IS_ENABLED(EFI_LOADER) + .get_dp_node = mmc_get_dp_node, +#endif };
Use function uclass_get_dp_node() to construct device paths.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- lib/efi_loader/efi_device_path.c | 325 +++---------------------------- 1 file changed, 28 insertions(+), 297 deletions(-)
diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index d5cc495830..288baa1ca7 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -45,24 +45,6 @@ static const struct efi_device_path_vendor ROOT = { .guid = U_BOOT_GUID, };
-#if defined(CONFIG_MMC) -/* - * Determine if an MMC device is an SD card. - * - * @desc block device descriptor - * Return: true if the device is an SD card - */ -static bool is_sd(struct blk_desc *desc) -{ - struct mmc *mmc = find_mmc_device(desc->devnum); - - if (!mmc) - return false; - - return IS_SD(mmc) != 0U; -} -#endif - /* * Iterate to next block in device-path, terminating (returning NULL) * at /End* node. @@ -500,87 +482,24 @@ bool efi_dp_is_multi_instance(const struct efi_device_path *dp) /* size of device-path not including END node for device and all parents * up to the root device. */ -__maybe_unused static unsigned int dp_size(struct udevice *dev) +__maybe_unused static size_t dp_size(struct udevice *dev) { - if (!dev || !dev->driver) - return sizeof(ROOT); - - switch (device_get_uclass_id(dev)) { - case UCLASS_ROOT: - case UCLASS_SIMPLE_BUS: - /* stop traversing parents at this point: */ - return sizeof(ROOT); - case UCLASS_ETH: - return dp_size(dev->parent) + - sizeof(struct efi_device_path_mac_addr); - case UCLASS_BLK: - switch (dev->parent->uclass->uc_drv->id) { -#ifdef CONFIG_IDE - case UCLASS_IDE: - return dp_size(dev->parent) + - sizeof(struct efi_device_path_atapi); -#endif -#if defined(CONFIG_SCSI) - case UCLASS_SCSI: - return dp_size(dev->parent) + - sizeof(struct efi_device_path_scsi); -#endif -#if defined(CONFIG_MMC) - case UCLASS_MMC: - return dp_size(dev->parent) + - sizeof(struct efi_device_path_sd_mmc_path); -#endif -#if defined(CONFIG_AHCI) || defined(CONFIG_SATA) - case UCLASS_AHCI: - return dp_size(dev->parent) + - sizeof(struct efi_device_path_sata); -#endif -#if defined(CONFIG_NVME) - case UCLASS_NVME: - return dp_size(dev->parent) + - sizeof(struct efi_device_path_nvme); -#endif -#ifdef CONFIG_SANDBOX - case UCLASS_HOST: - /* - * Sandbox's host device will be represented - * as vendor device with extra one byte for - * device number - */ - return dp_size(dev->parent) - + sizeof(struct efi_device_path_vendor) + 1; -#endif -#ifdef CONFIG_USB - case UCLASS_MASS_STORAGE: - return dp_size(dev->parent) - + sizeof(struct efi_device_path_controller); -#endif -#ifdef CONFIG_VIRTIO_BLK - case UCLASS_VIRTIO: - /* - * Virtio devices will be represented as a vendor - * device node with an extra byte for the device - * number. - */ - return dp_size(dev->parent) - + sizeof(struct efi_device_path_vendor) + 1; -#endif - default: - return dp_size(dev->parent); - } -#if defined(CONFIG_MMC) - case UCLASS_MMC: - return dp_size(dev->parent) + - sizeof(struct efi_device_path_sd_mmc_path); -#endif - case UCLASS_MASS_STORAGE: - case UCLASS_USB_HUB: - return dp_size(dev->parent) + - sizeof(struct efi_device_path_usb); - default: - /* just skip over unknown classes: */ - return dp_size(dev->parent); + unsigned int size = 0; + struct efi_device_path *dp; + + if (!dev) + return 0; + + if (dev->parent) + size = dp_size(dev->parent); + + dp = uclass_get_dp_node(dev); + if (dp) { + size += dp->length; + efi_free_pool(dp); } + + return size; }
/* @@ -592,210 +511,22 @@ __maybe_unused static unsigned int dp_size(struct udevice *dev) */ __maybe_unused static void *dp_fill(void *buf, struct udevice *dev) { - if (!dev || !dev->driver) - return buf; + struct efi_device_path *dp;
- 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 = - dp_fill(buf, dev->parent); - struct eth_pdata *pdata = dev_get_plat(dev); - - dp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE; - dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MSG_MAC_ADDR; - dp->dp.length = sizeof(*dp); - memset(&dp->mac, 0, sizeof(dp->mac)); - /* We only support IPv4 */ - memcpy(&dp->mac, &pdata->enetaddr, ARP_HLEN); - /* Ethernet */ - dp->if_type = 1; - return &dp[1]; - } -#endif - case UCLASS_BLK: - switch (dev->parent->uclass->uc_drv->id) { -#ifdef CONFIG_SANDBOX - case UCLASS_HOST: { - /* stop traversing parents at this point: */ - struct efi_device_path_vendor *dp; - struct blk_desc *desc = dev_get_uclass_plat(dev); - - dp_fill(buf, dev->parent); - dp = buf; - ++dp; - dp->dp.type = DEVICE_PATH_TYPE_HARDWARE_DEVICE; - dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_VENDOR; - dp->dp.length = sizeof(*dp) + 1; - memcpy(&dp->guid, &efi_guid_host_dev, - sizeof(efi_guid_t)); - dp->vendor_data[0] = desc->devnum; - return &dp->vendor_data[1]; - } -#endif -#ifdef CONFIG_VIRTIO_BLK - case UCLASS_VIRTIO: { - struct efi_device_path_vendor *dp; - struct blk_desc *desc = dev_get_uclass_plat(dev); - - dp_fill(buf, dev->parent); - dp = buf; - ++dp; - dp->dp.type = DEVICE_PATH_TYPE_HARDWARE_DEVICE; - dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_VENDOR; - dp->dp.length = sizeof(*dp) + 1; - memcpy(&dp->guid, &efi_guid_virtio_dev, - sizeof(efi_guid_t)); - dp->vendor_data[0] = desc->devnum; - return &dp->vendor_data[1]; - } -#endif -#ifdef CONFIG_IDE - case UCLASS_IDE: { - struct efi_device_path_atapi *dp = - dp_fill(buf, dev->parent); - struct blk_desc *desc = dev_get_uclass_plat(dev); - - dp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE; - dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MSG_ATAPI; - dp->dp.length = sizeof(*dp); - dp->logical_unit_number = desc->devnum; - dp->primary_secondary = IDE_BUS(desc->devnum); - dp->slave_master = desc->devnum % - (CONFIG_SYS_IDE_MAXDEVICE / - CONFIG_SYS_IDE_MAXBUS); - return &dp[1]; - } -#endif -#if defined(CONFIG_SCSI) - case UCLASS_SCSI: { - struct efi_device_path_scsi *dp = - dp_fill(buf, dev->parent); - struct blk_desc *desc = dev_get_uclass_plat(dev); - - dp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE; - dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MSG_SCSI; - dp->dp.length = sizeof(*dp); - dp->logical_unit_number = desc->lun; - dp->target_id = desc->target; - return &dp[1]; - } -#endif -#if defined(CONFIG_MMC) - case UCLASS_MMC: { - struct efi_device_path_sd_mmc_path *sddp = - dp_fill(buf, dev->parent); - struct blk_desc *desc = dev_get_uclass_plat(dev); - - sddp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE; - sddp->dp.sub_type = is_sd(desc) ? - DEVICE_PATH_SUB_TYPE_MSG_SD : - DEVICE_PATH_SUB_TYPE_MSG_MMC; - sddp->dp.length = sizeof(*sddp); - sddp->slot_number = dev_seq(dev); - return &sddp[1]; - } -#endif -#if defined(CONFIG_AHCI) || defined(CONFIG_SATA) - case UCLASS_AHCI: { - struct efi_device_path_sata *dp = - dp_fill(buf, dev->parent); - struct blk_desc *desc = dev_get_uclass_plat(dev); - - dp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE; - dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MSG_SATA; - dp->dp.length = sizeof(*dp); - dp->hba_port = desc->devnum; - /* default 0xffff implies no port multiplier */ - dp->port_multiplier_port = 0xffff; - dp->logical_unit_number = desc->lun; - return &dp[1]; - } -#endif -#if defined(CONFIG_NVME) - case UCLASS_NVME: { - struct efi_device_path_nvme *dp = - dp_fill(buf, dev->parent); - u32 ns_id; - - dp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE; - dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MSG_NVME; - dp->dp.length = sizeof(*dp); - nvme_get_namespace_id(dev, &ns_id, dp->eui64); - memcpy(&dp->ns_id, &ns_id, sizeof(ns_id)); - return &dp[1]; - } -#endif -#if defined(CONFIG_USB) - case UCLASS_MASS_STORAGE: { - struct blk_desc *desc = desc = dev_get_uclass_plat(dev); - struct efi_device_path_controller *dp = - dp_fill(buf, dev->parent); - - dp->dp.type = DEVICE_PATH_TYPE_HARDWARE_DEVICE; - dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_CONTROLLER; - dp->dp.length = sizeof(*dp); - dp->controller_number = desc->lun; - return &dp[1]; - } -#endif - default: - debug("%s(%u) %s: unhandled parent class: %s (%u)\n", - __FILE__, __LINE__, __func__, - dev->name, dev->parent->uclass->uc_drv->id); - return dp_fill(buf, dev->parent); - } -#if defined(CONFIG_MMC) - case UCLASS_MMC: { - struct efi_device_path_sd_mmc_path *sddp = - dp_fill(buf, dev->parent); - struct mmc *mmc = mmc_get_mmc_dev(dev); - struct blk_desc *desc = mmc_get_blk_desc(mmc); - - sddp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE; - sddp->dp.sub_type = is_sd(desc) ? - DEVICE_PATH_SUB_TYPE_MSG_SD : - DEVICE_PATH_SUB_TYPE_MSG_MMC; - sddp->dp.length = sizeof(*sddp); - sddp->slot_number = dev_seq(dev); - - return &sddp[1]; - } -#endif - case UCLASS_MASS_STORAGE: - case UCLASS_USB_HUB: { - struct efi_device_path_usb *udp = dp_fill(buf, dev->parent); - - switch (device_get_uclass_id(dev->parent)) { - case UCLASS_USB_HUB: { - struct usb_device *udev = dev_get_parent_priv(dev); + if (!dev) + return buf;
- udp->parent_port_number = udev->portnr; - break; - } - default: - udp->parent_port_number = 0; - } - udp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE; - udp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MSG_USB; - udp->dp.length = sizeof(*udp); - udp->usb_interface = 0; + if (dev->parent) + buf = dp_fill(buf, dev->parent);
- 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); + dp = uclass_get_dp_node(dev); + if (dp) { + memcpy(buf, dp, dp->length); + buf += dp->length; + efi_free_pool(dp); } + + return buf; }
static unsigned dp_part_size(struct blk_desc *desc, int part)
Hi Heinrich,
On Mon, 27 Mar 2023 at 06:28, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
Use function uclass_get_dp_node() to construct device paths.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
lib/efi_loader/efi_device_path.c | 325 +++---------------------------- 1 file changed, 28 insertions(+), 297 deletions(-)
This is definitely a step in the right direction. Thank you for working on it.
[..]
dp = uclass_get_dp_node(dev);if (dp) {memcpy(buf, dp, dp->length);buf += dp->length;efi_free_pool(dp);
This suggests that the API should be:
int uclass_get_dp_node(struct xxx *dp)
with it writing the data to the provided pointer address and returning the number of bytes used. That would avoid the malloc() / free().
}
return buf;}
static unsigned dp_part_size(struct blk_desc *desc, int part)
2.39.2
Regards, Simon
participants (3)
-
Heinrich Schuchardt -
Ilias Apalodimas -
Simon Glass